Skip to content

feat(identity): rewrite mergeUsers to bind, don't delete (Phase 2a)#3715

Merged
bokelley merged 2 commits into
mainfrom
bokelley/merge-bind-not-delete
May 1, 2026
Merged

feat(identity): rewrite mergeUsers to bind, don't delete (Phase 2a)#3715
bokelley merged 2 commits into
mainfrom
bokelley/merge-bind-not-delete

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 1, 2026

Summary

Phase 2a of the identity layer. Confirming an "I have two accounts" link no longer destroys the secondary WorkOS user — both linked emails stay live as real sign-in credentials, both bound to one identity. Reads still work because we still move data to the primary; the auth middleware id-swaps non-primary logins to the canonical user.

Builds on Phase 1 (#3711, merged). Phase 2b will be the admin "create + bind" tool that finally unblocks Ahmed; this PR is the prerequisite that makes the binding actually useful (without id-swap, a non-primary login would land in an empty workspace).

What changes

server/src/db/user-merge-db.ts

  • Drop the WorkOS deleteUser call and the workos parameter (no longer needed).
  • After moving data, re-point the secondary's identity_workos_users row to the primary's identity, marked is_primary = FALSE.
  • Drop the secondary's now-orphan singleton identity row.
  • Drop workos_user_deleted and warnings from UserMergeSummary (deletion no longer happens).

server/src/middleware/auth.ts

  • Extend attachIdentityId to also resolve the identity's primary workos_user_id. When the authenticated user is a non-primary binding, swap req.user.id to the canonical and stash the original on req.user.authWorkosUserId.

server/src/types.ts

  • New WorkOSUser.authWorkosUserId for code that needs the actual auth user (WorkOS API calls, audit logs).
  • Doc comment on id clarifies the new contract: "canonical workos_user_id for app-state queries."

server/src/routes/account-linking.ts

  • Verify-email-link confirmation page: "signing in with either email leads to the same workspace" (was: "this cannot be undone").
  • Drop the getWorkos() arg to mergeUsers.

server/src/http.ts

  • Drop the workos! arg from the Google-alias-merge call site.

Test plan

  • New: merge-bind-not-delete.test.ts (5 tests) — secondary WorkOS user stays alive; both bindings point at one identity; secondary's binding is non-primary; orphan identity is dropped; org_memberships still move to primary; audit row written.
  • Phase 1 tests still pass (identity-layer.test.ts, 6 tests).
  • Adjacent: set-primary-email.test.ts (8), membership-webhook.test.ts (60). 79/79 across all auth/identity/merge.
  • tsc --noEmit clean.
  • Pre-existing flakes in registry-reader-baseline / brand-properties tests confirmed to also fail on main — not introduced here.

Phase plan recap

  • ✅ Phase 1 (feat(identity): identity layer foundation (Phase 1) #3711): identity layer foundation
  • 👉 Phase 2a (this PR): mergeUsers binds, doesn't delete
  • ⏳ Phase 2b: admin "create + bind" tool — unblocks Ahmed directly
  • ⏳ Phase 2c: linked-emails UI reads from identity_workos_users (tells the truth); drop user_email_aliases
  • ⏳ Phase 3: identity_id columns on hot tables (eventual cleanup of the id-swap)

🤖 Generated with Claude Code

Confirming an "I have two accounts" link no longer destroys the secondary
WorkOS user. App-state rows still move to the primary so existing reads
keep working, but the secondary's WorkOS user stays alive — each linked
email remains a real, working sign-in credential. Both WorkOS users end up
bound to one identity (the primary's); the secondary's orphan singleton
identity gets dropped.

Auth middleware: when a non-primary binding signs in, swap req.user.id to
the identity's primary workos_user_id so app-state reads land on the right
person. The actual authenticated WorkOS user is preserved on
req.user.authWorkosUserId for WorkOS API calls and audit logs.

Verify-email-link page copy updated: "signing in with either email leads to
the same workspace" instead of "this cannot be undone."

Phase 2b (admin "create + bind" tool) is the direct fix for the Ahmed
class of escalation; this PR is the prerequisite that makes such a binding
useful (without id-swap, the new gmail user would log in to an empty
workspace).

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

import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest';
import { initializeDatabase, closeDatabase, getPool } from '../../src/db/client.js';
Code review + security review caught real issues:

H1 (confused deputy): admin.ts widget createToken and account-linking.ts
updateUser were passing req.user.id post-id-swap to WorkOS APIs that
operate on the auth credential. Both now use authWorkosUserId ?? id.
Other WorkOS callers (createOrganizationMembership, listOrganization-
Memberships) intentionally keep canonical id — they're identity-scoped,
not credential-scoped.

H2 (silent takeover): the pre-existing email-link confused-deputy attack
(attacker initiates link to victim, victim confirms, attacker becomes
primary) was previously loud because it deleted the victim's WorkOS user.
Bind-not-delete makes it silent and persistent. Block the merge-existing-
account path entirely at initiation (POST /api/me/linked-emails) and
defense-in-depth at verify time. Consolidating two existing accounts is
now admin-only — Phase 2b adds that tool. Bare alias adds (target email
has no existing WorkOS user) are unaffected.

H3 (cache stale): export invalidateSessionsForUsers() from auth and call
it from mergeUsers post-commit. Sessions for either user get evicted so
subsequent requests re-resolve the new binding instead of serving a
stale id-swap.

Plus L2 (cleaner LEFT JOIN in attachIdentityId) and a missing test for
"throws when primary lacks identity binding."

Deferred as follow-ups: M1 (audit-log forensics — record both canonical
and auth ids; touches many sites; not yet load-bearing because no users
have non-singleton identities until Phase 2b ships); M2 (primary-deletion
DoS via WorkOS dashboard — needs webhook-side promote-on-delete logic).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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