fix(webapp): accept Plain customers without an external id on customer cards (#4575)

Plain sends `customer.externalId` as an explicit `null` rather than
omitting the key. The schema validated it with `z.string().optional()`,
which accepts `undefined` but rejects `null`, so every customer we don't
set an `externalId` for got a 400 instead of a card — while the rest
worked, which made it look intermittent.

`email`, `externalId` and `thread` are now `nullish`. One of
email/externalId is still required, and the route's existing email
fallback resolves these customers.

Three related fixes in the same path:

- The route returned `{ cards: [] }` when no user matched. Plain records
an integration error for any requested key it doesn't get back, so that
surfaced as a broken card rather than a hidden one. Every requested key
is now answered, with `components: null` where there's no data.
- The impersonation link is offered only when the customer matched on
`externalId` — a value we set ourselves. An email match is a weaker
claim, since the address on a Plain customer isn't verified and for
customers created outside our own writes it comes from whoever sent the
message. Email-matched customers get the account rows without a
one-click impersonation link.
- The not-found log recorded raw customer identifiers; it now keeps
presence flags only.

The schema and the response helper moved to
`app/utils/plainCustomerCards.ts` so they can be unit-tested without
pulling in the db and env modules.

## Testing

`app/utils/plainCustomerCards.test.ts` — 11 tests covering the null
shapes, the every-key-answered response, and the missing-vs-zero
distinction. Verified locally.

Split out of #4571, which bundled this with an unrelated impersonation
fix.

.server-changes/plain-customer-card-external-id.md

@@ -0,0 +1,6 @@
+---
+area: webapp
+type: fix
+---
+
+Fixed support threads showing no account details for some customers, so the team can see your plan, organizations and projects when you get in touch.

apps/webapp/app/routes/api.v1.plain.customer-cards.ts

@@ -1,31 +1,15 @@
 import { json, type ActionFunctionArgs } from "@remix-run/server-runtime";
 import { timingSafeEqual } from "crypto";
 import { uiComponent } from "@team-plain/ui-components";
-import { z } from "zod";
 import { prisma } from "~/db.server";
 import { env } from "~/env.server";
 import { logger } from "~/services/logger.server";
 import { generateImpersonationToken } from "~/services/impersonation.server";
-
-// Schema for the request body from Plain
-const PlainCustomerCardRequestSchema = z.object({
-  cardKeys: z.array(z.string()),
-  customer: z
-    .object({
-      id: z.string(),
-      email: z.string().optional(),
-      externalId: z.string().optional(),
-    })
-    .refine((data) => data.email || data.externalId, {
-      message: "Either customer.email or customer.externalId must be provided",
-      path: ["customer"],
-    }),
-  thread: z
-    .object({
-      id: z.string(),
-    })
-    .optional(),
-});
+import {
+  answerAllCardKeys,
+  emailLookupCandidates,
+  PlainCustomerCardRequestSchema,
+} from "~/utils/plainCustomerCards";
 
 function sanitizeHeaders(
   request: Request,
@@ -133,22 +117,55 @@ export async function action({ request }: ActionFunctionArgs) {
       },
     };
 
-    const where = customer.externalId
-      ? { id: customer.externalId }
-      : customer.email
-        ? { email: customer.email }
-        : null;
+    // The external id is ours (`User.id`), so it's tried first. Falling back to email when it
+    // doesn't resolve covers a stale id — one naming a user row that no longer exists — instead of
+    // leaving the card blank for a customer we could still identify.
+    const byExternalId = customer.externalId
+      ? await prisma.user.findFirst({ where: { id: customer.externalId }, include: userInclude })
+      : null;
 
-    const user = where ? await prisma.user.findFirst({ where, include: userInclude }) : null;
+    // Emails aren't stored consistently cased, so try the address as sent and then its lowercased
+    // form — see `emailLookupCandidates`. Both are exact matches on the unique index.
+    const findByEmail = async () => {
+      for (const email of emailLookupCandidates(customer.email)) {
+        const match = await prisma.user.findFirst({ where: { email }, include: userInclude });
+        if (match) return match;
+      }
+      return null;
+    };
 
-    // If user not found, return empty cards
+    const user = byExternalId ?? (await findByEmail());
+
+    // An external id we set ourselves that no longer resolves is an anomaly worth seeing, even
+    // though the email match keeps the card useful — otherwise the stale link stays invisible.
+    if (customer.externalId && !byExternalId) {
+      logger.warn("Plain customer card external id did not resolve", {
+        resolvedByEmail: !!user,
+      });
+    }
+
+    /**
+     * Impersonation is offered only when the customer matched on `externalId` — a value we set
+     * ourselves from `User.id`.
+     *
+     * Matching on email is a weaker claim: the address on a Plain customer isn't verified, and for
+     * customers created outside our own writes it comes from whoever sent the message. Offering a
+     * one-click impersonation link off the back of that would let an unverified address stand in
+     * for an account, so email-matched customers get the account rows without it.
+     *
+     * Derived from which lookup actually matched, not from whether an external id was *sent* — an
+     * id that misses and falls through to email must not unlock impersonation.
+     */
+    const canImpersonate = Boolean(byExternalId);
+
+    // No matching user: still answer every requested key, with no data so Plain hides the cards.
     if (!user) {
+      // Presence flags only — the identifiers themselves don't need to persist in log storage.
       logger.info("User not found for Plain customer card request", {
-        customerId: customer.id,
-        externalId: customer.externalId,
+        hasExternalId: !!customer.externalId,
         hasEmail: !!customer.email,
       });
-      return json({ cards: [] });
+      return json({ cards: answerAllCardKeys(cardKeys, []) });
     }
 
     // Build cards based on requested cardKeys
@@ -158,10 +175,21 @@ export async function action({ request }: ActionFunctionArgs) {
     for (const cardKey of cardKeys) {
       switch (cardKey) {
         case accountDetailsKey: {
-          // Generate a signed one-time token for impersonation
-          const impersonationToken = await generateImpersonationToken(user.id);
-          // Build the impersonate URL with token for CSRF protection
-          const impersonateUrl = `${env.APP_ORIGIN}/admin/impersonate?impersonate=${user.id}&impersonationToken=${encodeURIComponent(impersonationToken)}`;
+          // Only mint a token when the button will actually be rendered — see `canImpersonate`.
+          const impersonationComponents = canImpersonate
+            ? [
+                uiComponent.spacer({ size: "M" }),
+                uiComponent.divider({ spacingSize: "M" }),
+                uiComponent.spacer({ size: "M" }),
+                uiComponent.linkButton({
+                  label: "Impersonate User",
+                  // The one-time token is what protects this link against CSRF.
+                  url: `${env.APP_ORIGIN}/admin/impersonate?impersonate=${user.id}&impersonationToken=${encodeURIComponent(
+                    await generateImpersonationToken(user.id)
+                  )}`,
+                }),
+              ]
+            : [];
 
           cards.push({
             key: accountDetailsKey,
@@ -241,13 +269,7 @@ export async function action({ request }: ActionFunctionArgs) {
                       }),
                     ],
                   }),
-                  uiComponent.spacer({ size: "M" }),
-                  uiComponent.divider({ spacingSize: "M" }),
-                  uiComponent.spacer({ size: "M" }),
-                  uiComponent.linkButton({
-                    label: "Impersonate User",
-                    url: impersonateUrl,
-                  }),
+                  ...impersonationComponents,
                 ],
               }),
             ],
@@ -420,13 +442,13 @@ export async function action({ request }: ActionFunctionArgs) {
         }
 
         default:
-          // Unknown card key - skip it
+          // Unknown card key - answered with no data by answerAllCardKeys below.
           logger.info("Unknown card key requested", { cardKey });
           break;
       }
     }
 
-    return json({ cards });
+    return json({ cards: answerAllCardKeys(cardKeys, cards) });
   } catch (error) {
     logger.error("Error processing Plain customer card request", {
       error: error instanceof Error ? error.message : String(error),

apps/webapp/app/utils/plainCustomerCards.test.ts

@@ -0,0 +1,128 @@
+import { describe, expect, it } from "vitest";
+import {
+  answerAllCardKeys,
+  emailLookupCandidates,
+  PlainCustomerCardRequestSchema,
+} from "./plainCustomerCards";
+
+const request = (overrides: Record<string, unknown> = {}) => ({
+  cardKeys: ["account-details"],
+  customer: { id: "c_1", email: "dev@example.com", externalId: "user_1" },
+  ...overrides,
+});
+
+describe("PlainCustomerCardRequestSchema", () => {
+  it("accepts a fully populated request", () => {
+    expect(
+      PlainCustomerCardRequestSchema.safeParse(request({ thread: { id: "th_1" } })).success
+    ).toBe(true);
+  });
+
+  // Plain sends explicit nulls rather than omitting these keys. Rejecting them meant every
+  // customer created outside our own writes got a 400 instead of a card.
+  it("accepts a null externalId when there is an email", () => {
+    const result = PlainCustomerCardRequestSchema.safeParse(
+      request({ customer: { id: "c_1", email: "dev@example.com", externalId: null } })
+    );
+
+    expect(result.success).toBe(true);
+  });
+
+  it("accepts a null email when there is an externalId", () => {
+    const result = PlainCustomerCardRequestSchema.safeParse(
+      request({ customer: { id: "c_1", email: null, externalId: "user_1" } })
+    );
+
+    expect(result.success).toBe(true);
+  });
+
+  it("accepts a null thread", () => {
+    expect(PlainCustomerCardRequestSchema.safeParse(request({ thread: null })).success).toBe(true);
+  });
+
+  it("accepts an omitted thread", () => {
+    expect(PlainCustomerCardRequestSchema.safeParse(request()).success).toBe(true);
+  });
+
+  // A contact created by an integration can have neither identifier. There's nothing to look up,
+  // but rejecting it would make Plain record an integration error rather than hide the card.
+  it("accepts a customer with neither email nor externalId", () => {
+    const result = PlainCustomerCardRequestSchema.safeParse(
+      request({ customer: { id: "c_1", email: null, externalId: null } })
+    );
+
+    expect(result.success).toBe(true);
+  });
+
+  it("rejects a body with no card keys field", () => {
+    expect(PlainCustomerCardRequestSchema.safeParse({ customer: { id: "c_1" } }).success).toBe(
+      false
+    );
+  });
+});
+
+// `User.email` casing depends on the signup path: the SSO upsert lowercases, magic-link and OAuth
+// store what the provider gave. Either candidate alone misses one of those populations.
+describe("emailLookupCandidates", () => {
+  it("tries the address as sent before its lowercased form", () => {
+    // Finds a magic-link user stored with capitals, then an SSO user stored lowercased.
+    expect(emailLookupCandidates("Dev@Example.com")).toEqual([
+      "Dev@Example.com",
+      "dev@example.com",
+    ]);
+  });
+
+  it("yields a single candidate when the address is already lowercase", () => {
+    expect(emailLookupCandidates("dev@example.com")).toEqual(["dev@example.com"]);
+  });
+
+  it("trims before comparing, so padding doesn't produce a duplicate candidate", () => {
+    expect(emailLookupCandidates("  dev@example.com  ")).toEqual(["dev@example.com"]);
+  });
+
+  it("is empty for absent or blank addresses, so the lookup can be skipped", () => {
+    expect(emailLookupCandidates(null)).toEqual([]);
+    expect(emailLookupCandidates(undefined)).toEqual([]);
+    expect(emailLookupCandidates("")).toEqual([]);
+    expect(emailLookupCandidates("   ")).toEqual([]);
+  });
+});
+
+describe("answerAllCardKeys", () => {
+  it("adds a no-data card for every unanswered key", () => {
+    expect(answerAllCardKeys(["a", "b"], [])).toEqual([
+      { key: "a", components: null, timeToLiveSeconds: 60 },
+      { key: "b", components: null, timeToLiveSeconds: 60 },
+    ]);
+  });
+
+  it("leaves answered cards untouched", () => {
+    const answered = { key: "a", components: [{ componentText: { text: "hi" } }] };
+
+    expect(answerAllCardKeys(["a"], [answered])).toEqual([answered]);
+  });
+
+  it("fills only the gaps, keeping answered cards first", () => {
+    const answered = { key: "b", components: [] };
+
+    expect(answerAllCardKeys(["a", "b", "c"], [answered])).toEqual([
+      answered,
+      { key: "a", components: null, timeToLiveSeconds: 60 },
+      { key: "c", components: null, timeToLiveSeconds: 60 },
+    ]);
+  });
+
+  // Omitting the TTL would fall back to the card's configured default, keeping an empty card in
+  // Plain's cache after the customer becomes resolvable.
+  it("caps how long an empty card is cached", () => {
+    const [filler] = answerAllCardKeys(["a"], []);
+
+    expect(filler).toMatchObject({ timeToLiveSeconds: 60 });
+  });
+
+  it("ignores extra cards that were not requested", () => {
+    const extra = { key: "unrequested", components: [] };
+
+    expect(answerAllCardKeys([], [extra])).toEqual([extra]);
+  });
+});

apps/webapp/app/utils/plainCustomerCards.ts

@@ -0,0 +1,92 @@
+import { z } from "zod";
+
+/**
+ * The request Plain sends to a customer card endpoint.
+ *
+ * `email`, `externalId` and `thread` are nullish rather than optional because Plain sends these
+ * keys as explicit nulls rather than omitting them — `externalId` whenever the customer was
+ * created outside our own writes (its Slack integration, for one), `thread` when the card is
+ * loaded on the customer page rather than in a thread. `.optional()` accepts `undefined` but
+ * rejects `null`, which failed the whole request before any lookup could run.
+ */
+export const PlainCustomerCardRequestSchema = z.object({
+  cardKeys: z.array(z.string()),
+  // A customer with neither an email nor an external id is valid input, not a malformed request:
+  // a contact created by an integration can legitimately have neither. There's nothing to look up,
+  // so the route answers every key with no data — rejecting it would make Plain record an
+  // integration error, which is the failure this schema change exists to remove.
+  customer: z.object({
+    id: z.string(),
+    email: z.string().nullish(),
+    externalId: z.string().nullish(),
+  }),
+  thread: z
+    .object({
+      id: z.string(),
+    })
+    .nullish(),
+});
+
+export type PlainCustomerCardRequest = z.infer<typeof PlainCustomerCardRequestSchema>;
+
+/**
+ * The values to try, in order, when looking a user up by email.
+ *
+ * `User.email` is not stored consistently cased: the SSO upsert writes
+ * `email.toLowerCase().trim()`, while `findOrCreateMagicLinkUser` and the OAuth paths store
+ * whatever the provider gave us. So neither an exact match nor a lowercased one finds everybody —
+ * exact misses an SSO user whose address arrives capitalised, lowercased misses a magic-link user
+ * stored with capitals.
+ *
+ * Hence two candidates: the address as sent (trimmed), then its lowercased form. Both are exact
+ * matches, so each uses the unique index on `email` — a case-insensitive query would not, and this
+ * table is far too big to scan. The common case hits on the first.
+ *
+ * Empty when there's no usable address, so callers can skip the lookup entirely.
+ */
+export function emailLookupCandidates(email: string | null | undefined): string[] {
+  const asSent = email?.trim();
+  if (!asSent) return [];
+
+  const lowercased = asSent.toLowerCase();
+  return asSent === lowercased ? [asSent] : [asSent, lowercased];
+}
+
+type NoDataCard = { key: string; components: null; timeToLiveSeconds: number };
+
+/**
+ * How long Plain may cache a card we had no data for.
+ *
+ * Explicit rather than omitted: omitting the field falls back to the TTL configured for that card
+ * in Plain's settings, so a customer who becomes resolvable — an external id gets set, or someone
+ * signs up with that address — would keep showing an empty card for however long that default is.
+ * Short enough to recover promptly, long enough not to re-ask on every glance at a thread.
+ */
+const NO_DATA_TTL_SECONDS = 60;
+
+/**
+ * Fills in a `components: null` card for every requested key that wasn't answered.
+ *
+ * Plain records an integration error against any key it asked for and didn't get back, so a
+ * partial response surfaces in the support app as a broken card. `components: null` is how you
+ * say "this card has no data" and have Plain hide it instead.
+ */
+export function answerAllCardKeys<TCard extends { key: string }>(
+  cardKeys: string[],
+  cards: TCard[]
+): (TCard | NoDataCard)[] {
+  const answered = new Set(cards.map((card) => card.key));
+
+  return [
+    ...cards,
+    ...cardKeys
+      .filter((key) => !answered.has(key))
+      .map(
+        (key): NoDataCard => ({
+          key,
+          components: null,
+          timeToLiveSeconds: NO_DATA_TTL_SECONDS,
+        })
+      ),
+  ];
+}