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. |
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.