[PM-39894] fix: Exclude provider-linked organizations from add-existing-organization flow

[cyprain-okeke]

🎟️ Tracking

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

📔 Objective

The Provider Portal's "Add existing organization" flow (POST /providers/{id}/clients/existing) never checked whether the target organization already had a ProviderOrganization row. A reseller-linked org keeps Status = Created and its own gateway fields, so it passed every filter in the addable query and could be absorbed into an MSP's consolidated billing — leaving the org linked to two providers (confirmed in production; see ticket).

This adds the missing guard at two layers:

• Eligibility query — Organization_ReadAddableToProviderByUserId (Dapper stored procedure + EF twin) now excludes any organization with an existing ProviderOrganization row via NOT EXISTS. Linked orgs no longer appear in the picker, and the endpoint's eligibility re-check returns 404 for them.
• Service guard (defense in depth) — ProviderBillingService.AddExistingOrganization now guards first with the same check as the legacy ProviderService.AddOrganization path (GetByOrganizationId) and throws ConflictException (409 "Organization already belongs to a provider.") before any Stripe or repository side effects. This covers the check-then-act race and direct service callers. Note: this intentionally returns 409 rather than the legacy path's 400 — the caller has no control over whether a ProviderOrganization row exists.

Includes a dated idempotent migration (CREATE OR ALTER), unit tests asserting the guard fires before any billing side effect, and integration tests for both ORM tracks covering standalone (included), reseller-linked, MSP-linked, and BusinessUnit-branch (excluded) cases.

Verified end-to-end locally against a running Api + Identity with the ticket's replication fixture (reseller-linked org with a real Stripe test-mode trialing subscription, billable MSP, owner-who-is-provider-admin):

Sproc version	GET /clients/addable	POST /clients/existing
old (pre-fix)	200 — linked org listed (bug reproduced)	409 — new guard blocked it, zero side effects
new (post-migration)	200 — []	404 — filtered before the service

Org row, both providers, the single ProviderOrganization link, and the Stripe subscription were verified untouched after every rejection.

QA note: GetAddableOrganizations also filters on live Stripe subscription status (Active/Trialing/PastDue). The target org must have a real test-mode subscription, or it vanishes from the addable list regardless of this fix and can masquerade as a pass.

Out of scope / follow-ups: cleanup of existing dual-linked production rows and a unique index on ProviderOrganization.OrganizationId (requires the cleanup first) — tracked with dbops via the ticket.

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the fix excluding provider-linked organizations from the "Add existing organization" flow. The change guards at two layers: the eligibility query (Dapper stored procedure plus its EF Core twin) now excludes organizations with an existing ProviderOrganization row via NOT EXISTS, and ProviderBillingService.AddExistingOrganization adds a defense-in-depth ConflictException guard before any Stripe or repository side effects. Verified dual-ORM parity, correct T-SQL operator precedence (the appended NOT EXISTS is joined at top level with AND outside the parenthesized plan-type OR group), an idempotent CREATE OR ALTER dated migration, and comprehensive unit and integration test coverage for standalone, reseller-linked, MSP-linked, and BusinessUnit cases.

No findings meeting the confidence threshold.

Code Review Details

No blocking or actionable findings.

Notes verified during review:

• SQL precedence is correct — the new NOT EXISTS predicate applies to all rows regardless of @ProviderType, confirmed by the LinkedOrg_ExcludedForBusinessUnit integration test.
• EF Core !dbContext.ProviderOrganizations.Any(...) mirrors the stored procedure exactly, preserving dual-ORM parity.
• The intentional 409 (ConflictException) versus the legacy path's 400 (BadRequestException) is documented in the PR description with sound rationale.
• Unit test asserts the guard short-circuits before Release, Stripe calls, and repository writes.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.64%. Comparing base (c2d97d5) to head (a0d3261).

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #7988       +/-   ##
===========================================
+ Coverage   14.86%   66.64%   +51.78%     
===========================================
  Files        1386     2284      +898     
  Lines       60245    99672    +39427     
  Branches     4799     9017     +4218     
===========================================
+ Hits         8957    66431    +57474     
+ Misses      51137    30970    -20167     
- Partials      151     2271     +2120     

☔ View full report in Codecov by Harness.
📢 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.

[mkincaid-bw]

DB changes look good. The formatting of the code doesn't match our style guide but since you are only adding a small piece, I don't expect you to change it (unless you want to 😄).