Skip to content

feat: accept ordered list of claim names for OIDC username and group claims - #66

Merged
philipgough merged 2 commits into
rhobs:rhobs-obs-api-konfluxfrom
redhat-chai-bot:chai-bot/oidc-claim-list
Oct 1, 2026
Merged

philipgough merged 2 commits into
rhobs:rhobs-obs-api-konfluxfrom
redhat-chai-bot:chai-bot/oidc-claim-list

Conversation

@redhat-chai-bot

Copy link
Copy Markdown

Accept an ordered list of claim names for username_claim and group_claim,
trying each in order of preference and using the first match. This enables
multi-tenant OIDC setups where different IdPs use different claim names for
the same concept.

Config (backward compatible)

# Single string still works (backward compat)
username_claim: "preferred_username"
group_claim: "groups"

# List of claims in order of preference (new)
username_claim: ["preferred_username", "email"]
group_claim: ["org_id", "rh-org-id", "groups"]

Implementation

  • StringOrSlice type with UnmarshalJSON/MarshalJSON — accepts both
    "string" and ["list"] in JSON/YAML config
  • checkAuth iterates each claim list in order, first match wins
  • Safety gate: at least one match required across both categories
  • mapstructure decode hook for CLI config compatibility

Tests (21 total, all pass with -race)

  • 10 existing scenarios preserved (backward compat)
  • 7 new claim list scenarios: first match, second match, no match, mixed lists
  • 4 StringOrSlice JSON marshal/unmarshal tests

Verification

  • go test -v -race ./authentication/... ✅ 21/21 pass
  • make build ✅
  • make test --always-make ✅
  • make lint --always-make ✅
  • make generate validate ✅

Ref: RHOBS-1795


AI-generated. Review for accuracy.

@philipgough requested from Slack

Accept an ordered list of claim names for username_claim and
group_claim, trying each in order of preference and using the first
match.  A single string value still works (backward compatible).

- Add StringOrSlice custom type with UnmarshalJSON and MarshalJSON
  that accepts both "string" and ["list"] JSON forms
- Add mapstructure decode hook for StringOrSlice conversion
- Update oidcConfig to use StringOrSlice for UsernameClaim/GroupClaim
- Refactor checkAuth to iterate claim lists with first-match-wins
- Add safety gate: at least one of username or group must match when
  both claim lists are configured
- Update main.go legacy tenant struct for list support via YAML config
- Add 7 new test scenarios for claim list behavior (first match,
  second match, no match, mixed lists) plus 4 StringOrSlice unit tests
- All 10 existing test scenarios continue to pass (backward compat)

Implements: RHOBS-1795

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@philipgough philipgough left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@moadz moadz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

Comment thread authentication/oidc.go Outdated

switch v := data.(type) {
case string:
return []string{v}, nil

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it safer here to just return StringOrSlice{v} ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d06225b — both conversions are now explicit: StringOrSlice{v} and StringOrSlice(result). Thanks for the review!


AI-generated. Review for accuracy.

Comment thread authentication/oidc.go Outdated
result[i] = s
}

return result, nil

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: same as above, instead of implicit conversion we can StringOrSlice(result)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@philipgough
philipgough merged commit 99a6eb9 into rhobs:rhobs-obs-api-konflux Oct 1, 2026
5 checks passed
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.

3 participants