From 0019996b15d7a60d3ac61cc9cba2e5b766b8c17b Mon Sep 17 00:00:00 2001 From: Joel Brock Date: Mon, 15 Jun 2026 16:03:48 -0700 Subject: [PATCH] Staff file proxy: restore IDOR check via column ownership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the dropped entity_file→org check with a probe against the actual ownership chain — the file_id stored in a custom-field column on the org or on one of its activities. For each request: 1. Discover file-typed CustomField refs in ACTIVITY_GROUP_NAMES and ORG_GROUP_NAMES (one CustomField.get). 2. In parallel, probe: - Contact.get(id=orgId) WHERE any org file field == fileId - Activity.get(target=orgId) WHERE any activity file field == fileId using APIv4 OR clauses. 3. Allow only if at least one probe returns a hit. This is the same ownership the staff report itself uses to surface the file — the proxy now refuses to broker bytes for any file id that wouldn't appear in the org's own report. civicrm_entity_file remains unused for auth (it's anchored to the submitter, not the org). --- app/api/staff/file/route.ts | 149 +++++++++++++++++++++++++++++------- 1 file changed, 122 insertions(+), 27 deletions(-) diff --git a/app/api/staff/file/route.ts b/app/api/staff/file/route.ts index ad960e6..b3fd3fb 100644 --- a/app/api/staff/file/route.ts +++ b/app/api/staff/file/route.ts @@ -9,12 +9,12 @@ * * Authorization layers: * 1. STAFF_REPORT_KEY query param (same gate as /api/staff/report). - * 2. Server-side check that the requested file is actually linked to the - * `org`. This prevents the staff key from being used to pull arbitrary - * file ids out of CiviCRM — a file is reachable only if its - * entity_table/entity_id ties back to the org (directly, for org - * custom-field files; or via an activity's target_contact_id, for - * activity custom-field files). + * 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 + * ownership chain the staff report uses to surface the file in the + * first place. NOT enforced via civicrm_entity_file — that linkage + * is metadata-only here (upload anchors to submitter, not org). * * The upstream fetch uses the URL Civi returns from `Attachment.get`, * which includes a freshly-minted `fcs` JWT. We don't carry any user @@ -26,7 +26,7 @@ import { NextRequest, NextResponse } from "next/server"; import { isStaffKeyValid } from "@/lib/staff-auth"; -import { civi3 } from "@/lib/civicrm"; +import { civi, civi3 } from "@/lib/civicrm"; import { resolveMime } from "@/lib/mime.mjs"; const MAX_BYTES = 10 * 1024 * 1024; @@ -66,38 +66,128 @@ interface AttachmentRow { url?: string; mime_type?: string; name?: string; - entity_table?: string; } async function fetchAttachment(fileId: number): Promise { const res = await civi3("Attachment", "get", { id: fileId, - return: "id,url,mime_type,name,entity_table", + return: "id,url,mime_type,name", sequential: 1, }); return res.values?.[0] ?? null; } -const ALLOWED_ENTITY_TABLES = new Set(["civicrm_contact", "civicrm_activity"]); +// Same groups /api/staff/report scans. Kept in sync deliberately: this +// proxy must only authorise files reachable through the same set of +// fields the report itself surfaces. +const ACTIVITY_GROUP_NAMES = [ + "Check_in_data__organizing_", + "Stage_1", + "Stage_2", + "Stage_3", + "Stage_4", + "Stage_5", +]; +const ORG_GROUP_NAMES = ["Food_Co_op_Organizing"]; +const ACTIVITY_TYPE_NAME = "Check-in (organizing)"; + +interface FileFieldRefs { + org: string[]; + activity: string[]; +} /** - * Confirm this file id is one our staff report would reasonably surface. - * - * We deliberately do NOT tie the file to `orgId` via `civicrm_entity_file`. - * That linkage is metadata-only on this install: our upload route anchors - * uploads to the submitter's contact id (not the org's), because the - * semantic ownership lives in the custom-field column on the activity or - * org, not in `civicrm_entity_file`. The rest of `/api/staff/*` already - * trusts the staff key as the org-agnostic gate, so requiring strict - * file→org binding here would be stricter (and broken: false-404s) than - * the rest of the surface. - * - * The entity_table whitelist still serves as defence: anything that didn't - * come from the Civi paths we know about (contact files, activity files) - * never gets proxied. + * Discover the APIv4 field references for File-typed custom fields the + * staff report cares about. Returns refs like "Stage_1.Vision_Upload" + * split by whether they live on the Organization Contact or on Activities. */ -function isProxyableEntityTable(row: AttachmentRow): boolean { - return ALLOWED_ENTITY_TABLES.has(String(row.entity_table ?? "")); +async function discoverFileFieldRefs(): Promise { + const res = await civi<{ + name: string; + data_type: string; + "custom_group_id.name": string; + }>("CustomField", "get", { + select: ["name", "data_type", "custom_group_id.name"], + where: [ + ["custom_group_id.name", "IN", [...ACTIVITY_GROUP_NAMES, ...ORG_GROUP_NAMES]], + ["data_type", "=", "File"], + ["is_active", "=", true], + ], + limit: 500, + }); + const org: string[] = []; + const activity: string[] = []; + for (const row of res.values ?? []) { + const group = row["custom_group_id.name"]; + const ref = `${group}.${row.name}`; + if (ORG_GROUP_NAMES.includes(group)) org.push(ref); + else if (ACTIVITY_GROUP_NAMES.includes(group)) activity.push(ref); + } + return { org, activity }; +} + +/** + * 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. + * + * 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. + */ +async function fileBelongsToOrg(fileId: number, orgId: number): Promise { + let refs: FileFieldRefs; + try { + refs = await discoverFileFieldRefs(); + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + console.error("[staff/file] CustomField.get failed:", msg); + return false; + } + + const probes: Array> = []; + + 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, + }), + ); + } + + 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, + }), + ); + } + + 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, + ); } export async function GET(req: NextRequest) { @@ -136,7 +226,12 @@ export async function GET(req: NextRequest) { return new NextResponse("Not found", { status: 404 }); } - if (!isProxyableEntityTable(row)) { + // Authorisation: prove this file actually belongs to `orgId` via the + // custom-field columns the staff report itself surfaces. Without this a + // staff key (which is org-agnostic) could be used to enumerate file ids + // outside any report context. + const owned = await fileBelongsToOrg(fileId, orgId); + if (!owned) { return new NextResponse("Not found", { status: 404 }); }