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.test.server.ts

@@ -0,0 +1,186 @@
+// @vitest-environment node
+/**
+ * Server-side authorization test for the asset activity (notes) loader.
+ *
+ * The loader used to gate on `asset:read` and then hand every note back in the
+ * page payload, leaving `note:read` to a check in the component. BASE and
+ * SELF_SERVICE hold `asset: [read]` and `note: []`, so both roles received the
+ * full note list and it was hidden only by React — a server-side authorization
+ * gap, not a display bug.
+ *
+ * These tests drive the real `Role2PermissionMap` through a `requirePermission`
+ * stub, so they assert the actual role matrix rather than a restatement of it:
+ * a role without `note:read` must be rejected BEFORE any note is read, and a
+ * role with it must still get its notes.
+ *
+ * @see {@link file://./assets.$assetId.activity.tsx}
+ */
+import { OrganizationRoles } from "@prisma/client";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { ShelfError } from "~/utils/error";
+import {
+  PermissionAction,
+  PermissionEntity,
+  Role2PermissionMap,
+} from "~/utils/permissions/permission.data";
+
+const { getAsset, getPaginatedAndFilterableAssetNotes, requirePermission } =
+  vi.hoisted(() => ({
+    getAsset: vi.fn(),
+    getPaginatedAndFilterableAssetNotes: vi.fn(),
+    requirePermission: vi.fn(),
+  }));
+
+// why: the loader's only authorization call. Stubbed so each test can drive it
+// with a different role while still resolving against the REAL permission
+// matrix (see `actAs` below) — that keeps the assertion honest if the matrix
+// changes.
+vi.mock("~/utils/roles.server", () => ({ requirePermission }));
+
+// why: both are DB reads; we only care about WHETHER they run, not what they
+// return.
+vi.mock("~/modules/asset/service.server", () => ({ getAsset }));
+vi.mock("~/modules/note/service.server", () => ({
+  getPaginatedAndFilterableAssetNotes,
+}));
+
+// why: the success path serializes a per-page cookie preference, which needs a
+// signing secret from the environment. Orthogonal to authorization.
+vi.mock("~/utils/cookies.server", () => ({
+  setCookie: vi.fn().mockReturnValue(["Set-Cookie", "perPage=20"]),
+  userPrefs: { serialize: vi.fn().mockResolvedValue("perPage=20") },
+}));
+
+import { loader } from "./assets.$assetId.activity";
+
+/**
+ * Points the `requirePermission` stub at a role, resolving each call against
+ * the real `Role2PermissionMap` (with the ADMIN/OWNER allow-all short-circuit
+ * the server applies) and throwing a 403-shaped error when the role lacks the
+ * requested permission.
+ */
+function actAs(role: OrganizationRoles) {
+  requirePermission.mockImplementation(
+    ({
+      entity,
+      action,
+    }: {
+      entity: PermissionEntity;
+      action: PermissionAction;
+    }) => {
+      const isAdminOrOwner =
+        role === OrganizationRoles.ADMIN || role === OrganizationRoles.OWNER;
+      const granted = Role2PermissionMap[role]?.[entity] ?? [];
+
+      if (!isAdminOrOwner && !granted.includes(action)) {
+        // why: a real `ShelfError`, not a bare Error — `validatePermission`
+        // throws exactly this shape, and the loader's `makeShelfError` only
+        // preserves the 403 for a ShelfError (anything else becomes a generic
+        // 500). A bare Error would let the denial assertion pass while the
+        // route actually surfaced the wrong status.
+        return Promise.reject(
+          new ShelfError({
+            cause: null,
+            title: "Unauthorized",
+            message: "You have no permission to perform this action",
+            status: 403,
+            label: "Permission",
+            shouldBeCaptured: false,
+          })
+        );
+      }
+
+      return Promise.resolve({
+        organizationId: "org-1",
+        userOrganizations: [],
+        role,
+      });
+    }
+  );
+}
+
+/** Minimal loader args for `/assets/asset-1/activity`. */
+function loaderArgs() {
+  return {
+    context: { getSession: () => ({ userId: "user-1" }) },
+    request: new Request("http://localhost/assets/asset-1/activity"),
+    params: { assetId: "asset-1" },
+  } as unknown as Parameters<typeof loader>[0];
+}
+
+describe("asset activity loader — note:read is enforced server-side", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    getAsset.mockResolvedValue({ id: "asset-1", title: "Camera" });
+    getPaginatedAndFilterableAssetNotes.mockResolvedValue({
+      page: 1,
+      perPage: 20,
+      search: null,
+      items: [{ id: "note-1", content: "Serial number is 8891-B" }],
+      totalItems: 1,
+      totalPages: 1,
+      hasNotes: true,
+      cookie: { perPage: 20 },
+    });
+  });
+
+  it("asks for note:read, not asset:read", async () => {
+    actAs(OrganizationRoles.OWNER);
+
+    await loader(loaderArgs());
+
+    // The parent route already enforces `asset:read`; this child must require
+    // the permission for the data it actually returns.
+    expect(requirePermission).toHaveBeenCalledWith(
+      expect.objectContaining({
+        entity: PermissionEntity.note,
+        action: PermissionAction.read,
+      })
+    );
+  });
+
+  it.each([OrganizationRoles.BASE, OrganizationRoles.SELF_SERVICE])(
+    "rejects %s and never reads a single note",
+    async (role) => {
+      // Precondition: this is exactly the gap. The role CAN read the asset but
+      // has no note permission at all.
+      expect(Role2PermissionMap[role]?.[PermissionEntity.asset]).toContain(
+        PermissionAction.read
+      );
+      expect(Role2PermissionMap[role]?.[PermissionEntity.note]).toEqual([]);
+
+      actAs(role);
+
+      // The loader rethrows as `data(..., { status })`. Assert the observable
+      // outcome is a 403 specifically — `toBeDefined()` alone would also pass
+      // on a parse error or a 500 from error mapping, which is not the denial
+      // this test is about.
+      const thrown = await loader(loaderArgs()).then(
+        () => null,
+        (caught: unknown) => caught
+      );
+
+      expect((thrown as { init?: { status?: number } })?.init?.status).toBe(
+        403
+      );
+
+      // The point of the fix: the notes are never fetched, so they can never
+      // reach the payload. A client-side check would have let this call run.
+      expect(getPaginatedAndFilterableAssetNotes).not.toHaveBeenCalled();
+    }
+  );
+
+  it("still returns notes for a role that holds note:read", async () => {
+    actAs(OrganizationRoles.ADMIN);
+
+    // `data()` returns a DataWithResponseInit wrapper, not a Response.
+    const result = (await loader(loaderArgs())) as unknown as {
+      data: { items: unknown[] };
+    };
+
+    expect(getPaginatedAndFilterableAssetNotes).toHaveBeenCalledWith(
+      expect.objectContaining({ assetId: "asset-1", organizationId: "org-1" })
+    );
+    expect(result.data.items).toHaveLength(1);
+  });
+});