From 855fe2e143ae7ee7bf758c21a63b0bd3fca7803f Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 16 Oct 2017 16:44:20 +0100 Subject: [PATCH] If user is project owner, don't add them as a token user --- .../Authorization/PublicAccessLevels.coffee | 4 +- .../TokenAccess/TokenAccessController.coffee | 18 +++-- .../TokenAccess/TokenAccessHandler.coffee | 4 +- .../TokenAccessControllerTests.coffee | 68 ++++++++++++++++++- 4 files changed, 85 insertions(+), 9 deletions(-) diff --git a/services/web/app/coffee/Features/Authorization/PublicAccessLevels.coffee b/services/web/app/coffee/Features/Authorization/PublicAccessLevels.coffee index 8fc69f6077..e31426221e 100644 --- a/services/web/app/coffee/Features/Authorization/PublicAccessLevels.coffee +++ b/services/web/app/coffee/Features/Authorization/PublicAccessLevels.coffee @@ -1,5 +1,5 @@ module.exports = - READ_ONLY: "readOnly" - READ_AND_WRITE: "readAndWrite" + READ_ONLY: "readOnly" # LEGACY + READ_AND_WRITE: "readAndWrite" # LEGACY PRIVATE: "private" TOKEN_BASED: "tokenBased" diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee index 682b926a0b..2c60835230 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee +++ b/services/web/app/coffee/Features/TokenAccess/TokenAccessController.coffee @@ -7,6 +7,10 @@ logger = require 'logger-sharelatex' module.exports = TokenAccessController = + _loadEditor: (projectId, req, res, next) -> + req.params.Project_id = projectId.toString() + return ProjectController.loadEditor(req, res, next) + readAndWriteToken: (req, res, next) -> userId = AuthenticationController.getLoggedInUserId(req) token = req.params['read_and_write_token'] @@ -32,6 +36,10 @@ module.exports = TokenAccessController = logger.log {token, projectId: project._id}, "redirecting user to project" res.redirect(302, "/project/#{project._id}") else + if project.owner_ref.toString() == userId + logger.log {userId, projectId: project._id}, + "user is already project owner" + return TokenAccessController._loadEditor(project._id, req, res, next) logger.log {userId, projectId: project._id}, "adding user to project with readAndWrite token" TokenAccessHandler.addReadAndWriteUserToProject userId, project._id, (err) -> @@ -39,8 +47,7 @@ module.exports = TokenAccessController = logger.err {err, token, userId, projectId: project._id}, "error adding user to project with readAndWrite token" return next(err) - req.params.Project_id = project._id.toString() - return ProjectController.loadEditor(req, res, next) + return TokenAccessController._loadEditor(project._id, req, res, next) readOnlyToken: (req, res, next) -> userId = AuthenticationController.getLoggedInUserId(req) @@ -59,10 +66,13 @@ module.exports = TokenAccessController = logger.log {userId, projectId: project._id}, "adding anonymous user to project with readOnly token" TokenAccessHandler.grantSessionReadOnlyTokenAccess(req, project._id, token) - req.params.Project_id = project._id.toString() req._anonToken = token - return ProjectController.loadEditor(req, res, next) + return TokenAccessController._loadEditor(project._id, req, res, next) else + if project.owner_ref.toString() == userId + logger.log {userId, projectId: project._id}, + "user is already project owner" + return TokenAccessController._loadEditor(project._id, req, res, next) logger.log {userId, projectId: project._id}, "adding user to project with readOnly token" TokenAccessHandler.addReadOnlyUserToProject userId, project._id, (err) -> diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee index 8275b10ae4..c7628a45f6 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee +++ b/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee @@ -8,13 +8,13 @@ module.exports = TokenAccessHandler = Project.findOne { 'tokens.readOnly': token, 'publicAccesLevel': PublicAccessLevels.TOKEN_BASED - }, {_id: 1, publicAccesLevel: 1}, callback + }, {_id: 1, publicAccesLevel: 1, owner_ref: 1}, callback findProjectWithReadAndWriteToken: (token, callback=(err, project)->) -> Project.findOne { 'tokens.readAndWrite': token, 'publicAccesLevel': PublicAccessLevels.TOKEN_BASED - }, {_id: 1, publicAccesLevel: 1}, callback + }, {_id: 1, publicAccesLevel: 1, owner_ref: 1}, callback findPrivateOverleafProjectWithReadAndWriteToken: (token, callback=(err, project)->) -> Project.findOne { diff --git a/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessControllerTests.coffee b/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessControllerTests.coffee index 31d1d70094..6f7aa9c586 100644 --- a/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessControllerTests.coffee @@ -16,12 +16,14 @@ describe "TokenAccessController", -> @readOnlyToken = 'somereadonlytoken' @readAndWriteToken = '42somereadandwritetoken' @projectId = ObjectId() + @ownerId = 'owner' @project = _id: @projectId publicAccesLevel: 'tokenBased' tokens: readOnly: @readOnlyToken readAndWrite: @readAndWriteToken + owner_ref: @ownerId @userId = ObjectId() @TokenAccessController = SandboxedModule.require modulePath, requires: '../Project/ProjectController': @ProjectController = {} @@ -70,6 +72,38 @@ describe "TokenAccessController", -> expect(@ProjectController.loadEditor.calledWith(@req, @res, @next)).to.equal true done() + describe 'when the user is already the owner', -> + beforeEach -> + @req = new MockRequest() + @res = new MockResponse() + @next = sinon.stub() + @req.params['read_and_write_token'] = @readAndWriteToken + @project.owner_ref = @userId + @TokenAccessHandler.findProjectWithReadAndWriteToken = sinon.stub() + .callsArgWith(1, 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 not add the user to the project with read-write access', (done) -> + expect(@TokenAccessHandler.addReadAndWriteUserToProject.callCount) + .to.equal 0 + done() + + it 'should pass control to loadEditor', (done) -> + expect(@req.params.Project_id).to.equal @projectId.toString() + expect(@ProjectController.loadEditor.callCount).to.equal 1 + expect(@ProjectController.loadEditor.calledWith(@req, @res, @next)).to.equal true + done() + describe 'when findProject produces an error', -> beforeEach -> @req = new MockRequest() @@ -282,7 +316,39 @@ describe "TokenAccessController", -> expect(@ProjectController.loadEditor.callCount).to.equal 1 expect(@ProjectController.loadEditor.calledWith(@req, @res, @next)).to.equal true done() - + + describe 'when the user is already the owner', -> + beforeEach -> + @req = new MockRequest() + @res = new MockResponse() + @next = sinon.stub() + @req.params['read_only_token'] = @readOnlyToken + @project.owner_ref = @userId + @TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub() + .callsArgWith(1, null, @project) + @TokenAccessHandler.addReadOnlyUserToProject = sinon.stub() + .callsArgWith(2, null) + @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 not add the user to the project with read-only access', (done) -> + expect(@TokenAccessHandler.addReadOnlyUserToProject.callCount) + .to.equal 0 + done() + + it 'should pass control to loadEditor', (done) -> + expect(@req.params.Project_id).to.equal @projectId.toString() + expect(@ProjectController.loadEditor.callCount).to.equal 1 + expect(@ProjectController.loadEditor.calledWith(@req, @res, @next)).to.equal true + done() + describe 'when findProject produces an error', -> beforeEach -> @req = new MockRequest()