From 972c6e4710a84134d21cdecc3acd4395663316ba Mon Sep 17 00:00:00 2001 From: Andrew Rumble Date: Tue, 31 Mar 2026 16:00:44 +0200 Subject: [PATCH] Merge pull request #31327 from overleaf/ar-allow-split-test-ui-without-admin-privilege [web/admin-roles] allow split test UI without admin privilege GitOrigin-RevId: 1d10153d7762196dd7a8df835af6193b38670fbc --- .../Helpers/AdminAuthorizationHelper.mjs | 39 +++++++++++++++++++ .../UserMembershipAuthorization.mjs | 2 + .../UserMembershipMiddleware.mjs | 15 ++++--- .../app/src/infrastructure/ExpressLocals.mjs | 5 ++- services/web/app/views/layout-react.pug | 2 +- .../web/app/views/layout/navbar-marketing.pug | 2 +- services/web/scripts/e2e_test_setup.mjs | 11 +++++- 7 files changed, 66 insertions(+), 10 deletions(-) diff --git a/services/web/app/src/Features/Helpers/AdminAuthorizationHelper.mjs b/services/web/app/src/Features/Helpers/AdminAuthorizationHelper.mjs index 0e10abb04f..93c0f7a3cf 100644 --- a/services/web/app/src/Features/Helpers/AdminAuthorizationHelper.mjs +++ b/services/web/app/src/Features/Helpers/AdminAuthorizationHelper.mjs @@ -11,6 +11,9 @@ export default { getAdminCapabilities, useHasAdminCapability, useAdminCapabilities: expressify(useAdminCapabilities), + useNonAdminDomainCapabilities: expressify(useNonAdminDomainCapabilities), + hasNonAdminDomainCapability, + useHasNonAdminDomainCapability, } function hasAdminAccess(user) { @@ -36,6 +39,36 @@ function hasAdminCapability(capability, requireAdminRoles = true) { } } +function hasNonAdminDomainCapability(capability) { + return req => { + return req.nonAdminDomainCapabilities?.includes(capability) + } +} + +async function useNonAdminDomainCapabilities(req, _res, next) { + if (req.nonAdminDomainCapabilities) { + return next() + } + const user = SessionManager.getSessionUser(req.session) + // Equivalent to `hasAdminAccess` but without the `Settings.adminPrivilegeAvailable` check. + if (!user?.isAdmin) { + req.nonAdminDomainCapabilities = [] + return next() + } + + try { + const capabilities = await Modules.promises.hooks.fire( + 'getNonAdminDomainCapabilities', + user + ) + req.nonAdminDomainCapabilities = [...new Set(capabilities.flat())] + } catch (err) { + // This can throw when the user doesn't exist or isn't logged in. + req.nonAdminDomainCapabilities = [] + } + next() +} + async function getAdminCapabilities(user) { const rawAdminCapabilties = await Modules.promises.hooks.fire( 'getAdminCapabilities', @@ -77,6 +110,12 @@ function useHasAdminCapability(req, res, next) { next() } +function useHasNonAdminDomainCapability(req, res, next) { + res.locals.hasNonAdminDomainCapability = capability => + hasNonAdminDomainCapability(capability)(req) + next() +} + function canRedirectToAdminDomain(user) { if (Settings.adminPrivilegeAvailable) return false if (!Settings.adminUrl) return false diff --git a/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.mjs b/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.mjs index 025ecc63fe..7f18e38c67 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.mjs +++ b/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.mjs @@ -3,6 +3,8 @@ import SessionManager from '../Authentication/SessionManager.mjs' const UserMembershipAuthorization = { hasAdminCapability: AdminAuthorizationHelper.hasAdminCapability, + hasNonAdminDomainCapability: + AdminAuthorizationHelper.hasNonAdminDomainCapability, hasAdminAccess(req) { return AdminAuthorizationHelper.hasAdminAccess( diff --git a/services/web/app/src/Features/UserMembership/UserMembershipMiddleware.mjs b/services/web/app/src/Features/UserMembership/UserMembershipMiddleware.mjs index fb558ae44a..0a17d61d58 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipMiddleware.mjs +++ b/services/web/app/src/Features/UserMembership/UserMembershipMiddleware.mjs @@ -12,7 +12,8 @@ import TemplatesManager from '../Templates/TemplatesManager.mjs' import { z, zz, parseReq } from '../../infrastructure/Validation.mjs' import AdminAuthorizationHelper from '../Helpers/AdminAuthorizationHelper.mjs' -const { useAdminCapabilities } = AdminAuthorizationHelper +const { useAdminCapabilities, useNonAdminDomainCapabilities } = + AdminAuthorizationHelper // set of middleware arrays or functions that checks user access to an entity // (publisher, institution, group, template, etc.) const UserMembershipMiddleware = { @@ -209,17 +210,21 @@ const UserMembershipMiddleware = { requireSplitTestMetricsAccess: [ AuthenticationController.requireLogin(), - useAdminCapabilities, + useNonAdminDomainCapabilities, allowAccessIfAny([ - UserMembershipAuthorization.hasAdminCapability('view-split-test'), + UserMembershipAuthorization.hasNonAdminDomainCapability( + 'view-split-test' + ), ]), ], requireSplitTestManagementAccess: [ AuthenticationController.requireLogin(), - useAdminCapabilities, + useNonAdminDomainCapabilities, allowAccessIfAny([ - UserMembershipAuthorization.hasAdminCapability('modify-split-test'), + UserMembershipAuthorization.hasNonAdminDomainCapability( + 'modify-split-test' + ), ]), ], diff --git a/services/web/app/src/infrastructure/ExpressLocals.mjs b/services/web/app/src/infrastructure/ExpressLocals.mjs index 3638d83eb9..4587356b38 100644 --- a/services/web/app/src/infrastructure/ExpressLocals.mjs +++ b/services/web/app/src/infrastructure/ExpressLocals.mjs @@ -21,6 +21,8 @@ const { hasAdminAccess, useAdminCapabilities, useHasAdminCapability, + useNonAdminDomainCapabilities, + useHasNonAdminDomainCapability, } = AdminAuthorizationHelper const IEEE_BRAND_ID = Settings.ieeeBrandId @@ -279,8 +281,9 @@ export default async function (webRouter, privateApiRouter, publicApiRouter) { }) webRouter.use(useAdminCapabilities) - webRouter.use(useHasAdminCapability) + webRouter.use(useNonAdminDomainCapabilities) + webRouter.use(useHasNonAdminDomainCapability) webRouter.use(function (req, res, next) { // Clone the nav settings so they can be modified for each request diff --git a/services/web/app/views/layout-react.pug b/services/web/app/views/layout-react.pug index 757bb50ddc..6054e6cb39 100644 --- a/services/web/app/views/layout-react.pug +++ b/services/web/app/views/layout-react.pug @@ -12,7 +12,7 @@ block append meta - const canDisplayAdminRedirect = canRedirectToAdminDomain() - const sessionUser = getSessionUser() - const canDisplayProjectUrlLookup = settings.adminPrivilegeAvailable && canDisplayAdminMenu && hasAdminCapability('view-project-setting', false) - - const canDisplaySplitTestMenu = hasFeature('saas') && canDisplayAdminMenu && hasAdminCapability('view-split-test') + - const canDisplaySplitTestMenu = hasFeature('saas') && hasNonAdminDomainCapability('view-split-test') - 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') diff --git a/services/web/app/views/layout/navbar-marketing.pug b/services/web/app/views/layout/navbar-marketing.pug index d99e4db580..6509b3f0f5 100644 --- a/services/web/app/views/layout/navbar-marketing.pug +++ b/services/web/app/views/layout/navbar-marketing.pug @@ -31,7 +31,7 @@ nav.navbar.navbar-default.navbar-main.navbar-expand-lg( - var canDisplayAdminMenu = hasAdminAccess() - var canDisplayAdminRedirect = canRedirectToAdminDomain() - var canDisplayProjectUrlLookup = settings.adminPrivilegeAvailable && canDisplayAdminMenu && hasAdminCapability('view-project-setting', false) - - var canDisplaySplitTestMenu = hasFeature('saas') && canDisplayAdminMenu && hasAdminCapability('view-split-test') + - var canDisplaySplitTestMenu = hasFeature('saas') && hasNonAdminDomainCapability('view-split-test') - var canDisplaySurveyMenu = hasFeature('saas') && canDisplayAdminMenu && hasAdminCapability('manage-survey', false) - var canDisplayScriptLogMenu = hasFeature('saas') && hasAdminCapability('view-script-log', false) && canDisplayAdminMenu diff --git a/services/web/scripts/e2e_test_setup.mjs b/services/web/scripts/e2e_test_setup.mjs index 603b78acb1..52bd39a67f 100644 --- a/services/web/scripts/e2e_test_setup.mjs +++ b/services/web/scripts/e2e_test_setup.mjs @@ -28,13 +28,20 @@ async function createUser(email) { const features = email.startsWith('free+') ? Settings.defaultFeatures : Settings.features.professional + const isAdmin = email.startsWith('admin+') + let adminRoles = [] + if (email.startsWith('admin+finance')) { + adminRoles = ['finance'] + } else if (isAdmin) { + adminRoles = ['engineering'] + } await db.users.updateOne( { _id: user._id }, { $set: { // Set admin flag. - isAdmin: email.startsWith('admin+'), - adminRoles: email.startsWith('admin+') ? ['engineering'] : [], + isAdmin, + adminRoles, // Disable spell-checking for performance and flakiness reasons. 'ace.spellCheckLanguage': '', // Override features.