[PM-38769] Fix OrganizationConnectionsController logic

[eliykat]

🎟️ Tracking

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

📔 Objective

Fixes for OrganizationConnectionsController logic - see Jira ticket for more details. I avoided any more comprehensive refactors because this code rarely changes, and aligning this with our current practices would require a rewrite of the controller and related commands. Better to make the surgical fix with an xmldoc signpost for the future.

That said, I did already add a bunch of tests before I decided on this route, so I've included them here.

I've also removed the #nullable disable directive and the obsolete PUT delete endpoint, because they were quick wins. This is only consumed by clients, and it uses the DELETE verb.

📸 Screenshots

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the authorization and correctness fixes in OrganizationConnectionsController. The core change correctly sources the connection Type and OrganizationId from the persisted entity rather than the request body when authorizing updates, closing a privilege-escalation gap where a user with one permission (e.g. Manage SCIM) could update a connection requiring another (e.g. Organization Owner). The new type-change guard, the license null check guarding against an NRE in VerifyLicense, the removal of the obsolete POST /delete endpoint, and the #nullable disable cleanup are all sound, and the change is backed by thorough unit and integration tests.

Code Review Details

No blocking findings.

Validated during review:

• The UpdateConnection permission check now uses existingOrganizationConnection.Type (persisted) instead of model.Type (request body), preventing cross-type privilege escalation. Verified the repository fetch is scoped by both id and org id, so the authorized org is also trustworthy.
• license == null || !VerifyLicense(license) correctly prevents an NRE — VerifyLicense dereferences license.Token before any null guard.
• Removal of PostDeleteConnection is safe — no remaining server references, and clients use the DELETE verb per the linked client code.
• The [JsonConstructor] parameterless constructor is benign and supports response deserialization in the new integration tests.

[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 61.25%. Comparing base (19cb812) to head (e7b4ac7).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7845      +/-   ##
==========================================
+ Coverage   61.22%   61.25%   +0.02%     
==========================================
  Files        2209     2209              
  Lines       97716    97744      +28     
  Branches     8815     8816       +1     
==========================================
+ Hits        59824    59870      +46     
+ Misses      35768    35751      -17     
+ Partials     2124     2123       -1     

☔ 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.