diff --git a/api/strategies/openidStrategy.js b/api/strategies/openidStrategy.js index 198c8735ae..15e21f67ef 100644 --- a/api/strategies/openidStrategy.js +++ b/api/strategies/openidStrategy.js @@ -451,7 +451,7 @@ async function processOpenIDAuth(tokenset, existingUsersOnly = false) { throw new Error(`You must have ${rolesList} role to log in.`); } - const roleValues = Array.isArray(roles) ? roles : [roles]; + const roleValues = Array.isArray(roles) ? roles : roles.split(/[\s,]+/).filter(Boolean); if (!requiredRoles.some((role) => roleValues.includes(role))) { const rolesList = @@ -524,13 +524,14 @@ async function processOpenIDAuth(tokenset, existingUsersOnly = false) { } const adminRoles = get(adminRoleObject, adminRoleParameterPath); + let adminRoleValues = []; + if (Array.isArray(adminRoles)) { + adminRoleValues = adminRoles; + } else if (typeof adminRoles === 'string') { + adminRoleValues = adminRoles.split(/[\s,]+/).filter(Boolean); + } - if ( - adminRoles && - (adminRoles === true || - adminRoles === adminRole || - (Array.isArray(adminRoles) && adminRoles.includes(adminRole))) - ) { + if (adminRoles && (adminRoles === true || adminRoleValues.includes(adminRole))) { user.role = SystemRoles.ADMIN; logger.info(`[openidStrategy] User ${username} is an admin based on role: ${adminRole}`); } else if (user.role === SystemRoles.ADMIN) { diff --git a/api/strategies/openidStrategy.spec.js b/api/strategies/openidStrategy.spec.js index b1dc54d77b..00c65106ad 100644 --- a/api/strategies/openidStrategy.spec.js +++ b/api/strategies/openidStrategy.spec.js @@ -384,6 +384,62 @@ describe('setupOpenId', () => { expect(details.message).toBe('You must have "read" role to log in.'); }); + it('should allow login when roles claim is a space-separated string containing the required role', async () => { + // Arrange – IdP returns roles as a space-delimited string + jwtDecode.mockReturnValue({ + roles: 'role1 role2 requiredRole', + }); + + // Act + const { user } = await validate(tokenset); + + // Assert – login succeeds when required role is present after splitting + expect(user).toBeTruthy(); + expect(createUser).toHaveBeenCalled(); + }); + + it('should allow login when roles claim is a comma-separated string containing the required role', async () => { + // Arrange – IdP returns roles as a comma-delimited string + jwtDecode.mockReturnValue({ + roles: 'role1,role2,requiredRole', + }); + + // Act + const { user } = await validate(tokenset); + + // Assert – login succeeds when required role is present after splitting + expect(user).toBeTruthy(); + expect(createUser).toHaveBeenCalled(); + }); + + it('should allow login when roles claim is a mixed comma-and-space-separated string containing the required role', async () => { + // Arrange – IdP returns roles with comma-and-space delimiters + jwtDecode.mockReturnValue({ + roles: 'role1, role2, requiredRole', + }); + + // Act + const { user } = await validate(tokenset); + + // Assert – login succeeds when required role is present after splitting + expect(user).toBeTruthy(); + expect(createUser).toHaveBeenCalled(); + }); + + it('should reject login when roles claim is a space-separated string that does not contain the required role', async () => { + // Arrange – IdP returns a delimited string but required role is absent + jwtDecode.mockReturnValue({ + roles: 'role1 role2 otherRole', + }); + + // Act + const { user, details } = await validate(tokenset); + + // Assert – login is rejected with the correct error message + expect(user).toBe(false); + expect(details.message).toBe('You must have "requiredRole" role to log in.'); + }); + it('should allow login when single required role is present (backward compatibility)', async () => { // Arrange – ensure single role configuration (as set in beforeEach) // OPENID_REQUIRED_ROLE = 'requiredRole' @@ -1182,6 +1238,46 @@ describe('setupOpenId', () => { expect(user.role).toBeUndefined(); }); + it('should grant admin when admin role claim is a space-separated string containing the admin role', async () => { + // Arrange – IdP returns admin roles as a space-delimited string + 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').__getVerifyCallbackByName('openid'); + + // Act + const { user } = await validate(tokenset); + + // Assert – admin role is granted after splitting the delimited string + expect(user.role).toBe('ADMIN'); + }); + + it('should not grant admin when admin role claim is a space-separated string that does not contain the admin role', async () => { + // Arrange – delimited string present but admin role is absent + 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').__getVerifyCallbackByName('openid'); + + // Act + const { user } = await validate(tokenset); + + // Assert – admin role is not granted + 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';