From c2ca6a6d0d1e88fe6e948344c10ae2f9143713e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hartmut=20N=C3=B6renberg?= Date: Mon, 30 Mar 2026 10:36:05 +0200 Subject: [PATCH] feat(holiday-calendar): restrict catalog reads to admins --- docs/route-access-matrix.md | 11 ++ .../src/__tests__/assistant-router.test.ts | 10 +- .../__tests__/holiday-calendar-router.test.ts | 128 +++++++++++++++++- packages/api/src/router/assistant.ts | 2 + packages/api/src/router/holiday-calendar.ts | 10 +- 5 files changed, 151 insertions(+), 10 deletions(-) diff --git a/docs/route-access-matrix.md b/docs/route-access-matrix.md index 578413c..18803e7 100644 --- a/docs/route-access-matrix.md +++ b/docs/route-access-matrix.md @@ -125,6 +125,17 @@ Reasoning: - the categories feed project configuration and planning/reporting workflows instead of broad self-service screens - `getById` includes `_count.projects`, so the detailed read should not remain a generic authenticated route +### `packages/api/src/router/holiday-calendar.ts` + +- `listCalendars`, `listCalendarsDetail`, `getCalendarByIdentifier`, `getCalendarByIdentifierDetail`, `getCalendarById`: `admin-only` +- create, update, delete calendar and entry mutations: `admin-only` +- holiday resolution and preview helpers remain unchanged in this rollout + +Reasoning: + +- the calendar catalog is currently consumed in the web app only by the admin vacation editor, so broad authenticated reads expose internal configuration without a product need +- narrowing just the catalog reads keeps the hardening slice small while avoiding regressions in shared holiday-resolution helpers used by vacation, timeline, and assistant flows + ## 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 792f46a..63f7863 100644 --- a/packages/api/src/__tests__/assistant-router.test.ts +++ b/packages/api/src/__tests__/assistant-router.test.ts @@ -629,17 +629,21 @@ describe("assistant router tool gating", () => { expect(userNames).toContain("get_ai_configured"); }); - it("keeps holiday calendar mutation tools admin-only while leaving read tools available", () => { + it("keeps holiday calendar catalog tools admin-only while leaving preview available", () => { const adminNames = getToolNames([], SystemRole.ADMIN); const managerNames = getToolNames([], SystemRole.MANAGER); + const userNames = getToolNames([], SystemRole.USER); expect(adminNames).toContain("list_holiday_calendars"); expect(adminNames).toContain("get_holiday_calendar"); expect(adminNames).toContain("preview_resolved_holiday_calendar"); expect(adminNames).toContain("create_holiday_calendar"); - expect(managerNames).toContain("list_holiday_calendars"); - expect(managerNames).toContain("get_holiday_calendar"); + expect(managerNames).not.toContain("list_holiday_calendars"); + expect(managerNames).not.toContain("get_holiday_calendar"); expect(managerNames).toContain("preview_resolved_holiday_calendar"); + expect(userNames).not.toContain("list_holiday_calendars"); + expect(userNames).not.toContain("get_holiday_calendar"); + expect(userNames).toContain("preview_resolved_holiday_calendar"); expect(managerNames).not.toContain("create_holiday_calendar"); expect(managerNames).not.toContain("update_holiday_calendar"); expect(managerNames).not.toContain("delete_holiday_calendar"); diff --git a/packages/api/src/__tests__/holiday-calendar-router.test.ts b/packages/api/src/__tests__/holiday-calendar-router.test.ts index f9e0f09..4b44988 100644 --- a/packages/api/src/__tests__/holiday-calendar-router.test.ts +++ b/packages/api/src/__tests__/holiday-calendar-router.test.ts @@ -40,6 +40,130 @@ function createAdminCaller(db: Record) { } describe("holiday calendar router", () => { + it("requires admin access for holiday calendar catalog reads", async () => { + const findMany = vi.fn(); + const findUnique = vi.fn(); + const findFirst = vi.fn(); + const caller = createProtectedCaller({ + holidayCalendar: { + findMany, + findUnique, + findFirst, + }, + }); + + await expect(caller.listCalendars({ includeInactive: true })).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Admin role required", + }); + await expect(caller.listCalendarsDetail({ includeInactive: true })).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Admin role required", + }); + await expect(caller.getCalendarByIdentifier({ identifier: "Deutschland" })).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Admin role required", + }); + await expect(caller.getCalendarByIdentifierDetail({ identifier: "Deutschland" })).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Admin role required", + }); + await expect(caller.getCalendarById({ id: "cal_de" })).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Admin role required", + }); + + expect(findMany).not.toHaveBeenCalled(); + expect(findUnique).not.toHaveBeenCalled(); + expect(findFirst).not.toHaveBeenCalled(); + }); + + it("allows admins to read holiday calendar catalog routes", async () => { + const listRows = [ + { + id: "cal_de", + name: "Deutschland", + scopeType: "COUNTRY", + stateCode: null, + isActive: true, + priority: 0, + country: { id: "country_de", code: "DE", name: "Deutschland" }, + metroCity: null, + _count: { entries: 1 }, + entries: [ + { + id: "entry_1", + date: new Date("2026-01-01T00:00:00.000Z"), + name: "Neujahr", + isRecurringAnnual: true, + source: "builtin", + }, + ], + }, + ]; + const detailRow = { + id: "cal_de", + name: "Deutschland", + scopeType: "COUNTRY", + stateCode: null, + isActive: true, + priority: 0, + country: { id: "country_de", code: "DE", name: "Deutschland" }, + metroCity: null, + entries: [ + { + id: "entry_1", + date: new Date("2026-01-01T00:00:00.000Z"), + name: "Neujahr", + isRecurringAnnual: true, + source: "builtin", + }, + ], + }; + const findMany = vi.fn().mockResolvedValue(listRows); + const findUnique = vi + .fn() + .mockResolvedValueOnce(detailRow) + .mockResolvedValueOnce(detailRow) + .mockResolvedValueOnce(detailRow); + const findFirst = vi.fn(); + const caller = createAdminCaller({ + holidayCalendar: { + findMany, + findUnique, + findFirst, + }, + }); + + const listResult = await caller.listCalendars({ includeInactive: true }); + const detailResult = await caller.listCalendarsDetail({ includeInactive: true }); + const byIdentifierResult = await caller.getCalendarByIdentifier({ identifier: "cal_de" }); + const byIdentifierDetailResult = await caller.getCalendarByIdentifierDetail({ identifier: "cal_de" }); + const byIdResult = await caller.getCalendarById({ id: "cal_de" }); + + expect(listResult).toEqual(listRows); + expect(detailResult).toEqual({ + count: 1, + calendars: [ + expect.objectContaining({ + id: "cal_de", + entryCount: 1, + }), + ], + }); + expect(byIdentifierResult).toEqual(detailRow); + expect(byIdentifierDetailResult).toEqual( + expect.objectContaining({ + id: "cal_de", + entryCount: 1, + }), + ); + expect(byIdResult).toEqual(detailRow); + expect(findMany).toHaveBeenCalledTimes(2); + expect(findUnique).toHaveBeenCalledTimes(3); + expect(findFirst).not.toHaveBeenCalled(); + }); + it("lists holiday calendars with assistant-facing detail formatting", async () => { const db = { holidayCalendar: { @@ -68,7 +192,7 @@ describe("holiday calendar router", () => { }, }; - const caller = createProtectedCaller(db); + const caller = createAdminCaller(db); const result = await caller.listCalendarsDetail({ countryCode: "DE", scopeType: "STATE", @@ -127,7 +251,7 @@ describe("holiday calendar router", () => { }, }; - const caller = createProtectedCaller(db); + const caller = createAdminCaller(db); const result = await caller.getCalendarByIdentifierDetail({ identifier: "Augsburg lokal" }); expect(result).toEqual( diff --git a/packages/api/src/router/assistant.ts b/packages/api/src/router/assistant.ts index 9553972..1fe525c 100644 --- a/packages/api/src/router/assistant.ts +++ b/packages/api/src/router/assistant.ts @@ -360,6 +360,8 @@ const ADMIN_ONLY_TOOLS = new Set([ "create_metro_city", "update_metro_city", "delete_metro_city", + "list_holiday_calendars", + "get_holiday_calendar", "create_holiday_calendar", "update_holiday_calendar", "delete_holiday_calendar", diff --git a/packages/api/src/router/holiday-calendar.ts b/packages/api/src/router/holiday-calendar.ts index b4bc620..d684d54 100644 --- a/packages/api/src/router/holiday-calendar.ts +++ b/packages/api/src/router/holiday-calendar.ts @@ -548,7 +548,7 @@ async function assertScopeConsistency( } export const holidayCalendarRouter = createTRPCRouter({ - listCalendars: protectedProcedure + listCalendars: adminProcedure .input(z.object({ includeInactive: z.boolean().optional(), countryCode: z.string().trim().min(1).optional(), @@ -558,7 +558,7 @@ export const holidayCalendarRouter = createTRPCRouter({ }).optional()) .query(async ({ ctx, input }) => readCalendarsSnapshot(ctx, input)), - listCalendarsDetail: protectedProcedure + listCalendarsDetail: adminProcedure .input(z.object({ includeInactive: z.boolean().optional(), countryCode: z.string().trim().min(1).optional(), @@ -574,18 +574,18 @@ export const holidayCalendarRouter = createTRPCRouter({ }; }), - getCalendarByIdentifier: protectedProcedure + getCalendarByIdentifier: adminProcedure .input(z.object({ identifier: z.string().trim().min(1) })) .query(async ({ ctx, input }) => readCalendarByIdentifierSnapshot(ctx, input.identifier)), - getCalendarByIdentifierDetail: protectedProcedure + getCalendarByIdentifierDetail: adminProcedure .input(z.object({ identifier: z.string().trim().min(1) })) .query(async ({ ctx, input }) => { const calendar = await readCalendarByIdentifierSnapshot(ctx, input.identifier); return formatHolidayCalendarDetail(calendar); }), - getCalendarById: protectedProcedure + getCalendarById: adminProcedure .input(z.object({ id: z.string() })) .query(async ({ ctx, input }) => { const db = asHolidayCalendarDb(ctx.db);