Merge pull request #981 from sharelatex/as-fix-intelligent-redirect-loop

Prevent redirect loop if project was imported to v2 then deleted

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"
+					"[TokenAccess] no project found for this token"
-				return res.redirect(302, "/sign_in_to_v1?return_to=/#{token}")
+				TokenAccessHandler.checkV1ProjectExported token, (err, exported) ->
-			if !project?
+					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?
@@ -83,9 +87,12 @@ module.exports = TokenAccessController =
 				return next(err)
 			if !projectExists and settings.overleaf
 				logger.log {token, userId},
-						"[TokenAccess] no project found for this token"
+					"[TokenAccess] no project found for this token"
-				return res.redirect(302, settings.overleaf.host + '/read/' + token)
+				TokenAccessHandler.checkV1ProjectExported token, (err, exported) ->
-			if !project?
+					return next err if err?
+					return next(new Errors.NotFoundError()) if exported
+					return res.redirect(302, "/sign_in_to_v1?return_to=/read/#{token}")
+			else if !project?
 				logger.log {token, userId},
 					"[TokenAccess] no project found for readOnly token"
 				if !userId?

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

services/web/test/acceptance/coffee/TokenAccessTests.coffee

@@ -431,6 +431,6 @@ describe 'TokenAccess', ->
 			try_read_only_token_access(@owner, unimportedV1Token, (response, body) =>
 				expect(response.statusCode).to.equal 302
 				expect(response.headers.location).to.equal(
-					'http://overleaf.test:5000/read/abcd'
+					'/sign_in_to_v1?return_to=/read/abcd'
 				)
 			, done)

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()

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) ->
+					describe 'when project was not exported from v1', ->
-						expect(@res.redirect.callCount).to.equal 1
+						beforeEach ->
-						expect(@res.redirect.calledWith(
+							@TokenAccessHandler.checkV1ProjectExported = sinon.stub()
-							302,
+								.callsArgWith(1, null, false)
-							'/sign_in_to_v1?return_to=/123abc'
+							@TokenAccessController.readAndWriteToken @req, @res, @next
-						)).to.equal true
+
-						done()
+						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 ->
@@ -521,6 +533,44 @@ describe "TokenAccessController", ->
 					done()
 
 		describe 'when findProject does not find a project', ->
+			describe 'when project does not exist', ->
+				beforeEach ->
+					@req = new MockRequest()
+					@res = new MockResponse()
+					@res.redirect = sinon.stub()
+					@next = sinon.stub()
+					@req.params['read_only_token'] = 'abcd'
+					@TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub()
+						.callsArgWith(1, null, null, false)
+					@TokenAccessHandler.checkV1ProjectExported = sinon.stub()
+						.callsArgWith(1, null, false)
+					@TokenAccessController.readOnlyToken @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=/read/abcd'
+					)).to.equal true
+					done()
+
+			describe 'when project was exported from v1', ->
+				beforeEach ->
+					@req = new MockRequest()
+					@res = new MockResponse()
+					@res.redirect = sinon.stub()
+					@next = sinon.stub()
+					@req.params['read_only_token'] = 'abcd'
+					@TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub()
+						.callsArgWith(1, null, null, false)
+					@TokenAccessHandler.checkV1ProjectExported = sinon.stub()
+						.callsArgWith(1, null, true)
+					@TokenAccessController.readOnlyToken @req, @res, @next
+
+				it 'should call next with a not-found error', (done) ->
+					expect(@next.callCount).to.equal 1
+					done()
+
 			describe 'when token access is off, but user has higher access anyway', ->
 				beforeEach ->
 					@req = new MockRequest()
@@ -749,6 +799,8 @@ describe "TokenAccessController", ->
 					@req.params['read_only_token'] = @readOnlyToken
 					@TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub()
 						.callsArgWith(1, null, null)
+					@TokenAccessHandler.checkV1ProjectExported = sinon.stub()
+						.callsArgWith(1, null, false)
 					@TokenAccessHandler.addReadOnlyUserToProject = sinon.stub()
 						.callsArgWith(2, null)
 					@ProjectController.loadEditor = sinon.stub()
@@ -776,11 +828,17 @@ describe "TokenAccessController", ->
 						.to.equal 0
 					done()
 
-				it 'should redirect to v1', (done) ->
+				describe 'when project was exported to v2', ->
-					expect(@res.redirect.callCount).to.equal 1
+					beforeEach ->
-					expect(@res.redirect.calledWith(
+						@TokenAccessHandler.checkV1ProjectExported = sinon.stub()
-						302,
+							.callsArgWith(1, null, true)
-						"http://overleaf.test:5000/read/#{@readOnlyToken}"
+						@TokenAccessController.readOnlyToken @req, @res, @next
-					)).to.equal true
+
-					done()
+					it 'should redirect to v1', (done) ->
+						expect(@res.redirect.callCount).to.equal 1
+						expect(@res.redirect.calledWith(
+							302,
+							"/sign_in_to_v1?return_to=/read/#{@readOnlyToken}"
+						)).to.equal true
+						done()