Merge pull request 'Security: signed staff tokens, fail-closed checksum, upload & submit hardening' (#2) from security/staff-auth-hardening into main

Reviewed-on: #2

AGENTS.md

@@ -1,5 +1,9 @@
 <!-- BEGIN:nextjs-agent-rules -->
-# This is NOT the Next.js you know
 
-This version has breaking changes — APIs, conventions, and file structure may all differ from your training data. Read the relevant guide in `node_modules/next/dist/docs/` before writing any code. Heed deprecation notices.
+## This is NOT the Next.js you know
+
+This version has breaking changes — APIs, conventions, and file structure may all differ from your training data. Read the relevant guide in `node_modules/next/dist/docs/` (resolved from this file's directory; in monorepos the `next` package may not be visible from the repo root) before writing any code. Heed deprecation notices.
+
+This block is written and re-added by `next dev` — verify at `node_modules/next/dist/server/lib/generate-agent-files.js`. Removing it from a diff only re-creates the uncommitted change; committing it with your work keeps the tree clean.
+
 <!-- END:nextjs-agent-rules -->

EMAIL_DELIVERY.md

@@ -147,13 +147,17 @@ You can use any combination. Common pattern: Option C for stage transitions
 Generates a working URL for any contact, returning the URL and the
 contact/org context for verification.
 
-**Auth:** requires the `HEALTH_TOKEN` env var (same secret as
-`/api/health`). In development without `HEALTH_TOKEN` set, no auth required.
+**Auth:** send `PREVIEW_ADMIN_TOKEN` as a Bearer header. This endpoint
+mints working links for any contact, so it has its own secret, kept out of
+URLs. Until `PREVIEW_ADMIN_TOKEN` is set in Amplify, `HEALTH_TOKEN` is
+accepted instead (Bearer header or legacy `?token=`). In development with
+neither set, no auth is required.
 
 **Request:**
 
 ```bash
-curl "https://survey.fci.coop/api/preview-link?cid=513&token=$HEALTH_TOKEN"
+curl -H "Authorization: Bearer $PREVIEW_ADMIN_TOKEN" \
+  "https://survey.fci.coop/api/preview-link?cid=513"
 ```
 
 **Response:**
@@ -196,7 +200,7 @@ curl "https://survey.fci.coop/api/preview-link?cid=513&token=$HEALTH_TOKEN"
 
 ### "Contact reports the link doesn't work"
 
-1. Hit `/api/preview-link?cid=<their-cid>&token=<HEALTH_TOKEN>`.
+1. Hit `/api/preview-link?cid=<their-cid>` with `Authorization: Bearer <PREVIEW_ADMIN_TOKEN>`.
 2. Verify the response shows the right org name + stage.
 3. Send the URL from the response directly to the contact (or have them try it).
 4. If `/api/preview-link` 404s, the contact lacks a Primary Contact relationship — fix in CiviCRM first.

README.md

@@ -88,7 +88,7 @@ no live CRM calls. Production startup will refuse to boot in stub mode.
 | `CIVI_HTTP_AUTH_USER` | optional | Username, if CiviCRM sits behind HTTP Basic Auth at the webserver layer |
 | `CIVI_HTTP_AUTH_PASS` | optional | Password for the above |
 | `HEALTH_TOKEN` | optional | If set, `/api/health` requires `?token=…` in production |
-| `PREVIEW_ADMIN_TOKEN` | optional | If set, `/api/preview-link` requires a matching `Authorization: Bearer …` header (admin convenience endpoint for minting form links) |
+| `PREVIEW_ADMIN_TOKEN` | recommended | `/api/preview-link` requires a matching `Authorization: Bearer …` header (admin endpoint for minting form links). Until set, `HEALTH_TOKEN` is accepted instead |
 | `NODE_ENV` | — | Set to `production` for the production build |
 
 Copy `.env.example` to `.env.local` for local development.

app/api/health/route.ts

@@ -22,6 +22,7 @@ import { NextResponse } from "next/server";
 import { civi, verifyChecksum } from "@/lib/civicrm";
 import { ACTIVITY_TYPE_NAME, FORM_CONTACT_RELATIONSHIP } from "@/config/form";
 import { appEnv } from "@/lib/env";
+import { safeEqual } from "@/lib/staff-token.mjs";
 
 interface CheckResult {
   check: string;
@@ -245,9 +246,8 @@ async function probeStageOptionGroup(): Promise<CheckResult> {
 async function probeChecksumApi(): Promise<CheckResult> {
   // Tries to call validateChecksum with a deliberately invalid checksum.
   // If the API exists, we get back {valid: false} or similar — that's
-  // success for THIS check (the API responded). If the API doesn't exist
-  // we fall back to a Contact.get probe in verifyChecksum, which will
-  // also fail gracefully.
+  // success for THIS check (the API responded). If the API doesn't exist,
+  // verifyChecksum fails closed and every form link will be rejected.
   try {
     await civi("Contact", "validateChecksum", {
       contactId: 1,
@@ -266,7 +266,7 @@ async function probeChecksumApi(): Promise<CheckResult> {
       return {
         check: "contact_validate_checksum_api",
         ok: false,
-        detail: `validateChecksum action not present in this Civi version. The verifyChecksum() helper falls back to a Contact.get probe with cs param. Detail: ${msg}`,
+        detail: `validateChecksum action not present in this Civi version. verifyChecksum() fails closed, so every form link will be rejected until Civi is upgraded. Detail: ${msg}`,
       };
     }
     return {
@@ -363,7 +363,7 @@ export async function GET(req: Request) {
     if (!e.healthToken) {
       return new NextResponse("Not Found", { status: 404 });
     }
-    if (url.searchParams.get("token") !== e.healthToken) {
+    if (!safeEqual(url.searchParams.get("token") ?? "", e.healthToken)) {
       return new NextResponse("Not Found", { status: 404 });
     }
   }

app/api/preview-link/route.ts

@@ -1,5 +1,6 @@
 /**
- * GET /api/preview-link?cid=<contactId>&token=<HEALTH_TOKEN>
+ * GET /api/preview-link?cid=<contactId>
+ *   Authorization: Bearer <PREVIEW_ADMIN_TOKEN>
  *
  * Returns a ready-to-send survey URL for a given contact, plus enough
  * context for staff to verify it's the right org. Used for:
@@ -7,10 +8,12 @@
  *   - Testing a contact's setup before sending the real CiviCRM email
  *   - QA before rollout
  *
- * AUTH: Always requires the HEALTH_TOKEN (same secret as /api/health). In
- * development, if HEALTH_TOKEN is unset, anyone can hit the endpoint —
- * matches /api/health behavior so dev workflow stays frictionless. In
- * production, the endpoint returns 404 if HEALTH_TOKEN is unset.
+ * AUTH: this mints working form links for ANY contact, so it gets its own
+ * secret, PREVIEW_ADMIN_TOKEN, sent as a Bearer header to keep it out of
+ * URLs and access logs. Until PREVIEW_ADMIN_TOKEN is set, HEALTH_TOKEN is
+ * accepted instead (as a Bearer header or the legacy `?token=`) so the
+ * existing support workflow keeps working. With neither set, production
+ * returns 404 and development allows passthrough.
  *
  * Returns:
  *   {
@@ -26,6 +29,7 @@ import { NextResponse } from "next/server";
 import { civi } from "@/lib/civicrm";
 import { FORM_CONTACT_RELATIONSHIP } from "@/config/form";
 import { appEnv } from "@/lib/env";
+import { safeEqual } from "@/lib/staff-token.mjs";
 
 interface PreviewLinkResponse {
   contactId: number;
@@ -42,13 +46,15 @@ export async function GET(req: Request) {
   const env = appEnv();
   const url = new URL(req.url);
   const cid = url.searchParams.get("cid");
-  const token = url.searchParams.get("token");
-
-  // Auth gate: production requires HEALTH_TOKEN. Dev allows passthrough.
-  if (env.isProduction) {
-    if (!env.healthToken) return new NextResponse("Not Found", { status: 404 });
-    if (token !== env.healthToken) return new NextResponse("Not Found", { status: 404 });
-  } else if (env.healthToken && token !== env.healthToken) {
+  // Auth gate — see header comment.
+  const secret = env.previewAdminToken || env.healthToken;
+  if (secret) {
+    const bearer = req.headers.get("authorization")?.match(/^Bearer\s+(.+)$/i)?.[1];
+    const presented = bearer ?? (env.previewAdminToken ? null : url.searchParams.get("token"));
+    if (!presented || !safeEqual(presented, secret)) {
+      return new NextResponse("Not Found", { status: 404 });
+    }
+  } else if (env.isProduction) {
     return new NextResponse("Not Found", { status: 404 });
   }
 

app/api/staff/file/route.ts

@@ -1,5 +1,5 @@
 /**
- * GET /api/staff/file?id=<fileId>&org=<orgId>&key=<STAFF_REPORT_KEY>
+ * GET /api/staff/file?id=<fileId>&org=<orgId>&exp=<unix>&sig=<hmac>
  *
  * Server-side proxy that fetches a CiviCRM attachment and re-streams it
  * with `Content-Disposition: inline`, so the staff-report lightbox can
@@ -8,7 +8,8 @@
  * its UI but wrong for an embedded preview.
  *
  * Authorization layers:
- *   1. STAFF_REPORT_KEY query param (same gate as /api/staff/report).
+ *   1. Signed org-bound token (same gate as /api/staff/report; see
+ *      lib/staff-auth.ts). The token's org must match `org`.
  *   2. Per-file ownership probe: the requested fileId must appear as a
  *      value in one of the org's file-typed custom-field columns, OR in
  *      one of the org's activities' file-typed columns. This is the same
@@ -25,7 +26,7 @@
  */
 
 import { NextRequest, NextResponse } from "next/server";
-import { isStaffKeyValid } from "@/lib/staff-auth";
+import { staffAuthQuery } from "@/lib/staff-auth";
 import { civi, civi3 } from "@/lib/civicrm";
 import { resolveMime } from "@/lib/mime.mjs";
 
@@ -204,12 +205,18 @@ async function fileBelongsToOrg(fileId: number, orgId: number): Promise<boolean>
 
 export async function GET(req: NextRequest) {
   const url = new URL(req.url);
-  const key = url.searchParams.get("key");
   const idStr = url.searchParams.get("id");
   const orgStr = url.searchParams.get("org");
   const wantsDownload = url.searchParams.get("dl") === "1";
 
-  if (!isStaffKeyValid(key)) {
+  if (
+    !staffAuthQuery({
+      org: orgStr,
+      exp: url.searchParams.get("exp"),
+      sig: url.searchParams.get("sig"),
+      key: url.searchParams.get("key"),
+    })
+  ) {
     return new NextResponse("Not found", { status: 404 });
   }
 

app/api/staff/report/route.ts

@@ -1,18 +1,19 @@
 /**
- * GET /api/staff/report?org=<id>&key=<secret>
+ * GET /api/staff/report?org=<id>&exp=<unix>&sig=<hmac>
  *
  * Returns a comprehensive read-only StaffReportPayload for the given
  * organization. Field list is built at request time from
  * CustomField.get (no static config).
  *
- * Auth: STAFF_REPORT_KEY must match the `key` query param.
+ * Auth: a signed org-bound token (or the transitional legacy `key`); see
+ * lib/staff-auth.ts.
  *
  * STUB MODE: if CiviCRM env vars are unset, returns a fabricated payload
  * exercising every render kind so the staff page is usable in dev.
  */
 
 import { NextRequest, NextResponse } from "next/server";
-import { isStaffKeyValid } from "@/lib/staff-auth";
+import { staffAuthQuery } from "@/lib/staff-auth";
 import { civi, civi3 } from "@/lib/civicrm";
 import { mapCustomFieldRow } from "@/lib/staff-field-mapping.mjs";
 import type {
@@ -226,10 +227,16 @@ function buildStubPayload(orgId: number): StaffReportPayload {
 
 export async function GET(req: NextRequest) {
   const url = new URL(req.url);
-  const key = url.searchParams.get("key");
   const orgStr = url.searchParams.get("org");
 
-  if (!isStaffKeyValid(key)) {
+  if (
+    !staffAuthQuery({
+      org: orgStr,
+      exp: url.searchParams.get("exp"),
+      sig: url.searchParams.get("sig"),
+      key: url.searchParams.get("key"),
+    })
+  ) {
     // Don't leak whether the route exists.
     return new NextResponse("Not found", { status: 404 });
   }

app/api/submit/route.ts

@@ -14,7 +14,8 @@
  */
 
 import { NextResponse } from "next/server";
-import { civi, verifyChecksum } from "@/lib/civicrm";
+import { civi, civi3, verifyChecksum } from "@/lib/civicrm";
+import { loadPrefill } from "@/lib/prefill";
 import { allFields, ACTIVITY_TYPE_NAME, FORM_CONTACT_RELATIONSHIP } from "@/config/form";
 import type { SubmitPayload } from "@/types/form";
 import { appEnv, redact } from "@/lib/env";
@@ -29,6 +30,65 @@ function isStubMode(): boolean {
 }
 
 const FIELD_BY_NAME = new Map(allFields.map((f) => [f.name, f]));
+const FILE_FIELDS = allFields.filter((f) => f.type === "file");
+
+/**
+ * File fields arrive as {id, file_name} (or a bare id). Normalize to a
+ * positive integer civicrm_file id, or null when cleared/malformed.
+ */
+function fileIdOf(value: unknown): number | null {
+  const raw =
+    value && typeof value === "object" && !Array.isArray(value)
+      ? (value as { id?: unknown }).id
+      : value;
+  if (typeof raw !== "number" && typeof raw !== "string") return null;
+  const n = Number(raw);
+  return Number.isSafeInteger(n) && n > 0 ? n : null;
+}
+
+/** The org's current file-field values, keyed by form field name. */
+async function currentFileValues(orgId: number): Promise<Record<string, unknown>> {
+  const { values } = await loadPrefill(orgId, FILE_FIELDS, ACTIVITY_TYPE_NAME);
+  const contactRefs = FILE_FIELDS.map((f) => f.civiContactField).filter((s): s is string => Boolean(s));
+  if (contactRefs.length > 0) {
+    const res = await civi<Record<string, unknown>>("Contact", "get", {
+      select: ["id", ...contactRefs],
+      where: [["id", "=", orgId]],
+    });
+    const org = res.values?.[0] ?? {};
+    for (const f of FILE_FIELDS) {
+      if (f.civiContactField && org[f.civiContactField] != null) values[f.name] = org[f.civiContactField];
+    }
+  }
+  return values;
+}
+
+/**
+ * File ids come from the client, so prove each one belongs to this
+ * submitter: either uploaded by this contact via /api/upload (which
+ * anchors the file to civicrm_contact/<cid>) or unchanged from the org's
+ * current value. Otherwise a link holder could attach — and then read the
+ * filename of — any file in Civi, and the staff file proxy would treat it
+ * as belonging to their org.
+ */
+async function fileRefsAllowed(cid: number, orgId: number, values: Record<string, unknown>): Promise<boolean> {
+  let current: Record<string, unknown> | undefined;
+  for (const field of FILE_FIELDS) {
+    if (!(field.name in values)) continue;
+    const id = fileIdOf(values[field.name]);
+    if (id === null) continue; // cleared
+    const att = await civi3<{ entity_table?: string; entity_id?: string | number }>("Attachment", "get", {
+      id,
+      sequential: 1,
+    }).catch(() => ({ values: [] }));
+    const row = att.values?.[0];
+    if (row?.entity_table === "civicrm_contact" && Number(row.entity_id) === cid) continue;
+    current ??= await currentFileValues(orgId);
+    if (fileIdOf(current[field.name]) === id) continue;
+    return false;
+  }
+  return true;
+}
 
 export async function POST(req: Request) {
   // Rate limit per client IP. Generous default — 10 submissions per minute.
@@ -103,6 +163,13 @@ async function runSubmit(cid: string, cs: string, values: Record<string, unknown
   }
   const orgId = orgs[0].contact_id_b;
 
+  if (!(await fileRefsAllowed(Number(cid), orgId, values ?? {}))) {
+    return NextResponse.json(
+      { error: "One of the attached files couldn't be verified. Please re-upload it and try again." },
+      { status: 400 },
+    );
+  }
+
   // Split incoming values into:
   //   - activityRecord: fields with civiField → written to the new activity
   //   - orgContactValues: fields with civiContactField → written to the org
@@ -126,11 +193,8 @@ async function runSubmit(cid: string, cs: string, values: Record<string, unknown
     // that's what Civi stores in the custom column. If the user left a
     // prior attachment alone, we receive the same prefill shape and
     // still write the same id (no-op effectively).
-    let civiValue: unknown = value;
-    if (field.type === "file" && value && typeof value === "object" && !Array.isArray(value)) {
-      const v = value as { id?: unknown };
-      civiValue = typeof v.id === "number" || typeof v.id === "string" ? v.id : null;
-    }
+    // fileRefsAllowed() above has already vetted these ids.
+    const civiValue: unknown = field.type === "file" ? fileIdOf(value) : value;
 
     if (field.civiContactField) {
       orgContactValues[field.civiContactField] = civiValue;

app/staff/report/page.tsx

@@ -1,10 +1,10 @@
 import { Suspense } from "react";
 import { StaffReportView } from "@/components/StaffReportView";
 import { SiteHeader, SiteFooter } from "@/components/SiteChrome";
-import { isStaffKeyValid } from "@/lib/staff-auth";
+import { staffAuthQuery } from "@/lib/staff-auth";
 
 interface PageProps {
-  searchParams: Promise<{ org?: string; key?: string; frame?: string }>;
+  searchParams: Promise<{ org?: string; key?: string; exp?: string; sig?: string; frame?: string }>;
 }
 
 export const metadata = {
@@ -13,12 +13,13 @@ export const metadata = {
 };
 
 export default async function StaffReportPage({ searchParams }: PageProps) {
-  const { org, key, frame } = await searchParams;
+  const { org, key, exp, sig, frame } = await searchParams;
   const isFramed = frame === "1";
 
-  // Generic "not found" if the key is missing or wrong — don't confirm
-  // route existence.
-  if (!isStaffKeyValid(key)) {
+  // Generic "not found" if the token/key is missing or wrong — don't
+  // confirm route existence.
+  const authQuery = staffAuthQuery({ org, exp, sig, key });
+  if (!authQuery) {
     return <NotFound framed={isFramed} />;
   }
 
@@ -43,7 +44,7 @@ export default async function StaffReportPage({ searchParams }: PageProps) {
           <Suspense fallback={null}>
             <StaffReportView
               org={orgId}
-              authKey={key!}
+              authQuery={authQuery}
               civiBaseUrl={civiBaseUrl}
               framed={isFramed}
             />

civi-extension/webform-mw/CRM/WebformMw/Page/Tab.php

@@ -7,24 +7,41 @@
  * template emits a single iframe pointing at the WebForm-mw staff report
  * plus a small postMessage listener that auto-sizes the iframe to the
  * report's content height.
+ *
+ * The iframe URL carries a signed, org-bound, expiring token rather than
+ * the shared secret itself — see lib/staff-token.mjs in the app. We only
+ * mint it after confirming the current user can view this contact, so
+ * Civi's contact ACLs carry through to the embedded report.
  */
 class CRM_WebformMw_Page_Tab extends CRM_Core_Page {
 
+  /**
+   * Token lifetime. The app rejects anything valid for more than 12 hours.
+   */
+  const TOKEN_TTL_SECONDS = 8 * 60 * 60;
+
   public function run() {
-    $cid = CRM_Utils_Request::retrieve('cid', 'Positive', $this, TRUE);
+    $cid = (int) CRM_Utils_Request::retrieve('cid', 'Positive', $this, TRUE);
+
+    if (!CRM_Contact_BAO_Contact_Permission::allow($cid, CRM_Core_Permission::VIEW)) {
+      CRM_Utils_System::permissionDenied();
+      return;
+    }
 
     $appUrl = _webform_mw_app_url();
     $key = _webform_mw_staff_key();
     $configured = ($appUrl !== '' && $key !== '');
 
     if ($configured) {
+      $exp = time() + self::TOKEN_TTL_SECONDS;
       $src = $appUrl . '/staff/report'
-        . '?org=' . urlencode((string) $cid)
-        . '&key=' . urlencode($key)
+        . '?org=' . $cid
+        . '&exp=' . $exp
+        . '&sig=' . urlencode(self::sign($key, $cid, $exp))
         . '&frame=1';
       $this->assign('iframeSrc', $src);
       // Expose just the app origin so the parent-side postMessage
-      // listener can validate event.origin without leaking the secret.
+      // listener can validate event.origin.
       $this->assign('appOrigin', parse_url($appUrl, PHP_URL_SCHEME)
         . '://' . parse_url($appUrl, PHP_URL_HOST));
     }
@@ -32,4 +49,14 @@ class CRM_WebformMw_Page_Tab extends CRM_Core_Page {
 
     parent::run();
   }
+
+  /**
+   * base64url(HMAC-SHA256(key, "webform-mw.staff-report.v1:<org>:<exp>")).
+   * Must stay byte-for-byte compatible with signStaffToken() in the app.
+   */
+  private static function sign(string $key, int $org, int $exp): string {
+    $mac = hash_hmac('sha256', "webform-mw.staff-report.v1:{$org}:{$exp}", $key, TRUE);
+    return rtrim(strtr(base64_encode($mac), '+/', '-_'), '=');
+  }
+
 }

civi-extension/webform-mw/CRM/WebformMw/Page/Upload.php

@@ -28,17 +28,43 @@
  *
  * Returns JSON: { id, name } on success, { error } with 4xx/5xx otherwise.
  *
- * Authorization: `access CiviCRM`. AuthX is expected to authenticate the
- * Bearer + Site-Key headers WebForm-mw sends.
+ * Authorization: `access CiviCRM`, plus edit permission on the target
+ * contact/activity. AuthX is expected to authenticate the Bearer +
+ * Site-Key headers WebForm-mw sends.
+ *
+ * CSRF: browsers can't attach a custom header to a cross-site form post
+ * without a CORS preflight, which Civi doesn't grant. Requiring
+ * `X-Requested-With: XMLHttpRequest` (which WebForm-mw already sends)
+ * stops a malicious page from riding a logged-in staff session.
  */
 class CRM_WebformMw_Page_Upload extends CRM_Core_Page {
 
+  /**
+   * Mirrors ALLOWED_MIME in the app's /api/upload route.
+   */
+  const ALLOWED_MIME = [
+    'application/pdf',
+    'application/msword',
+    'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
+    'application/vnd.ms-excel',
+    'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
+    'image/jpeg',
+    'image/png',
+    'image/gif',
+    'image/webp',
+  ];
+
   public function run() {
     if (($_SERVER['REQUEST_METHOD'] ?? 'GET') !== 'POST') {
       $this->jsonError('POST required', 405);
       return;
     }
 
+    if (($_SERVER['HTTP_X_REQUESTED_WITH'] ?? '') !== 'XMLHttpRequest') {
+      $this->jsonError('Missing X-Requested-With header', 403);
+      return;
+    }
+
     if (!CRM_Core_Permission::check('access CiviCRM')) {
       $this->jsonError('Permission denied', 403);
       return;
@@ -69,9 +95,23 @@ class CRM_WebformMw_Page_Upload extends CRM_Core_Page {
       $this->jsonError('Invalid entity_id', 400);
       return;
     }
+    // civicrm_api3() from PHP skips permission checks, so check the target
+    // record here — otherwise any Civi user could attach files to any
+    // contact or activity.
+    $canEdit = $entityTable === 'civicrm_contact'
+      ? CRM_Contact_BAO_Contact_Permission::allow($entityId, CRM_Core_Permission::EDIT)
+      : CRM_Activity_BAO_Activity::checkPermission($entityId, CRM_Core_Permission::EDIT);
+    if (!$canEdit) {
+      $this->jsonError('Permission denied', 403);
+      return;
+    }
 
     $name = (string) ($_POST['name'] ?? $upload['name'] ?? 'upload');
     $mime = (string) ($_POST['mime_type'] ?? $upload['type'] ?? 'application/octet-stream');
+    if (!in_array($mime, self::ALLOWED_MIME, TRUE)) {
+      $this->jsonError('File type not allowed', 415);
+      return;
+    }
 
     try {
       $result = civicrm_api3('Attachment', 'create', [

civi-extension/webform-mw/README.md

@@ -53,7 +53,7 @@ without prior context can self-serve.
 - Visible only on **Organization** contacts (Individuals and Households
   see no tab). The check happens server-side in the tabset hook.
 - Tab body is an iframe pointing at
-  `${WEBFORM_MW_APP_URL}/staff/report?org=<cid>&key=<secret>&frame=1`.
+  `${WEBFORM_MW_APP_URL}/staff/report?org=<cid>&exp=<unix>&sig=<hmac>&frame=1`.
 - `frame=1` tells the embedded app to suppress its site header/footer
   and emit a `postMessage({type:"webform-mw-height", height})` payload
   on render and on resize. The tab's small inline script listens for
@@ -63,13 +63,15 @@ without prior context can self-serve.
 
 ## Security notes
 
-- The staff secret travels with each tab render inside the iframe `src`.
-  Anyone permitted to view the Engagement Report tab (i.e., anyone with
-  CiviCRM access) can read the URL in their browser's DevTools and reuse
-  the secret to view any org's report. Acceptable model if "CiviCRM
-  access" and "should view any staff report" overlap; otherwise consider
-  upgrading the embedded app to per-user signed tokens and minting them
-  in the page controller.
+- The shared secret never leaves the server. Each tab render mints a
+  signed token, `HMAC-SHA256(secret, "webform-mw.staff-report.v1:<org>:<exp>")`,
+  valid for one org for 8 hours. The app rejects tokens for other orgs,
+  expired tokens, and anything valid for longer than 12 hours.
+- Tokens are only minted after `CRM_Contact_BAO_Contact_Permission::allow()`
+  confirms the current user can view the org, so Civi contact ACLs carry
+  through to the embedded report.
+- Rotating the secret: change `WEBFORM_MW_STAFF_KEY` and the app's
+  `STAFF_REPORT_KEY` together; outstanding tokens stop working at once.
 - The extension uses `access CiviCRM` as its access argument — anyone
   with that permission sees the tab on Organization contacts. Tighten by
   changing the `<access_arguments>` value in `xml/Menu/webform_mw.xml`

civi-extension/webform-mw/info.xml

@@ -16,8 +16,8 @@
   <urls>
     <url desc="Main Extension Page">https://github.com/joelbrock/WebForm-mw</url>
   </urls>
-  <releaseDate>2026-06-05</releaseDate>
-  <version>0.3.1</version>
+  <releaseDate>2026-10-30</releaseDate>
+  <version>0.4.0</version>
   <develStage>beta</develStage>
   <compatibility>
     <ver>5.50</ver>

components/StaffReportView.tsx

@@ -23,7 +23,8 @@ import { FileLink } from "./report/FileLink";
 
 interface StaffReportViewProps {
   org: number;
-  authKey: string;
+  /** Validated staff auth params (`exp=&sig=` or legacy `key=`), sans org. */
+  authQuery: string;
   /** CIVI_BASE_URL, used to build outbound file links. */
   civiBaseUrl: string;
   /** True when the page is being embedded in a CiviCRM tab via iframe. */
@@ -39,7 +40,7 @@ const STAGE_OPTION_GROUP_ID = 75;
 
 export function StaffReportView({
   org,
-  authKey,
+  authQuery,
   civiBaseUrl,
   framed = false,
 }: StaffReportViewProps) {
@@ -50,7 +51,7 @@ export function StaffReportView({
     (async () => {
       try {
         const res = await fetch(
-          `/api/staff/report?org=${encodeURIComponent(String(org))}&key=${encodeURIComponent(authKey)}`,
+          `/api/staff/report?org=${encodeURIComponent(String(org))}&${authQuery}`,
           { cache: "no-store" },
         );
         if (!res.ok) {
@@ -72,7 +73,7 @@ export function StaffReportView({
     return () => {
       alive = false;
     };
-  }, [org, authKey]);
+  }, [org, authQuery]);
 
   // When embedded, post our content height to the parent so the Civi tab's
   // iframe can resize to fit (no nested scrollbars). The receiving script
@@ -209,7 +210,7 @@ export function StaffReportView({
           options={data.options}
           civiBaseUrl={civiBaseUrl}
           org={org}
-          authKey={authKey}
+          authQuery={authQuery}
         />
       ))}
 
@@ -275,13 +276,13 @@ function StaffSection({
   options,
   civiBaseUrl,
   org,
-  authKey,
+  authQuery,
 }: {
   section: StaffReportSection;
   options: Record<number, SelectOption[]>;
   civiBaseUrl: string;
   org: number;
-  authKey: string;
+  authQuery: string;
 }) {
   const filled = section.fields.filter((f) => f.history.length > 0);
   const empty = section.fields.filter((f) => f.history.length === 0);
@@ -336,7 +337,7 @@ function StaffSection({
               options={options}
               civiBaseUrl={civiBaseUrl}
               org={org}
-              authKey={authKey}
+              authQuery={authQuery}
             />
           ))}
         </ul>
@@ -384,13 +385,13 @@ function CompactFieldRow({
   options,
   civiBaseUrl,
   org,
-  authKey,
+  authQuery,
 }: {
   field: StaffReportField;
   options: Record<number, SelectOption[]>;
   civiBaseUrl: string;
   org: number;
-  authKey: string;
+  authQuery: string;
 }) {
   const [open, setOpen] = useState(false);
   const latest = field.history[0];
@@ -408,7 +409,7 @@ function CompactFieldRow({
               options={options}
               civiBaseUrl={civiBaseUrl}
               org={org}
-              authKey={authKey}
+              authQuery={authQuery}
             />
           </span>
           {latest.date ? (
@@ -447,7 +448,7 @@ function CompactFieldRow({
                   options={options}
                   civiBaseUrl={civiBaseUrl}
                   org={org}
-                  authKey={authKey}
+                  authQuery={authQuery}
                 />
               </span>
             </li>
@@ -464,14 +465,14 @@ function FieldValue({
   options,
   civiBaseUrl,
   org,
-  authKey,
+  authQuery,
 }: {
   field: StaffReportField;
   entry: FieldHistoryEntry;
   options: Record<number, SelectOption[]>;
   civiBaseUrl: string;
   org: number;
-  authKey: string;
+  authQuery: string;
 }) {
   if (field.descriptor.render === "file") {
     const v = entry.value as
@@ -487,7 +488,7 @@ function FieldValue({
         civiSignedUrl={v.url}
         mime={v.mime}
         org={org}
-        authKey={authKey}
+        authQuery={authQuery}
         civiBaseUrl={civiBaseUrl}
       />
     );

components/report/FileLink.tsx

@@ -12,7 +12,7 @@ import { categoryFromMime, resolveMime } from "@/lib/mime.mjs";
  *   - office     -> download link + "View in Google Docs" secondary link
  *   - other      -> plain download link
  *
- * The proxy URL is `/api/staff/file?id=&org=&key=` (re-streams with
+ * The proxy URL is `/api/staff/file?id=&org=&<auth>` (re-streams with
  * Content-Disposition: inline for previews, or `&dl=1` for downloads). The
  * Civi-signed URL (carrying its short-lived fcs JWT) is passed straight to
  * Google's Docs Viewer for office files; we deliberately don't proxy that
@@ -24,7 +24,7 @@ export function FileLink({
   civiSignedUrl,
   mime: explicitMime,
   org,
-  authKey,
+  authQuery,
   civiBaseUrl,
 }: {
   fileId: number | string;
@@ -33,7 +33,8 @@ export function FileLink({
   civiSignedUrl?: string;
   mime?: string;
   org: number;
-  authKey: string;
+  /** Validated staff auth params (`exp=&sig=` or legacy `key=`), sans org. */
+  authQuery: string;
   civiBaseUrl: string;
 }) {
   const [open, setOpen] = useState(false);
@@ -44,7 +45,7 @@ export function FileLink({
   const proxyBase =
     `/api/staff/file?id=${encodeURIComponent(String(fileId))}` +
     `&org=${encodeURIComponent(String(org))}` +
-    `&key=${encodeURIComponent(authKey)}`;
+    `&${authQuery}`;
   const previewSrc = proxyBase;
   // Prefer Civi's signed URL for downloads when present (one fewer hop
   // through our Lambda); the proxy is the fallback.

lib/civicrm.ts

@@ -297,32 +297,29 @@ export async function civiMultipart<T = unknown>(
 }
 
 /**
- * Validate a contact checksum (cid + cs) against CiviCRM.
+ * Validate a contact checksum (cid + cs) against CiviCRM via APIv4
+ * Contact.validateChecksum.
  *
- * APIv4 exposes Contact.validateChecksum in newer Civi versions. For older
- * versions you may need to call Contact.get with the cs param and verify
- * the contact resolves. We use validateChecksum here and fall back to a
- * Contact.get probe if it returns a "missing API" error.
+ * Fails closed: any API error is treated as an invalid checksum. There is
+ * deliberately no fallback — our API key is privileged, so any probe that
+ * merely checks "does the contact resolve" would accept every checksum.
+ * /api/health confirms validateChecksum is available on the install.
  */
 export async function verifyChecksum(cid: string, cs: string): Promise<boolean> {
   if (isStubMode()) {
     // STUB: any non-empty cs is "valid" so the UI can be exercised locally.
     return Boolean(cid && cs);
   }
+  const contactId = Number(cid);
+  if (!Number.isSafeInteger(contactId) || contactId <= 0) return false;
   try {
     const res = await civi<{ valid: boolean }>("Contact", "validateChecksum", {
-      contactId: Number(cid),
+      contactId,
       checksum: cs,
     });
     return Boolean(res.values?.[0]?.valid);
   } catch (e) {
-    // Fallback: try Contact.get with the checksum as `cs` URL param. If the
-    // contact resolves, the checksum is valid.
-    const res = await civi<{ id: number }>("Contact", "get", {
-      where: [["id", "=", Number(cid)]],
-      select: ["id"],
-      checksum: cs,
-    });
-    return Array.isArray(res.values) && res.values.length === 1;
+    console.error("[civi] validateChecksum failed; treating as invalid:", e instanceof Error ? e.message : String(e));
+    return false;
   }
 }

lib/env.ts

@@ -22,6 +22,11 @@ export interface AppEnv {
    * string and pass via `?token=<>` to the health endpoint to read diagnostics.
    */
   healthToken?: string;
+  /**
+   * Dedicated secret for /api/preview-link, sent as `Authorization: Bearer`.
+   * When unset, that endpoint falls back to HEALTH_TOKEN.
+   */
+  previewAdminToken?: string;
   /**
    * Same-origin host this app runs at. Used to build `frame-ancestors 'self'`
    * and any absolute self-links. Defaults to inferring from the request.
@@ -46,6 +51,7 @@ export function appEnv(): AppEnv {
     basicAuthUser: process.env.CIVI_HTTP_AUTH_USER,
     basicAuthPass: process.env.CIVI_HTTP_AUTH_PASS,
     healthToken: process.env.HEALTH_TOKEN,
+    previewAdminToken: process.env.PREVIEW_ADMIN_TOKEN,
     publicOrigin: process.env.PUBLIC_ORIGIN,
   };
   // In production, refuse to start in stub mode — that's almost certainly a
@@ -64,7 +70,14 @@ export function appEnv(): AppEnv {
 export function redact(s: string): string {
   let out = s;
   const e = appEnv();
-  for (const v of [e.civiApiKey, e.civiSiteKey, e.basicAuthPass, e.healthToken]) {
+  for (const v of [
+    e.civiApiKey,
+    e.civiSiteKey,
+    e.basicAuthPass,
+    e.healthToken,
+    e.previewAdminToken,
+    process.env.STAFF_REPORT_KEY,
+  ]) {
     if (v && v.length >= 6) out = out.split(v).join("«redacted»");
   }
   return out;

lib/staff-auth.ts

@@ -1,18 +1,48 @@
 /**
- * Shared-secret auth for the internal staff routes.
+ * Auth for the internal staff routes (/staff/report, /api/staff/report,
+ * /api/staff/file).
  *
- * Staff hit URLs of the form /staff/report?org=<id>&key=<secret>. The
- * secret is read from the STAFF_REPORT_KEY env var. If unset, the routes
- * refuse every request (closed by default).
+ * Preferred: signed, org-bound, expiring tokens minted by the CiviCRM
+ * extension — /staff/report?org=<id>&exp=<unix>&sig=<hmac>. See
+ * lib/staff-token.mjs. The extension only mints a token after checking the
+ * Civi user can view that org, so Civi ACLs carry through.
  *
- * Stub mode (CIVI_* unset) does NOT bypass this check — we want to test
- * the auth surface in dev too. For local dev, set STAFF_REPORT_KEY=dev in
- * .env.local.
+ * Legacy (transitional): ?key=<STAFF_REPORT_KEY>. Still accepted so the app
+ * can deploy before the updated extension reaches the Civi server. Remove
+ * the legacy branch once every Civi install emits signed tokens.
+ *
+ * If STAFF_REPORT_KEY is unset, every request is refused (closed by
+ * default). Stub mode (CIVI_* unset) does NOT bypass this check — for local
+ * dev, set STAFF_REPORT_KEY=dev in .env.local.
  */
 
-export function isStaffKeyValid(key: string | null | undefined): boolean {
-  const expected = process.env.STAFF_REPORT_KEY;
-  if (!expected) return false;
-  if (!key) return false;
-  return key === expected;
+import { safeEqual, verifyStaffToken } from "./staff-token.mjs";
+
+interface StaffAuthParams {
+  org: string | null | undefined;
+  exp?: string | null;
+  sig?: string | null;
+  key?: string | null;
+}
+
+/**
+ * Validate staff credentials for `org`. Returns the query-string fragment
+ * (without `org`) that downstream fetches and file links must carry, or
+ * null when unauthorized.
+ */
+export function staffAuthQuery({ org, exp, sig, key }: StaffAuthParams): string | null {
+  const secret = process.env.STAFF_REPORT_KEY;
+  if (!secret) return null;
+
+  if (exp || sig) {
+    return verifyStaffToken(secret, org, exp, sig)
+      ? `exp=${encodeURIComponent(exp!)}&sig=${encodeURIComponent(sig!)}`
+      : null;
+  }
+
+  if (key && safeEqual(key, secret)) {
+    console.warn("[staff-auth] legacy ?key= access used; update the CiviCRM extension to signed tokens");
+    return `key=${encodeURIComponent(key)}`;
+  }
+  return null;
 }

lib/staff-token.mjs

@@ -0,0 +1,78 @@
+// @ts-check
+/**
+ * Signed, org-bound, expiring tokens for the internal staff report.
+ *
+ * The CiviCRM extension's Engagement Report tab mints a token per render:
+ *
+ *   sig = base64url( HMAC-SHA256( STAFF_REPORT_KEY,
+ *                    "webform-mw.staff-report.v1:<org>:<exp>" ) )
+ *
+ * and links to /staff/report?org=<org>&exp=<exp>&sig=<sig>. The raw shared
+ * secret never leaves the two servers, and a captured URL only unlocks one
+ * org until it expires — unlike the legacy `?key=<secret>` form, which
+ * unlocked every org forever.
+ *
+ * The PHP side (civi-extension/.../Page/Tab.php) must build the message
+ * string byte-for-byte the same way. The fixed test vector in
+ * staff-token.test.mjs was produced by the PHP implementation.
+ *
+ * Written as JS+JSDoc rather than TS so Node's built-in --test runner can
+ * import this file directly — matches lib/mime.mjs.
+ */
+
+import { createHmac, timingSafeEqual } from "node:crypto";
+
+/**
+ * Longest validity window we accept, measured from now. The extension
+ * issues 8-hour tokens; the extra headroom absorbs clock skew between the
+ * Civi server and Amplify without letting anyone mint long-lived links.
+ */
+export const STAFF_TOKEN_MAX_TTL_SECONDS = 12 * 60 * 60;
+
+/**
+ * @param {string} org
+ * @param {string} exp
+ */
+function message(org, exp) {
+  return `webform-mw.staff-report.v1:${org}:${exp}`;
+}
+
+/**
+ * @param {string} secret
+ * @param {string} org
+ * @param {string} exp
+ * @returns {string}
+ */
+export function signStaffToken(secret, org, exp) {
+  return createHmac("sha256", secret).update(message(org, exp)).digest("base64url");
+}
+
+/**
+ * Constant-time string comparison. Lengths may differ; that case returns
+ * false without calling timingSafeEqual (which would throw).
+ *
+ * @param {string} a
+ * @param {string} b
+ */
+export function safeEqual(a, b) {
+  const ab = Buffer.from(a);
+  const bb = Buffer.from(b);
+  return ab.length === bb.length && timingSafeEqual(ab, bb);
+}
+
+/**
+ * @param {string | null | undefined} secret
+ * @param {string | null | undefined} org
+ * @param {string | null | undefined} exp
+ * @param {string | null | undefined} sig
+ * @param {number} [nowSeconds]
+ * @returns {boolean}
+ */
+export function verifyStaffToken(secret, org, exp, sig, nowSeconds = Math.floor(Date.now() / 1000)) {
+  if (!secret || !org || !exp || !sig) return false;
+  if (!/^[1-9]\d*$/.test(org) || !/^[1-9]\d*$/.test(exp)) return false;
+  const expN = Number(exp);
+  if (expN <= nowSeconds) return false;
+  if (expN - nowSeconds > STAFF_TOKEN_MAX_TTL_SECONDS) return false;
+  return safeEqual(signStaffToken(secret, org, exp), sig);
+}

lib/staff-token.test.mjs

@@ -0,0 +1,64 @@
+// Run with: npm run test:token
+import test from "node:test";
+import assert from "node:assert/strict";
+import {
+  signStaffToken,
+  verifyStaffToken,
+  safeEqual,
+  STAFF_TOKEN_MAX_TTL_SECONDS,
+} from "./staff-token.mjs";
+
+const SECRET = "test-secret";
+const NOW = 1_800_000_000 - 3600; // one hour before the vector's exp
+
+// Produced by the PHP implementation in civi-extension Tab.php:
+//   rtrim(strtr(base64_encode(hash_hmac('sha256',
+//     'webform-mw.staff-report.v1:1234:1800000000', 'test-secret', TRUE)),
+//     '+/', '-_'), '=')
+const PHP_VECTOR = "HlcxJCgUHfa6fhuvldUxZmCk_y62TcuQ4dH2JjFSqCo";
+
+test("signStaffToken matches the PHP implementation", () => {
+  assert.equal(signStaffToken(SECRET, "1234", "1800000000"), PHP_VECTOR);
+});
+
+test("verifyStaffToken accepts a valid, unexpired token", () => {
+  assert.equal(verifyStaffToken(SECRET, "1234", "1800000000", PHP_VECTOR, NOW), true);
+});
+
+test("verifyStaffToken rejects a token for a different org", () => {
+  assert.equal(verifyStaffToken(SECRET, "1235", "1800000000", PHP_VECTOR, NOW), false);
+});
+
+test("verifyStaffToken rejects a tampered exp", () => {
+  assert.equal(verifyStaffToken(SECRET, "1234", "1800000001", PHP_VECTOR, NOW), false);
+});
+
+test("verifyStaffToken rejects a wrong secret", () => {
+  assert.equal(verifyStaffToken("other-secret", "1234", "1800000000", PHP_VECTOR, NOW), false);
+});
+
+test("verifyStaffToken rejects an expired token", () => {
+  assert.equal(verifyStaffToken(SECRET, "1234", "1800000000", PHP_VECTOR, 1_800_000_000), false);
+});
+
+test("verifyStaffToken rejects exp too far in the future", () => {
+  const now = 1_800_000_000 - STAFF_TOKEN_MAX_TTL_SECONDS - 1;
+  assert.equal(verifyStaffToken(SECRET, "1234", "1800000000", PHP_VECTOR, now), false);
+});
+
+test("verifyStaffToken rejects missing or malformed inputs", () => {
+  assert.equal(verifyStaffToken("", "1234", "1800000000", PHP_VECTOR, NOW), false);
+  assert.equal(verifyStaffToken(SECRET, null, "1800000000", PHP_VECTOR, NOW), false);
+  assert.equal(verifyStaffToken(SECRET, "1234", undefined, PHP_VECTOR, NOW), false);
+  assert.equal(verifyStaffToken(SECRET, "1234", "1800000000", "", NOW), false);
+  assert.equal(verifyStaffToken(SECRET, "01234", "1800000000", PHP_VECTOR, NOW), false);
+  assert.equal(verifyStaffToken(SECRET, "1234", "18e8", PHP_VECTOR, NOW), false);
+  assert.equal(verifyStaffToken(SECRET, "-1", "1800000000", PHP_VECTOR, NOW), false);
+});
+
+test("safeEqual compares by value and tolerates length mismatch", () => {
+  assert.equal(safeEqual("abc", "abc"), true);
+  assert.equal(safeEqual("abc", "abd"), false);
+  assert.equal(safeEqual("abc", "abcd"), false);
+  assert.equal(safeEqual("", ""), true);
+});

package.json

@@ -10,7 +10,8 @@
     "sync-help": "node --env-file=.env.local scripts/sync-help-from-civi.mjs",
     "test:mapping": "node --test lib/staff-field-mapping.test.mjs",
     "test:mime": "node --test lib/mime.test.mjs",
-    "test": "node --test lib/staff-field-mapping.test.mjs lib/mime.test.mjs"
+    "test:token": "node --test lib/staff-token.test.mjs",
+    "test": "node --test lib/staff-field-mapping.test.mjs lib/mime.test.mjs lib/staff-token.test.mjs"
   },
   "dependencies": {
     "next": "16.4.3",