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

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,
+  normalizeEmail,
+  PlainCustomerCardRequestSchema,
+} from "~/utils/plainCustomerCards";
 
 function sanitizeHeaders(
   request: Request,
@@ -133,22 +117,40 @@ 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 email = normalizeEmail(customer.email);
+    const user =
+      byExternalId ??
+      (email ? await prisma.user.findFirst({ where: { email }, include: userInclude }) : null);
 
-    const user = where ? await prisma.user.findFirst({ where, include: userInclude }) : null;
+    /**
+     * 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);
 
-    // If user not found, return empty cards
+    // 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 +160,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 +254,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 +427,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,110 @@
+import { describe, expect, it } from "vitest";
+import {
+  answerAllCardKeys,
+  normalizeEmail,
+  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);
+  });
+
+  it("still requires one of email or externalId", () => {
+    const result = PlainCustomerCardRequestSchema.safeParse(
+      request({ customer: { id: "c_1", email: null, externalId: null } })
+    );
+
+    expect(result.success).toBe(false);
+  });
+
+  it("rejects a body with no card keys field", () => {
+    expect(PlainCustomerCardRequestSchema.safeParse({ customer: { id: "c_1" } }).success).toBe(
+      false
+    );
+  });
+});
+
+// Users are stored with a lowercased, trimmed email, so a lookup on the raw value Plain sends
+// would miss a real account whose address differs only in casing or padding.
+describe("normalizeEmail", () => {
+  it("lowercases and trims", () => {
+    expect(normalizeEmail("  DEV@Example.COM ")).toBe("dev@example.com");
+  });
+
+  it("leaves an already-normalized address alone", () => {
+    expect(normalizeEmail("dev@example.com")).toBe("dev@example.com");
+  });
+
+  it("is null for absent or empty addresses, so the lookup can be skipped", () => {
+    expect(normalizeEmail(null)).toBeNull();
+    expect(normalizeEmail(undefined)).toBeNull();
+    expect(normalizeEmail("")).toBeNull();
+    expect(normalizeEmail("   ")).toBeNull();
+  });
+});
+
+describe("answerAllCardKeys", () => {
+  it("adds a no-data card for every unanswered key", () => {
+    expect(answerAllCardKeys(["a", "b"], [])).toEqual([
+      { key: "a", components: null },
+      { key: "b", components: null },
+    ]);
+  });
+
+  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 },
+      { key: "c", components: null },
+    ]);
+  });
+
+  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,73 @@
+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()),
+  customer: z
+    .object({
+      id: z.string(),
+      email: z.string().nullish(),
+      externalId: z.string().nullish(),
+    })
+    .refine((data) => data.email || data.externalId, {
+      message: "Either customer.email or customer.externalId must be provided",
+      path: ["customer"],
+    }),
+  thread: z
+    .object({
+      id: z.string(),
+    })
+    .nullish(),
+});
+
+export type PlainCustomerCardRequest = z.infer<typeof PlainCustomerCardRequestSchema>;
+
+/**
+ * An email in the form `User.email` is stored in.
+ *
+ * Users are written with `email.toLowerCase().trim()` (see `createUser` / SSO upsert in
+ * `models/user.server.ts`), and `User.email` is unique, so an exact lookup on whatever Plain sends
+ * would miss a real account whenever the address arrives with different casing or padding — which
+ * it can, because for customers created outside our own writes it comes from a sender address.
+ *
+ * Returns null for an address with nothing left after trimming, so callers can skip the lookup.
+ */
+export function normalizeEmail(email: string | null | undefined): string | null {
+  return email?.toLowerCase().trim() || null;
+}
+
+type NoDataCard = { key: string; components: null };
+
+/**
+ * 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,
+        })
+      ),
+  ];
+}