From eab77aba91ac76a46dd875227e6066c5da309c51 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 19 Oct 2017 16:26:01 +0100 Subject: [PATCH] Abstract away the token-protection logic --- .../Editor/EditorHttpController.coffee | 3 +-- .../Features/Project/ProjectController.coffee | 11 ++-------- .../TokenAccess/TokenAccessHandler.coffee | 10 ++++++++++ .../Editor/EditorHttpControllerTests.coffee | 1 + .../Project/ProjectControllerTests.coffee | 1 + .../Security/OneTimeTokenHandlerTests.coffee | 1 - .../TokenAccessHandlerTests.coffee | 20 +++++++++++++++++++ 7 files changed, 35 insertions(+), 12 deletions(-) diff --git a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee index fc1e2dd3c0..fab4069369 100644 --- a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee @@ -24,8 +24,7 @@ module.exports = EditorHttpController = EditorHttpController._buildJoinProjectView req, project_id, user_id, (error, project, privilegeLevel) -> return next(error) if error? # Hide access tokens if this is not the project owner - if privilegeLevel != 'owner' && project?.tokens? - project.tokens = {readOnly: '', readAndWrite: ''} + TokenAccessHandler.protectTokens(project, privilegeLevel) res.json { project: project privilegeLevel: privilegeLevel diff --git a/services/web/app/coffee/Features/Project/ProjectController.coffee b/services/web/app/coffee/Features/Project/ProjectController.coffee index 33a75d1f8a..04576e8f84 100644 --- a/services/web/app/coffee/Features/Project/ProjectController.coffee +++ b/services/web/app/coffee/Features/Project/ProjectController.coffee @@ -331,14 +331,7 @@ module.exports = ProjectController = return projects _buildProjectViewModel: (project, accessLevel, source) -> - tokens = - readOnly: '' - readAndWrite: '' - if project.tokens? - if accessLevel == 'owner' || (accessLevel == 'readAndWrite' && source == 'token') - tokens.readAndWrite = project.tokens.readAndWrite - if accessLevel == 'owner' || (accessLevel == 'readOnly' && source == 'token') - tokens.readOnly = project.tokens.readOnly + TokenAccessHandler.protectTokens(project, accessLevel) model = { id: project._id name: project.name @@ -348,7 +341,7 @@ module.exports = ProjectController = source: source archived: !!project.archived owner_ref: project.owner_ref - tokens: tokens + tokens: project.tokens } return model diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee index e61b7ec44e..4862628858 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee +++ b/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee @@ -1,5 +1,6 @@ Project = require('../../models/Project').Project PublicAccessLevels = require '../Authorization/PublicAccessLevels' +PrivilegeLevels = require '../Authorization/PrivilegeLevels' ObjectId = require("mongojs").ObjectId Settings = require('settings-sharelatex') @@ -72,3 +73,12 @@ module.exports = TokenAccessHandler = return callback(err) if err? isValidReadOnly = _validate(readOnlyProject) callback null, isValidReadAndWrite, isValidReadOnly + + protectTokens: (project, privilegeLevel) -> + if project? && project.tokens? + if privilegeLevel == PrivilegeLevels.OWNER + return + if privilegeLevel != PrivilegeLevels.READ_AND_WRITE + project.tokens.readAndWrite = '' + if privilegeLevel != PrivilegeLevels.READ_ONLY + project.tokens.readOnly = '' diff --git a/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee b/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee index 258b6aafbe..79efc86479 100644 --- a/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee @@ -31,6 +31,7 @@ describe "EditorHttpController", -> json: sinon.stub() @callback = sinon.stub() @TokenAccessHandler.getRequestToken = sinon.stub().returns(@token = null) + @TokenAccessHandler.protectTokens = sinon.stub() describe "joinProject", -> beforeEach -> diff --git a/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee index 1014f23538..d207aab3cb 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee @@ -63,6 +63,7 @@ describe "ProjectController", -> getLastOccurance: sinon.stub() @TokenAccessHandler = getRequestToken: sinon.stub().returns(@token) + protectTokens: sinon.stub() @ProjectController = SandboxedModule.require modulePath, requires: "settings-sharelatex":@settings "logger-sharelatex": diff --git a/services/web/test/UnitTests/coffee/Security/OneTimeTokenHandlerTests.coffee b/services/web/test/UnitTests/coffee/Security/OneTimeTokenHandlerTests.coffee index 046acaa720..1940f34d07 100644 --- a/services/web/test/UnitTests/coffee/Security/OneTimeTokenHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Security/OneTimeTokenHandlerTests.coffee @@ -66,4 +66,3 @@ describe "OneTimeTokenHandler", -> done() - diff --git a/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessHandlerTests.coffee b/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessHandlerTests.coffee index 7ea63b7f12..c00067a361 100644 --- a/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessHandlerTests.coffee @@ -360,3 +360,23 @@ describe "TokenAccessHandler", -> expect(rw).to.equal false expect(ro).to.equal false done() + + + describe 'protectTokens', -> + beforeEach -> + @project = {tokens: {readAndWrite: 'rw', readOnly: 'ro'}} + + it 'should hide write token from read-only user', -> + @TokenAccessHandler.protectTokens(@project, 'readOnly') + expect(@project.tokens.readAndWrite).to.equal '' + expect(@project.tokens.readOnly).to.equal 'ro' + + it 'should hide read token from read-write user', -> + @TokenAccessHandler.protectTokens(@project, 'readAndWrite') + expect(@project.tokens.readAndWrite).to.equal 'rw' + expect(@project.tokens.readOnly).to.equal '' + + it 'should leave tokens in place for owner', -> + @TokenAccessHandler.protectTokens(@project, 'owner') + expect(@project.tokens.readAndWrite).to.equal 'rw' + expect(@project.tokens.readOnly).to.equal 'ro'