Account for higher-access in the token read-only path too

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

@@ -25,7 +25,7 @@ module.exports = TokenAccessController =
 					"[TokenAccess] no token-based project found for readAndWrite token"
 				if !userId?
 					logger.log {token},
-						"[TokenAccess] No project found with read-write token, anonymous user"
+						"[TokenAccess] No project found with read-write token, anonymous user, deny"
 					return next(new Errors.NotFoundError())
 				TokenAccessHandler.findProjectWithHigherAccess token, userId, (err, project) ->
 					if err?
@@ -34,7 +34,7 @@ module.exports = TokenAccessController =
 						return next(err)
 					if !project?
 						logger.log {token, userId},
-							"[TokenAccess] no project with higher access found for token and user"
+							"[TokenAccess] no project with higher access found for this user and token"
 						return next(new Errors.NotFoundError())
 					logger.log {token, userId, projectId: project._id},
 						"[TokenAccess] user has higher access to project, redirecting"
@@ -75,25 +75,41 @@ module.exports = TokenAccessController =
 				return next(err)
 			if !project?
 				logger.log {token, userId},
-					"[TokenAccess] no project found for readAndWrite token"
-				return next(new Errors.NotFoundError())
-			if !userId?
-				logger.log {userId, projectId: project._id},
-					"[TokenAccess] adding anonymous user to project with readOnly token"
-				TokenAccessHandler.grantSessionTokenAccess(req, project._id, token)
-				req._anonymousAccessToken = token
-				return TokenAccessController._loadEditor(project._id, req, res, next)
-			else
-				if project.owner_ref.toString() == userId
-					logger.log {userId, projectId: project._id},
-						"[TokenAccess] user is already project owner"
-					return TokenAccessController._loadEditor(project._id, req, res, next)
-				logger.log {userId, projectId: project._id},
-					"[TokenAccess] adding user to project with readOnly token"
-				TokenAccessHandler.addReadOnlyUserToProject userId, project._id, (err) ->
+					"[TokenAccess] no project found for readOnly token"
+				if !userId?
+					logger.log {token},
+						"[TokenAccess] No project found with readOnly token, anonymous user, deny"
+					return next(new Errors.NotFoundError())
+				TokenAccessHandler.findProjectWithHigherAccess token, userId, (err, project) ->
 					if err?
-						logger.err {err, token, userId, projectId: project._id},
-							"[TokenAccess] error adding user to project with readAndWrite token"
+						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 this user and token"
+						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?
+					logger.log {userId, projectId: project._id},
+						"[TokenAccess] adding anonymous user to project with readOnly token"
+					TokenAccessHandler.grantSessionTokenAccess(req, project._id, token)
+					req._anonymousAccessToken = token
 					return TokenAccessController._loadEditor(project._id, req, res, next)
+				else
+					if project.owner_ref.toString() == userId
+						logger.log {userId, projectId: project._id},
+							"[TokenAccess] user is already project owner"
+						return TokenAccessController._loadEditor(project._id, req, res, next)
+					logger.log {userId, projectId: project._id},
+						"[TokenAccess] adding user to project with readOnly token"
+					TokenAccessHandler.addReadOnlyUserToProject userId, project._id, (err) ->
+						if err?
+							logger.err {err, token, userId, projectId: project._id},
+								"[TokenAccess] error adding user to project with readAndWrite token"
+							return next(err)
+						return TokenAccessController._loadEditor(project._id, req, res, next)
 

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

@@ -385,28 +385,42 @@ describe "TokenAccessController", ->
 					expect(@next.lastCall.args[0]).to.be.instanceof Error
 					done()
 
-			describe 'when findProject does not find a project', ->
+			##
+		describe 'when findProject does not find a project', ->
+			beforeEach ->
+
+			describe 'when token access is off, but user has higher access anyway', ->
 				beforeEach ->
 					@req = new MockRequest()
 					@res = new MockResponse()
+					@res.redirect = sinon.stub()
 					@next = sinon.stub()
-					@req.params['read_only_token'] = @readOnlyToken
-					@TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub()
+					@req.params['read_and_write_token'] = @readAndWriteToken
+					@TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub()
 						.callsArgWith(1, null, null)
-					@TokenAccessHandler.addReadOnlyUserToProject = sinon.stub()
+					@TokenAccessHandler.findProjectWithHigherAccess =
+						sinon.stub()
+						.callsArgWith(2, null, @project)
+					@TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub()
 						.callsArgWith(2, null)
 					@ProjectController.loadEditor = sinon.stub()
-					@TokenAccessController.readOnlyToken @req, @res, @next
+					@TokenAccessController.readAndWriteToken @req, @res, @next
 
 				it 'should try to find a project with this token', (done) ->
-					expect(@TokenAccessHandler.findProjectWithReadOnlyToken.callCount)
+					expect(@TokenAccessHandler.findProjectWithReadAndWriteToken.callCount)
 						.to.equal 1
-					expect(@TokenAccessHandler.findProjectWithReadOnlyToken.calledWith(@readOnlyToken))
+					expect(@TokenAccessHandler.findProjectWithReadAndWriteToken.calledWith(@readAndWriteToken))
 						.to.equal true
 					done()
 
-				it 'should not add the user to the project with read-only access', (done) ->
-					expect(@TokenAccessHandler.addReadOnlyUserToProject.callCount)
+				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
 					done()
 
@@ -415,38 +429,48 @@ describe "TokenAccessController", ->
 					expect(@ProjectController.loadEditor.calledWith(@req, @res, @next)).to.equal false
 					done()
 
-				it 'should call next with a not-found error', (done) ->
-					expect(@next.callCount).to.equal 1
-					expect(@next.lastCall.args[0]).to.be.instanceof Error
+				it 'should not call next with a not-found error', (done) ->
+					expect(@next.callCount).to.equal 0
+					done()
+
+				it 'should redirect to the canonical project url', (done) ->
+					expect(@res.redirect.callCount).to.equal 1
+					expect(@res.redirect.calledWith(302, "/project/#{@project._id}")).to.equal true
 					done()
 
-			describe 'when adding user to project produces an error', ->
+			describe 'when higher access is not available', ->
 				beforeEach ->
 					@req = new MockRequest()
 					@res = new MockResponse()
 					@next = sinon.stub()
-					@req.params['read_only_token'] = @readOnlyToken
-					@TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub()
-						.callsArgWith(1, null, @project)
+					@req.params['read_and_write_token'] = @readAndWriteToken
+					@TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub()
+						.callsArgWith(1, null, null)
+					@TokenAccessHandler.findProjectWithHigherAccess =
+						sinon.stub()
+						.callsArgWith(2, null, null)
 					@TokenAccessHandler.addReadOnlyUserToProject = sinon.stub()
-						.callsArgWith(2, new Error('woops'))
+						.callsArgWith(2, null)
 					@ProjectController.loadEditor = sinon.stub()
-					@TokenAccessController.readOnlyToken @req, @res, @next
+					@TokenAccessController.readAndWriteToken @req, @res, @next
 
 				it 'should try to find a project with this token', (done) ->
-					expect(@TokenAccessHandler.findProjectWithReadOnlyToken.callCount)
+					expect(@TokenAccessHandler.findProjectWithReadAndWriteToken.callCount)
 						.to.equal 1
-					expect(@TokenAccessHandler.findProjectWithReadOnlyToken.calledWith(@readOnlyToken))
-						.to.equal true
+					expect(@TokenAccessHandler.findProjectWithReadAndWriteToken.calledWith(
+						@readAndWriteToken
+					)).to.equal true
 					done()
 
-				it 'should add the user to the project with read-only access', (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.addReadOnlyUserToProject.callCount)
-						.to.equal 1
-					expect(@TokenAccessHandler.addReadOnlyUserToProject.calledWith(
-						@userId.toString(), @projectId
-					))
-						.to.equal true
+						.to.equal 0
 					done()
 
 				it 'should not pass control to loadEditor', (done) ->
@@ -454,12 +478,49 @@ describe "TokenAccessController", ->
 					expect(@ProjectController.loadEditor.calledWith(@req, @res, @next)).to.equal false
 					done()
 
-				it 'should call next with an error', (done) ->
+				it 'should call next with a not-found error', (done) ->
 					expect(@next.callCount).to.equal 1
 					expect(@next.lastCall.args[0]).to.be.instanceof Error
 					done()
 
+		describe 'when adding user to project produces an error', ->
+			beforeEach ->
+				@req = new MockRequest()
+				@res = new MockResponse()
+				@next = sinon.stub()
+				@req.params['read_only_token'] = @readOnlyToken
+				@TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub()
+					.callsArgWith(1, null, @project)
+				@TokenAccessHandler.addReadOnlyUserToProject = sinon.stub()
+					.callsArgWith(2, new Error('woops'))
+				@ProjectController.loadEditor = sinon.stub()
+				@TokenAccessController.readOnlyToken @req, @res, @next
+
+			it 'should try to find a project with this token', (done) ->
+				expect(@TokenAccessHandler.findProjectWithReadOnlyToken.callCount)
+					.to.equal 1
+				expect(@TokenAccessHandler.findProjectWithReadOnlyToken.calledWith(@readOnlyToken))
+					.to.equal true
+				done()
+
+			it 'should add the user to the project with read-only access', (done) ->
+				expect(@TokenAccessHandler.addReadOnlyUserToProject.callCount)
+					.to.equal 1
+				expect(@TokenAccessHandler.addReadOnlyUserToProject.calledWith(
+					@userId.toString(), @projectId
+				))
+					.to.equal true
+				done()
+
+			it 'should not pass control to loadEditor', (done) ->
+				expect(@ProjectController.loadEditor.callCount).to.equal 0
+				expect(@ProjectController.loadEditor.calledWith(@req, @res, @next)).to.equal false
+				done()
 
+			it 'should call next with an error', (done) ->
+				expect(@next.callCount).to.equal 1
+				expect(@next.lastCall.args[0]).to.be.instanceof Error
+				done()
 
 		describe 'anonymous', ->
 			beforeEach ->

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

@@ -367,6 +367,12 @@ describe 'TokenAccess', ->
 				expect(response.headers.location).to.equal "/project/#{@project_id}"
 			, done)
 
+		it 'should redirect to canonical path, when user uses read-only token', (done) ->
+			try_read_only_token_access(@other1, @tokens.readOnly, (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