From 6fa3db2969cf8a487798465d198c4da1e083fc07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Pedro=20Silva?= Date: Thu, 9 Oct 2025 08:35:22 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=91=91=20feat:=20Add=20OIDC=20Claim-Based?= =?UTF-8?q?=20Admin=20Role=20Assignment=20(#9170)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: Add support for users to be admins when logging in using OpenID * fix: Linting issues * fix: whitespace * chore: add unit tests for OIDC_ADMIN_ROLE * refactor: Replace custom property retrieval function with lodash's get for improved readability and maintainability * feat: Enhance OpenID role extraction and error handling in setupOpenId function - Improved role validation to check for both array and string types. - Added detailed error messages for missing or invalid role paths in tokens. - Expanded unit tests to cover various scenarios for nested role extraction and error handling. * fix: Improve error handling for role extraction in OpenID strategy - Enhanced validation to check for invalid role types (array or string). - Updated error messages for clarity when roles are missing or of incorrect type. - Added unit tests to cover scenarios where roles return invalid types (object, number). * feat: Implement user role demotion in OpenID strategy when admin role is absent from token - Added logic to demote users from 'ADMIN' to 'USER' if the admin role is not present in the token. - Enhanced logging to capture role changes for better traceability. - Introduced unit tests to verify the demotion behavior and ensure correct handling when admin role environment variables are not configured. --------- Co-authored-by: Danny Avila --- .env.example | 3 + api/strategies/openidStrategy.js | 72 +++- api/strategies/openidStrategy.spec.js | 475 ++++++++++++++++++++++++++ 3 files changed, 539 insertions(+), 11 deletions(-) diff --git a/.env.example b/.env.example index dadcc140ae..78208307ff 100644 --- a/.env.example +++ b/.env.example @@ -459,6 +459,9 @@ OPENID_CALLBACK_URL=/oauth/openid/callback OPENID_REQUIRED_ROLE= OPENID_REQUIRED_ROLE_TOKEN_KIND= OPENID_REQUIRED_ROLE_PARAMETER_PATH= +OPENID_ADMIN_ROLE= +OPENID_ADMIN_ROLE_PARAMETER_PATH= +OPENID_ADMIN_ROLE_TOKEN_KIND= # Set to determine which user info property returned from OpenID Provider to store as the User's username OPENID_USERNAME_CLAIM= # Set to determine which user info property returned from OpenID Provider to store as the User's name diff --git a/api/strategies/openidStrategy.js b/api/strategies/openidStrategy.js index ce564fc655..079bed9e10 100644 --- a/api/strategies/openidStrategy.js +++ b/api/strategies/openidStrategy.js @@ -1,4 +1,5 @@ const undici = require('undici'); +const { get } = require('lodash'); const fetch = require('node-fetch'); const passport = require('passport'); const client = require('openid-client'); @@ -329,6 +330,12 @@ async function setupOpenId() { : 'OPENID_GENERATE_NONCE=false - Standard flow without explicit nonce or metadata', }); + // Set of env variables that specify how to set if a user is an admin + // If not set, all users will be treated as regular users + const adminRole = process.env.OPENID_ADMIN_ROLE; + const adminRoleParameterPath = process.env.OPENID_ADMIN_ROLE_PARAMETER_PATH; + const adminRoleTokenKind = process.env.OPENID_ADMIN_ROLE_TOKEN_KIND; + const openidLogin = new CustomOpenIDStrategy( { config: openidConfig, @@ -386,20 +393,19 @@ async function setupOpenId() { } else if (requiredRoleTokenKind === 'id') { decodedToken = jwtDecode(tokenset.id_token); } - const pathParts = requiredRoleParameterPath.split('.'); - let found = true; - let roles = pathParts.reduce((o, key) => { - if (o === null || o === undefined || !(key in o)) { - found = false; - return []; - } - return o[key]; - }, decodedToken); - if (!found) { + let roles = get(decodedToken, requiredRoleParameterPath); + if (!roles || (!Array.isArray(roles) && typeof roles !== 'string')) { logger.error( - `[openidStrategy] Key '${requiredRoleParameterPath}' not found in ${requiredRoleTokenKind} token!`, + `[openidStrategy] Key '${requiredRoleParameterPath}' not found or invalid type in ${requiredRoleTokenKind} token!`, ); + const rolesList = + requiredRoles.length === 1 + ? `"${requiredRoles[0]}"` + : `one of: ${requiredRoles.map((r) => `"${r}"`).join(', ')}`; + return done(null, false, { + message: `You must have ${rolesList} role to log in.`, + }); } if (!requiredRoles.some((role) => roles.includes(role))) { @@ -447,6 +453,50 @@ async function setupOpenId() { } } + 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'.`, + ); + return done(new Error('Invalid admin role token kind')); + } + + const adminRoles = get(adminRoleObject, adminRoleParameterPath); + + // Accept 3 types of values for the object extracted from adminRoleParameterPath: + // 1. A boolean value indicating if the user is an admin + // 2. A string with a single role name + // 3. An array of role names + + if ( + adminRoles && + (adminRoles === true || + adminRoles === adminRole || + (Array.isArray(adminRoles) && adminRoles.includes(adminRole))) + ) { + user.role = 'ADMIN'; + logger.info( + `[openidStrategy] User ${username} is an admin based on role: ${adminRole}`, + ); + } else if (user.role === 'ADMIN') { + user.role = 'USER'; + logger.info( + `[openidStrategy] User ${username} demoted from admin - role no longer present in token`, + ); + } + } + if (!!userinfo && userinfo.picture && !user.avatar?.includes('manual=true')) { /** @type {string | undefined} */ const imageUrl = userinfo.picture; diff --git a/api/strategies/openidStrategy.spec.js b/api/strategies/openidStrategy.spec.js index e668e078de..fa6af7f40f 100644 --- a/api/strategies/openidStrategy.spec.js +++ b/api/strategies/openidStrategy.spec.js @@ -125,6 +125,9 @@ describe('setupOpenId', () => { process.env.OPENID_REQUIRED_ROLE = 'requiredRole'; process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'roles'; process.env.OPENID_REQUIRED_ROLE_TOKEN_KIND = 'id'; + process.env.OPENID_ADMIN_ROLE = 'admin'; + process.env.OPENID_ADMIN_ROLE_PARAMETER_PATH = 'permissions'; + process.env.OPENID_ADMIN_ROLE_TOKEN_KIND = 'id'; delete process.env.OPENID_USERNAME_CLAIM; delete process.env.OPENID_NAME_CLAIM; delete process.env.PROXY; @@ -133,6 +136,7 @@ describe('setupOpenId', () => { // Default jwtDecode mock returns a token that includes the required role. jwtDecode.mockReturnValue({ roles: ['requiredRole'], + permissions: ['admin'], }); // By default, assume that no user is found, so createUser will be called @@ -441,4 +445,475 @@ describe('setupOpenId', () => { expect(callOptions.usePKCE).toBe(false); expect(callOptions.params?.code_challenge_method).toBeUndefined(); }); + + it('should set role to "ADMIN" if OPENID_ADMIN_ROLE is set and user has that role', async () => { + // Act + const { user } = await validate(tokenset); + + // Assert – verify that the user role is set to "ADMIN" + expect(user.role).toBe('ADMIN'); + }); + + it('should not set user role if OPENID_ADMIN_ROLE is set but the user does not have that role', async () => { + // Arrange – simulate a token without the admin permission + jwtDecode.mockReturnValue({ + roles: ['requiredRole'], + permissions: ['not-admin'], + }); + + // Act + const { user } = await validate(tokenset); + + // Assert – verify that the user role is not defined + expect(user.role).toBeUndefined(); + }); + + it('should demote existing admin user when admin role is removed from token', async () => { + // Arrange – simulate an existing user who is currently an admin + const existingAdminUser = { + _id: 'existingAdminId', + provider: 'openid', + email: tokenset.claims().email, + openidId: tokenset.claims().sub, + username: 'adminuser', + name: 'Admin User', + role: 'ADMIN', + }; + + findUser.mockImplementation(async (query) => { + if (query.openidId === tokenset.claims().sub || query.email === tokenset.claims().email) { + return existingAdminUser; + } + return null; + }); + + // Token without admin permission + jwtDecode.mockReturnValue({ + roles: ['requiredRole'], + permissions: ['not-admin'], + }); + + const { logger } = require('@librechat/data-schemas'); + + // Act + const { user } = await validate(tokenset); + + // Assert – verify that the user was demoted + expect(user.role).toBe('USER'); + expect(updateUser).toHaveBeenCalledWith( + existingAdminUser._id, + expect.objectContaining({ + role: 'USER', + }), + ); + expect(logger.info).toHaveBeenCalledWith( + expect.stringContaining('demoted from admin - role no longer present in token'), + ); + }); + + it('should NOT demote admin user when admin role env vars are not configured', async () => { + // Arrange – remove admin role env vars + delete process.env.OPENID_ADMIN_ROLE; + delete process.env.OPENID_ADMIN_ROLE_PARAMETER_PATH; + delete process.env.OPENID_ADMIN_ROLE_TOKEN_KIND; + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallback(); + + // Simulate an existing admin user + const existingAdminUser = { + _id: 'existingAdminId', + provider: 'openid', + email: tokenset.claims().email, + openidId: tokenset.claims().sub, + username: 'adminuser', + name: 'Admin User', + role: 'ADMIN', + }; + + findUser.mockImplementation(async (query) => { + if (query.openidId === tokenset.claims().sub || query.email === tokenset.claims().email) { + return existingAdminUser; + } + return null; + }); + + jwtDecode.mockReturnValue({ + roles: ['requiredRole'], + }); + + // Act + const { user } = await validate(tokenset); + + // Assert – verify that the admin user was NOT demoted + expect(user.role).toBe('ADMIN'); + expect(updateUser).toHaveBeenCalledWith( + existingAdminUser._id, + expect.objectContaining({ + role: 'ADMIN', + }), + ); + }); + + describe('lodash get - nested path extraction', () => { + it('should extract roles from deeply nested token path', async () => { + process.env.OPENID_REQUIRED_ROLE = 'app-user'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'resource_access.my-client.roles'; + + jwtDecode.mockReturnValue({ + resource_access: { + 'my-client': { + roles: ['app-user', 'viewer'], + }, + }, + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallback(); + + const { user } = await validate(tokenset); + + expect(user).toBeTruthy(); + expect(user.email).toBe(tokenset.claims().email); + }); + + it('should extract roles from three-level nested path', async () => { + process.env.OPENID_REQUIRED_ROLE = 'editor'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'data.access.permissions.roles'; + + jwtDecode.mockReturnValue({ + data: { + access: { + permissions: { + roles: ['editor', 'reader'], + }, + }, + }, + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallback(); + + const { user } = await validate(tokenset); + + expect(user).toBeTruthy(); + }); + + it('should log error and reject login when required role path does not exist in token', async () => { + const { logger } = require('@librechat/data-schemas'); + process.env.OPENID_REQUIRED_ROLE = 'app-user'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'resource_access.nonexistent.roles'; + + jwtDecode.mockReturnValue({ + resource_access: { + 'my-client': { + roles: ['app-user'], + }, + }, + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallback(); + + const { user, details } = await validate(tokenset); + + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining( + "Key 'resource_access.nonexistent.roles' not found or invalid type in id token!", + ), + ); + expect(user).toBe(false); + expect(details.message).toContain('role to log in'); + }); + + it('should handle missing intermediate nested path gracefully', async () => { + const { logger } = require('@librechat/data-schemas'); + process.env.OPENID_REQUIRED_ROLE = 'user'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'org.team.roles'; + + jwtDecode.mockReturnValue({ + org: { + other: 'value', + }, + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallback(); + + const { user } = await validate(tokenset); + + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining("Key 'org.team.roles' not found or invalid type in id token!"), + ); + expect(user).toBe(false); + }); + + it('should extract admin role from nested path in access token', async () => { + process.env.OPENID_ADMIN_ROLE = 'admin'; + process.env.OPENID_ADMIN_ROLE_PARAMETER_PATH = 'realm_access.roles'; + process.env.OPENID_ADMIN_ROLE_TOKEN_KIND = 'access'; + + jwtDecode.mockImplementation((token) => { + if (token === 'fake_access_token') { + return { + realm_access: { + roles: ['admin', 'user'], + }, + }; + } + return { + roles: ['requiredRole'], + }; + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallback(); + + const { user } = await validate(tokenset); + + expect(user.role).toBe('ADMIN'); + }); + + it('should extract admin role from nested path in userinfo', async () => { + process.env.OPENID_ADMIN_ROLE = 'admin'; + process.env.OPENID_ADMIN_ROLE_PARAMETER_PATH = 'organization.permissions'; + process.env.OPENID_ADMIN_ROLE_TOKEN_KIND = 'userinfo'; + + const userinfoWithNestedGroups = { + ...tokenset.claims(), + organization: { + permissions: ['admin', 'write'], + }, + }; + + require('openid-client').fetchUserInfo.mockResolvedValue({ + organization: { + permissions: ['admin', 'write'], + }, + }); + + jwtDecode.mockReturnValue({ + roles: ['requiredRole'], + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallback(); + + const { user } = await validate({ + ...tokenset, + claims: () => userinfoWithNestedGroups, + }); + + expect(user.role).toBe('ADMIN'); + }); + + it('should handle boolean admin role value', async () => { + process.env.OPENID_ADMIN_ROLE = 'admin'; + process.env.OPENID_ADMIN_ROLE_PARAMETER_PATH = 'is_admin'; + + jwtDecode.mockReturnValue({ + roles: ['requiredRole'], + is_admin: true, + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallback(); + + const { user } = await validate(tokenset); + + expect(user.role).toBe('ADMIN'); + }); + + it('should handle string admin role value matching exactly', async () => { + process.env.OPENID_ADMIN_ROLE = 'super-admin'; + process.env.OPENID_ADMIN_ROLE_PARAMETER_PATH = 'role'; + + jwtDecode.mockReturnValue({ + roles: ['requiredRole'], + role: 'super-admin', + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallback(); + + const { user } = await validate(tokenset); + + expect(user.role).toBe('ADMIN'); + }); + + it('should not set admin role when string value does not match', async () => { + process.env.OPENID_ADMIN_ROLE = 'super-admin'; + process.env.OPENID_ADMIN_ROLE_PARAMETER_PATH = 'role'; + + jwtDecode.mockReturnValue({ + roles: ['requiredRole'], + role: 'regular-user', + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallback(); + + const { user } = await validate(tokenset); + + expect(user.role).toBeUndefined(); + }); + + it('should handle array admin role value', async () => { + process.env.OPENID_ADMIN_ROLE = 'site-admin'; + process.env.OPENID_ADMIN_ROLE_PARAMETER_PATH = 'app_roles'; + + jwtDecode.mockReturnValue({ + roles: ['requiredRole'], + app_roles: ['user', 'site-admin', 'moderator'], + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallback(); + + const { user } = await validate(tokenset); + + expect(user.role).toBe('ADMIN'); + }); + + it('should not set admin when role is not in array', async () => { + process.env.OPENID_ADMIN_ROLE = 'site-admin'; + process.env.OPENID_ADMIN_ROLE_PARAMETER_PATH = 'app_roles'; + + jwtDecode.mockReturnValue({ + roles: ['requiredRole'], + app_roles: ['user', 'moderator'], + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallback(); + + const { user } = await validate(tokenset); + + expect(user.role).toBeUndefined(); + }); + + it('should handle nested path with special characters in keys', async () => { + process.env.OPENID_REQUIRED_ROLE = 'app-user'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'resource_access.my-app-123.roles'; + + jwtDecode.mockReturnValue({ + resource_access: { + 'my-app-123': { + roles: ['app-user'], + }, + }, + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallback(); + + const { user } = await validate(tokenset); + + expect(user).toBeTruthy(); + }); + + it('should handle empty object at nested path', async () => { + const { logger } = require('@librechat/data-schemas'); + process.env.OPENID_REQUIRED_ROLE = 'user'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'access.roles'; + + jwtDecode.mockReturnValue({ + access: {}, + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallback(); + + const { user } = await validate(tokenset); + + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining("Key 'access.roles' not found or invalid type in id token!"), + ); + expect(user).toBe(false); + }); + + it('should handle null value at intermediate path', async () => { + const { logger } = require('@librechat/data-schemas'); + process.env.OPENID_REQUIRED_ROLE = 'user'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'data.roles'; + + jwtDecode.mockReturnValue({ + data: null, + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallback(); + + const { user } = await validate(tokenset); + + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining("Key 'data.roles' not found or invalid type in id token!"), + ); + expect(user).toBe(false); + }); + + it('should reject login with invalid admin role token kind', async () => { + process.env.OPENID_ADMIN_ROLE = 'admin'; + process.env.OPENID_ADMIN_ROLE_PARAMETER_PATH = 'roles'; + process.env.OPENID_ADMIN_ROLE_TOKEN_KIND = 'invalid'; + + const { logger } = require('@librechat/data-schemas'); + + jwtDecode.mockReturnValue({ + roles: ['requiredRole', 'admin'], + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallback(); + + await expect(validate(tokenset)).rejects.toThrow('Invalid admin role token kind'); + + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining( + "Invalid admin role token kind: invalid. Must be one of 'access', 'id', or 'userinfo'", + ), + ); + }); + + it('should reject login when roles path returns invalid type (object)', async () => { + const { logger } = require('@librechat/data-schemas'); + process.env.OPENID_REQUIRED_ROLE = 'app-user'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'roles'; + + jwtDecode.mockReturnValue({ + roles: { admin: true, user: false }, + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallback(); + + const { user, details } = await validate(tokenset); + + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining("Key 'roles' not found or invalid type in id token!"), + ); + expect(user).toBe(false); + expect(details.message).toContain('role to log in'); + }); + + it('should reject login when roles path returns invalid type (number)', async () => { + const { logger } = require('@librechat/data-schemas'); + process.env.OPENID_REQUIRED_ROLE = 'user'; + process.env.OPENID_REQUIRED_ROLE_PARAMETER_PATH = 'roleCount'; + + jwtDecode.mockReturnValue({ + roleCount: 5, + }); + + await setupOpenId(); + verifyCallback = require('openid-client/passport').__getVerifyCallback(); + + const { user } = await validate(tokenset); + + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining("Key 'roleCount' not found or invalid type in id token!"), + ); + expect(user).toBe(false); + }); + }); });