Conversation
7e06a8b to
e0d0228
Compare
e0d0228 to
ce12016
Compare
d4036bd to
c7f6afb
Compare
|
Tracked in #87. |
c7f6afb to
232dcd1
Compare
Both packages are pinned exactly so patch-level claim changes cannot float in noticed. The lockfile diff is limited to the better-auth family (1.7.5 -> 1.7.7) plus one `@types/node` patch float from peer materialisation; no unrelated runtime deps move.
Users signing in through the auth app could be dropped into the planner's new-member flow, losing their subscriptions and roles: the planner resolves members by the id_token `email` claim and falls back to `sub` (the better-auth user id) when the claim is absent, and better-auth 1.7 stopped including user-record claims for sessions without provider data. Two duplicate members were created in production on 2026-10-02. `customIdTokenClaims` now always emits `email` and `name` from the user record, keeping `github_id` for returning-member resolution. The claims builder lives in `src/auth/id-token-claims.js` and is shared with the test helper, so the regression tests exercise the code production runs; the test instance is aligned with the production scopes list. Tests cover the magic-link path (email/name asserted against the user record) and the linked-GitHub path (all three claims in one payload), and the oauth-flow test now registers tap teardown so it stops leaking a schema per run.
The marker is a working note, not documentation. Keep the explanation of why `skipStateCookieCheck` is set.
232dcd1 to
4f32edd
Compare
till
left a comment
There was a problem hiding this comment.
Would this be fixed if the planner requested email explicitly?
No. Me and my synthetic research assistant checked the 1.7.7 source to be sure |
|
TL;DR: The planner already requests Why requesting
|
Users signing in through the auth app could be dropped into the planner's new-member flow, losing their subscriptions and roles, because the planner received an id_token without an
emailclaim and fell back tosub(the better-auth user id). This fix makes every id_token carry the user's email and name, and bumps better-auth to 1.7.7.customIdTokenClaimsnow always emitsemailandnamefrom the user record, for every session type (previously it only returnedgithub_id)Background and mechanism
The planner's codebar OmniAuth strategy resolves the member by the id_token's
emailclaim, falling back tosubwhen the claim is absent:better-auth 1.7 stopped including user-record claims (
email,name) in the id_token for sessions not created through a social provider. A user who signed in by magic link therefore got a planner member whose email and name were the better-auth user id — an empty account with no subscriptions or roles. Two such duplicate members were created in production on 2026-10-02; the incident was mitigated by rolling back the auth deploy.The missing user-record claims affect any session without provider data; the explicit claims in
customIdTokenClaimsare spread after the library's default claims, so they are always present.A separate integration test reproduces the scenario: magic-link session → authorize → token exchange → id_token claims. Under 1.7.5/1.7.7 without this change, the
emailandnameclaims are absent; with it, they match the user record.Review notes
Focus first on the claims semantics: every id_token now carries the better-auth user-record email. For users whose GitHub primary email differs from their stored planner email, the planner now matches (or keys a new member) on the user-record email, with the
github_idclaim still available to resolve returning members. Check that interplay for accounts where the two emails diverge.Also worth a look: the lockfile diff is now minimal. Only the better-auth family moves 1.7.5 to 1.7.7 (plus one @types/node patch float from peer materialisation), and better-auth is pinned exactly to 1.7.7 to match the @better-auth/oauth-provider pin. Review fixes landed on top: the claims builder is shared between src/auth.js and the test helper, the linked-GitHub test asserts email and name, and the oauth-flow test registers teardown.
Deliberately not done here: hardening the planner's
|| payload['sub']fallback (separate planner change), and setting the client'sscopescolumn in the seed (authorize falls back to the plugin's allowed scopes).