feat(custom-oauth): preserve non-standard IdP claims in identity_data#2520
feat(custom-oauth): preserve non-standard IdP claims in identity_data#2520cemalkilic wants to merge 1 commit into
identity_data#2520Conversation
fadymak
left a comment
fadymak
left a comment
There was a problem hiding this comment.
Thanks @cemalkilic — I left a few comments 👀 My hesitation is around persisting all fields to identity data as opposed to having some allow list that can be configured per-provider which feels like the cleaner/safer approach.
| if err := idTokenObj.Claims(&raw); err == nil { | ||
| captureCustomClaims(raw, userData.Metadata) | ||
| } |
There was a problem hiding this comment.
I believe this would cause us to re-add any claims we explicitly excluded for certain providers (e.g.: removeAzureClaimsFromCustomClaims):
auth/internal/api/provider/oidc.go
Lines 307 to 327 in be317c1
| // standardClaimKeys is the set of JSON keys handled by the typed Claims | ||
| // struct (OIDC standard claims plus our typed extensions, and the | ||
| // custom_claims sink itself). Derived from the struct's json tags so it | ||
| // can't drift when Claims changes. | ||
| var standardClaimKeys = jsonKeysOf(reflect.TypeOf(Claims{})) |
There was a problem hiding this comment.
My concern is that Claims doesn't capture actual standard claims (including SB-specific ones), e.g.: nbf, nonce, etc... which would end up in the custom claims and it doesn't feel like the right place, nor should they be persisted IMO
|
Thanks again @cemalkilic , I wanted to add that we were running into this problem as well, so it's great to see this addressed.
-- @fadymak I understand this hesitation, we would still love to see this as is or with a allow list (or even better claims mapping like with saml attribute mapping) that can be set by us by for example using the createProvider method, this because we are using a standardized IdP proxy for our market and they along with many others use non standard claims like mail, sn, nlEduPersonProfileId which we need in our before insert on auth.users trigger. Hitting the userinfo endpoint ourselves with the token would mean we lose transactional integrity which is a big reason we wanted to go with Supabase auth. |
|
Sorry for the delay here @IdrisCelik ! Closing in favor of #2576 |
No worries @cemalkilic we are really happy with your work. Thank you very much! |
## What kind of change does this PR introduce?
Feature
## What
Adds a per-provider `custom_claims_allowlist` to custom OAuth/OIDC
providers: a flat list of raw IdP claim keys that get copied verbatim
into `custom_claims` on the user's `identity_data` /
`raw_user_meta_data`.
```
PATCH /admin/custom-providers/custom:acme
{ "custom_claims_allowlist": ["groups", "org_id", "mail", "sn"] }
Result, queryable in a before insert on auth.users trigger (and stored
in the auth.identities table as well for future queries):
"custom_claims": { "groups": [...], "org_id": "...", "mail": "...",
"sn": "..." }
```
## Why
Admins integrating non-standard IdPs need to read provider-specific claims (e.g. groups, mail, nlEduPersonProfileId) that don't map to standard fields. This is the allowlist design rather than a denylist (previous implementation: #2520):
- No Azure "re-add stripped claims" risk: we only copy keys explicitly named, so a parser that strips a claim stays authoritative.
- No reflection, no exhaustive exclusion set to maintain, no ordering hazard with ParseIDToken.
Design decisions
- Default = empty -> capture nothing. Opt-in only.
- text[] column (slices.String), matching `scopes` / `acceptable_client_ids`.
- Distinct from attribute_mapping: the allowlist copies raw source keys into the opaque custom_claims bucket; attribute_mapping remaps typed fields. No privilege-escalation surface, so no blocked-target guard (only a non-empty-entry check).
- Capture runs before applyAttributeMapping at all sources (OAuth userinfo, OIDC userinfo) via one captureAllowedClaims helper.
3 participants
Summary
groups,roles,org_id,tenant_id, …) inauth.identities.identity_dataandauth.users.raw_user_meta_dataunder thecustom_claimskey. Previously these were silently dropped by the typedClaimsdecode.CustomOAuthProviderandCustomOIDCProvider. Built-in providers (Google, Apple, GitHub, Azure, …) are unchanged.Why
userinfoclaims they rely on (org IDs, tenant IDs, group / role lists from their custom OIDC IdPs) were not appearing in their Postgres auth schema. They want to use these claims transactionally, reject sign-in if a required claim is missing, and keep that data inside Supabase rather than re-fetching out-of-band. The IdP already returns the data and we already verify it during callback; we just weren't persisting it.