[PM-36574] fix: scope provider client invoice report to the authorized provider#7770
Conversation
…d provider The provider client invoice CSV endpoint authorized the route provider but generated the report from the caller-supplied invoiceId without verifying the invoice belonged to that provider. A provider admin could therefore read another provider's client billing data by passing a foreign invoiceId through their own provider's route (VULN-565, IDOR / broken object-level authorization). Thread the authorized provider id from the controller through GenerateClientInvoiceReport into a new provider-scoped lookup (GetByProviderIdAndInvoiceId), implemented in both Dapper (stored procedure) and EF Core, and return 404 when the invoice is not owned by the authorized provider. The unscoped GetByInvoiceId is retained only for the internal Stripe webhook path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the IDOR fix (VULN-565 / PM-36574) scoping the provider client invoice CSV endpoint to the authorized provider. The fix correctly threads the authorized The change is well-tested: the new integration test genuinely reproduces the cross-provider attack, and the 404-on-not-owned behavior avoids leaking an existence oracle. No findings. Code Review DetailsNo issues found. Validation notes (not findings):
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7770 +/- ##
==========================================
+ Coverage 61.20% 65.69% +4.49%
==========================================
Files 2209 2209
Lines 97732 97750 +18
Branches 8813 8816 +3
==========================================
+ Hits 59812 64216 +4404
+ Misses 35796 31316 -4480
- Partials 2124 2218 +94 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
sbrown-livefront
left a comment
sbrown-livefront
left a comment
There was a problem hiding this comment.
✅ Great work on this with great test coverage.
| public async Task<ICollection<ProviderInvoiceItem>> GetByProviderId(Guid providerId) | ||
| { | ||
| var sqlConnection = new SqlConnection(ConnectionString); | ||
| await using var sqlConnection = new SqlConnection(ConnectionString); |
There was a problem hiding this comment.
📝 Good catch with these.
| @@ -0,0 +1,18 @@ | |||
| -- Scope the provider client invoice report to the authorized provider (VULN-565 / PM-36574). | |||
| -- Adds a provider-scoped lookup so a provider can only read invoice items it owns. | |||
| CREATE OR ALTER PROCEDURE [dbo].[ProviderInvoiceItem_ReadByProviderIdAndInvoiceId] | |||
There was a problem hiding this comment.
The name of the stored procedure should be ProviderInvoiceItem_ReadByProviderIdInvoiceId
See docs.
There was a problem hiding this comment.
Pushed a commit just now addressing this
…eId per SQL naming convention Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
3 participants
🎟️ Tracking
📔 Objective
Fixes a broken object-level authorization (IDOR) flaw in the provider client invoice CSV endpoint
GET /providers/{providerId}/billing/invoices/{invoiceId}.The endpoint authorized the caller against the route
providerId, butProviderBillingService.GenerateClientInvoiceReportthen loadedProviderInvoiceItemrows by the caller-suppliedinvoiceIdalone, with no provider-ownership check. A provider admin for provider A who knew provider B's Stripe invoice ID could therefore retrieve provider B's client invoice CSV — client organization name/ID, assigned/used/remaining seats, plan, and estimated total — by requesting B'sinvoiceIdthrough A's route.Fix: thread the authorized
provider.Idfrom the controller intoGenerateClientInvoiceReport(Guid providerId, string invoiceId), which now reads through a new provider-scoped repository methodGetByProviderIdAndInvoiceId. Scoping is enforced at the data-access layer in both ORMs:ProviderInvoiceItem_ReadByProviderIdAndInvoiceId(WHERE [ProviderId] = @ProviderId AND [InvoiceId] = @InvoiceId), with SSDT source + migration script.where ProviderId == providerId && InvoiceId == invoiceId.When the authorized provider doesn't own the requested invoice, the scoped lookup returns nothing and the endpoint now responds 404 Not Found (previously 500) — no cross-provider data is returned and no existence oracle is leaked. The pre-existing unscoped
GetByInvoiceIdis retained only for the internal Stripe-webhook path (ProviderEventService), which is server-to-server and not attacker-controlled.Testing: adds an API integration test (
ProviderBillingControllerAuthorizationTests) that reproduces the cross-provider attack — failing against the old code (victim CSV returned), passing after the fix (404, no victim data) — plus a control test confirming the attacker cannot use the victim provider's own route. Service/controller unit tests updated for the new signature and 404 behavior.