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/staff/report/page.tsx b/app/staff/report/page.tsx index f0eef31..a7b17e1 100644 --- a/app/staff/report/page.tsx +++ b/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 ; } @@ -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/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/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",