Skip to content

feat(identity): admin "bind sign-in email" tool (Phase 2b) — fixes Ahmed#3722

Merged
bokelley merged 5 commits into
mainfrom
bokelley/admin-bind-email
May 1, 2026
Merged

feat(identity): admin "bind sign-in email" tool (Phase 2b) — fixes Ahmed#3722
bokelley merged 5 commits into
mainfrom
bokelley/admin-bind-email

Conversation

@bokelley

@bokelley bokelley commented May 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Phase 2b of the identity layer. Adds an admin endpoint and UI button that creates a fresh WorkOS user for a given email and binds it as a non-primary credential under an existing user's identity. Direct fix for the Ahmed-class escalation (lost sign-in via gmail after the old delete-the-secondary merge flow).

Builds on:

What changes

POST /api/admin/users/:userId/linked-emails

  • Validates the new email + verifies the existing user
  • Refuses if the email already has an AAO account (higher-risk consolidation tracked in Identity layer: redesign self-service merge-existing-accounts UX #3719)
  • Calls `workos.userManagement.createUser` to create a fresh WorkOS user at the new email
  • Inserts into local `users` (Phase 1 trigger creates a singleton identity)
  • Calls `mergeUsers` to re-point the new user's binding under the existing user's identity (no data movement; the new user has nothing)
  • Returns the binding info

Admin UI

  • "Add sign-in email" button on `/admin/people` detail panel for users with a `workos_user_id`
  • Two-step prompt: enter email, then confirm

Trust model

  • Admin asserts the email belongs to the person (no verification email sent)
  • Endpoint logs the bind to `registry_audit_log`-equivalent via `mergeUsers`'s existing `merge_user` audit row
  • Same protection as the rest of the admin surface (`requireAdmin` middleware)

How this fixes Ahmed

  1. Brian opens `/admin/people`, searches "Ahmed Karim"
  2. Clicks his row → detail panel
  3. Clicks "Add sign-in email"
  4. Enters `akarim.250@gmail.com` → confirms
  5. Endpoint creates a fresh WorkOS user for gmail, binds it to Ahmed's identity
  6. Ahmed signs in with gmail magic-link → WorkOS auth → identity resolves to his existing identity → swap → he sees Kyber1

Local DB: gmail's WorkOS user has zero app-state rows; the swap routes him to his canonical (kyber1.com) WorkOS user where all his data lives.

Test plan

  • New: `admin-bind-email.test.ts` (5 tests) — happy path, invalid email, already-primary, already-claimed, user-not-found
  • Phase 1 + 2a tests still pass: `identity-layer.test.ts` (6), `merge-bind-not-delete.test.ts` (6). 17/17 across all identity tests.
  • `tsc --noEmit` clean.

Phase plan recap

Follow-ups filed: #3717 (audit forensics), #3718 (primary deletion DoS), #3719 (self-service merge UX redesign).

🤖 Generated with Claude Code

POST /api/admin/users/:userId/linked-emails creates a fresh WorkOS user
for the given email and binds it under the existing user's identity as a
non-primary credential. After this, the user can sign in with either
email — the auth middleware id-swaps non-primary logins to the canonical
workos_user_id so they see the same workspace.

UI: "Add sign-in email" button on the admin /admin/people detail panel
for users that have a workos_user_id.

Trust model: admin asserts the email belongs to the person. The endpoint
refuses if the email is already an existing AAO account (higher-risk
consolidation path; tracked in issue #3719).

Direct fix for Ahmed-class escalations: admin opens his person detail,
clicks "Add sign-in email," enters gmail, done.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
process.env.WORKOS_COOKIE_PASSWORD ??= 'test-cookie-password-at-least-32-chars-long';
});
import request from 'supertest';
import { initializeDatabase, closeDatabase, getPool } from '../../src/db/client.js';
Comment thread server/src/routes/admin/users.ts Fixed
Security review caught three real improvements (no blockers):

1. Partial-failure leak: if local INSERT or mergeUsers throws after WorkOS
   createUser succeeded, the WorkOS user was orphaned and a retry would
   silently create a new one. Now best-effort deleteUser cleanup, with
   the WorkOS id only surfaced when cleanup itself fails.

2. Admin UI confirmation: the bind action grants permanent sign-in
   access. Strengthen the prompt to require typing the target user's
   existing primary email to confirm. The button now shows whose
   account it'll bind to (display name + email). Avoids the
   "panel-open-on-the-wrong-row" footgun.

3. Test coverage gaps: add a test for the WorkOS 422 path (existing
   account in WorkOS that's not in our local DB) and the partial-failure
   rollback path.

Deferred to follow-up: add CREATE UNIQUE INDEX CONCURRENTLY users_email
_lower_unique. WorkOS's own gate currently makes the local TOCTOU
unexploitable end-to-end, but the local check shouldn't depend on that.

Also fixed the onclick HTML-injection footgun by switching to data-*
attributes + event listener instead of inline JSON.stringify.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley and others added 3 commits April 30, 2026 22:54
CI on Phase 2b PR #3722 caught two pre-existing issues:

1. main has two migrations numbered 459 — 459_addie_escalation_dedup_key
   (mine, from earlier work) and 459_create_type_reclassification_log
   (from #3567, merged independently). Rename the latter to 461 (Phase 1's
   460_identities is taken). Migration uses CREATE TABLE / INDEX IF NOT
   EXISTS so it's idempotent on systems that already applied it as 459.

2. CodeQL flagged the email-validation regex
   /^[^\s@]+@[^\s@]+\.[^\s@]+$/ as polynomial (overlap between [^\s@]+
   and the literal \. allows backtracking). Replace with a linear
   isPlausibleEmail helper. WorkOS validates real syntax upstream — this
   is just a shape check.

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

The Phase 1 identity-layer test asserted "every user has a primary
binding," which was correct in Phase 1 (singleton identities only). After
Phase 2a's mergeUsers rewrite and Phase 2b's admin bind tool, bound users
intentionally have is_primary=FALSE — so on CI when all tests run
together, the global invariant query saw the bind-email test's freshly-
bound non-primary user and fired a false negative.

Loosen to two scoped invariants:
  - every user has exactly one binding
  - every identity has exactly one primary binding

The partial unique index idx_identity_workos_users_one_primary already
enforces the second at the DB level; keeping it as a test gives us a
clear failure if the index is ever dropped.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit 61b1e4a into main May 1, 2026
13 of 14 checks passed
@bokelley bokelley deleted the bokelley/admin-bind-email branch May 1, 2026 03:07
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