diff --git a/.env.example b/.env.example index db09bb471f..f8bc03310d 100644 --- a/.env.example +++ b/.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= diff --git a/api/server/routes/config.js b/api/server/routes/config.js index 9f16d3a14b..12961d3ff5 100644 --- a/api/server/routes/config.js +++ b/api/server/routes/config.js @@ -41,7 +41,7 @@ router.get('/', async function (req, res) { const isOpenIdEnabled = !!process.env.OPENID_CLIENT_ID && - (isEnabled(process.env.OPENID_USE_PKCE) || !!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; diff --git a/api/server/socialLogins.js b/api/server/socialLogins.js index d8b364fb95..79ede61778 100644 --- a/api/server/socialLogins.js +++ b/api/server/socialLogins.js @@ -73,7 +73,7 @@ const configureSocialLogins = async (app) => { } if ( process.env.OPENID_CLIENT_ID && - (isEnabled(process.env.OPENID_USE_PKCE) || 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 diff --git a/api/strategies/openidStrategy.js b/api/strategies/openidStrategy.js index ea925da3ac..b8f9b17b7c 100644 --- a/api/strategies/openidStrategy.js +++ b/api/strategies/openidStrategy.js @@ -805,10 +805,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( @@ -817,7 +817,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(), ); diff --git a/api/strategies/openidStrategy.spec.js b/api/strategies/openidStrategy.spec.js index 4436fab672..7edd4fbd62 100644 --- a/api/strategies/openidStrategy.spec.js +++ b/api/strategies/openidStrategy.spec.js @@ -140,6 +140,8 @@ describe('setupOpenId', () => { beforeEach(async () => { // Clear previous mock calls and reset implementations jest.clearAllMocks(); + const { isEnabled } = require('@librechat/api'); + isEnabled.mockImplementation(jest.requireActual('@librechat/api').isEnabled); // Reset environment variables needed by the strategy process.env.OPENID_ISSUER = 'https://fake-issuer.com'; @@ -159,6 +161,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({ @@ -190,6 +193,72 @@ describe('setupOpenId', () => { verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); }); + describe('clientMetadata construction in setupOpenId', () => { + it('sets token_endpoint_auth_method to none for PKCE without a client secret', async () => { + const openidClient = require('openid-client'); + openidClient.discovery.mockClear(); + process.env.OPENID_USE_PKCE = 'true'; + delete 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('leaves token_endpoint_auth_method unset for secret-based clients without nonce', async () => { + const openidClient = require('openid-client'); + openidClient.discovery.mockClear(); + 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_post when nonce generation is enabled for secret-based clients', async () => { + const openidClient = require('openid-client'); + openidClient.discovery.mockClear(); + 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 a whitespace-only client secret as absent', async () => { + const openidClient = require('openid-client'); + openidClient.discovery.mockClear(); + 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 a public-client auth method when PKCE and a client secret are both configured', async () => { + const openidClient = require('openid-client'); + openidClient.discovery.mockClear(); + 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();