diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee index d540098877..8b59be37e3 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee +++ b/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 diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee index b4d2f063f3..9eb792e55b 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee +++ b/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()) diff --git a/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessControllerTests.coffee b/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessControllerTests.coffee index 00e506fd6e..0601c4cc26 100644 --- a/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessControllerTests.coffee +++ b/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 diff --git a/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessHandlerTests.coffee b/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessHandlerTests.coffee index c00067a361..722a8496a2 100644 --- a/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessHandlerTests.coffee +++ b/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.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 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 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 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', (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 -> diff --git a/services/web/test/acceptance/coffee/TokenAccessTests.coffee b/services/web/test/acceptance/coffee/TokenAccessTests.coffee index 9e7e0a441e..3da565ee9d 100644 --- a/services/web/test/acceptance/coffee/TokenAccessTests.coffee +++ b/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)