feat(sso): extract groups claim for generic OIDC providers#3719
feat(sso): extract groups claim for generic OIDC providers#3719crivetimihai merged 3 commits intoIBM:mainfrom
Conversation
07a8e5f to
63f33df
Compare
|
Thanks @charlieleith. Clean implementation — configurable |
The generic OIDC normalization path in _normalize_user_info did not extract group claims from the identity token or userinfo response. This meant that _apply_team_mapping had no groups to work with for any provider that wasn't Keycloak or Entra ID. This patch adds groups extraction to the generic OIDC fallback, following the same pattern used by the Keycloak and Entra paths: - Reads groups_claim from provider_metadata (defaults to "groups") - Extracts the claim from user_data as list or string - Includes groups in the normalized return dict This enables team_mapping for providers like JumpCloud, Okta, Auth0, and any other generic OIDC provider that includes a groups claim. Signed-off-by: Charlie Leitheiser <leitch01@orac.localdomain>
Cover all branches of the new generic OIDC groups extraction logic: - Default 'groups' claim extraction (list) - Custom claim via provider_metadata.groups_claim - Single string group value wrapped in list - Absent groups claim defaults to empty list - None provider_metadata falls back to 'groups' claim - Non-list/non-string values are safely ignored - Groups extraction works alongside email_verified handling Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
…sing Address two gaps in the generic OIDC groups extraction: 1. Generic OIDC providers that emit groups only in the verified id_token (not the userinfo response) now have the configured groups_claim merged into user_data before normalization, following the same pattern already used by Keycloak and Entra. Userinfo groups take precedence when present. 2. Non-string elements (dicts, ints, None) in a groups claim list are now filtered out during normalization to prevent downstream TypeError in _map_groups_to_roles, which performs dict-key lookup on each element. Adds 4 new tests: - id_token groups merge for generic OIDC (default and custom claim names) - userinfo groups take precedence over id_token - nested/non-string list elements are filtered Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
63f33df to
99014a9
Compare
crivetimihai
left a comment
There was a problem hiding this comment.
Thanks for the contribution, @charlieleith! The core fix is correct and well-scoped — the generic OIDC path was the only one missing groups extraction, which silently broke team_mapping for providers like JumpCloud.
I've rebased onto main (resolved one conflict where the generic OIDC section was refactored to handle email_verified omission safely), removed the AI co-author tag, and added two follow-up commits on top of yours:
Changes I made
1. ID token groups merging for generic OIDC (sso_service.py:1257-1263)
The Entra and Keycloak paths in _get_user_info() merge claims from the verified ID token into user_data before normalization, but generic OIDC providers didn't. This means providers that emit groups only in the ID token (not the userinfo response) would still get groups=[]. Added a merge block that copies the configured groups_claim from verified_id_token_claims into user_data when userinfo doesn't include it. Userinfo takes precedence when present.
2. Non-string element filtering (sso_service.py:1462)
Changed groups = gc to groups = [g for g in gc if isinstance(g, str)]. The downstream _map_groups_to_roles() does if group in role_mappings (dict-key lookup), which would raise TypeError: unhashable type: 'dict' if a malformed IdP response included non-string elements in the groups list.
3. Test coverage (11 new tests total)
- 8 normalization tests covering all branches: default claim, custom claim, string value, absent claim, None metadata, non-list/non-string, nested structures filtered, email_verified interaction
- 3 integration tests for
_get_user_info(): id_token merge, userinfo precedence, custom claim name from id_token
All 216 SSO tests pass.
Summary
The generic OIDC normalization path in
_normalize_user_infodoes not extract group claims from the identity token or userinfo response. This means_apply_team_mappinghas no groups to work with for any provider that isn't Keycloak or Entra ID — soteam_mappingon generic OIDC providers (JumpCloud, Okta, Auth0, etc.) silently does nothing.Change
Adds groups extraction to the generic OIDC fallback in
_normalize_user_info, following the exact same pattern already used by the Keycloak and Entra paths:groups_claimfromprovider_metadata(defaults to"groups")user_data(handles both list and string)"groups"key in the normalized return dict10 lines added, 0 removed. Single file change.
Use case
We're using ContextForge with JumpCloud as a generic OIDC provider. JumpCloud includes a
groupsclaim in the ID token when configured. Without this patch, those groups are silently dropped by_normalize_user_info, makingteam_mappingon the SSO provider record non-functional.With the patch, setting
provider_metadata: {"groups_claim": "groups"}andteam_mapping: {"Engineering": "<team-id>"}on the SSO provider works as expected — users are auto-added to teams on login based on their IdP groups.Testing
keycloak,entra,github, oroktaby ID