diff --git a/AGENTS.md b/AGENTS.md index 8bd0e39..af3b7c8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,5 +1,9 @@ -# 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. + diff --git a/EMAIL_DELIVERY.md b/EMAIL_DELIVERY.md index 432cd34..8a8b032 100644 --- a/EMAIL_DELIVERY.md +++ b/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=&token=`. +1. Hit `/api/preview-link?cid=` with `Authorization: Bearer `. 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. diff --git a/README.md b/README.md index b88a957..a0d3d6d 100644 --- a/README.md +++ b/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. diff --git a/app/api/health/route.ts b/app/api/health/route.ts index ae9b67a..28cf629 100644 --- a/app/api/health/route.ts +++ b/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 { async function probeChecksumApi(): Promise { // 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 { 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 }); } } diff --git a/app/api/preview-link/route.ts b/app/api/preview-link/route.ts index 94aa23a..7cc809b 100644 --- a/app/api/preview-link/route.ts +++ b/app/api/preview-link/route.ts @@ -1,5 +1,6 @@ /** - * GET /api/preview-link?cid=&token= + * GET /api/preview-link?cid= + * Authorization: Bearer * * 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 }); } diff --git a/app/api/staff/file/route.ts b/app/api/staff/file/route.ts index a90e435..9249699 100644 --- a/app/api/staff/file/route.ts +++ b/app/api/staff/file/route.ts @@ -1,5 +1,5 @@ /** - * GET /api/staff/file?id=&org=&key= + * GET /api/staff/file?id=&org=&exp=&sig= * * 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 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 }); } diff --git a/app/api/staff/report/route.ts b/app/api/staff/report/route.ts index a7d3733..3e15dd4 100644 --- a/app/api/staff/report/route.ts +++ b/app/api/staff/report/route.ts @@ -1,18 +1,19 @@ /** - * GET /api/staff/report?org=&key= + * GET /api/staff/report?org=&exp=&sig= * * 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 }); } diff --git a/app/api/submit/route.ts b/app/api/submit/route.ts index b995f0f..ea274be 100644 --- a/app/api/submit/route.ts +++ b/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> { + 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>("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/) 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): Promise { + let current: Record | 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; + 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 ; } @@ -43,7 +44,7 @@ export default async function StaffReportPage({ searchParams }: PageProps) { diff --git a/civi-extension/webform-mw/CRM/WebformMw/Page/Tab.php b/civi-extension/webform-mw/CRM/WebformMw/Page/Tab.php index 4a07a9f..48c88ae 100644 --- a/civi-extension/webform-mw/CRM/WebformMw/Page/Tab.php +++ b/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::")). + * 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), '+/', '-_'), '='); + } + } diff --git a/civi-extension/webform-mw/CRM/WebformMw/Page/Upload.php b/civi-extension/webform-mw/CRM/WebformMw/Page/Upload.php index 14b0e30..673be86 100644 --- a/civi-extension/webform-mw/CRM/WebformMw/Page/Upload.php +++ b/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', [ diff --git a/civi-extension/webform-mw/README.md b/civi-extension/webform-mw/README.md index c9dfce3..dc1aac1 100644 --- a/civi-extension/webform-mw/README.md +++ b/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=&key=&frame=1`. + `${WEBFORM_MW_APP_URL}/staff/report?org=&exp=&sig=&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::")`, + 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 `` value in `xml/Menu/webform_mw.xml` diff --git a/civi-extension/webform-mw/info.xml b/civi-extension/webform-mw/info.xml index f4b6e8e..7a5704c 100644 --- a/civi-extension/webform-mw/info.xml +++ b/civi-extension/webform-mw/info.xml @@ -16,8 +16,8 @@ https://github.com/joelbrock/WebForm-mw - 2026-06-05 - 0.3.1 + 2026-10-30 + 0.4.0 beta 5.50 diff --git a/components/StaffReportView.tsx b/components/StaffReportView.tsx index b1eb6b3..52d0630 100644 --- a/components/StaffReportView.tsx +++ b/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; 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} /> ))} @@ -384,13 +385,13 @@ function CompactFieldRow({ options, civiBaseUrl, org, - authKey, + authQuery, }: { field: StaffReportField; options: Record; 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} /> {latest.date ? ( @@ -447,7 +448,7 @@ function CompactFieldRow({ options={options} civiBaseUrl={civiBaseUrl} org={org} - authKey={authKey} + authQuery={authQuery} /> @@ -464,14 +465,14 @@ function FieldValue({ options, civiBaseUrl, org, - authKey, + authQuery, }: { field: StaffReportField; entry: FieldHistoryEntry; options: Record; 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} /> ); diff --git a/components/report/FileLink.tsx b/components/report/FileLink.tsx index 6ab5c43..de221cb 100644 --- a/components/report/FileLink.tsx +++ b/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=&` (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. diff --git a/lib/civicrm.ts b/lib/civicrm.ts index 02b7d4c..28e7439 100644 --- a/lib/civicrm.ts +++ b/lib/civicrm.ts @@ -297,32 +297,29 @@ export async function civiMultipart( } /** - * 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 { 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; } } diff --git a/lib/env.ts b/lib/env.ts index 0e9e27d..e6d6731 100644 --- a/lib/env.ts +++ b/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; diff --git a/lib/staff-auth.ts b/lib/staff-auth.ts index 0fa79ae..f9acba6 100644 --- a/lib/staff-auth.ts +++ b/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=&key=. 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=&exp=&sig=. 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=. 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; } diff --git a/lib/staff-token.mjs b/lib/staff-token.mjs new file mode 100644 index 0000000..9916324 --- /dev/null +++ b/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::" ) ) + * + * and links to /staff/report?org=&exp=&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=` 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); +} diff --git a/lib/staff-token.test.mjs b/lib/staff-token.test.mjs new file mode 100644 index 0000000..2c46ded --- /dev/null +++ b/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); +}); diff --git a/package.json b/package.json index 15a8eb5..d5fe79f 100644 --- a/package.json +++ b/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",