diff --git a/packages/api/src/admin/roles.spec.ts b/packages/api/src/admin/roles.spec.ts index a66ad38ee4..125c2a3723 100644 --- a/packages/api/src/admin/roles.spec.ts +++ b/packages/api/src/admin/roles.spec.ts @@ -1196,6 +1196,28 @@ describe('createAdminRolesHandlers', () => { }); }); + 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()), diff --git a/packages/api/src/admin/roles.ts b/packages/api/src/admin/roles.ts index 9ed1582aee..76b51eacb0 100644 --- a/packages/api/src/admin/roles.ts +++ b/packages/api/src/admin/roles.ts @@ -8,9 +8,15 @@ import type { ServerRequest } from '~/types/http'; const MAX_NAME_LENGTH = 500; const MAX_DESCRIPTION_LENGTH = 2000; -function parsePagination(query: Record): { limit: number; offset: number } { +const DEFAULT_PAGE_LIMIT = 50; +const MAX_PAGE_LIMIT = 200; + +function parsePagination(query: { limit?: string; offset?: string }): { + limit: number; + offset: number; +} { return { - limit: Math.min(Math.max(Number(query.limit) || 50, 1), 200), + limit: Math.min(Math.max(Number(query.limit) || DEFAULT_PAGE_LIMIT, 1), MAX_PAGE_LIMIT), offset: Math.max(Number(query.offset) || 0, 0), }; } @@ -417,7 +423,11 @@ export function createAdminRolesHandlers(deps: AdminRolesDeps) { if (name === SystemRoles.ADMIN) { const postCount = await countUsersByRole(SystemRoles.ADMIN); if (postCount === 0) { - await updateUser(userId, { role: SystemRoles.ADMIN }); + try { + await updateUser(userId, { role: SystemRoles.ADMIN }); + } catch (rollbackError) { + logger.error('[adminRoles] CRITICAL: admin rollback failed:', rollbackError); + } return res.status(400).json({ error: 'Cannot remove the last admin user' }); } } diff --git a/packages/data-schemas/src/methods/role.methods.spec.ts b/packages/data-schemas/src/methods/role.methods.spec.ts index 08f0a0262c..a9468994e0 100644 --- a/packages/data-schemas/src/methods/role.methods.spec.ts +++ b/packages/data-schemas/src/methods/role.methods.spec.ts @@ -817,6 +817,16 @@ describe('listRoles', () => { 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', () => { diff --git a/packages/data-schemas/src/methods/role.ts b/packages/data-schemas/src/methods/role.ts index 69e88d347d..b9365e0e0e 100644 --- a/packages/data-schemas/src/methods/role.ts +++ b/packages/data-schemas/src/methods/role.ts @@ -62,13 +62,12 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol const Role = mongoose.models.Role; const limit = options?.limit ?? 50; const offset = options?.offset ?? 0; - const results = await Role.find({}) + return await Role.find({}) .select('name description') .sort({ name: 1 }) .skip(offset) .limit(limit) - .lean(); - return results as unknown[] as IRole[]; + .lean(); } async function countRoles(): Promise { @@ -137,7 +136,7 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol const targetName = updates.name ?? roleName; throw new RoleConflictError(`Role "${targetName}" already exists`); } - throw new Error(`Failed to update role: ${(error as Error).message}`); + throw new Error(`Failed to update role: ${(error as Error).message}`, { cause: error }); } }