From 6a04fb89e24caf61a7d517e260e41865c735bdeb Mon Sep 17 00:00:00 2001 From: nangelovv <82118320+nangelovv@users.noreply.github.com> Date: Sat, 30 May 2026 17:52:05 +0300 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=AC=20fix:=20Honor=20Admin-Panel=20`al?= =?UTF-8?q?lowedDomains`=20Override=20at=20Registration=20(#13204)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: honor admin-panel allowedDomains override at registration registerUser called getAppConfig({ baseOnly: true }), which short- circuits before any DB override merge. As a result, admin-panel edits to registration.allowedDomains were silently ignored at signup, even though they correctly apply to SSO callbacks via checkDomainAllowed (which calls getAppConfig() with the full resolution). The admin panel writes registration.allowedDomains to the __base__ principal in the configs collection. That principal is unconditionally injected by getApplicableConfigs (no user identity required), so a fully-resolved getAppConfig call picks up the override even before any user exists. This aligns native signup with the SSO paths and lets admins tighten or relax the allowed list without a backend restart. Per review feedback: pass the ALS tenantId explicitly. /api/auth runs through preAuthTenantMiddleware, which puts a tenantId into AsyncLocalStorage. Mongoose queries inside getApplicableConfigs are ALS-scoped, but the per-principal merged-config cache key uses the *explicit* tenantId parameter (see overrideCacheKey in packages/api/src/app/service.ts). If we leave tenantId undefined while ALS holds tenant A, the merged result caches at `__default__` — and a later request from tenant B would hit that entry, leaking tenant A's allowedDomains (and balance) across tenants. Reading getTenantId() and forwarding it makes the cache key match the DB scope, so __base__ overrides apply per-tenant correctly. Behavior when no admin override exists is unchanged (the merged config equals the YAML config; optional chaining handles missing fields). Tests in AuthService.spec.js: - Regression guard that getAppConfig is called with `{}` (no baseOnly) when ALS has no tenant — protects against reintroduction of the short-circuit. - New tenant-context test verifying getAppConfig({ tenantId }) when getTenantId() returns a tenant ID — protects against cross-tenant cache bleed. - Behavioral test confirming a disallowed domain returns 403 before any DB user lookup. * test: remove unused registerSchema import after merge resolution --------- Co-authored-by: Danny Avila --- api/server/services/AuthService.js | 4 +- api/server/services/AuthService.spec.js | 62 ++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/api/server/services/AuthService.js b/api/server/services/AuthService.js index 6462c88682..c3aca089d4 100644 --- a/api/server/services/AuthService.js +++ b/api/server/services/AuthService.js @@ -3,6 +3,7 @@ const jwt = require('jsonwebtoken'); const { webcrypto } = require('node:crypto'); const { logger, + getTenantId, DEFAULT_SESSION_EXPIRY, DEFAULT_REFRESH_TOKEN_EXPIRY, } = require('@librechat/data-schemas'); @@ -212,7 +213,8 @@ const registerUser = async (user, additionalData = {}) => { let newUserId; try { - const appConfig = await getAppConfig({ baseOnly: true }); + const tenantId = getTenantId(); + const appConfig = await getAppConfig(tenantId ? { tenantId } : {}); if (!isEmailDomainAllowed(email, appConfig?.registration?.allowedDomains)) { const errorMessage = 'The email address provided cannot be used. Please use a different email address.'; diff --git a/api/server/services/AuthService.spec.js b/api/server/services/AuthService.spec.js index 4ee7fc660f..3fce12cc20 100644 --- a/api/server/services/AuthService.spec.js +++ b/api/server/services/AuthService.spec.js @@ -1,5 +1,6 @@ jest.mock('@librechat/data-schemas', () => ({ logger: { info: jest.fn(), warn: jest.fn(), debug: jest.fn(), error: jest.fn() }, + getTenantId: jest.fn(() => undefined), DEFAULT_SESSION_EXPIRY: 900000, DEFAULT_REFRESH_TOKEN_EXPIRY: 604800000, })); @@ -69,7 +70,7 @@ const { parseCloudFrontCookieScope, } = require('@librechat/api'); const jwt = require('jsonwebtoken'); -const { logger } = require('@librechat/data-schemas'); +const { logger, getTenantId } = require('@librechat/data-schemas'); const { findUser, createUser, @@ -887,3 +888,62 @@ describe('CloudFront cookie integration', () => { }); }); }); + +describe('registerUser - allowedDomains admin-panel override', () => { + const validUser = { + email: 'new-user@example.com', + password: 'a-secure-password', + name: 'New User', + username: 'new-user', + }; + + beforeEach(() => { + jest.clearAllMocks(); + getTenantId.mockReturnValue(undefined); + isEmailDomainAllowed.mockReturnValue(true); + getAppConfig.mockResolvedValue({ + registration: { allowedDomains: ['example.com'] }, + balance: undefined, + }); + findUser.mockResolvedValue(null); + countUsers.mockResolvedValue(0); + }); + + it('should resolve the full app config so admin-panel overrides on the __base__ principal apply', async () => { + // Regression guard for getAppConfig({ baseOnly: true }): that option short-circuits + // before the DB override merge, which silently ignores any admin-panel edits to + // registration.allowedDomains (the admin panel writes overrides to the __base__ + // principal in the configs collection). registerUser must request the merged config + // so the global __base__ override is honored, same as it is for SSO callbacks via + // checkDomainAllowed. + await registerUser(validUser); + + expect(getAppConfig).toHaveBeenCalledTimes(1); + expect(getAppConfig).toHaveBeenCalledWith({}); + expect(getAppConfig).not.toHaveBeenCalledWith(expect.objectContaining({ baseOnly: true })); + }); + + it('should pass tenantId from ALS so the merged-config cache key matches tenant-scoped DB queries', async () => { + // /api/auth runs through preAuthTenantMiddleware, which puts a tenantId into + // AsyncLocalStorage. Mongoose queries inside getApplicableConfigs are scoped by ALS, + // but the per-principal merged-config cache key uses the explicit tenantId param. + // If we don't forward the ALS tenantId, tenant A's request caches at `__default__` + // and a later tenant B request can hit that entry — leaking config across tenants. + getTenantId.mockReturnValue('tenant-x'); + + await registerUser(validUser); + + expect(getAppConfig).toHaveBeenCalledWith({ tenantId: 'tenant-x' }); + }); + + it('should block registration when the resolved allowedDomains rejects the email', async () => { + isEmailDomainAllowed.mockReturnValue(false); + + const result = await registerUser({ ...validUser, email: 'blocked@evil.com' }); + + expect(result.status).toBe(403); + expect(result.message).toMatch(/cannot be used/i); + // Domain check must happen before any DB user lookup. + expect(findUser).not.toHaveBeenCalled(); + }); +});