diff --git a/docs/route-access-matrix.md b/docs/route-access-matrix.md index 3e89f00..31de257 100644 --- a/docs/route-access-matrix.md +++ b/docs/route-access-matrix.md @@ -79,6 +79,28 @@ Reasoning: - system role defaults define the effective permission model and therefore belong to the smallest operational audience +### `packages/api/src/router/country.ts` + +- `list`, `resolveByIdentifier`, `getCityById`: `authenticated-safe-lookup` +- `getByIdentifier`, `getById`: `resource-overview` +- create, update, metro-city writes: `admin-only` + +Reasoning: + +- minimal country lookups are needed broadly for forms, filters, and location resolution +- detailed country reads include metro-city detail plus `_count.resources`, so they should align with broad people-directory visibility + +### `packages/api/src/router/org-unit.ts` + +- `list`, `getTree`, `resolveByIdentifier`: `authenticated-safe-lookup` +- `getByIdentifier`, `getById`: `resource-overview` +- create, update, deactivate: `admin-only` + +Reasoning: + +- minimal org-unit lookups are low-risk master data +- detailed org-unit reads expose `_count.resources` and parent/child context that maps the staffing structure + ## 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 95cb49f..56a08be 100644 --- a/packages/api/src/__tests__/assistant-router.test.ts +++ b/packages/api/src/__tests__/assistant-router.test.ts @@ -645,21 +645,25 @@ describe("assistant router tool gating", () => { it("keeps country and metro-city mutation tools admin-only while leaving read tools available", () => { const adminNames = getToolNames([], SystemRole.ADMIN); const managerNames = getToolNames([], SystemRole.MANAGER); + const userNames = getToolNames([], SystemRole.USER); + const userWithResourceOverview = getToolNames([PermissionKey.VIEW_ALL_RESOURCES], SystemRole.USER); expect(adminNames).toContain("list_countries"); - expect(adminNames).toContain("get_country"); expect(adminNames).toContain("create_country"); expect(adminNames).toContain("update_country"); expect(adminNames).toContain("create_metro_city"); expect(adminNames).toContain("update_metro_city"); expect(adminNames).toContain("delete_metro_city"); expect(managerNames).toContain("list_countries"); - expect(managerNames).toContain("get_country"); expect(managerNames).not.toContain("create_country"); expect(managerNames).not.toContain("update_country"); expect(managerNames).not.toContain("create_metro_city"); expect(managerNames).not.toContain("update_metro_city"); expect(managerNames).not.toContain("delete_metro_city"); + expect(userNames).not.toContain("get_country"); + expect(userNames).not.toContain("list_org_units"); + expect(userWithResourceOverview).toContain("get_country"); + expect(userWithResourceOverview).toContain("list_org_units"); }); it("blocks mutation tools until the user confirms a prior assistant summary", () => { diff --git a/packages/api/src/__tests__/master-data-router-auth.test.ts b/packages/api/src/__tests__/master-data-router-auth.test.ts new file mode 100644 index 0000000..8843b0c --- /dev/null +++ b/packages/api/src/__tests__/master-data-router-auth.test.ts @@ -0,0 +1,175 @@ +import { PermissionKey, SystemRole } from "@capakraken/shared"; +import { describe, expect, it, vi } from "vitest"; +import { countryRouter } from "../router/country.js"; +import { orgUnitRouter } from "../router/org-unit.js"; +import { createCallerFactory } from "../trpc.js"; + +function createProtectedContext( + db: Record, + overrides: { granted?: PermissionKey[]; denied?: PermissionKey[] } | null = null, +) { + return { + session: { + user: { email: "user@example.com", name: "User", image: null }, + expires: "2099-01-01T00:00:00.000Z", + }, + db: db as never, + dbUser: { + id: "user_1", + systemRole: SystemRole.USER, + permissionOverrides: overrides, + }, + }; +} + +describe("master-data router authorization", () => { + it("keeps minimal country lookups available to authenticated users", async () => { + const findFirst = vi.fn().mockResolvedValue({ + id: "country_de", + code: "DE", + name: "Germany", + isActive: true, + dailyWorkingHours: 8, + }); + const caller = createCallerFactory(countryRouter)(createProtectedContext({ + country: { + findUnique: vi.fn().mockResolvedValue(null), + findFirst, + }, + })); + + const result = await caller.resolveByIdentifier({ identifier: "de" }); + + expect(result).toEqual({ + id: "country_de", + code: "DE", + name: "Germany", + isActive: true, + dailyWorkingHours: 8, + }); + expect(findFirst).toHaveBeenCalled(); + }); + + it("requires resource overview access for detailed country reads with resource counts", async () => { + const caller = createCallerFactory(countryRouter)(createProtectedContext({ + country: { + findFirst: vi.fn(), + findUnique: vi.fn(), + }, + })); + + await expect(caller.getByIdentifier({ identifier: "DE" })).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Resource overview access required", + }); + await expect(caller.getById({ id: "country_de" })).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Resource overview access required", + }); + }); + + it("allows detailed country reads for users with resource overview access", async () => { + const findFirst = vi.fn().mockResolvedValue({ + id: "country_de", + code: "DE", + name: "Germany", + isActive: true, + dailyWorkingHours: 8, + scheduleRules: null, + metroCities: [{ id: "city_muc", name: "Munich", countryId: "country_de" }], + _count: { resources: 12 }, + }); + const caller = createCallerFactory(countryRouter)(createProtectedContext({ + country: { + findUnique: vi.fn().mockResolvedValue(null), + findFirst, + }, + }, { + granted: [PermissionKey.VIEW_ALL_RESOURCES], + })); + + const result = await caller.getByIdentifier({ identifier: "DE" }); + + expect(result._count.resources).toBe(12); + expect(findFirst).toHaveBeenCalledWith(expect.objectContaining({ + include: expect.objectContaining({ + metroCities: expect.any(Object), + _count: expect.any(Object), + }), + })); + }); + + it("keeps minimal org-unit lookups available to authenticated users", async () => { + const findFirst = vi.fn() + .mockResolvedValueOnce(null) + .mockResolvedValueOnce({ + id: "ou_1", + name: "Delivery", + shortName: "DEL", + level: 5, + isActive: true, + }); + const caller = createCallerFactory(orgUnitRouter)(createProtectedContext({ + orgUnit: { + findUnique: vi.fn().mockResolvedValue(null), + findFirst, + }, + })); + + const result = await caller.resolveByIdentifier({ identifier: "DEL" }); + + expect(result).toEqual({ + id: "ou_1", + name: "Delivery", + shortName: "DEL", + level: 5, + isActive: true, + }); + }); + + it("requires resource overview access for detailed org-unit reads with staffing counts", async () => { + const caller = createCallerFactory(orgUnitRouter)(createProtectedContext({ + orgUnit: { + findFirst: vi.fn(), + findUnique: vi.fn(), + }, + })); + + await expect(caller.getByIdentifier({ identifier: "Delivery" })).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Resource overview access required", + }); + await expect(caller.getById({ id: "ou_1" })).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Resource overview access required", + }); + }); + + it("allows detailed org-unit reads for users with resource overview access", async () => { + const findFirst = vi.fn().mockResolvedValue({ + id: "ou_1", + name: "Delivery", + shortName: "DEL", + level: 5, + parentId: null, + sortOrder: 10, + isActive: true, + _count: { resources: 7 }, + }); + const caller = createCallerFactory(orgUnitRouter)(createProtectedContext({ + orgUnit: { + findUnique: vi.fn().mockResolvedValue(null), + findFirst, + }, + }, { + granted: [PermissionKey.VIEW_ALL_RESOURCES], + })); + + const result = await caller.getByIdentifier({ identifier: "Delivery" }); + + expect(result._count.resources).toBe(7); + expect(findFirst).toHaveBeenCalledWith(expect.objectContaining({ + include: { _count: { select: { resources: true } } }, + })); + }); +}); diff --git a/packages/api/src/router/assistant.ts b/packages/api/src/router/assistant.ts index 5227aae..c4a9261 100644 --- a/packages/api/src/router/assistant.ts +++ b/packages/api/src/router/assistant.ts @@ -236,6 +236,12 @@ const PLANNING_READ_TOOLS = new Set([ "find_best_project_resource", ]); +/** Tools that require broad people-directory visibility because the backing routes expose resource-linked counts. */ +const RESOURCE_OVERVIEW_TOOLS = new Set([ + "get_country", + "list_org_units", +]); + /** Tools that follow controllerProcedure access rules in the main API. */ const CONTROLLER_ONLY_TOOLS = new Set([ "search_projects", @@ -363,6 +369,8 @@ export function getAvailableAssistantTools(permissions: Set, user return TOOL_DEFINITIONS.filter((tool) => { const toolName = tool.function.name; const requiredPerm = TOOL_PERMISSION_MAP[toolName]; + const hasResourceOverviewAccess = permissions.has(PermissionKey.VIEW_ALL_RESOURCES) + || permissions.has(PermissionKey.MANAGE_RESOURCES); const hasControllerAccess = userRole === SystemRole.ADMIN || userRole === SystemRole.MANAGER || userRole === SystemRole.CONTROLLER; @@ -378,6 +386,9 @@ export function getAvailableAssistantTools(permissions: Set, user if (MANAGER_ONLY_TOOLS.has(toolName) && !hasManagerAccess) { return false; } + if (RESOURCE_OVERVIEW_TOOLS.has(toolName) && !hasResourceOverviewAccess) { + return false; + } if (CONTROLLER_ONLY_TOOLS.has(toolName) && !hasControllerAccess) { return false; } diff --git a/packages/api/src/router/country.ts b/packages/api/src/router/country.ts index 49f8797..06f8cc8 100644 --- a/packages/api/src/router/country.ts +++ b/packages/api/src/router/country.ts @@ -9,7 +9,12 @@ import { TRPCError } from "@trpc/server"; import { z } from "zod"; import { findUniqueOrThrow } from "../db/helpers.js"; import { createAuditEntry } from "../lib/audit.js"; -import { adminProcedure, createTRPCRouter, protectedProcedure } from "../trpc.js"; +import { + adminProcedure, + createTRPCRouter, + protectedProcedure, + resourceOverviewProcedure, +} from "../trpc.js"; /** Convert nullable JSON to Prisma-compatible value (null → Prisma.JsonNull). */ function jsonOrNull(val: unknown): Prisma.InputJsonValue | typeof Prisma.JsonNull { @@ -75,7 +80,7 @@ export const countryRouter = createTRPCRouter({ return country; }), - getByIdentifier: protectedProcedure + getByIdentifier: resourceOverviewProcedure .input(z.object({ identifier: z.string().trim().min(1) })) .query(async ({ ctx, input }) => { const identifier = input.identifier.trim(); @@ -124,7 +129,7 @@ export const countryRouter = createTRPCRouter({ return country; }), - getById: protectedProcedure + getById: resourceOverviewProcedure .input(z.object({ id: z.string() })) .query(async ({ ctx, input }) => { const country = await findUniqueOrThrow( diff --git a/packages/api/src/router/org-unit.ts b/packages/api/src/router/org-unit.ts index c839a90..6def50a 100644 --- a/packages/api/src/router/org-unit.ts +++ b/packages/api/src/router/org-unit.ts @@ -3,7 +3,12 @@ import { TRPCError } from "@trpc/server"; import { z } from "zod"; import { findUniqueOrThrow } from "../db/helpers.js"; import { createAuditEntry } from "../lib/audit.js"; -import { adminProcedure, createTRPCRouter, protectedProcedure } from "../trpc.js"; +import { + adminProcedure, + createTRPCRouter, + protectedProcedure, + resourceOverviewProcedure, +} from "../trpc.js"; import type { OrgUnitTree } from "@capakraken/shared"; @@ -61,7 +66,7 @@ export const orgUnitRouter = createTRPCRouter({ return buildTree(all); }), - getById: protectedProcedure + getById: resourceOverviewProcedure .input(z.object({ id: z.string() })) .query(async ({ ctx, input }) => { const unit = await findUniqueOrThrow( @@ -128,7 +133,7 @@ export const orgUnitRouter = createTRPCRouter({ return unit; }), - getByIdentifier: protectedProcedure + getByIdentifier: resourceOverviewProcedure .input(z.object({ identifier: z.string().trim().min(1) })) .query(async ({ ctx, input }) => { const identifier = input.identifier.trim();