Generalise the higher-access logic for read-write token path

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

@@ -22,23 +22,23 @@ module.exports = TokenAccessController =
 				return next(err)
 			if !project?
 				logger.log {token, userId},
-					"[TokenAccess] no project found for readAndWrite token"
+					"[TokenAccess] no token-based project found for readAndWrite token"
 				if !userId?
 					logger.log {token},
 						"[TokenAccess] No project found with read-write token, anonymous user"
 					return next(new Errors.NotFoundError())
-				TokenAccessHandler
-					.findPrivateOverleafProjectWithReadAndWriteToken token, (err, project) ->
-						if err?
-							logger.err {err, token, userId},
-								"[TokenAccess] error getting project by readAndWrite token"
-							return next(err)
-						if !project?
-							logger.log {token, userId},
-								"[TokenAccess] no private-overleaf project found with readAndWriteToken"
-							return next(new Errors.NotFoundError())
-						logger.log {token, projectId: project._id}, "[TokenAccess] redirecting user to project"
-						res.redirect(302, "/project/#{project._id}")
+				TokenAccessHandler.findProjectWithHigherAccess token, userId, (err, project) ->
+					if err?
+						logger.err {err, token, userId},
+							"[TokenAccess] error finding project with higher access"
+						return next(err)
+					if !project?
+						logger.log {token, userId},
+							"[TokenAccess] no project with higher access found for token and user"
+						return next(new Errors.NotFoundError())
+					logger.log {token, userId, projectId: project._id},
+						"[TokenAccess] user has higher access to project, redirecting"
+					res.redirect(302, "/project/#{project._id}")
 			else
 				if !userId?
 					if TokenAccessHandler.ANONYMOUS_READ_AND_WRITE_ENABLED

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

@@ -1,4 +1,5 @@
 Project = require('../../models/Project').Project
+CollaboratorsHandler = require('../Collaborators/CollaboratorsHandler')
 PublicAccessLevels = require '../Authorization/PublicAccessLevels'
 PrivilegeLevels = require '../Authorization/PrivilegeLevels'
 ObjectId = require("mongojs").ObjectId
@@ -21,12 +22,22 @@ module.exports = TokenAccessHandler =
 			'publicAccesLevel': PublicAccessLevels.TOKEN_BASED
 		}, {_id: 1, publicAccesLevel: 1, owner_ref: 1}, callback
 
-	findPrivateOverleafProjectWithReadAndWriteToken: (token, callback=(err, project)->) ->
+	findProjectWithHigherAccess: (token, userId, callback=(err, project)->) ->
 		Project.findOne {
-			'tokens.readAndWrite': token,
-			'publicAccesLevel': PublicAccessLevels.PRIVATE,
-			'overleaf.id': {'$exists': true}
-		}, {_id: 1, publicAccesLevel: 1, owner_ref: 1}, callback
+			$or: [
+				{'tokens.readAndWrite': token},
+				{'tokens.readOnly': token}
+			]
+		}, {_id: 1}, (err, project) ->
+			if err?
+				return callback(err)
+			if !project?
+				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)
 
 	addReadOnlyUserToProject: (userId, projectId, callback=(err)->) ->
 		userId = ObjectId(userId.toString())

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

@@ -142,7 +142,7 @@ describe "TokenAccessController", ->
 		describe 'when findProject does not find a project', ->
 			beforeEach ->
 
-			describe 'when it is a private overleaf project', ->
+			describe 'when token access is off, but user has higher access anyway', ->
 				beforeEach ->
 					@req = new MockRequest()
 					@res = new MockResponse()
@@ -151,9 +151,9 @@ describe "TokenAccessController", ->
 					@req.params['read_and_write_token'] = @readAndWriteToken
 					@TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub()
 						.callsArgWith(1, null, null)
-					@TokenAccessHandler.findPrivateOverleafProjectWithReadAndWriteToken =
+					@TokenAccessHandler.findProjectWithHigherAccess =
 						sinon.stub()
-						.callsArgWith(1, null, @project)
+						.callsArgWith(2, null, @project)
 					@TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub()
 						.callsArgWith(2, null)
 					@ProjectController.loadEditor = sinon.stub()
@@ -166,15 +166,10 @@ describe "TokenAccessController", ->
 						.to.equal true
 					done()
 
-				it 'should try to find a private overleaf project', (done) ->
+				it 'should check if user has higher access to the token project', (done) ->
 					expect(
-						@TokenAccessHandler.findPrivateOverleafProjectWithReadAndWriteToken
-							.callCount
+						@TokenAccessHandler.findProjectWithHigherAccess.callCount
 					).to.equal 1
-					expect(
-						@TokenAccessHandler.findPrivateOverleafProjectWithReadAndWriteToken
-							.calledWith(@readAndWriteToken)
-					).to.equal true
 					done()
 
 				it 'should not add the user to the project with read-write access', (done) ->
@@ -196,7 +191,7 @@ describe "TokenAccessController", ->
 					expect(@res.redirect.calledWith(302, "/project/#{@project._id}")).to.equal true
 					done()
 
-			describe 'when it is not a private overleaf project', ->
+			describe 'when higher access is not available', ->
 				beforeEach ->
 					@req = new MockRequest()
 					@res = new MockResponse()
@@ -204,9 +199,9 @@ describe "TokenAccessController", ->
 					@req.params['read_and_write_token'] = @readAndWriteToken
 					@TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub()
 						.callsArgWith(1, null, null)
-					@TokenAccessHandler.findPrivateOverleafProjectWithReadAndWriteToken =
+					@TokenAccessHandler.findProjectWithHigherAccess =
 						sinon.stub()
-						.callsArgWith(1, null, null)
+						.callsArgWith(2, null, null)
 					@TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub()
 						.callsArgWith(2, null)
 					@ProjectController.loadEditor = sinon.stub()
@@ -220,6 +215,12 @@ describe "TokenAccessController", ->
 					)).to.equal true
 					done()
 
+				it 'should check if user has higher access to the token project', (done) ->
+					expect(
+						@TokenAccessHandler.findProjectWithHigherAccess.callCount
+					).to.equal 1
+					done()
+
 				it 'should not add the user to the project with read-write access', (done) ->
 					expect(@TokenAccessHandler.addReadAndWriteUserToProject.callCount)
 						.to.equal 0

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

@@ -20,6 +20,7 @@ describe "TokenAccessHandler", ->
 		@TokenAccessHandler = SandboxedModule.require modulePath, requires:
 			'../../models/Project': {Project: @Project = {}}
 			'settings-sharelatex': {}
+			'../Collaborators/CollaboratorsHandler': @CollaboratorsHandler = {}
 
 
 	describe 'findProjectWithReadOnlyToken', ->
@@ -85,38 +86,91 @@ describe "TokenAccessHandler", ->
 					done()
 
 
-	describe 'findPrivateOverleafProjectWithReadAndWriteToken', ->
-		beforeEach ->
-			@Project.findOne = sinon.stub().callsArgWith(2, null, @project)
+	describe 'findProjectWithHigherAccess', ->
+		describe 'when user does have higher access', ->
+			beforeEach ->
+				@Project.findOne = sinon.stub().callsArgWith(2, null, @project)
+				@CollaboratorsHandler.isUserInvitedMemberOfProject = sinon.stub()
+					.callsArgWith(2, null, true)
+
+			it 'should call Project.findOne', (done) ->
+				@TokenAccessHandler.findProjectWithHigherAccess @token, @userId, (err, project) =>
+					expect(@Project.findOne.callCount).to.equal 1
+					expect(@Project.findOne.calledWith($or: [
+						{'tokens.readAndWrite': @token},
+						{'tokens.readOnly': @token}
+					])).to.equal true
+					done()
 
-		it 'should call Project.findOne', (done) ->
-			@TokenAccessHandler.findPrivateOverleafProjectWithReadAndWriteToken @token, (err, project) =>
-				expect(@Project.findOne.callCount).to.equal 1
-				expect(@Project.findOne.calledWith({
-					'tokens.readAndWrite': @token,
-					'publicAccesLevel': 'private',
-					'overleaf.id': {$exists: true}
-				})).to.equal true
-				done()
+			it 'should call isUserInvitedMemberOfProject', (done) ->
+				@TokenAccessHandler.findProjectWithHigherAccess @token, @userId, (err, project) =>
+					expect(@CollaboratorsHandler.isUserInvitedMemberOfProject.callCount)
+						.to.equal 1
+					expect(@CollaboratorsHandler.isUserInvitedMemberOfProject.calledWith(
+						@userId, @project._id
+					)).to.equal true
+					done()
 
-		it 'should produce a project object with no error', (done) ->
-			@TokenAccessHandler.findPrivateOverleafProjectWithReadAndWriteToken @token, (err, project) =>
-				expect(err).to.not.exist
-				expect(project).to.exist
-				expect(project).to.deep.equal @project
-				done()
+			it 'should produce a project object', (done) ->
+				@TokenAccessHandler.findProjectWithHigherAccess @token, @userId, (err, project) =>
+					expect(err).to.not.exist
+					expect(project).to.exist
+					expect(project).to.deep.equal @project
+					done()
+
+		describe 'when user does not have higher access', ->
+			beforeEach ->
+				@Project.findOne = sinon.stub().callsArgWith(2, null, @project)
+				@CollaboratorsHandler.isUserInvitedMemberOfProject = sinon.stub()
+					.callsArgWith(2, null, false)
+
+			it 'should call Project.findOne', (done) ->
+				@TokenAccessHandler.findProjectWithHigherAccess @token, @userId, (err, project) =>
+					expect(@Project.findOne.callCount).to.equal 1
+					expect(@Project.findOne.calledWith($or: [
+						{'tokens.readAndWrite': @token},
+						{'tokens.readOnly': @token}
+					])).to.equal true
+					done()
+
+			it 'should call isUserInvitedMemberOfProject', (done) ->
+				@TokenAccessHandler.findProjectWithHigherAccess @token, @userId, (err, project) =>
+					expect(@CollaboratorsHandler.isUserInvitedMemberOfProject.callCount)
+						.to.equal 1
+					expect(@CollaboratorsHandler.isUserInvitedMemberOfProject.calledWith(
+						@userId, @project._id
+					)).to.equal true
+					done()
+
+			it 'should not produce a project', (done) ->
+				@TokenAccessHandler.findProjectWithHigherAccess @token, @userId, (err, project) =>
+					expect(err).to.not.exist
+					expect(project).to.not.exist
+					done()
 
 		describe 'when Project.findOne produces an error', ->
 			beforeEach ->
 				@Project.findOne = sinon.stub().callsArgWith(2, new Error('woops'))
 
 			it 'should produce an error', (done) ->
-				@TokenAccessHandler.findPrivateOverleafProjectWithReadAndWriteToken @token, (err, project) =>
+				@TokenAccessHandler.findProjectWithHigherAccess @token, @userId, (err, project) =>
 					expect(err).to.exist
 					expect(project).to.not.exist
 					expect(err).to.be.instanceof Error
 					done()
 
+		describe 'when isUserInvitedMemberOfProject produces an error', ->
+			beforeEach ->
+				@Project.findOne = sinon.stub().callsArgWith(2, null, @project)
+				@CollaboratorsHandler.isUserInvitedMemberOfProject = sinon.stub()
+					.callsArgWith(2, new Error('woops'))
+
+			it 'should produce an error', (done) ->
+				@TokenAccessHandler.findProjectWithHigherAccess @token, @userId, (err, project) =>
+					expect(err).to.exist
+					expect(project).to.not.exist
+					expect(err).to.be.instanceof Error
+					done()
 
 	describe 'addReadOnlyUserToProject', ->
 		beforeEach ->

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

@@ -349,3 +349,35 @@ describe 'TokenAccess', ->
 			try_content_access(@other2, @project_id, (response, body) =>
 				expect(body.privilegeLevel).to.equal false
 			, done)
+
+	describe 'private project, with higher access', ->
+		before (done) ->
+			@owner.createProject "higher-access-test-#{Math.random()}", (err, project_id) =>
+				@project_id = project_id
+				@owner.addUserToProject @project_id, @other1, 'readAndWrite', (err) =>
+					@owner.makeTokenBased @project_id, (err) =>
+						@owner.getProject @project_id, (err, project) =>
+							@tokens = project.tokens
+							@owner.makePrivate @project_id, () =>
+								setTimeout done, 1000
+
+		it 'should redirect to canonical path, when user uses read-write token', (done) ->
+			try_read_and_write_token_access(@other1, @tokens.readAndWrite, (response, body) =>
+				expect(response.statusCode).to.equal 302
+				expect(response.headers.location).to.equal "/project/#{@project_id}"
+			, done)
+
+		it 'should allow the user access to the project', (done) ->
+			try_read_access(@other1, @project_id, (response, body) =>
+				expect(response.statusCode).to.equal 200
+			, done)
+
+		it 'should allow user to join the project', (done) ->
+			try_content_access(@other1, @project_id, (response, body) =>
+				expect(body.privilegeLevel).to.equal 'readAndWrite'
+			, done)
+
+		it 'should not allow a different user to join the project', (done) ->
+			try_content_access(@other2, @project_id, (response, body) =>
+				expect(body.privilegeLevel).to.equal false
+			, done)