diff --git a/services/web/app/src/Features/Helpers/AdminAuthorizationHelper.js b/services/web/app/src/Features/Helpers/AdminAuthorizationHelper.js index 54668696cb..9dd8a5380c 100644 --- a/services/web/app/src/Features/Helpers/AdminAuthorizationHelper.js +++ b/services/web/app/src/Features/Helpers/AdminAuthorizationHelper.js @@ -6,6 +6,7 @@ const logger = require('@overleaf/logger') module.exports = { hasAdminAccess, + hasAdminCapability, canRedirectToAdminDomain, getAdminCapabilities, useHasAdminCapability, @@ -18,6 +19,20 @@ function hasAdminAccess(user) { return Boolean(user.isAdmin) } +function hasAdminCapability(capability) { + return req => { + if (!hasAdminAccess(SessionManager.getSessionUser(req.session))) { + return false + } + const { adminCapabilitiesAvailable, adminCapabilities } = req + if (!adminCapabilitiesAvailable) { + // We can't know which capabilities are possible, so we assume all are available for admins. + return true + } + return adminCapabilities?.includes(capability) + } +} + async function getAdminCapabilities(user) { const rawAdminCapabilties = await Modules.promises.hooks.fire( 'getAdminCapabilities', @@ -54,17 +69,8 @@ async function useAdminCapabilities(req, res, next) { } function useHasAdminCapability(req, res, next) { - res.locals.hasAdminCapability = capability => { - if (!hasAdminAccess(SessionManager.getSessionUser(req.session))) { - return false - } - const { adminCapabilitiesAvailable, adminCapabilities } = req - if (!adminCapabilitiesAvailable) { - // We can't know which capabilities are possible, so we assume all are available for admins. - return true - } - return adminCapabilities.includes(capability) - } + res.locals.hasAdminCapability = capability => + hasAdminCapability(capability)(req) next() } diff --git a/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.js b/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.js index 866e74763e..111d96a408 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.js +++ b/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.js @@ -1,4 +1,5 @@ -const { hasAdminAccess } = require('../Helpers/AdminAuthorizationHelper') +const { hasAdminCapability } = require('../Helpers/AdminAuthorizationHelper') + const UserMembershipAuthorization = { hasStaffAccess(requiredStaffAccess) { return req => { @@ -13,17 +14,7 @@ const UserMembershipAuthorization = { } }, - hasAdminCapability(capability) { - return req => { - if (!hasAdminAccess(req.user)) { - return false - } - if (!req.adminCapabilitiesAvailable) { - return true - } - return req.adminCapabilities?.includes(capability) - } - }, + hasAdminCapability, hasEntityAccess() { return req => { diff --git a/services/web/test/unit/src/HelperFiles/AdminAuthorizationHelperTests.js b/services/web/test/unit/src/HelperFiles/AdminAuthorizationHelperTests.js index a14824ad85..4c4e171dbb 100644 --- a/services/web/test/unit/src/HelperFiles/AdminAuthorizationHelperTests.js +++ b/services/web/test/unit/src/HelperFiles/AdminAuthorizationHelperTests.js @@ -369,4 +369,63 @@ describe('AdminAuthorizationHelper', function () { }) }) }) + describe('hasAdminCapability', function () { + describe('when user is not an admin', function () { + it('returns false', function () { + const req = { + session: { + user: { isAdmin: false }, + }, + } + expect( + this.AdminAuthorizationHelper.hasAdminCapability('capability')(req) + ).to.be.false + }) + }) + describe('when user is an admin', function () { + describe('when adminCapabilitiesAvailable is falsey', function () { + it('returns true', function () { + const req = { + session: { + user: { isAdmin: true }, + }, + adminCapabilitiesAvailable: false, + } + expect( + this.AdminAuthorizationHelper.hasAdminCapability('capability')(req) + ).to.be.true + }) + }) + describe('when adminCapabilitiesAvailable is true', function () { + describe('when user has the requested capability', function () { + it('returns true', function () { + const req = { + session: { user: { isAdmin: true } }, + adminCapabilitiesAvailable: true, + adminCapabilities: ['capability'], + } + expect( + this.AdminAuthorizationHelper.hasAdminCapability('capability')( + req + ) + ).to.be.true + }) + }) + describe('when user does not have the requested capability', function () { + it('returns false', function () { + const req = { + session: { user: { isAdmin: true } }, + adminCapabilitiesAvailable: true, + adminCapabilities: ['other-capability'], + } + expect( + this.AdminAuthorizationHelper.hasAdminCapability('capability')( + req + ) + ).to.be.false + }) + }) + }) + }) + }) }) diff --git a/services/web/test/unit/src/UserMembership/UserMembershipAuthorizationTests.js b/services/web/test/unit/src/UserMembership/UserMembershipAuthorizationTests.js deleted file mode 100644 index f95ec183c6..0000000000 --- a/services/web/test/unit/src/UserMembership/UserMembershipAuthorizationTests.js +++ /dev/null @@ -1,67 +0,0 @@ -const { expect } = require('chai') -const sinon = require('sinon') -const SandboxedModule = require('sandboxed-module') - -const modulePath = - '../../../../app/src/Features/UserMembership/UserMembershipAuthorization' - -describe('UserMembershipAuthorization', function () { - let hasAdminAccess, UserMembershipAuthorization - beforeEach(function () { - hasAdminAccess = sinon.stub().returns(true) - UserMembershipAuthorization = SandboxedModule.require(modulePath, { - requires: { - '../Helpers/AdminAuthorizationHelper': { - hasAdminAccess, - }, - }, - }) - }) - describe('hasAdminCapability', function () { - describe('when user is not an admin', function () { - it('returns false', function () { - hasAdminAccess.returns(false) - const req = { user: {} } - expect( - UserMembershipAuthorization.hasAdminCapability('capability')(req) - ).to.be.false - }) - }) - describe('when user is an admin', function () { - describe('when adminCapabilitiesAvailable is falsey', function () { - it('returns true', function () { - const req = { user: {}, adminCapabilitiesAvailable: false } - expect( - UserMembershipAuthorization.hasAdminCapability('capability')(req) - ).to.be.true - }) - }) - describe('when adminCapabilitiesAvailable is true', function () { - describe('when user has the requested capability', function () { - it('returns true', function () { - const req = { - user: {}, - adminCapabilitiesAvailable: true, - adminCapabilities: ['capability'], - } - expect( - UserMembershipAuthorization.hasAdminCapability('capability')(req) - ).to.be.true - }) - }) - describe('when user does not have the requested capability', function () { - it('returns false', function () { - const req = { - user: {}, - adminCapabilitiesAvailable: true, - adminCapabilities: ['other-capability'], - } - expect( - UserMembershipAuthorization.hasAdminCapability('capability')(req) - ).to.be.false - }) - }) - }) - }) - }) -})