From 5124010b8a63e12d3ed391b6b6b39b1705547725 Mon Sep 17 00:00:00 2001 From: Joel Brock Date: Tue, 16 Jun 2026 16:15:28 -0700 Subject: [PATCH] Lightbox: fix proxy 404, cap modal size, allow multi-Civi embed Three quick fixes off first-deploy testing: 1. /api/staff/file 404'd for valid files. Refactor fileBelongsToOrg to SELECT the org's and activities' file columns and JS-compare instead of WHERE ... OR with custom field refs (APIv4 fragility around nested OR + dotted custom fields). Same ownership probe, same shape /api/staff/report itself uses to read file values. 2. Lightbox ballooned to full report height because the staff iframe auto-grows to fit content (often 3000+ px). Cap to a fixed 640px x min(92vw, 900px) box so it stays a reasonable preview regardless of iframe document size. 3. Production frame-ancestors blocked crm.fci.coop from iframing survey.fci.coop -- the CSP only included the dev Civi origin derived from CIVI_BASE_URL. Add CIVI_FRAME_ALLOWED_ORIGINS (comma-separated) so one app deploy can be embedded by both dev and prod Civi. Falls back to CIVI_BASE_URL for single-Civi compatibility. PRODUCTION_CUTOVER.md updated inline and in the change log. --- PRODUCTION_CUTOVER.md | 33 +++++++-- app/api/staff/file/route.ts | 94 +++++++++++++----------- components/report/AttachmentLightbox.tsx | 21 ++++-- next.config.ts | 40 ++++++++-- 4 files changed, 124 insertions(+), 64 deletions(-) diff --git a/PRODUCTION_CUTOVER.md b/PRODUCTION_CUTOVER.md index ded2a89..d44c05d 100644 --- a/PRODUCTION_CUTOVER.md +++ b/PRODUCTION_CUTOVER.md @@ -197,13 +197,20 @@ Full extension docs: ## 6. CSP / `frame-ancestors` — app side -The Next.js app's `/staff/report` route must allow the CiviCRM origin -in its `frame-ancestors` CSP, or the iframe will refuse to render. +The Next.js app's `/staff/report` and `/api/staff/file` routes must +allow every CiviCRM origin that will iframe them, or the browser will +refuse to render. -The build reads `CIVI_BASE_URL` and adds its origin to the CSP -automatically — so make sure `CIVI_BASE_URL` on the app deploy points -at the **production** CRM origin (`https://crm.fci.coop`), not -`client.crm.fci.coop`. +Set **`CIVI_FRAME_ALLOWED_ORIGINS`** (comma-separated) in the app +deploy env. Each origin needs the scheme: + +``` +CIVI_FRAME_ALLOWED_ORIGINS=https://crm.fci.coop,https://client.crm.fci.coop +``` + +Include both prod and any staging Civi origins you want to keep +embedding. If unset, the build falls back to the origin of +`CIVI_BASE_URL` (single-Civi compatibility). Confirm after deploy: @@ -211,8 +218,11 @@ Confirm after deploy: curl -sI https://survey.fci.coop/staff/report | grep -i content-security-policy ``` -Should include `frame-ancestors 'self' https://crm.fci.coop` (or -whatever your production CRM origin is). +Should include +`frame-ancestors 'self' https://crm.fci.coop https://client.crm.fci.coop` +(or whatever list you configured). If you see only one origin and the +other Civi is failing to embed, the env var is missing or stale — +trigger a new build, not just a restart. --- @@ -276,3 +286,10 @@ so the rationale survives. - **2026-06-08** — Documented the field-242 "Unknown" default issue after a production submission was stamped `Stage = "Unknown"`. Cleared via `CustomField.update`; see step 2. +- **2026-06-16** — Step 6 split off `CIVI_FRAME_ALLOWED_ORIGINS` as a + separate env from `CIVI_BASE_URL`. Surfaced after the production + cutover hit a `frame-ancestors` block: the app's CSP only listed the + staging Civi origin (derived from `CIVI_BASE_URL`), so prod + (`crm.fci.coop`) couldn't iframe `survey.fci.coop`. The new var + takes a comma-separated list so one app deploy can be embedded by + both dev and prod Civi. diff --git a/app/api/staff/file/route.ts b/app/api/staff/file/route.ts index b3fd3fb..a90e435 100644 --- a/app/api/staff/file/route.ts +++ b/app/api/staff/file/route.ts @@ -130,18 +130,17 @@ async function discoverFileFieldRefs(): Promise { * Confirm fileId is reachable from this org through the same column * ownership the staff report uses. * - * Strategy: probe both ownership sides in parallel using APIv4 OR clauses. - * - Contact.get for the org itself, asking whether ANY org-side file - * custom field equals fileId. - * - Activity.get for activities with target_contact_id=orgId, asking - * whether ANY activity-side file custom field equals fileId. - * If either query returns a row, the file genuinely belongs to this org. + * Implementation: fetch the org row and the org's activities, selecting + * the file-typed custom-field columns, then check in JS whether any + * column value equals fileId. We avoid APIv4 OR clauses against custom + * fields because that combination has been fragile in practice; the + * SELECT-and-compare path is the same shape /api/staff/report uses + * successfully. * * We do NOT use civicrm_entity_file for this check: our upload route - * anchors files to the submitter's contact id (not the org's) because - * Attachment.create needs *some* entity, but the real ownership is in - * the custom-field column. Trusting entity_file would refuse legitimate - * files and admit-or-refuse incorrectly for the rest. + * anchors files to the submitter's contact id (not the org's), so that + * linkage doesn't reflect ownership. The custom-field column is the + * authoritative chain. */ async function fileBelongsToOrg(fileId: number, orgId: number): Promise { let refs: FileFieldRefs; @@ -153,40 +152,53 @@ async function fileBelongsToOrg(fileId: number, orgId: number): Promise return false; } - const probes: Array> = []; + const matches = (rows: Array>, fieldRefs: string[]): boolean => { + for (const row of rows) { + for (const ref of fieldRefs) { + const v = row[ref]; + if (v === undefined || v === null || v === "") continue; + if (Number(v) === fileId) return true; + } + } + return false; + }; - if (refs.org.length > 0) { - probes.push( - civi<{ id: number }>("Contact", "get", { - where: [ - ["id", "=", orgId], - ["OR", refs.org.map((ref) => [ref, "=", fileId])], - ], - select: ["id"], - limit: 1, - }), - ); - } + const orgProbe = + refs.org.length > 0 + ? civi>("Contact", "get", { + where: [["id", "=", orgId]], + select: ["id", ...refs.org], + limit: 1, + }).catch((e: unknown) => { + console.error( + "[staff/file] Contact.get probe failed:", + e instanceof Error ? e.message : String(e), + ); + return { values: [] as Array> }; + }) + : Promise.resolve({ values: [] as Array> }); - if (refs.activity.length > 0) { - probes.push( - civi<{ id: number }>("Activity", "get", { - where: [ - ["target_contact_id", "=", orgId], - ["activity_type_id:name", "=", ACTIVITY_TYPE_NAME], - ["OR", refs.activity.map((ref) => [ref, "=", fileId])], - ], - select: ["id"], - limit: 1, - }), - ); - } + const activityProbe = + refs.activity.length > 0 + ? civi>("Activity", "get", { + where: [ + ["target_contact_id", "=", orgId], + ["activity_type_id:name", "=", ACTIVITY_TYPE_NAME], + ], + select: ["id", ...refs.activity], + limit: 500, + }).catch((e: unknown) => { + console.error( + "[staff/file] Activity.get probe failed:", + e instanceof Error ? e.message : String(e), + ); + return { values: [] as Array> }; + }) + : Promise.resolve({ values: [] as Array> }); - if (probes.length === 0) return false; - - const results = await Promise.allSettled(probes); - return results.some( - (r) => r.status === "fulfilled" && Array.isArray(r.value.values) && r.value.values.length > 0, + const [orgRes, actRes] = await Promise.all([orgProbe, activityProbe]); + return ( + matches(orgRes.values ?? [], refs.org) || matches(actRes.values ?? [], refs.activity) ); } diff --git a/components/report/AttachmentLightbox.tsx b/components/report/AttachmentLightbox.tsx index 63eacb2..fe620eb 100644 --- a/components/report/AttachmentLightbox.tsx +++ b/components/report/AttachmentLightbox.tsx @@ -65,21 +65,26 @@ export function AttachmentLightbox({ const isPdf = mime === "application/pdf"; return ( + // Sizing note: this dialog opens inside the staff-report iframe, which + // auto-grows to fit content (often 2000–4000 px tall). "h-full"/"vh" + // values inside that iframe resolve to the full iframe document, so the + // dialog would balloon. Cap to a fixed pixel box that fits comfortably + // on a typical laptop and still gives PDFs/images enough room. -
-
-

+

+
+

{filename}

-
+
{isImage ? ( // eslint-disable-next-line @next/next/no-img-element ) : ( // Defensive: FileLink shouldn't open the lightbox for non-previewable diff --git a/next.config.ts b/next.config.ts index 7388d1a..1a62355 100644 --- a/next.config.ts +++ b/next.config.ts @@ -43,13 +43,38 @@ const sharedHeaders = [ }, ]; -function civiOriginForCsp(): string { - const raw = process.env.CIVI_BASE_URL; - if (!raw) return ""; +/** + * Civi origins permitted to embed the staff report in an iframe. + * + * Reads `CIVI_FRAME_ALLOWED_ORIGINS` (comma-separated origins) so a single + * survey.fci.coop deployment can be framed by both the dev Civi + * (client.crm.fci.coop) and the production Civi (crm.fci.coop). Falls + * back to the origin of `CIVI_BASE_URL` when the multi-origin var isn't + * set so existing single-Civi deployments keep working. + * + * Each entry is validated as a parsable URL; bad values are dropped and + * logged at build time rather than silently making the CSP invalid. + */ +function civiOriginsForCsp(): string[] { + const explicit = process.env.CIVI_FRAME_ALLOWED_ORIGINS; + if (explicit) { + const raws = explicit.split(",").map((s) => s.trim()).filter(Boolean); + const origins: string[] = []; + for (const raw of raws) { + try { + origins.push(new URL(raw).origin); + } catch { + console.warn(`[next.config] ignoring invalid CIVI_FRAME_ALLOWED_ORIGINS entry: ${raw}`); + } + } + return origins; + } + const base = process.env.CIVI_BASE_URL; + if (!base) return []; try { - return new URL(raw).origin; + return [new URL(base).origin]; } catch { - return ""; + return []; } } @@ -60,12 +85,13 @@ const strictHeaders = [ ]; const staffEmbedHeaders = (() => { - const origin = civiOriginForCsp(); - const frameAncestors = origin ? `'self' ${origin}` : "'self'"; + const origins = civiOriginsForCsp(); + const frameAncestors = origins.length > 0 ? `'self' ${origins.join(" ")}` : "'self'"; return [ { key: "Content-Security-Policy", value: buildCsp(frameAncestors) }, ...sharedHeaders, // Intentionally NO X-Frame-Options: frame-ancestors above is the policy. + // (X-Frame-Options can only encode one origin; CSP supersedes here.) ]; })();