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.
This commit is contained in:
Joel Brock
+25
-8
@@ -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.
|
||||
|
||||
+48
-36
@@ -130,18 +130,17 @@ async function discoverFileFieldRefs(): Promise<FileFieldRefs> {
|
||||
* 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<boolean> {
|
||||
let refs: FileFieldRefs;
|
||||
@@ -153,40 +152,53 @@ async function fileBelongsToOrg(fileId: number, orgId: number): Promise<boolean>
|
||||
return false;
|
||||
}
|
||||
|
||||
const probes: Array<Promise<{ values?: unknown[] }>> = [];
|
||||
|
||||
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 matches = (rows: Array<Record<string, unknown>>, 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.activity.length > 0) {
|
||||
probes.push(
|
||||
civi<{ id: number }>("Activity", "get", {
|
||||
const orgProbe =
|
||||
refs.org.length > 0
|
||||
? civi<Record<string, unknown>>("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<Record<string, unknown>> };
|
||||
})
|
||||
: Promise.resolve({ values: [] as Array<Record<string, unknown>> });
|
||||
|
||||
const activityProbe =
|
||||
refs.activity.length > 0
|
||||
? civi<Record<string, unknown>>("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,
|
||||
}),
|
||||
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<Record<string, unknown>> };
|
||||
})
|
||||
: Promise.resolve({ values: [] as Array<Record<string, unknown>> });
|
||||
|
||||
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)
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -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.
|
||||
<dialog
|
||||
ref={ref}
|
||||
onClick={onDialogClick}
|
||||
aria-label={`Preview: ${filename}`}
|
||||
className="m-0 h-full max-h-screen w-full max-w-screen-2xl rounded-none bg-transparent p-0 backdrop:bg-ink/70"
|
||||
className="m-auto w-[min(92vw,900px)] rounded-md bg-transparent p-0 shadow-2xl backdrop:bg-ink/70"
|
||||
>
|
||||
<div className="flex h-full flex-col">
|
||||
<header className="flex items-center justify-between gap-4 bg-paper px-4 py-3 shadow-sm sm:px-6">
|
||||
<p className="min-w-0 truncate font-display text-base text-ink">
|
||||
<div className="flex h-[640px] max-h-[85vh] flex-col overflow-hidden rounded-md">
|
||||
<header className="flex items-center justify-between gap-4 bg-paper px-4 py-2.5 sm:px-5">
|
||||
<p className="min-w-0 truncate font-display text-sm text-ink">
|
||||
{filename}
|
||||
</p>
|
||||
<div className="flex flex-shrink-0 items-center gap-3">
|
||||
<a
|
||||
href={downloadHref}
|
||||
className="text-sm font-medium text-leaf-700 underline decoration-rule underline-offset-4 hover:decoration-ink hover:text-leaf-800"
|
||||
className="text-xs font-medium text-leaf-700 underline decoration-rule underline-offset-4 hover:decoration-ink hover:text-leaf-800"
|
||||
target="_blank"
|
||||
rel="noopener noreferrer"
|
||||
>
|
||||
@@ -88,14 +93,14 @@ export function AttachmentLightbox({
|
||||
<button
|
||||
type="button"
|
||||
onClick={onClose}
|
||||
className="rounded px-2 py-1 text-sm font-medium text-ink-soft hover:bg-rule-soft/40 focus:outline-none focus-visible:ring-2 focus-visible:ring-leaf-700"
|
||||
className="rounded px-2 py-1 text-xs font-medium text-ink-soft hover:bg-rule-soft/40 focus:outline-none focus-visible:ring-2 focus-visible:ring-leaf-700"
|
||||
aria-label="Close preview"
|
||||
>
|
||||
Close
|
||||
</button>
|
||||
</div>
|
||||
</header>
|
||||
<div className="flex flex-1 items-center justify-center overflow-hidden bg-ink/90 p-4">
|
||||
<div className="flex flex-1 items-center justify-center overflow-hidden bg-ink/90 p-3">
|
||||
{isImage ? (
|
||||
// eslint-disable-next-line @next/next/no-img-element
|
||||
<img
|
||||
@@ -107,7 +112,7 @@ export function AttachmentLightbox({
|
||||
<iframe
|
||||
src={previewSrc}
|
||||
title={filename}
|
||||
className="h-full w-full max-w-screen-lg border-0 bg-paper"
|
||||
className="h-full w-full border-0 bg-paper"
|
||||
/>
|
||||
) : (
|
||||
// Defensive: FileLink shouldn't open the lightbox for non-previewable
|
||||
|
||||
+33
-7
@@ -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 {
|
||||
return new URL(raw).origin;
|
||||
origins.push(new URL(raw).origin);
|
||||
} catch {
|
||||
return "";
|
||||
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(base).origin];
|
||||
} catch {
|
||||
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.)
|
||||
];
|
||||
})();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user