Auth/PM-34130 - Fix DeviceAuthDetails constructor and stored procedure for EDD compliance

[JaredSnider-Bitwarden]

🎟️ Tracking

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

📔 Objective

1. Fixed EDD compliance in Device_ReadActiveWithPendingAuthRequestsByUserId — replaces SELECT D.* and a positional 14-arg Dapper constructor with an explicit column list and name-based property mapping. Note: per discussion with DbOps, the use of explicit column mapping is preferred in this scenario over the style guide's current select D.* recommendation just to be safer regarding an alias / column name collision between the device and auth request tables.
2. Fixed the EF constructor in DeviceAuthDetails which was missing five field copies, causing IsTrusted to always return false for EF-sourced results.
3. Converted IsTrusted from a stored value to a computed property.
4. Added test coverage

Note: no API changes are present so there won't be any corresponding clients work.

Detailed Changes

Core Models

src/Core/Auth/Models/Data/DeviceAuthDetails.cs

1. Replaced positional 14-arg Dapper constructor with a parameterless constructor for name-based property mapping
2. Converted IsTrusted to a computed property (DeviceExtensions.IsTrusted(this)) so its value is always derived from the current state of the encrypted key fields rather than depending on correct assignment in every constructor code path
3. Completed the EF constructor to copy all Device fields (previously missing UserId, PushToken, RevisionDate, EncryptedPrivateKey, Active)
4. Renamed AuthRequestCreatedAt → AuthRequestCreationDate — required for Dapper name-based mapping; the property name must match the column alias returned by the stored procedure (AR.CreationDate AS AuthRequestCreationDate)

src/Core/Auth/Models/Api/Response/DeviceAuthRequestResponseModel.cs

1. Updated reference from AuthRequestCreatedAt to AuthRequestCreationDate

Database

src/Sql/dbo/Auth/Stored Procedures/Device_ReadActiveWithPendingAuthRequestsByUserId.sql

1. Replaced SELECT D.* with an explicit column list to enable name-based Dapper mapping via property setters, eliminating the positional dependency that caused silent mismatches on schema changes
2. Renamed LEFT JOIN → LEFT OUTER JOIN — required by the Bitwarden SQL style guide (full join type names must be used)
3. Added bracket quoting to all column and object references — required by the Bitwarden SQL style guide

util/Migrator/DbScripts/2026-04-07_00_Alter_Device_ReadActiveWithPendingAuthRequestsByUserId.sql

1. SQL Server migration script deploying the stored procedure change (idempotent via CREATE OR ALTER)

Tests

test/Infrastructure.IntegrationTest/Auth/Repositories/DeviceRepositoryTests.cs

Expanded integration test coverage for GetManyByUserIdWithDeviceAuth:

1. Full device field mapping
2. Most-recent pending auth request selection
3. Cross-user isolation
4. IsTrusted computed correctly from encrypted keys
5. Ineligible auth request types (AdminApproval, already approved, expired)
6. Unlock type eligibility
7. Inactive device exclusion
8. Empty device list

📸 Screenshots

PM-34130.-.Devices.screen.works.mov

[github-actions]

Checkmarx One – Scan Summary & Details – ffa464e9-7ad9-43c4-9315-7af5b1e7e60e

---

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 (1)

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

Severity	Issue	Source File / Package
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 287

[codecov]

Codecov Report

❌ Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 62.99%. Comparing base (e4bf05c) to head (f99a14b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...els/Api/Response/DeviceAuthRequestResponseModel.cs	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7416      +/-   ##
==========================================
+ Coverage   58.49%   62.99%   +4.50%     
==========================================
  Files        2066     2066              
  Lines       91140    91104      -36     
  Branches     8111     8109       -2     
==========================================
+ Hits        53308    57388    +4080     
+ Misses      35924    31722    -4202     
- Partials     1908     1994      +86     

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

Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR fixes EDD compliance issues in the DeviceAuthDetails model and its associated stored procedure. The EF constructor was missing five field copies (UserId, PushToken, RevisionDate, EncryptedPrivateKey, Active), causing IsTrusted to always return false for EF-sourced results. The positional 14-argument Dapper constructor is replaced with a parameterless constructor and name-based property mapping backed by an explicit SQL column list, eliminating silent mismatches on schema changes. Converting IsTrusted to a computed property removes the entire class of bugs where constructors forget to derive it correctly. Integration test coverage is thorough, validating field mapping, cross-user isolation, computed trust, ineligible auth request filtering, unlock type eligibility, inactive device exclusion, and empty results.

Code Review Details

No findings.

[mkincaid-bw]

LGTM

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud