From 86e74b922866c0bc67f9388015c8568b07b1069e Mon Sep 17 00:00:00 2001 From: Antoine Clausse Date: Thu, 4 Sep 2025 11:36:42 +0200 Subject: [PATCH] [web] Update admin permissions to view/modify project contents (#28162) * Split capability definitions `modify-project`/`view-project` into `modify-project-content`/`modify-project-setting`/`view-project-content`/`view-project-setting` * Add admin capabilities check in AuthorizationManager * Update checks in router * Update frontend checks * Remove UI elements for admins without `view-project-content` * Update tests * Remove `modify-project-content` from the roles' capabilities * Update tests * Add "with admin roles" tests in AuthorizationTests.mjs GitOrigin-RevId: 3311bcb2da792968927b5b3703b24e069d0baf5b --- .../Authorization/AuthorizationManager.js | 76 ++++++++++++----- .../acceptance/src/AuthorizationTests.mjs | 81 +++++++++++++++++++ .../AuthorizationManagerTests.js | 13 ++- services/web/types/admin-capabilities.ts | 6 +- 4 files changed, 150 insertions(+), 26 deletions(-) diff --git a/services/web/app/src/Features/Authorization/AuthorizationManager.js b/services/web/app/src/Features/Authorization/AuthorizationManager.js index b111ef0139..11666048d3 100644 --- a/services/web/app/src/Features/Authorization/AuthorizationManager.js +++ b/services/web/app/src/Features/Authorization/AuthorizationManager.js @@ -156,10 +156,10 @@ async function getPrivilegeLevelForProjectWithUser( return PrivilegeLevels.OWNER } const { adminCapabilities } = await getAdminCapabilities({ _id: userId }) - if (adminCapabilities.includes('modify-project')) { + if (adminCapabilities.includes('modify-project-content')) { return PrivilegeLevels.OWNER } - if (adminCapabilities.includes('view-project')) { + if (adminCapabilities.includes('view-project-content')) { adminReadOnly = true } } @@ -261,24 +261,31 @@ async function canUserReadProject(userId, projectId, token) { const privilegeLevel = await getPrivilegeLevelForProject( userId, projectId, - token + token, + { ignoreSiteAdmin: true } + ) + return ( + [ + PrivilegeLevels.OWNER, + PrivilegeLevels.READ_AND_WRITE, + PrivilegeLevels.READ_ONLY, + PrivilegeLevels.REVIEW, + ].includes(privilegeLevel) || + (await hasAdminProjectCapability(userId, 'view-project-content')) ) - return [ - PrivilegeLevels.OWNER, - PrivilegeLevels.READ_AND_WRITE, - PrivilegeLevels.READ_ONLY, - PrivilegeLevels.REVIEW, - ].includes(privilegeLevel) } async function canUserWriteProjectContent(userId, projectId, token) { const privilegeLevel = await getPrivilegeLevelForProject( userId, projectId, - token + token, + { ignoreSiteAdmin: true } ) - return [PrivilegeLevels.OWNER, PrivilegeLevels.READ_AND_WRITE].includes( - privilegeLevel + return ( + [PrivilegeLevels.OWNER, PrivilegeLevels.READ_AND_WRITE].includes( + privilegeLevel + ) || (await hasAdminProjectCapability(userId, 'modify-project-content')) ) } @@ -286,12 +293,14 @@ async function canUserWriteOrReviewProjectContent(userId, projectId, token) { const privilegeLevel = await getPrivilegeLevelForProject( userId, projectId, - token + token, + { ignoreSiteAdmin: true } ) return ( privilegeLevel === PrivilegeLevels.OWNER || privilegeLevel === PrivilegeLevels.READ_AND_WRITE || - privilegeLevel === PrivilegeLevels.REVIEW + privilegeLevel === PrivilegeLevels.REVIEW || + (await hasAdminProjectCapability(userId, 'modify-project-content')) ) } @@ -300,10 +309,12 @@ async function canUserWriteProjectSettings(userId, projectId, token) { userId, projectId, token, - { ignorePublicAccess: true } + { ignorePublicAccess: true, ignoreSiteAdmin: true } ) - return [PrivilegeLevels.OWNER, PrivilegeLevels.READ_AND_WRITE].includes( - privilegeLevel + return ( + [PrivilegeLevels.OWNER, PrivilegeLevels.READ_AND_WRITE].includes( + privilegeLevel + ) || (await hasAdminProjectCapability(userId, 'modify-project-setting')) ) } @@ -311,18 +322,26 @@ async function canUserRenameProject(userId, projectId, token) { const privilegeLevel = await getPrivilegeLevelForProject( userId, projectId, - token + token, + { ignoreSiteAdmin: true } + ) + return ( + privilegeLevel === PrivilegeLevels.OWNER || + (await hasAdminProjectCapability(userId, 'modify-project-setting')) ) - return privilegeLevel === PrivilegeLevels.OWNER } async function canUserAdminProject(userId, projectId, token) { const privilegeLevel = await getPrivilegeLevelForProject( userId, projectId, - token + token, + { ignoreSiteAdmin: true } + ) + return ( + privilegeLevel === PrivilegeLevels.OWNER || + (await hasAdminProjectCapability(userId, 'modify-project-setting')) ) - return privilegeLevel === PrivilegeLevels.OWNER } async function isUserSiteAdmin(userId) { @@ -334,6 +353,21 @@ async function isUserSiteAdmin(userId) { return hasAdminAccess(user) } +/** + * @param {string} userId + * @param {'view-project-setting'|'view-project-content'|'modify-project-setting'|'modify-project-content'} adminCapability + */ +async function hasAdminProjectCapability(userId, adminCapability) { + if (!Settings.adminPrivilegeAvailable || !(await isUserSiteAdmin(userId))) { + return false + } + if (!Settings.adminRolesEnabled) { + return true + } + const { adminCapabilities } = await getAdminCapabilities({ _id: userId }) + return adminCapabilities.includes(adminCapability) +} + async function canUserDeleteOrResolveThread( userId, projectId, diff --git a/services/web/test/acceptance/src/AuthorizationTests.mjs b/services/web/test/acceptance/src/AuthorizationTests.mjs index c80c2dc4ba..dca3a2383e 100644 --- a/services/web/test/acceptance/src/AuthorizationTests.mjs +++ b/services/web/test/acceptance/src/AuthorizationTests.mjs @@ -364,6 +364,87 @@ describe('Authorization', function () { await expectRedirectedAdminAccess(this.site_admin) }) }) + + describe('with admin roles', function () { + beforeEach(function () { + if (!settings.moduleImportSequence.includes('admin-roles')) { + this.skip() + } + settings.adminRolesEnabled = true + settings.adminPrivilegeAvailable = true + }) + + afterEach(function () { + settings.adminRolesEnabled = false + settings.adminPrivilegeAvailable = true + this.site_admin.mongoUpdate({ + $set: { adminRoles: [] }, + }) + }) + + describe('engineering', function () { + beforeEach(function () { + this.site_admin.mongoUpdate({ + $set: { adminRoles: ['engineering'] }, + }) + }) + + it('should allow site admin users read access to it', async function () { + await expectReadAccess(this.site_admin, this.projectId) + }) + + it('should not allow site admin users write access to its content', async function () { + await expectNoContentWriteAccess(this.site_admin, this.projectId) + }) + + it('should allow site admin users write access to its settings', async function () { + await expectSettingsWriteAccess(this.site_admin, this.projectId) + }) + + it('should allow site admin users to rename the project', async function () { + await expectRenameProjectAccess(this.site_admin, this.projectId) + }) + + it('should allow site admin users project admin access to it', async function () { + await expectProjectAdminAccess(this.site_admin, this.projectId) + }) + + it('should allow site admin users site admin access to site admin endpoints', async function () { + await expectAdminAccess(this.site_admin) + }) + }) + describe('no admin role assigned', function () { + beforeEach(function () { + this.site_admin.mongoUpdate({ + $set: { adminRoles: [] }, + }) + }) + + it('should not allow site admin users read access to it', async function () { + await expectNoReadAccess(this.site_admin, this.projectId) + }) + + it('should not allow site admin users write access to its content', async function () { + await expectNoContentWriteAccess(this.site_admin, this.projectId) + }) + + it('should not allow site admin users write access to its settings', async function () { + await expectNoSettingsWriteAccess(this.site_admin, this.projectId) + }) + + it('should not allow site admin users to rename the project', async function () { + await expectNoRenameProjectAccess(this.site_admin, this.projectId) + }) + + it('should not allow site admin users project admin access to it', async function () { + await expectNoProjectAdminAccess(this.site_admin, this.projectId) + }) + + it('should allow site admin users site admin access to site admin endpoints', async function () { + await expectAdminAccess(this.site_admin) + }) + }) + }) }) describe('shared project', function () { diff --git a/services/web/test/unit/src/Authorization/AuthorizationManagerTests.js b/services/web/test/unit/src/Authorization/AuthorizationManagerTests.js index 20ef3bcb0e..435ebcc011 100644 --- a/services/web/test/unit/src/Authorization/AuthorizationManagerTests.js +++ b/services/web/test/unit/src/Authorization/AuthorizationManagerTests.js @@ -709,13 +709,13 @@ function testPermission(permission, privilegeLevels) { }) expectPermission(permission, false) }) - describe('admin with `view-project`', function () { + describe('admin with `view-project-content`', function () { beforeEach(function () { this.user.isAdmin = true this.settings.adminRolesEnabled = true this.Modules.promises.hooks.fire .withArgs('getAdminCapabilities') - .resolves([['view-project']]) + .resolves([['view-project-content']]) }) expectPermission(permission, privilegeLevels.readOnly || false) }) @@ -725,7 +725,14 @@ function testPermission(permission, privilegeLevels) { this.settings.adminRolesEnabled = true this.Modules.promises.hooks.fire .withArgs('getAdminCapabilities') - .resolves([['view-project', 'modify-project']]) + .resolves([ + [ + 'view-project-content', + 'view-project-setting', + 'modify-project-content', + 'modify-project-setting', + ], + ]) }) expectPermission(permission, privilegeLevels.siteAdmin || false) }) diff --git a/services/web/types/admin-capabilities.ts b/services/web/types/admin-capabilities.ts index 81b30bdd4f..8d9c70c786 100644 --- a/services/web/types/admin-capabilities.ts +++ b/services/web/types/admin-capabilities.ts @@ -11,14 +11,16 @@ export type AdminCapability = | 'modify-managed-group' | 'modify-managed-group-member' | 'modify-user-account-status' - | 'modify-project' + | 'modify-project-content' + | 'modify-project-setting' | 'manage-survey' | 'modify-split-test' | 'modify-user-email' | 'modify-user-name' | 'view-audit-log' | 'view-group-manager' - | 'view-project' + | 'view-project-content' + | 'view-project-setting' | 'view-session' | 'view-script-log' | 'view-split-test'