👨👨👦👦 feat: Admin Users API Endpoints#12446
Merged
Merged
Conversation
10 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces new admin user management endpoints (/api/admin/users) to support listing, searching, and deleting users, with capability-based protection and supporting data-layer pagination/sorting.
Changes:
- Added
createAdminUsersHandlers(list/search/delete) and wired new/api/admin/usersExpress routes with admin/capability middleware. - Extended
findUsersdata-schema method to support{ sort, offset, limit }options to enable pagination/sorting from API handlers. - Added Jest unit tests for the new admin users handlers; also includes a small performance refactor in admin grants capability checks.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/data-schemas/src/methods/user.ts | Adds optional sort/offset/limit support to findUsers() to enable paginated user listing/search. |
| packages/api/src/middleware/capabilities.ts | Changes 403 response message to include missing capability string. |
| packages/api/src/admin/users.ts | Implements admin user list/search/delete handlers and cascade cleanup on delete. |
| packages/api/src/admin/users.spec.ts | Adds unit tests covering success/error/validation paths for the new handlers. |
| packages/api/src/admin/index.ts | Exports the new handler factory + deps types from the admin module. |
| packages/api/src/admin/grants.ts | Parallelizes capability checks when computing allowed types. |
| api/server/routes/index.js | Registers new adminUsers route module export. |
| api/server/routes/admin/users.js | Wires /api/admin/users endpoints with JWT + capability middleware and injected db methods. |
| api/server/index.js | Mounts the new /api/admin/users router on the Express app. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dustinhealy
force-pushed
the
feat/admin-people-picker-v2
branch
from
f20a888 to
b1173b5
Compare
dustinhealy
changed the title
dustinhealy
force-pushed
the
feat/admin-grants
branch
from
f205630 to
5d26261
Compare
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.
- 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
- 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
- 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
…ity 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
…-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
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.
dustinhealy
force-pushed
the
feat/admin-people-picker-v2
branch
from
5e3997a to
b334cad
Compare
… 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
Owner
|
@codex review |
…ort, 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
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Add tenantId to createReqRes user type and test that a non-undefined tenantId is threaded through to deleteGrantsForPrincipal.
jcbartle
pushed a commit
to jcbartle/LibreChat
that referenced
this pull request
May 11, 2026
* 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 <danny@librechat.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This pull request introduces a new set of admin user management endpoints to the API, enabling listing and searching users via the
/api/admin/usersroute. It also adds support for paginated and sorted user queries at the data layer, and includes comprehensive tests for these features. Additionally, there are minor improvements to capability checks and logging.Admin user management API:
/api/admin/userswith endpoints for listing and searching users, protected by capability-based access control. The delete endpoint is scaffolded but currently commented out. (api/server/routes/admin/users.js,api/server/index.js,api/server/routes/index.js) [1] [2] [3] [4]createAdminUsersHandlersinpackages/api/src/admin/users.tsto provide handlers for listing, searching, and deleting users, including logic for pagination, search validation, and safe deletion (e.g., preventing self-deletion and last-admin deletion). (packages/api/src/admin/users.ts)packages/api/src/admin/index.ts)User data layer enhancements:
findUsersmethod to supportlimit,offset, andsortoptions for efficient pagination and sorting, and added corresponding tests to ensure correct behavior. (packages/data-schemas/src/methods/user.ts,packages/data-schemas/src/methods/user.methods.spec.ts) [1] [2]AdminUserSearchResulttype to includeusernamefor richer search results. (packages/data-schemas/src/types/admin.ts)Other improvements:
packages/api/src/middleware/capabilities.ts)packages/api/src/admin/grants.ts)Change Type
Testing
listUsers: paginated response, pagination params forwarded, empty list, error handlingsearchUsers: regex search across name/email/username, special character escaping, input validation (missing/empty/short/long query), limit enforcement and capping, error handlingdeleteUser: successful deletion, self-deletion guard (403), last-admin guard (400), non-admin skip of admin check, cascade cleanup (Config/AclEntries/SystemGrants with ObjectId cast), partial cascade failure resilience, invalid ObjectId (400), user not found (404), error handlingmongodb-memory-server) forfindUsersoptions: limit, offset, sort, pagination, limit=0 behavior, no-options defaultChecklist