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

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

email, externalId and thread are now nullish; one of email/externalId is still
required, and the existing email fallback resolves these customers.

Two 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 rendered
  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 — the address on a
  Plain customer isn't verified and, for customers created outside our own
  writes, comes from whoever sent the message — so email-matched customers now
  get the account rows without a one-click impersonation link.
- The not-found log recorded raw customer identifiers; it keeps presence flags
  only.

The schema and the response helper live in app/utils so they can be unit-tested
without pulling in the db and env modules.

Author: isshaddad

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

@@ -1,31 +1,11 @@
 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, PlainCustomerCardRequestSchema } from "~/utils/plainCustomerCards";
 
 function sanitizeHeaders(
   request: Request,
@@ -141,14 +121,25 @@ export async function action({ request }: ActionFunctionArgs) {
 
     const user = where ? await prisma.user.findFirst({ where, include: userInclude }) : null;
 
-    // If user not found, return empty cards
+    /**
+     * Impersonation is offered only when the customer was 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.
+     */
+    const canImpersonate = !!customer.externalId;
+
+    // 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 +149,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 +243,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 +416,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,87 @@
+import { describe, expect, it } from "vitest";
+import { answerAllCardKeys, 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
+    );
+  });
+});
+
+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,59 @@
+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>;
+
+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,
+        })
+      ),
+  ];
+}