From e3bc34d12b9e5ff06f6167bb9e6134bc3f6ce081 Mon Sep 17 00:00:00 2001 From: Max schwenk Date: Wed, 12 Aug 2026 22:22:24 -0400 Subject: [PATCH] Use indexed WorkOS lookup for admin user emails --- apps/cloud/src/admin/admin-users-api.ts | 38 +++-- .../src/admin/admin-users-email.node.test.ts | 145 ++++++++---------- apps/cloud/src/auth/workos.ts | 11 ++ 3 files changed, 93 insertions(+), 101 deletions(-) diff --git a/apps/cloud/src/admin/admin-users-api.ts b/apps/cloud/src/admin/admin-users-api.ts index 6127f95f47..351f8d46e4 100644 --- a/apps/cloud/src/admin/admin-users-api.ts +++ b/apps/cloud/src/admin/admin-users-api.ts @@ -25,6 +25,7 @@ // every query by that tenant. // --------------------------------------------------------------------------- +import { env } from "cloudflare:workers"; import { HttpRouter } from "effect/unstable/http"; import { Context, Effect, Layer, Option } from "effect"; @@ -188,24 +189,16 @@ const identityDirectory = /** * Cloud's REVERSE directory lookup: email → the WorkOS `user_...` id. * - * WHY THE MEMBER LIST AND NOT A SERVER-SIDE EMAIL QUERY. WorkOS's SDK does - * expose `listUsers({ email })`, which would be one indexed call instead of a - * scan — but the pinned `@executor-js/emulate` WorkOS emulator serves no - * list-users route at all. Its route table was read directly from the shipped - * bundle: under `/user_management` it has only `users/:id`, - * `organization_memberships`, the auth/session routes, and invitations. Taking - * the `listUsers` path would therefore 404 in every cloud e2e run and could - * only be exercised in production — so the lookup is built on the SAME - * membership read the forward join already makes, which the emulator does - * serve. + * Production asks WorkOS for the email AND organization in one request. Both + * filters matter: email makes the lookup indexed rather than one `getUser` + * request per member, while organization keeps the reverse lookup bound to the + * same tenant as the platform view. * - * THE TRADEOFF is an in-memory scan of the org's members, bounded by org size - * and capped by the identity fan-out below. That is acceptable at the sizes - * this plane serves, and it has a real correctness advantage: the membership - * list is the authority on who belongs to THIS org, so a resolver built on it - * cannot return a user from another tenant, which a bare `listUsers({ email })` - * could. If org sizes ever make the scan hurt, the fix is a server-side query - * gated on emulator support, not a cache. + * The pinned `@executor-js/emulate` WorkOS emulator has no list-users route. + * `WORKOS_API_URL` is the explicit test/dev emulator override, so that path + * retains the membership scan until the emulator supports the production + * query. The fallback still starts from the tenant's membership list and can + * never return a user from another organization. * * CASING: WorkOS preserves whatever casing an email was created with (and the * emulator compares byte-exact), so the directory value is normalized here @@ -216,12 +209,17 @@ export const emailResolver = (email) => Effect.gen(function* () { const workos = yield* WorkOSClient; + + if (!env.WORKOS_API_URL) { + const users = yield* workos.listUsers({ email, organizationId }); + return users.data[0]?.id ?? null; + } + const memberships = yield* workos.listOrgMembers(organizationId); const userIds = memberships.data.map((membership) => membership.userId); - // Short-circuits on the first match: `Effect.findFirst` stops fetching - // once a user's email matches, so the common case costs far fewer reads - // than the member count. + // Emulator compatibility only. Short-circuit once the normalized email + // matches so the fallback makes as few unsupported-detail reads as it can. const match = yield* Effect.findFirst(userIds, (userId) => workos.getUser(userId).pipe( Effect.map((user) => normalizeAdminUserEmail(user.email ?? "") === email), diff --git a/apps/cloud/src/admin/admin-users-email.node.test.ts b/apps/cloud/src/admin/admin-users-email.node.test.ts index 08aa3168fb..463c83df09 100644 --- a/apps/cloud/src/admin/admin-users-email.node.test.ts +++ b/apps/cloud/src/admin/admin-users-email.node.test.ts @@ -1,3 +1,4 @@ +import { env } from "cloudflare:workers"; import { expect, it } from "@effect/vitest"; import { Data, Effect, Layer } from "effect"; @@ -5,140 +6,122 @@ import { WorkOSClient, type WorkOSClientService } from "../auth/workos"; import { emailResolver } from "./admin-users-api"; // Cloud's REVERSE directory lookup: email -> the WorkOS `user_...` id that the -// subject table records in `external_id`. -// -// WHY THIS IS BUILT ON THE MEMBERSHIP LIST. WorkOS's SDK exposes -// `listUsers({ email })`, but the pinned `@executor-js/emulate` WorkOS emulator -// serves no list-users route at all — under `/user_management` it has only -// `users/:id`, `organization_memberships`, the auth/session routes, and -// invitations. A resolver built on `listUsers` would therefore 404 in every -// cloud e2e run. So the lookup reuses the SAME membership read the forward -// identity join already makes, which the emulator does serve, and scans it in -// memory (bounded by org size). -// -// The membership list is also the authority on who belongs to THIS org, so the -// resolver structurally cannot return a user from another tenant — something a -// bare `listUsers({ email })` could do. +// subject table records in `external_id`. Production resolves it with one +// tenant-scoped list-users query. The WorkOS emulator lacks that route, so +// tests/dev retain the membership-backed scan exercised below. const ORG = "org_placeholder"; +const OTHER_ORG = "org_other"; -/** The failure a real WorkOS read raises for one user. Typed rather than a bare - * `Error`, so the resolver's skip-and-continue behaviour is exercised against - * the shape the client actually fails with. */ class WorkOSUnavailable extends Data.TaggedError("WorkOSUnavailable")<{ readonly userId: string; }> {} -/** Members of the org, with the casing WorkOS would have stored. WorkOS - * preserves whatever casing a user was created with (and the emulator - * compares byte-exact), so the mixed-case entry is the realistic case. */ -const DIRECTORY: Record = { - user_ada: { email: "Ada@Placeholder.test" }, - user_grace: { email: "grace@placeholder.test" }, - // A member the directory cannot name: `getUser` succeeds but carries no - // email. It must never match, and must not break the scan for others. - user_nameless: { email: null }, -}; +const DIRECTORY = [ + // Same email in another tenant must never win either lookup path. + { id: "user_foreign", email: "ada@placeholder.test", organizationId: OTHER_ORG }, + // WorkOS preserves submitted casing, while the resolver seam is normalized. + { id: "user_ada", email: "Ada@Placeholder.test", organizationId: ORG }, + { id: "user_grace", email: "grace@placeholder.test", organizationId: ORG }, + { id: "user_nameless", email: null, organizationId: ORG }, +] as const; -const stubWorkOS = (calls: string[]) => +const stubWorkOS = (calls: string[], unreadableUserIds: ReadonlySet) => Layer.succeed( WorkOSClient, new Proxy({} as WorkOSClientService, { get: (_target, prop) => { + if (prop === "listUsers") { + return (params: { email: string; organizationId: string }) => { + calls.push(`listUsers:${params.organizationId}:${params.email}`); + return Effect.succeed({ + data: DIRECTORY.filter( + (user) => + user.organizationId === params.organizationId && + user.email?.toLowerCase() === params.email, + ), + }); + }; + } if (prop === "listOrgMembers") { return (organizationId: string) => { calls.push(`listOrgMembers:${organizationId}`); return Effect.succeed({ - data: Object.keys(DIRECTORY).map((userId) => ({ userId, organizationId })), + data: DIRECTORY.filter((user) => user.organizationId === organizationId).map( + (user) => ({ userId: user.id, organizationId }), + ), }); }; } if (prop === "getUser") { return (userId: string) => { calls.push(`getUser:${userId}`); - const user = DIRECTORY[userId]; + if (unreadableUserIds.has(userId)) { + return Effect.fail(new WorkOSUnavailable({ userId })); + } + const user = DIRECTORY.find((candidate) => candidate.id === userId); if (!user) return Effect.die(`unexpected user ${userId}`); return Effect.succeed(user); }; } - // A resolver that reaches for any other WorkOS call — notably a - // `listUsers` the emulator cannot serve — is the failure this catches. return () => Effect.die(`unexpected WorkOSClient.${String(prop)} call`); }, }), ); -const resolve = (email: string, calls: string[]) => - Effect.gen(function* () { +const resolve = ( + email: string, + calls: string[], + emulator = false, + unreadableUserIds: ReadonlySet = new Set(), +) => { + const previousApiUrl = env.WORKOS_API_URL; + return Effect.gen(function* () { + yield* Effect.sync(() => + Object.assign(env, { + WORKOS_API_URL: emulator ? "http://workos-emulator.invalid" : undefined, + }), + ); const context = yield* Effect.context(); return yield* emailResolver(ORG, context)(email); - }).pipe(Effect.provide(stubWorkOS(calls))); + }).pipe( + Effect.provide(stubWorkOS(calls, unreadableUserIds)), + Effect.ensuring(Effect.sync(() => Object.assign(env, { WORKOS_API_URL: previousApiUrl }))), + ); +}; -it.effect("resolves an email to the member's WorkOS user id", () => +it.effect("resolves an email with one tenant-scoped WorkOS query", () => Effect.gen(function* () { const calls: string[] = []; - // The seam hands the resolver an already-normalized address; the STORED - // value is mixed-case, so this pins that the directory side is normalized - // too. Without that, `Ada@Placeholder.test` would never match. expect(yield* resolve("ada@placeholder.test", calls)).toBe("user_ada"); - expect(calls[0]).toBe(`listOrgMembers:${ORG}`); + expect(calls).toEqual([`listUsers:${ORG}:ada@placeholder.test`]); }), ); -it.effect("returns null for an email no member holds", () => +it.effect("returns null from one query when the organization has no matching email", () => Effect.gen(function* () { const calls: string[] = []; expect(yield* resolve("nobody@placeholder.test", calls)).toBeNull(); - // It looked at everyone before saying no, and a member with no email - // neither matched nor derailed the scan. - expect(calls).toContain("getUser:user_nameless"); + expect(calls).toEqual([`listUsers:${ORG}:nobody@placeholder.test`]); }), ); -it.effect("stops fetching once it finds the match", () => +it.effect("keeps the emulator fallback tenant-scoped and case-insensitive", () => Effect.gen(function* () { const calls: string[] = []; - // `user_ada` is first in the member list, so a short-circuiting scan must - // never reach the members behind it. This is what keeps the in-memory scan - // acceptable at the sizes this plane serves. - expect(yield* resolve("ada@placeholder.test", calls)).toBe("user_ada"); - expect(calls).not.toContain("getUser:user_grace"); - expect(calls).not.toContain("getUser:user_nameless"); + expect(yield* resolve("ada@placeholder.test", calls, true)).toBe("user_ada"); + expect(calls).toEqual([`listOrgMembers:${ORG}`, "getUser:user_ada"]); + expect(calls).not.toContain("getUser:user_foreign"); + expect(calls.some((call) => call.startsWith("listUsers:"))).toBe(false); }), ); -it.effect("does not let one unreadable member hide the real match", () => +it.effect("lets the emulator fallback continue past one unreadable member", () => Effect.gen(function* () { const calls: string[] = []; - const failing = Layer.succeed( - WorkOSClient, - new Proxy({} as WorkOSClientService, { - get: (_target, prop) => { - if (prop === "listOrgMembers") { - return () => - Effect.succeed({ - data: [{ userId: "user_broken" }, { userId: "user_grace" }], - }); - } - if (prop === "getUser") { - return (userId: string) => { - calls.push(userId); - return userId === "user_broken" - ? Effect.fail(new WorkOSUnavailable({ userId })) - : Effect.succeed(DIRECTORY[userId]!); - }; - } - return () => Effect.die(`unexpected WorkOSClient.${String(prop)} call`); - }, - }), + expect(yield* resolve("grace@placeholder.test", calls, true, new Set(["user_ada"]))).toBe( + "user_grace", ); - - const resolved = yield* Effect.gen(function* () { - const context = yield* Effect.context(); - return yield* emailResolver(ORG, context)("grace@placeholder.test"); - }).pipe(Effect.provide(failing)); - - expect(resolved, "an unreadable member is skipped, not fatal").toBe("user_grace"); - expect(calls).toEqual(["user_broken", "user_grace"]); + expect(calls).toEqual([`listOrgMembers:${ORG}`, "getUser:user_ada", "getUser:user_grace"]); }), ); diff --git a/apps/cloud/src/auth/workos.ts b/apps/cloud/src/auth/workos.ts index 6b101f3417..88281115af 100644 --- a/apps/cloud/src/auth/workos.ts +++ b/apps/cloud/src/auth/workos.ts @@ -598,6 +598,17 @@ const make = Effect.gen(function* () { /** Get a user by ID. */ getUser: (userId: string) => use((wos) => wos.userManagement.getUser(userId)), + /** List users matching an email within one organization. */ + listUsers: (params: { email: string; organizationId: string }) => + use(async (wos) => + collectWorkOSList( + await wos.userManagement.listUsers({ + email: params.email, + organizationId: params.organizationId, + }), + ), + ), + /** Send an organization invitation. */ sendInvitation: (params: { email: string; organizationId: string; roleSlug?: string }) => use((wos) =>