Merge 6dd2adaff2 into 8ed0bcf5ca

.env.example

@@ -526,7 +526,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

@@ -27,7 +27,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

@@ -783,18 +783,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} */
@@ -809,10 +816,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(
@@ -821,7 +828,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,7 +3,7 @@ const fetch = require('node-fetch');
 const jwtDecode = require('jsonwebtoken/decode');
 const { ErrorTypes } = require('librechat-data-provider');
 const { findUser, createUser, updateUser } = require('~/models');
-const { resolveAppConfigForUser } = require('@librechat/api');
+const { resolveAppConfigForUser, isEnabled } = require('@librechat/api');
 const { getAppConfig } = require('~/server/services/Config');
 const { setupOpenId } = require('./openidStrategy');
 
@@ -143,6 +143,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';
@@ -162,6 +163,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({
@@ -193,6 +195,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();