Normalize opt-in multi-valued JWT claims for Cedar

[jhrozek]

Summary

OAuth scope claims arrive in different shapes across identity providers: Entra (scp) and Keycloak (scope) emit a single space-delimited string, while Okta (scp) emits a JSON array. The Cedar authorizer maps a string to a Cedar String and an array to a Set, and there is no single Cedar expression that tests element membership across both shapes — a type mismatch errors, and an erroring policy is silently ignored (a permit fails closed). So a policy authored for one IdP's claim shape silently mis-authorizes another's.

This adds an opt-in multi_valued_claims field to the cedarv1 authz config. For each listed claim name, the authorizer exposes two forms, on both the principal entity and the request context:

• claim_<name> — a canonical unpadded space-delimited string (arrays joined with single spaces; strings passed through unchanged), so existing like/== string-membership policies match uniformly across IdPs.
• claimset_<name> — a companion Cedar Set of the claim's elements, for exact-element membership via .contains() / .containsAll() / .containsAny() (no substring bleed — Mail.Read never matches Mail.ReadWrite, and no like-metacharacter escaping concern).

The claimset_ prefix is reserved and cannot be shadowed or spoofed by a JWT claim, because every JWT claim is emitted under the claim_ prefix. The field is opt-in: with it empty, output is byte-identical to today, so existing deployments are unaffected.

Closes #5828

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)
• E2E tests (task test-e2e)
• Linting (task lint-fix)
• Manual testing (describe below)

Unit tests were run scoped to the changed package (go test ./pkg/authz/authorizers/cedar/...) and are green, covering: array→join, verbatim string passthrough, single-element, empty array, absent claim, non-listed claim, non-string elements, multi-space input; claimset_ Set construction from both the array and string shapes; exact-element .contains with no substring bleed and .containsAll; the bare-key / never-claim_claimset_ assertion; and an empty-config backward-compat regression. Full task test and task lint-fix run in CI.

Does this introduce a user-facing change?

Yes. A new optional multi_valued_claims list on the cedarv1 authorization config. When set, each named claim is additionally exposed as a claimset_<name> Cedar Set and its claim_<name> string form is normalized (arrays joined). Unset (default) changes nothing.

Special notes for reviewers

• Scope is deliberately the Cedar authorizer only — no operator CRD field and no vMCP config plumbing. The claimset_/join behavior is consumed via the raw cedarv1 config (the thv CLI, a raw MCPAuthzConfig configMap, or the enterprise policy compiler, which writes the cedar config directly). A typed operator CRD field was intentionally omitted (YAGNI) — and because VirtualMCPServerSpec embeds the vmcp config struct, a vMCP config field would surface in the CRD schema, which we're deliberately avoiding here.
• Why keep the joined string and add a Set: the joined string preserves the already-shipped like-based enterprise Contains predicate byte-for-byte, so array-claim (Okta) support lights up as a runtime-only, zero-coordination change with no regression to existing string-IdP (Entra) deployments. The Set is the strictly-better representation for new multi-value policies and is purely additive.
• Related enterprise work depends on this: feat(authz/cedar): normalize multi-valued claims to canonical space-delimited form #5828 is consumed by the enterprise Contains claim operator.

_{Generated with Claude Code}

[JAORMX]

LGTM — clean, well-scoped opt-in feature, and the security-critical parts hold up. ✅

Reviewed by a three-lens panel (security · OAuth/OIDC · Go/duplication/reuse) against 3067776, verified against the real code:

Security (no blockers): the claimset_ prefix is genuinely unspoofable — preprocessClaims prefixes every JWT claim with claim_ (core.go:624), and the only writer of bare claimset_* keys is operator-config-driven addMultiValuedClaimSets reading trusted resolved-token values, so an attacker claim named claimset_scp lands inertly at claim_claimset_scp. Cedar also fails closed on any policy error (core.go:509-512 returns false when diagnostic.Errors is non-empty), so type confusion / absent / nested / numeric claims all deny. Principal and context surfaces share one processedClaims map, so they can't diverge.

OAuth: the claimset_<name> Set + .contains/.containsAll model matches RFC 6749 §3.3 scope semantics exactly (order-independent, exact-element — the Mail.Read vs Mail.ReadWrite test proves no substring bleed). Shape convergence across Entra/Keycloak (scp/scope string) and Okta (scp array) is solid, and the explicit warning against groups/roles (elements may contain spaces) is the right RFC-vs-reality boundary.

Go/reuse: genuine extension, not a parallel path — it reuses the existing convertToCedarValueAtDepth/convertStringArrayToCedar pipeline for Set construction, respects copy-before-mutate, and is consistent with the #5713 nested-map claim work (nested elements deliberately skipped for flat scope claims). Tests are thorough and table-driven, including the empty-config byte-identical backward-compat assertion.

Non-blocking follow-ups (see inline on core.go:674):

• Migration warning: opting an array-shaped claim in flips its claim_<name> from Set→String, which can turn a previously-erroring (fail-closed) like policy into a fail-open substring match. Worth a CHANGELOG/doc note to audit existing like policies before enabling, and steering operators to claimset_.
• Doc vs behavior: claim_<name> is documented "canonical unpadded" but the string-input path is a verbatim passthrough (the case string branch is a no-op re-assign) — either canonicalize it (strings.Join(strings.Fields(val), " ")) or soften the doc.
• Harden the invariant in code: extract claim_/claimset_ into shared consts (currently bare literals at :624, :706-710) so the collision-freedom relationship is enforced, not incidental.
• Minor: add a note that the extractGroups (:1118) vs claimset type-switches deliberately differ, so nobody "dedups" them; add a string→claimset multi-space test case.

None of these block — approving. CI still running (no failures); merge remains gated on it.

_{🤖 AI-assisted panel review via Claude Code (security · OAuth · Go/reuse). Line numbers against 3067776.}

[codecov]

Codecov Report

❌ Patch coverage is 84.31373% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.40%. Comparing base (0b6e1f1) to head (81090d1).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/authz/authorizers/cedar/core.go	84.31%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5880      +/-   ##
==========================================
+ Coverage   71.34%   71.40%   +0.05%     
==========================================
  Files         693      693              
  Lines       70587    70679      +92     
==========================================
+ Hits        50358    50466     +108     
+ Misses      16596    16563      -33     
- Partials     3633     3650      +17     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[rdimitrov]

Automated review (medium effort, precision-focused). The design is sound and the test coverage is genuinely thorough — I confirmed the claimset_ spoofing defense holds, no caller input is mutated, no nil-map panic path, and the normalize→group-extraction→preprocess ordering is correct. Three findings below: one correctness edge (non-conformant tokens) and two minor cleanups. Comfortable with this PR once finding 1 is addressed or explicitly deemed out of scope.

[JAORMX]

Re-approving on 81090d1 — this revision addresses the feedback from both this pass and @rdimitrov's, and I've verified each:

• Non-string array elements no longer produce spurious tokens (@rdimitrov's parity finding): multiValuedTokens now keeps only string elements and skips non-strings, instead of fmt.Sprint-ing them into "1e+06"/"<nil>". Tests updated to match (non_string_elements_skipped now expects "a" / ["a"]).
• claim_/claimset_ extracted to constants — the collision-freedom invariant is now enforced by shared consts rather than incidental literals.
• Doc corrected from "canonical unpadded" to "VERBATIM (spacing/order preserved)" for the string surface, and a clear Migration hazard note added warning that listing an array-shaped claim flips claim_<name> Set→String and can turn a fail-closed like policy into a fail-open substring match — steering operators to claimset_ + .contains. Exactly the concern from the earlier inline.
• The intentional normalize-vs-claimset divergence is now documented so it won't get "deduped" incorrectly.

Real checks are green (Test Go Code, Lint); the only red is the unrelated E2E Core (core) group-removal flake (same one seen on #5839 — nothing to do with Cedar authz). Nicely done.

_{🤖 AI-assisted panel review via Claude Code (security · OAuth · Go/reuse). Verified against 81090d1.}