Staff file proxy: restore IDOR check via column ownership

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).
2026-06-15 16:03:48 -07:00
parent e74462da0b
commit 0019996b15
1 changed files with 122 additions and 27 deletions
@@ -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<AttachmentRow | null> {
  const res = await civi3<AttachmentRow>("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<FileFieldRefs> {
+  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<boolean> {
+  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<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,
+      }),
+    );
+  }
+
+  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 });
  }