From 1bd8b8d1a3427f55019054d08785feed8a432da6 Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 10 Mar 2016 17:17:26 +0000 Subject: [PATCH] Delete SecurityManager and replace with (unwritten) AuthorizationManager --- .../Authorization/AuthorizationManager.coffee | 12 ++ .../AuthorizationMiddlewear.coffee | 27 +++ .../Collaborators/CollaboratorsRouter.coffee | 6 +- .../Editor/EditorHttpController.coffee | 4 +- .../Features/Editor/EditorRouter.coffee | 18 +- .../Features/Project/ProjectController.coffee | 5 +- .../Security/AuthorizationManager.coffee | 38 ---- .../SubscriptionController.coffee | 26 +-- .../Features/Uploads/UploadsRouter.coffee | 4 +- .../coffee/managers/SecurityManager.coffee | 194 ------------------ services/web/app/coffee/router.coffee | 77 ++++--- .../Editor/EditorHttpControllerTests.coffee | 4 +- .../Project/ProjectControllerTests.coffee | 10 +- .../Security/AuthorizationManagerTests.coffee | 96 --------- .../SubscriptionControllerTests.coffee | 6 +- 15 files changed, 119 insertions(+), 408 deletions(-) create mode 100644 services/web/app/coffee/Features/Authorization/AuthorizationManager.coffee create mode 100644 services/web/app/coffee/Features/Authorization/AuthorizationMiddlewear.coffee delete mode 100644 services/web/app/coffee/Features/Security/AuthorizationManager.coffee delete mode 100644 services/web/app/coffee/managers/SecurityManager.coffee delete mode 100644 services/web/test/UnitTests/coffee/Security/AuthorizationManagerTests.coffee diff --git a/services/web/app/coffee/Features/Authorization/AuthorizationManager.coffee b/services/web/app/coffee/Features/Authorization/AuthorizationManager.coffee new file mode 100644 index 0000000000..a6a80edae0 --- /dev/null +++ b/services/web/app/coffee/Features/Authorization/AuthorizationManager.coffee @@ -0,0 +1,12 @@ +module.exports = + getPrivilegeLevelForProject: (user_id, project_id, callback = (error, canAccess, privilegeLevel) ->) -> + return callback(null, true, "readAndWrite") + + canUserReadProject: (user_id, project_id, callback = (error, canRead) ->) -> + + canUserWriteProjectSettings: (user_id, project_id, callback = (error, canWriteSettings) ->) -> + + canUserAdminProject: (user_id, project_id, callback = (error, canAdmin) ->) -> + + isUserSiteAdmin: (user_id, callback = (error, isAdmin) ->) -> + \ No newline at end of file diff --git a/services/web/app/coffee/Features/Authorization/AuthorizationMiddlewear.coffee b/services/web/app/coffee/Features/Authorization/AuthorizationMiddlewear.coffee new file mode 100644 index 0000000000..a3bce5a2cc --- /dev/null +++ b/services/web/app/coffee/Features/Authorization/AuthorizationMiddlewear.coffee @@ -0,0 +1,27 @@ +module.exports = + ensureUserCanReadMultipleProjects: (req, res, next) -> + next() + + ensureUserCanReadProject: (req, res, next) -> + next() + + ensureUserCanWriteProjectSettings: (req, res, next) -> + next() + + ensureUserCanWriteProjectContent: (req, res, next) -> + next() + + ensureUserCanAdminProject: (req, res, next) -> + next() + + ensureUserIsSiteAdmin: (req, res, next) -> + next() + + restricted : (req, res, next)-> + if req.session.user? + res.render 'user/restricted', + title:'restricted' + else + logger.log "user not logged in and trying to access #{req.url}, being redirected to login" + res.redirect '/register' + \ No newline at end of file diff --git a/services/web/app/coffee/Features/Collaborators/CollaboratorsRouter.coffee b/services/web/app/coffee/Features/Collaborators/CollaboratorsRouter.coffee index f4f1b0343c..34a6da9a02 100644 --- a/services/web/app/coffee/Features/Collaborators/CollaboratorsRouter.coffee +++ b/services/web/app/coffee/Features/Collaborators/CollaboratorsRouter.coffee @@ -1,10 +1,10 @@ CollaboratorsController = require('./CollaboratorsController') -SecurityManager = require('../../managers/SecurityManager') AuthenticationController = require('../Authentication/AuthenticationController') +AuthorizationMiddlewear = require('../Authorization/AuthorizationMiddlewear') module.exports = apply: (webRouter, apiRouter) -> webRouter.post '/project/:Project_id/leave', AuthenticationController.requireLogin(), CollaboratorsController.removeSelfFromProject - webRouter.post '/project/:Project_id/users', SecurityManager.requestIsOwner, CollaboratorsController.addUserToProject - webRouter.delete '/project/:Project_id/users/:user_id', SecurityManager.requestIsOwner, CollaboratorsController.removeUserFromProject + webRouter.post '/project/:Project_id/users', AuthorizationMiddlewear.ensureUserCanAdminProject, CollaboratorsController.addUserToProject + webRouter.delete '/project/:Project_id/users/:user_id', AuthorizationMiddlewear.ensureUserCanAdminProject, CollaboratorsController.removeUserFromProject diff --git a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee index 18e8f7d308..776a1c1fb0 100644 --- a/services/web/app/coffee/Features/Editor/EditorHttpController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorHttpController.coffee @@ -5,7 +5,7 @@ EditorRealTimeController = require "./EditorRealTimeController" EditorController = require "./EditorController" ProjectGetter = require('../Project/ProjectGetter') UserGetter = require('../User/UserGetter') -AuthorizationManager = require("../Security/AuthorizationManager") +AuthorizationManager = require("../Authorization/AuthorizationManager") ProjectEditorHandler = require('../Project/ProjectEditorHandler') Metrics = require('../../infrastructure/Metrics') CollaboratorsHandler = require("../Collaborators/CollaboratorsHandler") @@ -34,7 +34,7 @@ module.exports = EditorHttpController = return callback(error) if error? UserGetter.getUser user_id, { isAdmin: true }, (error, user) -> return callback(error) if error? - AuthorizationManager.getPrivilegeLevelForProject project, user, (error, canAccess, privilegeLevel) -> + AuthorizationManager.getPrivilegeLevelForProject user_id, project_id, (error, canAccess, privilegeLevel) -> return callback(error) if error? if !canAccess callback null, null, false diff --git a/services/web/app/coffee/Features/Editor/EditorRouter.coffee b/services/web/app/coffee/Features/Editor/EditorRouter.coffee index 0576a4adce..9de1544875 100644 --- a/services/web/app/coffee/Features/Editor/EditorRouter.coffee +++ b/services/web/app/coffee/Features/Editor/EditorRouter.coffee @@ -1,20 +1,20 @@ EditorHttpController = require('./EditorHttpController') -SecurityManager = require('../../managers/SecurityManager') AuthenticationController = require "../Authentication/AuthenticationController" +AuthorizationMiddlewear = require('../Authorization/AuthorizationMiddlewear') module.exports = apply: (webRouter, apiRouter) -> - webRouter.post '/project/:Project_id/doc', SecurityManager.requestCanModifyProject, EditorHttpController.addDoc - webRouter.post '/project/:Project_id/folder', SecurityManager.requestCanModifyProject, EditorHttpController.addFolder + webRouter.post '/project/:Project_id/doc', AuthorizationMiddlewear.ensureUserCanWriteProjectContent, EditorHttpController.addDoc + webRouter.post '/project/:Project_id/folder', AuthorizationMiddlewear.ensureUserCanWriteProjectContent, EditorHttpController.addFolder - webRouter.post '/project/:Project_id/:entity_type/:entity_id/rename', SecurityManager.requestCanModifyProject, EditorHttpController.renameEntity - webRouter.post '/project/:Project_id/:entity_type/:entity_id/move', SecurityManager.requestCanModifyProject, EditorHttpController.moveEntity + webRouter.post '/project/:Project_id/:entity_type/:entity_id/rename', AuthorizationMiddlewear.ensureUserCanWriteProjectContent, EditorHttpController.renameEntity + webRouter.post '/project/:Project_id/:entity_type/:entity_id/move', AuthorizationMiddlewear.ensureUserCanWriteProjectContent, EditorHttpController.moveEntity - webRouter.delete '/project/:Project_id/file/:entity_id', SecurityManager.requestCanModifyProject, EditorHttpController.deleteFile - webRouter.delete '/project/:Project_id/doc/:entity_id', SecurityManager.requestCanModifyProject, EditorHttpController.deleteDoc - webRouter.delete '/project/:Project_id/folder/:entity_id', SecurityManager.requestCanModifyProject, EditorHttpController.deleteFolder + webRouter.delete '/project/:Project_id/file/:entity_id', AuthorizationMiddlewear.ensureUserCanWriteProjectContent, EditorHttpController.deleteFile + webRouter.delete '/project/:Project_id/doc/:entity_id', AuthorizationMiddlewear.ensureUserCanWriteProjectContent, EditorHttpController.deleteDoc + webRouter.delete '/project/:Project_id/folder/:entity_id', AuthorizationMiddlewear.ensureUserCanWriteProjectContent, EditorHttpController.deleteFolder - webRouter.post '/project/:Project_id/doc/:doc_id/restore', SecurityManager.requestCanModifyProject, EditorHttpController.restoreDoc + webRouter.post '/project/:Project_id/doc/:doc_id/restore', AuthorizationMiddlewear.ensureUserCanWriteProjectContent, EditorHttpController.restoreDoc # Called by the real-time API to load up the current project state. # This is a post request because it's more than just a getting of data. We take actions diff --git a/services/web/app/coffee/Features/Project/ProjectController.coffee b/services/web/app/coffee/Features/Project/ProjectController.coffee index 0ca14090c2..ba5ff559f0 100644 --- a/services/web/app/coffee/Features/Project/ProjectController.coffee +++ b/services/web/app/coffee/Features/Project/ProjectController.coffee @@ -13,7 +13,7 @@ NotificationsHandler = require("../Notifications/NotificationsHandler") LimitationsManager = require("../Subscription/LimitationsManager") _ = require("underscore") Settings = require("settings-sharelatex") -SecurityManager = require("../../managers/SecurityManager") +AuthorizationManager = require("../Authorization/AuthorizationManager") fs = require "fs" InactiveProjectManager = require("../InactiveData/InactiveProjectManager") ProjectUpdateHandler = require("./ProjectUpdateHandler") @@ -225,7 +225,8 @@ module.exports = ProjectController = daysSinceLastUpdated = (new Date() - project.lastUpdated) /86400000 logger.log project_id:project_id, daysSinceLastUpdated:daysSinceLastUpdated, "got db results for loading editor" - SecurityManager.userCanAccessProject user, project, (canAccess, privilegeLevel)-> + AuthorizationManager.getPrivilegeLevelForProject user_id, project_id, (error, canAccess, privilegeLevel)-> + return next(error) if error? if !canAccess return res.sendStatus 401 diff --git a/services/web/app/coffee/Features/Security/AuthorizationManager.coffee b/services/web/app/coffee/Features/Security/AuthorizationManager.coffee deleted file mode 100644 index 0ec4985f35..0000000000 --- a/services/web/app/coffee/Features/Security/AuthorizationManager.coffee +++ /dev/null @@ -1,38 +0,0 @@ -SecurityManager = require '../../managers/SecurityManager' - -module.exports = AuthorizationManager = - getPrivilegeLevelForProject: ( - project, user, - callback = (error, canAccess, privilegeLevel)-> - ) -> - # This is not tested because eventually this function should be brought into - # this module. - SecurityManager.userCanAccessProject user, project, (canAccess, privilegeLevel) -> - if canAccess - callback null, true, privilegeLevel - else - callback null, false - - setPrivilegeLevelOnClient: (client, privilegeLevel) -> - client.set("privilege_level", privilegeLevel) - - ensureClientCanViewProject: (client, callback = (error, project_id)->) -> - @ensureClientHasPrivilegeLevelForProject client, ["owner", "readAndWrite", "readOnly"], callback - - ensureClientCanEditProject: (client, callback = (error, project_id)->) -> - @ensureClientHasPrivilegeLevelForProject client, ["owner", "readAndWrite"], callback - - ensureClientCanAdminProject: (client, callback = (error, project_id)->) -> - @ensureClientHasPrivilegeLevelForProject client, ["owner"], callback - - ensureClientHasPrivilegeLevelForProject: (client, levels, callback = (error, project_id)->) -> - client.get "privilege_level", (error, level) -> - return callback(error) if error? - if level? - client.get "project_id", (error, project_id) -> - return callback(error) if error? - if project_id? - if levels.indexOf(level) > -1 - callback null, project_id - - diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee index a2287a5d44..ba3d7b2115 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee @@ -1,4 +1,4 @@ -SecurityManager = require '../../managers/SecurityManager' +AuthenticationController = require '../Authentication/AuthenticationController' SubscriptionHandler = require './SubscriptionHandler' PlansLocator = require("./PlansLocator") SubscriptionFormatters = require("./SubscriptionFormatters") @@ -32,7 +32,7 @@ module.exports = SubscriptionController = #get to show the recurly.js page paymentPage: (req, res, next) -> - SecurityManager.getCurrentUser req, (error, user) => + AuthenticationController.getLoggedInUser req, (error, user) => return next(error) if error? plan = PlansLocator.findLocalPlanInSettings(req.query.planCode) LimitationsManager.userHasSubscription user, (err, hasSubscription)-> @@ -81,7 +81,7 @@ module.exports = SubscriptionController = userSubscriptionPage: (req, res, next) -> - SecurityManager.getCurrentUser req, (error, user) => + AuthenticationController.getLoggedInUser req, (error, user) => return next(error) if error? LimitationsManager.userHasSubscriptionOrIsGroupMember user, (err, hasSubOrIsGroupMember, subscription)-> groupLicenceInviteUrl = SubscriptionDomainHandler.getDomainLicencePage(user) @@ -110,7 +110,7 @@ module.exports = SubscriptionController = userCustomSubscriptionPage: (req, res, next)-> - SecurityManager.getCurrentUser req, (error, user) -> + AuthenticationController.getLoggedInUser req, (error, user) -> LimitationsManager.userHasSubscriptionOrIsGroupMember user, (err, hasSubOrIsGroupMember, subscription)-> res.render "subscriptions/custom_account", title: "your_subscription" @@ -118,7 +118,7 @@ module.exports = SubscriptionController = editBillingDetailsPage: (req, res, next) -> - SecurityManager.getCurrentUser req, (error, user) -> + AuthenticationController.getLoggedInUser req, (error, user) -> return next(error) if error? LimitationsManager.userHasSubscription user, (err, hasSubscription)-> if !hasSubscription @@ -139,7 +139,7 @@ module.exports = SubscriptionController = id : user.id createSubscription: (req, res, next)-> - SecurityManager.getCurrentUser req, (error, user) -> + AuthenticationController.getLoggedInUser req, (error, user) -> return callback(error) if error? recurly_token_id = req.body.recurly_token_id subscriptionDetails = req.body.subscriptionDetails @@ -151,14 +151,14 @@ module.exports = SubscriptionController = res.sendStatus 201 successful_subscription: (req, res)-> - SecurityManager.getCurrentUser req, (error, user) => + AuthenticationController.getLoggedInUser req, (error, user) => SubscriptionViewModelBuilder.buildUsersSubscriptionViewModel user, (error, subscription) -> res.render "subscriptions/successful_subscription", title: "thank_you" subscription:subscription cancelSubscription: (req, res, next) -> - SecurityManager.getCurrentUser req, (error, user) -> + AuthenticationController.getLoggedInUser req, (error, user) -> logger.log user_id:user._id, "canceling subscription" return next(error) if error? SubscriptionHandler.cancelSubscription user, (err)-> @@ -167,7 +167,7 @@ module.exports = SubscriptionController = res.redirect "/user/subscription" updateSubscription: (req, res)-> - SecurityManager.getCurrentUser req, (error, user) -> + AuthenticationController.getLoggedInUser req, (error, user) -> return next(error) if error? planCode = req.body.plan_code logger.log planCode: planCode, user_id:user._id, "updating subscription" @@ -177,7 +177,7 @@ module.exports = SubscriptionController = res.redirect "/user/subscription" reactivateSubscription: (req, res)-> - SecurityManager.getCurrentUser req, (error, user) -> + AuthenticationController.getLoggedInUser req, (error, user) -> logger.log user_id:user._id, "reactivating subscription" return next(error) if error? SubscriptionHandler.reactivateSubscription user, (err)-> @@ -196,7 +196,7 @@ module.exports = SubscriptionController = res.sendStatus 200 renderUpgradeToAnnualPlanPage: (req, res)-> - SecurityManager.getCurrentUser req, (error, user) -> + AuthenticationController.getLoggedInUser req, (error, user) -> LimitationsManager.userHasSubscription user, (err, hasSubscription, subscription)-> planCode = subscription?.planCode.toLowerCase() if planCode?.indexOf("annual") != -1 @@ -213,7 +213,7 @@ module.exports = SubscriptionController = planName: planName processUpgradeToAnnualPlan: (req, res)-> - SecurityManager.getCurrentUser req, (error, user) -> + AuthenticationController.getLoggedInUser req, (error, user) -> {planName} = req.body coupon_code = Settings.coupon_codes.upgradeToAnnualPromo[planName] annualPlanName = "#{planName}-annual" @@ -226,7 +226,7 @@ module.exports = SubscriptionController = res.sendStatus 200 extendTrial: (req, res)-> - SecurityManager.getCurrentUser req, (error, user) -> + AuthenticationController.getLoggedInUser req, (error, user) -> LimitationsManager.userHasSubscription user, (err, hasSubscription, subscription)-> SubscriptionHandler.extendTrial subscription, 14, (err)-> if err? diff --git a/services/web/app/coffee/Features/Uploads/UploadsRouter.coffee b/services/web/app/coffee/Features/Uploads/UploadsRouter.coffee index c82144acc0..fdff8d0ea3 100644 --- a/services/web/app/coffee/Features/Uploads/UploadsRouter.coffee +++ b/services/web/app/coffee/Features/Uploads/UploadsRouter.coffee @@ -1,4 +1,4 @@ -SecurityManager = require('../../managers/SecurityManager') +AuthorizationMiddlewear = require('../Authorization/AuthorizationMiddlewear') AuthenticationController = require('../Authentication/AuthenticationController') ProjectUploadController = require "./ProjectUploadController" RateLimiterMiddlewear = require('../Security/RateLimiterMiddlewear') @@ -16,6 +16,6 @@ module.exports = maxRequests: 200 timeInterval: 60 * 30 }), - SecurityManager.requestCanModifyProject, + AuthorizationMiddlewear.ensureUserCanWriteProjectContent, ProjectUploadController.uploadFile diff --git a/services/web/app/coffee/managers/SecurityManager.coffee b/services/web/app/coffee/managers/SecurityManager.coffee deleted file mode 100644 index 699c307679..0000000000 --- a/services/web/app/coffee/managers/SecurityManager.coffee +++ /dev/null @@ -1,194 +0,0 @@ -logger = require('logger-sharelatex') -crypto = require 'crypto' -Assert = require 'assert' -Settings = require 'settings-sharelatex' -User = require('../models/User').User -Project = require('../models/Project').Project -ErrorController = require("../Features/Errors/ErrorController") -AuthenticationController = require("../Features/Authentication/AuthenticationController") -_ = require('underscore') -metrics = require('../infrastructure/Metrics') -querystring = require('querystring') -async = require "async" - -module.exports = SecurityManager = - restricted : (req, res, next)-> - if req.session.user? - res.render 'user/restricted', - title:'restricted' - else - logger.log "user not logged in and trying to access #{req.url}, being redirected to login" - res.redirect '/register' - - getCurrentUser: (req, callback) -> - if req.session.user? - User.findById req.session.user._id, callback - else - callback null, null - - requestCanAccessMultipleProjects: (req, res, next) -> - project_ids = req.query.project_ids?.split(",") - jobs = [] - for project_id in project_ids or [] - do (project_id) -> - jobs.push (callback) -> - # This is a bit hacky - better to have an abstracted method - # that we can pass project_id to, but this whole file needs - # a serious refactor ATM. - req.params.Project_id = project_id - SecurityManager.requestCanAccessProject req, res, (error) -> - delete req.params.Project_id - callback(error) - async.series jobs, next - - requestCanAccessProject : (req, res, next)-> - doRequest = (req, res, next) -> - getRequestUserAndProject req, res, {allow_auth_token: options?.allow_auth_token}, (err, user, project)-> - if !project? or project.archived - return ErrorController.notFound(req, res, next) - userCanAccessProject user, project, (canAccess, permissionLevel)-> - if canAccess - next() - else if user? - logger.log "user_id: #{user._id} email: #{user.email} trying to access restricted page #{req.path}" - res.redirect('/restricted') - else - logger.log "user not logged in and trying to access #{req.url}, being redirected to login" - AuthenticationController._redirectToLoginOrRegisterPage(req, res) - if arguments.length > 1 - options = - allow_auth_token: false - doRequest.apply(this, arguments) - else - options = req - return doRequest - - requestCanModifyProject : (req, res, next)-> - getRequestUserAndProject req, res, {}, (err, user, project)=> - userCanModifyProject user, project, (canModify)-> - if canModify - next() - else - logger.log "user_id: #{user?._id} email: #{user?.email} can not modify project redirecting to restricted page" - res.redirect('/restricted') - - userCanModifyProject : userCanModifyProject = (user, project, callback)-> - if !user? or !project? - callback false - else if userIsOwner user, project - callback true - else if userIsCollaberator user, project - callback true - else if project.publicAccesLevel == "readAndWrite" - callback true - else if user.isAdmin - callback true - else - callback false - - - requestIsOwner : (req, res, next)-> - getRequestUserAndProject req, res, {}, (err, user, project)-> - if !user? - return res.redirect('/restricted') - else if userIsOwner user, project || user.isAdmin - next() - else - logger.log user_id: user?._id, email: user?.email, "user is not owner of project redirecting to restricted page" - res.redirect('/restricted') - - requestIsAdmin : isAdmin = (req, res, next)-> - logger.log "checking if user is admin" - user = req.session.user - if(user? && user.isAdmin) - logger.log user: user, "User is admin" - next() - else - res.redirect('/restricted') - logger.log user:user, "is not admin redirecting to restricted page" - - userCanAccessProject : userCanAccessProject = (user, project, callback)=> - if !user? - user = {_id:'anonymous-user'} - if !project? - callback false - logger.log user:user, project:project, "Checking if can access" - if userIsOwner user, project - callback true, "owner" - else if userIsCollaberator user, project - callback true, "readAndWrite" - else if userIsReadOnly user, project - callback true, "readOnly" - else if user.isAdmin - logger.log user:user, project:project, "user is admin and can access project" - callback true, "owner" - else if project.publicAccesLevel == "readAndWrite" - logger.log user:user, project:project, "project is a public read and write project" - callback true, "readAndWrite" - else if project.publicAccesLevel == "readOnly" - logger.log user:user, project:project, "project is a public read only project" - callback true, "readOnly" - else - metrics.inc "security.denied" - logger.log user:user, project:project, "Security denied - user can not enter project" - callback false - - userIsOwner : userIsOwner = (user, project)-> - if !user? - return false - else - userId = user._id+'' - ownerRef = getProjectIdFromRef(project.owner_ref) - if userId == ownerRef - true - else - false - - userIsCollaberator : userIsCollaberator = (user, project)-> - if !user? - return false - else - userId = user._id+'' - result = false - _.each project.collaberator_refs, (colabRef)-> - colabRef = getProjectIdFromRef(colabRef) - if colabRef == userId - result = true - return result - - userIsReadOnly : userIsReadOnly = (user, project)-> - if !user? - return false - else - userId = user._id+'' - result = false - _.each project.readOnly_refs, (readOnlyRef)-> - readOnlyRef = getProjectIdFromRef(readOnlyRef) - - if readOnlyRef == userId - result = true - return result - -getRequestUserAndProject = (req, res, options, callback)-> - project_id = req.params.Project_id - if !project_id? - logger.log project_id:project_id, options:options, url:req?.url, "no project_id trying to getRequestUserAndProject" - return res.send 422 - Project.findById project_id, 'name owner_ref readOnly_refs collaberator_refs publicAccesLevel archived', (err, project)=> - if err? - logger.err err:err, "error getting project for security check" - return callback err - AuthenticationController.getLoggedInUser req, options, (err, user)=> - if err? - logger.err err:err, "error getting last logged in user for security check" - callback err, user, project - -getProjectIdFromRef = (ref)-> - if !ref? - return null - else if ref._id? - return ref._id+'' - else - return ref+'' - - diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 36b1bd7ea2..305b3bd564 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -3,8 +3,6 @@ ErrorController = require('./Features/Errors/ErrorController') ProjectController = require("./Features/Project/ProjectController") ProjectApiController = require("./Features/Project/ProjectApiController") SpellingController = require('./Features/Spelling/SpellingController') -SecurityManager = require('./managers/SecurityManager') -AuthorizationManager = require('./Features/Security/AuthorizationManager') EditorController = require("./Features/Editor/EditorController") EditorRouter = require("./Features/Editor/EditorRouter") Settings = require('settings-sharelatex') @@ -39,6 +37,7 @@ RealTimeProxyRouter = require('./Features/RealTimeProxy/RealTimeProxyRouter') InactiveProjectController = require("./Features/InactiveData/InactiveProjectController") ContactRouter = require("./Features/Contacts/ContactRouter") ReferencesController = require('./Features/References/ReferencesController') +AuthorizationMiddlewear = require('./Features/Authorization/AuthorizationMiddlewear') logger = require("logger-sharelatex") _ = require("underscore") @@ -54,7 +53,7 @@ module.exports = class Router webRouter.post '/login', AuthenticationController.login webRouter.get '/logout', UserController.logout - webRouter.get '/restricted', SecurityManager.restricted + webRouter.get '/restricted', AuthorizationMiddlewear.restricted # Left as a placeholder for implementing a public register page webRouter.get '/register', UserPagesController.registerPage @@ -99,13 +98,13 @@ module.exports = class Router params: ["Project_id"] maxRequests: 10 timeInterval: 60 - }), SecurityManager.requestCanAccessProject, ProjectController.loadEditor - webRouter.get '/Project/:Project_id/file/:File_id', SecurityManager.requestCanAccessProject, FileStoreController.getFile - webRouter.post '/project/:Project_id/settings', SecurityManager.requestCanModifyProject, ProjectController.updateProjectSettings - webRouter.post '/project/:Project_id/settings/admin', SecurityManager.requestIsOwner, ProjectController.updateProjectAdminSettings + }), AuthorizationMiddlewear.ensureUserCanReadProject, ProjectController.loadEditor + webRouter.get '/Project/:Project_id/file/:File_id', AuthorizationMiddlewear.ensureUserCanReadProject, FileStoreController.getFile + webRouter.post '/project/:Project_id/settings', AuthorizationMiddlewear.ensureUserCanWriteProjectSettings, ProjectController.updateProjectSettings + webRouter.post '/project/:Project_id/settings/admin', AuthorizationMiddlewear.ensureUserCanAdminProject, ProjectController.updateProjectAdminSettings - webRouter.post '/project/:Project_id/compile', SecurityManager.requestCanAccessProject, CompileController.compile - webRouter.get '/Project/:Project_id/output/output.pdf', SecurityManager.requestCanAccessProject, CompileController.downloadPdf + webRouter.post '/project/:Project_id/compile', AuthorizationMiddlewear.ensureUserCanReadProject, CompileController.compile + webRouter.get '/Project/:Project_id/output/output.pdf', AuthorizationMiddlewear.ensureUserCanReadProject, CompileController.downloadPdf webRouter.get /^\/project\/([^\/]*)\/output\/(.*)$/, ((req, res, next) -> params = @@ -113,24 +112,24 @@ module.exports = class Router "file": req.params[1] req.params = params next() - ), SecurityManager.requestCanAccessProject, CompileController.getFileFromClsi - webRouter.delete "/project/:Project_id/output", SecurityManager.requestCanAccessProject, CompileController.deleteAuxFiles - webRouter.get "/project/:Project_id/sync/code", SecurityManager.requestCanAccessProject, CompileController.proxySync - webRouter.get "/project/:Project_id/sync/pdf", SecurityManager.requestCanAccessProject, CompileController.proxySync - webRouter.get "/project/:Project_id/wordcount", SecurityManager.requestCanAccessProject, CompileController.wordCount + ), AuthorizationMiddlewear.ensureUserCanReadProject, CompileController.getFileFromClsi + webRouter.delete "/project/:Project_id/output", AuthorizationMiddlewear.ensureUserCanReadProject, CompileController.deleteAuxFiles + webRouter.get "/project/:Project_id/sync/code", AuthorizationMiddlewear.ensureUserCanReadProject, CompileController.proxySync + webRouter.get "/project/:Project_id/sync/pdf", AuthorizationMiddlewear.ensureUserCanReadProject, CompileController.proxySync + webRouter.get "/project/:Project_id/wordcount", AuthorizationMiddlewear.ensureUserCanReadProject, CompileController.wordCount - webRouter.delete '/Project/:Project_id', SecurityManager.requestIsOwner, ProjectController.deleteProject - webRouter.post '/Project/:Project_id/restore', SecurityManager.requestIsOwner, ProjectController.restoreProject - webRouter.post '/Project/:Project_id/clone', SecurityManager.requestCanAccessProject, ProjectController.cloneProject + webRouter.delete '/Project/:Project_id', AuthorizationMiddlewear.ensureUserCanAdminProject, ProjectController.deleteProject + webRouter.post '/Project/:Project_id/restore', AuthorizationMiddlewear.ensureUserCanAdminProject, ProjectController.restoreProject + webRouter.post '/Project/:Project_id/clone', AuthorizationMiddlewear.ensureUserCanReadProject, ProjectController.cloneProject - webRouter.post '/project/:Project_id/rename', SecurityManager.requestIsOwner, ProjectController.renameProject + webRouter.post '/project/:Project_id/rename', AuthorizationMiddlewear.ensureUserCanAdminProject, ProjectController.renameProject - webRouter.get "/project/:Project_id/updates", SecurityManager.requestCanAccessProject, TrackChangesController.proxyToTrackChangesApi - webRouter.get "/project/:Project_id/doc/:doc_id/diff", SecurityManager.requestCanAccessProject, TrackChangesController.proxyToTrackChangesApi - webRouter.post "/project/:Project_id/doc/:doc_id/version/:version_id/restore", SecurityManager.requestCanAccessProject, TrackChangesController.proxyToTrackChangesApi + webRouter.get "/project/:Project_id/updates", AuthorizationMiddlewear.ensureUserCanReadProject, TrackChangesController.proxyToTrackChangesApi + webRouter.get "/project/:Project_id/doc/:doc_id/diff", AuthorizationMiddlewear.ensureUserCanReadProject, TrackChangesController.proxyToTrackChangesApi + webRouter.post "/project/:Project_id/doc/:doc_id/version/:version_id/restore", AuthorizationMiddlewear.ensureUserCanReadProject, TrackChangesController.proxyToTrackChangesApi - webRouter.get '/Project/:Project_id/download/zip', SecurityManager.requestCanAccessProject, ProjectDownloadsController.downloadProject - webRouter.get '/project/download/zip', SecurityManager.requestCanAccessMultipleProjects, ProjectDownloadsController.downloadMultipleProjects + webRouter.get '/Project/:Project_id/download/zip', AuthorizationMiddlewear.ensureUserCanReadProject, ProjectDownloadsController.downloadProject + webRouter.get '/project/download/zip', AuthorizationMiddlewear.ensureUserCanReadMultipleProjects, ProjectDownloadsController.downloadMultipleProjects webRouter.get '/tag', AuthenticationController.requireLogin(), TagsController.getAllTags webRouter.post '/tag', AuthenticationController.requireLogin(), TagsController.createTag @@ -174,26 +173,26 @@ module.exports = class Router webRouter.post "/spelling/check", AuthenticationController.requireLogin(), SpellingController.proxyRequestToSpellingApi webRouter.post "/spelling/learn", AuthenticationController.requireLogin(), SpellingController.proxyRequestToSpellingApi - webRouter.get "/project/:Project_id/messages", SecurityManager.requestCanAccessProject, ChatController.getMessages - webRouter.post "/project/:Project_id/messages", SecurityManager.requestCanAccessProject, ChatController.sendMessage + webRouter.get "/project/:Project_id/messages", AuthorizationMiddlewear.ensureUserCanReadProject, ChatController.getMessages + webRouter.post "/project/:Project_id/messages", AuthorizationMiddlewear.ensureUserCanReadProject, ChatController.sendMessage webRouter.get /learn(\/.*)?/, WikiController.getPage - webRouter.post "/project/:Project_id/references/index", SecurityManager.requestCanAccessProject, ReferencesController.index - webRouter.post "/project/:Project_id/references/indexAll", SecurityManager.requestCanAccessProject, ReferencesController.indexAll + webRouter.post "/project/:Project_id/references/index", AuthorizationMiddlewear.ensureUserCanReadProject, ReferencesController.index + webRouter.post "/project/:Project_id/references/indexAll", AuthorizationMiddlewear.ensureUserCanReadProject, ReferencesController.indexAll #Admin Stuff - webRouter.get '/admin', SecurityManager.requestIsAdmin, AdminController.index - webRouter.get '/admin/user', SecurityManager.requestIsAdmin, (req, res)-> res.redirect("/admin/register") #this gets removed by admin-panel addon - webRouter.get '/admin/register', SecurityManager.requestIsAdmin, AdminController.registerNewUser - webRouter.post '/admin/register', SecurityManager.requestIsAdmin, UserController.register - webRouter.post '/admin/closeEditor', SecurityManager.requestIsAdmin, AdminController.closeEditor - webRouter.post '/admin/dissconectAllUsers', SecurityManager.requestIsAdmin, AdminController.dissconectAllUsers - webRouter.post '/admin/syncUserToSubscription', SecurityManager.requestIsAdmin, AdminController.syncUserToSubscription - webRouter.post '/admin/flushProjectToTpds', SecurityManager.requestIsAdmin, AdminController.flushProjectToTpds - webRouter.post '/admin/pollDropboxForUser', SecurityManager.requestIsAdmin, AdminController.pollDropboxForUser - webRouter.post '/admin/messages', SecurityManager.requestIsAdmin, AdminController.createMessage - webRouter.post '/admin/messages/clear', SecurityManager.requestIsAdmin, AdminController.clearMessages + webRouter.get '/admin', AuthorizationMiddlewear.ensureUserIsSiteAdmin, AdminController.index + webRouter.get '/admin/user', AuthorizationMiddlewear.ensureUserIsSiteAdmin, (req, res)-> res.redirect("/admin/register") #this gets removed by admin-panel addon + webRouter.get '/admin/register', AuthorizationMiddlewear.ensureUserIsSiteAdmin, AdminController.registerNewUser + webRouter.post '/admin/register', AuthorizationMiddlewear.ensureUserIsSiteAdmin, UserController.register + webRouter.post '/admin/closeEditor', AuthorizationMiddlewear.ensureUserIsSiteAdmin, AdminController.closeEditor + webRouter.post '/admin/dissconectAllUsers', AuthorizationMiddlewear.ensureUserIsSiteAdmin, AdminController.dissconectAllUsers + webRouter.post '/admin/syncUserToSubscription', AuthorizationMiddlewear.ensureUserIsSiteAdmin, AdminController.syncUserToSubscription + webRouter.post '/admin/flushProjectToTpds', AuthorizationMiddlewear.ensureUserIsSiteAdmin, AdminController.flushProjectToTpds + webRouter.post '/admin/pollDropboxForUser', AuthorizationMiddlewear.ensureUserIsSiteAdmin, AdminController.pollDropboxForUser + webRouter.post '/admin/messages', AuthorizationMiddlewear.ensureUserIsSiteAdmin, AdminController.createMessage + webRouter.post '/admin/messages/clear', AuthorizationMiddlewear.ensureUserIsSiteAdmin, AdminController.clearMessages apiRouter.get '/perfTest', (req,res)-> res.send("hello") @@ -205,7 +204,7 @@ module.exports = class Router webRouter.get '/health_check', HealthCheckController.check webRouter.get '/health_check/redis', HealthCheckController.checkRedis - apiRouter.get "/status/compiler/:Project_id", SecurityManager.requestCanAccessProject, (req, res) -> + apiRouter.get "/status/compiler/:Project_id", AuthorizationMiddlewear.ensureUserCanReadProject, (req, res) -> sendRes = _.once (statusCode, message)-> res.writeHead statusCode res.end message diff --git a/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee b/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee index 21c8a195e4..1160c81c37 100644 --- a/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Editor/EditorHttpControllerTests.coffee @@ -10,7 +10,7 @@ describe "EditorHttpController", -> '../Project/ProjectDeleter' : @ProjectDeleter = {} '../Project/ProjectGetter' : @ProjectGetter = {} '../User/UserGetter' : @UserGetter = {} - "../Security/AuthorizationManager": @AuthorizationManager = {} + "../Authorization/AuthorizationManager": @AuthorizationManager = {} '../Project/ProjectEditorHandler': @ProjectEditorHandler = {} "./EditorRealTimeController": @EditorRealTimeController = {} "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() } @@ -119,7 +119,7 @@ describe "EditorHttpController", -> it "should check the privilege level", -> @AuthorizationManager.getPrivilegeLevelForProject - .calledWith(@project, @user) + .calledWith(@user_id, @project_id) .should.equal true it "should return the project model view, privilege level and protocol version", -> diff --git a/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee index 0be616da51..1dbdc8539e 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectControllerTests.coffee @@ -39,8 +39,8 @@ describe "ProjectController", -> findOne: sinon.stub() @UserModel = findById: sinon.stub() - @SecurityManager = - userCanAccessProject:sinon.stub() + @AuthorizationManager = + getPrivilegeLevelForProject:sinon.stub() @EditorController = renameProject:sinon.stub() @InactiveProjectManager = @@ -66,7 +66,7 @@ describe "ProjectController", -> "../Notifications/NotificationsHandler":@NotificationsHandler '../../models/Project': Project:@ProjectModel "../../models/User":User:@UserModel - "../../managers/SecurityManager":@SecurityManager + "../Authorization/AuthorizationManager":@AuthorizationManager "../InactiveData/InactiveProjectManager":@InactiveProjectManager "./ProjectUpdateHandler":@ProjectUpdateHandler "../ReferencesSearch/ReferencesSearchHandler": @ReferencesSearchHandler @@ -299,7 +299,7 @@ describe "ProjectController", -> @ProjectModel.findOne.callsArgWith 1, null, @project @UserModel.findById.callsArgWith(1, null, @user) @SubscriptionLocator.getUsersSubscription.callsArgWith(1, null, {}) - @SecurityManager.userCanAccessProject.callsArgWith 2, true, "owner" + @AuthorizationManager.getPrivilegeLevelForProject.callsArgWith 2, null, true, "owner" @ProjectDeleter.unmarkAsDeletedByExternalSource = sinon.stub() @InactiveProjectManager.reactivateProjectIfRequired.callsArgWith(1) @ProjectUpdateHandler.markAsOpened.callsArgWith(1) @@ -332,7 +332,7 @@ describe "ProjectController", -> @ProjectController.loadEditor @req, @res it "should not render the page if the project can not be accessed", (done)-> - @SecurityManager.userCanAccessProject = sinon.stub().callsArgWith 2, false + @AuthorizationManager.getPrivilegeLevelForProject = sinon.stub().callsArgWith 2, null, false @res.sendStatus = (resCode, opts)=> resCode.should.equal 401 done() diff --git a/services/web/test/UnitTests/coffee/Security/AuthorizationManagerTests.coffee b/services/web/test/UnitTests/coffee/Security/AuthorizationManagerTests.coffee deleted file mode 100644 index 5be9c5ea20..0000000000 --- a/services/web/test/UnitTests/coffee/Security/AuthorizationManagerTests.coffee +++ /dev/null @@ -1,96 +0,0 @@ -SandboxedModule = require('sandboxed-module') -sinon = require('sinon') -require('chai').should() -modulePath = require('path').join __dirname, '../../../../app/js/Features/Security/AuthorizationManager' -MockClient = require "../helpers/MockClient" - -describe "AuthorizationManager", -> - beforeEach -> - @client = new MockClient() - @AuthorizationManager = SandboxedModule.require modulePath, requires: - '../../managers/SecurityManager':{} - - describe "ensureClientCanViewProject", -> - beforeEach -> - @client.set("project_id", "project-id") - - it "should let the request through for a readOnly privilege", (done) -> - @client.set("privilege_level", "readOnly") - @AuthorizationManager.ensureClientCanViewProject @client, done - - it "should let the request through for a readAndWrite privilege", (done) -> - @client.set("privilege_level", "readAndWrite") - @AuthorizationManager.ensureClientCanViewProject @client, done - - it "should let the request through for a owner privilege", (done) -> - @client.set("privilege_level", "owner") - @AuthorizationManager.ensureClientCanViewProject @client, done - - it "should ignore an empty privilege", -> - @AuthorizationManager.ensureClientCanViewProject @client, () -> - throw new Error("Should not be called") - - describe "ensureClientCanEditProject", -> - beforeEach -> - @client.set("project_id", "project-id") - - it "should ignore a readOnly privilege", -> - @client.set("privilege_level", "readOnly") - @AuthorizationManager.ensureClientCanEditProject @client, () -> - throw new Error("Should not be called") - - it "should let the request through for a readAndWrite privilege", (done) -> - @client.set("privilege_level", "readAndWrite") - @AuthorizationManager.ensureClientCanEditProject @client, done - - it "should let the request through for a owner privilege", (done) -> - @client.set("privilege_level", "owner") - @AuthorizationManager.ensureClientCanEditProject @client, done - - it "should ignore an empty privilege", -> - @AuthorizationManager.ensureClientCanEditProject @client, () -> - throw new Error("Should not be called") - - describe "ensureClientCanAdminProject", -> - beforeEach -> - @client.set("project_id", "project-id") - - it "should ignore a readOnly privilege", -> - @client.set("privilege_level", "readOnly") - @AuthorizationManager.ensureClientCanAdminProject @client, () -> - throw new Error("Should not be called") - - it "should ignore a readAndWrite privilege", -> - @client.set("privilege_level", "readAndWrite") - @AuthorizationManager.ensureClientCanAdminProject @client, () -> - throw new Error("Should not be called") - - it "should let the request through for a owner privilege", (done) -> - @client.set("privilege_level", "owner") - @AuthorizationManager.ensureClientCanAdminProject @client, done - - it "should ignore an empty privilege", -> - @AuthorizationManager.ensureClientCanAdminProject @client, () -> - throw new Error("Should not be called") - - describe "ensureClientHasPrivilegeLevelForProject", -> - it "should ignore callback if privilege_level is not set", -> - @client.set("project_id", "project-id") - @AuthorizationManager.ensureClientHasPrivilegeLevelForProject @client, - ["owner"], (error, project_id) -> - throw new Error("Should not be called") - - it "should ignore callback if project_id is not set", -> - @client.set("privilege_level", "owner") - @AuthorizationManager.ensureClientHasPrivilegeLevelForProject @client, - ["owner"], (error, project_id) -> - throw new Error("Should not be called") - - it "should return the project_id", (done) -> - @client.set("privilege_level", "owner") - @client.set("project_id", "project-id-123") - @AuthorizationManager.ensureClientHasPrivilegeLevelForProject @client, - ["owner"], (error, project_id) -> - project_id.should.equal "project-id-123" - done() - diff --git a/services/web/test/UnitTests/coffee/Subscription/SubscriptionControllerTests.coffee b/services/web/test/UnitTests/coffee/Subscription/SubscriptionControllerTests.coffee index 87cbc11671..631e4b57b6 100644 --- a/services/web/test/UnitTests/coffee/Subscription/SubscriptionControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/SubscriptionControllerTests.coffee @@ -23,8 +23,8 @@ describe "SubscriptionController sanboxed", -> @user = {email:"tom@yahoo.com"} @activeRecurlySubscription = mockSubscriptions["subscription-123-active"] - @SecurityManager = - getCurrentUser: sinon.stub().callsArgWith(1, null, @user) + @AuthenticationController = + getLoggedInUser: sinon.stub().callsArgWith(1, null, @user) @SubscriptionHandler = createSubscription: sinon.stub().callsArgWith(3) updateSubscription: sinon.stub().callsArgWith(3) @@ -61,7 +61,7 @@ describe "SubscriptionController sanboxed", -> @SubscriptionDomainHandler = getDomainLicencePage:sinon.stub() @SubscriptionController = SandboxedModule.require modulePath, requires: - '../../managers/SecurityManager': @SecurityManager + '../Authentication/AuthenticationController': @AuthenticationController './SubscriptionHandler': @SubscriptionHandler "./PlansLocator": @PlansLocator './SubscriptionViewModelBuilder': @SubscriptionViewModelBuilder