From ba97b968157985d32bb3e922296bbf2f8289d570 Mon Sep 17 00:00:00 2001 From: Antoine Clausse Date: Thu, 14 Aug 2025 13:37:06 +0200 Subject: [PATCH] [web] Add admin permissions `modify-group-member` and `modify-managed-group-member` (#27665) * Add capability `modify-managed-group-member` & `modify-group-member` * Check `modify-managed-group-member` & `modify-group-member` (backend) * Check `modify-managed-group-member` & `modify-group-member` (frontend) * Update tests * Update with `ol-hasWriteAccess` flag * Update tests * Move functions to AdminAuthorizationHelper.js * Update import to fix build error * Add `ol-hasWriteAccess` to types * Use `hasAdminAccess()` instead of `req?.user?.isAdmin` * Add tests on `/manage/groups/:id/invites` depending on admin roles * Reuse `UserMembershipAuthorization.hasAdminCapability` * Fix: Add entityAccess check * Update unit test * Rename `hasAdminGroupMemberCapability` to `hasModifyGroupMemberCapability` * Remove useless and redundant `hasWriteAccess` check * Restore stub in afterEach GitOrigin-RevId: 4b6d83751121b43d4c19d0dbd82a4833cf7a6f24 --- .../UserMembershipAuthorization.js | 8 ++++++ .../UserMembershipController.mjs | 2 ++ .../UserMembershipMiddleware.js | 13 ++++++++++ .../UserMembership/UserMembershipRouter.mjs | 6 ++--- .../user_membership/group-members-react.pug | 2 ++ .../components/group-members.tsx | 8 +++--- .../components/members-table/member-row.tsx | 26 +++++++++++-------- .../components/members-table/members-list.tsx | 11 +++++--- .../components/group-members.spec.tsx | 3 +++ .../components/managed-group-members.spec.tsx | 1 + .../members-table/member-row.spec.tsx | 16 ++++++++++++ .../members-table/members-list.spec.tsx | 4 +-- .../UserMembershipController.test.mjs | 2 ++ services/web/types/admin-capabilities.ts | 2 ++ 14 files changed, 82 insertions(+), 22 deletions(-) diff --git a/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.js b/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.js index 111d96a408..e4d8735c59 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.js +++ b/services/web/app/src/Features/UserMembership/UserMembershipAuthorization.js @@ -16,6 +16,14 @@ const UserMembershipAuthorization = { hasAdminCapability, + hasModifyGroupMemberCapability(req, res) { + return hasAdminCapability( + req.entity.managedUsersEnabled + ? 'modify-managed-group-member' + : 'modify-group-member' + )(req, res) + }, + hasEntityAccess() { return req => { if (!req.entity) { diff --git a/services/web/app/src/Features/UserMembership/UserMembershipController.mjs b/services/web/app/src/Features/UserMembership/UserMembershipController.mjs index 7b632d7569..eaac266c8d 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipController.mjs +++ b/services/web/app/src/Features/UserMembership/UserMembershipController.mjs @@ -14,6 +14,7 @@ import { expressify } from '@overleaf/promise-utils' import PlansLocator from '../Subscription/PlansLocator.js' import RecurlyClient from '../Subscription/RecurlyClient.js' import Modules from '../../infrastructure/Modules.js' +import UserMembershipAuthorization from './UserMembershipAuthorization.js' async function manageGroupMembers(req, res, next) { const { entity: subscription, entityConfig } = req @@ -59,6 +60,7 @@ async function manageGroupMembers(req, res, next) { groupSSOActive: ssoConfig?.enabled, canUseFlexibleLicensing: plan?.canUseFlexibleLicensing, canUseAddSeatsFeature, + entityAccess: UserMembershipAuthorization.hasEntityAccess()(req), }) } diff --git a/services/web/app/src/Features/UserMembership/UserMembershipMiddleware.js b/services/web/app/src/Features/UserMembership/UserMembershipMiddleware.js index 165edc3309..5c47baa85a 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipMiddleware.js +++ b/services/web/app/src/Features/UserMembership/UserMembershipMiddleware.js @@ -51,6 +51,19 @@ const UserMembershipMiddleware = { ]), ], + requireGroupMemberManagementAccess: [ + AuthenticationController.requireLogin(), + fetchEntityConfig('group'), + fetchEntity(), + requireEntity(), + useAdminCapabilities, + allowAccessIfAny([ + UserMembershipAuthorization.hasEntityAccess(), + UserMembershipAuthorization.hasStaffAccess('groupManagement'), + UserMembershipAuthorization.hasModifyGroupMemberCapability, + ]), + ], + requireGroupMetricsAccess: [ AuthenticationController.requireLogin(), fetchEntityConfig('group'), diff --git a/services/web/app/src/Features/UserMembership/UserMembershipRouter.mjs b/services/web/app/src/Features/UserMembership/UserMembershipRouter.mjs index 7013b1c93f..10633d3440 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipRouter.mjs +++ b/services/web/app/src/Features/UserMembership/UserMembershipRouter.mjs @@ -26,13 +26,13 @@ export default { ) webRouter.post( '/manage/groups/:id/invites', - UserMembershipMiddleware.requireGroupManagementAccess, + UserMembershipMiddleware.requireGroupMemberManagementAccess, RateLimiterMiddleware.rateLimit(rateLimiters.createTeamInvite), TeamInvitesController.createInvite ) webRouter.post( '/manage/groups/:id/resendInvite', - UserMembershipMiddleware.requireGroupManagementAccess, + UserMembershipMiddleware.requireGroupMemberManagementAccess, RateLimiterMiddleware.rateLimit(rateLimiters.createTeamInvite), TeamInvitesController.resendInvite ) @@ -43,7 +43,7 @@ export default { ) webRouter.delete( '/manage/groups/:id/invites/:email', - UserMembershipMiddleware.requireGroupManagementAccess, + UserMembershipMiddleware.requireGroupMemberManagementAccess, TeamInvitesController.revokeInvite ) webRouter.get( diff --git a/services/web/app/views/user_membership/group-members-react.pug b/services/web/app/views/user_membership/group-members-react.pug index 7767cf026e..e950f5f224 100644 --- a/services/web/app/views/user_membership/group-members-react.pug +++ b/services/web/app/views/user_membership/group-members-react.pug @@ -4,11 +4,13 @@ block entrypointVar - entrypoint = 'pages/user/subscription/group-management/group-members' block append meta + - var hasWriteAccess = entityAccess || (hasAdminAccess() && hasAdminCapability(managedUsersActive ? 'modify-managed-group-member' : 'modify-group-member')) || (getSessionUser().staffAccess && getSessionUser().staffAccess.groupManagement) meta(name='ol-user' data-type='json' content=user) meta(name='ol-users' data-type='json' content=users) meta(name='ol-groupId' data-type='string' content=groupId) meta(name='ol-groupName' data-type='string' content=name) meta(name='ol-groupSize' data-type='number' content=groupSize) + meta(name='ol-hasWriteAccess' data-type='boolean' content=hasWriteAccess) meta( name='ol-managedUsersActive' data-type='boolean' diff --git a/services/web/frontend/js/features/group-management/components/group-members.tsx b/services/web/frontend/js/features/group-management/components/group-members.tsx index 73e6da8e7c..99210c5c5f 100644 --- a/services/web/frontend/js/features/group-management/components/group-members.tsx +++ b/services/web/frontend/js/features/group-management/components/group-members.tsx @@ -37,6 +37,7 @@ export default function GroupMembers() { const groupSize = getMeta('ol-groupSize') const canUseFlexibleLicensing = getMeta('ol-canUseFlexibleLicensing') const canUseAddSeatsFeature = getMeta('ol-canUseAddSeatsFeature') + const hasWriteAccess = getMeta('ol-hasWriteAccess') const handleEmailsChange = useCallback( (e: ChangeEvent) => { @@ -133,10 +134,10 @@ export default function GroupMembers() {
- +
- {users.length < groupSize && ( + {hasWriteAccess && users.length < groupSize && (
)} - {users.length >= groupSize && users.length > 0 && ( + {(!hasWriteAccess || + (users.length >= groupSize && users.length > 0)) && ( <> diff --git a/services/web/frontend/js/features/group-management/components/members-table/member-row.tsx b/services/web/frontend/js/features/group-management/components/members-table/member-row.tsx index 84d97b0767..c1beb8938c 100644 --- a/services/web/frontend/js/features/group-management/components/members-table/member-row.tsx +++ b/services/web/frontend/js/features/group-management/components/members-table/member-row.tsx @@ -20,6 +20,7 @@ type ManagedUserRowProps = { openUnlinkUserModal: (user: User) => void groupId: string setGroupUserAlert: Dispatch> + hasWriteAccess: boolean } export default function MemberRow({ @@ -29,6 +30,7 @@ export default function MemberRow({ openUnlinkUserModal, setGroupUserAlert, groupId, + hasWriteAccess, }: ManagedUserRowProps) { const { t } = useTranslation() const managedUsersActive = getMeta('ol-managedUsersActive') @@ -36,7 +38,7 @@ export default function MemberRow({ return ( - + {hasWriteAccess && } )} - - - + {hasWriteAccess && ( + + + + )} ) } diff --git a/services/web/frontend/js/features/group-management/components/members-table/members-list.tsx b/services/web/frontend/js/features/group-management/components/members-table/members-list.tsx index 9202315a31..102dc3f021 100644 --- a/services/web/frontend/js/features/group-management/components/members-table/members-list.tsx +++ b/services/web/frontend/js/features/group-management/components/members-table/members-list.tsx @@ -26,6 +26,7 @@ const USERS_DISPLAY_LIMIT = 50 type ManagedUsersListProps = { groupId: string + hasWriteAccess: boolean } function isUserSearchMatch(user: User, search: NonEmptyString): boolean { @@ -40,7 +41,10 @@ function isUserSearchMatch(user: User, search: NonEmptyString): boolean { ) } -export default function MembersList({ groupId }: ManagedUsersListProps) { +export default function MembersList({ + groupId, + hasWriteAccess, +}: ManagedUsersListProps) { const { t } = useTranslation() const [userToOffboard, setUserToOffboard] = useState( undefined @@ -155,7 +159,7 @@ export default function MembersList({ groupId }: ManagedUsersListProps) { > - + {hasWriteAccess && } {t('email')} {t('name')} @@ -178,7 +182,7 @@ export default function MembersList({ groupId }: ManagedUsersListProps) { {managedUsersActive && ( {t('managed')} )} - + {hasWriteAccess && } @@ -203,6 +207,7 @@ export default function MembersList({ groupId }: ManagedUsersListProps) { openUnlinkUserModal={setUserToUnlink} setGroupUserAlert={setGroupUserAlert} groupId={groupId} + hasWriteAccess={hasWriteAccess} /> ))} diff --git a/services/web/test/frontend/features/group-management/components/group-members.spec.tsx b/services/web/test/frontend/features/group-management/components/group-members.spec.tsx index 8bcc1d2edc..b825be74a3 100644 --- a/services/web/test/frontend/features/group-management/components/group-members.spec.tsx +++ b/services/web/test/frontend/features/group-management/components/group-members.spec.tsx @@ -43,6 +43,7 @@ describe('GroupMembers', function () { win.metaAttributesCache.set('ol-groupName', 'My Awesome Team') win.metaAttributesCache.set('ol-groupSize', 10) win.metaAttributesCache.set('ol-users', [JOHN_DOE, BOBBY_LAPOINTE]) + win.metaAttributesCache.set('ol-hasWriteAccess', true) }) cy.mount( @@ -243,6 +244,7 @@ describe('GroupMembers', function () { win.metaAttributesCache.set('ol-groupName', 'My Awesome Team') win.metaAttributesCache.set('ol-groupSize', 10) win.metaAttributesCache.set('ol-managedUsersActive', true) + win.metaAttributesCache.set('ol-hasWriteAccess', true) }) mountGroupMembersProvider() }) @@ -514,6 +516,7 @@ describe('GroupMembers', function () { win.metaAttributesCache.set('ol-groupSize', 10) win.metaAttributesCache.set('ol-canUseFlexibleLicensing', true) win.metaAttributesCache.set('ol-canUseAddSeatsFeature', true) + win.metaAttributesCache.set('ol-hasWriteAccess', true) }) }) diff --git a/services/web/test/frontend/features/group-management/components/managed-group-members.spec.tsx b/services/web/test/frontend/features/group-management/components/managed-group-members.spec.tsx index cfcadc0332..b17ea6b85f 100644 --- a/services/web/test/frontend/features/group-management/components/managed-group-members.spec.tsx +++ b/services/web/test/frontend/features/group-management/components/managed-group-members.spec.tsx @@ -77,6 +77,7 @@ describe('group members, with managed users', function () { win.metaAttributesCache.set('ol-groupName', 'My Awesome Team') win.metaAttributesCache.set('ol-groupSize', 10) win.metaAttributesCache.set('ol-managedUsersActive', true) + win.metaAttributesCache.set('ol-hasWriteAccess', true) }) mountGroupMembersProvider() }) diff --git a/services/web/test/frontend/features/group-management/components/members-table/member-row.spec.tsx b/services/web/test/frontend/features/group-management/components/members-table/member-row.spec.tsx index 0ae6ee8d04..4450f1d687 100644 --- a/services/web/test/frontend/features/group-management/components/members-table/member-row.spec.tsx +++ b/services/web/test/frontend/features/group-management/components/members-table/member-row.spec.tsx @@ -34,6 +34,7 @@ describe('MemberRow', function () { openUnlinkUserModal={sinon.stub()} groupId={subscriptionId} setGroupUserAlert={sinon.stub()} + hasWriteAccess /> ) @@ -87,6 +88,7 @@ describe('MemberRow', function () { openUnlinkUserModal={sinon.stub()} groupId={subscriptionId} setGroupUserAlert={sinon.stub()} + hasWriteAccess /> ) @@ -127,6 +129,7 @@ describe('MemberRow', function () { openUnlinkUserModal={sinon.stub()} groupId={subscriptionId} setGroupUserAlert={sinon.stub()} + hasWriteAccess /> ) @@ -166,6 +169,7 @@ describe('MemberRow', function () { openUnlinkUserModal={sinon.stub()} groupId={subscriptionId} setGroupUserAlert={sinon.stub()} + hasWriteAccess /> ) @@ -215,6 +219,7 @@ describe('MemberRow', function () { openUnlinkUserModal={sinon.stub()} groupId={subscriptionId} setGroupUserAlert={sinon.stub()} + hasWriteAccess /> ) @@ -267,6 +272,7 @@ describe('MemberRow', function () { openUnlinkUserModal={sinon.stub()} groupId={subscriptionId} setGroupUserAlert={sinon.stub()} + hasWriteAccess /> ) @@ -307,6 +313,7 @@ describe('MemberRow', function () { openUnlinkUserModal={sinon.stub()} groupId={subscriptionId} setGroupUserAlert={sinon.stub()} + hasWriteAccess /> ) @@ -346,6 +353,7 @@ describe('MemberRow', function () { openUnlinkUserModal={sinon.stub()} groupId={subscriptionId} setGroupUserAlert={sinon.stub()} + hasWriteAccess /> ) @@ -395,6 +403,7 @@ describe('MemberRow', function () { openUnlinkUserModal={sinon.stub()} groupId={subscriptionId} setGroupUserAlert={sinon.stub()} + hasWriteAccess /> ) @@ -448,6 +457,7 @@ describe('MemberRow', function () { openUnlinkUserModal={sinon.stub()} groupId={subscriptionId} setGroupUserAlert={sinon.stub()} + hasWriteAccess /> ) @@ -488,6 +498,7 @@ describe('MemberRow', function () { openUnlinkUserModal={sinon.stub()} groupId={subscriptionId} setGroupUserAlert={sinon.stub()} + hasWriteAccess /> ) @@ -527,6 +538,7 @@ describe('MemberRow', function () { openUnlinkUserModal={sinon.stub()} groupId={subscriptionId} setGroupUserAlert={sinon.stub()} + hasWriteAccess /> ) @@ -577,6 +589,7 @@ describe('MemberRow', function () { openUnlinkUserModal={sinon.stub()} groupId={subscriptionId} setGroupUserAlert={sinon.stub()} + hasWriteAccess /> ) @@ -630,6 +643,7 @@ describe('MemberRow', function () { openUnlinkUserModal={sinon.stub()} groupId={subscriptionId} setGroupUserAlert={sinon.stub()} + hasWriteAccess /> ) @@ -670,6 +684,7 @@ describe('MemberRow', function () { openUnlinkUserModal={sinon.stub()} groupId={subscriptionId} setGroupUserAlert={sinon.stub()} + hasWriteAccess /> ) @@ -709,6 +724,7 @@ describe('MemberRow', function () { openUnlinkUserModal={sinon.stub()} groupId={subscriptionId} setGroupUserAlert={sinon.stub()} + hasWriteAccess /> ) diff --git a/services/web/test/frontend/features/group-management/components/members-table/members-list.spec.tsx b/services/web/test/frontend/features/group-management/components/members-table/members-list.spec.tsx index 087aa74d5f..930139b7a4 100644 --- a/services/web/test/frontend/features/group-management/components/members-table/members-list.spec.tsx +++ b/services/web/test/frontend/features/group-management/components/members-table/members-list.spec.tsx @@ -7,7 +7,7 @@ const groupId = 'somegroup' function mountManagedUsersList() { cy.mount( - + ) } @@ -166,7 +166,7 @@ describe('MembersList', function () { }) cy.mount( - + ) }) diff --git a/services/web/test/unit/src/UserMembership/UserMembershipController.test.mjs b/services/web/test/unit/src/UserMembership/UserMembershipController.test.mjs index 1726b2a41b..5fcb791ec5 100644 --- a/services/web/test/unit/src/UserMembership/UserMembershipController.test.mjs +++ b/services/web/test/unit/src/UserMembership/UserMembershipController.test.mjs @@ -35,6 +35,7 @@ describe('UserMembershipController', function () { ctx.subscription = { _id: 'mock-subscription-id', admin_id: 'mock-admin-id', + manager_ids: ['mock-admin-id'], fetchV1Data: callback => callback(null, ctx.subscription), } ctx.institution = { @@ -201,6 +202,7 @@ describe('UserMembershipController', function () { describe('index', function () { beforeEach(function (ctx) { + ctx.req.user = ctx.user ctx.req.entity = ctx.subscription ctx.req.entityConfig = EntityConfigs.group }) diff --git a/services/web/types/admin-capabilities.ts b/services/web/types/admin-capabilities.ts index 7de5755c98..8e8e02f79d 100644 --- a/services/web/types/admin-capabilities.ts +++ b/services/web/types/admin-capabilities.ts @@ -5,9 +5,11 @@ export type AdminCapability = | 'create-subscription' | 'modify-feature-override' | 'modify-group' + | 'modify-group-member' | 'modify-group-setting' | 'modify-login-status' | 'modify-managed-group' + | 'modify-managed-group-member' | 'modify-project' | 'manage-survey' | 'modify-split-test'