From 2c44d657852aa67e415953a5efecb2693a843e17 Mon Sep 17 00:00:00 2001 From: Antoine Clausse Date: Tue, 19 Aug 2025 14:13:12 +0200 Subject: [PATCH] [web] Add `requireAdminRoles` param to `hasAdminCapability` (#28006) * Add `requireAdminRoles` param to `hasAdminCapability` https://github.com/overleaf/internal/pull/27965#discussion_r2284808889 Co-authored-by: Andrew Rumble * Update test --------- Co-authored-by: Andrew Rumble GitOrigin-RevId: 83f8af84debc70c7a2e294638747369c786be22f --- .../Helpers/AdminAuthorizationHelper.js | 9 ++- .../UserMembershipAuthorization.js | 3 +- services/web/app/views/layout-react.pug | 4 +- .../web/app/views/layout/navbar-marketing.pug | 4 +- .../views/layout/navbar-website-redesign.pug | 4 +- .../AdminAuthorizationHelperTests.js | 56 +++++++++++++++++-- 6 files changed, 66 insertions(+), 14 deletions(-) diff --git a/services/web/app/src/Features/Helpers/AdminAuthorizationHelper.js b/services/web/app/src/Features/Helpers/AdminAuthorizationHelper.js index 9dd8a5380c..44fe8a9b96 100644 --- a/services/web/app/src/Features/Helpers/AdminAuthorizationHelper.js +++ b/services/web/app/src/Features/Helpers/AdminAuthorizationHelper.js @@ -19,8 +19,11 @@ function hasAdminAccess(user) { return Boolean(user.isAdmin) } -function hasAdminCapability(capability) { +function hasAdminCapability(capability, requireAdminRoles = true) { return req => { + if (requireAdminRoles && !Settings.adminRolesEnabled) { + return false + } if (!hasAdminAccess(SessionManager.getSessionUser(req.session))) { return false } @@ -69,8 +72,8 @@ async function useAdminCapabilities(req, res, next) { } function useHasAdminCapability(req, res, next) { - res.locals.hasAdminCapability = capability => - hasAdminCapability(capability)(req) + res.locals.hasAdminCapability = (capability, requireAdminRoles = true) => + hasAdminCapability(capability, requireAdminRoles)(req) next() } diff --git a/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.js b/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.js index e4d8735c59..fa3b4c4261 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.js +++ b/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.js @@ -20,7 +20,8 @@ const UserMembershipAuthorization = { return hasAdminCapability( req.entity.managedUsersEnabled ? 'modify-managed-group-member' - : 'modify-group-member' + : 'modify-group-member', + true )(req, res) }, diff --git a/services/web/app/views/layout-react.pug b/services/web/app/views/layout-react.pug index 777bab4a1b..bcefee2ca2 100644 --- a/services/web/app/views/layout-react.pug +++ b/services/web/app/views/layout-react.pug @@ -17,8 +17,8 @@ block append meta - const sessionUser = getSessionUser() - const staffAccess = sessionUser?.staffAccess - const canDisplaySplitTestMenu = hasFeature('saas') && ((canDisplayAdminMenu && hasAdminCapability('view-split-test')) || staffAccess?.splitTestMetrics || staffAccess?.splitTestManagement) - - const canDisplaySurveyMenu = hasFeature('saas') && canDisplayAdminMenu && hasAdminCapability('manage-survey') - - const canDisplayScriptLogMenu = hasFeature('saas') && hasAdminCapability('view-script-log') && canDisplayAdminMenu + - const canDisplaySurveyMenu = hasFeature('saas') && canDisplayAdminMenu && hasAdminCapability('manage-survey', false) + - const canDisplayScriptLogMenu = hasFeature('saas') && hasAdminCapability('view-script-log', false) && canDisplayAdminMenu - const enableUpgradeButton = projectDashboardReact && usersBestSubscription && (usersBestSubscription.type === 'free' || usersBestSubscription.type === 'standalone-ai-add-on') - const showSignUpLink = hasFeature('registration-page') diff --git a/services/web/app/views/layout/navbar-marketing.pug b/services/web/app/views/layout/navbar-marketing.pug index 63781f9260..01577ccbf9 100644 --- a/services/web/app/views/layout/navbar-marketing.pug +++ b/services/web/app/views/layout/navbar-marketing.pug @@ -34,8 +34,8 @@ nav.navbar.navbar-default.navbar-main.navbar-expand-lg( - var canDisplayAdminMenu = hasAdminAccess() - var canDisplayAdminRedirect = canRedirectToAdminDomain() - var canDisplaySplitTestMenu = hasFeature('saas') && ((canDisplayAdminMenu && hasAdminCapability('view-split-test')) || (getSessionUser() && getSessionUser().staffAccess && (getSessionUser().staffAccess.splitTestMetrics || getSessionUser().staffAccess.splitTestManagement))) - - var canDisplaySurveyMenu = hasFeature('saas') && canDisplayAdminMenu && hasAdminCapability('manage-survey') - - var canDisplayScriptLogMenu = hasFeature('saas') && hasAdminCapability('view-script-log') && canDisplayAdminMenu + - var canDisplaySurveyMenu = hasFeature('saas') && canDisplayAdminMenu && hasAdminCapability('manage-survey', false) + - var canDisplayScriptLogMenu = hasFeature('saas') && hasAdminCapability('view-script-log', false) && canDisplayAdminMenu if typeof suppressNavbarRight === 'undefined' button#navbar-toggle-btn.navbar-toggler.collapsed( diff --git a/services/web/app/views/layout/navbar-website-redesign.pug b/services/web/app/views/layout/navbar-website-redesign.pug index 13cf8c2a71..397494bbbe 100644 --- a/services/web/app/views/layout/navbar-website-redesign.pug +++ b/services/web/app/views/layout/navbar-website-redesign.pug @@ -40,8 +40,8 @@ nav.navbar.navbar-default.navbar-main.website-redesign-navbar( - var canDisplayAdminMenu = hasAdminAccess() - var canDisplayAdminRedirect = canRedirectToAdminDomain() - var canDisplaySplitTestMenu = hasFeature('saas') && ((canDisplayAdminMenu && hasAdminCapability('view-split-test')) || (getSessionUser() && getSessionUser().staffAccess && (getSessionUser().staffAccess.splitTestMetrics || getSessionUser().staffAccess.splitTestManagement))) - - var canDisplaySurveyMenu = hasFeature('saas') && canDisplayAdminMenu && hasAdminCapability('manage-survey') - - var canDisplayScriptLogMenu = hasFeature('saas') && hasAdminCapability('view-script-log') && canDisplayAdminMenu + - var canDisplaySurveyMenu = hasFeature('saas') && canDisplayAdminMenu && hasAdminCapability('manage-survey', false) + - var canDisplayScriptLogMenu = hasFeature('saas') && hasAdminCapability('view-script-log', false) && canDisplayAdminMenu if typeof suppressNavbarRight == 'undefined' #navbar-main-collapse.navbar-collapse.collapse diff --git a/services/web/test/unit/src/HelperFiles/AdminAuthorizationHelperTests.js b/services/web/test/unit/src/HelperFiles/AdminAuthorizationHelperTests.js index 4c4e171dbb..691bd20dd1 100644 --- a/services/web/test/unit/src/HelperFiles/AdminAuthorizationHelperTests.js +++ b/services/web/test/unit/src/HelperFiles/AdminAuthorizationHelperTests.js @@ -10,12 +10,14 @@ const modulePath = describe('AdminAuthorizationHelper', function () { beforeEach(function () { this.fireHook = sinon.stub().resolves([]) + this.settings = { + adminPrivilegeAvailable: true, + adminUrl: 'https://admin.overleaf.com', + adminRolesEnabled: true, + } this.AdminAuthorizationHelper = SandboxedModule.require(modulePath, { requires: { - '@overleaf/settings': { - adminPrivilegeAvailable: true, - adminUrl: 'https://admin.overleaf.com', - }, + '@overleaf/settings': this.settings, '../../infrastructure/Modules': { promises: { hooks: { @@ -395,6 +397,24 @@ describe('AdminAuthorizationHelper', function () { this.AdminAuthorizationHelper.hasAdminCapability('capability')(req) ).to.be.true }) + it('ignores the "requireAdminRoles" argument', function () { + const req = { + session: { user: { isAdmin: true } }, + adminCapabilitiesAvailable: false, + } + expect( + this.AdminAuthorizationHelper.hasAdminCapability( + 'capability', + true + )(req) + ).to.be.true + expect( + this.AdminAuthorizationHelper.hasAdminCapability( + 'capability', + false + )(req) + ).to.be.true + }) }) describe('when adminCapabilitiesAvailable is true', function () { describe('when user has the requested capability', function () { @@ -427,5 +447,33 @@ describe('AdminAuthorizationHelper', function () { }) }) }) + + describe('when admin roles are not enabled', function () { + beforeEach(function () { + this.settings.adminRolesEnabled = false + }) + + it('returns false even for admins', function () { + const req = { session: { user: { isAdmin: true } } } + expect( + this.AdminAuthorizationHelper.hasAdminCapability('capability')(req) + ).to.be.false + expect( + this.AdminAuthorizationHelper.hasAdminCapability( + 'capability', + true + )(req) + ).to.be.false + }) + it('returns true when requireAdminRoles=false', function () { + const req = { session: { user: { isAdmin: true } } } + expect( + this.AdminAuthorizationHelper.hasAdminCapability( + 'capability', + false + )(req) + ).to.be.true + }) + }) }) })