From ae74700f7c259f1d7a46619a5cae77581d379e89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hartmut=20N=C3=B6renberg?= Date: Mon, 30 Mar 2026 10:24:52 +0200 Subject: [PATCH] feat(client): scope planning reads to explicit audience --- docs/route-access-matrix.md | 13 ++ .../src/__tests__/assistant-router.test.ts | 2 + .../__tests__/master-data-router-auth.test.ts | 179 ++++++++++++++++++ packages/api/src/router/assistant.ts | 1 + packages/api/src/router/client.ts | 16 +- 5 files changed, 206 insertions(+), 5 deletions(-) diff --git a/docs/route-access-matrix.md b/docs/route-access-matrix.md index 69f8d9c..45b422d 100644 --- a/docs/route-access-matrix.md +++ b/docs/route-access-matrix.md @@ -102,6 +102,19 @@ Reasoning: - `list` and especially `getTree` expose the internal org hierarchy, parent links, sort order, and structure metadata, so they should not remain broad authenticated reads - detailed org-unit reads also expose `_count.resources` and parent/child context that maps the staffing structure +### `packages/api/src/router/client.ts` + +- `resolveByIdentifier`: `authenticated-safe-lookup` +- `list`, `getTree`, `getByIdentifier`, `getById`: `planning-read` +- create and update: `manager-write` +- delete: `admin-only` + +Reasoning: + +- `resolveByIdentifier` returns a deliberately narrow lookup shape for code/name resolution +- `list` already exposes `_count.children` and `_count.projects`, and `getTree` reveals the full client hierarchy used in planning and reporting flows +- detailed client reads add parent/child structure plus project counts, so they should align with the explicit planning audience instead of broad authenticated access + ## Assistant Parity Rule - assistant tool visibility must never widen the audience of the backing router diff --git a/packages/api/src/__tests__/assistant-router.test.ts b/packages/api/src/__tests__/assistant-router.test.ts index c80b41b..d5e5b21 100644 --- a/packages/api/src/__tests__/assistant-router.test.ts +++ b/packages/api/src/__tests__/assistant-router.test.ts @@ -387,6 +387,7 @@ describe("assistant router tool gating", () => { expect(userWithoutPlanning).not.toContain("list_allocations"); expect(userWithoutPlanning).not.toContain("list_demands"); + expect(userWithoutPlanning).not.toContain("list_clients"); expect(userWithoutPlanning).not.toContain("list_roles"); expect(userWithoutPlanning).not.toContain("check_resource_availability"); expect(userWithoutPlanning).not.toContain("find_capacity"); @@ -394,6 +395,7 @@ describe("assistant router tool gating", () => { expect(userWithoutPlanning).not.toContain("find_best_project_resource"); expect(userWithPlanning).toContain("list_allocations"); expect(userWithPlanning).toContain("list_demands"); + expect(userWithPlanning).toContain("list_clients"); expect(userWithPlanning).toContain("list_roles"); expect(userWithPlanning).toContain("check_resource_availability"); expect(userWithPlanning).toContain("find_capacity"); diff --git a/packages/api/src/__tests__/master-data-router-auth.test.ts b/packages/api/src/__tests__/master-data-router-auth.test.ts index 76b198a..0848a48 100644 --- a/packages/api/src/__tests__/master-data-router-auth.test.ts +++ b/packages/api/src/__tests__/master-data-router-auth.test.ts @@ -1,5 +1,6 @@ import { PermissionKey, SystemRole } from "@capakraken/shared"; import { describe, expect, it, vi } from "vitest"; +import { clientRouter } from "../router/client.js"; import { countryRouter } from "../router/country.js"; import { orgUnitRouter } from "../router/org-unit.js"; import { createCallerFactory } from "../trpc.js"; @@ -327,4 +328,182 @@ describe("master-data router authorization", () => { orderBy: [{ sortOrder: "asc" }, { name: "asc" }], }); }); + + it("keeps minimal client lookups available to authenticated users", async () => { + const findUnique = vi.fn() + .mockResolvedValueOnce(null) + .mockResolvedValueOnce({ + id: "client_1", + name: "Acme Studios", + code: "ACME", + parentId: null, + isActive: true, + }); + const caller = createCallerFactory(clientRouter)(createProtectedContext({ + client: { + findUnique, + findFirst: vi.fn(), + }, + })); + + const result = await caller.resolveByIdentifier({ identifier: "ACME" }); + + expect(result).toEqual({ + id: "client_1", + name: "Acme Studios", + code: "ACME", + parentId: null, + isActive: true, + }); + }); + + it("requires planning read access for client list and tree reads", async () => { + const findMany = vi.fn(); + const caller = createCallerFactory(clientRouter)(createProtectedContext({ + client: { + findMany, + }, + })); + + await expect(caller.list({ isActive: true })).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Planning read access required", + }); + await expect(caller.getTree({ isActive: true })).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Planning read access required", + }); + + expect(findMany).not.toHaveBeenCalled(); + }); + + it("requires planning read access for detailed client reads with project counts", async () => { + const caller = createCallerFactory(clientRouter)(createProtectedContext({ + client: { + findFirst: vi.fn(), + findUnique: vi.fn(), + }, + })); + + await expect(caller.getByIdentifier({ identifier: "Acme" })).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Planning read access required", + }); + await expect(caller.getById({ id: "client_1" })).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Planning read access required", + }); + }); + + it("allows client list, tree, and detail reads for users with planning access", async () => { + const listFindMany = vi.fn().mockResolvedValue([ + { + id: "client_1", + name: "Acme Studios", + code: "ACME", + parentId: null, + isActive: true, + sortOrder: 10, + tags: ["enterprise"], + createdAt: new Date("2026-03-01T00:00:00.000Z"), + updatedAt: new Date("2026-03-01T00:00:00.000Z"), + _count: { children: 1, projects: 4 }, + }, + ]); + const treeFindMany = vi.fn().mockResolvedValue([ + { + id: "client_1", + name: "Acme Studios", + code: "ACME", + parentId: null, + isActive: true, + sortOrder: 10, + tags: ["enterprise"], + createdAt: new Date("2026-03-01T00:00:00.000Z"), + updatedAt: new Date("2026-03-01T00:00:00.000Z"), + }, + ]); + const getByIdFindUnique = vi.fn() + .mockResolvedValueOnce({ + id: "client_1", + name: "Acme Studios", + code: "ACME", + parentId: null, + isActive: true, + sortOrder: 10, + tags: ["enterprise"], + createdAt: new Date("2026-03-01T00:00:00.000Z"), + updatedAt: new Date("2026-03-01T00:00:00.000Z"), + parent: null, + children: [], + _count: { children: 1, projects: 4 }, + }) + .mockResolvedValueOnce(null); + const getByIdentifierFindFirst = vi.fn().mockResolvedValue({ + id: "client_1", + name: "Acme Studios", + code: "ACME", + parentId: null, + isActive: true, + sortOrder: 10, + tags: ["enterprise"], + createdAt: new Date("2026-03-01T00:00:00.000Z"), + updatedAt: new Date("2026-03-01T00:00:00.000Z"), + _count: { children: 1, projects: 4 }, + }); + const caller = createCallerFactory(clientRouter)(createProtectedContext({ + client: { + findMany: vi.fn() + .mockImplementationOnce(listFindMany) + .mockImplementationOnce(treeFindMany), + findUnique: getByIdFindUnique, + findFirst: getByIdentifierFindFirst, + }, + }, { + granted: [PermissionKey.VIEW_PLANNING], + })); + + const listResult = await caller.list({ isActive: true, search: "Acme" }); + const treeResult = await caller.getTree({ isActive: true }); + const byIdResult = await caller.getById({ id: "client_1" }); + const byIdentifierResult = await caller.getByIdentifier({ identifier: "Acme Studios" }); + + expect(listResult).toHaveLength(1); + expect(treeResult).toEqual([ + expect.objectContaining({ + id: "client_1", + name: "Acme Studios", + children: [], + }), + ]); + expect(byIdResult._count.projects).toBe(4); + expect(byIdentifierResult._count.projects).toBe(4); + expect(listFindMany).toHaveBeenCalledWith({ + where: { + isActive: true, + OR: [ + { name: { contains: "Acme", mode: "insensitive" } }, + { code: { contains: "Acme", mode: "insensitive" } }, + ], + }, + include: { _count: { select: { children: true, projects: true } } }, + orderBy: [{ sortOrder: "asc" }, { name: "asc" }], + }); + expect(treeFindMany).toHaveBeenCalledWith({ + where: { isActive: true }, + orderBy: [{ sortOrder: "asc" }, { name: "asc" }], + }); + expect(getByIdFindUnique).toHaveBeenCalledWith({ + where: { id: "client_1" }, + include: { + parent: true, + children: { orderBy: { sortOrder: "asc" } }, + _count: { select: { projects: true, children: true } }, + }, + }); + expect(getByIdentifierFindFirst).toHaveBeenCalledWith(expect.objectContaining({ + where: { name: { equals: "Acme Studios", mode: "insensitive" } }, + include: { _count: { select: { projects: true, children: true } } }, + })); + }); }); diff --git a/packages/api/src/router/assistant.ts b/packages/api/src/router/assistant.ts index fb850d2..432153c 100644 --- a/packages/api/src/router/assistant.ts +++ b/packages/api/src/router/assistant.ts @@ -228,6 +228,7 @@ const COST_TOOLS = new Set([ const PLANNING_READ_TOOLS = new Set([ "list_allocations", "list_demands", + "list_clients", "list_roles", "check_resource_availability", "get_staffing_suggestions", diff --git a/packages/api/src/router/client.ts b/packages/api/src/router/client.ts index 458c4cb..8524572 100644 --- a/packages/api/src/router/client.ts +++ b/packages/api/src/router/client.ts @@ -3,7 +3,13 @@ import { TRPCError } from "@trpc/server"; import { z } from "zod"; import { findUniqueOrThrow } from "../db/helpers.js"; import { createAuditEntry } from "../lib/audit.js"; -import { adminProcedure, createTRPCRouter, managerProcedure, protectedProcedure } from "../trpc.js"; +import { + adminProcedure, + createTRPCRouter, + managerProcedure, + planningReadProcedure, + protectedProcedure, +} from "../trpc.js"; import type { ClientTree } from "@capakraken/shared"; @@ -30,7 +36,7 @@ function buildClientTree(flatItems: FlatClient[], parentId: string | null = null } export const clientRouter = createTRPCRouter({ - list: protectedProcedure + list: planningReadProcedure .input( z.object({ parentId: z.string().nullable().optional(), @@ -57,7 +63,7 @@ export const clientRouter = createTRPCRouter({ }); }), - getTree: protectedProcedure + getTree: planningReadProcedure .input(z.object({ isActive: z.boolean().optional() }).optional()) .query(async ({ ctx, input }) => { const all = await ctx.db.client.findMany({ @@ -69,7 +75,7 @@ export const clientRouter = createTRPCRouter({ return buildClientTree(all); }), - getById: protectedProcedure + getById: planningReadProcedure .input(z.object({ id: z.string() })) .query(async ({ ctx, input }) => { const client = await findUniqueOrThrow( @@ -136,7 +142,7 @@ export const clientRouter = createTRPCRouter({ return client; }), - getByIdentifier: protectedProcedure + getByIdentifier: planningReadProcedure .input(z.object({ identifier: z.string().trim().min(1) })) .query(async ({ ctx, input }) => { const identifier = input.identifier.trim();