🪪 fix: Support OpenID PKCE Without Client Secret (#12364)

* fix: allow OpenID PKCE authentication without client secret

* Linting

* Strategy fix

* fix(openid): trim secret gates and add PKCE client metadata tests

* chore(openid): normalize spec line endings

* ⚡ perf: Short-Circuit Config Override Resolution for Empty Principals (#12549)

Skip the getApplicableConfigs DB query when buildPrincipals returns
an empty array, since there are no principals to match against.

* ⚡ perf: Separate Error Handling for Principal Resolution vs Config Overrides (#12550)

Distinguish between buildPrincipals and getApplicableConfigs failures
so the uncached fallback to baseConfig is intentional and logged
separately from config override errors.

* Revert "⚡ perf: Separate Error Handling for Principal Resolution vs Config Overrides (#12550)"

This reverts commit 1729378a65.

* Revert "⚡ perf: Short-Circuit Config Override Resolution for Empty Principals (#12549)"

This reverts commit a100aa5738.

---------

Co-authored-by: CMF\e-leite <EduardoLeite@criticalmanufacturing.com>
Co-authored-by: Danny Avila <danny@librechat.ai>

.env.example

@@ -570,7 +570,8 @@ OPENID_IMAGE_URL=
 # Set to true to automatically redirect to the OpenID provider when a user visits the login page
 # This will bypass the login form completely for users, only use this if OpenID is your only authentication method
 OPENID_AUTO_REDIRECT=false
-# Set to true to use PKCE (Proof Key for Code Exchange) for OpenID authentication
+# Set to true to use PKCE (Proof Key for Code Exchange) for OpenID authentication.
+# For public clients (no client secret), leave OPENID_CLIENT_SECRET empty and set this to true.
 OPENID_USE_PKCE=false
 #Set to true to reuse openid tokens for authentication management instead of using the mongodb session and the custom refresh token.
 OPENID_REUSE_TOKENS=

api/server/routes/config.js

@@ -33,7 +33,7 @@ function isBirthday() {
 function buildSharedPayload() {
   const isOpenIdEnabled =
     !!process.env.OPENID_CLIENT_ID &&
-    !!process.env.OPENID_CLIENT_SECRET &&
+    (isEnabled(process.env.OPENID_USE_PKCE) || !!process.env.OPENID_CLIENT_SECRET?.trim()) &&
     !!process.env.OPENID_ISSUER &&
     !!process.env.OPENID_SESSION_SECRET;
 

api/server/socialLogins.js

@@ -83,7 +83,7 @@ const configureSocialLogins = async (app) => {
   }
   if (
     process.env.OPENID_CLIENT_ID &&
-    process.env.OPENID_CLIENT_SECRET &&
+    (isEnabled(process.env.OPENID_USE_PKCE) || process.env.OPENID_CLIENT_SECRET?.trim()) &&
     process.env.OPENID_ISSUER &&
     process.env.OPENID_SCOPE &&
     process.env.OPENID_SESSION_SECRET

api/strategies/openidStrategy.js

@@ -767,18 +767,25 @@ const setupOpenIdAdmin = (openidConfig) => {
  */
 async function setupOpenId() {
   try {
+    const usePKCE = isEnabled(process.env.OPENID_USE_PKCE);
     const shouldGenerateNonce = isEnabled(process.env.OPENID_GENERATE_NONCE);
 
     /** @type {ClientMetadata} */
     const clientMetadata = {
       client_id: process.env.OPENID_CLIENT_ID,
-      client_secret: process.env.OPENID_CLIENT_SECRET,
+      response_types: ['code'],
+      grant_types: ['authorization_code'],
     };
 
-    if (shouldGenerateNonce) {
-      clientMetadata.response_types = ['code'];
-      clientMetadata.grant_types = ['authorization_code'];
-      clientMetadata.token_endpoint_auth_method = 'client_secret_post';
+    const clientSecret = process.env.OPENID_CLIENT_SECRET?.trim();
+
+    if (clientSecret) {
+      clientMetadata.client_secret = clientSecret;
+      if (shouldGenerateNonce) {
+        clientMetadata.token_endpoint_auth_method = 'client_secret_post';
+      }
+    } else if (usePKCE) {
+      clientMetadata.token_endpoint_auth_method = 'none';
     }
 
     /** @type {Configuration} */
@@ -793,10 +800,10 @@ async function setupOpenId() {
     );
 
     logger.info(`[openidStrategy] OpenID authentication configuration`, {
+      usePKCE,
+      hasClientSecret: !!clientSecret,
+      tokenEndpointAuthMethod: clientMetadata.token_endpoint_auth_method ?? '(library default)',
       generateNonce: shouldGenerateNonce,
-      reason: shouldGenerateNonce
-        ? 'OPENID_GENERATE_NONCE=true - Will generate nonce and use explicit metadata for federated providers'
-        : 'OPENID_GENERATE_NONCE=false - Standard flow without explicit nonce or metadata',
     });
 
     const openidLogin = new CustomOpenIDStrategy(
@@ -805,7 +812,7 @@ async function setupOpenId() {
         scope: process.env.OPENID_SCOPE,
         callbackURL: process.env.DOMAIN_SERVER + process.env.OPENID_CALLBACK_URL,
         clockTolerance: process.env.OPENID_CLOCK_TOLERANCE || 300,
-        usePKCE: isEnabled(process.env.OPENID_USE_PKCE),
+        usePKCE,
       },
       createOpenIDCallback(),
     );

api/strategies/openidStrategy.spec.js

@@ -3,10 +3,12 @@ const fetch = require('node-fetch');
 const jwtDecode = require('jsonwebtoken/decode');
 const { ErrorTypes, FileSources } = require('librechat-data-provider');
 const { findUser, createUser, updateUser } = require('~/models');
-const { getOpenIdIssuer, resolveAppConfigForUser } = require('@librechat/api');
+const { getOpenIdIssuer, resolveAppConfigForUser, isEnabled } = require('@librechat/api');
 const { getAppConfig } = require('~/server/services/Config');
 const { setupOpenId } = require('./openidStrategy');
 
+const mockCloudfrontFileSource = FileSources.cloudfront ?? 'cloudfront';
+
 // --- Mocks ---
 jest.mock('node-fetch');
 jest.mock('jsonwebtoken/decode');
@@ -22,18 +24,68 @@ jest.mock('~/server/services/Files/strategies', () => ({
 jest.mock('~/server/services/Config', () => ({
   getAppConfig: jest.fn().mockResolvedValue({}),
 }));
-jest.mock('@librechat/api', () => ({
-  ...jest.requireActual('@librechat/api'),
-  isEnabled: jest.fn(() => false),
-  isEmailDomainAllowed: jest.fn(() => true),
-  findOpenIDUser: jest.requireActual('@librechat/api').findOpenIDUser,
-  getOpenIdEmail: jest.requireActual('@librechat/api').getOpenIdEmail,
-  getBalanceConfig: jest.fn(() => ({
-    enabled: false,
-  })),
-  getOpenIdIssuer: jest.fn(() => 'https://fake-issuer.com'),
-  resolveAppConfigForUser: jest.fn(async (_getAppConfig, _user) => ({})),
-}));
+jest.mock('@librechat/api', () => {
+  const actual = jest.requireActual('@librechat/api');
+  const getStringClaim = (claims, claim) => {
+    const value = claims[claim];
+    return typeof value === 'string' && value ? value : undefined;
+  };
+
+  return {
+    ...actual,
+    isEnabled: jest.fn(() => false),
+    isEmailDomainAllowed: jest.fn(() => true),
+    findOpenIDUser: actual.findOpenIDUser,
+    getOpenIdEmail: jest.fn((claims, strategyName = 'openidStrategy') => {
+      if (claims == null) {
+        return undefined;
+      }
+
+      const claimKey = process.env.OPENID_EMAIL_CLAIM?.trim();
+      if (claimKey) {
+        const value = claims[claimKey];
+        if (typeof value === 'string' && value) {
+          return value;
+        }
+
+        const { logger } = require('@librechat/data-schemas');
+        if (value != null) {
+          logger.warn(
+            `[${strategyName}] OPENID_EMAIL_CLAIM="${claimKey}" resolved to a non-string value (type: ${typeof value}). Falling back to: email -> preferred_username -> upn.`,
+          );
+        } else {
+          logger.warn(
+            `[${strategyName}] OPENID_EMAIL_CLAIM="${claimKey}" not present in userinfo. Falling back to: email -> preferred_username -> upn.`,
+          );
+        }
+      }
+
+      return (
+        getStringClaim(claims, 'email') ??
+        getStringClaim(claims, 'preferred_username') ??
+        getStringClaim(claims, 'upn')
+      );
+    }),
+    getBalanceConfig: jest.fn(() => ({
+      enabled: false,
+    })),
+    getOpenIdIssuer: jest.fn(() => 'https://fake-issuer.com'),
+    getAvatarFileStrategy: jest.fn((config, fallbackStrategy) => {
+      const { FileSources } = jest.requireActual('librechat-data-provider');
+      if (config?.fileStrategies) {
+        return config.fileStrategies.avatar ?? config.fileStrategies.default ?? config.fileStrategy;
+      }
+      return config?.fileStrategy ?? fallbackStrategy ?? FileSources.local;
+    }),
+    getAvatarSaveParams: jest.fn((strategy, params) => {
+      const { FileSources } = jest.requireActual('librechat-data-provider');
+      return strategy === FileSources.s3 || strategy === mockCloudfrontFileSource
+        ? { ...params, basePath: 'avatars' }
+        : params;
+    }),
+    resolveAppConfigForUser: jest.fn(async (_getAppConfig, _user) => ({})),
+  };
+});
 jest.mock('~/models', () => ({
   findUser: jest.fn(),
   createUser: jest.fn(),
@@ -145,6 +197,7 @@ describe('setupOpenId', () => {
   beforeEach(async () => {
     // Clear previous mock calls and reset implementations
     jest.clearAllMocks();
+    isEnabled.mockImplementation(jest.requireActual('@librechat/api').isEnabled);
 
     // Reset environment variables needed by the strategy
     process.env.OPENID_ISSUER = 'https://fake-issuer.com';
@@ -164,6 +217,7 @@ describe('setupOpenId', () => {
     delete process.env.OPENID_EMAIL_CLAIM;
     delete process.env.PROXY;
     delete process.env.OPENID_USE_PKCE;
+    delete process.env.OPENID_GENERATE_NONCE;
 
     // Default jwtDecode mock returns a token that includes the required role.
     jwtDecode.mockReturnValue({
@@ -195,6 +249,71 @@ describe('setupOpenId', () => {
     verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid');
   });
 
+  describe('clientMetadata construction in setupOpenId', () => {
+    let openidClient;
+
+    beforeEach(() => {
+      openidClient = require('openid-client');
+      openidClient.discovery.mockClear();
+    });
+
+    it('sets token_endpoint_auth_method to none for PKCE without a client secret', async () => {
+      process.env.OPENID_USE_PKCE = 'true';
+      delete process.env.OPENID_CLIENT_SECRET;
+
+      await setupOpenId();
+
+      const [, , metadata] = openidClient.discovery.mock.calls.at(-1);
+      expect(metadata.token_endpoint_auth_method).toBe('none');
+      expect(metadata.client_secret).toBeUndefined();
+    });
+
+    it('leaves token_endpoint_auth_method unset for secret-based clients without nonce', async () => {
+      process.env.OPENID_USE_PKCE = 'false';
+      process.env.OPENID_CLIENT_SECRET = 'my-secret';
+
+      await setupOpenId();
+
+      const [, , metadata] = openidClient.discovery.mock.calls.at(-1);
+      expect(metadata.client_secret).toBe('my-secret');
+      expect(metadata.token_endpoint_auth_method).toBeUndefined();
+    });
+
+    it('sets client_secret and client_secret_post when nonce generation is enabled', async () => {
+      process.env.OPENID_USE_PKCE = 'false';
+      process.env.OPENID_GENERATE_NONCE = 'true';
+      process.env.OPENID_CLIENT_SECRET = 'my-secret';
+
+      await setupOpenId();
+
+      const [, , metadata] = openidClient.discovery.mock.calls.at(-1);
+      expect(metadata.client_secret).toBe('my-secret');
+      expect(metadata.token_endpoint_auth_method).toBe('client_secret_post');
+    });
+
+    it('treats whitespace-only secret as absent', async () => {
+      process.env.OPENID_USE_PKCE = 'true';
+      process.env.OPENID_CLIENT_SECRET = '   ';
+
+      await setupOpenId();
+
+      const [, , metadata] = openidClient.discovery.mock.calls.at(-1);
+      expect(metadata.client_secret).toBeUndefined();
+      expect(metadata.token_endpoint_auth_method).toBe('none');
+    });
+
+    it('does not force an auth method when PKCE and a client secret are both configured without nonce', async () => {
+      process.env.OPENID_USE_PKCE = 'true';
+      process.env.OPENID_CLIENT_SECRET = 'my-secret';
+
+      await setupOpenId();
+
+      const [, , metadata] = openidClient.discovery.mock.calls.at(-1);
+      expect(metadata.client_secret).toBe('my-secret');
+      expect(metadata.token_endpoint_auth_method).toBeUndefined();
+    });
+  });
+
   it('should create a new user with correct username when preferred_username claim exists', async () => {
     // Arrange – our userinfo already has preferred_username 'testusername'
     const userinfo = tokenset.claims();
@@ -1122,7 +1241,7 @@ describe('setupOpenId', () => {
 
   it('should save CloudFront IdP avatars under the shared avatar prefix', async () => {
     const { getStrategyFunctions } = require('~/server/services/Files/strategies');
-    getAppConfig.mockResolvedValueOnce({ fileStrategy: FileSources.cloudfront });
+    getAppConfig.mockResolvedValueOnce({ fileStrategy: mockCloudfrontFileSource });
 
     const { user } = await validate(tokenset);
     const strategyResult =
@@ -1130,7 +1249,7 @@ describe('setupOpenId', () => {
     const { saveBuffer } = strategyResult.value;
     const [saveParams] = saveBuffer.mock.calls[0];
 
-    expect(getStrategyFunctions).toHaveBeenLastCalledWith(FileSources.cloudfront);
+    expect(getStrategyFunctions).toHaveBeenLastCalledWith(mockCloudfrontFileSource);
     expect(saveParams).toEqual(
       expect.objectContaining({
         basePath: 'avatars',