diff --git a/README.md b/README.md index d9f4f91..fd5f695 100644 --- a/README.md +++ b/README.md @@ -534,6 +534,20 @@ The emulator issues a new refresh token on every refresh and invalidates the one `/sso/token` is OAuth-shaped throughout, matching its spec definition. +### Magic Auth doubles as sign-up + +`POST /user_management/magic_auth` creates the user when the email has none, so a sign-up flow needs no separate `POST /user_management/users` first. Production does the same at code-creation time rather than at authenticate: the 201 already carries a `user_id`, the user is immediately listable with `email_verified: false`, and the email it sends uses the "Sign up" template. + +Redeeming a Magic Auth code sets `email_verified` to `true`, matching the live authenticate response for the same flow. This applies to **any** user who was not already verified, not only ones the endpoint just created, so a fixture seeded `email_verified: false` comes back verified after its first Magic Auth login. + +An email is resolved case-insensitively (`User@x.test` and `user@x.test` are the same account, stored under whichever case created it), and one that could only be a typo is rejected rather than turned into an account nothing can reach. `POST /user_management/users` applies both — so it answers 409 for an address that differs from an existing one only in case, rather than creating a second account no lookup can tell apart from the first. + +Every lookup by email is case-insensitive, not just Magic Auth's, so an account created by a Magic Auth sign-up is reachable by whatever casing the caller has: the password grant, `login_hint` on the authorize endpoints, `POST /user_management/password_reset`, the `email` filter on `GET /user_management/users` and `GET /user_management/invitations`, accepting an invitation, the `user_id` on SSO authentication events, the profile `/sso/authorize` resolves from a `login_hint`, and the email a seeded organization membership joins its user by. Seeded `users` are held to the same uniqueness the API enforces — two entries differing only in case are a config error, since a seed was otherwise the one way left to produce the pair of accounts no lookup can tell apart. + +A field named `email` must be a string wherever it is accepted. A number or object is a `400` (`422` on `POST /user_management/users` and `POST /user_management/invitations`, which keep those routes' validation shape) naming the type, distinct from the `email is required` reported for one that is genuinely absent — which includes an explicit `null`, since that is how a JSON body spells absence. Addresses are trimmed before they are stored or compared, so a padded copy of an address finds the account written under it. + +The typo guard applies wherever an address is written rather than looked up: both routes that create users, `POST /user_management/invitations` (acceptance resolves the recipient by email, so a typo is an invitation that is spent without enrolling anyone), and seeded `users`, `invitations`, and organization `memberships`. Read paths are left alone — an address that resolves to nothing is still a `404` you can act on, not a validation error. + ### Emitted events Authentication events carry the spec payload `{ type, status, user_id, email, ip_address, user_agent }` (plus `error` on failures and `sso` details on SSO events). diff --git a/src/workos/config-validator.ts b/src/workos/config-validator.ts index 1a1e38a..e1d1f23 100644 --- a/src/workos/config-validator.ts +++ b/src/workos/config-validator.ts @@ -3,6 +3,20 @@ */ import type { WorkOSSeedConfig } from './index.js'; import { validateJwtTemplateContent } from './jwt-template.js'; +import { normalizeEmail, type NormalizedEmail } from './helpers.js'; + +/** + * A seed is the one creation path that does not go through a route, so it is held to what the + * routes enforce: an address is trimmed and is shaped like an address. Anything looser and a seed + * is the remaining way to write a user under a spelling no lookup by email resolves — which is the + * state all of this exists to prevent. + * + * Returns the stored form, or the problem for the caller to word in its own terms: each site + * already says something more specific than "email" about what the address is for. + */ +function seedEmail(value: unknown): NormalizedEmail { + return normalizeEmail(value, { requireShape: true }); +} /** * A pinned id is addressed as a single path segment (`/organizations/:id`, @@ -28,10 +42,17 @@ export function validateSeedConfig(config: WorkOSSeedConfig): ConfigValidationRe const errors: ConfigValidationError[] = []; // Seeded user ids are generated at insert time, so org memberships reference users - // by email — collect the emails defined in this config for cross-referencing. + // by email — collect the emails defined in this config for cross-referencing. Normalized the way + // the store resolves them: lowercased, because every lookup by email is case-insensitive, and + // trimmed, because that is the form seeding writes. A membership for 'a@x.test' names the user + // seeded as ' A@x.test ', and resolving it any other way would reject a reference the running + // emulator then honours. const userEmails = new Set( Array.isArray(config.users) - ? config.users.map((u) => u.email).filter((e): e is string => typeof e === 'string') + ? config.users + .map((u) => seedEmail(u.email)) + .filter((r): r is { ok: true; email: string } => r.ok) + .map((r) => r.email.toLowerCase()) : [], ); @@ -45,10 +66,17 @@ export function validateSeedConfig(config: WorkOSSeedConfig): ConfigValidationRe }); } else { config.users.forEach((user, index) => { - if (!user.email || typeof user.email !== 'string') { + const email = seedEmail(user.email); + if (!email.ok) { errors.push({ path: `users[${index}].email`, - message: 'email is required and must be a string', + message: + email.problem === 'malformed' + ? // Same standard as the two routes that create users: an address that could only + // be a typo becomes an account nothing can reach, and a seed is the one creation + // path with no route in front of it to say so. + 'email must be a valid email address' + : 'email is required and must be a string', value: user.email, }); } @@ -76,18 +104,23 @@ export function validateSeedConfig(config: WorkOSSeedConfig): ConfigValidationRe }); // Email is the lookup key org memberships join on; duplicates would silently - // bind a membership to the first match. + // bind a membership to the first match. Compared case-insensitively, matching the + // uniqueness the API enforces: `POST /user_management/users` answers 409 for an address + // differing only in case, so a seed that got two through would be the one way left to + // manufacture the pair of accounts no lookup by email can tell apart. const seenEmails = new Set(); config.users.forEach((user, index) => { - if (!user.email || typeof user.email !== 'string') return; - if (seenEmails.has(user.email)) { + const email = seedEmail(user.email); + if (!email.ok) return; + const normalized = email.email.toLowerCase(); + if (seenEmails.has(normalized)) { errors.push({ path: `users[${index}].email`, message: 'email must be unique across users', value: user.email, }); } - seenEmails.add(user.email); + seenEmails.add(normalized); }); // A pinned user id is the primary key in the store; two users sharing one would @@ -179,7 +212,8 @@ export function validateSeedConfig(config: WorkOSSeedConfig): ConfigValidationRe // The pre-rename key: it read as "pass a user_... id", which can never // resolve (ids are generated at startup) — point at `email` instead. const legacyUserId = (membership as { user_id?: unknown }).user_id; - if (!membership.email || typeof membership.email !== 'string') { + const memberEmail = seedEmail(membership.email); + if (!memberEmail.ok) { if (legacyUserId !== undefined) { errors.push({ path: `organizations[${index}].memberships[${mIndex}].user_id`, @@ -190,11 +224,14 @@ export function validateSeedConfig(config: WorkOSSeedConfig): ConfigValidationRe } else { errors.push({ path: `organizations[${index}].memberships[${mIndex}].email`, - message: 'email is required and must be the email of a user defined in users', + message: + memberEmail.problem === 'malformed' + ? 'email must be a valid email address' + : 'email is required and must be the email of a user defined in users', value: membership.email, }); } - } else if (!userEmails.has(membership.email)) { + } else if (!userEmails.has(memberEmail.email.toLowerCase())) { // A dangling reference would seed a membership whose embedded user // cannot resolve, which membership serialization rejects. errors.push({ @@ -394,10 +431,16 @@ export function validateSeedConfig(config: WorkOSSeedConfig): ConfigValidationRe }); } else { config.invitations.forEach((inv, index) => { - if (!inv.email || typeof inv.email !== 'string') { + const email = seedEmail(inv.email); + if (!email.ok) { errors.push({ path: `invitations[${index}].email`, - message: 'email is required and must be a string', + message: + email.problem === 'malformed' + ? // As POST /user_management/invitations now answers: acceptance resolves the + // recipient by this address, so a typo is an invitation that enrolls nobody. + 'email must be a valid email address' + : 'email is required and must be a string', value: inv.email, }); } diff --git a/src/workos/helpers.ts b/src/workos/helpers.ts index 5589fce..e3fae22 100644 --- a/src/workos/helpers.ts +++ b/src/workos/helpers.ts @@ -1,7 +1,14 @@ import { randomBytes, createHash, createCipheriv } from 'node:crypto'; import { isIPv6 } from 'node:net'; import { domainToASCII } from 'node:url'; -import { WorkOSApiError, generateId, type CursorPaginatedResult, type Entity, type Store } from '../core/index.js'; +import { + WorkOSApiError, + validationError, + generateId, + type CursorPaginatedResult, + type Entity, + type Store, +} from '../core/index.js'; import { EVENTS, STORE_KEYS, type AuthenticationEventData, type WorkOSEventName } from './constants.js'; import type { WorkOSStore } from './store.js'; import type { EventBus } from './event-bus.js'; @@ -337,6 +344,106 @@ export function generateCode(): string { return String(Math.floor(100000 + Math.random() * 900000)); } +/** + * Whether a string is shaped enough like an email to be worth storing. Deliberately loose — + * the emulator is not an address validator, it just refuses input that could only be a typo. + */ +export function isEmailShaped(value: string): boolean { + const at = value.indexOf('@'); + return at > 0 && at === value.lastIndexOf('@') && at < value.length - 1 && !/\s/.test(value); +} + +/** Why a supplied `email` cannot be used, kept apart so a caller can report which one happened. */ +export type EmailProblem = 'missing' | 'not_a_string' | 'malformed'; + +const EMAIL_PROBLEM_MESSAGES: Record = { + missing: 'email is required', + not_a_string: 'email must be a string', + malformed: 'email must be a valid email address', +}; + +/** The `errors[].code` each problem carries on the routes that report a 422. */ +const EMAIL_PROBLEM_FIELD_CODES: Record = { + missing: 'required', + not_a_string: 'invalid_type', + malformed: 'invalid', +}; + +export type NormalizedEmail = { ok: true; email: string } | { ok: false; problem: EmailProblem }; + +/** + * Normalize a request's `email` to a trimmed string, or say why it can't be. Not every value + * handed to this is normalizable, so the problem comes back as a value rather than an exception: + * the routes disagree about how to report it — `validationError`'s 422 on the user-management + * CRUD routes, a 400 `invalid_request` on the grants — and only about that. + * + * Callers used to type-assert instead, which was survivable while a lookup by email could only + * miss — `findOneBy` returns undefined for a number as readily as for an unknown address. + * Resolving case-insensitively means calling `toLowerCase` on it, so the same assertion throws and + * a malformed request comes back a 500 that tells the caller nothing. + * + * `null` is `missing`, not `not_a_string`: in a JSON body it is how a caller spells absence, and + * the guard exists to name what the caller must fix, not what `typeof` says. + * + * `requireShape` is for the routes that create something from the address rather than look one up. + * A read that misses is a 404 the caller can act on; a write that stores a typo is an account or + * an invitation nothing can ever reach. Read paths leave it off, so an address that does not + * resolve still 404s rather than changing error shape. + */ +export function normalizeEmail(value: unknown, opts?: { requireShape?: boolean }): NormalizedEmail { + if (value === undefined || value === null) return { ok: false, problem: 'missing' }; + if (typeof value !== 'string') return { ok: false, problem: 'not_a_string' }; + const email = value.trim(); + if (!email) return { ok: false, problem: 'missing' }; + if (opts?.requireShape && !isEmailShaped(email)) return { ok: false, problem: 'malformed' }; + return { ok: true, email }; +} + +/** + * The trimmed `email` from a request body, as a route that reports 400 `invalid_request` wants it. + * + * Absence comes back as `''` rather than throwing, because the grants name it alongside whatever + * else they also require ("code and email are required") — a message that is more use than one + * field at a time. + */ +export function requireEmailString(value: unknown, opts?: { requireShape?: boolean }): string { + const result = normalizeEmail(value, opts); + if (result.ok) return result.email; + if (result.problem === 'missing') return ''; + throw new WorkOSApiError(400, EMAIL_PROBLEM_MESSAGES[result.problem], 'invalid_request'); +} + +/** + * The trimmed `email` from a request body, as the user-management CRUD routes want it: a 422 with + * the per-field code, which is the validation shape those routes already answer in. + */ +export function requireEmailField(value: unknown, opts?: { requireShape?: boolean }): string { + const result = normalizeEmail(value, opts); + if (result.ok) return result.email; + throw validationError(EMAIL_PROBLEM_MESSAGES[result.problem], [ + { field: 'email', code: EMAIL_PROBLEM_FIELD_CODES[result.problem] }, + ]); +} + +/** + * Whether two addresses name the same account. Case-insensitive, like every lookup by email, and + * trimmed for the same reason storage is: a padded copy of an address names the same person, and a + * filter that skipped the trim would not return what creation had just written. + */ +export function emailsMatch(a: string, b: string): boolean { + return a.trim().toLowerCase() === b.trim().toLowerCase(); +} + +/** + * Look a user up by email, ignoring case. `findOneBy` is an exact-match index lookup, which is + * fine for a read but forks the account in two anywhere a miss creates a user instead. + */ +export function findUserByEmail(ws: WorkOSStore, email: string): WorkOSUser | undefined { + const exact = ws.users.findOneBy('email', email); + if (exact) return exact; + return ws.users.all().find((u) => emailsMatch(u.email, email)); +} + /** * Hash password using SHA256. * NOTE: This is intentionally weak for emulator/testing only. diff --git a/src/workos/index.ts b/src/workos/index.ts index 91ab0d3..3547645 100644 --- a/src/workos/index.ts +++ b/src/workos/index.ts @@ -63,6 +63,7 @@ import { formatApiKeyRecord, formatFeatureFlag, generateClientId, + findUserByEmail, } from './helpers.js'; import type { WorkOSConnectionType, @@ -275,7 +276,10 @@ export function seedFromConfig(store: Store, _baseUrl: string, config: WorkOSSee ws.users.insert({ object: 'user', id: userConfig.id, - email: userConfig.email, + // Trimmed, as both routes that create users store it: a padded seed would otherwise be + // written under a spelling no lookup by email resolves. validateSeedConfig normalizes the + // same way, so what it cross-referenced is what lands here. + email: userConfig.email.trim(), name: userConfig.name ?? null, first_name: userConfig.first_name ?? null, last_name: userConfig.last_name ?? null, @@ -325,8 +329,8 @@ export function seedFromConfig(store: Store, _baseUrl: string, config: WorkOSSee // insert time, so an id literal in a config could never resolve, and a // dangling membership would break membership serialization (which requires // a resolvable embedded user). validateSeedConfig guarantees the reference - // matches a seeded user. - const memberUser = ws.users.findOneBy('email', mm.email); + // matches a seeded user — case-insensitively, as it does here. + const memberUser = findUserByEmail(ws, mm.email); if (!memberUser) { throw new Error(`Seed membership references unknown user '${mm.email}' (organization '${orgConfig.name}')`); } @@ -436,7 +440,7 @@ export function seedFromConfig(store: Store, _baseUrl: string, config: WorkOSSee const token = generateVerificationToken(); ws.invitations.insert({ object: 'invitation', - email: invConfig.email, + email: invConfig.email.trim(), state: 'pending', token, accept_invitation_url: `${_baseUrl}/user_management/invitations/accept?token=${token}`, diff --git a/src/workos/routes/auth.spec.ts b/src/workos/routes/auth.spec.ts index b7fdb9a..30a465b 100644 --- a/src/workos/routes/auth.spec.ts +++ b/src/workos/routes/auth.spec.ts @@ -3,6 +3,7 @@ import { createServer, type ApiKeyMap } from '../../core/index.js'; import { workosPlugin } from '../index.js'; import { getWorkOSStore } from '../store.js'; import { STORE_KEYS } from '../constants.js'; +import { hashPassword } from '../helpers.js'; import type { Store } from '../../core/index.js'; const apiKeys: ApiKeyMap = { sk_test_auth: { environment: 'test' } }; @@ -784,6 +785,212 @@ describe('Auth routes', () => { expect(body.authentication_method).toBe('MagicAuth'); }); + it('rejects a non-string email on magic auth creation', async () => { + const res = await req('/user_management/magic_auth', { + method: 'POST', + body: JSON.stringify({ email: 123 }), + }); + expect(res.status).toBe(400); + const body = await json(res); + expect(body.code).toBe('invalid_request'); + // Named for what it is. Routing this into the presence check reported "email is required" for + // an address that was supplied — the same backwards message the shape guard exists to avoid. + expect(body.message).toBe('email must be a string'); + }); + + // Every one of these paths type-asserted `email` and then lowercased it to resolve the account + // case-insensitively, so a non-string arrived at `.toLowerCase()` and came back a 500 — + // `server_error` in a consumer's suite reads as an emulator defect, not a malformed request. + it('rejects a non-string email with 400 on every grant that resolves one', async () => { + const password = await app.request('/user_management/authenticate', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ grant_type: 'password', email: 123, password: 'whatever' }), + }); + expect(password.status).toBe(400); + expect((await json(password)).message).toBe('email must be a string'); + + const magic = await app.request('/user_management/authenticate', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + grant_type: 'urn:workos:oauth:grant-type:magic-auth:code', + code: '123456', + email: { not: 'a string' }, + }), + }); + expect(magic.status).toBe(400); + expect((await json(magic)).message).toBe('email must be a string'); + }); + + // Creation trims before storing, so a read that skipped the trim could not find the account + // creation had just written under the same address. + it('trims a padded address on the paths that resolve one', async () => { + const signup = await json( + await req('/user_management/magic_auth', { + method: 'POST', + body: JSON.stringify({ email: ' padded@x.test ' }), + }), + ); + expect(signup.email).toBe('padded@x.test'); + + getWorkOSStore(store).users.update(signup.user_id, { password_hash: hashPassword('pw') }); + const password = await app.request('/user_management/authenticate', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ grant_type: 'password', email: ' padded@x.test ', password: 'pw' }), + }); + expect(password.status).toBe(200); + + const reset = await req('/user_management/password_reset', { + method: 'POST', + body: JSON.stringify({ email: ' padded@x.test ' }), + }); + expect(reset.status).toBe(201); + }); + + it('creates the user at magic auth code creation for an unknown email', async () => { + const magicRes = await req('/user_management/magic_auth', { + method: 'POST', + body: JSON.stringify({ email: 'signup@test.com' }), + }); + expect(magicRes.status).toBe(201); + const magicBody = await json(magicRes); + expect(magicBody.user_id).toBeTruthy(); + + const usersRes = await req('/user_management/users?email=signup%40test.com'); + const users = await json(usersRes); + expect(users.data).toHaveLength(1); + expect(users.data[0].id).toBe(magicBody.user_id); + expect(users.data[0].email_verified).toBe(false); + }); + + // A sign-up that creates a user is a sign-up a webhook consumer expects to hear about, which is + // a large part of why this endpoint creating one is useful to test against at all. + it('emits user.created for a sign-up, and none when the user already existed', async () => { + const ws = getWorkOSStore(store); + const created = () => + ws.events.all().filter((e: { event: string; data: Record }) => e.event === 'user.created'); + + const signup = await json( + await req('/user_management/magic_auth', { method: 'POST', body: JSON.stringify({ email: 'evented@test.com' }) }), + ); + expect(created()).toHaveLength(1); + expect(created()[0].data).toMatchObject({ id: signup.user_id, email: 'evented@test.com', email_verified: false }); + + // A second code for the same address resolves the existing user, so nothing was created. + await req('/user_management/magic_auth', { method: 'POST', body: JSON.stringify({ email: 'evented@test.com' }) }); + expect(created()).toHaveLength(1); + }); + + it('resolves an existing user case-insensitively instead of forking the account', async () => { + const first = await json( + await req('/user_management/magic_auth', { + method: 'POST', + body: JSON.stringify({ email: 'Casing@Test.com' }), + }), + ); + const second = await json( + await req('/user_management/magic_auth', { + method: 'POST', + body: JSON.stringify({ email: 'casing@test.com' }), + }), + ); + + expect(second.user_id).toBe(first.user_id); + // Stored as first given — production preserves the case it was handed. + const users = getWorkOSStore(store).users.all(); + expect(users).toHaveLength(1); + expect(users[0].email).toBe('Casing@Test.com'); + + // And the code is redeemable with the address it was requested for, not only the stored + // casing. Resolving the user case-insensitively while matching the code exactly would + // return a 201 carrying a code that this authenticate call could never spend. + const auth = await app.request('/user_management/authenticate', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + grant_type: 'urn:workos:oauth:grant-type:magic-auth:code', + code: second.code, + email: 'casing@test.com', + }), + }); + expect(auth.status).toBe(200); + expect((await json(auth)).user.id).toBe(first.user_id); + }); + + it('rejects an email that could only be a typo, rather than creating a ghost user', async () => { + for (const email of ['', ' ', 'not-an-email', 'a b@test.com', '@test.com', 'nope@']) { + const res = await req('/user_management/magic_auth', { + method: 'POST', + body: JSON.stringify({ email }), + }); + expect(res.status).toBe(400); + expect((await json(res)).code).toBe('invalid_request'); + } + expect(getWorkOSStore(store).users.all()).toHaveLength(0); + }); + + // Absent and malformed have the same fix only if the caller is told which one happened. `null` + // counts as absent — it is how a JSON body spells it, and both creation paths agree on that. + it('distinguishes a missing email from an unusable one', async () => { + for (const body of [{}, { email: null }]) { + const missing = await req('/user_management/magic_auth', { method: 'POST', body: JSON.stringify(body) }); + expect(missing.status).toBe(400); + expect((await json(missing)).message).toBe('email is required'); + } + + const malformed = await req('/user_management/magic_auth', { + method: 'POST', + body: JSON.stringify({ email: 'not-an-email' }), + }); + expect((await json(malformed)).message).toBe('email must be a valid email address'); + }); + + // Magic Auth stores the case it was handed, so every other way in has to resolve that way too + // — otherwise a sign-up creates an account the rest of the API cannot reach. + it('reaches a Magic Auth account by any casing of its address', async () => { + await req('/user_management/magic_auth', { method: 'POST', body: JSON.stringify({ email: 'Mixed@Case.test' }) }); + const user = getWorkOSStore(store).users.all()[0]; + getWorkOSStore(store).users.update(user.id, { password_hash: hashPassword('correct horse') }); + + const password = await app.request('/user_management/authenticate', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ grant_type: 'password', email: 'mixed@case.test', password: 'correct horse' }), + }); + expect(password.status).toBe(200); + expect((await json(password)).user.id).toBe(user.id); + + const reset = await req('/user_management/password_reset', { + method: 'POST', + body: JSON.stringify({ email: 'mixed@case.test' }), + }); + expect(reset.status).toBe(201); + }); + + it('emits one user.updated per magic auth sign-in, and none for a no-op re-verify', async () => { + const ws = getWorkOSStore(store); + const countUpdates = () => ws.events.all().filter((e: { event: string }) => e.event === 'user.updated').length; + + await signInWithMagicAuth('quiet@test.com'); + const afterFirst = countUpdates(); + // The sign-up login both verifies the email and stamps last_sign_in_at — one write. + expect(afterFirst).toBe(1); + + await signInWithMagicAuth('quiet@test.com'); + // The second login only stamps last_sign_in_at; email_verified is already true. + expect(countUpdates()).toBe(2); + }); + + it('magic auth sign-up verifies the email and yields an org-less session', async () => { + const res = await signInWithMagicAuth('signup2@test.com'); + expect(res.status).toBe(200); + const body = await json(res); + expect(body.user.email_verified).toBe(true); + expect(decodeJwt(body.access_token).org_id).toBeUndefined(); + }); + // --- Device code tests --- it('device authorization + device_code grant flow', async () => { diff --git a/src/workos/routes/auth.ts b/src/workos/routes/auth.ts index 177142e..5455673 100644 --- a/src/workos/routes/auth.ts +++ b/src/workos/routes/auth.ts @@ -25,6 +25,9 @@ import { generateCode, formatAuthChallenge, acceptInvitation, + findUserByEmail, + requireEmailString, + emailsMatch, } from '../helpers.js'; import { renderConfiguredJwtTemplate } from '../jwt-template.js'; import type { EventBus } from '../event-bus.js'; @@ -72,7 +75,9 @@ export function authRoutes(ctx: RouteContext): void { let user; if (loginHint) { - user = ws.users.findOneBy('email', loginHint); + // Case-insensitively, like every other lookup by email: Magic Auth stores the case it was + // handed, so an account created as 'User@x.test' has to be reachable as 'user@x.test'. + user = findUserByEmail(ws, loginHint); if (!user) { const redirect = new URL(redirectUri); redirect.searchParams.set('error', 'user_not_found'); @@ -379,13 +384,13 @@ export function authRoutes(ctx: RouteContext): void { } case 'password': { - const email = body.email as string; + const email = requireEmailString(body.email); const password = body.password as string; if (!email || !password) { throw new OauthApiError(400, 'invalid_request', 'email and password are required.'); } - user = ws.users.findOneBy('email', email); + user = findUserByEmail(ws, email); if (!user || !user.password_hash || !verifyPassword(password, user.password_hash)) { // Verified live: 400 (not 401) with the email interpolated. `password` is an RFC 6749 // grant that nonetheless fails with the plain shape, which is why the credential @@ -414,12 +419,16 @@ export function authRoutes(ctx: RouteContext): void { case 'urn:workos:oauth:grant-type:magic-auth': case 'urn:workos:oauth:grant-type:magic-auth:code': { const code = body.code as string; - const email = body.email as string; + const email = requireEmailString(body.email); if (!code || !email) { throw new OauthApiError(400, 'invalid_request', 'code and email are required.'); } - const magicAuth = ws.magicAuths.all().find((ma) => ma.code === code && ma.email === email); + // Case-insensitively, because code creation resolves the user that way: a code requested + // for 'user@x.test' against a stored 'User@X.test' is recorded under the stored casing, + // so an exact match here would hand back a code that the address it was requested for + // could never redeem. + const magicAuth = ws.magicAuths.all().find((ma) => ma.code === code && emailsMatch(ma.email, email)); if (!magicAuth) { failAuth('MagicAuth', { email }, new WorkOSApiError(400, 'Invalid one-time code', 'invalid_one_time_code')); } @@ -670,7 +679,7 @@ export function authRoutes(ctx: RouteContext): void { // neither their credential nor the invitation. Compared case-insensitively: an invitation to // Foo@example.com is for the same person as foo@example.com, and rejecting on letter case alone // would be a false negative. - if (invitation && invitation.email.toLowerCase() !== user.email.toLowerCase()) { + if (invitation && !emailsMatch(invitation.email, user.email)) { throw new WorkOSApiError( 400, 'The invitation was issued for a different email address', @@ -750,7 +759,16 @@ export function authRoutes(ctx: RouteContext): void { // reuses the existing session, so it emits neither session.created nor an auth event. let session; if (isFreshLogin) { - ws.users.update(user.id, { last_sign_in_at: new Date().toISOString() }); + // A redeemed magic-auth code proves mailbox ownership, so production marks the email + // verified. Folded into the sign-in write rather than done up in the grant: one + // user.updated per login instead of two, and nothing is persisted before the template + // gate above — which is what keeps a failed render from implying a login that never + // completed. Only set when it actually changes, so a repeat sign-in stays quiet. + const verifyEmail = authMethod === 'MagicAuth' && !user.email_verified; + ws.users.update(user.id, { + last_sign_in_at: new Date().toISOString(), + ...(verifyEmail ? { email_verified: true } : {}), + }); session = ws.sessions.insert({ object: 'session', user_id: user.id, diff --git a/src/workos/routes/invitations.spec.ts b/src/workos/routes/invitations.spec.ts index cb6236e..6e93e92 100644 --- a/src/workos/routes/invitations.spec.ts +++ b/src/workos/routes/invitations.spec.ts @@ -132,6 +132,94 @@ describe('Invitation routes', () => { expect(memberships.data[0].organization_id).toBe(org.id); }); + // Resolving the recipient exactly enrolled nobody for an account stored under a different case: + // the invitation was still spent and invitation.accepted still fired, with no membership to show + // for it and no error anywhere. Magic Auth sign-up makes accounts under whatever case it was + // handed, and the authenticate flow already compares the two addresses case-insensitively. + it('accepts an invitation for an account stored under a different case', async () => { + const user = await json( + await req('/user_management/users', { method: 'POST', body: JSON.stringify({ email: 'Member@X.test' }) }), + ); + const org = await json(await req('/organizations', { method: 'POST', body: JSON.stringify({ name: 'Case Org' }) })); + + const inv = await json( + await req('/user_management/invitations', { + method: 'POST', + body: JSON.stringify({ email: 'member@x.test', organization_id: org.id }), + }), + ); + const accepted = await req(`/user_management/invitations/${inv.id}/accept`, { method: 'POST' }); + expect(accepted.status).toBe(200); + expect((await json(accepted)).state).toBe('accepted'); + + const memberships = await json(await req(`/user_management/organization_memberships?organization_id=${org.id}`)); + expect(memberships.data).toHaveLength(1); + expect(memberships.data[0].user_id).toBe(user.id); + }); + + it('filters invitations by email case-insensitively', async () => { + await req('/user_management/invitations', { method: 'POST', body: JSON.stringify({ email: 'Filter@X.test' }) }); + + const list = await json(await req('/user_management/invitations?email=filter%40x.test')); + expect(list.data).toHaveLength(1); + expect(list.data[0].email).toBe('Filter@X.test'); + }); + + // A non-string was survivable while every consumer of a stored email compared it with `!==`. + // Resolving the recipient case-insensitively means calling `toLowerCase` on it, so accepting a + // number here turned both the email filter and accepting the invitation into a 500 — a + // `server_error` in a consumer's suite reads as an emulator defect rather than a bad request. + it('rejects a non-string email rather than storing one nothing can read back', async () => { + for (const email of [123, { not: 'a string' }, ['a@x.test']]) { + const res = await req('/user_management/invitations', { method: 'POST', body: JSON.stringify({ email }) }); + expect(res.status).toBe(422); + const body = await json(res); + expect(body.code).toBe('unprocessable_entity'); + expect(body.message).toBe('email must be a string'); + } + + // The two reads that would have 500d on a stored non-string. + expect((await req('/user_management/invitations?email=a%40x.test')).status).toBe(200); + expect((await json(await req('/user_management/invitations'))).data).toHaveLength(0); + }); + + // Acceptance resolves the recipient by this address, so a typo is spent silently: 200, the + // invitation marked accepted, invitation.accepted emitted, and nobody enrolled. Held to the same + // standard as the two routes that create users, which reject for the same reason. + it('rejects an email that could only be a typo', async () => { + for (const email of ['', ' ', 'not-an-email', 'a b@test.com', '@test.com', 'nope@', 'two@at@test.com']) { + const res = await req('/user_management/invitations', { method: 'POST', body: JSON.stringify({ email }) }); + expect(res.status).toBe(422); + expect((await json(res)).message).toMatch(/email (is required|must be a valid email address)/); + } + expect((await json(await req('/user_management/invitations'))).data).toHaveLength(0); + }); + + // Absent and malformed have the same fix only if the caller is told which one happened, and + // `null` in a JSON body is how a caller spells absence. + it('reports an absent email as absent, including an explicit null', async () => { + for (const body of [{}, { email: null }]) { + const res = await req('/user_management/invitations', { method: 'POST', body: JSON.stringify(body) }); + expect(res.status).toBe(422); + const parsed = await json(res); + expect(parsed.message).toBe('email is required'); + expect(parsed.errors[0]).toMatchObject({ field: 'email', code: 'required' }); + } + }); + + it('trims a padded address before storing it', async () => { + const inv = await json( + await req('/user_management/invitations', { + method: 'POST', + body: JSON.stringify({ email: ' padded@x.test ' }), + }), + ); + expect(inv.email).toBe('padded@x.test'); + + const list = await json(await req('/user_management/invitations?email=padded%40x.test')); + expect(list.data).toHaveLength(1); + }); + it('revokes an invitation', async () => { const created = await json( await req('/user_management/invitations', { diff --git a/src/workos/routes/invitations.ts b/src/workos/routes/invitations.ts index 6f2252c..1b9a188 100644 --- a/src/workos/routes/invitations.ts +++ b/src/workos/routes/invitations.ts @@ -1,11 +1,4 @@ -import { - type RouteContext, - notFound, - validationError, - parseJsonBody, - WorkOSApiError, - parseListParams, -} from '../../core/index.js'; +import { type RouteContext, notFound, parseJsonBody, WorkOSApiError, parseListParams } from '../../core/index.js'; import { getWorkOSStore } from '../store.js'; import { formatInvitation, @@ -13,6 +6,9 @@ import { expiresIn, formatListResponse, acceptInvitation, + findUserByEmail, + emailsMatch, + requireEmailField, } from '../helpers.js'; import type { EventBus } from '../event-bus.js'; import { STORE_KEYS, EVENTS } from '../constants.js'; @@ -23,10 +19,13 @@ export function invitationRoutes(ctx: RouteContext): void { app.post('/user_management/invitations', async (c) => { const body = await parseJsonBody(c); - const email = body.email as string | undefined; - if (!email) { - throw validationError('email is required', [{ field: 'email', code: 'required' }]); - } + // The same guard the two user-creation routes apply, for a related reason. Accepting a + // non-string here used to be survivable because everything downstream compared the address + // with `!==`; resolving the recipient case-insensitively means calling `toLowerCase` on it, + // so a stored number turned both the email filter and accepting the invitation into a 500. + // And an address that could only be a typo makes an invitation nobody can accept: acceptance + // resolves a user by this email, so a typo is spent silently, enrolling no one. + const email = requireEmailField(body.email, { requireShape: true }); const token = generateVerificationToken(); const inv = ws.invitations.insert({ @@ -53,7 +52,8 @@ export function invitationRoutes(ctx: RouteContext): void { const result = ws.invitations.list({ ...params, filter: (inv) => { - if (emailFilter && inv.email !== emailFilter) return false; + // Case-insensitively, like every other lookup by email. + if (emailFilter && !emailsMatch(inv.email, emailFilter)) return false; if (orgFilter && inv.organization_id !== orgFilter) return false; return true; }, @@ -82,7 +82,11 @@ export function invitationRoutes(ctx: RouteContext): void { throw new WorkOSApiError(400, `Invitation is ${inv.state}`, 'invalid_invitation_state'); } - acceptInvitation(inv, ws.users.findOneBy('email', inv.email), ws, store.getData(STORE_KEYS.eventBus)); + // Case-insensitively: an exact match here enrolled nobody for an account stored under a + // different case, spending the invitation and emitting invitation.accepted with no membership + // to show for it. The authenticate flow already compares the two addresses this way, and Magic + // Auth sign-up makes accounts under whatever case the caller sent. + acceptInvitation(inv, findUserByEmail(ws, inv.email), ws, store.getData(STORE_KEYS.eventBus)); const updated = ws.invitations.get(inv.id)!; return c.json(formatInvitation(updated)); diff --git a/src/workos/routes/magic-auth.ts b/src/workos/routes/magic-auth.ts index e668651..d752010 100644 --- a/src/workos/routes/magic-auth.ts +++ b/src/workos/routes/magic-auth.ts @@ -1,6 +1,6 @@ import { type RouteContext, notFound, parseJsonBody, WorkOSApiError } from '../../core/index.js'; import { getWorkOSStore } from '../store.js'; -import { formatMagicAuth, generateCode, expiresIn } from '../helpers.js'; +import { formatMagicAuth, generateCode, expiresIn, findUserByEmail, requireEmailString } from '../helpers.js'; export function magicAuthRoutes(ctx: RouteContext): void { const { app, store } = ctx; @@ -14,13 +14,38 @@ export function magicAuthRoutes(ctx: RouteContext): void { app.post('/user_management/magic_auth', async (c) => { const body = await parseJsonBody(c); - const email = body.email as string | undefined; + // This handler now creates users, so its input guard is the only thing standing between a + // typo and a permanent ghost account. A bare presence check was enough when the endpoint + // could only ever read. A malformed address is reported apart from an absent one — the two + // have the same fix only if the caller is told which one happened, and "email is required" + // describes an address that was in fact supplied exactly backwards. + const email = requireEmailString(body.email, { requireShape: true }); if (!email) { throw new WorkOSApiError(400, 'email is required', 'invalid_request'); } - const user = ws.users.findOneBy('email', email); - if (!user) throw notFound('User'); + // Magic Auth doubles as sign-up: production creates the user at code-creation + // time (the response already carries its user_id), not at authenticate. + // The lookup is case-insensitive because the creating branch below is: an exact-match + // miss on 'User@x.test' vs 'user@x.test' used to be a harmless 404 and would now fork + // the account in two. The address is stored as given — production preserves case. + const user = + findUserByEmail(ws, email) ?? + ws.users.insert({ + object: 'user', + email, + name: null, + first_name: null, + last_name: null, + email_verified: false, + profile_picture_url: null, + last_sign_in_at: null, + external_id: null, + metadata: {}, + locale: null, + password_hash: null, + impersonator: null, + }); const ma = ws.magicAuths.insert({ object: 'magic_auth', diff --git a/src/workos/routes/password-reset.spec.ts b/src/workos/routes/password-reset.spec.ts index 568f588..148c417 100644 --- a/src/workos/routes/password-reset.spec.ts +++ b/src/workos/routes/password-reset.spec.ts @@ -45,6 +45,34 @@ describe('Password reset routes', () => { return { user, reset }; } + // Resolving the account case-insensitively means lowercasing the address, so a type-asserted + // non-string reached `.toLowerCase()` and this came back a 500 rather than a named 400. + it('rejects a non-string email with 400, not 500', async () => { + const res = await req('/user_management/password_reset', { + method: 'POST', + body: JSON.stringify({ email: 123 }), + }); + expect(res.status).toBe(400); + expect((await json(res)).message).toBe('email must be a string'); + }); + + it('still reports an absent email as absent', async () => { + const res = await req('/user_management/password_reset', { method: 'POST', body: JSON.stringify({}) }); + expect(res.status).toBe(400); + expect((await json(res)).message).toBe('email is required'); + }); + + it('resolves the account by any casing of its address', async () => { + await req('/user_management/users', { method: 'POST', body: JSON.stringify({ email: 'Mixed@Reset.test' }) }); + const res = await req('/user_management/password_reset', { + method: 'POST', + body: JSON.stringify({ email: 'mixed@reset.test' }), + }); + expect(res.status).toBe(201); + // The reset is recorded against the stored casing, not the one the caller sent. + expect((await json(res)).email).toBe('Mixed@Reset.test'); + }); + it('emits password_reset.created when a reset is requested', async () => { const { user } = await createUserAndRequestReset(); diff --git a/src/workos/routes/password-reset.ts b/src/workos/routes/password-reset.ts index 8bc7ec9..bd65eb8 100644 --- a/src/workos/routes/password-reset.ts +++ b/src/workos/routes/password-reset.ts @@ -1,6 +1,14 @@ import { type RouteContext, notFound, parseJsonBody, WorkOSApiError } from '../../core/index.js'; import { getWorkOSStore } from '../store.js'; -import { formatPasswordReset, generateVerificationToken, hashPassword, expiresIn, isExpired } from '../helpers.js'; +import { + formatPasswordReset, + generateVerificationToken, + hashPassword, + expiresIn, + isExpired, + findUserByEmail, + requireEmailString, +} from '../helpers.js'; import { STORE_KEYS, EVENTS } from '../constants.js'; import type { EventBus } from '../event-bus.js'; @@ -16,12 +24,14 @@ export function passwordResetRoutes(ctx: RouteContext): void { app.post('/user_management/password_reset', async (c) => { const body = await parseJsonBody(c); - const email = body.email as string | undefined; + const email = requireEmailString(body.email); if (!email) { throw new WorkOSApiError(400, 'email is required', 'invalid_request'); } - const user = ws.users.findOneBy('email', email); + // Case-insensitively, like every other lookup by email: an account Magic Auth created as + // 'User@x.test' must not be unresettable by the address the caller actually has. + const user = findUserByEmail(ws, email); if (!user) throw notFound('User'); const pr = ws.passwordResets.insert({ diff --git a/src/workos/routes/sso.spec.ts b/src/workos/routes/sso.spec.ts index fa1a9bf..ecaa9b5 100644 --- a/src/workos/routes/sso.spec.ts +++ b/src/workos/routes/sso.spec.ts @@ -14,10 +14,12 @@ function createTestApp() { describe('SSO routes', () => { let app: ReturnType['app']; + let store: Store; beforeEach(() => { const server = createTestApp(); app = server.app; + store = server.store; }); const req = (path: string, init?: RequestInit) => app.request(path, { headers, ...init }); @@ -57,6 +59,54 @@ describe('SSO routes', () => { expect(url.searchParams.get('state')).toBe('abc'); }); + // The last exact-match lookup by email. A login_hint differing only in case is the same + // federated person, so it reuses the profile rather than minting a second one for the same + // connection — which is the pair of records no lookup by email can tell apart, in profile form. + it('reuses one profile across casings of the same login_hint', async () => { + const { conn } = await createOrgWithConnection(); + + for (const hint of ['Person%40sso.example.com', 'person%40sso.example.com', 'PERSON%40SSO.EXAMPLE.COM']) { + const res = await app.request( + `/sso/authorize?connection=${conn.id}&redirect_uri=http://localhost:3000/callback&login_hint=${hint}`, + ); + expect(res.status).toBe(302); + } + + const profiles = getWorkOSStore(store).ssoProfiles.all(); + expect(profiles).toHaveLength(1); + // Stored as first given, like every other address the emulator writes. + expect(profiles[0].email).toBe('Person@sso.example.com'); + }); + + // Matching on the connection at the same time as the email, not after: `findOneBy` returned the + // first profile for the address whatever connection it belonged to, so the second connection + // never matched its own profile and minted another on every authorize. + it('keeps one profile per connection for the same address', async () => { + const { conn } = await createOrgWithConnection(); + const org2 = await json(await req('/organizations', { method: 'POST', body: JSON.stringify({ name: 'Other' }) })); + const conn2 = await json( + await req('/connections', { + method: 'POST', + body: JSON.stringify({ + name: 'Other SSO', + organization_id: org2.id, + connection_type: 'GenericSAML', + domains: ['sso.example.com'], + }), + }), + ); + + for (const id of [conn.id, conn2.id, conn.id, conn2.id]) { + await app.request( + `/sso/authorize?connection=${id}&redirect_uri=http://localhost:3000/callback&login_hint=shared%40sso.example.com`, + ); + } + + const profiles = getWorkOSStore(store).ssoProfiles.all(); + expect(profiles).toHaveLength(2); + expect(new Set(profiles.map((p) => p.connection_id))).toEqual(new Set([conn.id, conn2.id])); + }); + it('sso token exchange returns profile and access_token', async () => { const { conn } = await createOrgWithConnection(); @@ -291,6 +341,32 @@ describe('SSO authentication events', () => { expect(event.data).toHaveProperty('email'); }); + // SSO is profile-based, so the event's user_id is resolved from the profile's email. Resolving it + // exactly reported user_id: null for an account that existed under a different case — and Magic + // Auth sign-up creates accounts under whatever case it was handed. + it('resolves the event user_id for an account stored under a different case', async () => { + const { conn } = await createOrgWithConnection(); + const user = await json( + await req('/user_management/users', { + method: 'POST', + body: JSON.stringify({ email: 'Federated@SSO-Events.example.com' }), + }), + ); + + const authRes = await app.request( + `/sso/authorize?connection=${conn.id}&redirect_uri=http://localhost:3000/callback&login_hint=federated%40sso-events.example.com`, + ); + const code = new URL(authRes.headers.get('location')!).searchParams.get('code')!; + await app.request('/sso/token', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ grant_type: 'authorization_code', code }), + }); + + const [event] = eventsNamed('authentication.sso_succeeded'); + expect(event.data).toMatchObject({ user_id: user.id, email: 'federated@sso-events.example.com' }); + }); + it('emits authentication.sso_failed with an error object for an invalid code', async () => { const res = await app.request('/sso/token', { method: 'POST', diff --git a/src/workos/routes/sso.ts b/src/workos/routes/sso.ts index 17b1169..0dd07c3 100644 --- a/src/workos/routes/sso.ts +++ b/src/workos/routes/sso.ts @@ -7,6 +7,8 @@ import { isExpired, assertAllowedRedirectUri, emitAuthenticationEvent, + findUserByEmail, + emailsMatch, } from '../helpers.js'; import type { WorkOSConnection } from '../entities.js'; import type { EventBus } from '../event-bus.js'; @@ -48,8 +50,13 @@ export function ssoRoutes(ctx: RouteContext): void { } const email = loginHint ?? `user@${connection.domains[0]?.domain ?? 'example.com'}`; - let profile = ws.ssoProfiles.findOneBy('email', email); - if (!profile || profile.connection_id !== connection.id) { + // The last exact-match lookup by email, matched on the connection at the same time rather + // than after. `findOneBy` returns the first profile for the address whatever connection it + // belongs to, so a second connection never matched its own profile and minted another on + // every authorize. Case-insensitive for the reason the rest are: a login_hint differing only + // in case is the same federated person. + let profile = ws.ssoProfiles.all().find((p) => p.connection_id === connection.id && emailsMatch(p.email, email)); + if (!profile) { profile = ws.ssoProfiles.insert({ object: 'profile', connection_id: connection.id, @@ -196,7 +203,7 @@ export function ssoRoutes(ctx: RouteContext): void { method: 'SSO', status: 'failed', email: expiredProfile?.email, - userId: ws.users.findOneBy('email', expiredProfile?.email ?? '')?.id, + userId: findUserByEmail(ws, expiredProfile?.email ?? '')?.id, error: { code: error.code, message: error.message }, ipAddress: c.req.header('x-forwarded-for') ?? null, userAgent: c.req.header('user-agent') ?? null, @@ -229,13 +236,15 @@ export function ssoRoutes(ctx: RouteContext): void { store.setData(`${STORE_KEY_PREFIXES.ssoToken}${accessToken}`, profile.id); - // SSO is profile-based; a user-management user may not exist for this email + // SSO is profile-based; a user-management user may not exist for this email. Resolved + // case-insensitively, like every other lookup by email, so the event carries the id of an + // account stored under a different case rather than reporting none. emitAuthenticationEvent({ eventBus: store.getData(STORE_KEYS.eventBus), method: 'SSO', status: 'succeeded', email: profile.email, - userId: ws.users.findOneBy('email', profile.email)?.id ?? null, + userId: findUserByEmail(ws, profile.email)?.id ?? null, ipAddress: c.req.header('x-forwarded-for') ?? null, userAgent: c.req.header('user-agent') ?? null, sso: { diff --git a/src/workos/routes/users.spec.ts b/src/workos/routes/users.spec.ts index 643e354..dc4489d 100644 --- a/src/workos/routes/users.spec.ts +++ b/src/workos/routes/users.spec.ts @@ -33,6 +33,79 @@ describe('User routes', () => { expect(user.password_hash).toBeUndefined(); }); + // Held to the same standard as the magic auth handler, which validates for the same reason: + // both create users, and an address that could only be a typo becomes an unreachable account. + it('rejects an email that could only be a typo', async () => { + for (const email of ['', ' ', 'not-an-email', 'a b@test.com', '@test.com', 'nope@', 'two@at@test.com', 123]) { + const res = await req('/user_management/users', { + method: 'POST', + body: JSON.stringify({ email }), + }); + expect(res.status).toBe(422); + expect((await json(res)).code).toBe('unprocessable_entity'); + } + const list = await json(await req('/user_management/users')); + expect(list.data).toHaveLength(0); + }); + + // `null` is how a JSON body spells absence, so it is reported as absence — the same answer the + // magic auth handler gives it. Classifying it as a type error instead had the two creation paths + // disagreeing about which of the two distinctions this route exists to draw it falls on. + it('reports an absent email as absent, including an explicit null', async () => { + for (const body of [{}, { email: null }]) { + const res = await req('/user_management/users', { method: 'POST', body: JSON.stringify(body) }); + expect(res.status).toBe(422); + const parsed = await json(res); + expect(parsed.message).toBe('email is required'); + expect(parsed.errors[0]).toMatchObject({ field: 'email', code: 'required' }); + } + }); + + it('names a non-string email as the wrong type, not as missing', async () => { + const res = await req('/user_management/users', { method: 'POST', body: JSON.stringify({ email: 123 }) }); + expect(res.status).toBe(422); + const body = await json(res); + expect(body.message).toBe('email must be a string'); + expect(body.errors[0]).toMatchObject({ field: 'email', code: 'invalid_type' }); + }); + + // Case-insensitively, like the magic auth handler: two accounts differing only in case left the + // two creation paths disagreeing about which one an address names, with magic auth's resolver + // settling it by insertion order. + it('rejects a duplicate email that differs only in case', async () => { + const first = await req('/user_management/users', { + method: 'POST', + body: JSON.stringify({ email: 'User@x.test' }), + }); + expect(first.status).toBe(201); + + const second = await req('/user_management/users', { + method: 'POST', + body: JSON.stringify({ email: 'user@x.test' }), + }); + expect(second.status).toBe(409); + expect((await json(second)).code).toBe('user_already_exists'); + }); + + // This is the lookup an SDK's listUsers({ email }) maps to, so it is how a caller finds the + // account a Magic Auth sign-up just made — and sign-up stores whatever case it was handed. + // Filtering exactly meant the address the caller had returned nothing for a user that existed. + it('filters by email case-insensitively', async () => { + const created = await json( + await req('/user_management/users', { method: 'POST', body: JSON.stringify({ email: 'Listed@X.test' }) }), + ); + + for (const query of ['listed%40x.test', 'Listed%40X.test', 'LISTED%40X.TEST']) { + const list = await json(await req(`/user_management/users?email=${query}`)); + expect(list.data).toHaveLength(1); + expect(list.data[0].id).toBe(created.id); + } + + // Still a filter, not a fuzzy match. + const miss = await json(await req('/user_management/users?email=listed%40y.test')); + expect(miss.data).toHaveLength(0); + }); + it('rejects duplicate email', async () => { await req('/user_management/users', { method: 'POST', diff --git a/src/workos/routes/users.ts b/src/workos/routes/users.ts index b55e090..aac86f2 100644 --- a/src/workos/routes/users.ts +++ b/src/workos/routes/users.ts @@ -7,7 +7,15 @@ import { parseListParams, } from '../../core/index.js'; import { getWorkOSStore } from '../store.js'; -import { formatUser, formatIdentity, hashPassword, formatListResponse } from '../helpers.js'; +import { + formatUser, + formatIdentity, + hashPassword, + formatListResponse, + findUserByEmail, + emailsMatch, + requireEmailField, +} from '../helpers.js'; export function userRoutes(ctx: RouteContext): void { const { app, store } = ctx; @@ -15,12 +23,18 @@ export function userRoutes(ctx: RouteContext): void { app.post('/user_management/users', async (c) => { const body = await parseJsonBody(c); - const email = body.email as string | undefined; - if (!email) { - throw validationError('email is required', [{ field: 'email', code: 'required' }]); - } - - const existing = ws.users.findOneBy('email', email); + // The same guard the magic auth handler applies, for the same reason: this route creates + // users, and an address that could only be a typo becomes an account nothing can reach. + // Holding the two paths to one standard is what stops `{email: 'nope'}` being a 422 on one + // and a 201 on the other — shared rather than restated, since a second copy is how the two + // drifted over `null` in the first place. + const email = requireEmailField(body.email, { requireShape: true }); + + // Case-insensitively, for the same reason the magic auth handler resolves that way: an + // exact-match miss on 'User@x.test' vs 'user@x.test' let both be created, and then the two + // creation paths disagreed about which account an address names — with magic auth resolving + // the ambiguity by insertion order. + const existing = findUserByEmail(ws, email); if (existing) { throw new WorkOSApiError(409, 'A user with this email already exists', 'user_already_exists'); } @@ -63,7 +77,10 @@ export function userRoutes(ctx: RouteContext): void { const result = ws.users.list({ ...params, filter: (user) => { - if (emailFilter && user.email !== emailFilter) return false; + // Case-insensitively, like every other lookup by email. This is the lookup an SDK's + // listUsers({ email }) reaches for, so it is how a caller finds the account a Magic Auth + // sign-up just made — and that account is stored under whatever case created it. + if (emailFilter && !emailsMatch(user.email, emailFilter)) return false; if (orgUserIds && !orgUserIds.has(user.id)) return false; return true; }, diff --git a/src/workos/seed-memberships.spec.ts b/src/workos/seed-memberships.spec.ts index d687d5b..72dd76f 100644 --- a/src/workos/seed-memberships.spec.ts +++ b/src/workos/seed-memberships.spec.ts @@ -49,6 +49,45 @@ describe('Seeding organization memberships', () => { expect(m.user).toMatchObject({ object: 'user', id: m.user_id, email: 'admin@acme.com' }); }); + // The join resolves case-insensitively, like every other lookup by email, so a reference the + // running emulator would honour is not rejected at startup on letter case alone. + it('joins a membership to its user by any casing of the address', async () => { + emulator = await createEmulator({ + port: 0, + seed: { + users: [{ email: 'Admin@Acme.com' }], + organizations: [{ name: 'Acme Corp', memberships: [{ email: 'admin@acme.com', role: 'admin' }] }], + }, + }); + + const res = await fetch(`${emulator.url}/user_management/organization_memberships`, { + headers: auth(emulator.apiKey), + }); + const list = (await res.json()) as any; + expect(list.data).toHaveLength(1); + // Stored under the case that seeded it, reached by the case the membership named. + expect(list.data[0].user).toMatchObject({ email: 'Admin@Acme.com' }); + }); + + // Seeding stores the trimmed address, so a padded seed is reachable by the address the caller + // actually has — and the membership that named it joins the same account. + it('stores a padded seeded address trimmed', async () => { + emulator = await createEmulator({ + port: 0, + seed: { + users: [{ email: ' padded@acme.com ' }], + organizations: [{ name: 'Acme Corp', memberships: [{ email: 'padded@acme.com', role: 'member' }] }], + }, + }); + + const res = await fetch(`${emulator.url}/user_management/users?email=padded%40acme.com`, { + headers: auth(emulator.apiKey), + }); + const list = (await res.json()) as any; + expect(list.data).toHaveLength(1); + expect(list.data[0].email).toBe('padded@acme.com'); + }); + it('rejects startup when a membership references an email with no seeded user', async () => { await expect( createEmulator({ @@ -81,6 +120,65 @@ describe('Seeding organization memberships', () => { expect(error.message).toContain('must match a user defined in users'); }); + it('accepts a membership email differing from its user only in case', () => { + const { valid } = validateSeedConfig({ + users: [{ email: 'Admin@Acme.com' }], + organizations: [{ name: 'Acme', memberships: [{ email: 'admin@acme.com' }] }], + }); + expect(valid).toBe(true); + }); + + it('rejects two seeded users with the same email', () => { + const error = findError({ users: [{ email: 'dup@acme.com' }, { email: 'dup@acme.com' }] }, 'users[1].email'); + expect(error.message).toContain('unique across users'); + }); + + // The uniqueness the API enforces: POST /user_management/users answers 409 for an address + // differing only in case, so a seed that got two through would be the one remaining way to + // manufacture the pair of accounts no lookup by email can tell apart. + it('rejects two seeded users whose emails differ only in case', () => { + const error = findError({ users: [{ email: 'Dup@Acme.com' }, { email: 'dup@acme.com' }] }, 'users[1].email'); + expect(error.message).toContain('unique across users'); + }); + + // A seed is the one creation path with no route in front of it, so it is the remaining way to + // write a user under an address no lookup by email resolves — the state the two routes' typo + // guards exist to prevent. + it('rejects a seeded user email that could only be a typo', () => { + for (const email of [' ', 'not-an-email', 'a b@acme.com', '@acme.com', 'nope@', 'two@at@acme.com']) { + const error = findError({ users: [{ email }] }, 'users[0].email'); + expect(error.message).toMatch(/email (is required and must be a string|must be a valid email address)/); + } + }); + + it('rejects a seeded invitation email that could only be a typo', () => { + const error = findError({ invitations: [{ email: 'not-an-email' }] }, 'invitations[0].email'); + expect(error.message).toContain('must be a valid email address'); + }); + + it('rejects a membership email that could only be a typo', () => { + const error = findError( + { users: [{ email: 'admin@acme.com' }], organizations: [{ name: 'Acme', memberships: [{ email: 'nope' }] }] }, + 'organizations[0].memberships[0].email', + ); + expect(error.message).toContain('must be a valid email address'); + }); + + // Seeding trims, so the cross-reference has to: matching the raw value would reject a + // membership that resolves fine once both addresses are stored the way the store stores them. + it('accepts a membership email padded differently from its user', () => { + const { valid } = validateSeedConfig({ + users: [{ email: ' admin@acme.com' }], + organizations: [{ name: 'Acme', memberships: [{ email: 'admin@acme.com ' }] }], + }); + expect(valid).toBe(true); + }); + + it('rejects two seeded users whose emails differ only in padding', () => { + const error = findError({ users: [{ email: 'dup@acme.com' }, { email: ' dup@acme.com ' }] }, 'users[1].email'); + expect(error.message).toContain('unique across users'); + }); + it('rejects a membership when no users are defined at all', () => { findError( { organizations: [{ name: 'Acme', memberships: [{ email: 'admin@acme.com' }] }] },