Merge pull request #966 from sharelatex/as-no-project-token-based

Redirect logged out users to v1 if project is not found from token based url

services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee

@@ -12,16 +12,11 @@ module.exports = TokenAccessController =
 		return ProjectController.loadEditor(req, res, next)
 
 	_tryHigherAccess: (token, userId, req, res, next) ->
-		TokenAccessHandler.findProjectWithHigherAccess token, userId, (err, project, projectExists) ->
+		TokenAccessHandler.findProjectWithHigherAccess token, userId, (err, project) ->
 			if err?
 				logger.err {err, token, userId},
 					"[TokenAccess] error finding project with higher access"
 				return next(err)
-			if !projectExists and settings.overleaf
-				logger.log {token, userId},
-					"[TokenAccess] no project found for this token"
-				# Project does not exist, but may be unimported - try it on v1
-				return res.redirect(settings.overleaf.host + req.url)
 			if !project?
 				logger.log {token, userId},
 					"[TokenAccess] no project with higher access found for this user and token"
@@ -34,11 +29,15 @@ module.exports = TokenAccessController =
 		userId = AuthenticationController.getLoggedInUserId(req)
 		token = req.params['read_and_write_token']
 		logger.log {userId, token}, "[TokenAccess] requesting read-and-write token access"
-		TokenAccessHandler.findProjectWithReadAndWriteToken token, (err, project) ->
+		TokenAccessHandler.findProjectWithReadAndWriteToken token, (err, project, projectExists) ->
 			if err?
 				logger.err {err, token, userId},
 					"[TokenAccess] error getting project by readAndWrite token"
 				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?
 				logger.log {token, userId},
 					"[TokenAccess] no token-based project found for readAndWrite token"
@@ -77,11 +76,15 @@ module.exports = TokenAccessController =
 		userId = AuthenticationController.getLoggedInUserId(req)
 		token = req.params['read_only_token']
 		logger.log {userId, token}, "[TokenAccess] requesting read-only token access"
-		TokenAccessHandler.findProjectWithReadOnlyToken token, (err, project) ->
+		TokenAccessHandler.findProjectWithReadOnlyToken token, (err, project, projectExists) ->
 			if err?
 				logger.err {err, token, userId},
 					"[TokenAccess] error getting project by readOnly token"
 				return next(err)
+			if !projectExists and settings.overleaf
+				logger.log {token, userId},
+						"[TokenAccess] no project found for this token"
+				return res.redirect(302, settings.overleaf.host + '/read/' + token)
 			if !project?
 				logger.log {token, userId},
 					"[TokenAccess] no project found for readOnly token"

services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee

@@ -10,19 +10,31 @@ module.exports = TokenAccessHandler =
 	ANONYMOUS_READ_AND_WRITE_ENABLED:
 		Settings.allowAnonymousReadAndWriteSharing == true
 
-	findProjectWithReadOnlyToken: (token, callback=(err, project)->) ->
+	findProjectWithReadOnlyToken: (token, callback=(err, project, projectExists)->) ->
 		Project.findOne {
-			'tokens.readOnly': token,
+			'tokens.readOnly': token
-			'publicAccesLevel': PublicAccessLevels.TOKEN_BASED
+		}, {_id: 1, publicAccesLevel: 1, owner_ref: 1}, (err, project) ->
-		}, {_id: 1, publicAccesLevel: 1, owner_ref: 1}, callback
+			if err?
+				return callback(err)
+			if !project?
+				return callback(null, null, false) # Project doesn't exist, so we handle differently
+			if project.publicAccesLevel != PublicAccessLevels.TOKEN_BASED
+				return callback(null, null, true) # Project does exist, but it isn't token based
+			return callback(null, project, true)
 
-	findProjectWithReadAndWriteToken: (token, callback=(err, project)->) ->
+	findProjectWithReadAndWriteToken: (token, callback=(err, project, projectExists)->) ->
 		Project.findOne {
-			'tokens.readAndWrite': token,
+			'tokens.readAndWrite': token
-			'publicAccesLevel': PublicAccessLevels.TOKEN_BASED
+		}, {_id: 1, publicAccesLevel: 1, owner_ref: 1}, (err, project) ->
-		}, {_id: 1, publicAccesLevel: 1, owner_ref: 1}, callback
+			if err?
+				return callback(err)
+			if !project?
+				return callback(null, null, false) # Project doesn't exist, so we handle differently
+			if project.publicAccesLevel != PublicAccessLevels.TOKEN_BASED
+				return callback(null, null, true) # Project does exist, but it isn't token based
+			return callback(null, project, true)
 
-	findProjectWithHigherAccess: (token, userId, callback=(err, project, projectExists)->) ->
+	findProjectWithHigherAccess: (token, userId, callback=(err, project)->) ->
 		Project.findOne {
 			$or: [
 				{'tokens.readAndWrite': token},
@@ -32,15 +44,14 @@ module.exports = TokenAccessHandler =
 			if err?
 				return callback(err)
 			if !project?
-				return callback(null, null, false) # Project doesn't exist, so we handle differently
+				return callback(null, null)
 			projectId = project._id
 			CollaboratorsHandler.isUserInvitedMemberOfProject userId, projectId, (err, isMember) ->
 				if err?
 					return callback(err)
 				callback(
 					null,
-					if isMember == true then project else null,
+					if isMember == true then project else null
-					true # Project does exist, but user doesn't have access
 				)
 
 	addReadOnlyUserToProject: (userId, projectId, callback=(err)->) ->

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

@@ -417,11 +417,20 @@ describe 'TokenAccess', ->
 			, done)
 
 	describe 'unimported v1 project', ->
-		it 'should redirect to v1', (done) ->
+		it 'should redirect read and write token to v1', (done) ->
 			unimportedV1Token = '123abc'
 			try_read_and_write_token_access(@owner, unimportedV1Token, (response, body) =>
 				expect(response.statusCode).to.equal 302
 				expect(response.headers.location).to.equal(
-					'http://overleaf.test:5000/123abc'
+					'/sign_in_to_v1?return_to=/123abc'
+				)
+			, done)
+
+		it 'should redirect read only token to v1', (done) ->
+			unimportedV1Token = 'abcd'
+			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'
 				)
 			, done)

services/web/test/unit/coffee/TokenAccess/TokenAccessControllerTests.coffee

@@ -48,7 +48,7 @@ describe "TokenAccessController", ->
 				@next = sinon.stub()
 				@req.params['read_and_write_token'] = @readAndWriteToken
 				@TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub()
-					.callsArgWith(1, null, @project)
+					.callsArgWith(1, null, @project, true)
 				@TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub()
 					.callsArgWith(2, null)
 				@ProjectController.loadEditor = sinon.stub()
@@ -85,7 +85,7 @@ describe "TokenAccessController", ->
 				@req.params['read_and_write_token'] = @readAndWriteToken
 				@project.owner_ref = @userId
 				@TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub()
-					.callsArgWith(1, null, @project)
+					.callsArgWith(1, null, @project, true)
 				@TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub()
 					.callsArgWith(2, null)
 				@ProjectController.loadEditor = sinon.stub()
@@ -123,7 +123,7 @@ describe "TokenAccessController", ->
 					@next = sinon.stub()
 					@req.params['read_and_write_token'] = @readAndWriteToken
 					@TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub()
-						.callsArgWith(1, null, @project)
+						.callsArgWith(1, null, @project, true)
 					@TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub()
 						.callsArgWith(2, null)
 					@ProjectController.loadEditor = sinon.stub()
@@ -159,7 +159,7 @@ describe "TokenAccessController", ->
 					@next = sinon.stub()
 					@req.params['read_and_write_token'] = @readAndWriteToken
 					@TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub()
-						.callsArgWith(1, null, @project)
+						.callsArgWith(1, null, @project, true)
 					@TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub()
 						.callsArgWith(2, null)
 					@ProjectController.loadEditor = sinon.stub()
@@ -244,16 +244,18 @@ describe "TokenAccessController", ->
 						@next = sinon.stub()
 						@req.params['read_and_write_token'] = '123abc'
 						@TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub()
-							.callsArgWith(1, null, null)
+							.callsArgWith(1, null, null, false)
 						@TokenAccessHandler.findProjectWithHigherAccess =
 							sinon.stub()
-							.callsArgWith(2, null, @project, false)
+							.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.firstCall.args[0])
+						expect(@res.redirect.calledWith(
-							.to.equal 'http://overleaf.test:5000/123abc'
+							302,
+							'/sign_in_to_v1?return_to=/123abc'
+						)).to.equal true
 						done()
 
 				describe 'when token access is off, but user has higher access anyway', ->
@@ -264,10 +266,10 @@ describe "TokenAccessController", ->
 						@next = sinon.stub()
 						@req.params['read_and_write_token'] = @readAndWriteToken
 						@TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub()
-							.callsArgWith(1, null, null)
+							.callsArgWith(1, null, null, true)
 						@TokenAccessHandler.findProjectWithHigherAccess =
 							sinon.stub()
-							.callsArgWith(2, null, @project, true)
+							.callsArgWith(2, null, @project)
 						@TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub()
 							.callsArgWith(2, null)
 						@ProjectController.loadEditor = sinon.stub()
@@ -313,10 +315,10 @@ describe "TokenAccessController", ->
 						@next = sinon.stub()
 						@req.params['read_and_write_token'] = @readAndWriteToken
 						@TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub()
-							.callsArgWith(1, null, null)
+							.callsArgWith(1, null, null, true)
 						@TokenAccessHandler.findProjectWithHigherAccess =
 							sinon.stub()
-							.callsArgWith(2, null, null, true)
+							.callsArgWith(2, null, null)
 						@TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub()
 							.callsArgWith(2, null)
 						@ProjectController.loadEditor = sinon.stub()
@@ -358,7 +360,7 @@ describe "TokenAccessController", ->
 				@next = sinon.stub()
 				@req.params['read_and_write_token'] = @readAndWriteToken
 				@TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub()
-					.callsArgWith(1, null, @project)
+					.callsArgWith(1, null, @project, true)
 				@TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub()
 					.callsArgWith(2, new Error('woops'))
 				@ProjectController.loadEditor = sinon.stub()
@@ -405,7 +407,7 @@ describe "TokenAccessController", ->
 					@next = sinon.stub()
 					@req.params['read_only_token'] = @readOnlyToken
 					@TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub()
-						.callsArgWith(1, null, @project)
+						.callsArgWith(1, null, @project, true)
 					@TokenAccessHandler.addReadOnlyUserToProject = sinon.stub()
 						.callsArgWith(2, null)
 					@ProjectController.loadEditor = sinon.stub()
@@ -441,7 +443,7 @@ describe "TokenAccessController", ->
 					@req.params['read_only_token'] = @readOnlyToken
 					@project.owner_ref = @userId
 					@TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub()
-						.callsArgWith(1, null, @project)
+						.callsArgWith(1, null, @project, true)
 					@TokenAccessHandler.addReadOnlyUserToProject = sinon.stub()
 						.callsArgWith(2, null)
 					@ProjectController.loadEditor = sinon.stub()
@@ -500,31 +502,7 @@ describe "TokenAccessController", ->
 					expect(@next.lastCall.args[0]).to.be.instanceof Error
 					done()
 
-			##
 		describe 'when findProject does not find a project', ->
-			beforeEach ->
-
-			describe 'when project does not exist', ->
-				beforeEach ->
-					@req = new MockRequest()
-					@req.url = '/123abc'
-					@res = new MockResponse()
-					@res.redirect = sinon.stub()
-					@next = sinon.stub()
-					@req.params['read_and_write_token'] = '123abc'
-					@TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub()
-						.callsArgWith(1, null, null)
-					@TokenAccessHandler.findProjectWithHigherAccess =
-						sinon.stub()
-						.callsArgWith(2, null, @project, false)
-					@TokenAccessController.readOnlyToken @req, @res, @next
-
-				it 'should return a ProjectNotTokenAccessError', (done) ->
-					expect(@res.redirect.callCount).to.equal 1
-					expect(@res.redirect.firstCall.args[0])
-						.to.equal 'http://overleaf.test:5000/123abc'
-					done()
-
 			describe 'when token access is off, but user has higher access anyway', ->
 				beforeEach ->
 					@req = new MockRequest()
@@ -533,10 +511,10 @@ describe "TokenAccessController", ->
 					@next = sinon.stub()
 					@req.params['read_and_write_token'] = @readAndWriteToken
 					@TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub()
-						.callsArgWith(1, null, null)
+						.callsArgWith(1, null, null, true)
 					@TokenAccessHandler.findProjectWithHigherAccess =
 						sinon.stub()
-						.callsArgWith(2, null, @project, true)
+						.callsArgWith(2, null, @project)
 					@TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub()
 						.callsArgWith(2, null)
 					@ProjectController.loadEditor = sinon.stub()
@@ -581,10 +559,10 @@ describe "TokenAccessController", ->
 					@next = sinon.stub()
 					@req.params['read_and_write_token'] = @readAndWriteToken
 					@TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub()
-						.callsArgWith(1, null, null)
+						.callsArgWith(1, null, null, true)
 					@TokenAccessHandler.findProjectWithHigherAccess =
 						sinon.stub()
-						.callsArgWith(2, null, null, true)
+						.callsArgWith(2, null, null)
 					@TokenAccessHandler.addReadOnlyUserToProject = sinon.stub()
 						.callsArgWith(2, null)
 					@ProjectController.loadEditor = sinon.stub()
@@ -626,7 +604,7 @@ describe "TokenAccessController", ->
 				@next = sinon.stub()
 				@req.params['read_only_token'] = @readOnlyToken
 				@TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub()
-					.callsArgWith(1, null, @project)
+					.callsArgWith(1, null, @project, true)
 				@TokenAccessHandler.addReadOnlyUserToProject = sinon.stub()
 					.callsArgWith(2, new Error('woops'))
 				@ProjectController.loadEditor = sinon.stub()
@@ -670,7 +648,7 @@ describe "TokenAccessController", ->
 					@next = sinon.stub()
 					@req.params['read_only_token'] = @readOnlyToken
 					@TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub()
-						.callsArgWith(1, null, @project)
+						.callsArgWith(1, null, @project, true)
 					@TokenAccessHandler.addReadOnlyUserToProject = sinon.stub()
 						.callsArgWith(2, null)
 					@ProjectController.loadEditor = sinon.stub()
@@ -748,6 +726,7 @@ describe "TokenAccessController", ->
 				beforeEach ->
 					@req = new MockRequest()
 					@res = new MockResponse()
+					@res.redirect = sinon.stub()
 					@next = sinon.stub()
 					@req.params['read_only_token'] = @readOnlyToken
 					@TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub()
@@ -779,8 +758,11 @@ describe "TokenAccessController", ->
 						.to.equal 0
 					done()
 
-				it 'should call next with a not-found error', (done) ->
+				it 'should redirect to v1', (done) ->
-					expect(@next.callCount).to.equal 1
+					expect(@res.redirect.callCount).to.equal 1
-					expect(@next.lastCall.args[0]).to.be.instanceof Error
+					expect(@res.redirect.calledWith(
+						302,
+						"http://overleaf.test:5000/read/#{@readOnlyToken}"
+					)).to.equal true
 					done()
 

services/web/test/unit/coffee/TokenAccess/TokenAccessHandlerTests.coffee

@@ -31,8 +31,7 @@ describe "TokenAccessHandler", ->
 			@TokenAccessHandler.findProjectWithReadOnlyToken @token, (err, project) =>
 				expect(@Project.findOne.callCount).to.equal 1
 				expect(@Project.findOne.calledWith({
-					'tokens.readOnly': @token,
+					'tokens.readOnly': @token
-					'publicAccesLevel': 'tokenBased'
 				})).to.equal true
 				done()
 
@@ -43,6 +42,11 @@ describe "TokenAccessHandler", ->
 				expect(project).to.deep.equal @project
 				done()
 
+		it 'should return projectExists flag as true', (done) ->
+			@TokenAccessHandler.findProjectWithReadOnlyToken @token, (err, project, projectExists) ->
+				expect(projectExists).to.equal true
+				done()
+
 		describe 'when Project.findOne produces an error', ->
 			beforeEach ->
 				@Project.findOne = sinon.stub().callsArgWith(2, new Error('woops'))
@@ -54,6 +58,37 @@ describe "TokenAccessHandler", ->
 					expect(err).to.be.instanceof Error
 					done()
 
+		describe 'when project does not have tokenBased access level', ->
+			beforeEach ->
+				@project.publicAccesLevel = 'private'
+				@Project.findOne = sinon.stub().callsArgWith(2, null, @project, true)
+
+			it 'should not return a project', (done) ->
+				@TokenAccessHandler.findProjectWithReadOnlyToken @token, (err, project) ->
+					expect(err).to.not.exist
+					expect(project).to.not.exist
+					done()
+
+			it 'should return projectExists flag as true', (done) ->
+				@TokenAccessHandler.findProjectWithReadOnlyToken @token, (err, project, projectExists) ->
+					expect(projectExists).to.equal true
+					done()
+
+		describe 'when project does not exist', ->
+			beforeEach ->
+				@Project.findOne = sinon.stub().callsArgWith(2, null, null)
+
+			it 'should not return a project', (done) ->
+				@TokenAccessHandler.findProjectWithReadOnlyToken @token, (err, project) ->
+					expect(err).to.not.exist
+					expect(project).to.not.exist
+					done()
+
+			it 'should return projectExists flag as false', (done) ->
+				@TokenAccessHandler.findProjectWithReadOnlyToken @token, (err, project, projectExists) ->
+					expect(projectExists).to.equal false
+					done()
+
 	describe 'findProjectWithReadAndWriteToken', ->
 		beforeEach ->
 			@Project.findOne = sinon.stub().callsArgWith(2, null, @project)
@@ -62,8 +97,7 @@ describe "TokenAccessHandler", ->
 			@TokenAccessHandler.findProjectWithReadAndWriteToken @token, (err, project) =>
 				expect(@Project.findOne.callCount).to.equal 1
 				expect(@Project.findOne.calledWith({
-					'tokens.readAndWrite': @token,
+					'tokens.readAndWrite': @token
-					'publicAccesLevel': 'tokenBased'
 				})).to.equal true
 				done()
 
@@ -74,6 +108,11 @@ describe "TokenAccessHandler", ->
 				expect(project).to.deep.equal @project
 				done()
 
+		it 'should return projectExists flag as true', (done) ->
+			@TokenAccessHandler.findProjectWithReadAndWriteToken @token, (err, project, projectExists) ->
+				expect(projectExists).to.equal true
+				done()
+
 		describe 'when Project.findOne produces an error', ->
 			beforeEach ->
 				@Project.findOne = sinon.stub().callsArgWith(2, new Error('woops'))
@@ -85,6 +124,22 @@ describe "TokenAccessHandler", ->
 					expect(err).to.be.instanceof Error
 					done()
 
+		describe 'when project does not have tokenBased access level', ->
+			beforeEach ->
+				@project.publicAccesLevel = 'private'
+				@Project.findOne = sinon.stub().callsArgWith(2, null, @project, true)
+
+			it 'should not return a project', (done) ->
+				@TokenAccessHandler.findProjectWithReadAndWriteToken @token, (err, project) ->
+					expect(err).to.not.exist
+					expect(project).to.not.exist
+					done()
+
+			it 'should return projectExists flag as true', (done) ->
+				@TokenAccessHandler.findProjectWithReadAndWriteToken @token, (err, project, projectExists) ->
+					expect(projectExists).to.equal true
+					done()
+
 
 	describe 'findProjectWithHigherAccess', ->
 		describe 'when user does have higher access', ->