Skip to content

feat(identity): identity layer foundation (Phase 1)#3711

Merged
bokelley merged 2 commits into
mainfrom
bokelley/ahmed-kyber1-merge
May 1, 2026
Merged

feat(identity): identity layer foundation (Phase 1)#3711
bokelley merged 2 commits into
mainfrom
bokelley/ahmed-kyber1-merge

Conversation

@bokelley

@bokelley bokelley commented May 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Phase 1 of decoupling person from WorkOS user so that "linked emails" can stop being a local-only fiction.

A WorkOS user is one credential bundle for one email — that's WorkOS's data model and we won't fight it. An identity is the person, and may back multiple WorkOS users (one per email). Today every user is its own singleton identity. Phase 2 will rewrite mergeUsers to bind multiple WorkOS users to one identity instead of deleting the secondary user, so that each linked email is a real WorkOS user that actually works for sign-in and our local DB becomes a truthful cache of WorkOS state.

This PR is no functional change. No app code reads identityId yet.

What's in here

  • Migration 460_identities.sql: adds identities and identity_workos_users, backfills 1:1 from users, installs an AFTER INSERT trigger that auto-creates a singleton identity for any new user. The trigger fires on INSERT only — ON CONFLICT DO UPDATE upserts route through AFTER UPDATE, which is correct (existing users already have an identity).
  • WorkOSUser.identityId?: string on the type (server/src/types.ts).
  • Auth middleware resolves identityId from identity_workos_users and attaches it to req.user for real users (skipped for synthetic admin / WorkOS-API-key users). Resolved once per session miss; cached. JWT and dev-mode paths also resolve.

Why now

Escalation #298 (Ahmed Karim, Kyber1) surfaced that today's mergeUsers deletes the secondary WorkOS user but only records the email locally. The "Linked emails" UI implies you can sign in with any of them — you can't, because WorkOS doesn't know about the alias. The right fix is to stop deleting WorkOS users on merge; instead, bind both to one identity. That requires the identity layer to exist first.

Test plan

  • Migration runs cleanly (verified locally; backfill is set-based via a transient column).
  • Backfill invariant: every existing user has exactly one identity_workos_users row marked primary.
  • Trigger creates a singleton identity for newly inserted users.
  • ON CONFLICT DO UPDATE upsert does not double-fire the trigger.
  • Partial unique index rejects a second primary binding on the same identity.
  • DELETE FROM users cascades to remove the binding.
  • set-primary-email.test.ts and membership-webhook.test.ts still pass (68/68) — no regression in adjacent flows.
  • npx tsc --noEmit clean.

Phase 2 preview (separate PR)

  • Rewrite mergeUsers → bind both WorkOS users to one identity, no deletion.
  • "Linked emails" UI shows real WorkOS users, each working for sign-in.
  • Critical tables (organization_memberships, learner_progress, certification_attempts, person_relationships) start dual-writing identity_id.
  • Admin tool: "create new WorkOS user for <email>, bind to existing identity" — fixes the Ahmed class of escalation directly.

🤖 Generated with Claude Code

bokelley and others added 2 commits April 30, 2026 21:29
… change)

A WorkOS user is one credential bundle for one email; an identity is the
person and may back multiple WorkOS users. Today every user is a singleton
identity. Phase 2 will rewrite mergeUsers to bind multiple WorkOS users to
one identity instead of deleting the secondary user, so that linked emails
actually work for sign-in (each email is a real WorkOS user) and our DB
becomes a truthful cache of WorkOS state.

Migration 460 adds `identities` and `identity_workos_users`, backfills 1:1
from `users`, and installs an AFTER INSERT trigger so any new user gets a
singleton identity automatically. Auth middleware now resolves identityId
on req.user for real users (synthetic admin/API-key users skip it). No app
code reads identityId yet — that comes in Phase 2.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…paths

Code review caught two paths that set req.user without resolving identityId.
optionalAuth was the most important — it shares sessionCache with requireAuth,
so an optionalAuth request would poison subsequent requireAuth cache hits
with identityId=undefined for the cache TTL. Phase 1 has no readers so it's
latent, but Phase 2 would have hit the bug immediately.

Also:
- Extract isSyntheticUser(id) helper so the admin/api-key skip convention is
  single-sourced.
- Drop unused identities.updated_at column (no UPDATE trigger, nothing reads
  it; add it back when Phase 2 actually mutates identities).
- Note "do not serialize identityId to clients" on the type — correlating
  identityId across surfaces would let a client tie multiple emails to one
  person.
- Add scale-threshold comment to migration 460 backfill, matching house style
  in 448.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit 72b4398 into main May 1, 2026
14 checks passed
@bokelley bokelley deleted the bokelley/ahmed-kyber1-merge branch May 1, 2026 01:43
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.

1 participant