Skip to content

fix: group mapping case sensitivity#441

Merged
gusfcarvalho merged 1 commit into
mainfrom
gc-fix-oidc-case-sensitivity-matching
Jun 30, 2026
Merged

fix: group mapping case sensitivity#441
gusfcarvalho merged 1 commit into
mainfrom
gc-fix-oidc-case-sensitivity-matching

Conversation

@gusfcarvalho

@gusfcarvalho gusfcarvalho commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Bug Fixes

    • Improved SSO group mapping so claim-based group names are translated more reliably into the correct login groups.
    • Fixed cases where group mappings with mixed casing or extra spaces could fail to resolve as expected.
  • Tests

    • Added coverage for group-mapping behavior across common edge cases.
    • Added a regression test to verify group assignment still works after configuration loading normalizes key names.

Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3c2ec4da-765a-4bb3-969d-e4a362a1aa0d

📥 Commits

Reviewing files that changed from the base of the PR and between ce77d44 and fbb9d92.

📒 Files selected for processing (4)
  • internal/service/sso/providers/base_oauth.go
  • internal/service/sso/providers/base_oidc.go
  • internal/service/sso/providers/group_mapping_test.go
  • internal/service/sso/providers/helpers.go

📝 Walkthrough

Walkthrough

Introduces a shared mapClaimGroups helper in helpers.go that normalizes group_mapping keys case-insensitively before matching claim-derived group identifiers. Both BaseOAuthProvider.extractGroups and BaseOIDCProvider.extractGroups are updated to delegate to this helper, removing duplicated mapping loops. Tests cover nil mappings, case normalization, and a Viper key-lowercasing regression.

SSO Group Mapping Refactor

Layer / File(s) Summary
mapClaimGroups helper
internal/service/sso/providers/helpers.go
Adds strings import and new mapClaimGroups function that builds a lowercased lookup table from group_mapping keys, derives claim groups via buildClaimGroups, and returns matched mapped group names. Returns nil when mapping is empty.
OAuth and OIDC caller updates
internal/service/sso/providers/base_oauth.go, internal/service/sso/providers/base_oidc.go
Replaces the manual buildClaimGroups + iterate + append loop in both extractGroups implementations with a single mapClaimGroups(p.config.GroupMapping, claims) call.
Unit and regression tests
internal/service/sso/providers/group_mapping_test.go
Adds loadProviderConfig helper, TestMapClaimGroups subtests (nil mapping, exact match, mixed-case, string-typed claims), and TestExtractGroups_SurvivesViperKeyLowercasing end-to-end regression confirming correct mapping when Viper lowercases config keys.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • compliance-framework/api#438: Introduces UserInfo.RawGroups for SSO reconciliation and modifies extractGroups / group mapping behavior that this PR directly refactors.

Poem

🐇 Hop, hop, a helper appears,
To lower the case and quiet the fears.
No more loops in OAuth or OIDC land,
One mapClaimGroups call, neat and grand.
The rabbit cheers — groups mapped just right! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately reflects the main change: fixing case-sensitive group mapping behavior.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gusfcarvalho gusfcarvalho enabled auto-merge (squash) June 30, 2026 07:44
@gusfcarvalho gusfcarvalho merged commit 29102db into main Jun 30, 2026
5 checks passed
@gusfcarvalho gusfcarvalho deleted the gc-fix-oidc-case-sensitivity-matching branch June 30, 2026 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant