From 5972a21479e6cda4b8aff93adb8e7fc17f31772f Mon Sep 17 00:00:00 2001 From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com> Date: Fri, 27 Mar 2026 12:44:47 -0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=AA=AA=20feat:=20Admin=20Roles=20API=20En?= =?UTF-8?q?dpoints=20(#12400)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 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 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()` - 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 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 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 --- api/server/index.js | 1 + api/server/routes/admin/roles.js | 43 + api/server/routes/index.js | 2 + packages/api/src/admin/groups.ts | 7 +- packages/api/src/admin/index.ts | 2 + packages/api/src/admin/pagination.ts | 17 + packages/api/src/admin/roles.spec.ts | 1484 +++++++++++++++++ packages/api/src/admin/roles.ts | 550 ++++++ packages/data-schemas/package.json | 2 +- packages/data-schemas/src/index.ts | 1 + packages/data-schemas/src/methods/index.ts | 6 +- .../src/methods/role.methods.spec.ts | 387 ++++- packages/data-schemas/src/methods/role.ts | 187 ++- packages/data-schemas/src/schema/role.ts | 1 + packages/data-schemas/src/schema/user.ts | 1 + packages/data-schemas/src/types/admin.ts | 2 +- packages/data-schemas/src/types/role.ts | 3 + 17 files changed, 2673 insertions(+), 23 deletions(-) create mode 100644 api/server/routes/admin/roles.js create mode 100644 packages/api/src/admin/pagination.ts create mode 100644 packages/api/src/admin/roles.spec.ts create mode 100644 packages/api/src/admin/roles.ts diff --git a/api/server/index.js b/api/server/index.js index 4ecc966476..813b453468 100644 --- a/api/server/index.js +++ b/api/server/index.js @@ -145,6 +145,7 @@ const startServer = async () => { app.use('/api/admin', routes.adminAuth); app.use('/api/admin/config', routes.adminConfig); app.use('/api/admin/groups', routes.adminGroups); + app.use('/api/admin/roles', routes.adminRoles); app.use('/api/actions', routes.actions); app.use('/api/keys', routes.keys); app.use('/api/api-keys', routes.apiKeys); diff --git a/api/server/routes/admin/roles.js b/api/server/routes/admin/roles.js new file mode 100644 index 0000000000..2d0f1b1128 --- /dev/null +++ b/api/server/routes/admin/roles.js @@ -0,0 +1,43 @@ +const express = require('express'); +const { createAdminRolesHandlers } = require('@librechat/api'); +const { SystemCapabilities } = require('@librechat/data-schemas'); +const { requireCapability } = require('~/server/middleware/roles/capabilities'); +const { requireJwtAuth } = require('~/server/middleware'); +const db = require('~/models'); + +const router = express.Router(); + +const requireAdminAccess = requireCapability(SystemCapabilities.ACCESS_ADMIN); +const requireReadRoles = requireCapability(SystemCapabilities.READ_ROLES); +const requireManageRoles = requireCapability(SystemCapabilities.MANAGE_ROLES); + +const handlers = createAdminRolesHandlers({ + listRoles: db.listRoles, + countRoles: db.countRoles, + getRoleByName: db.getRoleByName, + createRoleByName: db.createRoleByName, + updateRoleByName: db.updateRoleByName, + updateAccessPermissions: db.updateAccessPermissions, + deleteRoleByName: db.deleteRoleByName, + findUser: db.findUser, + updateUser: db.updateUser, + updateUsersByRole: db.updateUsersByRole, + findUserIdsByRole: db.findUserIdsByRole, + updateUsersRoleByIds: db.updateUsersRoleByIds, + listUsersByRole: db.listUsersByRole, + countUsersByRole: db.countUsersByRole, +}); + +router.use(requireJwtAuth, requireAdminAccess); + +router.get('/', requireReadRoles, handlers.listRoles); +router.post('/', requireManageRoles, handlers.createRole); +router.get('/:name', requireReadRoles, handlers.getRole); +router.patch('/:name', requireManageRoles, handlers.updateRole); +router.delete('/:name', requireManageRoles, handlers.deleteRole); +router.patch('/:name/permissions', requireManageRoles, handlers.updateRolePermissions); +router.get('/:name/members', requireReadRoles, handlers.getRoleMembers); +router.post('/:name/members', requireManageRoles, handlers.addRoleMember); +router.delete('/:name/members/:userId', requireManageRoles, handlers.removeRoleMember); + +module.exports = router; diff --git a/api/server/routes/index.js b/api/server/routes/index.js index f9a088649c..71ae041fc2 100644 --- a/api/server/routes/index.js +++ b/api/server/routes/index.js @@ -4,6 +4,7 @@ const categories = require('./categories'); const adminAuth = require('./admin/auth'); const adminConfig = require('./admin/config'); const adminGroups = require('./admin/groups'); +const adminRoles = require('./admin/roles'); const endpoints = require('./endpoints'); const staticRoute = require('./static'); const messages = require('./messages'); @@ -35,6 +36,7 @@ module.exports = { adminAuth, adminConfig, adminGroups, + adminRoles, keys, apiKeys, user, diff --git a/packages/api/src/admin/groups.ts b/packages/api/src/admin/groups.ts index 58ff4d9782..ab4490e05f 100644 --- a/packages/api/src/admin/groups.ts +++ b/packages/api/src/admin/groups.ts @@ -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; @@ -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 }); diff --git a/packages/api/src/admin/index.ts b/packages/api/src/admin/index.ts index d833c7e2b0..fe60f1d993 100644 --- a/packages/api/src/admin/index.ts +++ b/packages/api/src/admin/index.ts @@ -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'; diff --git a/packages/api/src/admin/pagination.ts b/packages/api/src/admin/pagination.ts new file mode 100644 index 0000000000..69003f0418 --- /dev/null +++ b/packages/api/src/admin/pagination.ts @@ -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), + }; +} diff --git a/packages/api/src/admin/roles.spec.ts b/packages/api/src/admin/roles.spec.ts new file mode 100644 index 0000000000..3f43079bfb --- /dev/null +++ b/packages/api/src/admin/roles.spec.ts @@ -0,0 +1,1484 @@ +import { Types } from 'mongoose'; +import { SystemRoles } from 'librechat-data-provider'; +import type { IRole, IUser } from '@librechat/data-schemas'; +import type { Response } from 'express'; +import type { ServerRequest } from '~/types/http'; +import type { AdminRolesDeps } from './roles'; +import { createAdminRolesHandlers } from './roles'; + +const { RoleConflictError } = jest.requireActual('@librechat/data-schemas'); + +jest.mock('@librechat/data-schemas', () => ({ + ...jest.requireActual('@librechat/data-schemas'), + logger: { error: jest.fn() }, +})); + +const validUserId = new Types.ObjectId().toString(); + +function mockRole(overrides: Partial = {}): IRole { + return { + name: 'editor', + description: 'Can edit content', + permissions: {}, + ...overrides, + } as IRole; +} + +function mockUser(overrides: Partial = {}): IUser { + return { + _id: new Types.ObjectId(validUserId), + name: 'Test User', + email: 'test@example.com', + avatar: 'https://example.com/avatar.png', + role: 'editor', + ...overrides, + } as IUser; +} + +function createReqRes( + overrides: { + params?: Record; + query?: Record; + body?: Record; + } = {}, +) { + const req = { + params: overrides.params ?? {}, + query: overrides.query ?? {}, + body: overrides.body ?? {}, + } as unknown as ServerRequest; + + const json = jest.fn(); + const status = jest.fn().mockReturnValue({ json }); + const res = { status, json } as unknown as Response; + + return { req, res, status, json }; +} + +function createDeps(overrides: Partial = {}): AdminRolesDeps { + return { + listRoles: jest.fn().mockResolvedValue([]), + countRoles: jest.fn().mockResolvedValue(0), + getRoleByName: jest.fn().mockResolvedValue(null), + createRoleByName: jest.fn().mockResolvedValue(mockRole()), + updateRoleByName: jest.fn().mockResolvedValue(mockRole()), + updateAccessPermissions: jest.fn().mockResolvedValue(undefined), + deleteRoleByName: jest.fn().mockResolvedValue(mockRole()), + findUser: jest.fn().mockResolvedValue(null), + updateUser: jest.fn().mockResolvedValue(mockUser()), + updateUsersByRole: jest.fn().mockResolvedValue(undefined), + findUserIdsByRole: jest.fn().mockResolvedValue(['uid-1', 'uid-2']), + updateUsersRoleByIds: jest.fn().mockResolvedValue(undefined), + listUsersByRole: jest.fn().mockResolvedValue([]), + countUsersByRole: jest.fn().mockResolvedValue(0), + ...overrides, + }; +} + +describe('createAdminRolesHandlers', () => { + describe('listRoles', () => { + it('returns paginated roles with 200', async () => { + const roles = [mockRole()]; + const deps = createDeps({ + listRoles: jest.fn().mockResolvedValue(roles), + countRoles: jest.fn().mockResolvedValue(1), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes(); + + await handlers.listRoles(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ roles, total: 1, limit: 50, offset: 0 }); + expect(deps.listRoles).toHaveBeenCalledWith({ limit: 50, offset: 0 }); + }); + + it('passes custom limit and offset from query', async () => { + const deps = createDeps({ + countRoles: jest.fn().mockResolvedValue(100), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + query: { limit: '25', offset: '50' }, + }); + + await handlers.listRoles(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ roles: [], total: 100, limit: 25, offset: 50 }); + expect(deps.listRoles).toHaveBeenCalledWith({ limit: 25, offset: 50 }); + }); + + it('clamps limit to 200', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res } = createReqRes({ query: { limit: '999' } }); + + await handlers.listRoles(req, res); + + expect(deps.listRoles).toHaveBeenCalledWith({ limit: 200, offset: 0 }); + }); + + it('clamps negative offset to 0', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res } = createReqRes({ query: { offset: '-5' } }); + + await handlers.listRoles(req, res); + + expect(deps.listRoles).toHaveBeenCalledWith({ limit: 50, offset: 0 }); + }); + + it('treats non-numeric limit as default', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res } = createReqRes({ query: { limit: 'abc' } }); + + await handlers.listRoles(req, res); + + expect(deps.listRoles).toHaveBeenCalledWith({ limit: 50, offset: 0 }); + }); + + it('clamps limit=0 to 1', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res } = createReqRes({ query: { limit: '0' } }); + + await handlers.listRoles(req, res); + + expect(deps.listRoles).toHaveBeenCalledWith({ limit: 1, offset: 0 }); + }); + + it('truncates float offset to integer', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res } = createReqRes({ query: { offset: '1.7' } }); + + await handlers.listRoles(req, res); + + expect(deps.listRoles).toHaveBeenCalledWith({ limit: 50, offset: 1 }); + }); + + it('returns 500 on error', async () => { + const deps = createDeps({ listRoles: jest.fn().mockRejectedValue(new Error('db down')) }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes(); + + await handlers.listRoles(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to list roles' }); + }); + }); + + describe('getRole', () => { + it('returns role with 200', async () => { + const role = mockRole(); + const deps = createDeps({ getRoleByName: jest.fn().mockResolvedValue(role) }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { name: 'editor' } }); + + await handlers.getRole(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ role }); + }); + + it('returns 404 when role not found', async () => { + const deps = createDeps({ getRoleByName: jest.fn().mockResolvedValue(null) }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { name: 'nonexistent' } }); + + await handlers.getRole(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'Role not found' }); + }); + + it('returns 500 on error', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockRejectedValue(new Error('db down')), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { name: 'editor' } }); + + await handlers.getRole(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to get role' }); + }); + }); + + describe('createRole', () => { + it('creates role and returns 201', async () => { + const role = mockRole(); + const deps = createDeps({ createRoleByName: jest.fn().mockResolvedValue(role) }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + body: { name: 'editor', description: 'Can edit' }, + }); + + await handlers.createRole(req, res); + + expect(status).toHaveBeenCalledWith(201); + expect(json).toHaveBeenCalledWith({ role }); + expect(deps.createRoleByName).toHaveBeenCalledWith({ + name: 'editor', + description: 'Can edit', + permissions: {}, + }); + }); + + it('passes provided permissions to createRoleByName', async () => { + const perms = { chat: { read: true, write: false } } as unknown as IRole['permissions']; + const role = mockRole({ permissions: perms }); + const deps = createDeps({ createRoleByName: jest.fn().mockResolvedValue(role) }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + body: { name: 'editor', permissions: perms }, + }); + + await handlers.createRole(req, res); + + expect(status).toHaveBeenCalledWith(201); + expect(json).toHaveBeenCalledWith({ role }); + expect(deps.createRoleByName).toHaveBeenCalledWith({ + name: 'editor', + permissions: perms, + }); + }); + + it('returns 400 when name is missing', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ body: {} }); + + await handlers.createRole(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'name is required' }); + expect(deps.createRoleByName).not.toHaveBeenCalled(); + }); + + it('returns 400 when name is whitespace-only', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ body: { name: ' ' } }); + + await handlers.createRole(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'name is required' }); + }); + + it('returns 400 when name contains control characters', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ body: { name: 'bad\x00name' } }); + + await handlers.createRole(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'name contains invalid characters' }); + expect(deps.createRoleByName).not.toHaveBeenCalled(); + }); + + it('returns 400 when name is a reserved path segment', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ body: { name: 'members' } }); + + await handlers.createRole(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'name is a reserved path segment' }); + expect(deps.createRoleByName).not.toHaveBeenCalled(); + }); + + it('returns 400 when name exceeds max length', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + body: { name: 'a'.repeat(501) }, + }); + + await handlers.createRole(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'name must not exceed 500 characters' }); + expect(deps.createRoleByName).not.toHaveBeenCalled(); + }); + + it('returns 400 when description exceeds max length', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + body: { name: 'editor', description: 'a'.repeat(2001) }, + }); + + await handlers.createRole(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ + error: 'description must not exceed 2000 characters', + }); + expect(deps.createRoleByName).not.toHaveBeenCalled(); + }); + + it('returns 409 when role already exists', async () => { + const deps = createDeps({ + createRoleByName: jest + .fn() + .mockRejectedValue(new RoleConflictError('Role "editor" already exists')), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ body: { name: 'editor' } }); + + await handlers.createRole(req, res); + + expect(status).toHaveBeenCalledWith(409); + expect(json).toHaveBeenCalledWith({ error: 'Role "editor" already exists' }); + }); + + it('returns 409 when name is reserved system role', async () => { + const deps = createDeps({ + createRoleByName: jest + .fn() + .mockRejectedValue( + new RoleConflictError('Cannot create role with reserved system name: ADMIN'), + ), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ body: { name: 'ADMIN' } }); + + await handlers.createRole(req, res); + + expect(status).toHaveBeenCalledWith(409); + expect(json).toHaveBeenCalledWith({ + error: 'Cannot create role with reserved system name: ADMIN', + }); + }); + + it('returns 500 on unexpected error', async () => { + const deps = createDeps({ + createRoleByName: jest.fn().mockRejectedValue(new Error('db crash')), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ body: { name: 'editor' } }); + + await handlers.createRole(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to create role' }); + }); + + it('does not classify unrelated errors as 409', async () => { + const deps = createDeps({ + createRoleByName: jest + .fn() + .mockRejectedValue(new Error('Disk space reserved for system use')), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status } = createReqRes({ body: { name: 'test' } }); + + await handlers.createRole(req, res); + + expect(status).toHaveBeenCalledWith(500); + }); + + it('returns 400 when description is not a string', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + body: { name: 'editor', description: 123 }, + }); + + await handlers.createRole(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'description must be a string' }); + expect(deps.createRoleByName).not.toHaveBeenCalled(); + }); + + it('returns 400 when permissions is an array', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + body: { name: 'editor', permissions: [1, 2, 3] }, + }); + + await handlers.createRole(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'permissions must be an object' }); + expect(deps.createRoleByName).not.toHaveBeenCalled(); + }); + }); + + describe('updateRole', () => { + it('updates role and returns 200', async () => { + const role = mockRole({ name: 'senior-editor' }); + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValueOnce(mockRole()).mockResolvedValueOnce(null), + updateRoleByName: jest.fn().mockResolvedValue(role), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { name: 'senior-editor' }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ role }); + expect(deps.updateRoleByName).toHaveBeenCalledWith('editor', { name: 'senior-editor' }); + }); + + it('trims name before storage', async () => { + const role = mockRole({ name: 'trimmed' }); + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValueOnce(mockRole()).mockResolvedValueOnce(null), + updateRoleByName: jest.fn().mockResolvedValue(role), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res } = createReqRes({ + params: { name: 'editor' }, + body: { name: ' trimmed ' }, + }); + + await handlers.updateRole(req, res); + + expect(deps.updateRoleByName).toHaveBeenCalledWith('editor', { name: 'trimmed' }); + }); + + it('migrates users before renaming role', async () => { + const role = mockRole({ name: 'new-name' }); + const callOrder: string[] = []; + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValueOnce(mockRole()).mockResolvedValueOnce(null), + findUserIdsByRole: jest.fn().mockImplementation(() => { + callOrder.push('findUserIdsByRole'); + return Promise.resolve(['uid-1']); + }), + updateUsersByRole: jest.fn().mockImplementation(() => { + callOrder.push('updateUsersByRole'); + return Promise.resolve(); + }), + updateRoleByName: jest.fn().mockImplementation(() => { + callOrder.push('updateRoleByName'); + return Promise.resolve(role); + }), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status } = createReqRes({ + params: { name: 'editor' }, + body: { name: 'new-name' }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(deps.findUserIdsByRole).toHaveBeenCalledWith('editor'); + expect(deps.updateUsersByRole).toHaveBeenCalledWith('editor', 'new-name'); + expect(callOrder).toEqual(['findUserIdsByRole', 'updateUsersByRole', 'updateRoleByName']); + }); + + it('does not rename role when user migration fails', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValueOnce(mockRole()).mockResolvedValueOnce(null), + updateUsersByRole: jest.fn().mockRejectedValue(new Error('migration failed')), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status } = createReqRes({ + params: { name: 'editor' }, + body: { name: 'new-name' }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(deps.updateRoleByName).not.toHaveBeenCalled(); + }); + + it('does not migrate users when name unchanged', async () => { + const role = mockRole({ description: 'updated' }); + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + updateRoleByName: jest.fn().mockResolvedValue(role), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res } = createReqRes({ + params: { name: 'editor' }, + body: { description: 'updated' }, + }); + + await handlers.updateRole(req, res); + + expect(deps.updateUsersByRole).not.toHaveBeenCalled(); + }); + + it('renames and updates description in a single request', async () => { + const role = mockRole({ name: 'senior-editor', description: 'Updated desc' }); + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValueOnce(mockRole()).mockResolvedValueOnce(null), + updateRoleByName: jest.fn().mockResolvedValue(role), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { name: 'senior-editor', description: 'Updated desc' }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ role }); + expect(deps.updateUsersByRole).toHaveBeenCalledWith('editor', 'senior-editor'); + expect(deps.updateRoleByName).toHaveBeenCalledWith('editor', { + name: 'senior-editor', + description: 'Updated desc', + }); + }); + + it('returns 403 when renaming a system role', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: SystemRoles.ADMIN }, + body: { name: 'custom-admin' }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(403); + expect(json).toHaveBeenCalledWith({ error: 'Cannot rename system role' }); + expect(deps.getRoleByName).not.toHaveBeenCalled(); + }); + + it('returns 403 when renaming to a system role name', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { name: SystemRoles.ADMIN }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(403); + expect(json).toHaveBeenCalledWith({ error: 'Cannot use a reserved system role name' }); + }); + + it('returns 409 when target name already exists', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { name: 'viewer' }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(409); + expect(json).toHaveBeenCalledWith({ error: 'Role "viewer" already exists' }); + }); + + it('returns 400 when name is empty string', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { name: '' }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'name must be a non-empty string' }); + expect(deps.getRoleByName).not.toHaveBeenCalled(); + }); + + it('returns 400 when name is whitespace-only', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { name: ' ' }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'name must be a non-empty string' }); + }); + + it('returns 400 when name exceeds max length', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { name: 'a'.repeat(501) }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'name must not exceed 500 characters' }); + expect(deps.getRoleByName).not.toHaveBeenCalled(); + }); + + it('returns 404 when role not found', async () => { + const deps = createDeps({ getRoleByName: jest.fn().mockResolvedValue(null) }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'nonexistent' }, + body: { description: 'updated' }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'Role not found' }); + }); + + it('returns 404 when updateRoleByName returns null', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + updateRoleByName: jest.fn().mockResolvedValue(null), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { description: 'updated' }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'Role not found' }); + }); + + it('rolls back user migration when rename fails', async () => { + const ids = ['uid-1', 'uid-2']; + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValueOnce(mockRole()).mockResolvedValueOnce(null), + findUserIdsByRole: jest.fn().mockResolvedValue(ids), + updateRoleByName: jest.fn().mockResolvedValue(null), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { name: 'new-name' }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'Role not found' }); + expect(deps.updateUsersByRole).toHaveBeenCalledTimes(1); + expect(deps.updateUsersByRole).toHaveBeenCalledWith('editor', 'new-name'); + expect(deps.updateUsersRoleByIds).toHaveBeenCalledWith(ids, 'editor'); + }); + + it('rolls back user migration when rename throws', async () => { + const ids = ['uid-1', 'uid-2']; + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValueOnce(mockRole()).mockResolvedValueOnce(null), + findUserIdsByRole: jest.fn().mockResolvedValue(ids), + updateRoleByName: jest.fn().mockRejectedValue(new Error('db crash')), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status } = createReqRes({ + params: { name: 'editor' }, + body: { name: 'new-name' }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(deps.updateUsersByRole).toHaveBeenCalledTimes(1); + expect(deps.updateUsersByRole).toHaveBeenCalledWith('editor', 'new-name'); + expect(deps.updateUsersRoleByIds).toHaveBeenCalledWith(ids, 'editor'); + }); + + it('logs rollback failure and still returns 500', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValueOnce(mockRole()).mockResolvedValueOnce(null), + findUserIdsByRole: jest.fn().mockResolvedValue(['uid-1']), + updateUsersRoleByIds: jest.fn().mockRejectedValue(new Error('rollback failed')), + updateRoleByName: jest.fn().mockRejectedValue(new Error('rename failed')), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status } = createReqRes({ + params: { name: 'editor' }, + body: { name: 'new-name' }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(deps.updateUsersByRole).toHaveBeenCalledTimes(1); + expect(deps.updateUsersRoleByIds).toHaveBeenCalledTimes(1); + }); + + it('returns 400 when description exceeds max length', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { description: 'a'.repeat(2001) }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ + error: 'description must not exceed 2000 characters', + }); + expect(deps.getRoleByName).not.toHaveBeenCalled(); + }); + + it('returns 500 on unexpected error', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + updateRoleByName: jest.fn().mockRejectedValue(new Error('db error')), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { description: 'updated' }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to update role' }); + }); + + it('does not roll back when error occurs before user migration', async () => { + const deps = createDeps({ + getRoleByName: jest + .fn() + .mockResolvedValueOnce(mockRole()) + .mockRejectedValueOnce(new Error('db crash')), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status } = createReqRes({ + params: { name: 'editor' }, + body: { name: 'new-name' }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(deps.updateUsersByRole).not.toHaveBeenCalled(); + }); + + it('does not migrate users when findUserIdsByRole throws', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValueOnce(mockRole()).mockResolvedValueOnce(null), + findUserIdsByRole: jest.fn().mockRejectedValue(new Error('db crash')), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status } = createReqRes({ + params: { name: 'editor' }, + body: { name: 'new-name' }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(deps.updateUsersByRole).not.toHaveBeenCalled(); + expect(deps.updateUsersRoleByIds).not.toHaveBeenCalled(); + }); + + it('returns existing role early when update body has no changes', async () => { + const role = mockRole(); + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(role), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: {}, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ role }); + expect(deps.updateRoleByName).not.toHaveBeenCalled(); + }); + + it('rejects invalid description before making DB calls', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { description: 123 }, + }); + + await handlers.updateRole(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'description must be a string' }); + expect(deps.getRoleByName).not.toHaveBeenCalled(); + }); + }); + + describe('updateRolePermissions', () => { + it('updates permissions and returns 200 with updated role', async () => { + const role = mockRole(); + const updatedRole = mockRole({ + permissions: { chat: { read: true, write: true } } as IRole['permissions'], + }); + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValueOnce(role).mockResolvedValueOnce(updatedRole), + }); + const handlers = createAdminRolesHandlers(deps); + const perms = { chat: { read: true, write: true } }; + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { permissions: perms }, + }); + + await handlers.updateRolePermissions(req, res); + + expect(deps.updateAccessPermissions).toHaveBeenCalledWith('editor', perms, role); + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ role: updatedRole }); + }); + + it('returns 400 when permissions is missing', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: {}, + }); + + await handlers.updateRolePermissions(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'permissions object is required' }); + }); + + it('returns 400 when permissions is an array', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { permissions: [1, 2, 3] }, + }); + + await handlers.updateRolePermissions(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'permissions object is required' }); + }); + + it('returns 404 when role not found', async () => { + const deps = createDeps({ getRoleByName: jest.fn().mockResolvedValue(null) }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'nonexistent' }, + body: { permissions: { chat: { read: true } } }, + }); + + await handlers.updateRolePermissions(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'Role not found' }); + }); + + it('returns 500 on error', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + updateAccessPermissions: jest.fn().mockRejectedValue(new Error('db down')), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { permissions: { chat: { read: true } } }, + }); + + await handlers.updateRolePermissions(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to update role permissions' }); + }); + }); + + describe('deleteRole', () => { + it('deletes role and returns 200', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { name: 'editor' } }); + + await handlers.deleteRole(req, res); + + expect(deps.deleteRoleByName).toHaveBeenCalledWith('editor'); + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ success: true }); + }); + + it('returns 403 for system role', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { name: SystemRoles.ADMIN } }); + + await handlers.deleteRole(req, res); + + expect(status).toHaveBeenCalledWith(403); + expect(json).toHaveBeenCalledWith({ error: 'Cannot delete system role' }); + expect(deps.deleteRoleByName).not.toHaveBeenCalled(); + }); + + it('returns 404 when role not found', async () => { + const deps = createDeps({ deleteRoleByName: jest.fn().mockResolvedValue(null) }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { name: 'nonexistent' } }); + + await handlers.deleteRole(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'Role not found' }); + }); + + it('returns 500 on error', async () => { + const deps = createDeps({ + deleteRoleByName: jest.fn().mockRejectedValue(new Error('db down')), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { name: 'editor' } }); + + await handlers.deleteRole(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to delete role' }); + }); + }); + + describe('getRoleMembers', () => { + it('returns paginated members with 200', async () => { + const user = mockUser(); + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + listUsersByRole: jest.fn().mockResolvedValue([user]), + countUsersByRole: jest.fn().mockResolvedValue(1), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { name: 'editor' } }); + + await handlers.getRoleMembers(req, res); + + expect(deps.listUsersByRole).toHaveBeenCalledWith('editor', { limit: 50, offset: 0 }); + expect(deps.countUsersByRole).toHaveBeenCalledWith('editor'); + expect(status).toHaveBeenCalledWith(200); + const response = json.mock.calls[0][0]; + expect(response.members).toHaveLength(1); + expect(response.members[0]).toEqual({ + userId: validUserId, + name: 'Test User', + email: 'test@example.com', + avatarUrl: 'https://example.com/avatar.png', + }); + expect(response.total).toBe(1); + expect(response.limit).toBe(50); + expect(response.offset).toBe(0); + }); + + it('passes pagination parameters from query', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + countUsersByRole: jest.fn().mockResolvedValue(0), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res } = createReqRes({ + params: { name: 'editor' }, + query: { limit: '10', offset: '20' }, + }); + + await handlers.getRoleMembers(req, res); + + expect(deps.listUsersByRole).toHaveBeenCalledWith('editor', { limit: 10, offset: 20 }); + }); + + it('clamps limit to 200', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + countUsersByRole: jest.fn().mockResolvedValue(0), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res } = createReqRes({ + params: { name: 'editor' }, + query: { limit: '999' }, + }); + + await handlers.getRoleMembers(req, res); + + expect(deps.listUsersByRole).toHaveBeenCalledWith('editor', { limit: 200, offset: 0 }); + }); + + it('does not include joinedAt in response', async () => { + const user = mockUser(); + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + listUsersByRole: jest.fn().mockResolvedValue([user]), + countUsersByRole: jest.fn().mockResolvedValue(1), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, json } = createReqRes({ params: { name: 'editor' } }); + + await handlers.getRoleMembers(req, res); + + const member = json.mock.calls[0][0].members[0]; + expect(member).not.toHaveProperty('joinedAt'); + }); + + it('returns empty array when no members', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + countUsersByRole: jest.fn().mockResolvedValue(0), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { name: 'editor' } }); + + await handlers.getRoleMembers(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ members: [], total: 0, limit: 50, offset: 0 }); + }); + + it('returns 404 when role not found', async () => { + const deps = createDeps({ getRoleByName: jest.fn().mockResolvedValue(null) }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { name: 'nonexistent' } }); + + await handlers.getRoleMembers(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'Role not found' }); + }); + + it('returns 500 on error', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + listUsersByRole: jest.fn().mockRejectedValue(new Error('db down')), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { name: 'editor' } }); + + await handlers.getRoleMembers(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to get role members' }); + }); + }); + + describe('addRoleMember', () => { + it('adds member and returns 200', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + findUser: jest.fn().mockResolvedValue(mockUser({ role: 'viewer' })), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { userId: validUserId }, + }); + + await handlers.addRoleMember(req, res); + + expect(deps.updateUser).toHaveBeenCalledWith(validUserId, { role: 'editor' }); + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ success: true }); + }); + + it('skips DB write when user already has the target role', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + findUser: jest.fn().mockResolvedValue(mockUser({ role: 'editor' })), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { userId: validUserId }, + }); + + await handlers.addRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ success: true }); + expect(deps.updateUser).not.toHaveBeenCalled(); + }); + + it('returns 400 when userId is missing', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: {}, + }); + + await handlers.addRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'userId is required' }); + }); + + it('returns 400 for invalid ObjectId', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { userId: 'not-valid' }, + }); + + await handlers.addRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Invalid user ID format' }); + }); + + it('returns 404 when role not found', async () => { + const deps = createDeps({ getRoleByName: jest.fn().mockResolvedValue(null) }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'nonexistent' }, + body: { userId: validUserId }, + }); + + await handlers.addRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'Role not found' }); + }); + + it('returns 404 when user not found', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + findUser: jest.fn().mockResolvedValue(null), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { userId: validUserId }, + }); + + await handlers.addRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'User not found' }); + }); + + it('returns 400 when reassigning the last admin to another role', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole({ name: 'editor' })), + findUser: jest.fn().mockResolvedValue(mockUser({ role: SystemRoles.ADMIN })), + countUsersByRole: jest.fn().mockResolvedValue(1), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { userId: validUserId }, + }); + + await handlers.addRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Cannot remove the last admin user' }); + expect(deps.updateUser).not.toHaveBeenCalled(); + }); + + it('allows reassigning an admin when multiple admins exist', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole({ name: 'editor' })), + findUser: jest.fn().mockResolvedValue(mockUser({ role: SystemRoles.ADMIN })), + countUsersByRole: jest.fn().mockResolvedValue(3), + updateUser: jest.fn().mockResolvedValue(mockUser({ role: 'editor' })), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status } = createReqRes({ + params: { name: 'editor' }, + body: { userId: validUserId }, + }); + + await handlers.addRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(deps.updateUser).toHaveBeenCalledWith(validUserId, { role: 'editor' }); + }); + + it('rolls back assignment when post-write admin count is zero', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole({ name: 'editor' })), + findUser: jest.fn().mockResolvedValue(mockUser({ role: SystemRoles.ADMIN })), + countUsersByRole: jest.fn().mockResolvedValueOnce(2).mockResolvedValueOnce(0), + updateUser: jest.fn().mockResolvedValue(mockUser()), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { userId: validUserId }, + }); + + await handlers.addRoleMember(req, res); + + expect(deps.updateUser).toHaveBeenCalledTimes(2); + expect(deps.updateUser).toHaveBeenLastCalledWith(validUserId, { role: SystemRoles.ADMIN }); + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Cannot remove the last admin user' }); + }); + + it('returns 403 when adding to a non-ADMIN system role', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: SystemRoles.USER }, + body: { userId: validUserId }, + }); + + await handlers.addRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(403); + expect(json).toHaveBeenCalledWith({ + error: 'Cannot directly assign members to a system role', + }); + expect(deps.updateUser).not.toHaveBeenCalled(); + }); + + it('allows promoting a non-admin user to the ADMIN role', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole({ name: SystemRoles.ADMIN })), + findUser: jest.fn().mockResolvedValue(mockUser({ role: 'editor' })), + updateUser: jest.fn().mockResolvedValue(mockUser({ role: SystemRoles.ADMIN })), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: SystemRoles.ADMIN }, + body: { userId: validUserId }, + }); + + await handlers.addRoleMember(req, res); + + expect(deps.updateUser).toHaveBeenCalledWith(validUserId, { role: SystemRoles.ADMIN }); + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ success: true }); + }); + + it('returns 500 on unexpected error', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + findUser: jest.fn().mockResolvedValue(mockUser({ role: 'viewer' })), + updateUser: jest.fn().mockRejectedValue(new Error('timeout')), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor' }, + body: { userId: validUserId }, + }); + + await handlers.addRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to add role member' }); + }); + }); + + describe('removeRoleMember', () => { + it('removes member and returns 200', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + findUser: jest.fn().mockResolvedValue(mockUser({ role: 'editor' })), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor', userId: validUserId }, + }); + + await handlers.removeRoleMember(req, res); + + expect(deps.updateUser).toHaveBeenCalledWith(validUserId, { role: SystemRoles.USER }); + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ success: true }); + }); + + it('returns 403 when removing from a non-ADMIN system role', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: SystemRoles.USER, userId: validUserId }, + }); + + await handlers.removeRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(403); + expect(json).toHaveBeenCalledWith({ error: 'Cannot remove members from a system role' }); + expect(deps.getRoleByName).not.toHaveBeenCalled(); + }); + + it('returns 400 for invalid ObjectId', async () => { + const deps = createDeps(); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor', userId: 'bad' }, + }); + + await handlers.removeRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Invalid user ID format' }); + expect(deps.findUser).not.toHaveBeenCalled(); + }); + + it('returns 404 when role not found', async () => { + const deps = createDeps({ getRoleByName: jest.fn().mockResolvedValue(null) }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'nonexistent', userId: validUserId }, + }); + + await handlers.removeRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'Role not found' }); + expect(deps.findUser).not.toHaveBeenCalled(); + }); + + it('returns 404 when user not found', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + findUser: jest.fn().mockResolvedValue(null), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor', userId: validUserId }, + }); + + await handlers.removeRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'User not found' }); + }); + + it('returns 400 when user is not a member of the role', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + findUser: jest.fn().mockResolvedValue(mockUser({ role: 'other-role' })), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor', userId: validUserId }, + }); + + await handlers.removeRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'User is not a member of this role' }); + expect(deps.updateUser).not.toHaveBeenCalled(); + }); + + it('returns 400 when removing the last admin user', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole({ name: SystemRoles.ADMIN })), + findUser: jest.fn().mockResolvedValue(mockUser({ role: SystemRoles.ADMIN })), + countUsersByRole: jest.fn().mockResolvedValue(1), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: SystemRoles.ADMIN, userId: validUserId }, + }); + + await handlers.removeRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Cannot remove the last admin user' }); + expect(deps.updateUser).not.toHaveBeenCalled(); + }); + + it('allows removing an admin when multiple admins exist', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole({ name: SystemRoles.ADMIN })), + findUser: jest.fn().mockResolvedValue(mockUser({ role: SystemRoles.ADMIN })), + countUsersByRole: jest.fn().mockResolvedValue(3), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: SystemRoles.ADMIN, userId: validUserId }, + }); + + await handlers.removeRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ success: true }); + expect(deps.updateUser).toHaveBeenCalledWith(validUserId, { role: SystemRoles.USER }); + }); + + it('rolls back removal when post-write check finds zero admins', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole({ name: SystemRoles.ADMIN })), + findUser: jest.fn().mockResolvedValue(mockUser({ role: SystemRoles.ADMIN })), + countUsersByRole: jest.fn().mockResolvedValueOnce(2).mockResolvedValueOnce(0), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: SystemRoles.ADMIN, userId: validUserId }, + }); + + await handlers.removeRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Cannot remove the last admin user' }); + expect(deps.updateUser).toHaveBeenCalledTimes(2); + expect(deps.updateUser).toHaveBeenNthCalledWith(1, validUserId, { + role: SystemRoles.USER, + }); + expect(deps.updateUser).toHaveBeenNthCalledWith(2, validUserId, { + role: SystemRoles.ADMIN, + }); + }); + + it('returns 400 even when rollback updateUser throws', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole({ name: SystemRoles.ADMIN })), + findUser: jest.fn().mockResolvedValue(mockUser({ role: SystemRoles.ADMIN })), + countUsersByRole: jest.fn().mockResolvedValueOnce(2).mockResolvedValueOnce(0), + updateUser: jest + .fn() + .mockResolvedValueOnce(mockUser({ role: SystemRoles.USER })) + .mockRejectedValueOnce(new Error('rollback failed')), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: SystemRoles.ADMIN, userId: validUserId }, + }); + + await handlers.removeRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Cannot remove the last admin user' }); + expect(deps.updateUser).toHaveBeenCalledTimes(2); + }); + + it('returns 500 on unexpected error', async () => { + const deps = createDeps({ + getRoleByName: jest.fn().mockResolvedValue(mockRole()), + findUser: jest.fn().mockResolvedValue(mockUser({ role: 'editor' })), + updateUser: jest.fn().mockRejectedValue(new Error('timeout')), + }); + const handlers = createAdminRolesHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { name: 'editor', userId: validUserId }, + }); + + await handlers.removeRoleMember(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to remove role member' }); + }); + }); +}); diff --git a/packages/api/src/admin/roles.ts b/packages/api/src/admin/roles.ts new file mode 100644 index 0000000000..b8c87c23ea --- /dev/null +++ b/packages/api/src/admin/roles.ts @@ -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(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; + countRoles: () => Promise; + getRoleByName: (name: string, fields?: string | string[] | null) => Promise; + createRoleByName: (roleData: Partial) => Promise; + updateRoleByName: (name: string, updates: Partial) => Promise; + updateAccessPermissions: ( + name: string, + perms: Record>, + roleData?: IRole, + ) => Promise; + deleteRoleByName: (name: string) => Promise; + findUser: ( + criteria: FilterQuery, + fields?: string | string[] | null, + ) => Promise; + updateUser: (userId: string, data: Partial) => Promise; + updateUsersByRole: (oldRole: string, newRole: string) => Promise; + findUserIdsByRole: (roleName: string) => Promise; + updateUsersRoleByIds: (userIds: string[], newRole: string) => Promise; + listUsersByRole: ( + roleName: string, + options?: { limit?: number; offset?: number }, + ) => Promise; + countUsersByRole: (roleName: string) => Promise; +} + +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 = { + 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 { + 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, + ): Promise { + const migratedIds = await findUserIdsByRole(currentName); + await updateUsersByRole(currentName, newName); + try { + const updates: Partial = { 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 = {}; + 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>; + }; + + 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, + }; +} diff --git a/packages/data-schemas/package.json b/packages/data-schemas/package.json index 0124552002..145b8925d1 100644 --- a/packages/data-schemas/package.json +++ b/packages/data-schemas/package.json @@ -1,6 +1,6 @@ { "name": "@librechat/data-schemas", - "version": "0.0.46", + "version": "0.0.47", "description": "Mongoose schemas and models for LibreChat", "type": "module", "main": "dist/index.cjs", diff --git a/packages/data-schemas/src/index.ts b/packages/data-schemas/src/index.ts index cd683c937c..d673db1f5c 100644 --- a/packages/data-schemas/src/index.ts +++ b/packages/data-schemas/src/index.ts @@ -7,6 +7,7 @@ export * from './utils'; export { createModels } from './models'; export { createMethods, + RoleConflictError, DEFAULT_REFRESH_TOKEN_EXPIRY, DEFAULT_SESSION_EXPIRY, tokenValues, diff --git a/packages/data-schemas/src/methods/index.ts b/packages/data-schemas/src/methods/index.ts index 4202cac0eb..830d88ff4c 100644 --- a/packages/data-schemas/src/methods/index.ts +++ b/packages/data-schemas/src/methods/index.ts @@ -1,9 +1,8 @@ import { createSessionMethods, DEFAULT_REFRESH_TOKEN_EXPIRY, type SessionMethods } from './session'; import { createTokenMethods, type TokenMethods } from './token'; -import { createRoleMethods, type RoleMethods, type RoleDeps } from './role'; +import { createRoleMethods, RoleConflictError } from './role'; +import type { RoleMethods, RoleDeps } from './role'; import { createUserMethods, DEFAULT_SESSION_EXPIRY, type UserMethods } from './user'; - -export { DEFAULT_REFRESH_TOKEN_EXPIRY, DEFAULT_SESSION_EXPIRY }; import { createKeyMethods, type KeyMethods } from './key'; import { createFileMethods, type FileMethods } from './file'; /* Memories */ @@ -51,6 +50,7 @@ import { createAgentMethods, type AgentMethods, type AgentDeps } from './agent'; /* Config */ import { createConfigMethods, type ConfigMethods } from './config'; +export { RoleConflictError, DEFAULT_REFRESH_TOKEN_EXPIRY, DEFAULT_SESSION_EXPIRY }; export { tokenValues, cacheTokenValues, premiumTokenValues, defaultRate }; export type AllMethods = UserMethods & diff --git a/packages/data-schemas/src/methods/role.methods.spec.ts b/packages/data-schemas/src/methods/role.methods.spec.ts index 78d7f98ea1..f8a66bef5d 100644 --- a/packages/data-schemas/src/methods/role.methods.spec.ts +++ b/packages/data-schemas/src/methods/role.methods.spec.ts @@ -1,10 +1,17 @@ import mongoose from 'mongoose'; import { MongoMemoryServer } from 'mongodb-memory-server'; import { SystemRoles, Permissions, roleDefaults, PermissionTypes } from 'librechat-data-provider'; -import type { IRole, RolePermissions } from '..'; +import type { IRole, IUser, RolePermissions } from '..'; import { createRoleMethods } from './role'; import { createModels } from '../models'; +jest.mock('~/config/winston', () => ({ + error: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + debug: jest.fn(), +})); + const mockCache = { get: jest.fn(), set: jest.fn(), @@ -14,9 +21,18 @@ const mockCache = { const mockGetCache = jest.fn().mockReturnValue(mockCache); let Role: mongoose.Model; +let User: mongoose.Model; let getRoleByName: ReturnType['getRoleByName']; let updateAccessPermissions: ReturnType['updateAccessPermissions']; let initializeRoles: ReturnType['initializeRoles']; +let createRoleByName: ReturnType['createRoleByName']; +let deleteRoleByName: ReturnType['deleteRoleByName']; +let updateUsersByRole: ReturnType['updateUsersByRole']; +let listUsersByRole: ReturnType['listUsersByRole']; +let countUsersByRole: ReturnType['countUsersByRole']; +let updateRoleByName: ReturnType['updateRoleByName']; +let listRoles: ReturnType['listRoles']; +let countRoles: ReturnType['countRoles']; let mongoServer: MongoMemoryServer; beforeAll(async () => { @@ -25,10 +41,19 @@ beforeAll(async () => { await mongoose.connect(mongoUri); createModels(mongoose); Role = mongoose.models.Role; + User = mongoose.models.User as mongoose.Model; const methods = createRoleMethods(mongoose, { getCache: mockGetCache }); getRoleByName = methods.getRoleByName; updateAccessPermissions = methods.updateAccessPermissions; initializeRoles = methods.initializeRoles; + createRoleByName = methods.createRoleByName; + deleteRoleByName = methods.deleteRoleByName; + updateRoleByName = methods.updateRoleByName; + updateUsersByRole = methods.updateUsersByRole; + listUsersByRole = methods.listUsersByRole; + countUsersByRole = methods.countUsersByRole; + listRoles = methods.listRoles; + countRoles = methods.countRoles; }); afterAll(async () => { @@ -38,6 +63,7 @@ afterAll(async () => { beforeEach(async () => { await Role.deleteMany({}); + await User.deleteMany({}); mockGetCache.mockClear(); mockCache.get.mockClear(); mockCache.set.mockClear(); @@ -515,3 +541,362 @@ describe('initializeRoles', () => { expect(userRole.permissions[PermissionTypes.MULTI_CONVO]?.USE).toBeDefined(); }); }); + +describe('createRoleByName', () => { + it('creates a custom role and caches it', async () => { + const role = await createRoleByName({ name: 'editor', description: 'Can edit' }); + + expect(role.name).toBe('editor'); + expect(role.description).toBe('Can edit'); + expect(mockCache.set).toHaveBeenCalledWith( + 'editor', + expect.objectContaining({ name: 'editor' }), + ); + + const persisted = await Role.findOne({ name: 'editor' }).lean(); + expect(persisted).toBeTruthy(); + }); + + it('trims whitespace from role name', async () => { + const role = await createRoleByName({ name: ' editor ' }); + + expect(role.name).toBe('editor'); + }); + + it('throws when name is empty', async () => { + await expect(createRoleByName({ name: '' })).rejects.toThrow('Role name is required'); + }); + + it('throws when name is whitespace-only', async () => { + await expect(createRoleByName({ name: ' ' })).rejects.toThrow('Role name is required'); + }); + + it('throws when name is undefined', async () => { + await expect(createRoleByName({})).rejects.toThrow('Role name is required'); + }); + + it('throws for reserved system role names', async () => { + await expect(createRoleByName({ name: SystemRoles.ADMIN })).rejects.toThrow( + /reserved system name/, + ); + await expect(createRoleByName({ name: SystemRoles.USER })).rejects.toThrow( + /reserved system name/, + ); + }); + + it('throws when role already exists', async () => { + await createRoleByName({ name: 'editor' }); + + await expect(createRoleByName({ name: 'editor' })).rejects.toThrow(/already exists/); + }); +}); + +describe('deleteRoleByName', () => { + it('deletes a custom role and reassigns users to USER', async () => { + await createRoleByName({ name: 'editor' }); + await User.create([ + { name: 'Alice', email: 'alice@test.com', role: 'editor', username: 'alice' }, + { name: 'Bob', email: 'bob@test.com', role: 'editor', username: 'bob' }, + { name: 'Carol', email: 'carol@test.com', role: SystemRoles.USER, username: 'carol' }, + ]); + + const deleted = await deleteRoleByName('editor'); + + expect(deleted).toBeTruthy(); + expect(deleted!.name).toBe('editor'); + + const alice = await User.findOne({ email: 'alice@test.com' }).lean(); + const bob = await User.findOne({ email: 'bob@test.com' }).lean(); + const carol = await User.findOne({ email: 'carol@test.com' }).lean(); + expect(alice!.role).toBe(SystemRoles.USER); + expect(bob!.role).toBe(SystemRoles.USER); + expect(carol!.role).toBe(SystemRoles.USER); + }); + + it('returns null when role does not exist', async () => { + const result = await deleteRoleByName('nonexistent'); + expect(result).toBeNull(); + }); + + it('throws for system roles', async () => { + await expect(deleteRoleByName(SystemRoles.ADMIN)).rejects.toThrow(/Cannot delete system role/); + await expect(deleteRoleByName(SystemRoles.USER)).rejects.toThrow(/Cannot delete system role/); + }); + + it('sets cache entry to null after deletion', async () => { + await createRoleByName({ name: 'editor' }); + mockCache.set.mockClear(); + + await deleteRoleByName('editor'); + + expect(mockCache.set).toHaveBeenCalledWith('editor', null); + }); + + it('returns null and invalidates cache when role does not exist', async () => { + mockCache.set.mockClear(); + + const result = await deleteRoleByName('nonexistent'); + + expect(result).toBeNull(); + expect(mockCache.set).toHaveBeenCalledWith('nonexistent', null); + }); +}); + +describe('updateRoleByName - cache on rename', () => { + it('invalidates old key and populates new key on rename', async () => { + await createRoleByName({ name: 'editor', description: 'Can edit' }); + mockCache.set.mockClear(); + + const updated = await updateRoleByName('editor', { name: 'senior-editor' }); + + expect(updated.name).toBe('senior-editor'); + expect(mockCache.set).toHaveBeenCalledWith('editor', null); + expect(mockCache.set).toHaveBeenCalledWith( + 'senior-editor', + expect.objectContaining({ name: 'senior-editor' }), + ); + }); + + it('writes same key when name unchanged', async () => { + await createRoleByName({ name: 'editor' }); + mockCache.set.mockClear(); + + await updateRoleByName('editor', { description: 'Updated desc' }); + + expect(mockCache.set).toHaveBeenCalledWith( + 'editor', + expect.objectContaining({ name: 'editor', description: 'Updated desc' }), + ); + expect(mockCache.set).toHaveBeenCalledTimes(1); + }); +}); + +describe('listUsersByRole', () => { + it('returns users matching the role', async () => { + await User.create([ + { name: 'Alice', email: 'alice@test.com', role: 'editor', username: 'alice' }, + { name: 'Bob', email: 'bob@test.com', role: 'editor', username: 'bob' }, + { name: 'Carol', email: 'carol@test.com', role: SystemRoles.USER, username: 'carol' }, + ]); + + const users = await listUsersByRole('editor'); + + expect(users).toHaveLength(2); + const names = users.map((u) => u.name).sort(); + expect(names).toEqual(['Alice', 'Bob']); + }); + + it('returns empty array when no users have the role', async () => { + const users = await listUsersByRole('nonexistent'); + expect(users).toEqual([]); + }); + + it('respects limit and offset for pagination', async () => { + await User.create([ + { name: 'Alice', email: 'a@test.com', role: 'editor', username: 'a' }, + { name: 'Bob', email: 'b@test.com', role: 'editor', username: 'b' }, + { name: 'Carol', email: 'c@test.com', role: 'editor', username: 'c' }, + { name: 'Dave', email: 'd@test.com', role: 'editor', username: 'd' }, + { name: 'Eve', email: 'e@test.com', role: 'editor', username: 'e' }, + ]); + + const page1 = await listUsersByRole('editor', { limit: 2, offset: 0 }); + const page2 = await listUsersByRole('editor', { limit: 2, offset: 2 }); + const page3 = await listUsersByRole('editor', { limit: 2, offset: 4 }); + + expect(page1).toHaveLength(2); + expect(page2).toHaveLength(2); + expect(page3).toHaveLength(1); + + const allIds = [...page1, ...page2, ...page3].map((u) => u._id!.toString()); + expect(new Set(allIds).size).toBe(5); + }); + + it('selects only expected fields', async () => { + await User.create({ + name: 'Alice', + email: 'alice@test.com', + role: 'editor', + username: 'alice', + password: 'secret123', + }); + + const users = await listUsersByRole('editor'); + + expect(users).toHaveLength(1); + expect(users[0].name).toBe('Alice'); + expect(users[0].email).toBe('alice@test.com'); + expect(users[0]._id).toBeDefined(); + expect('password' in users[0]).toBe(false); + expect('username' in users[0]).toBe(false); + }); +}); + +describe('updateUsersByRole', () => { + it('migrates all users from one role to another', async () => { + await User.create([ + { name: 'Alice', email: 'alice@test.com', role: 'editor', username: 'alice' }, + { name: 'Bob', email: 'bob@test.com', role: 'editor', username: 'bob' }, + { name: 'Carol', email: 'carol@test.com', role: SystemRoles.USER, username: 'carol' }, + ]); + + await updateUsersByRole('editor', 'senior-editor'); + + const alice = await User.findOne({ email: 'alice@test.com' }).lean(); + const bob = await User.findOne({ email: 'bob@test.com' }).lean(); + const carol = await User.findOne({ email: 'carol@test.com' }).lean(); + expect(alice!.role).toBe('senior-editor'); + expect(bob!.role).toBe('senior-editor'); + expect(carol!.role).toBe(SystemRoles.USER); + }); + + it('is a no-op when no users have the source role', async () => { + await User.create({ + name: 'Alice', + email: 'alice@test.com', + role: SystemRoles.USER, + username: 'alice', + }); + + await updateUsersByRole('nonexistent', 'new-role'); + + const alice = await User.findOne({ email: 'alice@test.com' }).lean(); + expect(alice!.role).toBe(SystemRoles.USER); + }); +}); + +describe('countUsersByRole', () => { + it('returns the count of users with the given role', async () => { + await User.create([ + { name: 'Alice', email: 'alice@test.com', role: 'editor', username: 'alice' }, + { name: 'Bob', email: 'bob@test.com', role: 'editor', username: 'bob' }, + { name: 'Carol', email: 'carol@test.com', role: SystemRoles.USER, username: 'carol' }, + ]); + + expect(await countUsersByRole('editor')).toBe(2); + expect(await countUsersByRole(SystemRoles.USER)).toBe(1); + }); + + it('returns 0 when no users have the role', async () => { + expect(await countUsersByRole('nonexistent')).toBe(0); + }); +}); + +describe('listRoles', () => { + beforeEach(async () => { + await Role.deleteMany({}); + }); + + it('returns roles sorted alphabetically by name', async () => { + await Role.create([ + { name: 'zebra', permissions: {} }, + { name: 'alpha', permissions: {} }, + { name: 'middle', permissions: {} }, + ]); + + const roles = await listRoles(); + + expect(roles.map((r) => r.name)).toEqual(['alpha', 'middle', 'zebra']); + }); + + it('respects limit and offset for pagination', async () => { + await Role.create([ + { name: 'a-role', permissions: {} }, + { name: 'b-role', permissions: {} }, + { name: 'c-role', permissions: {} }, + { name: 'd-role', permissions: {} }, + { name: 'e-role', permissions: {} }, + ]); + + const page1 = await listRoles({ limit: 2, offset: 0 }); + const page2 = await listRoles({ limit: 2, offset: 2 }); + const page3 = await listRoles({ limit: 2, offset: 4 }); + + expect(page1).toHaveLength(2); + expect(page1.map((r) => r.name)).toEqual(['a-role', 'b-role']); + expect(page2).toHaveLength(2); + expect(page2.map((r) => r.name)).toEqual(['c-role', 'd-role']); + expect(page3).toHaveLength(1); + expect(page3.map((r) => r.name)).toEqual(['e-role']); + }); + + it('defaults to limit 50 and offset 0', async () => { + await Role.create({ name: 'only-role', permissions: {} }); + + const roles = await listRoles(); + + expect(roles).toHaveLength(1); + expect(roles[0].name).toBe('only-role'); + }); + + it('returns only name and description fields', async () => { + await Role.create({ + name: 'editor', + description: 'Can edit', + permissions: { PROMPTS: { USE: true } }, + }); + + const roles = await listRoles(); + + expect(roles).toHaveLength(1); + expect(roles[0].name).toBe('editor'); + expect(roles[0].description).toBe('Can edit'); + expect(roles[0]._id).toBeDefined(); + expect('permissions' in roles[0]).toBe(false); + }); + + it('returns empty array when no roles exist', async () => { + const roles = await listRoles(); + expect(roles).toEqual([]); + }); + + it('returns undefined description for pre-existing roles without the field', async () => { + await Role.collection.insertOne({ name: 'legacy', permissions: {} }); + + const roles = await listRoles(); + + expect(roles).toHaveLength(1); + expect(roles[0].name).toBe('legacy'); + expect(roles[0].description).toBeUndefined(); + }); +}); + +describe('countRoles', () => { + beforeEach(async () => { + await Role.deleteMany({}); + }); + + it('returns the total number of roles', async () => { + await Role.create([ + { name: 'a', permissions: {} }, + { name: 'b', permissions: {} }, + { name: 'c', permissions: {} }, + ]); + + expect(await countRoles()).toBe(3); + }); + + it('returns 0 when no roles exist', async () => { + expect(await countRoles()).toBe(0); + }); +}); + +describe('createRoleByName - duplicate key race', () => { + beforeEach(async () => { + await Role.deleteMany({}); + }); + + it('throws RoleConflictError on concurrent insert (11000)', async () => { + await createRoleByName({ name: 'editor' }); + + const insertSpy = jest.spyOn(Role.prototype, 'save').mockImplementationOnce(() => { + const err = new Error('E11000 duplicate key error') as Error & { code: number }; + err.code = 11000; + throw err; + }); + + await expect(createRoleByName({ name: 'editor2' })).rejects.toThrow(/already exists/); + + insertSpy.mockRestore(); + }); +}); diff --git a/packages/data-schemas/src/methods/role.ts b/packages/data-schemas/src/methods/role.ts index 7b51e45330..442041dcde 100644 --- a/packages/data-schemas/src/methods/role.ts +++ b/packages/data-schemas/src/methods/role.ts @@ -5,9 +5,24 @@ import { permissionsSchema, removeNullishValues, } from 'librechat-data-provider'; -import type { IRole } from '~/types'; +import type { Model } from 'mongoose'; +import type { IRole, IUser } from '~/types'; import logger from '~/config/winston'; +const systemRoleValues = new Set(Object.values(SystemRoles)); + +/** Case-insensitive check — the legacy roles route uppercases params. */ +function isSystemRoleName(name: string): boolean { + return systemRoleValues.has(name.toUpperCase()); +} + +export class RoleConflictError extends Error { + constructor(message: string) { + super(message); + this.name = 'RoleConflictError'; + } +} + export interface RoleDeps { /** Returns a cache store for the given key. Injected from getLogStores. */ getCache?: (key: string) => { @@ -30,8 +45,11 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol const defaultPerms = roleDefaults[roleName].permissions; if (!role) { - role = new Role(roleDefaults[roleName]); + role = new Role({ ...roleDefaults[roleName], description: '' }); } else { + if (role.description == null) { + role.description = ''; + } const permissions = role.toObject()?.permissions ?? {}; role.permissions = role.permissions || {}; for (const permType of Object.keys(defaultPerms)) { @@ -45,11 +63,26 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol } /** - * List all roles in the system. + * List all roles in the system. Returns only name and description (projected). */ - async function listRoles() { + async function listRoles(options?: { + limit?: number; + offset?: number; + }): Promise[]> { const Role = mongoose.models.Role; - return await Role.find({}).select('name permissions').lean(); + const limit = options?.limit ?? 50; + const offset = options?.offset ?? 0; + return await Role.find({}) + .select('name description') + .sort({ name: 1 }) + .skip(offset) + .limit(limit) + .lean(); + } + + async function countRoles(): Promise { + const Role = mongoose.models.Role; + return await Role.countDocuments({}); } /** @@ -73,7 +106,7 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol } const role = await query.lean().exec(); - if (!role && SystemRoles[roleName as keyof typeof SystemRoles]) { + if (!role && systemRoleValues.has(roleName)) { const newRole = await new Role(roleDefaults[roleName as keyof typeof roleDefaults]).save(); if (cache) { await cache.set(roleName, newRole); @@ -96,20 +129,24 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol const cache = deps.getCache?.(CacheKeys.ROLES); try { const Role = mongoose.models.Role; - const role = await Role.findOneAndUpdate( - { name: roleName }, - { $set: updates }, - { new: true, lean: true }, - ) + const role = await Role.findOneAndUpdate({ name: roleName }, { $set: updates }, { new: true }) .select('-__v') .lean() .exec(); if (cache) { - await cache.set(roleName, role); + if (updates.name && updates.name !== roleName) { + await Promise.all([cache.set(roleName, null), cache.set(updates.name, role)]); + } else { + await cache.set(roleName, role); + } } return role as unknown as IRole; } catch (error) { - throw new Error(`Failed to update role: ${(error as Error).message}`); + if (error && typeof error === 'object' && 'code' in error && error.code === 11000) { + const targetName = updates.name ?? roleName; + throw new RoleConflictError(`Role "${targetName}" already exists`); + } + throw new Error(`Failed to update role: ${(error as Error).message}`, { cause: error }); } } @@ -342,13 +379,137 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol } } + /** Rejects names that match system roles. */ + async function createRoleByName(roleData: Partial): Promise { + const { name } = roleData; + if (!name || typeof name !== 'string' || !name.trim()) { + throw new Error('Role name is required'); + } + const trimmed = name.trim(); + if (isSystemRoleName(trimmed)) { + throw new RoleConflictError(`Cannot create role with reserved system name: ${name}`); + } + const Role = mongoose.models.Role; + const existing = await Role.findOne({ name: trimmed }).lean(); + if (existing) { + throw new RoleConflictError(`Role "${trimmed}" already exists`); + } + let role; + try { + role = await new Role({ ...roleData, name: trimmed }).save(); + } catch (err) { + /** + * The compound unique index `{ name: 1, tenantId: 1 }` on the role schema + * (roleSchema.index in schema/role.ts) triggers error 11000 when a concurrent + * request races past the findOne check above. This catch converts it into + * the same user-facing message as the application-level duplicate check. + */ + if (err && typeof err === 'object' && 'code' in err && err.code === 11000) { + throw new RoleConflictError(`Role "${trimmed}" already exists`); + } + throw err; + } + try { + const cache = deps.getCache?.(CacheKeys.ROLES); + if (cache) { + await cache.set(role.name, role.toObject()); + } + } catch (cacheError) { + logger.error(`[createRoleByName] cache set failed for "${role.name}":`, cacheError); + } + return role.toObject() as IRole; + } + + /** + * Guards against deleting system roles. Reassigns affected users back to USER. + * + * No existence pre-check is performed: for a nonexistent role the `updateMany` + * is a harmless no-op and `findOneAndDelete` returns null. This makes the + * function idempotent — a retry after a partial failure will still clean up + * orphaned user references and cache entries. + * + * Without a MongoDB transaction the two writes are non-atomic — if the delete + * fails after the reassignment, users will already have been moved to USER + * while the role document still exists. Recovery requires the caller to retry + * the delete call, which will succeed since the `updateMany` is a no-op on + * the second pass. + */ + async function deleteRoleByName(roleName: string): Promise { + if (isSystemRoleName(roleName)) { + throw new Error(`Cannot delete system role: ${roleName}`); + } + const Role = mongoose.models.Role; + const User = mongoose.models.User as Model; + await User.updateMany({ role: roleName }, { $set: { role: SystemRoles.USER } }); + const deleted = await Role.findOneAndDelete({ name: roleName }).lean(); + try { + const cache = deps.getCache?.(CacheKeys.ROLES); + if (cache) { + // Setting null evicts the stale document. getRoleByName treats falsy cached + // values as a miss and falls through to the DB, so this does not provide + // negative caching — it only prevents serving the pre-deletion document. + await cache.set(roleName, null); + } + } catch (cacheError) { + logger.error(`[deleteRoleByName] cache invalidation failed for "${roleName}":`, cacheError); + } + return deleted as IRole | null; + } + + async function updateUsersByRole(oldRole: string, newRole: string): Promise { + const User = mongoose.models.User as Model; + await User.updateMany({ role: oldRole }, { $set: { role: newRole } }); + } + + async function findUserIdsByRole(roleName: string): Promise { + const User = mongoose.models.User as Model; + const users = await User.find({ role: roleName }).select('_id').lean(); + return users.map((u) => u._id.toString()); + } + + async function updateUsersRoleByIds(userIds: string[], newRole: string): Promise { + if (userIds.length === 0) { + return; + } + const User = mongoose.models.User as Model; + await User.updateMany({ _id: { $in: userIds } }, { $set: { role: newRole } }); + } + + async function listUsersByRole( + roleName: string, + options?: { limit?: number; offset?: number }, + ): Promise { + const User = mongoose.models.User as Model; + const limit = options?.limit ?? 50; + const offset = options?.offset ?? 0; + return await User.find({ role: roleName }) + .select('_id name email avatar') + .sort({ _id: 1 }) + .skip(offset) + .limit(limit) + .lean(); + } + + async function countUsersByRole(roleName: string): Promise { + const User = mongoose.models.User as Model; + return await User.countDocuments({ role: roleName }); + } + return { listRoles, + countRoles, initializeRoles, getRoleByName, updateRoleByName, updateAccessPermissions, migrateRoleSchema, + createRoleByName, + deleteRoleByName, + updateUsersByRole, + findUserIdsByRole, + updateUsersRoleByIds, + listUsersByRole, + countUsersByRole, }; } diff --git a/packages/data-schemas/src/schema/role.ts b/packages/data-schemas/src/schema/role.ts index 1c27478ef6..ac478c2a83 100644 --- a/packages/data-schemas/src/schema/role.ts +++ b/packages/data-schemas/src/schema/role.ts @@ -73,6 +73,7 @@ const rolePermissionsSchema = new Schema( const roleSchema: Schema = new Schema({ name: { type: String, required: true, index: true }, + description: { type: String, default: '' }, permissions: { type: rolePermissionsSchema, }, diff --git a/packages/data-schemas/src/schema/user.ts b/packages/data-schemas/src/schema/user.ts index 92680415bd..f807ddd8d6 100644 --- a/packages/data-schemas/src/schema/user.ts +++ b/packages/data-schemas/src/schema/user.ts @@ -158,6 +158,7 @@ const userSchema = new Schema( ); userSchema.index({ email: 1, tenantId: 1 }, { unique: true }); +userSchema.index({ role: 1, tenantId: 1 }); const oAuthIdFields = [ 'googleId', diff --git a/packages/data-schemas/src/types/admin.ts b/packages/data-schemas/src/types/admin.ts index 99915f659d..9b30cdb98a 100644 --- a/packages/data-schemas/src/types/admin.ts +++ b/packages/data-schemas/src/types/admin.ts @@ -114,7 +114,7 @@ export type AdminMember = { name: string; email: string; avatarUrl?: string; - joinedAt: string; + joinedAt?: string; }; /** Minimal user info returned by user search endpoints. */ diff --git a/packages/data-schemas/src/types/role.ts b/packages/data-schemas/src/types/role.ts index 60a579240c..bc85284c34 100644 --- a/packages/data-schemas/src/types/role.ts +++ b/packages/data-schemas/src/types/role.ts @@ -5,6 +5,7 @@ import { CursorPaginationParams } from '~/common'; export interface IRole extends Document { name: string; + description?: string; permissions: { [PermissionTypes.BOOKMARKS]?: { [Permissions.USE]?: boolean; @@ -74,11 +75,13 @@ export type RolePermissionsInput = DeepPartial; export interface CreateRoleRequest { name: string; + description?: string; permissions: RolePermissionsInput; } export interface UpdateRoleRequest { name?: string; + description?: string; permissions?: RolePermissionsInput; }