Repository navigation
feat: propagate arbitrary identity traits into consent session tokens - #389
bobbyiliev wants to merge 3 commits into
Conversation
…ESSION_EXTRA_TRAITS_ID_TOKEN and SESSION_EXTRA_TRAITS_ACCESS_TOKEN env vars
bobbyiliev
changed the title
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds allowlist-based propagation of selected identity traits into consent session ChangesTrait propagation feature
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
README.md (1)
59-68: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDocument that trait propagation is scope-independent.
The README accurately describes the mechanics but doesn't mention that these traits are copied unconditionally — regardless of which OIDC scopes the client requested. This differs from the built-in
profileclaim mappings, which are scope-gated. Adding a brief note would help operators understand the privacy implications.📝 Suggested addition
- `SESSION_EXTRA_TRAITS_ID_TOKEN` (optional): Comma-separated list of Kratos identity trait names to propagate verbatim into `session.id_token` during the OAuth2 consent flow. Any trait present on `identity.traits` under one of the listed names is copied to the ID token; missing traits are skipped. Useful when downstream apps rely on identity attributes that aren't part of the built-in `email` / `profile` claim mapping (e.g. `groups`, `department`, - `tenant_id`). Example: `SESSION_EXTRA_TRAITS_ID_TOKEN=groups,department`. + `tenant_id`). Unlike the built-in `email` and `profile` claims, these traits + are copied regardless of the OIDC scopes requested by the client. Example: + `SESSION_EXTRA_TRAITS_ID_TOKEN=groups,department`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 59 - 68, Update the README entry for SESSION_EXTRA_TRAITS_ID_TOKEN and SESSION_EXTRA_TRAITS_ACCESS_TOKEN to explicitly state that these extra traits are propagated unconditionally, independent of the OIDC scopes requested by the client. Keep the note close to the existing description in README.md and contrast it with the built-in email/profile claim mappings so operators understand the scope-independent behavior.src/routes/consent.ts (1)
15-34: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider memoizing
parseTraitListresults.
parseTraitList(process.env.SESSION_EXTRA_TRAITS_ID_TOKEN)andparseTraitList(process.env.SESSION_EXTRA_TRAITS_ACCESS_TOKEN)are called on every consent request, but environment variables are immutable during the process lifetime. Parsing could be done once at module load.♻️ Optional refactor: parse once at module scope
+const EXTRA_TRAITS_ID_TOKEN = parseTraitList(process.env.SESSION_EXTRA_TRAITS_ID_TOKEN) +const EXTRA_TRAITS_ACCESS_TOKEN = parseTraitList(process.env.SESSION_EXTRA_TRAITS_ACCESS_TOKEN) + const copyTraitsInto = ( target: Record<string, unknown>, traits: Record<string, unknown> | undefined, names: string[], ): void => { if (!traits) return for (const name of names) { if (Object.prototype.hasOwnProperty.call(traits, name)) { target[name] = traits[name] } } }Then at the call sites:
copyTraitsInto( session.id_token as Record<string, unknown>, identity.traits as Record<string, unknown> | undefined, - parseTraitList(process.env.SESSION_EXTRA_TRAITS_ID_TOKEN), + EXTRA_TRAITS_ID_TOKEN, ) copyTraitsInto( session.access_token as Record<string, unknown>, identity.traits as Record<string, unknown> | undefined, - parseTraitList(process.env.SESSION_EXTRA_TRAITS_ACCESS_TOKEN), + EXTRA_TRAITS_ACCESS_TOKEN, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/consent.ts` around lines 15 - 34, parseTraitList is being recomputed on every consent request even though process.env values are static for the process lifetime. Move the parseTraitList(process.env.SESSION_EXTRA_TRAITS_ID_TOKEN) and parseTraitList(process.env.SESSION_EXTRA_TRAITS_ACCESS_TOKEN) calls to module scope in src/routes/consent.ts, keep using parseTraitList and copyTraitsInto for locating the logic, and reuse the cached arrays at the call sites instead of parsing inside the request path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/routes/consent.ts`:
- Around line 87-102: The trait-copying logic in consent handling applies
operator-configured extras to both tokens without any OIDC scope gate, so update
the project documentation to explicitly describe this behavior and its
consent/privacy implications. Reference the consent token enrichment flow in
copyTraitsInto, session.id_token, session.access_token, and parseTraitList so
operators understand that SESSION_EXTRA_TRAITS_ID_TOKEN and
SESSION_EXTRA_TRAITS_ACCESS_TOKEN are scope-independent before enabling them.
---
Nitpick comments:
In `@README.md`:
- Around line 59-68: Update the README entry for SESSION_EXTRA_TRAITS_ID_TOKEN
and SESSION_EXTRA_TRAITS_ACCESS_TOKEN to explicitly state that these extra
traits are propagated unconditionally, independent of the OIDC scopes requested
by the client. Keep the note close to the existing description in README.md and
contrast it with the built-in email/profile claim mappings so operators
understand the scope-independent behavior.
In `@src/routes/consent.ts`:
- Around line 15-34: parseTraitList is being recomputed on every consent request
even though process.env values are static for the process lifetime. Move the
parseTraitList(process.env.SESSION_EXTRA_TRAITS_ID_TOKEN) and
parseTraitList(process.env.SESSION_EXTRA_TRAITS_ACCESS_TOKEN) calls to module
scope in src/routes/consent.ts, keep using parseTraitList and copyTraitsInto for
locating the logic, and reuse the cached arrays at the call sites instead of
parsing inside the request path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b3b2265c-913b-4e0f-91c1-e2b24ff8250a
📒 Files selected for processing (2)
README.mdsrc/routes/consent.ts
|
|
||
| // Propagate additional identity traits into the session tokens, based on | ||
| // operator-provided allowlists. Applied unconditionally (no scope gate) so | ||
| // downstream apps get the trait even when the OIDC client requested only | ||
| // openid. Missing traits are skipped silently. | ||
| copyTraitsInto( | ||
| session.id_token as Record<string, unknown>, | ||
| identity.traits as Record<string, unknown> | undefined, | ||
| parseTraitList(process.env.SESSION_EXTRA_TRAITS_ID_TOKEN), | ||
| ) | ||
| copyTraitsInto( | ||
| session.access_token as Record<string, unknown>, | ||
| identity.traits as Record<string, unknown> | undefined, | ||
| parseTraitList(process.env.SESSION_EXTRA_TRAITS_ACCESS_TOKEN), | ||
| ) | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Security consideration: traits propagate unconditionally regardless of OIDC scope.
The code explicitly copies extra traits into id_token and access_token without any scope check. This means any OIDC client — even one requesting only openid — will receive all operator-configured traits (e.g., groups, tenant_id) in its tokens. While this is by design per the PR objectives, operators should be aware that these traits bypass the standard scope-based consent model. Consider documenting this scope-independent behavior in the README so operators understand the privacy implications before enabling it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/consent.ts` around lines 87 - 102, The trait-copying logic in
consent handling applies operator-configured extras to both tokens without any
OIDC scope gate, so update the project documentation to explicitly describe this
behavior and its consent/privacy implications. Reference the consent token
enrichment flow in copyTraitsInto, session.id_token, session.access_token, and
parseTraitList so operators understand that SESSION_EXTRA_TRAITS_ID_TOKEN and
SESSION_EXTRA_TRAITS_ACCESS_TOKEN are scope-independent before enabling them.
…less of OIDC scope
2 participants
Problem
extractSessioninsrc/routes/consent.tshardcodes which Kratos identity attributes are propagated into the Hydra consent session:email(fromverifiable_addresses[0]) and a handful ofprofileclaims (preferred_username,website,given_name,family_name,name,updated_at). Any other trait defined on the identity is silently dropped when Hydra issues the ID / access token.This bites deployments that store custom traits on the Kratos identity and want them in the JWT for downstream authorization. For example:
groupsextracted from a SAML assertion (via Polis or another SAML-to-OIDC bridge) or upstream OIDCgroupsclaim,department,tenant_id, etc.Today the only workarounds are to fork the UI or to patch
consent.jsin place at pod startup, both of which break on any upstream refactor.Change
Adds two optional env vars:
SESSION_EXTRA_TRAITS_ID_TOKEN: comma-separated list of trait names to copy fromidentity.traitsintosession.id_token.SESSION_EXTRA_TRAITS_ACCESS_TOKEN: same, forsession.access_token.Missing traits are skipped silently. Copying is unconditional on OIDC scope (customers who want a
groupsclaim without also granting theprofilescope shouldn't have to). Values are copied verbatim, so array / string / object shapes all flow through as-is.Example
Kratos identity schema declares a
groupstrait; the OIDC mapper populates it from the upstream provider. Settingon the UI pod gets
"groups": ["engineering", "admins"]into every issued ID token. Downstream apps consume the claim like any other JWT field.Alternatives considered
Env-driven allowlist is the smallest change that covers the "just include this trait" case.
Test plan
tsc --noEmitpassesprettier --checkcleanSESSION_EXTRA_TRAITS_ID_TOKEN=groups, confirmgroupsappears in the ID token issued via a Hydra OAuth2 authorization code flow."test": "exit 0"), otherwise happy to add cases.Summary by CodeRabbit
New Features
Documentation