diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee index 8b59be37e3..79d206f1f5 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee +++ b/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) diff --git a/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessControllerTests.coffee b/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessControllerTests.coffee index 0601c4cc26..5e69de958d 100644 --- a/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessControllerTests.coffee @@ -385,27 +385,90 @@ 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_and_write_token'] = @readAndWriteToken + @TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub() + .callsArgWith(1, null, null) + @TokenAccessHandler.findProjectWithHigherAccess = + sinon.stub() + .callsArgWith(2, null, @project) + @TokenAccessHandler.addReadAndWriteUserToProject = sinon.stub() + .callsArgWith(2, null) + @ProjectController.loadEditor = sinon.stub() + @TokenAccessController.readAndWriteToken @req, @res, @next + + it 'should try to find a project with this token', (done) -> + expect(@TokenAccessHandler.findProjectWithReadAndWriteToken.callCount) + .to.equal 1 + expect(@TokenAccessHandler.findProjectWithReadAndWriteToken.calledWith(@readAndWriteToken)) + .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 + 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 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 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() + @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, 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 not 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 0 done() @@ -420,46 +483,44 @@ describe "TokenAccessController", -> 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 + 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 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() + 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 -> diff --git a/services/web/test/acceptance/coffee/TokenAccessTests.coffee b/services/web/test/acceptance/coffee/TokenAccessTests.coffee index 3da565ee9d..14d099476d 100644 --- a/services/web/test/acceptance/coffee/TokenAccessTests.coffee +++ b/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