From 435fe11115c137247bdc08ec6a36bb46beb8c88f Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 27 Sep 2018 17:38:35 +0100 Subject: [PATCH] Check if v1 project was exported if not found This prevents a redirect loop for projects which were exported but then deleted on v2. v2 would not find the project, redirect to v1, which would find that it was exported and redirect back to v2. --- .../TokenAccess/TokenAccessController.coffee | 10 ++++-- .../TokenAccess/TokenAccessHandler.coffee | 6 ++++ .../coffee/helpers/MockV1Api.coffee | 3 ++ .../TokenAccessControllerTests.coffee | 34 +++++++++++++------ 4 files changed, 39 insertions(+), 14 deletions(-) diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee index e335b19181..c6123cecf6 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee +++ b/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee @@ -1,6 +1,7 @@ ProjectController = require "../Project/ProjectController" AuthenticationController = require '../Authentication/AuthenticationController' TokenAccessHandler = require './TokenAccessHandler' +V1Api = require '../V1/V1Api' Errors = require '../Errors/Errors' logger = require 'logger-sharelatex' settings = require 'settings-sharelatex' @@ -36,9 +37,12 @@ module.exports = TokenAccessController = return next(err) if !projectExists and settings.overleaf logger.log {token, userId}, - "[TokenAccess] no project found for this token" - return res.redirect(302, "/sign_in_to_v1?return_to=/#{token}") - if !project? + "[TokenAccess] no project found for this token" + TokenAccessHandler.checkV1ProjectExported token, (err, exported) -> + return next err if err? + return next(new Errors.NotFoundError()) if exported + return res.redirect(302, "/sign_in_to_v1?return_to=/#{token}") + else if !project? logger.log {token, userId}, "[TokenAccess] no token-based project found for readAndWrite token" if !userId? diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee index 8d9825da2c..100786f776 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee +++ b/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee @@ -116,3 +116,9 @@ module.exports = TokenAccessHandler = return callback err if err? callback null, false, body.published_path if body.allow == false callback null, true + + checkV1ProjectExported: (token, callback = (err, exists) ->) -> + return callback(null, false) unless Settings.apis?.v1? + V1Api.request { url: "/api/v1/sharelatex/docs/#{token}/exported_to_v2" }, (err, response, body) -> + return callback err if err? + callback null, body.exported diff --git a/services/web/test/acceptance/coffee/helpers/MockV1Api.coffee b/services/web/test/acceptance/coffee/helpers/MockV1Api.coffee index f9a7aed451..fcaf7a80df 100644 --- a/services/web/test/acceptance/coffee/helpers/MockV1Api.coffee +++ b/services/web/test/acceptance/coffee/helpers/MockV1Api.coffee @@ -85,4 +85,7 @@ module.exports = MockV1Api = app.get '/api/v1/sharelatex/docs/:token/is_published', (req, res, next) => res.json { allow: true } + app.get '/api/v1/sharelatex/docs/:token/exported_to_v2', (req, res, next) => + res.json { exported: false } + MockV1Api.run() diff --git a/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee b/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee index aa2e8c4ede..09123b5939 100644 --- a/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee +++ b/services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee @@ -248,18 +248,30 @@ describe "TokenAccessController", -> @req.params['read_and_write_token'] = '123abc' @TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub() .callsArgWith(1, null, null, false) - @TokenAccessHandler.findProjectWithHigherAccess = - sinon.stub() - .callsArgWith(2, null, @project) - @TokenAccessController.readAndWriteToken @req, @res, @next - it 'should redirect to v1', (done) -> - expect(@res.redirect.callCount).to.equal 1 - expect(@res.redirect.calledWith( - 302, - '/sign_in_to_v1?return_to=/123abc' - )).to.equal true - done() + describe 'when project was not exported from v1', -> + beforeEach -> + @TokenAccessHandler.checkV1ProjectExported = sinon.stub() + .callsArgWith(1, null, false) + @TokenAccessController.readAndWriteToken @req, @res, @next + + it 'should redirect to v1', (done) -> + expect(@res.redirect.callCount).to.equal 1 + expect(@res.redirect.calledWith( + 302, + '/sign_in_to_v1?return_to=/123abc' + )).to.equal true + done() + + describe 'when project was exported from v1', -> + beforeEach -> + @TokenAccessHandler.checkV1ProjectExported = sinon.stub() + .callsArgWith(1, null, false) + @TokenAccessController.readAndWriteToken @req, @res, @next + + it 'should call next with a not-found error', (done) -> + expect(@next.callCount).to.equal 0 + done() describe 'when token access is off, but user has higher access anyway', -> beforeEach ->