mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-04-03 14:27:20 +02:00
🪪 feat: Admin Roles API Endpoints (#12400)
* feat: add createRole and deleteRole methods to role
* feat: add admin roles handler factory and Express routes
* fix: address convention violations in admin roles handlers
* fix: rename createRole/deleteRole to avoid AccessRole name collision
The existing accessRole.ts already exports createRole/deleteRole for the
AccessRole model. In createMethods index.ts, these are spread after
roleMethods, overwriting them. Renamed our Role methods to
createRoleByName/deleteRoleByName to match the existing pattern
(getRoleByName, updateRoleByName) and avoid the collision.
* feat: add description field to Role model
- Add description to IRole, CreateRoleRequest, UpdateRoleRequest types
- Add description field to Mongoose roleSchema (default: '')
- Wire description through createRoleHandler and updateRoleHandler
- Include description in listRoles select clause so it appears in list
* fix: address Copilot review findings in admin roles handlers
* test: add unit tests for admin roles and groups handlers
* test: add data-layer tests for createRoleByName, deleteRoleByName, listUsersByRole
* fix: allow system role updates when name is unchanged
The updateRoleHandler guard rejected any request where body.name matched
a system role, even when the name was not being changed. This blocked
editing a system role's description. Compare against the URL param to
only reject actual renames to reserved names.
* fix: address external review findings for admin roles
- Block renaming system roles (ADMIN/USER) and add user migration on rename
- Add input validation: name max-length, trim on update, duplicate name check
- Replace fragile String.includes error matching with prefix-based classification
- Catch MongoDB 11000 duplicate key in createRoleByName
- Add pagination (limit/offset/total) to getRoleMembersHandler
- Reverse delete order in deleteRoleByName — reassign users before deletion
- Add role existence check in removeRoleMember; drop unused createdAt select
- Add Array.isArray guard for permissions input; use consistent ?? coalescing
- Fix import ordering per AGENTS.md conventions
- Type-cast mongoose.models.User as Model<IUser> for proper TS inference
- Add comprehensive tests: rename guards, pagination, validation, 500 paths
* fix: address re-review findings for admin roles
- Gate deleteRoleByName on existence check — skip user reassignment and
cache invalidation when role doesn't exist (fixes test mismatch)
- Reverse rename order: migrate users before renaming role so a migration
failure leaves the system in a consistent state
- Add .sort({ _id: 1 }) to listUsersByRole for deterministic pagination
- Import shared AdminMember type from data-schemas instead of local copy;
make joinedAt optional since neither groups nor roles populate it
- Change IRole.description from optional to required to match schema default
- Add data-layer tests for updateUsersByRole and countUsersByRole
- Add handler test verifying users-first rename ordering and migration
failure safety
* fix: add rollback on rename failure and update PR description
- Roll back user migration if updateRoleByName returns null during a
rename (race: role deleted between existence check and update)
- Add test verifying rollback calls updateUsersByRole in reverse
- Update PR #12400 description to reflect current test counts (56
handler tests, 40 data-layer tests) and safety features
* fix: rollback on rename throw, description validation, delete/DRY cleanup
- Hoist isRename/trimmedName above try block so catch can roll back user
migration when updateRoleByName throws (not just returns null)
- Add description type + max-length (2000) validation in create and update,
consistent with groups handler
- Remove redundant getRoleByName existence check in deleteRoleHandler —
use deleteRoleByName return value directly
- Skip no-op name write when body.name equals current name (use isRename)
- Extract getUserModel() accessor to DRY repeated Model<IUser> casts
- Use name.trim() consistently in createRoleByName error messages
- Add tests: rename-throw rollback, description validation (create+update),
update delete test mocks to match simplified handler
* fix: guard spurious rollback, harden createRole error path, validate before DB calls
- Add migrationRan flag to prevent rollback of user migration that never ran
- Return generic message on 500 in createRoleHandler, specific only for 409
- Move description validation before DB queries in updateRoleHandler
- Return existing role early when update body has no changes
- Wrap cache.set in createRoleByName with try/catch to prevent masking DB success
- Add JSDoc on 11000 catch explaining compound unique index
- Add tests: spurious rollback guard, empty update body, description validation
ordering, listUsersByRole pagination
* fix: validate permissions in create, RoleConflictError, rollback safety, cache consistency
- Add permissions type/array validation in createRoleHandler
- Introduce RoleConflictError class replacing fragile string-prefix matching
- Wrap rollback in !role null path with try/catch for correct 404 response
- Wrap deleteRoleByName cache.set in try/catch matching createRoleByName
- Narrow updateRoleHandler body type to { name?, description? }
- Add tests: non-string description in create, rollback failure logging,
permissions array rejection, description max-length assertion fix
* feat: prevent removing the last admin user
Add guard in removeRoleMember that checks countUsersByRole before
demoting an ADMIN user, returning 400 if they are the last one.
* fix: move interleaved export below imports, add await to countUsersByRole
* fix: paginate listRoles, null-guard permissions handler, fix export ordering
- Add limit/offset/total pagination to listRoles matching the groups pattern
- Add countRoles data-layer method
- Omit permissions from listRoles select (getRole returns full document)
- Null-guard re-fetched role in updateRolePermissionsHandler
- Move interleaved export below all imports in methods/index.ts
* fix: address review findings — race safety, validation DRY, type accuracy, test coverage
- Add post-write admin count verification in removeRoleMember to prevent
zero-admin race condition (TOCTOU → rollback if count hits 0)
- Make IRole.description optional; backfill in initializeRoles for
pre-existing roles that lack the field (.lean() bypasses defaults)
- Extract parsePagination, validateNameParam, validateRoleName, and
validateDescription helpers to eliminate duplicated validation
- Add validateNameParam guard to all 7 handlers reading req.params.name
- Catch 11000 in updateRoleByName and surface as 409 via RoleConflictError
- Add idempotent skip in addRoleMember when user already has target role
- Verify updateRolePermissions test asserts response body
- Add data-layer tests: listRoles sort/pagination/projection, countRoles,
and createRoleByName 11000 duplicate key race
* fix: defensive rollback in removeRoleMember, type/style cleanup, test coverage
- Wrap removeRoleMember post-write admin rollback in try/catch so a
transient DB failure cannot leave the system with zero administrators
- Replace double `as unknown[] as IRole[]` cast with `.lean<IRole[]>()`
- Type parsePagination param explicitly; extract DEFAULT/MAX page constants
- Preserve original error cause in updateRoleByName re-throw
- Add test for rollback failure path in removeRoleMember (returns 400)
- Add test for pre-existing roles missing description field (.lean())
* chore: bump @librechat/data-schemas to 0.0.47
* fix: stale cache on rename, extract renameRole helper, shared pagination, cleanup
- Fix updateRoleByName cache bug: invalidate old key and populate new key
when updates.name differs from roleName (prevents stale cache after rename)
- Extract renameRole helper to eliminate mutable outer-scope state flags
(isRename, trimmedName, migrationRan) in updateRoleHandler
- Unify system-role protection to 403 for both rename-from and rename-to
- Extract parsePagination to shared admin/pagination.ts; use in both
roles.ts and groups.ts
- Extract name.trim() to local const in createRoleByName (was called 5×)
- Remove redundant findOne pre-check in deleteRoleByName
- Replace getUserModel closure with local const declarations
- Remove redundant description ?? '' in createRoleHandler (schema default)
- Add doc comment on updateRolePermissionsHandler noting cache dependency
- Add data-layer tests for cache rename behavior (old key null, new key set)
* fix: harden role guards, add User.role index, validate names, improve tests
- Add index on User.role field for efficient member queries at scale
- Replace fragile SystemRoles key lookup with value-based Set check (6 sites)
- Elevate rename rollback failure logging to CRITICAL (matches removeRoleMember)
- Guard removeRoleMember against non-ADMIN system roles (403 for USER)
- Fix parsePagination limit=0 gotcha: use parseInt + NaN check instead of ||
- Add control character and reserved path segment validation to role names
- Simplify validateRoleName: remove redundant casts and dead conditions
- Add JSDoc to deleteRoleByName documenting non-atomic window
- Split mixed value+type import in methods/index.ts per AGENTS.md
- Add 9 new tests: permissions assertion, combined rename+desc, createRole
with permissions, pagination edge cases, control char/reserved name
rejection, system role removeRoleMember guard
* fix: exact-case reserved name check, consistent validation, cleaner createRole
- Remove .toLowerCase() from reserved name check so only exact matches
(members, permissions) are rejected, not legitimate names like "Members"
- Extract trimmed const in validateRoleName for consistent validation
- Add control char check to validateNameParam for parity with body validation
- Build createRole roleData conditionally to avoid passing description: undefined
- Expand deleteRoleByName JSDoc documenting self-healing design and no-op trade-off
* fix: scope rename rollback to only migrated users, prevent cross-role corruption
Capture user IDs before forward migration so the rollback path only
reverts users this request actually moved. Previously the rollback called
updateUsersByRole(newName, currentName) which would sweep all users with
the new role — including any independently assigned by a concurrent admin
request — causing silent cross-role data corruption.
Adds findUserIdsByRole and updateUsersRoleByIds to the data layer.
Extracts rollbackMigratedUsers helper to deduplicate rollback sites.
* fix: guard last admin in addRoleMember to prevent zero-admin lockout
Since each user has exactly one role, addRoleMember implicitly removes
the user from their current role. Without a guard, reassigning the sole
admin to a non-admin role leaves zero admins and locks out admin
management. Adds the same countUsersByRole check used in removeRoleMember.
* fix: wire findUserIdsByRole and updateUsersRoleByIds into roles route
The scoped rollback deps added in c89b5db were missing from the route
DI wiring, causing renameRole to call undefined and return a 500.
* fix: post-write admin guard in addRoleMember, compound role index, review cleanup
- Add post-write admin count check + rollback to addRoleMember to match
removeRoleMember's two-phase TOCTOU protection (prevents zero-admin via
concurrent requests)
- Replace single-field User.role index with compound { role: 1, tenantId: 1 }
to align with existing multi-tenant index pattern (email, OAuth IDs)
- Narrow listRoles dep return type to RoleListItem (projected fields only)
- Refactor validateDescription to early-return style per AGENTS.md
- Remove redundant double .lean() in updateRoleByName
- Document rename snapshot race window in renameRole JSDoc
- Document cache null-set behavior in deleteRoleByName
- Add routing-coupling comment on RESERVED_ROLE_NAMES
- Add test for addRoleMember post-write rollback
* fix: review cleanup — system-role guard, type safety, JSDoc accuracy, tests
- Add system-role guard to addRoleMember: block direct assignment to
non-ADMIN system roles (403), symmetric with removeRoleMember
- Fix RESERVED_ROLE_NAMES comment: explain semantic URL ambiguity, not
a routing conflict (Express resolves single vs multi-segment correctly)
- Replace _id: unknown with Types.ObjectId | string per AGENTS.md
- Narrow listRoles data-layer return type to Pick<IRole, 'name' | 'description'>
to match the actual .select() projection
- Move updateRoleHandler param check inside try/catch for consistency
- Include user IDs in all CRITICAL rollback failure logs for operator recovery
- Clarify deleteRoleByName JSDoc: replace "self-healing" with "idempotent",
document that recovery requires caller retry
- Add tests: system-role guard, promote non-admin to ADMIN,
findUserIdsByRole throw prevents migration
* fix: include _id in listRoles return type to match RoleListItem
Pick<IRole, 'name' | 'description'> omits _id, making it incompatible
with the handler dep's RoleListItem which requires _id.
* fix: case-insensitive system role guard, reject null permissions, check updateUser result
- System role name checks now use case-insensitive comparison via
toUpperCase() — prevents creating 'admin' or 'user' which would
collide with the legacy roles route that uppercases params
- Reject permissions: null in createRole (typeof null === 'object'
was bypassing the validation)
- Check updateUser return in addRoleMember — return 404 if the user
was deleted between the findUser and updateUser calls
* fix: check updateUser return in removeRoleMember for concurrent delete safety
---------
Co-authored-by: Danny Avila <danny@librechat.ai>
This commit is contained in:
Dustin Healy
GitHub
parent
2e3d66cfe2
commit
5972a21479
17 changed files with 2673 additions and 23 deletions
|
|
@ -13,6 +13,7 @@ import type { FilterQuery, ClientSession, DeleteResult } from 'mongoose';
|
|||
import type { Response } from 'express';
|
||||
import type { ValidationError } from '~/types/error';
|
||||
import type { ServerRequest } from '~/types/http';
|
||||
import { parsePagination } from './pagination';
|
||||
|
||||
type GroupListFilter = Pick<GroupFilterOptions, 'source' | 'search'>;
|
||||
|
||||
|
|
@ -119,8 +120,7 @@ export function createAdminGroupsHandlers(deps: AdminGroupsDeps) {
|
|||
if (search) {
|
||||
filter.search = search;
|
||||
}
|
||||
const limit = Math.min(Math.max(Number(req.query.limit) || 50, 1), 200);
|
||||
const offset = Math.max(Number(req.query.offset) || 0, 0);
|
||||
const { limit, offset } = parsePagination(req.query);
|
||||
const [groups, total] = await Promise.all([
|
||||
listGroups({ ...filter, limit, offset }),
|
||||
countGroups(filter),
|
||||
|
|
@ -348,8 +348,7 @@ export function createAdminGroupsHandlers(deps: AdminGroupsDeps) {
|
|||
*/
|
||||
const allMemberIds = [...new Set(group.memberIds || [])];
|
||||
const total = allMemberIds.length;
|
||||
const limit = Math.min(Math.max(Number(req.query.limit) || 50, 1), 200);
|
||||
const offset = Math.max(Number(req.query.offset) || 0, 0);
|
||||
const { limit, offset } = parsePagination(req.query);
|
||||
|
||||
if (total === 0 || offset >= total) {
|
||||
return res.status(200).json({ members: [], total, limit, offset });
|
||||
|
|
|
|||
|
|
@ -1,4 +1,6 @@
|
|||
export { createAdminConfigHandlers } from './config';
|
||||
export { createAdminGroupsHandlers } from './groups';
|
||||
export { createAdminRolesHandlers } from './roles';
|
||||
export type { AdminConfigDeps } from './config';
|
||||
export type { AdminGroupsDeps } from './groups';
|
||||
export type { AdminRolesDeps } from './roles';
|
||||
|
|
|
|||
17
packages/api/src/admin/pagination.ts
Normal file
17
packages/api/src/admin/pagination.ts
Normal file
|
|
@ -0,0 +1,17 @@
|
|||
export const DEFAULT_PAGE_LIMIT = 50;
|
||||
export const MAX_PAGE_LIMIT = 200;
|
||||
|
||||
export function parsePagination(query: { limit?: string; offset?: string }): {
|
||||
limit: number;
|
||||
offset: number;
|
||||
} {
|
||||
const rawLimit = parseInt(query.limit ?? '', 10);
|
||||
const rawOffset = parseInt(query.offset ?? '', 10);
|
||||
return {
|
||||
limit: Math.min(
|
||||
Math.max(Number.isNaN(rawLimit) ? DEFAULT_PAGE_LIMIT : rawLimit, 1),
|
||||
MAX_PAGE_LIMIT,
|
||||
),
|
||||
offset: Math.max(Number.isNaN(rawOffset) ? 0 : rawOffset, 0),
|
||||
};
|
||||
}
|
||||
1484
packages/api/src/admin/roles.spec.ts
Normal file
1484
packages/api/src/admin/roles.spec.ts
Normal file
File diff suppressed because it is too large
Load diff
550
packages/api/src/admin/roles.ts
Normal file
550
packages/api/src/admin/roles.ts
Normal file
|
|
@ -0,0 +1,550 @@
|
|||
import { SystemRoles } from 'librechat-data-provider';
|
||||
import { logger, isValidObjectIdString, RoleConflictError } from '@librechat/data-schemas';
|
||||
import type { IRole, IUser, AdminMember } from '@librechat/data-schemas';
|
||||
import type { FilterQuery, Types } from 'mongoose';
|
||||
import type { Response } from 'express';
|
||||
import type { ServerRequest } from '~/types/http';
|
||||
import { parsePagination } from './pagination';
|
||||
|
||||
const systemRoleValues = new Set<string>(Object.values(SystemRoles));
|
||||
|
||||
/** Case-insensitive check — the legacy roles route uppercases params. */
|
||||
function isSystemRoleName(name: string): boolean {
|
||||
return systemRoleValues.has(name.toUpperCase());
|
||||
}
|
||||
|
||||
const MAX_NAME_LENGTH = 500;
|
||||
const MAX_DESCRIPTION_LENGTH = 2000;
|
||||
const CONTROL_CHAR_RE = /\p{Cc}/u;
|
||||
/**
|
||||
* Role names that would create semantically ambiguous URLs.
|
||||
* e.g. GET /api/admin/roles/members — is that "list roles" or "get role named members"?
|
||||
* Express routing resolves this correctly (single vs multi-segment), but the URLs
|
||||
* are confusing for API consumers. Keep in sync with sub-path routes in routes/admin/roles.js.
|
||||
*/
|
||||
const RESERVED_ROLE_NAMES = new Set(['members', 'permissions']);
|
||||
|
||||
function validateNameParam(name: string): string | null {
|
||||
if (!name || typeof name !== 'string') {
|
||||
return 'name parameter is required';
|
||||
}
|
||||
if (name.length > MAX_NAME_LENGTH) {
|
||||
return `name must not exceed ${MAX_NAME_LENGTH} characters`;
|
||||
}
|
||||
if (CONTROL_CHAR_RE.test(name)) {
|
||||
return 'name contains invalid characters';
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function validateRoleName(name: unknown, required: boolean): string | null {
|
||||
if (name === undefined) {
|
||||
return required ? 'name is required' : null;
|
||||
}
|
||||
if (typeof name !== 'string' || !name.trim()) {
|
||||
return required ? 'name is required' : 'name must be a non-empty string';
|
||||
}
|
||||
const trimmed = name.trim();
|
||||
if (trimmed.length > MAX_NAME_LENGTH) {
|
||||
return `name must not exceed ${MAX_NAME_LENGTH} characters`;
|
||||
}
|
||||
if (CONTROL_CHAR_RE.test(trimmed)) {
|
||||
return 'name contains invalid characters';
|
||||
}
|
||||
if (RESERVED_ROLE_NAMES.has(trimmed)) {
|
||||
return 'name is a reserved path segment';
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function validateDescription(description: unknown): string | null {
|
||||
if (description === undefined) {
|
||||
return null;
|
||||
}
|
||||
if (typeof description !== 'string') {
|
||||
return 'description must be a string';
|
||||
}
|
||||
if (description.length > MAX_DESCRIPTION_LENGTH) {
|
||||
return `description must not exceed ${MAX_DESCRIPTION_LENGTH} characters`;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
interface RoleNameParams {
|
||||
name: string;
|
||||
}
|
||||
|
||||
interface RoleMemberParams extends RoleNameParams {
|
||||
userId: string;
|
||||
}
|
||||
|
||||
export type RoleListItem = { _id: Types.ObjectId | string; name: string; description?: string };
|
||||
|
||||
export interface AdminRolesDeps {
|
||||
listRoles: (options?: { limit?: number; offset?: number }) => Promise<RoleListItem[]>;
|
||||
countRoles: () => Promise<number>;
|
||||
getRoleByName: (name: string, fields?: string | string[] | null) => Promise<IRole | null>;
|
||||
createRoleByName: (roleData: Partial<IRole>) => Promise<IRole>;
|
||||
updateRoleByName: (name: string, updates: Partial<IRole>) => Promise<IRole | null>;
|
||||
updateAccessPermissions: (
|
||||
name: string,
|
||||
perms: Record<string, Record<string, boolean>>,
|
||||
roleData?: IRole,
|
||||
) => Promise<void>;
|
||||
deleteRoleByName: (name: string) => Promise<IRole | null>;
|
||||
findUser: (
|
||||
criteria: FilterQuery<IUser>,
|
||||
fields?: string | string[] | null,
|
||||
) => Promise<IUser | null>;
|
||||
updateUser: (userId: string, data: Partial<IUser>) => Promise<IUser | null>;
|
||||
updateUsersByRole: (oldRole: string, newRole: string) => Promise<void>;
|
||||
findUserIdsByRole: (roleName: string) => Promise<string[]>;
|
||||
updateUsersRoleByIds: (userIds: string[], newRole: string) => Promise<void>;
|
||||
listUsersByRole: (
|
||||
roleName: string,
|
||||
options?: { limit?: number; offset?: number },
|
||||
) => Promise<IUser[]>;
|
||||
countUsersByRole: (roleName: string) => Promise<number>;
|
||||
}
|
||||
|
||||
export function createAdminRolesHandlers(deps: AdminRolesDeps) {
|
||||
const {
|
||||
listRoles,
|
||||
countRoles,
|
||||
getRoleByName,
|
||||
createRoleByName,
|
||||
updateRoleByName,
|
||||
updateAccessPermissions,
|
||||
deleteRoleByName,
|
||||
findUser,
|
||||
updateUser,
|
||||
updateUsersByRole,
|
||||
findUserIdsByRole,
|
||||
updateUsersRoleByIds,
|
||||
listUsersByRole,
|
||||
countUsersByRole,
|
||||
} = deps;
|
||||
|
||||
async function listRolesHandler(req: ServerRequest, res: Response) {
|
||||
try {
|
||||
const { limit, offset } = parsePagination(req.query);
|
||||
const [roles, total] = await Promise.all([listRoles({ limit, offset }), countRoles()]);
|
||||
return res.status(200).json({ roles, total, limit, offset });
|
||||
} catch (error) {
|
||||
logger.error('[adminRoles] listRoles error:', error);
|
||||
return res.status(500).json({ error: 'Failed to list roles' });
|
||||
}
|
||||
}
|
||||
|
||||
async function getRoleHandler(req: ServerRequest, res: Response) {
|
||||
try {
|
||||
const { name } = req.params as RoleNameParams;
|
||||
const paramError = validateNameParam(name);
|
||||
if (paramError) {
|
||||
return res.status(400).json({ error: paramError });
|
||||
}
|
||||
const role = await getRoleByName(name);
|
||||
if (!role) {
|
||||
return res.status(404).json({ error: 'Role not found' });
|
||||
}
|
||||
return res.status(200).json({ role });
|
||||
} catch (error) {
|
||||
logger.error('[adminRoles] getRole error:', error);
|
||||
return res.status(500).json({ error: 'Failed to get role' });
|
||||
}
|
||||
}
|
||||
|
||||
async function createRoleHandler(req: ServerRequest, res: Response) {
|
||||
try {
|
||||
const { name, description, permissions } = req.body as {
|
||||
name?: string;
|
||||
description?: string;
|
||||
permissions?: IRole['permissions'];
|
||||
};
|
||||
const nameError = validateRoleName(name, true);
|
||||
if (nameError) {
|
||||
return res.status(400).json({ error: nameError });
|
||||
}
|
||||
const descError = validateDescription(description);
|
||||
if (descError) {
|
||||
return res.status(400).json({ error: descError });
|
||||
}
|
||||
if (
|
||||
permissions !== undefined &&
|
||||
(permissions === null || typeof permissions !== 'object' || Array.isArray(permissions))
|
||||
) {
|
||||
return res.status(400).json({ error: 'permissions must be an object' });
|
||||
}
|
||||
const roleData: Partial<IRole> = {
|
||||
name: (name as string).trim(),
|
||||
permissions: permissions ?? {},
|
||||
};
|
||||
if (description !== undefined) {
|
||||
roleData.description = description;
|
||||
}
|
||||
const role = await createRoleByName(roleData);
|
||||
return res.status(201).json({ role });
|
||||
} catch (error) {
|
||||
logger.error('[adminRoles] createRole error:', error);
|
||||
if (error instanceof RoleConflictError) {
|
||||
return res.status(409).json({ error: error.message });
|
||||
}
|
||||
return res.status(500).json({ error: 'Failed to create role' });
|
||||
}
|
||||
}
|
||||
|
||||
async function rollbackMigratedUsers(
|
||||
migratedIds: string[],
|
||||
currentName: string,
|
||||
newName: string,
|
||||
): Promise<void> {
|
||||
if (migratedIds.length === 0) {
|
||||
return;
|
||||
}
|
||||
try {
|
||||
await updateUsersRoleByIds(migratedIds, currentName);
|
||||
} catch (rollbackError) {
|
||||
logger.error(
|
||||
`[adminRoles] CRITICAL: rename rollback failed — ${migratedIds.length} users have dangling role "${newName}": [${migratedIds.join(', ')}]`,
|
||||
rollbackError,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Renames a role by migrating users to the new name and updating the role document.
|
||||
*
|
||||
* The ID snapshot from `findUserIdsByRole` is a point-in-time read. Users assigned
|
||||
* to `currentName` between the snapshot and the bulk `updateUsersByRole` write will
|
||||
* be moved to `newName` but will NOT be reverted on rollback. This window is narrow
|
||||
* and only relevant under concurrent admin operations during a rename.
|
||||
*/
|
||||
async function renameRole(
|
||||
currentName: string,
|
||||
newName: string,
|
||||
extraUpdates?: Partial<IRole>,
|
||||
): Promise<IRole | null> {
|
||||
const migratedIds = await findUserIdsByRole(currentName);
|
||||
await updateUsersByRole(currentName, newName);
|
||||
try {
|
||||
const updates: Partial<IRole> = { name: newName, ...extraUpdates };
|
||||
const role = await updateRoleByName(currentName, updates);
|
||||
if (!role) {
|
||||
await rollbackMigratedUsers(migratedIds, currentName, newName);
|
||||
}
|
||||
return role;
|
||||
} catch (error) {
|
||||
await rollbackMigratedUsers(migratedIds, currentName, newName);
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
async function updateRoleHandler(req: ServerRequest, res: Response) {
|
||||
try {
|
||||
const { name } = req.params as RoleNameParams;
|
||||
const paramError = validateNameParam(name);
|
||||
if (paramError) {
|
||||
return res.status(400).json({ error: paramError });
|
||||
}
|
||||
const body = req.body as { name?: string; description?: string };
|
||||
const nameError = validateRoleName(body.name, false);
|
||||
if (nameError) {
|
||||
return res.status(400).json({ error: nameError });
|
||||
}
|
||||
const descError = validateDescription(body.description);
|
||||
if (descError) {
|
||||
return res.status(400).json({ error: descError });
|
||||
}
|
||||
|
||||
const trimmedName = body.name?.trim() ?? '';
|
||||
const isRename = trimmedName !== '' && trimmedName !== name;
|
||||
|
||||
if (isRename && isSystemRoleName(name)) {
|
||||
return res.status(403).json({ error: 'Cannot rename system role' });
|
||||
}
|
||||
if (isRename && isSystemRoleName(trimmedName)) {
|
||||
return res.status(403).json({ error: 'Cannot use a reserved system role name' });
|
||||
}
|
||||
|
||||
const existing = await getRoleByName(name);
|
||||
if (!existing) {
|
||||
return res.status(404).json({ error: 'Role not found' });
|
||||
}
|
||||
|
||||
if (isRename) {
|
||||
const duplicate = await getRoleByName(trimmedName);
|
||||
if (duplicate) {
|
||||
return res.status(409).json({ error: `Role "${trimmedName}" already exists` });
|
||||
}
|
||||
}
|
||||
|
||||
const updates: Partial<IRole> = {};
|
||||
if (isRename) {
|
||||
updates.name = trimmedName;
|
||||
}
|
||||
if (body.description !== undefined) {
|
||||
updates.description = body.description;
|
||||
}
|
||||
|
||||
if (Object.keys(updates).length === 0) {
|
||||
return res.status(200).json({ role: existing });
|
||||
}
|
||||
|
||||
if (isRename) {
|
||||
const descUpdate =
|
||||
body.description !== undefined ? { description: body.description } : undefined;
|
||||
const role = await renameRole(name, trimmedName, descUpdate);
|
||||
if (!role) {
|
||||
return res.status(404).json({ error: 'Role not found' });
|
||||
}
|
||||
return res.status(200).json({ role });
|
||||
}
|
||||
|
||||
const role = await updateRoleByName(name, updates);
|
||||
if (!role) {
|
||||
return res.status(404).json({ error: 'Role not found' });
|
||||
}
|
||||
return res.status(200).json({ role });
|
||||
} catch (error) {
|
||||
if (error instanceof RoleConflictError) {
|
||||
return res.status(409).json({ error: error.message });
|
||||
}
|
||||
logger.error('[adminRoles] updateRole error:', error);
|
||||
return res.status(500).json({ error: 'Failed to update role' });
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The re-fetch via `getRoleByName` after `updateAccessPermissions` depends on the
|
||||
* callee having written the updated document to the role cache. If the cache layer
|
||||
* is refactored to stop writing from within `updateAccessPermissions`, this handler
|
||||
* must be updated to perform an explicit uncached DB read.
|
||||
*/
|
||||
async function updateRolePermissionsHandler(req: ServerRequest, res: Response) {
|
||||
try {
|
||||
const { name } = req.params as RoleNameParams;
|
||||
const paramError = validateNameParam(name);
|
||||
if (paramError) {
|
||||
return res.status(400).json({ error: paramError });
|
||||
}
|
||||
const { permissions } = req.body as {
|
||||
permissions: Record<string, Record<string, boolean>>;
|
||||
};
|
||||
|
||||
if (!permissions || typeof permissions !== 'object' || Array.isArray(permissions)) {
|
||||
return res.status(400).json({ error: 'permissions object is required' });
|
||||
}
|
||||
|
||||
const existing = await getRoleByName(name);
|
||||
if (!existing) {
|
||||
return res.status(404).json({ error: 'Role not found' });
|
||||
}
|
||||
|
||||
await updateAccessPermissions(name, permissions, existing);
|
||||
const updated = await getRoleByName(name);
|
||||
if (!updated) {
|
||||
return res.status(404).json({ error: 'Role not found' });
|
||||
}
|
||||
return res.status(200).json({ role: updated });
|
||||
} catch (error) {
|
||||
logger.error('[adminRoles] updateRolePermissions error:', error);
|
||||
return res.status(500).json({ error: 'Failed to update role permissions' });
|
||||
}
|
||||
}
|
||||
|
||||
async function deleteRoleHandler(req: ServerRequest, res: Response) {
|
||||
try {
|
||||
const { name } = req.params as RoleNameParams;
|
||||
const paramError = validateNameParam(name);
|
||||
if (paramError) {
|
||||
return res.status(400).json({ error: paramError });
|
||||
}
|
||||
if (isSystemRoleName(name)) {
|
||||
return res.status(403).json({ error: 'Cannot delete system role' });
|
||||
}
|
||||
|
||||
const deleted = await deleteRoleByName(name);
|
||||
if (!deleted) {
|
||||
return res.status(404).json({ error: 'Role not found' });
|
||||
}
|
||||
return res.status(200).json({ success: true });
|
||||
} catch (error) {
|
||||
logger.error('[adminRoles] deleteRole error:', error);
|
||||
return res.status(500).json({ error: 'Failed to delete role' });
|
||||
}
|
||||
}
|
||||
|
||||
async function getRoleMembersHandler(req: ServerRequest, res: Response) {
|
||||
try {
|
||||
const { name } = req.params as RoleNameParams;
|
||||
const paramError = validateNameParam(name);
|
||||
if (paramError) {
|
||||
return res.status(400).json({ error: paramError });
|
||||
}
|
||||
const existing = await getRoleByName(name);
|
||||
if (!existing) {
|
||||
return res.status(404).json({ error: 'Role not found' });
|
||||
}
|
||||
|
||||
const { limit, offset } = parsePagination(req.query);
|
||||
|
||||
const [users, total] = await Promise.all([
|
||||
listUsersByRole(name, { limit, offset }),
|
||||
countUsersByRole(name),
|
||||
]);
|
||||
const members: AdminMember[] = users.map((u) => ({
|
||||
userId: u._id?.toString() ?? '',
|
||||
name: u.name ?? u._id?.toString() ?? '',
|
||||
email: u.email ?? '',
|
||||
avatarUrl: u.avatar,
|
||||
}));
|
||||
return res.status(200).json({ members, total, limit, offset });
|
||||
} catch (error) {
|
||||
logger.error('[adminRoles] getRoleMembers error:', error);
|
||||
return res.status(500).json({ error: 'Failed to get role members' });
|
||||
}
|
||||
}
|
||||
|
||||
async function addRoleMemberHandler(req: ServerRequest, res: Response) {
|
||||
try {
|
||||
const { name } = req.params as RoleNameParams;
|
||||
const paramError = validateNameParam(name);
|
||||
if (paramError) {
|
||||
return res.status(400).json({ error: paramError });
|
||||
}
|
||||
const { userId } = req.body as { userId: string };
|
||||
|
||||
if (!userId || typeof userId !== 'string') {
|
||||
return res.status(400).json({ error: 'userId is required' });
|
||||
}
|
||||
if (!isValidObjectIdString(userId)) {
|
||||
return res.status(400).json({ error: 'Invalid user ID format' });
|
||||
}
|
||||
|
||||
if (isSystemRoleName(name) && name !== SystemRoles.ADMIN) {
|
||||
return res.status(403).json({ error: 'Cannot directly assign members to a system role' });
|
||||
}
|
||||
|
||||
const existing = await getRoleByName(name);
|
||||
if (!existing) {
|
||||
return res.status(404).json({ error: 'Role not found' });
|
||||
}
|
||||
|
||||
const user = await findUser({ _id: userId });
|
||||
if (!user) {
|
||||
return res.status(404).json({ error: 'User not found' });
|
||||
}
|
||||
|
||||
if (user.role === name) {
|
||||
return res.status(200).json({ success: true });
|
||||
}
|
||||
|
||||
if (user.role === SystemRoles.ADMIN && name !== SystemRoles.ADMIN) {
|
||||
const adminCount = await countUsersByRole(SystemRoles.ADMIN);
|
||||
if (adminCount <= 1) {
|
||||
return res.status(400).json({ error: 'Cannot remove the last admin user' });
|
||||
}
|
||||
}
|
||||
|
||||
const updated = await updateUser(userId, { role: name });
|
||||
if (!updated) {
|
||||
return res.status(404).json({ error: 'User not found' });
|
||||
}
|
||||
|
||||
if (user.role === SystemRoles.ADMIN && name !== SystemRoles.ADMIN) {
|
||||
const postCount = await countUsersByRole(SystemRoles.ADMIN);
|
||||
if (postCount === 0) {
|
||||
try {
|
||||
await updateUser(userId, { role: SystemRoles.ADMIN });
|
||||
} catch (rollbackError) {
|
||||
logger.error(
|
||||
`[adminRoles] CRITICAL: admin rollback failed in addRoleMember for user ${userId}:`,
|
||||
rollbackError,
|
||||
);
|
||||
}
|
||||
return res.status(400).json({ error: 'Cannot remove the last admin user' });
|
||||
}
|
||||
}
|
||||
|
||||
return res.status(200).json({ success: true });
|
||||
} catch (error) {
|
||||
logger.error('[adminRoles] addRoleMember error:', error);
|
||||
return res.status(500).json({ error: 'Failed to add role member' });
|
||||
}
|
||||
}
|
||||
|
||||
async function removeRoleMemberHandler(req: ServerRequest, res: Response) {
|
||||
try {
|
||||
const { name, userId } = req.params as RoleMemberParams;
|
||||
const paramError = validateNameParam(name);
|
||||
if (paramError) {
|
||||
return res.status(400).json({ error: paramError });
|
||||
}
|
||||
if (!isValidObjectIdString(userId)) {
|
||||
return res.status(400).json({ error: 'Invalid user ID format' });
|
||||
}
|
||||
|
||||
if (isSystemRoleName(name) && name !== SystemRoles.ADMIN) {
|
||||
return res.status(403).json({ error: 'Cannot remove members from a system role' });
|
||||
}
|
||||
|
||||
const existing = await getRoleByName(name);
|
||||
if (!existing) {
|
||||
return res.status(404).json({ error: 'Role not found' });
|
||||
}
|
||||
|
||||
const user = await findUser({ _id: userId });
|
||||
if (!user) {
|
||||
return res.status(404).json({ error: 'User not found' });
|
||||
}
|
||||
|
||||
if (user.role !== name) {
|
||||
return res.status(400).json({ error: 'User is not a member of this role' });
|
||||
}
|
||||
|
||||
if (name === SystemRoles.ADMIN) {
|
||||
const adminCount = await countUsersByRole(SystemRoles.ADMIN);
|
||||
if (adminCount <= 1) {
|
||||
return res.status(400).json({ error: 'Cannot remove the last admin user' });
|
||||
}
|
||||
}
|
||||
|
||||
const removed = await updateUser(userId, { role: SystemRoles.USER });
|
||||
if (!removed) {
|
||||
return res.status(404).json({ error: 'User not found' });
|
||||
}
|
||||
|
||||
if (name === SystemRoles.ADMIN) {
|
||||
const postCount = await countUsersByRole(SystemRoles.ADMIN);
|
||||
if (postCount === 0) {
|
||||
try {
|
||||
await updateUser(userId, { role: SystemRoles.ADMIN });
|
||||
} catch (rollbackError) {
|
||||
logger.error(
|
||||
`[adminRoles] CRITICAL: admin rollback failed for user ${userId}:`,
|
||||
rollbackError,
|
||||
);
|
||||
}
|
||||
return res.status(400).json({ error: 'Cannot remove the last admin user' });
|
||||
}
|
||||
}
|
||||
|
||||
return res.status(200).json({ success: true });
|
||||
} catch (error) {
|
||||
logger.error('[adminRoles] removeRoleMember error:', error);
|
||||
return res.status(500).json({ error: 'Failed to remove role member' });
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
listRoles: listRolesHandler,
|
||||
getRole: getRoleHandler,
|
||||
createRole: createRoleHandler,
|
||||
updateRole: updateRoleHandler,
|
||||
updateRolePermissions: updateRolePermissionsHandler,
|
||||
deleteRole: deleteRoleHandler,
|
||||
getRoleMembers: getRoleMembersHandler,
|
||||
addRoleMember: addRoleMemberHandler,
|
||||
removeRoleMember: removeRoleMemberHandler,
|
||||
};
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue