fix(auth): gate activity notes and last-scan data server-side

apps/webapp/app/modules/scan/last-scan-for-viewer.test.ts

@@ -0,0 +1,110 @@
+// @vitest-environment node
+/**
+ * Authorization tests for `getLastScanForViewer`.
+ *
+ * The asset overview loader used to fetch and return the parsed last scan
+ * unconditionally, and the component hid `<ScanDetails>` behind a client-side
+ * `scan:read` check. The parsed payload carries the scanner's display name and
+ * EMAIL, the scan's GPS COORDINATES and the device user-agent, so for BASE and
+ * SELF_SERVICE (both `scan: []`) that was PII sitting in the page payload,
+ * hidden only by React.
+ *
+ * These tests drive the real `Role2PermissionMap` through the real
+ * `hasPermission`, so they assert the actual matrix rather than a restatement
+ * of it. Only the DB read is mocked.
+ *
+ * @see {@link file://./service.server.ts}
+ */
+import { OrganizationRoles } from "@prisma/client";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const { scanFindFirst } = vi.hoisted(() => ({ scanFindFirst: vi.fn() }));
+
+// why: the only external dependency. `hasPermission` and `parseScanData` run
+// for real so the gate is tested end to end against the live matrix.
+vi.mock("~/database/db.server", () => ({
+  db: { scan: { findFirst: scanFindFirst } },
+}));
+
+import { getLastScanForViewer } from "./service.server";
+
+/** A scan row shaped as `getScanByQrId` returns it, carrying real PII. */
+const scanRow = {
+  id: "scan-1",
+  userId: "user-9",
+  latitude: "52.3676",
+  longitude: "4.9041",
+  userAgent: "Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X)",
+  manuallyGenerated: false,
+  createdAt: new Date("2026-07-01T10:00:00Z"),
+  qr: { id: "qr-1", organizationId: "org-1" },
+  user: {
+    id: "user-9",
+    firstName: "Dana",
+    lastName: "Reeves",
+    displayName: "Dana Reeves",
+    email: "dana@example.com",
+    userOrganizations: [{ organizationId: "org-1" }],
+  },
+};
+
+function args(roles: OrganizationRoles[]) {
+  return {
+    qrId: "qr-1",
+    userId: "user-1",
+    organizationId: "org-1",
+    roles,
+  };
+}
+
+describe("getLastScanForViewer", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    scanFindFirst.mockResolvedValue(scanRow);
+  });
+
+  it.each([OrganizationRoles.BASE, OrganizationRoles.SELF_SERVICE])(
+    "returns null for %s and never reads the scan row",
+    async (role) => {
+      const result = await getLastScanForViewer(args([role]));
+
+      expect(result).toBeNull();
+      // Not merely omitted from the response — never fetched at all.
+      expect(scanFindFirst).not.toHaveBeenCalled();
+    }
+  );
+
+  it.each([OrganizationRoles.ADMIN, OrganizationRoles.OWNER])(
+    "returns the parsed scan for %s",
+    async (role) => {
+      const result = await getLastScanForViewer(args([role]));
+
+      expect(scanFindFirst).toHaveBeenCalledTimes(1);
+      expect(result).toEqual(
+        expect.objectContaining({
+          scannedBy: "Dana Reeves(dana@example.com)",
+          coordinates: "52.3676, 4.9041",
+        })
+      );
+    }
+  );
+
+  it("returns null without touching the DB when the asset has no QR code", async () => {
+    const result = await getLastScanForViewer({
+      ...args([OrganizationRoles.OWNER]),
+      qrId: undefined,
+    });
+
+    expect(result).toBeNull();
+    expect(scanFindFirst).not.toHaveBeenCalled();
+  });
+
+  it("returns null when the asset has a QR but has never been scanned", async () => {
+    scanFindFirst.mockResolvedValue(null);
+
+    const result = await getLastScanForViewer(args([OrganizationRoles.OWNER]));
+
+    // Same shape an unauthorized viewer gets, so callers need no special case.
+    expect(result).toBeNull();
+  });
+});

apps/webapp/app/modules/scan/service.server.ts

@@ -1,8 +1,14 @@
-import type { Prisma, Scan } from "@prisma/client";
+import type { OrganizationRoles, Prisma, Scan } from "@prisma/client";
 import { db } from "~/database/db.server";
 import { ShelfError } from "~/utils/error";
 import type { ErrorLabel } from "~/utils/error";
 import { wrapUserLinkForNote } from "~/utils/markdoc-wrappers";
+import {
+  PermissionAction,
+  PermissionEntity,
+} from "~/utils/permissions/permission.data";
+import { hasPermission } from "~/utils/permissions/permission.validator.server";
+import { parseScanData } from "./utils.server";
 import { createNote } from "../note/service.server";
 import { getOrganizationById } from "../organization/service.server";
 import { getQr } from "../qr/service.server";
@@ -154,6 +160,64 @@ export async function getScanByQrId({ qrId }: { qrId: string }) {
   }
 }
 
+/** Arguments for resolving a viewer-scoped last scan. */
+type GetLastScanForViewerArgs = {
+  /** The asset's QR code id, or null/undefined when it has no QR yet */
+  qrId: string | null | undefined;
+  /** The user viewing the page */
+  userId: string;
+  /** The viewer's active organization */
+  organizationId: string;
+  /** The viewer's roles in that organization (skips the validator's DB lookup) */
+  roles?: OrganizationRoles[];
+};
+
+/**
+ * Resolves the parsed last scan for an asset, but ONLY for a viewer who holds
+ * `scan:read`.
+ *
+ * The parsed payload carries the scanner's display name and email address, the
+ * scan's GPS coordinates and the device user-agent. Gating that in the
+ * component is not enough: the loader would still ship it in the page payload
+ * to roles that hold `scan: []` (BASE and SELF_SERVICE), where anyone can read
+ * it out of the network response. This helper is the server-side gate, so a
+ * caller cannot fetch the scan and forget to check.
+ *
+ * Returns `null` — the same shape as "this asset has never been scanned" — for
+ * an unauthorized viewer, so callers need no special case.
+ *
+ * @param args - The QR to look up plus the viewer's identity and roles
+ * @returns The parsed scan, or `null` when the asset has no QR, has no scans,
+ *   or the viewer may not read scans
+ */
+export async function getLastScanForViewer({
+  qrId,
+  userId,
+  organizationId,
+  roles,
+}: GetLastScanForViewerArgs) {
+  if (!qrId) {
+    return null;
+  }
+
+  const canReadScan = await hasPermission({
+    userId,
+    organizationId,
+    roles,
+    entity: PermissionEntity.scan,
+    action: PermissionAction.read,
+  });
+
+  if (!canReadScan) {
+    return null;
+  }
+
+  return parseScanData({
+    scan: (await getScanByQrId({ qrId })) || null,
+    userId,
+  });
+}
+
 /**
  * Writes a system note onto the asset linked to a scanned QR code.
  *

apps/webapp/app/routes/_layout+/assets.$assetId.activity.tsx

@@ -28,6 +28,18 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) {
   });
 
   try {
+    /**
+     * Two-step gate, in this order deliberately.
+     *
+     * Step 1 is `asset:read` against the SELECTED workspace, which is what
+     * `getAsset` needs to run its cross-workspace resolution below. Gating on
+     * `note:read` here instead would 403 a deep link to an asset that lives in
+     * a DIFFERENT workspace of the same user — one where they may well hold
+     * `note:read` — before `getAsset` ever gets the chance to offer the
+     * switch-workspace path. Assets are unusual in having that affordance;
+     * the bookings activity route has no equivalent, which is why it can gate
+     * on `bookingNote:read` alone.
+     */
     const { organizationId, userOrganizations } = await requirePermission({
       userId,
       request,
@@ -36,37 +48,56 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) {
     });
 
     /**
-     * Fetch the asset (which also enforces org/permission scoping) for the page
-     * header and the "Export activity CSV" link, and the notes page in
-     * parallel. Notes are fetched separately — and paginated/searched/filtered —
-     * so the activity log behaves like every other list in the app. Both are
-     * independently org-scoped, so they can run concurrently.
+     * Resolve the asset FIRST. Besides supplying the page header and the
+     * "Export activity CSV" link, this is the call that detects an asset
+     * belonging to another of the user's workspaces and hands off to the
+     * switch-workspace path. It must run before the note gate so that
+     * hand-off still happens (see the ordering note above).
      */
-    const [
-      asset,
-      {
-        page,
-        perPage,
-        search,
-        items,
-        totalItems,
-        totalPages,
-        hasNotes,
-        cookie,
-      },
-    ] = await Promise.all([
-      getAsset({
-        id,
-        organizationId,
-        userOrganizations,
-        request,
-      }),
-      getPaginatedAndFilterableAssetNotes({
-        assetId: id,
-        organizationId,
-        request,
-      }),
-    ]);
+    const asset = await getAsset({
+      id,
+      organizationId,
+      userOrganizations,
+      request,
+    });
+
+    /**
+     * Step 2 of the gate: `note:read`, enforced BEFORE a single note is
+     * fetched. This MUST stay server-side. `asset: [read]` is granted to BASE
+     * and SELF_SERVICE while `note: []` is not, so gating only on `asset:read`
+     * and hiding the notes in the component put every asset note in the page
+     * payload of users not allowed to read them — visible to anyone who opens
+     * devtools.
+     *
+     * The two steps are sequential rather than parallel by necessity: the
+     * switch-workspace hand-off has to precede the gate, and the gate has to
+     * precede the fetch.
+     */
+    await requirePermission({
+      userId,
+      request,
+      entity: PermissionEntity.note,
+      action: PermissionAction.read,
+    });
+
+    /**
+     * Notes are fetched separately — and paginated/searched/filtered — so the
+     * activity log behaves like every other list in the app.
+     */
+    const {
+      page,
+      perPage,
+      search,
+      items,
+      totalItems,
+      totalPages,
+      hasNotes,
+      cookie,
+    } = await getPaginatedAndFilterableAssetNotes({
+      assetId: id,
+      organizationId,
+      request,
+    });
 
     const header: HeaderData = {
       title: `${asset.title}'s activity`,

apps/webapp/app/routes/_layout+/assets.$assetId.overview.tsx

@@ -83,8 +83,7 @@ import { getPrimaryCustody } from "~/modules/custody/utils";
 import { getActiveCustomFields } from "~/modules/custom-field/service.server";
 import { moveAssetKitUnits } from "~/modules/kit/service.server";
 import { generateQrObj } from "~/modules/qr/utils.server";
-import { getScanByQrId } from "~/modules/scan/service.server";
-import { parseScanData } from "~/modules/scan/utils.server";
+import { getLastScanForViewer } from "~/modules/scan/service.server";
 import { getTeamMembersForQuantityCustody } from "~/modules/team-member/service.server";
 import { appendToMetaTitle } from "~/utils/append-to-meta-title";
 import { formatAssetValueWithBreakdown } from "~/utils/asset-value";
@@ -159,6 +158,16 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) {
 
     const { locale, timeZone } = getClientHint(request);
 
+    /**
+     * The caller's roles in the active org. Derived once here and reused by
+     * every server-side `hasPermission` check in this loader (scan gating
+     * below, edit gating further down) — passing `roles` explicitly avoids
+     * the validator's DB fallback lookup on each call.
+     */
+    const roles = userOrganizations.find(
+      (o) => o.organization.id === organizationId
+    )?.roles;
+
     const asset = await getAsset({
       id,
       organizationId,
@@ -169,14 +178,21 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) {
 
     /**
      * We get the first QR code(for now we can only have 1)
-     * And using the ID of tha qr code, we find the latest scan
+     * And using the ID of tha qr code, we find the latest scan.
+     *
+     * `getLastScanForViewer` applies the `scan:read` gate SERVER-SIDE and
+     * returns null without it. The parsed scan carries the scanner's name and
+     * email, GPS coordinates and user-agent; the component renders
+     * `<ScanDetails>` behind the same check, but a client-side check only
+     * hides the data — BASE and SELF_SERVICE hold `scan: []` and were still
+     * receiving all of it in the page payload.
      */
-    const lastScan = asset.qrCodes[0]?.id
-      ? parseScanData({
-          scan: (await getScanByQrId({ qrId: asset.qrCodes[0].id })) || null,
-          userId,
-        })
-      : null;
+    const lastScan = await getLastScanForViewer({
+      qrId: asset.qrCodes[0]?.id,
+      userId,
+      organizationId,
+      roles,
+    });
 
     const qrObj = await generateQrObj({
       assetId: asset.id,
@@ -189,13 +205,9 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) {
      * skip the heavy categories/locations/custom-field-defs queries for
      * users who are view-only. Uses the server-side `hasPermission` because
      * the client-side `userHasPermission` validator file has the `.client.`
-     * suffix and is stripped from the SSR bundle. Passing `roles` explicitly
-     * avoids the validator's DB fallback lookup.
+     * suffix and is stripped from the SSR bundle. `roles` is derived once at
+     * the top of the loader.
      */
-    const roles = userOrganizations.find(
-      (o) => o.organization.id === organizationId
-    )?.roles;
-
     const canEditAsset = await hasPermission({
       userId,
       organizationId,

apps/webapp/app/routes/_layout+/assets.$assetId.tsx

@@ -486,7 +486,16 @@ export default function AssetDetailsPage() {
 
   const items = [
     { to: "overview", content: "Overview" },
-    { to: "activity", content: "Activity" },
+    // The activity loader requires `note:read` and 403s without it, so a role
+    // that can't read notes must not be offered the tab. Mirrors the bookings
+    // detail page.
+    ...(userHasPermission({
+      roles,
+      entity: PermissionEntity.note,
+      action: PermissionAction.read,
+    })
+      ? [{ to: "activity", content: "Activity" }]
+      : []),
     { to: "bookings", content: "Bookings" },
     ...(userHasPermission({
       roles,