From ac513a135584210f02dca697ae0a3329c3dd9d65 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 13 Oct 2017 11:20:57 +0100 Subject: [PATCH] Refactor to not pass `req` down into Auth modules --- .../Authorization/AuthorizationManager.coffee | 22 ++--- .../AuthorizationMiddlewear.coffee | 16 ++- .../Editor/EditorHttpController.coffee | 4 +- .../Features/Project/ProjectController.coffee | 4 +- .../TokenAccess/TokenAccessHandler.coffee | 7 +- .../AuthorizationManagerTests.coffee | 97 +++++++++---------- .../AuthorizationMiddlewearTests.coffee | 27 +++--- .../Editor/EditorHttpControllerTests.coffee | 4 +- .../Project/ProjectControllerTests.coffee | 4 + .../TokenAccessHandlerTests.coffee | 51 ++++------ 10 files changed, 120 insertions(+), 116 deletions(-) diff --git a/services/web/app/coffee/Features/Authorization/AuthorizationManager.coffee b/services/web/app/coffee/Features/Authorization/AuthorizationManager.coffee index 5a3fe9abf9..98407b0cc8 100644 --- a/services/web/app/coffee/Features/Authorization/AuthorizationManager.coffee +++ b/services/web/app/coffee/Features/Authorization/AuthorizationManager.coffee @@ -26,7 +26,7 @@ module.exports = AuthorizationManager = # access. false if the user does not have access # * becausePublic: true if the access level is only because the project is public. getPrivilegeLevelForProject: ( - req, user_id, project_id, + user_id, project_id, token, callback = (error, privilegeLevel, becausePublic) -> ) -> if !user_id? @@ -36,9 +36,9 @@ module.exports = AuthorizationManager = if publicAccessLevel == PublicAccessLevels.TOKEN_BASED # Anonymous users can have read-only access to token-based projects, # while read-write access must be logged in - TokenAccessHandler.requestHasReadOnlyTokenAccess req, project_id, (err, allowed) -> + TokenAccessHandler.isValidReadOnlyToken project_id, token, (err, isValid) -> return callback(err) if err? - if allowed + if isValid # Grant anonymous user read-only access callback null, PrivilegeLevels.READ_ONLY, false else @@ -77,18 +77,18 @@ module.exports = AuthorizationManager = else callback null, PrivilegeLevels.NONE, false - canUserReadProject: (req, user_id, project_id, callback = (error, canRead) ->) -> - AuthorizationManager.getPrivilegeLevelForProject req, user_id, project_id, (error, privilegeLevel) -> + canUserReadProject: (user_id, project_id, token, callback = (error, canRead) ->) -> + AuthorizationManager.getPrivilegeLevelForProject user_id, project_id, token, (error, privilegeLevel) -> return callback(error) if error? return callback null, (privilegeLevel in [PrivilegeLevels.OWNER, PrivilegeLevels.READ_AND_WRITE, PrivilegeLevels.READ_ONLY]) - canUserWriteProjectContent: (req, user_id, project_id, callback = (error, canWriteContent) ->) -> - AuthorizationManager.getPrivilegeLevelForProject req, user_id, project_id, (error, privilegeLevel) -> + canUserWriteProjectContent: (user_id, project_id, token, callback = (error, canWriteContent) ->) -> + AuthorizationManager.getPrivilegeLevelForProject user_id, project_id, token, (error, privilegeLevel) -> return callback(error) if error? return callback null, (privilegeLevel in [PrivilegeLevels.OWNER, PrivilegeLevels.READ_AND_WRITE]) - canUserWriteProjectSettings: (req, user_id, project_id, callback = (error, canWriteSettings) ->) -> - AuthorizationManager.getPrivilegeLevelForProject req, user_id, project_id, (error, privilegeLevel, becausePublic) -> + canUserWriteProjectSettings: (user_id, project_id, token, callback = (error, canWriteSettings) ->) -> + AuthorizationManager.getPrivilegeLevelForProject user_id, project_id, token, (error, privilegeLevel, becausePublic) -> return callback(error) if error? if privilegeLevel == PrivilegeLevels.OWNER return callback null, true @@ -97,8 +97,8 @@ module.exports = AuthorizationManager = else return callback null, false - canUserAdminProject: (req, user_id, project_id, callback = (error, canAdmin) ->) -> - AuthorizationManager.getPrivilegeLevelForProject req, user_id, project_id, (error, privilegeLevel) -> + canUserAdminProject: (user_id, project_id, token, callback = (error, canAdmin) ->) -> + AuthorizationManager.getPrivilegeLevelForProject user_id, project_id, token, (error, privilegeLevel) -> return callback(error) if error? return callback null, (privilegeLevel == PrivilegeLevels.OWNER) diff --git a/services/web/app/coffee/Features/Authorization/AuthorizationMiddlewear.coffee b/services/web/app/coffee/Features/Authorization/AuthorizationMiddlewear.coffee index dfbc773cf8..798cd34055 100644 --- a/services/web/app/coffee/Features/Authorization/AuthorizationMiddlewear.coffee +++ b/services/web/app/coffee/Features/Authorization/AuthorizationMiddlewear.coffee @@ -4,6 +4,7 @@ logger = require "logger-sharelatex" ObjectId = require("mongojs").ObjectId Errors = require "../Errors/Errors" AuthenticationController = require "../Authentication/AuthenticationController" +TokenAccessHandler = require '../TokenAccess/TokenAccessHandler' module.exports = AuthorizationMiddlewear = ensureUserCanReadMultipleProjects: (req, res, next) -> @@ -13,7 +14,8 @@ module.exports = AuthorizationMiddlewear = # Remove the projects we have access to. Note rejectSeries doesn't use # errors in callbacks async.rejectSeries project_ids, (project_id, cb) -> - AuthorizationManager.canUserReadProject req, user_id, project_id, (error, canRead) -> + token = TokenAccessHandler.getRequestToken(req, project_id) + AuthorizationManager.canUserReadProject user_id, project_id, token, (error, canRead) -> return next(error) if error? cb(canRead) , (unauthorized_project_ids) -> @@ -25,7 +27,8 @@ module.exports = AuthorizationMiddlewear = ensureUserCanReadProject: (req, res, next) -> AuthorizationMiddlewear._getUserAndProjectId req, (error, user_id, project_id) -> return next(error) if error? - AuthorizationManager.canUserReadProject req, user_id, project_id, (error, canRead) -> + token = TokenAccessHandler.getRequestToken(req, project_id) + AuthorizationManager.canUserReadProject user_id, project_id, token, (error, canRead) -> return next(error) if error? if canRead logger.log {user_id, project_id}, "allowing user read access to project" @@ -37,7 +40,8 @@ module.exports = AuthorizationMiddlewear = ensureUserCanWriteProjectSettings: (req, res, next) -> AuthorizationMiddlewear._getUserAndProjectId req, (error, user_id, project_id) -> return next(error) if error? - AuthorizationManager.canUserWriteProjectSettings req, user_id, project_id, (error, canWrite) -> + token = TokenAccessHandler.getRequestToken(req, project_id) + AuthorizationManager.canUserWriteProjectSettings user_id, project_id, token, (error, canWrite) -> return next(error) if error? if canWrite logger.log {user_id, project_id}, "allowing user write access to project settings" @@ -49,7 +53,8 @@ module.exports = AuthorizationMiddlewear = ensureUserCanWriteProjectContent: (req, res, next) -> AuthorizationMiddlewear._getUserAndProjectId req, (error, user_id, project_id) -> return next(error) if error? - AuthorizationManager.canUserWriteProjectContent req, user_id, project_id, (error, canWrite) -> + token = TokenAccessHandler.getRequestToken(req, project_id) + AuthorizationManager.canUserWriteProjectContent user_id, project_id, token, (error, canWrite) -> return next(error) if error? if canWrite logger.log {user_id, project_id}, "allowing user write access to project content" @@ -61,7 +66,8 @@ module.exports = AuthorizationMiddlewear = ensureUserCanAdminProject: (req, res, next) -> AuthorizationMiddlewear._getUserAndProjectId req, (error, user_id, project_id) -> return next(error) if error? - AuthorizationManager.canUserAdminProject req, user_id, project_id, (error, canAdmin) -> + token = TokenAccessHandler.getRequestToken(req, project_id) + AuthorizationManager.canUserAdminProject user_id, project_id, token, (error, canAdmin) -> return next(error) if error? if canAdmin logger.log {user_id, project_id}, "allowing user admin access to project" diff --git a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee index 077ee05508..eb22334684 100644 --- a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee @@ -11,6 +11,7 @@ Metrics = require('metrics-sharelatex') CollaboratorsHandler = require("../Collaborators/CollaboratorsHandler") CollaboratorsInviteHandler = require("../Collaborators/CollaboratorsInviteHandler") PrivilegeLevels = require "../Authorization/PrivilegeLevels" +TokenAccessHandler = require '../TokenAccess/TokenAccessHandler' module.exports = EditorHttpController = joinProject: (req, res, next) -> @@ -42,7 +43,8 @@ module.exports = EditorHttpController = return callback(error) if error? UserGetter.getUser user_id, { isAdmin: true }, (error, user) -> return callback(error) if error? - AuthorizationManager.getPrivilegeLevelForProject req, user_id, project_id, (error, privilegeLevel) -> + token = TokenAccessHandler.getRequestToken(req, project_id) + AuthorizationManager.getPrivilegeLevelForProject user_id, project_id, token, (error, privilegeLevel) -> return callback(error) if error? if !privilegeLevel? or privilegeLevel == PrivilegeLevels.NONE logger.log {project_id, user_id, privilegeLevel}, "not an acceptable privilege level, returning null" diff --git a/services/web/app/coffee/Features/Project/ProjectController.coffee b/services/web/app/coffee/Features/Project/ProjectController.coffee index 47b1e6914b..db386260c9 100644 --- a/services/web/app/coffee/Features/Project/ProjectController.coffee +++ b/services/web/app/coffee/Features/Project/ProjectController.coffee @@ -22,6 +22,7 @@ AuthenticationController = require("../Authentication/AuthenticationController") PackageVersions = require("../../infrastructure/PackageVersions") AnalyticsManager = require "../Analytics/AnalyticsManager" Sources = require "../Authorization/Sources" +TokenAccessHandler = require '../TokenAccess/TokenAccessHandler' module.exports = ProjectController = @@ -258,7 +259,8 @@ module.exports = ProjectController = daysSinceLastUpdated = (new Date() - project.lastUpdated) /86400000 logger.log project_id:project_id, daysSinceLastUpdated:daysSinceLastUpdated, "got db results for loading editor" - AuthorizationManager.getPrivilegeLevelForProject req, user_id, project_id, (error, privilegeLevel)-> + token = TokenAccessHandler.getRequestToken(req, project_id) + AuthorizationManager.getPrivilegeLevelForProject user_id, project_id, token, (error, privilegeLevel)-> return next(error) if error? if !privilegeLevel? or privilegeLevel == PrivilegeLevels.NONE return res.sendStatus 401 diff --git a/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee b/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee index 202e3882a7..da6ab55207 100644 --- a/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee +++ b/services/web/app/coffee/Features/TokenAccess/TokenAccessHandler.coffee @@ -40,11 +40,14 @@ module.exports = TokenAccessHandler = req.session.anonReadOnlyTokenAccess = {} req.session.anonReadOnlyTokenAccess[projectId.toString()] = token.toString() - requestHasReadOnlyTokenAccess: (req, projectId, callback=(err, allowed)->) -> + getRequestToken: (req, projectId) -> token = ( req?.session?.anonReadOnlyTokenAccess?[projectId.toString()] or - req.headers['x-sl-anon-token'] + req?.headers['x-sl-anon-token'] ) + return token + + isValidReadOnlyToken: (projectId, token, callback=(err, allowed)->) -> if !token return callback null, false TokenAccessHandler.findProjectWithReadOnlyToken token, (err, project) -> diff --git a/services/web/test/UnitTests/coffee/Authorization/AuthorizationManagerTests.coffee b/services/web/test/UnitTests/coffee/Authorization/AuthorizationManagerTests.coffee index 6882c71b52..a983f5ba35 100644 --- a/services/web/test/UnitTests/coffee/Authorization/AuthorizationManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Authorization/AuthorizationManagerTests.coffee @@ -5,7 +5,6 @@ expect = chai.expect modulePath = "../../../../app/js/Features/Authorization/AuthorizationManager.js" SandboxedModule = require('sandboxed-module') Errors = require "../../../../app/js/Features/Errors/Errors.js" -MockRequest = require '../helpers/MockRequest' describe "AuthorizationManager", -> beforeEach -> @@ -15,10 +14,11 @@ describe "AuthorizationManager", -> "../../models/User": User: @User = {} "../Errors/Errors": Errors "../TokenAccess/TokenAccessHandler": @TokenAccessHandler = { - requestHasReadOnlyTokenAccess: sinon.stub().callsArgWith(2, null, false) + isValidReadOnlyToken: sinon.stub().callsArgWith(2, null, false) } @user_id = "user-id-1" @project_id = "project-id-1" + @token = 'some-token' @callback = sinon.stub() describe "getPrivilegeLevelForProject", -> @@ -29,7 +29,6 @@ describe "AuthorizationManager", -> describe "with a private project", -> beforeEach -> - @req = new MockRequest() @Project.findOne .withArgs({ _id: @project_id }, { publicAccesLevel: 1 }) .yields(null, { publicAccesLevel: "private" }) @@ -40,7 +39,7 @@ describe "AuthorizationManager", -> @CollaboratorsHandler.getMemberIdPrivilegeLevel .withArgs(@user_id, @project_id) .yields(null, "readOnly") - @AuthorizationManager.getPrivilegeLevelForProject @req, @user_id, @project_id, @callback + @AuthorizationManager.getPrivilegeLevelForProject @user_id, @project_id, @token, @callback it "should return the user's privilege level", -> @callback.calledWith(null, "readOnly", false).should.equal true @@ -51,7 +50,7 @@ describe "AuthorizationManager", -> @CollaboratorsHandler.getMemberIdPrivilegeLevel .withArgs(@user_id, @project_id) .yields(null, false) - @AuthorizationManager.getPrivilegeLevelForProject @req, @user_id, @project_id, @callback + @AuthorizationManager.getPrivilegeLevelForProject @user_id, @project_id, @token, @callback it "should return false", -> @callback.calledWith(null, false, false).should.equal true @@ -62,14 +61,14 @@ describe "AuthorizationManager", -> @CollaboratorsHandler.getMemberIdPrivilegeLevel .withArgs(@user_id, @project_id) .yields(null, false) - @AuthorizationManager.getPrivilegeLevelForProject @req, @user_id, @project_id, @callback + @AuthorizationManager.getPrivilegeLevelForProject @user_id, @project_id, @token, @callback it "should return the user as an owner", -> @callback.calledWith(null, "owner", false).should.equal true describe "with no user (anonymous)", -> beforeEach -> - @AuthorizationManager.getPrivilegeLevelForProject @req, null, @project_id, @callback + @AuthorizationManager.getPrivilegeLevelForProject null, @project_id, @token, @callback it "should not call CollaboratorsHandler.getMemberIdPrivilegeLevel", -> @CollaboratorsHandler.getMemberIdPrivilegeLevel.called.should.equal false @@ -92,7 +91,7 @@ describe "AuthorizationManager", -> @CollaboratorsHandler.getMemberIdPrivilegeLevel .withArgs(@user_id, @project_id) .yields(null, "readOnly") - @AuthorizationManager.getPrivilegeLevelForProject @req, @user_id, @project_id, @callback + @AuthorizationManager.getPrivilegeLevelForProject @user_id, @project_id, @token, @callback it "should return the user's privilege level", -> @callback.calledWith(null, "readOnly", false).should.equal true @@ -103,7 +102,7 @@ describe "AuthorizationManager", -> @CollaboratorsHandler.getMemberIdPrivilegeLevel .withArgs(@user_id, @project_id) .yields(null, false) - @AuthorizationManager.getPrivilegeLevelForProject @req, @user_id, @project_id, @callback + @AuthorizationManager.getPrivilegeLevelForProject @user_id, @project_id, @token, @callback it "should return the public privilege level", -> @callback.calledWith(null, "readAndWrite", true).should.equal true @@ -114,14 +113,14 @@ describe "AuthorizationManager", -> @CollaboratorsHandler.getMemberIdPrivilegeLevel .withArgs(@user_id, @project_id) .yields(null, false) - @AuthorizationManager.getPrivilegeLevelForProject @req, @user_id, @project_id, @callback + @AuthorizationManager.getPrivilegeLevelForProject @user_id, @project_id, @token, @callback it "should return the user as an owner", -> @callback.calledWith(null, "owner", false).should.equal true describe "with no user (anonymous)", -> beforeEach -> - @AuthorizationManager.getPrivilegeLevelForProject @req, null, @project_id, @callback + @AuthorizationManager.getPrivilegeLevelForProject null, @project_id, @token, @callback it "should not call CollaboratorsHandler.getMemberIdPrivilegeLevel", -> @CollaboratorsHandler.getMemberIdPrivilegeLevel.called.should.equal false @@ -139,7 +138,7 @@ describe "AuthorizationManager", -> .yields(null, null) it "should return a NotFoundError", -> - @AuthorizationManager.getPrivilegeLevelForProject @req, @user_id, @project_id, (error) -> + @AuthorizationManager.getPrivilegeLevelForProject @user_id, @project_id, @token, (error) -> error.should.be.instanceof Errors.NotFoundError describe "when the project id is not valid", -> @@ -150,215 +149,211 @@ describe "AuthorizationManager", -> .yields(null, "readOnly") it "should return a error", (done)-> - @AuthorizationManager.getPrivilegeLevelForProject @req, undefined, "not project id", (err) => + @AuthorizationManager.getPrivilegeLevelForProject undefined, "not project id", @token, (err) => @Project.findOne.called.should.equal false expect(err).to.exist done() describe "canUserReadProject", -> beforeEach -> - @req = new MockRequest() @AuthorizationManager.getPrivilegeLevelForProject = sinon.stub() describe "when user is owner", -> beforeEach -> @AuthorizationManager.getPrivilegeLevelForProject - .withArgs(@req, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, "owner", false) it "should return true", (done) -> - @AuthorizationManager.canUserReadProject @req, @user_id, @project_id, (error, canRead) -> + @AuthorizationManager.canUserReadProject @user_id, @project_id, @token, (error, canRead) -> expect(canRead).to.equal true done() describe "when user has read-write access", -> beforeEach -> @AuthorizationManager.getPrivilegeLevelForProject - .withArgs(@req, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, "readAndWrite", false) it "should return true", (done) -> - @AuthorizationManager.canUserReadProject @req, @user_id, @project_id, (error, canRead) -> + @AuthorizationManager.canUserReadProject @user_id, @project_id, @token, (error, canRead) -> expect(canRead).to.equal true done() describe "when user has read-only access", -> beforeEach -> @AuthorizationManager.getPrivilegeLevelForProject - .withArgs(@req, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, "readOnly", false) it "should return true", (done) -> - @AuthorizationManager.canUserReadProject @req, @user_id, @project_id, (error, canRead) -> + @AuthorizationManager.canUserReadProject @user_id, @project_id, @token, (error, canRead) -> expect(canRead).to.equal true done() describe "when user has no access", -> beforeEach -> @AuthorizationManager.getPrivilegeLevelForProject - .withArgs(@req, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, false, false) it "should return false", (done) -> - @AuthorizationManager.canUserReadProject @req, @user_id, @project_id, (error, canRead) -> + @AuthorizationManager.canUserReadProject @user_id, @project_id, @token, (error, canRead) -> expect(canRead).to.equal false done() describe "canUserWriteProjectContent", -> beforeEach -> - @req = new MockRequest() @AuthorizationManager.getPrivilegeLevelForProject = sinon.stub() describe "when user is owner", -> beforeEach -> @AuthorizationManager.getPrivilegeLevelForProject - .withArgs(@req, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, "owner", false) it "should return true", (done) -> - @AuthorizationManager.canUserWriteProjectContent @req, @user_id, @project_id, (error, canWrite) -> + @AuthorizationManager.canUserWriteProjectContent @user_id, @project_id, @token, (error, canWrite) -> expect(canWrite).to.equal true done() describe "when user has read-write access", -> beforeEach -> @AuthorizationManager.getPrivilegeLevelForProject - .withArgs(@req, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, "readAndWrite", false) it "should return true", (done) -> - @AuthorizationManager.canUserWriteProjectContent @req, @user_id, @project_id, (error, canWrite) -> + @AuthorizationManager.canUserWriteProjectContent @user_id, @project_id, @token, (error, canWrite) -> expect(canWrite).to.equal true done() describe "when user has read-only access", -> beforeEach -> @AuthorizationManager.getPrivilegeLevelForProject - .withArgs(@req, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, "readOnly", false) it "should return false", (done) -> - @AuthorizationManager.canUserWriteProjectContent @req, @user_id, @project_id, (error, canWrite) -> + @AuthorizationManager.canUserWriteProjectContent @user_id, @project_id, @token, (error, canWrite) -> expect(canWrite).to.equal false done() describe "when user has no access", -> beforeEach -> @AuthorizationManager.getPrivilegeLevelForProject - .withArgs(@req, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, false, false) it "should return false", (done) -> - @AuthorizationManager.canUserWriteProjectContent @req, @user_id, @project_id, (error, canWrite) -> + @AuthorizationManager.canUserWriteProjectContent @user_id, @project_id, @token, (error, canWrite) -> expect(canWrite).to.equal false done() describe "canUserWriteProjectSettings", -> beforeEach -> - @req = new MockRequest() @AuthorizationManager.getPrivilegeLevelForProject = sinon.stub() describe "when user is owner", -> beforeEach -> @AuthorizationManager.getPrivilegeLevelForProject - .withArgs(@req, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, "owner", false) it "should return true", (done) -> - @AuthorizationManager.canUserWriteProjectSettings @req, @user_id, @project_id, (error, canWrite) -> + @AuthorizationManager.canUserWriteProjectSettings @user_id, @project_id, @token, (error, canWrite) -> expect(canWrite).to.equal true done() describe "when user has read-write access as a collaborator", -> beforeEach -> @AuthorizationManager.getPrivilegeLevelForProject - .withArgs(@req, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, "readAndWrite", false) it "should return true", (done) -> - @AuthorizationManager.canUserWriteProjectSettings @req, @user_id, @project_id, (error, canWrite) -> + @AuthorizationManager.canUserWriteProjectSettings @user_id, @project_id, @token, (error, canWrite) -> expect(canWrite).to.equal true done() describe "when user has read-write access as the public", -> beforeEach -> @AuthorizationManager.getPrivilegeLevelForProject - .withArgs(@req, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, "readAndWrite", true) it "should return false", (done) -> - @AuthorizationManager.canUserWriteProjectSettings @req, @user_id, @project_id, (error, canWrite) -> + @AuthorizationManager.canUserWriteProjectSettings @user_id, @project_id, @token, (error, canWrite) -> expect(canWrite).to.equal false done() describe "when user has read-only access", -> beforeEach -> @AuthorizationManager.getPrivilegeLevelForProject - .withArgs(@req, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, "readOnly", false) it "should return false", (done) -> - @AuthorizationManager.canUserWriteProjectSettings @req, @user_id, @project_id, (error, canWrite) -> + @AuthorizationManager.canUserWriteProjectSettings @user_id, @project_id, @token, (error, canWrite) -> expect(canWrite).to.equal false done() describe "when user has no access", -> beforeEach -> @AuthorizationManager.getPrivilegeLevelForProject - .withArgs(@req, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, false, false) it "should return false", (done) -> - @AuthorizationManager.canUserWriteProjectSettings @req, @user_id, @project_id, (error, canWrite) -> + @AuthorizationManager.canUserWriteProjectSettings @user_id, @project_id, @token, (error, canWrite) -> expect(canWrite).to.equal false done() describe "canUserAdminProject", -> beforeEach -> - @req = new MockRequest() @AuthorizationManager.getPrivilegeLevelForProject = sinon.stub() describe "when user is owner", -> beforeEach -> @AuthorizationManager.getPrivilegeLevelForProject - .withArgs(@req, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, "owner", false) it "should return true", (done) -> - @AuthorizationManager.canUserAdminProject @req, @user_id, @project_id, (error, canAdmin) -> + @AuthorizationManager.canUserAdminProject @user_id, @project_id, @token, (error, canAdmin) -> expect(canAdmin).to.equal true done() describe "when user has read-write access", -> beforeEach -> @AuthorizationManager.getPrivilegeLevelForProject - .withArgs(@req, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, "readAndWrite", false) it "should return false", (done) -> - @AuthorizationManager.canUserAdminProject @req, @user_id, @project_id, (error, canAdmin) -> + @AuthorizationManager.canUserAdminProject @user_id, @project_id, @token, (error, canAdmin) -> expect(canAdmin).to.equal false done() describe "when user has read-only access", -> beforeEach -> @AuthorizationManager.getPrivilegeLevelForProject - .withArgs(@req, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, "readOnly", false) it "should return false", (done) -> - @AuthorizationManager.canUserAdminProject @req, @user_id, @project_id, (error, canAdmin) -> + @AuthorizationManager.canUserAdminProject @user_id, @project_id, @token, (error, canAdmin) -> expect(canAdmin).to.equal false done() describe "when user has no access", -> beforeEach -> @AuthorizationManager.getPrivilegeLevelForProject - .withArgs(@req, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, false, false) it "should return false", (done) -> - @AuthorizationManager.canUserAdminProject @req, @user_id, @project_id, (error, canAdmin) -> + @AuthorizationManager.canUserAdminProject @user_id, @project_id, @token, (error, canAdmin) -> expect(canAdmin).to.equal false done() diff --git a/services/web/test/UnitTests/coffee/Authorization/AuthorizationMiddlewearTests.coffee b/services/web/test/UnitTests/coffee/Authorization/AuthorizationMiddlewearTests.coffee index f269782ba3..fc5b261d09 100644 --- a/services/web/test/UnitTests/coffee/Authorization/AuthorizationMiddlewearTests.coffee +++ b/services/web/test/UnitTests/coffee/Authorization/AuthorizationMiddlewearTests.coffee @@ -10,6 +10,7 @@ describe "AuthorizationMiddlewear", -> beforeEach -> @user_id = "user-id-123" @project_id = "project-id-123" + @token = 'some-token' @AuthenticationController = getLoggedInUserId: sinon.stub().returns(@user_id) isUserLoggedIn: sinon.stub().returns(true) @@ -19,6 +20,8 @@ describe "AuthorizationMiddlewear", -> "mongojs": ObjectId: @ObjectId = {} "../Errors/Errors": Errors '../Authentication/AuthenticationController': @AuthenticationController + "../TokenAccess/TokenAccessHandler": @TokenAccessHandler = + getRequestToken: sinon.stub().returns(@token) @req = {} @res = {} @ObjectId.isValid = sinon.stub() @@ -55,7 +58,7 @@ describe "AuthorizationMiddlewear", -> describe "when user has permission", -> beforeEach -> @AuthorizationManager[managerMethod] - .withArgs(sinon.match.any, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, true) it "should return next", -> @@ -65,7 +68,7 @@ describe "AuthorizationMiddlewear", -> describe "when user doesn't have permission", -> beforeEach -> @AuthorizationManager[managerMethod] - .withArgs(sinon.match.any, @user_id, @project_id) + .withArgs(@user_id, @project_id, @token) .yields(null, false) it "should redirect to redirectToRestricted", -> @@ -80,7 +83,7 @@ describe "AuthorizationMiddlewear", -> beforeEach -> @AuthenticationController.getLoggedInUserId.returns(null) @AuthorizationManager[managerMethod] - .withArgs(@req, null, @project_id) + .withArgs(null, @project_id, @token) .yields(null, true) it "should return next", -> @@ -91,7 +94,7 @@ describe "AuthorizationMiddlewear", -> beforeEach -> @AuthenticationController.getLoggedInUserId.returns(null) @AuthorizationManager[managerMethod] - .withArgs(@req, null, @project_id) + .withArgs(null, @project_id, @token) .yields(null, false) it "should redirect to redirectToRestricted", -> @@ -184,10 +187,10 @@ describe "AuthorizationMiddlewear", -> describe "when user has permission to access all projects", -> beforeEach -> @AuthorizationManager.canUserReadProject - .withArgs(sinon.match.any, @user_id, "project1") + .withArgs(@user_id, "project1", @token) .yields(null, true) @AuthorizationManager.canUserReadProject - .withArgs(sinon.match.any, @user_id, "project2") + .withArgs(@user_id, "project2", @token) .yields(null, true) it "should return next", -> @@ -197,10 +200,10 @@ describe "AuthorizationMiddlewear", -> describe "when user doesn't have permission to access one of the projects", -> beforeEach -> @AuthorizationManager.canUserReadProject - .withArgs(sinon.match.any, @user_id, "project1") + .withArgs(@user_id, "project1", @token) .yields(null, true) @AuthorizationManager.canUserReadProject - .withArgs(sinon.match.any, @user_id, "project2") + .withArgs(@user_id, "project2", @token) .yields(null, false) it "should redirect to redirectToRestricted", -> @@ -216,10 +219,10 @@ describe "AuthorizationMiddlewear", -> beforeEach -> @AuthenticationController.getLoggedInUserId.returns(null) @AuthorizationManager.canUserReadProject - .withArgs(sinon.match.any, null, "project1") + .withArgs(null, "project1", @token) .yields(null, true) @AuthorizationManager.canUserReadProject - .withArgs(sinon.match.any, null, "project2") + .withArgs(null, "project2", @token) .yields(null, true) it "should return next", -> @@ -230,10 +233,10 @@ describe "AuthorizationMiddlewear", -> beforeEach -> @AuthenticationController.getLoggedInUserId.returns(null) @AuthorizationManager.canUserReadProject - .withArgs(sinon.match.any, null, "project1") + .withArgs(null, "project1", @token) .yields(null, true) @AuthorizationManager.canUserReadProject - .withArgs(sinon.match.any, null, "project2") + .withArgs(null, "project2", @token) .yields(null, false) it "should redirect to redirectToRestricted", -> diff --git a/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee b/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee index 8bdb1e33b4..2ee9a10a93 100644 --- a/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee @@ -18,6 +18,7 @@ describe "EditorHttpController", -> 'metrics-sharelatex': @Metrics = {inc: sinon.stub()} "../Collaborators/CollaboratorsHandler": @CollaboratorsHandler = {} "../Collaborators/CollaboratorsInviteHandler": @CollaboratorsInviteHandler = {} + "../TokenAccess/TokenAccessHandler": @TokenAccessHandler = {} @project_id = "mock-project-id" @doc_id = "mock-doc-id" @@ -29,6 +30,7 @@ describe "EditorHttpController", -> sendStatus: sinon.stub() json: sinon.stub() @callback = sinon.stub() + @TokenAccessHandler.getRequestToken = sinon.stub().returns(@token = null) describe "joinProject", -> beforeEach -> @@ -136,7 +138,7 @@ describe "EditorHttpController", -> it "should check the privilege level", -> @AuthorizationManager.getPrivilegeLevelForProject - .calledWith(@req, @user_id, @project_id) + .calledWith(@user_id, @project_id, @token) .should.equal true it 'should include the invites', -> diff --git a/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee index 14f27ead68..bbae0bb91a 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee @@ -20,6 +20,7 @@ describe "ProjectController", -> chat: url:"chat.com" siteUrl: "mysite.com" + @token = 'some-token' @ProjectDeleter = archiveProject: sinon.stub().callsArg(1) deleteProject: sinon.stub().callsArg(1) @@ -60,6 +61,8 @@ describe "ProjectController", -> isUserLoggedIn: sinon.stub().returns(true) @AnalyticsManager = getLastOccurance: sinon.stub() + @TokenAccessHandler = + getRequestToken: sinon.stub().returns(@token) @ProjectController = SandboxedModule.require modulePath, requires: "settings-sharelatex":@settings "logger-sharelatex": @@ -85,6 +88,7 @@ describe "ProjectController", -> "./ProjectGetter": @ProjectGetter '../Authentication/AuthenticationController': @AuthenticationController "../Analytics/AnalyticsManager": @AnalyticsManager + "../TokenAccess/TokenAccessHandler": @TokenAccessHandler @projectName = "£12321jkj9ujkljds" @req = diff --git a/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessHandlerTests.coffee b/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessHandlerTests.coffee index 87bbc69ef0..6699b1dd4e 100644 --- a/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/TokenAccess/TokenAccessHandlerTests.coffee @@ -153,65 +153,53 @@ describe "TokenAccessHandler", -> done() - describe 'requestHasReadOnlyTokenAccess', -> + describe 'isValidReadOnlyToken', -> beforeEach -> - @req = {session: {}, headers: {}} @TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub() .callsArgWith(1, null, @project) - describe 'with header', -> - beforeEach -> - @req.headers['x-sl-anon-token'] = @token + it 'should call findProjectWithReadOnlyToken', (done) -> + @TokenAccessHandler.isValidReadOnlyToken @projectId, @token, (err, allowed) => + expect(@TokenAccessHandler.findProjectWithReadOnlyToken.callCount) + .to.equal 1 + done() - it 'should call findProjectWithReadOnlyToken', (done) -> - @TokenAccessHandler.requestHasReadOnlyTokenAccess @req, @projectId, (err, allowed) => - expect(@TokenAccessHandler.findProjectWithReadOnlyToken.callCount) - .to.equal 1 - done() - - it 'should allow access', (done) -> - @TokenAccessHandler.requestHasReadOnlyTokenAccess @req, @projectId, (err, allowed) => - expect(err).to.not.exist - expect(allowed).to.equal true - done() - - describe 'with session', -> - beforeEach -> - @req.session.anonReadOnlyTokenAccess = {} - @req.session.anonReadOnlyTokenAccess[@projectId.toString()] = @token + it 'should allow access', (done) -> + @TokenAccessHandler.isValidReadOnlyToken @projectId, @token, (err, allowed) => + expect(err).to.not.exist + expect(allowed).to.equal true + done() describe 'when no project is found', -> beforeEach -> - @req.headers['x-sl-anon-token'] = @token @TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub() .callsArgWith(1, null, null) it 'should call findProjectWithReadOnlyToken', (done) -> - @TokenAccessHandler.requestHasReadOnlyTokenAccess @req, @projectId, (err, allowed) => + @TokenAccessHandler.isValidReadOnlyToken @projectId, @token, (err, allowed) => expect(@TokenAccessHandler.findProjectWithReadOnlyToken.callCount) .to.equal 1 done() it 'should not allow access', (done) -> - @TokenAccessHandler.requestHasReadOnlyTokenAccess @req, @projectId, (err, allowed) => + @TokenAccessHandler.isValidReadOnlyToken @req, @projectId, (err, allowed) => expect(err).to.not.exist expect(allowed).to.equal false done() describe 'when no findProject produces an error', -> beforeEach -> - @req.headers['x-sl-anon-token'] = @token @TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub() .callsArgWith(1, new Error('woops')) it 'should call findProjectWithReadOnlyToken', (done) -> - @TokenAccessHandler.requestHasReadOnlyTokenAccess @req, @projectId, (err, allowed) => + @TokenAccessHandler.isValidReadOnlyToken @projectId, @token, (err, allowed) => expect(@TokenAccessHandler.findProjectWithReadOnlyToken.callCount) .to.equal 1 done() it 'should produce an error and not allow access', (done) -> - @TokenAccessHandler.requestHasReadOnlyTokenAccess @req, @projectId, (err, allowed) => + @TokenAccessHandler.isValidReadOnlyToken @projectId, @token, (err, allowed) => expect(err).to.exist expect(err).to.be.instanceof Error expect(allowed).to.equal undefined @@ -219,19 +207,18 @@ describe "TokenAccessHandler", -> describe 'when project is not set to token-based access', -> beforeEach -> - @req.headers['x-sl-anon-token'] = @token @project.publicAccesLevel = 'private' @TokenAccessHandler.findProjectWithReadOnlyToken = sinon.stub() .callsArgWith(1, null, @project) it 'should call findProjectWithReadOnlyToken', (done) -> - @TokenAccessHandler.requestHasReadOnlyTokenAccess @req, @projectId, (err, allowed) => + @TokenAccessHandler.isValidReadOnlyToken @projectId, @token, (err, allowed) => expect(@TokenAccessHandler.findProjectWithReadOnlyToken.callCount) .to.equal 1 done() it 'should not allow access', (done) -> - @TokenAccessHandler.requestHasReadOnlyTokenAccess @req, @projectId, (err, allowed) => + @TokenAccessHandler.isValidReadOnlyToken @projectId, @token, (err, allowed) => expect(err).to.not.exist expect(allowed).to.equal false done() @@ -240,13 +227,13 @@ describe "TokenAccessHandler", -> beforeEach -> it 'should not call findProjectWithReadOnlyToken', (done) -> - @TokenAccessHandler.requestHasReadOnlyTokenAccess @req, @projectId, (err, allowed) => + @TokenAccessHandler.isValidReadOnlyToken @projectId, null, (err, allowed) => expect(@TokenAccessHandler.findProjectWithReadOnlyToken.callCount) .to.equal 0 done() it 'should not allow access', (done) -> - @TokenAccessHandler.requestHasReadOnlyTokenAccess @req, @projectId, (err, allowed) => + @TokenAccessHandler.isValidReadOnlyToken @req, @projectId, (err, allowed) => expect(err).to.not.exist expect(allowed).to.equal false done()