⛩️ feat: Admin Grants API Endpoints (#12438)

* feat: add System Grants handler factory with tests

Handler factory with 4 endpoints: getEffectiveCapabilities (expanded
capability set for authenticated user), getPrincipalGrants (list grants
for a specific principal), assignGrant, and revokeGrant. Write ops
dynamically check MANAGE_ROLES/GROUPS/USERS based on target principal
type. 31 unit tests covering happy paths, validation, 403, and errors.

* feat: wire System Grants REST routes

Mount /api/admin/grants with requireJwtAuth + ACCESS_ADMIN gate.
Add barrel export for createAdminGrantsHandlers and AdminGrantsDeps.

* fix: cascade grant cleanup on role deletion

Add deleteGrantsForPrincipal to AdminRolesDeps and call it in
deleteRoleHandler via Promise.allSettled after successful deletion,
matching the groups cleanup pattern. 3 tests added for cleanup call,
skip on 404, and resilience to cleanup failure.

* fix: simplify cascade grant cleanup on role deletion

Replace Promise.allSettled wrapper with a direct try/catch for the
single deleteGrantsForPrincipal call.

* fix: harden grant handlers with auth, validation, types, and RESTful revoke

- Add per-handler auth checks (401) and granular capability gates
  (READ_* for getPrincipalGrants, possession check for assignGrant)
- Extract validatePrincipal helper; rewrite validateGrantBody to use
  direct type checks instead of unsafe `as string` casts
- Align DI types with data layer (ResolvedPrincipal.principalType
  widened to string, getUserPrincipals role made optional)
- Switch revoke route from DELETE body to RESTful URL params
- Return 201 for assignGrant to match roles/groups create convention
- Handle null grantCapability return with 500
- Add comprehensive test coverage for new auth/validation paths

* fix: deduplicate ResolvedPrincipal, typed body, defensive auth checks

- Remove duplicate ResolvedPrincipal from capabilities.ts; import the
  canonical export from grants.ts
- Replace Record<string, unknown> with explicit GrantRequestBody interface
- Add defensive 403 when READ_CAPABILITY_BY_TYPE lookup misses
- Document revoke asymmetry (no possession check) with JSDoc
- Use _id only in resolveUser (avoid Mongoose virtual reliance)
- Improve null-grant error message
- Complete logger mock in tests

* refactor: move ResolvedPrincipal to shared types to fix circular dep

Extract ResolvedPrincipal from admin/grants.ts to types/principal.ts
so middleware/capabilities.ts imports from shared types rather than
depending upward on the admin handler layer.

* chore: remove dead re-export, align logger mocks across admin tests

- Remove unused ResolvedPrincipal re-export from grants.ts (canonical
  source is types/principal.ts)
- Align logger mocks in roles.spec.ts and groups.spec.ts to include
  all log levels (error, warn, info, debug) matching grants.spec.ts

* fix: cascade Config and AclEntry cleanup on role deletion

Add deleteConfig and deleteAclEntries to role deletion cascade,
matching the group deletion pattern. Previously only grants were
cleaned up, leaving orphaned config overrides and ACL entries.

* perf: single-query batch for getEffectiveCapabilities

Add getCapabilitiesForPrincipals (plural) to the data layer — a single
$or query across all principals instead of N+1 parallel queries. Wire
it into the grants handler so getEffectiveCapabilities hits the DB once
regardless of how many principals the user has.

* fix: defer SystemCapabilities access to factory call time

Move all SystemCapabilities usage (VALID_CAPABILITIES,
MANAGE_CAPABILITY_BY_TYPE, READ_CAPABILITY_BY_TYPE) inside the
createAdminGrantsHandlers factory. External test suites that mock
@librechat/data-schemas without providing SystemCapabilities crashed
at import time when grants.ts was loaded transitively.

* test: add data-layer and handler test coverage for review findings

- Add 6 mongodb-memory-server tests for getCapabilitiesForPrincipals:
  multi-principal batch, empty array, filtering, tenant scoping
- Add handler test: all principals filtered (only PUBLIC)
- Add handler test: granting an implied capability succeeds
- Add handler test: all cascade cleanup operations fail simultaneously
- Document platform-scope-only tenantId behavior in JSDoc

* fix: resolveUser fallback to user.id, early-return empty principals

- Match capabilities middleware pattern: _id?.toString() ?? user.id
  to handle JWT-deserialized users without Mongoose _id
- Move empty-array guard before principals.map() to skip unnecessary
  normalizePrincipalId calls
- Add comment explaining VALID_PRINCIPAL_TYPES module-scope asymmetry

* refactor: derive VALID_PRINCIPAL_TYPES from capability maps

Make MANAGE_CAPABILITY_BY_TYPE and READ_CAPABILITY_BY_TYPE
non-Partial Records over a shared GrantPrincipalType union, then
derive VALID_PRINCIPAL_TYPES from the map keys. This makes divergence
between the three data structures structurally impossible.

* feat: add GET /api/admin/grants list-all-grants endpoint

Add listAllGrants data-layer method and handler so the admin panel
can fetch all grants in a single request instead of fanning out
N+M calls per role and group. Response is filtered to only include
grants for principal types the caller has read access to.

* fix: update principalType to use GrantPrincipalType for consistency in grants handling

- Refactor principalType in createAdminGrantsHandlers to use GrantPrincipalType instead of PrincipalType for better type accuracy.
- Ensure type consistency across the grants handling logic in the API.

* fix: address admin grants review findings — tenantId propagation, capability validation, pagination, and test coverage

Propagate tenantId through all grant operations for multi-tenancy support.
Extract isValidCapability to accept full SystemCapability union (base, section,
assign) and reuse it in both Mongoose schema validation and handler input checks.
Replace listAllGrants with paginated listGrants + countGrants. Filter PUBLIC
principals from getCapabilitiesForPrincipals queries. Export getCachedPrincipals
from ALS store for fast-path principal resolution. Move DELETE capability param
to query string to avoid colon-in-URL issues. Remove dead code and add
comprehensive handler and data-layer test coverage.

* refactor: harden admin grants — FilterQuery types, auth-first ordering, DELETE path param, isValidCapability tests

Replace Record<string, unknown> with FilterQuery<ISystemGrant> across all
data-layer query filters. Refactor buildTenantFilter to a pure tenantCondition
function that returns a composable FilterQuery fragment, eliminating the $or
collision between tenant and principal queries. Move auth check before input
validation in getPrincipalGrantsHandler, assignGrantHandler, and
revokeGrantHandler to avoid leaking valid type names to unauthenticated callers.
Switch DELETE route from query param back to path param (/:capability) with
encodeURIComponent per project conventions. Add compound index for listGrants
sort. Type VALID_PRINCIPAL_TYPES as Set<GrantPrincipalType>. Remove unused
GetCachedPrincipalsFn type export. Add dedicated isValidCapability unit tests
and revokeGrant idempotency test.

* refactor: batch capability checks in listGrantsHandler via getHeldCapabilities

Replace 3 parallel hasCapabilityForPrincipals DB calls with a single
getHeldCapabilities query that returns the subset of capabilities any
principal holds. Also: defensive limit(0) clamp, parallelized assignGrant
auth checks, principalId type-vs-required error split, tenantCondition
hoisted to factory top, JSDoc on cascade deps, DELETE route encoding note.

* fix: normalize principalId and filter undefined in getHeldCapabilities

Add normalizePrincipalId + null guard to getHeldCapabilities, matching
the contract of getCapabilitiesForPrincipals. Simplify allCaps build
with flatMap, add no-tenantId cross-check and undefined-principalId
test cases.

* refactor: use concrete types in GrantRequestBody, rename encoding test

Replace unknown fields with explicit string types in GrantRequestBody,
matching the established pattern in roles/groups/config handlers. Rename
misleading 'encoded' test to 'with colons' since Express auto-decodes
req.params.

* fix: support hierarchical parent capabilities in possession checks

hasCapabilityForPrincipals and getHeldCapabilities now resolve parent
base capabilities for section/assignment grants. An admin holding
manage:configs can now grant manage:configs:<section> and transitively
read:configs:<section>. Fixes anti-escalation 403 blocking config
capability delegation.

* perf: use getHeldCapabilities in assignGrant to halve DB round-trips

assignGrantHandler was making two parallel hasCapabilityForPrincipals
calls to check manage + capability possession. getHeldCapabilities was
introduced in this PR specifically for this pattern. Replace with a
single batched call. Update corresponding spec assertions.

* fix: validate role existence before granting capabilities

Grants for non-existent role names were silently persisted, creating
orphaned grants that could surprise-activate if a role with that name
was later created. Add optional checkRoleExists dep to assignGrant and
wire it to getRoleByName in the route file.

* refactor: tighten principalType typing and use grantCapability in tests

Narrow getCapabilitiesForPrincipals parameter from string to
PrincipalType, removing the redundant cast. Replace direct
SystemGrant.create() calls in getCapabilitiesForPrincipals tests with
methods.grantCapability() to honor the schema's normalization invariant.
Add getHeldCapabilities extended capability tests.

* test: rename misleading cascade cleanup test name

The test only injects failure into deleteGrantsForPrincipal, not all
cascade operations. Rename from 'cascade cleanup fails' to 'grant
cleanup fails' to match the actual scope.

* fix: reorder role check after permission guard, add tenantId to index

Move checkRoleExists after the getHeldCapabilities permission check so
that a sub-MANAGE_ROLES admin cannot probe role name existence via
400 vs 403 response codes.

Add tenantId to the { principalType, capability } index so listGrants
queries in multi-tenant deployments can use a covering index instead
of post-scanning for tenant condition.

Add missing test for checkRoleExists throwing.

* fix: scope deleteGrantsForPrincipal to tenant on role deletion

deleteGrantsForPrincipal previously filtered only on principalType +
principalId, deleting grants across all tenants. Since the role schema
supports multi-tenancy (compound unique index on name + tenantId), two
tenants can share a role name like 'editor'. Deleting that role in one
tenant would wipe grants for identically-named roles in other tenants.

Add optional tenantId parameter to deleteGrantsForPrincipal. When
provided, scopes the delete to that tenant plus platform-level grants.
Propagate req.user.tenantId through the role deletion cascade.

* fix: scope grant cleanup to tenant on group deletion

Same cross-tenant gap as the role deletion path: deleteGroupHandler
called deleteGrantsForPrincipal without tenantId, so deleting a group
would wipe its grants across all tenants. Extract req.user.tenantId
and pass it through.

* test: add HTTP integration test for admin grants routes

Supertest-based test with real MongoMemoryServer exercising the full
Express wiring: route registration, injected auth middleware, handler
DI deps, and real DB round-trips.

Covers GET /, GET /effective, POST / + DELETE / lifecycle, role
existence validation, and 401 for unauthenticated callers.

Also documents the expandImplications scope: the /effective endpoint
returns base-level capabilities only; section-level resolution is
handled at authorization check time by getParentCapabilities.

* fix: use exact tenant match in deleteGrantsForPrincipal, normalize principalId, harden API

CRITICAL: deleteGrantsForPrincipal was using tenantCondition (a
read-query helper) for deleteMany, which includes the
{ tenantId: { $exists: false } } arm. This silently destroyed
platform-level grants when a tenant-scoped role/group deletion
occurred. Replace with exact { tenantId } match for deletes so
platform-level grants survive tenant-scoped cascade cleanup.

Refactor deleteGrantsForPrincipal signature from fragile positional
overload (sessionOrTenantId union + maybeSession) to a clean options
object: { tenantId?, session? }. Update all callers and test assertions.

Add normalizePrincipalId to hasCapabilityForPrincipals to match the
pattern already used by getHeldCapabilities — prevents string/ObjectId
type mismatch on USER/GROUP principal queries.

Also: export GrantPrincipalType from barrel, add upper-bound cap to
listGrants, document GROUP/USER existence check trade-off, add
integration tests for tenant-isolation property of deleteGrantsForPrincipal.

* fix: forward tenantId to getUserPrincipals in resolvePrincipals

resolvePrincipals had tenantId available from the caller but only
forwarded it to getCachedPrincipals (cache lookup). The DB fallback
via getUserPrincipals omitted it. While the Group schema's
applyTenantIsolation Mongoose plugin handles scoping via
AsyncLocalStorage in HTTP request context, explicitly passing tenantId
makes the contract visible and prevents silent cross-tenant group
resolution if called outside request context.

* fix: remove unused import and add assertion to 401 integration test

Remove unused SystemCapabilities import flagged by ESLint. Add explicit
body assertion to the 401 test so it has a jest expect() call.

* chore: hoist grant limit constants to scope, remove dead isolateModules

Move GRANTS_DEFAULT_LIMIT / GRANTS_MAX_LIMIT from inside listGrants
function body to createSystemGrantMethods scope so they are evaluated
once at module load. Remove dead jest.isolateModules + jest.doMock
block in integration test — the ~/models mock was never exercised
since handlers are built with explicit DI deps.

---------

Co-authored-by: Danny Avila <danny@librechat.ai>
This commit is contained in:
Dustin Healy 2026-03-30 13:49:23 -07:00 committed by GitHub
parent 56d994e9ec
commit a4a17ac771
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
20 changed files with 2828 additions and 56 deletions

View file

@ -0,0 +1,35 @@
const express = require('express');
const { createAdminGrantsHandlers, getCachedPrincipals } = 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 handlers = createAdminGrantsHandlers({
listGrants: db.listGrants,
countGrants: db.countGrants,
getCapabilitiesForPrincipal: db.getCapabilitiesForPrincipal,
getCapabilitiesForPrincipals: db.getCapabilitiesForPrincipals,
grantCapability: db.grantCapability,
revokeCapability: db.revokeCapability,
getUserPrincipals: db.getUserPrincipals,
hasCapabilityForPrincipals: db.hasCapabilityForPrincipals,
getHeldCapabilities: db.getHeldCapabilities,
getCachedPrincipals,
checkRoleExists: async (name) => (await db.getRoleByName(name)) != null,
});
router.use(requireJwtAuth, requireAdminAccess);
router.get('/', handlers.listGrants);
router.get('/effective', handlers.getEffectiveCapabilities);
router.get('/:principalType/:principalId', handlers.getPrincipalGrants);
router.post('/', handlers.assignGrant);
/** Callers should encodeURIComponent the capability for client compatibility (e.g. manage%3Aconfigs%3Aendpoints). */
router.delete('/:principalType/:principalId/:capability', handlers.revokeGrant);
module.exports = router;