From 4b53b64277a6e3b1d3e94b8cc3bb705aa33629ec Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Tue, 4 Aug 2026 14:17:34 +0200 Subject: [PATCH] fix(accounting): gate cross-add counters on the permission their endpoint checks (#984) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #983. The two cross-add counter queries added in #979 were enabled on customers.edit, but neither endpoint checks that permission: HoursSection -> GET /expenses/inbound/by-customer/:id needs accounting.view CustomerCrmPanels -> GET /customers/:id/hour-entries needs customers.view An admin holding customers.edit but not the corresponding read permission fired a guaranteed 403 on every customer-detail render. It degraded safely — the count stayed at its 0 default so the cross-add was never offered, which is the right outcome for that role — so this was request noise rather than broken behaviour. Each guard now requires both: the read permission to fetch the count, and the write permission because there is no point offering the cross-add to someone who cannot create the combined invoice. No seeded role is affected: migration 123 grants accounting.view and accounting.manage together, and customers.edit projects forward from customers.create, which migration 090 always grants alongside customers.view. --- frontend/src/components/admin/CustomerCrmPanels.tsx | 13 ++++++++++--- frontend/src/components/admin/HoursSection.tsx | 11 +++++++++-- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/frontend/src/components/admin/CustomerCrmPanels.tsx b/frontend/src/components/admin/CustomerCrmPanels.tsx index b59b57a8..8c102ae5 100644 --- a/frontend/src/components/admin/CustomerCrmPanels.tsx +++ b/frontend/src/components/admin/CustomerCrmPanels.tsx @@ -252,6 +252,10 @@ const RebillsPanel: React.FC = ({ customerAccountId }) => { const canView = usePermission('accounting.view'); const canManage = usePermission('accounting.manage'); const canCombine = usePermission('customers.edit'); + // The open-hours counter below reads GET /customers/:id/hour-entries, which + // requires customers.view — a different permission from the customers.edit + // that authorises the combined billing itself (#983). + const canViewCustomers = usePermission('customers.view'); const [crossAddOpen, setCrossAddOpen] = useState(false); const [busy, setBusy] = useState(false); @@ -262,12 +266,15 @@ const RebillsPanel: React.FC = ({ customerAccountId }) => { staleTime: 30_000, }); - // Open hours count for the cross-add offer — only when hours logging is on - // AND the admin can actually create the combined invoice. + // Open hours count for the cross-add offer — only when hours logging is on, + // the admin can actually create the combined invoice (customers.edit) AND can + // read the hour entries the count comes from (customers.view). Both are + // required: without the read permission the request just 403s on every + // render (#983). const { data: openHours = 0 } = useQuery({ queryKey: ['customer-open-hours-count', customerAccountId], queryFn: async () => (await customerAdminService.listHourEntries(customerAccountId, 'unbilled')).length, - enabled: !!flags.hoursLogging && canCombine, + enabled: !!flags.hoursLogging && canCombine && canViewCustomers, staleTime: 30_000, }); diff --git a/frontend/src/components/admin/HoursSection.tsx b/frontend/src/components/admin/HoursSection.tsx index dc5217b5..585f3a47 100644 --- a/frontend/src/components/admin/HoursSection.tsx +++ b/frontend/src/components/admin/HoursSection.tsx @@ -56,6 +56,10 @@ export const HoursSection: React.FC = ({ const { flags } = useFeatureFlags(); // Billing hours (and the combined path) go through customers.edit (#866 review). const canBill = usePermission('customers.edit'); + // The open-re-bills counter below reads GET /expenses/inbound/by-customer/:id, + // which requires accounting.view — a different permission from the one that + // authorises the billing itself (#983). + const canViewAccounting = usePermission('accounting.view'); const { format: fmtDate, formatTime: fmtTime } = useLocalizedDate(); const [entryDate, setEntryDate] = useState(() => new Date().toISOString().slice(0, 10)); const [startTime, setStartTime] = useState('09:00'); @@ -167,11 +171,14 @@ export const HoursSection: React.FC = ({ }); // Open re-bills count for the cross-add offer (#866) — only when the - // incoming-invoices feature is on and the admin can create the invoice. + // incoming-invoices feature is on, the admin can create the invoice + // (customers.edit) AND can read the re-bills the count comes from + // (accounting.view). Both are required: without the read permission the + // request just 403s on every render (#983). const { data: openRebills = 0 } = useQuery({ queryKey: ['customer-open-rebills-count', customerId], queryFn: async () => (await accountingService.listCustomerRebills(customerId)).filter((r) => r.status === 'open').length, - enabled: !!flags.incomingInvoices && canBill, + enabled: !!flags.incomingInvoices && canBill && canViewAccounting, staleTime: 30_000, }); const [crossAddOpen, setCrossAddOpen] = useState(false);