Skip to content

fix: preserve raw claims for native Microsoft sign-in - #19

Merged
gn-kim-k merged 2 commits into
GoodNotes:goodnotesfrom
gn-kim-k:fix/microsoft-native-raw-claims
Oct 20, 2026
Merged

gn-kim-k merged 2 commits into
GoodNotes:goodnotesfrom
gn-kim-k:fix/microsoft-native-raw-claims

Conversation

@gn-kim-k

@gn-kim-k gn-kim-k commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Microsoft native id_token verification currently decodes only the typed Claims struct. Provider-specific fields such as xms_edov are discarded, so Jsonnet mappers cannot read claims.raw_claims.xms_edov, although the browser callback already exposes it.

Port upstream ory/kratos 373a2e6 (feat: more extension points (#4272)): verifyToken always decodes the verified token into RawClaims, so token_verifier.go matches upstream. Native Google and Apple verification now also return raw claims, as they do upstream.

Related issue(s)

Reproduced with locally signed tokens: before this patch, the same token produces populated raw claims through the Microsoft browser path and RawClaims: nil through native verification. The nine new raw-claim cases and the browser/native comparison fail before the fix and pass afterward.

Validation

Executed with Go 1.26.1 on Linux arm64 (WSL), with SQLite/CGO:

  • go test -tags sqlite ./selfservice/strategy/oidc -run 'Test(MicrosoftVerify|MicrosoftClaims|GoogleVerify|AppleVerify)$' -count=1 — passed.
  • go test -tags sqlite -short ./selfservice/strategy/oidc -skip '^TestPostEndpointRedirect$' -count=1 — passed.
  • goimports on changed Go files and git diff --check — passed.

Coverage includes boolean/string/numeric and absent xms_edov, JSON-serialized mapper input, browser/native parity with subject_source: oid, Google/Apple raw claims, and rejection of expired tokens. Existing signature, audience, issuer, tenant, additional-audience and subject tests also pass.

The initial broader run encountered Windows CRLF snapshot differences and a missing Docker socket. Unchanged snapshot files were normalized to LF locally (no snapshot diff). TestPostEndpointRedirect requires Hydra/Docker even in short mode and was explicitly excluded; other service-backed scenarios are skipped by short mode. Full integration tests were not run.

Compatibility review

  • Audited all three verifyToken callers (Apple, Google, Microsoft): all now return raw claims. Native claims reach only the Jsonnet mapper input and the claim-key span attribute; nothing stores them.
  • Browser decoding, token validation, nonce handling and subject selection are unchanged.
  • Applies to all native Microsoft provider configurations. Mappers can now observe the verified raw payload; no claim normalization or mapper-policy changes are included.
  • Existing identities are not remapped or backfilled by this patch.

Checklist

  • Contributing guidelines and security policy reviewed.
  • Regression tests added and run.
  • Code formatted according to the Go import conventions.
  • This fixes missing mapper input, not a security vulnerability.
  • Behavior and scope documented in code comments and this description; no new feature design required.

Further Comments

This PR contains the decoder fix and tests only. Deployment and any repair of previously stored identity metadata are separate work.

https://claude.ai/code/session_0172mHvWYKYpy7qvrugERAjV

Replace the preserveRawClaims flag with the hunk from upstream
ory/kratos 373a2e6 (feat: more extension points (ory#4272)), so that
token_verifier.go matches upstream and a later rebase applies cleanly.
Native Apple and Google verification now also return raw claims.

Claude-Session: https://claude.ai/code/session_0172mHvWYKYpy7qvrugERAjV
@gn-kim-k
gn-kim-k merged commit fb66b66 into GoodNotes:goodnotes Oct 20, 2026
4 of 14 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.

2 participants