From 3d1b883e9d130f9166053fbba6a214495af0b7d1 Mon Sep 17 00:00:00 2001 From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com> Date: Mon, 30 Mar 2026 20:06:50 -0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=91=A8=E2=80=8D=F0=9F=91=A8=E2=80=8D?= =?UTF-8?q?=F0=9F=91=A6=E2=80=8D=F0=9F=91=A6=20feat:=20Admin=20Users=20API?= =?UTF-8?q?=20Endpoints=20(#12446)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add admin user management endpoints Add /api/admin/users with list, search, and delete handlers gated by ACCESS_ADMIN + READ_USERS/MANAGE_USERS system grants. Handler factory in packages/api uses findUsers, countUsers, and deleteUserById from data-schemas. * fix: address convention violations in admin users handlers * fix: add pagination, self-deletion guard, and DB-level search limit - listUsers now uses parsePagination + countUsers for proper pagination matching the roles/groups pattern - findUsers extended with optional limit/offset options - deleteUser returns 403 when caller tries to delete own account - searchUsers passes limit to DB query instead of fetching all and slicing in JS - Fix import ordering per CLAUDE.md, complete logger mock - Replace fabricated date fallback with undefined * fix: deterministic sort, null-safe pagination, consistent search filter - Add sort option to findUsers; listUsers sorts by createdAt desc for deterministic pagination - Use != null guards for offset/limit to handle zero values correctly - Remove username from search filter since it is not in the projection or AdminUserSearchResult response type * fix: last-admin deletion guard and search query max-length - Prevent deleting the last admin user (look up target role, count admins, reject with 400 if count <= 1) - Cap search query at 200 characters to prevent regex DoS - Add tests for both guards * fix: include missing capability name in 403 Forbidden response * fix: cascade user deletion cleanup, search username, parallel capability checks - Cascade Config, AclEntry, and SystemGrant cleanup on user deletion (matching the pattern in roles/groups handlers) - Add username to admin search $or filter for parity with searchUsers - Parallelize READ_* capability checks in listAllGrants with Promise.all * fix: TOCTOU safety net, capability info leak, DRY/style cleanup, data-layer tests - Add post-delete admin recount with CRITICAL log if race leaves 0 admins - Revert capability name from 403 response to server-side log only - Document thin deleteUserById limitation (full cascade is a future task) - DRY: extract query.trim() to local variable in searchUsersHandler - Add username to search projection, response type, and AdminUserSearchResult - Functional filter/map in grants.ts parallel capability check - Consistent null guards and limit>0 guard in findUsers options - Fallback for empty result.message on delete response - Fix mockUser() to generate unique _id per call - Break long destructuring across multiple lines - Assert countUsers filter and non-admin skip in delete tests - Add data-layer tests for findUsers limit, offset, sort, and pagination * chore: comment out admin delete user endpoint (out of scope) * fix: cast USER principalId to ObjectId for ACL entry cleanup ACL entries store USER principalId as ObjectId (via grantPermission casting), but deleteAclEntries is a raw deleteMany that passes the filter through. Passing a string won't match stored ObjectIds, leaving orphaned entries. * chore: comment out unused requireManageUsers alongside disabled delete route * fix: add missing logger.warn mock in capabilities test * fix: harden admin users handlers — type safety, response consistency, test coverage - Unify response shape: AdminUserSearchResult.userId → id, add AdminUserListItem type - Fix unsafe req.query type assertion in searchUsersHandler (typeof guards) - Anchor search regex with ^ for prefix matching (enables index usage) - Add total/capped to search response for truncation signaling - Add parseInt radix, remove redundant new Date() wrap - Add tests: countUsers throw, countUsers call args, array query param, capped flag * fix: scope deleteGrantsForPrincipal to tenant, deterministic search sort, align test mocks - Add tenantId option to AdminUsersDeps.deleteGrantsForPrincipal and pass req.user.tenantId at the call site, matching the pattern already used by the roles and groups handlers - Add sort: { name: 1 } to searchUsersHandler for deterministic results - Align test mock deleteUserById messages with production output ('User was deleted successfully.') - Make capped-results test explicitly set limit: '20' instead of relying on the implicit default * test: add tenantId propagation test for deleteGrantsForPrincipal Add tenantId to createReqRes user type and test that a non-undefined tenantId is threaded through to deleteGrantsForPrincipal. * test: remove redundant deleteUserById override in tenantId test --------- Co-authored-by: Danny Avila --- api/server/index.js | 1 + api/server/routes/admin/users.js | 29 + api/server/routes/index.js | 2 + packages/api/src/admin/index.ts | 2 + packages/api/src/admin/users.spec.ts | 525 ++++++++++++++++++ packages/api/src/admin/users.ts | 198 +++++++ .../api/src/middleware/capabilities.spec.ts | 1 + packages/api/src/middleware/capabilities.ts | 1 + .../src/methods/user.methods.spec.ts | 58 ++ packages/data-schemas/src/methods/user.ts | 12 +- packages/data-schemas/src/types/admin.ts | 16 +- 11 files changed, 843 insertions(+), 2 deletions(-) create mode 100644 api/server/routes/admin/users.js create mode 100644 packages/api/src/admin/users.spec.ts create mode 100644 packages/api/src/admin/users.ts diff --git a/api/server/index.js b/api/server/index.js index 79776587b5..92e730332a 100644 --- a/api/server/index.js +++ b/api/server/index.js @@ -157,6 +157,7 @@ const startServer = async () => { app.use('/api/admin/grants', routes.adminGrants); app.use('/api/admin/groups', routes.adminGroups); app.use('/api/admin/roles', routes.adminRoles); + app.use('/api/admin/users', routes.adminUsers); 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/users.js b/api/server/routes/admin/users.js new file mode 100644 index 0000000000..46ef16e7c2 --- /dev/null +++ b/api/server/routes/admin/users.js @@ -0,0 +1,29 @@ +const express = require('express'); +const { createAdminUsersHandlers } = 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 requireReadUsers = requireCapability(SystemCapabilities.READ_USERS); +// const requireManageUsers = requireCapability(SystemCapabilities.MANAGE_USERS); + +const handlers = createAdminUsersHandlers({ + findUsers: db.findUsers, + countUsers: db.countUsers, + deleteUserById: db.deleteUserById, + deleteConfig: db.deleteConfig, + deleteAclEntries: db.deleteAclEntries, + deleteGrantsForPrincipal: db.deleteGrantsForPrincipal, +}); + +router.use(requireJwtAuth, requireAdminAccess); + +router.get('/', requireReadUsers, handlers.listUsers); +router.get('/search', requireReadUsers, handlers.searchUsers); +// router.delete('/:id', requireManageUsers, handlers.deleteUser); + +module.exports = router; diff --git a/api/server/routes/index.js b/api/server/routes/index.js index 245a7db8c6..1feaf63fdb 100644 --- a/api/server/routes/index.js +++ b/api/server/routes/index.js @@ -6,6 +6,7 @@ const adminConfig = require('./admin/config'); const adminGrants = require('./admin/grants'); const adminGroups = require('./admin/groups'); const adminRoles = require('./admin/roles'); +const adminUsers = require('./admin/users'); const endpoints = require('./endpoints'); const staticRoute = require('./static'); const messages = require('./messages'); @@ -39,6 +40,7 @@ module.exports = { adminGrants, adminGroups, adminRoles, + adminUsers, keys, apiKeys, user, diff --git a/packages/api/src/admin/index.ts b/packages/api/src/admin/index.ts index f32fb057e8..038ca23915 100644 --- a/packages/api/src/admin/index.ts +++ b/packages/api/src/admin/index.ts @@ -2,7 +2,9 @@ export { createAdminConfigHandlers } from './config'; export { createAdminGrantsHandlers } from './grants'; export { createAdminGroupsHandlers } from './groups'; export { createAdminRolesHandlers } from './roles'; +export { createAdminUsersHandlers } from './users'; export type { AdminConfigDeps } from './config'; export type { AdminGrantsDeps, GrantPrincipalType } from './grants'; export type { AdminGroupsDeps } from './groups'; export type { AdminRolesDeps } from './roles'; +export type { AdminUsersDeps } from './users'; diff --git a/packages/api/src/admin/users.spec.ts b/packages/api/src/admin/users.spec.ts new file mode 100644 index 0000000000..d4191eafac --- /dev/null +++ b/packages/api/src/admin/users.spec.ts @@ -0,0 +1,525 @@ +import { Types } from 'mongoose'; +import { PrincipalType, SystemRoles } from 'librechat-data-provider'; +import type { IUser, UserDeleteResult } from '@librechat/data-schemas'; +import type { Response } from 'express'; +import type { ServerRequest } from '~/types/http'; +import type { AdminUsersDeps } from './users'; +import { createAdminUsersHandlers } from './users'; + +jest.mock('@librechat/data-schemas', () => ({ + ...jest.requireActual('@librechat/data-schemas'), + logger: { error: jest.fn(), warn: jest.fn(), info: jest.fn(), debug: jest.fn() }, +})); + +const validUserId = new Types.ObjectId().toString(); + +function mockUser(overrides: Partial = {}): IUser { + return { + _id: new Types.ObjectId(), + name: 'Test User', + username: 'testuser', + email: 'test@example.com', + avatar: 'https://example.com/avatar.png', + role: 'USER', + provider: 'local', + createdAt: new Date('2025-01-01'), + updatedAt: new Date('2025-06-01'), + ...overrides, + } as IUser; +} + +function createReqRes( + overrides: { + params?: Record; + query?: Record; + user?: { _id?: Types.ObjectId; id?: string; role?: string; tenantId?: string }; + } = {}, +) { + const req = { + params: overrides.params ?? {}, + query: overrides.query ?? {}, + body: {}, + user: overrides.user ?? { _id: new Types.ObjectId(), role: 'admin' }, + } 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 = {}): AdminUsersDeps { + return { + findUsers: jest.fn().mockResolvedValue([]), + countUsers: jest.fn().mockResolvedValue(0), + deleteUserById: jest + .fn() + .mockResolvedValue({ deletedCount: 1, message: 'User was deleted successfully.' }), + deleteConfig: jest.fn().mockResolvedValue(null), + deleteAclEntries: jest.fn().mockResolvedValue(undefined), + deleteGrantsForPrincipal: jest.fn().mockResolvedValue(undefined), + ...overrides, + }; +} + +describe('createAdminUsersHandlers', () => { + describe('listUsers', () => { + it('returns paginated users with total count', async () => { + const users = [ + mockUser({ _id: new Types.ObjectId(validUserId) }), + mockUser({ name: 'Other' }), + ]; + const deps = createDeps({ + findUsers: jest.fn().mockResolvedValue(users), + countUsers: jest.fn().mockResolvedValue(2), + }); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes(); + + await handlers.listUsers(req, res); + + expect(status).toHaveBeenCalledWith(200); + const response = json.mock.calls[0][0]; + expect(response.users).toHaveLength(2); + expect(response.total).toBe(2); + expect(response).toHaveProperty('limit'); + expect(response).toHaveProperty('offset'); + expect(response.users[0]).toHaveProperty('id'); + expect(response.users[0]).toHaveProperty('name'); + expect(response.users[0]).toHaveProperty('email'); + expect(response.users[0]).toHaveProperty('role'); + }); + + it('passes pagination params to findUsers and unfiltered count', async () => { + const findUsers = jest.fn().mockResolvedValue([]); + const countUsers = jest.fn().mockResolvedValue(0); + const deps = createDeps({ findUsers, countUsers }); + const handlers = createAdminUsersHandlers(deps); + const { req, res } = createReqRes({ query: { limit: '10', offset: '20' } }); + + await handlers.listUsers(req, res); + + expect(findUsers).toHaveBeenCalledWith({}, expect.any(String), { + limit: 10, + offset: 20, + sort: { createdAt: -1 }, + }); + expect(countUsers).toHaveBeenCalledWith(); + }); + + it('returns empty list when no users', async () => { + const deps = createDeps(); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes(); + + await handlers.listUsers(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json.mock.calls[0][0].users).toEqual([]); + expect(json.mock.calls[0][0].total).toBe(0); + }); + + it('returns 500 when findUsers throws', async () => { + const deps = createDeps({ findUsers: jest.fn().mockRejectedValue(new Error('db down')) }); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes(); + + await handlers.listUsers(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to list users' }); + }); + + it('returns 500 when countUsers throws', async () => { + const deps = createDeps({ + countUsers: jest.fn().mockRejectedValue(new Error('count failed')), + }); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes(); + + await handlers.listUsers(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to list users' }); + }); + }); + + describe('searchUsers', () => { + it('returns matching users with total and capped flag', async () => { + const users = [mockUser()]; + const deps = createDeps({ findUsers: jest.fn().mockResolvedValue(users) }); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes({ query: { q: 'test' } }); + + await handlers.searchUsers(req, res); + + expect(status).toHaveBeenCalledWith(200); + const response = json.mock.calls[0][0]; + expect(response.users).toHaveLength(1); + expect(response.total).toBe(1); + expect(response.capped).toBe(false); + expect(response.users[0]).toHaveProperty('id'); + expect(response.users[0]).toHaveProperty('name'); + expect(response.users[0]).toHaveProperty('email'); + expect(response.users[0]).toHaveProperty('username'); + }); + + it('sets capped to true when results hit the limit', async () => { + const users = Array.from({ length: 20 }, () => mockUser()); + const deps = createDeps({ findUsers: jest.fn().mockResolvedValue(users) }); + const handlers = createAdminUsersHandlers(deps); + const { req, res, json } = createReqRes({ query: { q: 'test', limit: '20' } }); + + await handlers.searchUsers(req, res); + + const response = json.mock.calls[0][0]; + expect(response.total).toBe(20); + expect(response.capped).toBe(true); + }); + + it('searches name, email, and username with anchored prefix regex', async () => { + const findUsers = jest.fn().mockResolvedValue([]); + const deps = createDeps({ findUsers }); + const handlers = createAdminUsersHandlers(deps); + const { req, res } = createReqRes({ query: { q: 'test' } }); + + await handlers.searchUsers(req, res); + + const filter = findUsers.mock.calls[0][0]; + expect(filter.$or).toHaveLength(3); + expect(filter.$or[0]).toHaveProperty('name'); + expect(filter.$or[1]).toHaveProperty('email'); + expect(filter.$or[2]).toHaveProperty('username'); + expect(filter.$or[0].name.source).toBe('^test'); + }); + + it('projects username in the field selection', async () => { + const findUsers = jest.fn().mockResolvedValue([]); + const deps = createDeps({ findUsers }); + const handlers = createAdminUsersHandlers(deps); + const { req, res } = createReqRes({ query: { q: 'test' } }); + + await handlers.searchUsers(req, res); + + const projection = findUsers.mock.calls[0][1]; + expect(projection).toContain('username'); + }); + + it('escapes regex special characters in query', async () => { + const findUsers = jest.fn().mockResolvedValue([]); + const deps = createDeps({ findUsers }); + const handlers = createAdminUsersHandlers(deps); + const { req, res } = createReqRes({ query: { q: 'test.user+1' } }); + + await handlers.searchUsers(req, res); + + const filter = findUsers.mock.calls[0][0]; + expect(filter.$or[0].name).toBeInstanceOf(RegExp); + expect(filter.$or[0].name.source).toBe('^test\\.user\\+1'); + }); + + it('returns 400 when query is missing', async () => { + const deps = createDeps(); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes({ query: {} }); + + await handlers.searchUsers(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Query parameter "q" is required' }); + }); + + it('returns 400 when query is empty string', async () => { + const deps = createDeps(); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes({ query: { q: '' } }); + + await handlers.searchUsers(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Query parameter "q" is required' }); + }); + + it('returns 400 when query is whitespace-only', async () => { + const deps = createDeps(); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes({ query: { q: ' ' } }); + + await handlers.searchUsers(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Query parameter "q" is required' }); + }); + + it('returns 400 when query is too short', async () => { + const deps = createDeps(); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes({ query: { q: 'a' } }); + + await handlers.searchUsers(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Query must be at least 2 characters' }); + }); + + it('returns 400 when query exceeds max length', async () => { + const deps = createDeps(); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes({ query: { q: 'a'.repeat(201) } }); + + await handlers.searchUsers(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith( + expect.objectContaining({ error: expect.stringContaining('200') }), + ); + }); + + it('treats array query param as missing', async () => { + const deps = createDeps(); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes({ query: { q: ['foo', 'bar'] } }); + + await handlers.searchUsers(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Query parameter "q" is required' }); + }); + + it('passes limit to findUsers', async () => { + const findUsers = jest.fn().mockResolvedValue([mockUser()]); + const deps = createDeps({ findUsers }); + const handlers = createAdminUsersHandlers(deps); + const { req, res } = createReqRes({ query: { q: 'User', limit: '3' } }); + + await handlers.searchUsers(req, res); + + expect(findUsers).toHaveBeenCalledWith(expect.any(Object), expect.any(String), { + limit: 3, + sort: { name: 1 }, + }); + }); + + it('caps limit at 50', async () => { + const findUsers = jest.fn().mockResolvedValue([]); + const deps = createDeps({ findUsers }); + const handlers = createAdminUsersHandlers(deps); + const { req, res } = createReqRes({ query: { q: 'User', limit: '100' } }); + + await handlers.searchUsers(req, res); + + expect(findUsers).toHaveBeenCalledWith(expect.any(Object), expect.any(String), { + limit: 50, + sort: { name: 1 }, + }); + }); + + it('returns 500 on error', async () => { + const deps = createDeps({ findUsers: jest.fn().mockRejectedValue(new Error('db down')) }); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes({ query: { q: 'test' } }); + + await handlers.searchUsers(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to search users' }); + }); + }); + + describe('deleteUser', () => { + it('deletes user and returns 200', async () => { + const result: UserDeleteResult = { + deletedCount: 1, + message: 'User was deleted successfully.', + }; + const deps = createDeps({ deleteUserById: jest.fn().mockResolvedValue(result) }); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: validUserId } }); + + await handlers.deleteUser(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ message: 'User was deleted successfully.' }); + }); + + it('returns fallback message when result.message is empty', async () => { + const result: UserDeleteResult = { deletedCount: 1, message: '' }; + const deps = createDeps({ deleteUserById: jest.fn().mockResolvedValue(result) }); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: validUserId } }); + + await handlers.deleteUser(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ message: 'User deleted successfully' }); + }); + + it('returns 403 when deleting own account', async () => { + const userId = new Types.ObjectId(); + const deps = createDeps(); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: userId.toString() }, + user: { _id: userId, role: 'admin' }, + }); + + await handlers.deleteUser(req, res); + + expect(status).toHaveBeenCalledWith(403); + expect(json).toHaveBeenCalledWith({ error: 'Cannot delete your own account' }); + expect(deps.deleteUserById).not.toHaveBeenCalled(); + }); + + it('returns 400 when deleting the last admin', async () => { + const targetId = new Types.ObjectId().toString(); + const deps = createDeps({ + findUsers: jest.fn().mockResolvedValue([mockUser({ role: SystemRoles.ADMIN })]), + countUsers: jest.fn().mockResolvedValue(1), + }); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: targetId } }); + + await handlers.deleteUser(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Cannot delete the last admin user' }); + expect(deps.deleteUserById).not.toHaveBeenCalled(); + expect(deps.countUsers).toHaveBeenCalledWith({ role: SystemRoles.ADMIN }); + }); + + it('allows deleting an admin when other admins exist', async () => { + const targetId = new Types.ObjectId().toString(); + const deps = createDeps({ + findUsers: jest.fn().mockResolvedValue([mockUser({ role: SystemRoles.ADMIN })]), + countUsers: jest.fn().mockResolvedValue(3), + }); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status } = createReqRes({ params: { id: targetId } }); + + await handlers.deleteUser(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(deps.deleteUserById).toHaveBeenCalledWith(targetId); + }); + + it('does not check admin count when target is a regular user', async () => { + const targetId = new Types.ObjectId().toString(); + const deps = createDeps({ + findUsers: jest.fn().mockResolvedValue([mockUser({ role: 'USER' })]), + }); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status } = createReqRes({ params: { id: targetId } }); + + await handlers.deleteUser(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(deps.countUsers).not.toHaveBeenCalled(); + }); + + it('cascades cleanup of Config, AclEntries, and SystemGrants', async () => { + const result: UserDeleteResult = { + deletedCount: 1, + message: 'User was deleted successfully.', + }; + const deps = createDeps({ deleteUserById: jest.fn().mockResolvedValue(result) }); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status } = createReqRes({ params: { id: validUserId } }); + + await handlers.deleteUser(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(deps.deleteConfig).toHaveBeenCalledWith(PrincipalType.USER, validUserId); + expect(deps.deleteAclEntries).toHaveBeenCalledWith({ + principalType: PrincipalType.USER, + principalId: expect.any(Types.ObjectId), + }); + expect(deps.deleteGrantsForPrincipal).toHaveBeenCalledWith(PrincipalType.USER, validUserId, { + tenantId: undefined, + }); + }); + + it('scopes grant cleanup to the caller tenantId', async () => { + const deps = createDeps(); + const handlers = createAdminUsersHandlers(deps); + const { req, res } = createReqRes({ + params: { id: validUserId }, + user: { _id: new Types.ObjectId(), role: 'admin', tenantId: 'tenant-xyz' }, + }); + + await handlers.deleteUser(req, res); + + expect(deps.deleteGrantsForPrincipal).toHaveBeenCalledWith(PrincipalType.USER, validUserId, { + tenantId: 'tenant-xyz', + }); + }); + + it('returns success even when cascade cleanup partially fails', async () => { + const result: UserDeleteResult = { + deletedCount: 1, + message: 'User was deleted successfully.', + }; + const deps = createDeps({ + deleteUserById: jest.fn().mockResolvedValue(result), + deleteConfig: jest.fn().mockRejectedValue(new Error('cleanup failed')), + }); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: validUserId } }); + + await handlers.deleteUser(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ message: 'User was deleted successfully.' }); + }); + + it('does not cascade when user is not found', async () => { + const result: UserDeleteResult = { deletedCount: 0, message: '' }; + const deps = createDeps({ deleteUserById: jest.fn().mockResolvedValue(result) }); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status } = createReqRes({ params: { id: validUserId } }); + + await handlers.deleteUser(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(deps.deleteConfig).not.toHaveBeenCalled(); + expect(deps.deleteAclEntries).not.toHaveBeenCalled(); + expect(deps.deleteGrantsForPrincipal).not.toHaveBeenCalled(); + }); + + it('returns 400 for invalid ObjectId', async () => { + const deps = createDeps(); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: 'not-valid' } }); + + await handlers.deleteUser(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Invalid user ID format' }); + }); + + it('returns 404 when user not found', async () => { + const result: UserDeleteResult = { deletedCount: 0, message: '' }; + const deps = createDeps({ deleteUserById: jest.fn().mockResolvedValue(result) }); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: validUserId } }); + + await handlers.deleteUser(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'User not found' }); + }); + + it('returns 500 on error', async () => { + const deps = createDeps({ + deleteUserById: jest.fn().mockRejectedValue(new Error('db crash')), + }); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: validUserId } }); + + await handlers.deleteUser(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to delete user' }); + }); + }); +}); diff --git a/packages/api/src/admin/users.ts b/packages/api/src/admin/users.ts new file mode 100644 index 0000000000..8679b0dbcf --- /dev/null +++ b/packages/api/src/admin/users.ts @@ -0,0 +1,198 @@ +import { Types } from 'mongoose'; +import { PrincipalType, SystemRoles } from 'librechat-data-provider'; +import { logger, isValidObjectIdString } from '@librechat/data-schemas'; +import type { + IUser, + IConfig, + AdminUserListItem, + AdminUserSearchResult, + UserDeleteResult, +} from '@librechat/data-schemas'; +import type { FilterQuery } from 'mongoose'; +import type { Response } from 'express'; +import type { ServerRequest } from '~/types/http'; +import { parsePagination } from './pagination'; + +const MAX_SEARCH_LENGTH = 200; + +const USER_LIST_FIELDS = '_id name username email avatar role provider createdAt updatedAt'; + +export interface AdminUsersDeps { + findUsers: ( + searchCriteria: FilterQuery, + fieldsToSelect?: string | string[] | null, + options?: { limit?: number; offset?: number; sort?: Record }, + ) => Promise; + countUsers: (filter?: FilterQuery) => Promise; + /** + * Thin data-layer delete — removes the User document only. + * Full cascade of user-owned resources (conversations, messages, files, tokens, etc.) + * is handled by `UserController.deleteUserController` in the self-delete flow. + * This admin endpoint currently cascades Config, AclEntries, and SystemGrants. + * A future iteration should consolidate the full cascade into a shared service function. + */ + deleteUserById: (userId: string) => Promise; + deleteConfig: ( + principalType: PrincipalType, + principalId: string | Types.ObjectId, + ) => Promise; + deleteAclEntries: (filter: { + principalType: PrincipalType; + principalId: string | Types.ObjectId; + }) => Promise; + deleteGrantsForPrincipal: ( + principalType: PrincipalType, + principalId: string | Types.ObjectId, + options?: { tenantId?: string }, + ) => Promise; +} + +export function createAdminUsersHandlers(deps: AdminUsersDeps) { + const { + findUsers, + countUsers, + deleteUserById, + deleteConfig, + deleteAclEntries, + deleteGrantsForPrincipal, + } = deps; + + async function listUsersHandler(req: ServerRequest, res: Response) { + try { + const { limit, offset } = parsePagination(req.query); + const [users, total] = await Promise.all([ + findUsers({}, USER_LIST_FIELDS, { limit, offset, sort: { createdAt: -1 } }), + countUsers(), + ]); + + const mapped: AdminUserListItem[] = users.map((u) => ({ + id: u._id?.toString() ?? '', + name: u.name ?? '', + username: u.username ?? '', + email: u.email ?? '', + avatar: u.avatar ?? '', + role: u.role ?? 'USER', + provider: u.provider ?? 'local', + createdAt: u.createdAt?.toISOString(), + updatedAt: u.updatedAt?.toISOString(), + })); + + return res.status(200).json({ users: mapped, total, limit, offset }); + } catch (error) { + logger.error('[adminUsers] listUsers error:', error); + return res.status(500).json({ error: 'Failed to list users' }); + } + } + + async function searchUsersHandler(req: ServerRequest, res: Response) { + try { + const rawQ = req.query.q; + const rawLimit = req.query.limit; + const query = typeof rawQ === 'string' ? rawQ : undefined; + const limitStr = typeof rawLimit === 'string' ? rawLimit : '20'; + const trimmed = query?.trim() ?? ''; + + if (!trimmed) { + return res.status(400).json({ error: 'Query parameter "q" is required' }); + } + + if (trimmed.length < 2) { + return res.status(400).json({ error: 'Query must be at least 2 characters' }); + } + + if (trimmed.length > MAX_SEARCH_LENGTH) { + return res + .status(400) + .json({ error: `Query must not exceed ${MAX_SEARCH_LENGTH} characters` }); + } + + const searchLimit = Math.min(Math.max(1, parseInt(limitStr, 10) || 20), 50); + const escaped = trimmed.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const regex = new RegExp(`^${escaped}`, 'i'); + + const users = await findUsers( + { $or: [{ name: regex }, { email: regex }, { username: regex }] }, + '_id name email username avatar', + { limit: searchLimit, sort: { name: 1 } }, + ); + + const results: AdminUserSearchResult[] = users.map((u) => ({ + id: u._id?.toString() ?? '', + name: u.name ?? '', + email: u.email ?? '', + username: u.username, + avatarUrl: u.avatar, + })); + + return res + .status(200) + .json({ users: results, total: results.length, capped: results.length >= searchLimit }); + } catch (error) { + logger.error('[adminUsers] searchUsers error:', error); + return res.status(500).json({ error: 'Failed to search users' }); + } + } + + async function deleteUserHandler(req: ServerRequest, res: Response) { + try { + const { id } = req.params as { id: string }; + + if (!isValidObjectIdString(id)) { + return res.status(400).json({ error: 'Invalid user ID format' }); + } + + const callerId = req.user?._id?.toString() ?? req.user?.id; + if (callerId === id) { + return res.status(403).json({ error: 'Cannot delete your own account' }); + } + + const [targetUser] = await findUsers({ _id: id }, 'role', { limit: 1 }); + if (targetUser?.role === SystemRoles.ADMIN) { + const adminCount = await countUsers({ role: SystemRoles.ADMIN }); + if (adminCount <= 1) { + return res.status(400).json({ error: 'Cannot delete the last admin user' }); + } + } + + const result = await deleteUserById(id); + + if (result.deletedCount === 0) { + return res.status(404).json({ error: 'User not found' }); + } + + if (targetUser?.role === SystemRoles.ADMIN) { + const remaining = await countUsers({ role: SystemRoles.ADMIN }); + if (remaining === 0) { + logger.error( + `[adminUsers] CRITICAL: last admin deleted via race condition, user: ${id}. ` + + 'Manual DB intervention required to restore an ADMIN user.', + ); + } + } + + const objectId = new Types.ObjectId(id); + const tenantId = req.user?.tenantId; + const cleanupResults = await Promise.allSettled([ + deleteConfig(PrincipalType.USER, id), + deleteAclEntries({ principalType: PrincipalType.USER, principalId: objectId }), + deleteGrantsForPrincipal(PrincipalType.USER, id, { tenantId }), + ]); + for (const r of cleanupResults) { + if (r.status === 'rejected') { + logger.error('[adminUsers] cascade cleanup failed for user:', id, r.reason); + } + } + + return res.status(200).json({ message: result.message || 'User deleted successfully' }); + } catch (error) { + logger.error('[adminUsers] deleteUser error:', error); + return res.status(500).json({ error: 'Failed to delete user' }); + } + } + + return { + listUsers: listUsersHandler, + searchUsers: searchUsersHandler, + deleteUser: deleteUserHandler, + }; +} diff --git a/packages/api/src/middleware/capabilities.spec.ts b/packages/api/src/middleware/capabilities.spec.ts index 75d3142369..bfcc43f43d 100644 --- a/packages/api/src/middleware/capabilities.spec.ts +++ b/packages/api/src/middleware/capabilities.spec.ts @@ -11,6 +11,7 @@ import { generateCapabilityCheck } from './capabilities'; jest.mock('@librechat/data-schemas', () => ({ ...jest.requireActual('@librechat/data-schemas'), logger: { + warn: jest.fn(), error: jest.fn(), }, })); diff --git a/packages/api/src/middleware/capabilities.ts b/packages/api/src/middleware/capabilities.ts index 6e784a9aa7..7be93888c7 100644 --- a/packages/api/src/middleware/capabilities.ts +++ b/packages/api/src/middleware/capabilities.ts @@ -191,6 +191,7 @@ export function generateCapabilityCheck(deps: CapabilityDeps): { return; } + logger.warn(`[requireCapability] Forbidden: user ${id} missing capability '${capability}'`); res.status(403).json({ message: 'Forbidden' }); } catch (err) { logger.error(`[requireCapability] Error checking capability: ${capability}`, err); diff --git a/packages/data-schemas/src/methods/user.methods.spec.ts b/packages/data-schemas/src/methods/user.methods.spec.ts index 9fd33c5031..90f9100d1d 100644 --- a/packages/data-schemas/src/methods/user.methods.spec.ts +++ b/packages/data-schemas/src/methods/user.methods.spec.ts @@ -620,4 +620,62 @@ describe('User Methods - Database Tests', () => { expect(found?.provider).toBe('saml'); }); }); + + describe('findUsers with options', () => { + beforeEach(async () => { + await User.create([ + { name: 'Alice', email: 'alice@example.com', provider: 'local' }, + { name: 'Bob', email: 'bob@example.com', provider: 'local' }, + { name: 'Charlie', email: 'charlie@example.com', provider: 'local' }, + { name: 'Diana', email: 'diana@example.com', provider: 'local' }, + { name: 'Eve', email: 'eve@example.com', provider: 'local' }, + ]); + }); + + test('limit restricts the number of returned documents', async () => { + const users = await methods.findUsers({}, null, { limit: 2 }); + expect(users).toHaveLength(2); + }); + + test('offset skips the first N documents', async () => { + const all = await methods.findUsers({}, 'name', { sort: { name: 1 } }); + const skipped = await methods.findUsers({}, 'name', { offset: 2, sort: { name: 1 } }); + + expect(skipped).toHaveLength(3); + expect(skipped[0].name).toBe(all[2].name); + }); + + test('sort orders results by the specified field', async () => { + const asc = await methods.findUsers({}, 'name', { sort: { name: 1 } }); + const desc = await methods.findUsers({}, 'name', { sort: { name: -1 } }); + + expect(asc[0].name).toBe('Alice'); + expect(asc[4].name).toBe('Eve'); + expect(desc[0].name).toBe('Eve'); + expect(desc[4].name).toBe('Alice'); + }); + + test('limit + offset returns the correct page', async () => { + const sorted = await methods.findUsers({}, 'name', { sort: { name: 1 } }); + const page2 = await methods.findUsers({}, 'name', { + limit: 2, + offset: 2, + sort: { name: 1 }, + }); + + expect(page2).toHaveLength(2); + expect(page2[0].name).toBe(sorted[2].name); + expect(page2[1].name).toBe(sorted[3].name); + }); + + test('limit of 0 does not restrict results', async () => { + const users = await methods.findUsers({}, null, { limit: 0 }); + expect(users).toHaveLength(5); + }); + + test('returns all documents when no options provided', async () => { + const users = await methods.findUsers({}); + expect(users).toHaveLength(5); + }); + }); }); diff --git a/packages/data-schemas/src/methods/user.ts b/packages/data-schemas/src/methods/user.ts index 0b630e49b3..3c886f8b16 100644 --- a/packages/data-schemas/src/methods/user.ts +++ b/packages/data-schemas/src/methods/user.ts @@ -47,6 +47,7 @@ export function createUserMethods(mongoose: typeof import('mongoose')) { async function findUsers( searchCriteria: FilterQuery, fieldsToSelect?: string | string[] | null, + options?: { limit?: number; offset?: number; sort?: Record }, ): Promise { const User = mongoose.models.User as mongoose.Model; const normalizedCriteria = normalizeEmailInCriteria(searchCriteria); @@ -54,7 +55,16 @@ export function createUserMethods(mongoose: typeof import('mongoose')) { if (fieldsToSelect) { query.select(fieldsToSelect); } - return await query.lean(); + if (options?.sort != null) { + query.sort(options.sort); + } + if (options?.offset != null) { + query.skip(options.offset); + } + if (options?.limit != null && options.limit > 0) { + query.limit(options.limit); + } + return (await query.lean()) as IUser[]; } /** diff --git a/packages/data-schemas/src/types/admin.ts b/packages/data-schemas/src/types/admin.ts index a16f68ae9c..755bb7a2e6 100644 --- a/packages/data-schemas/src/types/admin.ts +++ b/packages/data-schemas/src/types/admin.ts @@ -111,10 +111,24 @@ export type AdminMember = { joinedAt?: string; }; +/** Full user info returned by the admin user list endpoint. */ +export type AdminUserListItem = { + id: string; + name: string; + username: string; + email: string; + avatar: string; + role: string; + provider: string; + createdAt?: string; + updatedAt?: string; +}; + /** Minimal user info returned by user search endpoints. */ export type AdminUserSearchResult = { - userId: string; + id: string; name: string; email: string; + username?: string; avatarUrl?: string; };