diff --git a/backend/__tests__/services/userManagementService.roleGrantGuard.test.js b/backend/__tests__/services/userManagementService.roleGrantGuard.test.js index f8ad380b..a8bc71c2 100644 --- a/backend/__tests__/services/userManagementService.roleGrantGuard.test.js +++ b/backend/__tests__/services/userManagementService.roleGrantGuard.test.js @@ -1,5 +1,6 @@ /** - * Privilege-escalation guard for PUT /api/admin/users/:id (GHSA-rv8w-m6mx-7j4q). + * Privilege-escalation guard for PUT /api/admin/users/:id and + * POST /api/admin/users/invite (GHSA-rv8w-m6mx-7j4q). * * updateAdminUser's role-change path previously enforced only: * (a) non-super_admin actors can't grant the super_admin role @@ -10,9 +11,21 @@ * built-in `admin` role) carrying far more permissions than the actor * itself held. * + * createInvitation() had the identical gap: it only ever blocked + * granting super_admin, so an admin holding only `users.create` could + * invite a brand-new admin into any other role — including one carrying + * far more permissions than the inviter itself held — via + * POST /admin/users/invite. + * * The fix reuses assertActorMayGrant() — the same containment already - * applied to roles.manage (see adminRolesGuards.test.js) — inside the - * role_id branch of updateAdminUser. + * applied to roles.manage (see adminRolesGuards.test.js) — inside both + * updateAdminUser's role_id branch and createInvitation(). + * + * Both describe blocks below share a single bootCrmDb() call: the + * `db` module (`src/database/db.js`) is a singleton keyed off + * TEST_DATABASE_PATH at first require, and bootCrmDb's own comment + * warns that a second call after the first's cleanup() destroys the + * pool, leaving "Unable to acquire a connection" for every later query. */ const path = require('path'); const fs = require('fs'); @@ -29,19 +42,25 @@ const { bootCrmDb, seedMinimal, assignAdminRole } = require('../integration/help const svc = require('../../src/services/userManagementService'); const { clearPermissionCache } = require('../../src/middleware/permissions'); +let db; let cleanup; +let superId; + +beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + ({ adminId: superId } = await seedMinimal(db)); + await assignAdminRole(db, superId, 'super_admin'); + clearPermissionCache(); +}, 120000); + +afterAll(async () => { if (cleanup) await cleanup(); }); + describe('updateAdminUser — role-grant privilege-escalation guard (GHSA-rv8w-m6mx-7j4q)', () => { - let db; let cleanup; - let superId; let limitedRoleId; let limitedId; // holds only users.edit + events.view let powerfulRoleId; // carries settings.banking, which limitedId does NOT hold let modestRoleId; // carries only events.view, a subset of what limitedId holds let targetId; // account whose role limitedId will try to change beforeAll(async () => { - ({ db, cleanup } = await bootCrmDb()); - ({ adminId: superId } = await seedMinimal(db)); - await assignAdminRole(db, superId, 'super_admin'); - // The attacker in GHSA-rv8w-m6mx-7j4q: users.edit only, nothing else. const limitedRole = await svc.createRole( { name: 'limited_user_editor', permissions: ['users.edit', 'events.view'] }, @@ -71,8 +90,6 @@ describe('updateAdminUser — role-grant privilege-escalation guard (GHSA-rv8w-m clearPermissionCache(); }, 120000); - afterAll(async () => { if (cleanup) await cleanup(); }); - beforeEach(async () => { // Fresh target for every test, role reset to modestRole so role-change // assertions always start from a known baseline. @@ -136,3 +153,83 @@ describe('updateAdminUser — role-grant privilege-escalation guard (GHSA-rv8w-m expect(updated.role_id).toBe(powerfulRoleId); }); }); + +describe('createInvitation — role-grant privilege-escalation guard (GHSA-rv8w-m6mx-7j4q)', () => { + let limitedRoleId; let limitedId; // holds only users.create + events.view + let powerfulRoleId; // carries settings.banking, which limitedId does NOT hold + let modestRoleId; // carries only events.view, a subset of what limitedId holds + let inviteCounter = 0; + + beforeAll(async () => { + const limitedRole = await svc.createRole( + { name: 'limited_inviter', permissions: ['users.create', 'events.view'] }, + superId, + ); + limitedRoleId = limitedRole.id; + const limitedIns = await db('admin_users').insert({ + username: 'limited_inviter', email: 'limited_inviter@example.com', password_hash: 'x', + role_id: limitedRoleId, must_change_password: false, created_at: new Date(), + }).returning('id'); + limitedId = limitedIns[0]?.id ?? limitedIns[0]; + + const powerfulRole = await svc.createRole( + { name: 'powerful_invite_role', permissions: ['users.create', 'settings.banking'] }, + superId, + ); + powerfulRoleId = powerfulRole.id; + + const modestRole = await svc.createRole( + { name: 'modest_invite_role', permissions: ['events.view'] }, + superId, + ); + modestRoleId = modestRole.id; + + clearPermissionCache(); + }, 120000); + + function nextEmail() { + inviteCounter += 1; + return `invitee-${inviteCounter}@example.com`; + } + + it('refuses to let an admin invite someone into a role carrying permissions the admin lacks', async () => { + await expect( + svc.createInvitation({ + email: nextEmail(), + roleId: powerfulRoleId, + invitedById: limitedId, + inviterRoleName: 'limited_inviter', + }), + ).rejects.toThrow(/only grant permissions your own role/i); + }); + + it('allows an admin to invite someone into a role whose permissions it already holds', async () => { + const invitation = await svc.createInvitation({ + email: nextEmail(), + roleId: limitedRoleId, + invitedById: limitedId, + inviterRoleName: 'limited_inviter', + }); + expect(invitation.role).toBeTruthy(); + }); + + it('allows an admin to invite someone into a role that is a subset of its own permissions', async () => { + const invitation = await svc.createInvitation({ + email: nextEmail(), + roleId: modestRoleId, + invitedById: limitedId, + inviterRoleName: 'limited_inviter', + }); + expect(invitation.role).toBeTruthy(); + }); + + it('super_admin can still invite into any role, including one carrying more permissions than a limited actor holds', async () => { + const invitation = await svc.createInvitation({ + email: nextEmail(), + roleId: powerfulRoleId, + invitedById: superId, + inviterRoleName: 'super_admin', + }); + expect(invitation.role).toBeTruthy(); + }); +}); diff --git a/backend/src/services/userManagementService.js b/backend/src/services/userManagementService.js index 2053a856..d1571eb5 100644 --- a/backend/src/services/userManagementService.js +++ b/backend/src/services/userManagementService.js @@ -48,6 +48,16 @@ async function createInvitation({ email, roleId, invitedById, inviterRoleName }) throw new ValidationError('Only Super Admins can invite new Super Admins'); } + // Privilege-escalation guard (GHSA-rv8w-m6mx-7j4q): holding `users.create` + // must not let an actor invite someone into a role carrying permissions + // they don't themselves have — same containment updateAdminUser already + // gives role assignment, reused here for invitations. + const targetRolePermissions = await db('role_permissions') + .join('permissions', 'permissions.id', 'role_permissions.permission_id') + .where('role_permissions.role_id', role.id) + .pluck('permissions.name'); + await assertActorMayGrant(invitedById, targetRolePermissions); + // Generate secure invitation token (64 characters hex = 32 bytes) const token = crypto.randomBytes(32).toString('hex'); const expiresAt = new Date(Date.now() + 7 * 24 * 60 * 60 * 1000); // 7 days