fix(accounting): gate cross-add counters on the permission their endpoint checks (#984)

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.
This commit is contained in:
Paul Nothaft
2026-08-04 14:17:34 +02:00
committed by GitHub
parent 83d514315e
commit 4b53b64277
2 changed files with 19 additions and 5 deletions
@@ -252,6 +252,10 @@ const RebillsPanel: React.FC<Props> = ({ 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<Props> = ({ 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,
});
@@ -56,6 +56,10 @@ export const HoursSection: React.FC<HoursSectionProps> = ({
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<HoursSectionProps> = ({
});
// 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);