[PM-32666] Fixes endpoint issue where you can update another by providing a valid org ID

[JaredScar]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-32666

📔 Objective

Fixes vulnerability following:

• The endpoint PUT /providers/{providerId}/clients/{providerOrganizationId} authorizes the caller
  only for the route providerId, but does not verify that providerOrganizationId belongs to that
  same provider.

[claude]

Claude finished @JaredScar's task in 2m 12s —— View job

---

Pull Request Re-Review: PM-32666 IDOR Fix in ProviderClientsController

Overview

This PR adds a security fix for an IDOR (Insecure Direct Object Reference) vulnerability in PUT /providers/{providerId}/clients/{providerOrganizationId}. The endpoint previously authorized the caller for providerId but did not verify that providerOrganizationId belongs to that same provider, allowing cross-provider modification of client organizations.

Previous Findings -- All Resolved

The previous review identified one critical finding: the new guard clause broke two existing tests and lacked its own dedicated test. The latest commit (52e5172) addresses all three items:

1. Existing test fix (UpdateAsync_ServiceUserMakingPurchase_Unauthorized): Added providerOrganization.ProviderId = provider.Id; so the test correctly passes through the new guard clause and continues to exercise the unauthorized-purchase path.
2. Existing test fix (UpdateAsync_Ok): Same fix applied, ensuring the happy path test reaches its intended assertions.
3. New IDOR test (UpdateAsync_ProviderOrganizationBelongsToDifferentProvider_NotFound): Dedicated test that sets providerOrganization.ProviderId = Guid.NewGuid() (mismatched provider) and asserts NotFound is returned, providing regression coverage for this security check.

Assessment

• Security approach: Correct. The guard clause at ProviderClientsController.cs L110-113 validates ownership before any business logic executes. Returning NotFound avoids information leakage.
• Code change: Minimal (+5 lines in controller) and well-placed in the existing validation chain.
• Test coverage: Complete. All existing tests pass through the new guard correctly, and the new security validation has dedicated coverage.

No remaining issues. This PR is ready to merge.

[github-actions]

Checkmarx One – Scan Summary & Details – 0e3159c9-189e-4347-962a-771e14686754

Great job! No new security vulnerabilities introduced in this pull request

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.08%. Comparing base (4732d7f) to head (52e5172).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7185   +/-   ##
=======================================
  Coverage   57.08%   57.08%           
=======================================
  Files        2028     2028           
  Lines       88794    88797    +3     
  Branches     7914     7915    +1     
=======================================
+ Hits        50684    50687    +3     
  Misses      36279    36279           
  Partials     1831     1831           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.