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
This commit is contained in:
Dustin Healy 2026-03-26 17:24:01 -07:00
parent 2a4cbfa41a
commit 2506644d58
5 changed files with 62 additions and 11 deletions

View file

@ -13,6 +13,7 @@ const requireManageRoles = requireCapability(SystemCapabilities.MANAGE_ROLES);
const handlers = createAdminRolesHandlers({
listRoles: db.listRoles,
countRoles: db.countRoles,
getRoleByName: db.getRoleByName,
createRoleByName: db.createRoleByName,
updateRoleByName: db.updateRoleByName,

View file

@ -58,6 +58,7 @@ function createReqRes(
function createDeps(overrides: Partial<AdminRolesDeps> = {}): 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()),
@ -74,16 +75,46 @@ function createDeps(overrides: Partial<AdminRolesDeps> = {}): AdminRolesDeps {
describe('createAdminRolesHandlers', () => {
describe('listRoles', () => {
it('returns roles with 200', async () => {
it('returns paginated roles with 200', async () => {
const roles = [mockRole()];
const deps = createDeps({ listRoles: jest.fn().mockResolvedValue(roles) });
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 });
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('returns 500 on error', async () => {

View file

@ -17,7 +17,8 @@ interface RoleMemberParams extends RoleNameParams {
}
export interface AdminRolesDeps {
listRoles: () => Promise<IRole[]>;
listRoles: (options?: { limit?: number; offset?: number }) => Promise<IRole[]>;
countRoles: () => Promise<number>;
getRoleByName: (name: string, fields?: string | string[] | null) => Promise<IRole | null>;
createRoleByName: (roleData: Partial<IRole>) => Promise<IRole>;
updateRoleByName: (name: string, updates: Partial<IRole>) => Promise<IRole | null>;
@ -43,6 +44,7 @@ export interface AdminRolesDeps {
export function createAdminRolesHandlers(deps: AdminRolesDeps) {
const {
listRoles,
countRoles,
getRoleByName,
createRoleByName,
updateRoleByName,
@ -55,10 +57,12 @@ export function createAdminRolesHandlers(deps: AdminRolesDeps) {
countUsersByRole,
} = deps;
async function listRolesHandler(_req: ServerRequest, res: Response) {
async function listRolesHandler(req: ServerRequest, res: Response) {
try {
const roles = await listRoles();
return res.status(200).json({ roles });
const limit = Math.min(Math.max(Number(req.query.limit) || 50, 1), 200);
const offset = Math.max(Number(req.query.offset) || 0, 0);
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' });
@ -233,6 +237,9 @@ export function createAdminRolesHandlers(deps: AdminRolesDeps) {
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);

View file

@ -2,8 +2,6 @@ import { createSessionMethods, DEFAULT_REFRESH_TOKEN_EXPIRY, type SessionMethods
import { createTokenMethods, type TokenMethods } from './token';
import { createRoleMethods, RoleConflictError, type RoleMethods, type RoleDeps } from './role';
import { createUserMethods, DEFAULT_SESSION_EXPIRY, type UserMethods } from './user';
export { RoleConflictError, DEFAULT_REFRESH_TOKEN_EXPIRY, DEFAULT_SESSION_EXPIRY };
import { createKeyMethods, type KeyMethods } from './key';
import { createFileMethods, type FileMethods } from './file';
/* Memories */
@ -51,6 +49,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 &

View file

@ -55,9 +55,21 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol
/**
* List all roles in the system.
*/
async function listRoles() {
async function listRoles(options?: { limit?: number; offset?: number }): Promise<IRole[]> {
const Role = mongoose.models.Role;
return await Role.find({}).select('name description 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<number> {
const Role = mongoose.models.Role;
return await Role.countDocuments({});
}
/**
@ -443,6 +455,7 @@ export function createRoleMethods(mongoose: typeof import('mongoose'), deps: Rol
return {
listRoles,
countRoles,
initializeRoles,
getRoleByName,
updateRoleByName,