[PM-32598] - Remove Unused sso/details Endpoint + Sprocs

[sven-bitwarden]

🎟️ Tracking

PM-32598

📔 Objective

This endpoint was responsible for some recent vulnerability investigation, because it could be inadvertently used to grab SSO details for a given identifier even when unverified. We don't have a need to display unverified SSO information this way, so we're preemptively removing the landmine. This endpoint has since been superseded by sso/verified, which of course only returns verified domains.

Sister frontend pr: bitwarden/clients#20012

Warning

This drops the stored procedure as well. Because it's unused, I don't believe there's any special considerations needs followed for EDD.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.23%. Comparing base (d9aef5c) to head (a71e47c).
⚠️ Report is 36 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7400      +/-   ##
==========================================
+ Coverage   58.75%   63.23%   +4.48%     
==========================================
  Files        2071     2069       -2     
  Lines       91252    91188      -64     
  Branches     8130     8127       -3     
==========================================
+ Hits        53611    57660    +4049     
+ Misses      35726    31530    -4196     
- Partials     1915     1998      +83     

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

[github-actions]

Checkmarx One – Scan Summary & Details – 9b7ce6dd-0c98-4f69-b21d-ca1f22622294

---

New Issues (4)

Checkmarx found the following issues in this Pull Request

#	Severity	Issue	Source File / Package	Checkmarx Insight
1		Path_Traversal	/src/Api/Controllers/SelfHosted/SelfHostedOrganizationLicensesController.cs: 56	details Method at line 56 of /src/Api/Controllers/SelfHosted/SelfHostedOrganizationLicensesController.cs gets dynamic data from the model element. This ... Attack Vector
2		CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1558	details Method at line 1558 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ... Attack Vector
3		CSRF	/src/Api/Auth/Controllers/AccountsController.cs: 452	details Method at line 452 of /src/Api/Auth/Controllers/AccountsController.cs gets a parameter from a user request from model. This parameter value flow... Attack Vector
4		CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1385	details Method at line 1385 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ... Attack Vector

---

Fixed Issues (4)

Great job! The following issues were fixed in this Pull Request

Severity	Issue	Source File / Package
	CSRF	/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs: 145
	CSRF	/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs: 97
	CSRF	/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs: 173
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 287

[eliykat]

Just a thought: it's possible this was used by mobile clients, please do a quick check to make sure they're not still using it.

[sven-bitwarden]

@eliykat I did a full code search on the org - mobile "had" usage in archived repos. Only usage outside of clients/server is our rust auto-api-generation. Documented a bit more here - https://bitwarden.atlassian.net/browse/PM-32598?focusedCommentId=145750

[eliykat]

My question has been resolved, I'll leave the rest of the review to @JaredScar

[github-actions]

Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR removes the unused sso/details endpoint along with its supporting code: the OrganizationDomainSsoDetailsResponseModel, OrganizationDomainSsoDetailsData model, repository implementations (Dapper and EF), the OrganizationDomainSsoDetails_ReadByEmail stored procedure definition, and a migration script to drop the sproc from the database. The removal is thorough with no orphaned references remaining in the codebase. The test updates correctly fix a pre-existing test that was calling the now-removed method instead of the intended GetVerifiedOrgDomainSsoDetailsAsync.

Code Review Details

No findings. The removal is complete and clean across all layers (API controller, response/data models, repository interface, Dapper and EF implementations, stored procedure, migration, and tests).

[JaredScar]

@claude please review

[claude]

Claude finished @JaredScar's task in 1m 31s —— View job

---

Code Review: PM-32598 Remove Unused sso/details Endpoint + Sprocs

• Gather context and read changed files
• Review code changes for correctness and completeness
• Check for leftover references to removed code
• Post review findings

---

Overall: This PR looks good. Clean removal of an unused and potentially risky endpoint. No issues found.

What the PR does

Removes the [AllowAnonymous] POST domain/sso/details endpoint, its response model, data model, repository methods (Dapper + EF), stored procedure, and associated tests. Adds a migration script to drop the sproc from the database. The replacement sso/verified endpoint (which only returns verified domains) remains intact.

Security

The removed endpoint was [AllowAnonymous] and could return SSO details for unverified domain claims — this is the right call to remove it preemptively.

Review findings

No issues found. Specifically verified:

• No stale references: Searched the codebase for OrganizationDomainSsoDetailsData, GetOrganizationDomainSsoDetailsAsync, OrganizationDomainSsoDetailsResponseModel, and GetOrgDomainSsoDetails — none remain outside of historical migration scripts (which is expected).
• Migration script is correct: Uses IF OBJECT_ID guard before DROP PROCEDURE, which is idempotent and safe.
• Test changes are correct: The two removed tests (GetOrgDomainSsoDetails_ShouldThrowNotFound_* and GetOrgDomainSsoDetails_ShouldReturnOrganizationDomainSsoDetails_*) tested the deleted endpoint. The third test was fixed — it previously called the deleted GetOrgDomainSsoDetails method but was meant to test GetVerifiedOrgDomainSsoDetailsAsync, and now correctly calls the verified endpoint and asserts on the right return type.
• All using imports in the test file are still needed after the removals.
• Interface, Dapper, and EF implementations are all consistently updated.
• EDD consideration: The PR description correctly notes that since the sproc is unused, no special EDD phasing is needed. The sso/verified sproc (VerfiedOrganaizationDomainSsoDetails_ReadByEmail) is untouched.

---

[JaredScar]

Looks good! Removing code is always fun lol

[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