From d75d26164d15b30823bd7568793e6c88f12c6ba9 Mon Sep 17 00:00:00 2001 From: Peter Boers Date: Sat, 30 May 2026 16:01:26 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=91=20fix:=20support=20'userinfo'=20in?= =?UTF-8?q?=20OPENID=5FREQUIRED=5FROLE=5FTOKEN=5FKIND=20(#13182)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: 'Key ... not found in userinfo token!' for OPENID_REQUIRED_ROLE_TOKEN_KIND Added userinfo as option to: OPENID_REQUIRED_ROLE_TOKEN_KIND handler. Added a small refactor to case match the OPENID_REQUIRED_ROLE_TOKEN_KIND setting and throw an explicit error. * Addressed review feedback and switched from case match to if/else * Extracted a function to be called so Admin and User token use same code to resolve token types --- api/strategies/openidStrategy.js | 52 ++++++----- api/strategies/openidStrategy.spec.js | 124 ++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 23 deletions(-) diff --git a/api/strategies/openidStrategy.js b/api/strategies/openidStrategy.js index 7321eb59d2..85dc400e7f 100644 --- a/api/strategies/openidStrategy.js +++ b/api/strategies/openidStrategy.js @@ -425,6 +425,32 @@ async function resolveGroupsFromOverage(accessToken, sub) { } } +/** + * Resolve the source object (decoded token or userinfo) for a role check + * based on the configured token kind. Throws on invalid configuration so + * misconfiguration surfaces loudly instead of silently denying every login. + * + * @param {string} kind - One of 'access', 'id', or 'userinfo' + * @param {string} label - Human-readable label for error messages (e.g. 'required role') + * @param {Object} tokenset - The OpenID tokenset + * @param {Object} userinfo - Merged userinfo (id-token claims + UserInfo endpoint response) + */ +function getRoleSource(kind, label, tokenset, userinfo) { + if (kind === 'access') { + return jwtDecode(tokenset.access_token); + } + if (kind === 'id') { + return jwtDecode(tokenset.id_token); + } + if (kind === 'userinfo') { + return userinfo; + } + logger.error( + `[openidStrategy] Invalid ${label} token kind: ${kind}. Must be one of 'access', 'id', or 'userinfo'.`, + ); + throw new Error(`Invalid ${label} token kind`); +} + /** * Process OpenID authentication tokenset and userinfo * This is the core logic extracted from the passport strategy callback @@ -493,12 +519,7 @@ async function processOpenIDAuth(tokenset, existingUsersOnly = false) { const requiredRoleParameterPath = process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH; const requiredRoleTokenKind = process.env.OPENID_REQUIRED_ROLE_TOKEN_KIND; - let decodedToken = ''; - if (requiredRoleTokenKind === 'access' && tokenset.access_token) { - decodedToken = jwtDecode(tokenset.access_token); - } else if (requiredRoleTokenKind === 'id' && tokenset.id_token) { - decodedToken = jwtDecode(tokenset.id_token); - } + const decodedToken = getRoleSource(requiredRoleTokenKind, 'required role', tokenset, userinfo); let roles = get(decodedToken, requiredRoleParameterPath); @@ -591,23 +612,7 @@ async function processOpenIDAuth(tokenset, existingUsersOnly = false) { const adminRoleTokenKind = process.env.OPENID_ADMIN_ROLE_TOKEN_KIND; if (adminRole && adminRoleParameterPath && adminRoleTokenKind) { - let adminRoleObject; - switch (adminRoleTokenKind) { - case 'access': - adminRoleObject = jwtDecode(tokenset.access_token); - break; - case 'id': - adminRoleObject = jwtDecode(tokenset.id_token); - break; - case 'userinfo': - adminRoleObject = userinfo; - break; - default: - logger.error( - `[openidStrategy] Invalid admin role token kind: ${adminRoleTokenKind}. Must be one of 'access', 'id', or 'userinfo'.`, - ); - throw new Error('Invalid admin role token kind'); - } + const adminRoleObject = getRoleSource(adminRoleTokenKind, 'admin role', tokenset, userinfo); let adminRoles = get(adminRoleObject, adminRoleParameterPath); @@ -842,4 +847,5 @@ module.exports = { setupOpenId, getOpenIdConfig, getOpenIdEmail, + getRoleSource, }; diff --git a/api/strategies/openidStrategy.spec.js b/api/strategies/openidStrategy.spec.js index 56cc97a3af..086376306d 100644 --- a/api/strategies/openidStrategy.spec.js +++ b/api/strategies/openidStrategy.spec.js @@ -622,6 +622,72 @@ describe('setupOpenId', () => { expect(createUser).toHaveBeenCalled(); }); + it('should allow login when required role is found in userinfo claims', async () => { + process.env.OPENID_REQUIRED_ROLE = 'requiredRole'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'roles'; + process.env.OPENID_REQUIRED_ROLE_TOKEN_KIND = 'userinfo'; + + // The role is intentionally absent from the id_token and only present in + // the userinfo response — exercises the userinfo branch of the switch. + jwtDecode.mockReturnValue({}); + require('openid-client').fetchUserInfo.mockResolvedValue({ + roles: ['requiredRole'], + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + const { user } = await validate(tokenset); + + expect(user).toBeTruthy(); + expect(user.email).toBe(tokenset.claims().email); + }); + + it('should reject login when required role is missing from userinfo claims', async () => { + const { logger } = require('@librechat/data-schemas'); + process.env.OPENID_REQUIRED_ROLE = 'requiredRole'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'roles'; + process.env.OPENID_REQUIRED_ROLE_TOKEN_KIND = 'userinfo'; + + jwtDecode.mockReturnValue({}); + require('openid-client').fetchUserInfo.mockResolvedValue({ + other_claim: 'value', + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + const { user, details } = await validate(tokenset); + + expect(user).toBe(false); + expect(details.message).toBe('You must have "requiredRole" role to log in.'); + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining("Key 'roles' not found in userinfo token!"), + ); + }); + + it('should reject login with invalid required role token kind', async () => { + const { logger } = require('@librechat/data-schemas'); + process.env.OPENID_REQUIRED_ROLE = 'requiredRole'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'roles'; + process.env.OPENID_REQUIRED_ROLE_TOKEN_KIND = 'invalid'; + + jwtDecode.mockReturnValue({ + roles: ['requiredRole'], + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallbackByName('openid'); + + await expect(validate(tokenset)).rejects.toThrow('Invalid required role token kind'); + + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining( + "Invalid required role token kind: invalid. Must be one of 'access', 'id', or 'userinfo'", + ), + ); + }); + describe('group overage and groups handling', () => { it.each([ ['groups array contains required group', ['group-required', 'other-group'], true, undefined], @@ -2035,3 +2101,61 @@ describe('setupOpenId', () => { }); }); }); + +describe('getRoleSource', () => { + const { getRoleSource } = require('./openidStrategy'); + const { logger } = require('@librechat/data-schemas'); + + const accessClaims = { roles: ['from-access'] }; + const idClaims = { roles: ['from-id'] }; + const userinfo = { roles: ['from-userinfo'] }; + const tokenset = { access_token: 'access.jwt', id_token: 'id.jwt' }; + + beforeEach(() => { + jest.clearAllMocks(); + jwtDecode.mockImplementation((token) => { + if (token === 'access.jwt') return accessClaims; + if (token === 'id.jwt') return idClaims; + return {}; + }); + }); + + it.each([ + ['access', accessClaims], + ['id', idClaims], + ['userinfo', userinfo], + ])('returns the expected source object for kind=%s', (kind, expected) => { + expect(getRoleSource(kind, 'required role', tokenset, userinfo)).toEqual(expected); + }); + + it.each([ + ['undefined', undefined], + ['empty string', ''], + ['unknown kind', 'bogus'], + ])('throws and logs for invalid kind: %s', (_name, kind) => { + expect(() => getRoleSource(kind, 'required role', tokenset, userinfo)).toThrow( + 'Invalid required role token kind', + ); + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining(`Invalid required role token kind: ${kind}`), + ); + }); + + it('uses the provided label in the error message and thrown error', () => { + expect(() => getRoleSource('bogus', 'admin role', tokenset, userinfo)).toThrow( + 'Invalid admin role token kind', + ); + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining('Invalid admin role token kind: bogus'), + ); + }); + + it('propagates jwtDecode errors when the requested token is missing', () => { + jwtDecode.mockImplementation(() => { + throw new Error('Invalid token specified'); + }); + expect(() => getRoleSource('access', 'required role', {}, userinfo)).toThrow( + 'Invalid token specified', + ); + }); +});