Skip to content

fix: close cross-tenant billing/tenant security holes + string-enum API contract#1271

Merged
iammukeshm merged 2 commits into
mainfrom
fix/billing-tenant-security-audit
May 30, 2026
Merged

fix: close cross-tenant billing/tenant security holes + string-enum API contract#1271
iammukeshm merged 2 commits into
mainfrom
fix/billing-tenant-security-audit

Conversation

@iammukeshm
Copy link
Copy Markdown
Member

Deep audit of the billing / subscription / tenant-management system across the backend and both React apps, plus the fixes. Every backend bug has a failing-first test.

Security (P0 — cross-tenant)

BillingDbContext extends a raw, non-tenant-filtered DbContext (for operator cross-tenant visibility), so every handler must scope explicitly. Several didn't:

Handler / area Before After
Invoice read / PDF / list by id any tenant could read others' root-gated (operator cross-tenant; tenants pinned to self)
Invoice mutations (issue/pay/void) any tenant admin could mutate others' 404 cross-tenant
AssignSubscription / CaptureUsageSnapshots could target any tenant via body tenantId pinned to caller
GetUsageSnapshots read every tenant's usage scoped to caller
GenerateInvoices any admin triggered platform-wide billing root-only (403)
RoleService.FilterRootPermissions privilege escalation — stripped a "Permissions.Root." prefix that matched no real root permission, so a non-root admin could grant their role root permissions filtered by the IsRoot permission catalog

All gated on MultitenancyConstants.Root.Id. Existing isolation tests missed this because they always authenticate with a matching tenant header — they never tested tenant-A's token acting on tenant-B's data.

Verified non-issue: the tenant-header-vs-JWT-claim concern is not exploitable — Finbuckle's ClaimStrategy is registered first and binds the resolved tenant to the JWT claim for non-root callers (proven by the existing TenantAdmin_HeaderOverride_Should_BeIgnored). No "fix" added.

Correctness

  • Usage/overage invoice was silently skipped when a subscription invoice shared the month (GenerateInvoiceForPeriodAsync idempotency lacked Purpose==Usage) — lost revenue.
  • Same-plan renewal advanced ValidUpto but not Subscription.EndUtc — fixed via Subscription.Extend (the dashboard term drift).
  • IdentityDbInitializer ignored the admin CreateAsync result — a silent failure marked a tenant "provisioned" with no usable admin login.
  • Invoice.Void is now idempotent; invoice pageSize clamped ≤100; AdjustTenantValidity blocks the root tenant.

API contract

  • Enums now serialize as string names globally (JsonStringEnumConverter via ConfigureHttpJsonOptions). [Flags] AuditTag/BodyCapture stay numeric via a NumericEnumConverter. Both frontends mirror the contract as string unions (AuditTag stays numeric/bitwise).

Frontend

  • Admin: permission-gate every plan/invoice/tenant action affordance (previously shown to View-only users and 403-ing); fix invoice-detail stuck "Loading…" on error.
  • Dashboard: "Valid for"/validity reads grace/expired from tenant status; persistent Expired banner; overview no longer masks API errors as a no-plan first-run; real invoice pagination; removed dead Suspended/Cancelled rendering; subscription term tracks start→end (not the calendar month).

Tests

  • Integration: 699 passed, 0 failed, 2 skipped (pre-existing).
  • Unit: Architecture 49 · Billing 91 · Multitenancy 87 · Identity 310.
  • Playwright: admin specs green · dashboard 136 green.
  • New: BillingTenantIsolationTests (5 cross-tenant mutation/usage/generate cases + usage-skip), RolePrivilegeEscalationTests, RenewTenant_Should_Advance_SubscriptionEndUtc, AdjustValidity_Should_Return400_When_TargetIsRootTenant, Void_Should_Be_Idempotent.

Deferred (documented — features/edge cases, not bugs)

ConnectionString in the tenant-list DTO (operator-only; contract change), WebSocket teardown on expiry (protected realtime infra, edge case), manual-issue notification (tenant-context nuance). Out of scope: payment gateway, proration, dunning, refunds, tenant suspend/delete, self-serve upgrade.

🤖 Generated with Claude Code

iammukeshm and others added 2 commits May 30, 2026 10:16
… + string-enum API contract

Deep audit of the billing/subscription/tenant system — backend.

Security (P0, cross-tenant):
- Invoice read/PDF/mutation (issue/pay/void), AssignSubscription, Get/CaptureUsage
  now root-gate via MultitenancyConstants.Root.Id (operator acts cross-tenant;
  tenants pinned to self). GetInvoices list scopes non-root callers to their own tenant.
- GenerateInvoices is now root-only (ForbiddenException for non-root).
- RoleService.FilterRootPermissions stripped only a "Permissions.Root." prefix that
  matched NO real root permission, letting a non-root admin grant root permissions to
  a role (privilege escalation). Now filtered by the IsRoot permission catalog.

Correctness:
- Usage/overage invoice was skipped when a subscription invoice shared the month
  (GenerateInvoiceForPeriodAsync idempotency lacked Purpose==Usage).
- Same-plan renewal now advances Subscription.EndUtc (Subscription.Extend) to track
  the renewed ValidUpto, fixing the dashboard term drift.
- IdentityDbInitializer now checks the admin CreateAsync result (silent failure
  previously marked a tenant provisioned with no usable admin login).
- Invoice.Void idempotent; invoice pageSize clamped <=100; AdjustTenantValidity
  blocks the root tenant.

API contract:
- Enums serialize as string names globally (JsonStringEnumConverter via
  ConfigureHttpJsonOptions); [Flags] AuditTag/BodyCapture stay numeric via NumericEnumConverter.

Every bug has a failing-first test (BillingTenantIsolationTests, RolePrivilegeEscalationTests,
RenewTenantTests, AdjustTenantValidityTests, InvoiceTests). Integration suite: 699 passed, 0 failed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tions, expiry UX

Frontend half of the billing/tenant audit.

Enum contract: mirror the API's string enums as string unions across both apps
(audits, chat, files, billing); AuditTag stays numeric (bitwise flags). Route-mock
fixtures updated to emit string enum values.

Admin: permission-gate every plan/invoice/tenant action affordance (previously shown
to View-only users and 403-ing on submit); fix invoice-detail stuck "Loading..." on
an error state.

Dashboard: "Valid for"/validity now reads grace/expired from tenant status (was a
healthy day-count off subscription.endUtc); persistent Expired banner; overview no
longer masks an API error as a no-plan first-run; real server-side invoice pagination;
removed dead Suspended/Cancelled rendering; subscription term now tracks start->end,
not the calendar month.

Playwright updated/added: admin specs green; dashboard 136 green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@iammukeshm iammukeshm merged commit b2d6fff into main May 30, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant