Skip to content

[PM-29152] Rename VNextSavePolicyCommand to SavePolicyCommand and remove deprecated policy interfaces#7364

Open
r-tome wants to merge 14 commits intomainfrom
ac/pm-29152/rename-vnextsavepolicycommand-to-savepolicycommand
Open

[PM-29152] Rename VNextSavePolicyCommand to SavePolicyCommand and remove deprecated policy interfaces#7364
r-tome wants to merge 14 commits intomainfrom
ac/pm-29152/rename-vnextsavepolicycommand-to-savepolicycommand

Conversation

@r-tome
Copy link
Copy Markdown
Contributor

@r-tome r-tome commented Mar 31, 2026

🎟️ Tracking

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

📔 Objective

Remove the old SavePolicyCommand, ISavePolicyCommand, IPolicyValidator, and IPostSavePolicySideEffect types, then promote the VNext equivalents to their final names. This completes the migration to the event-based policy validation pattern.

r-tome added 8 commits March 31, 2026 16:07
…ated implementations. Update PolicyServiceCollectionExtensions to eliminate deprecated methods. Adjust policy validator classes to remove IPolicyValidator dependency and streamline validation methods.
…cyModel in validation and side effect methods, improving code clarity and reducing method complexity.
…DomainCommandTests to better reflect their functionality, enhancing clarity and consistency across the test suite.
@r-tome r-tome changed the title Ac/pm 29152/rename vnextsavepolicycommand to savepolicycommand [PM-29152] Rename VNextSavePolicyCommand to SavePolicyCommand and remove deprecated policy interfaces Mar 31, 2026
@r-tome r-tome marked this pull request as ready for review March 31, 2026 16:03
@r-tome r-tome requested review from a team as code owners March 31, 2026 16:03
@r-tome r-tome requested review from enmande and jrmccannon March 31, 2026 16:03
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 31, 2026

Logo
Checkmarx One – Scan Summary & Details0fabc280-32d1-4606-b8a1-1723da96fd1e


New Issues (122) Checkmarx found the following issues in this Pull Request
# Severity Issue Source File / Package Checkmarx Insight
1 CRITICAL Stored_XSS /src/SharedWeb/Health/HealthCheckServiceExtensions.cs: 61
detailsThe method embeds untrusted data in generated output with WriteAsync, at line 60 of /src/SharedWeb/Health/HealthCheckServiceExtensions.cs. This ...
Attack Vector
2 CRITICAL Stored_XSS /util/Server/Startup.cs: 57
detailsThe method embeds untrusted data in generated output with WriteAsync, at line 59 of /util/Server/Startup.cs. This untrusted data is embedded int...
Attack Vector
3 MEDIUM CSRF /src/Api/Billing/Controllers/VNext/AccountBillingVNextController.cs: 55
detailsMethod at line 55 of /src/Api/Billing/Controllers/VNext/AccountBillingVNextController.cs gets a parameter from a user request from user. This pa...
Attack Vector
4 MEDIUM CSRF /src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs: 145
detailsMethod at line 145 of /src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs gets a parameter from a user request from request. T...
Attack Vector
5 MEDIUM CSRF /src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs: 145
detailsMethod at line 145 of /src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs gets a parameter from a user request from request. T...
Attack Vector
6 MEDIUM CSRF /src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs: 97
detailsMethod at line 97 of /src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs gets a parameter from a user request from model. This...
Attack Vector
7 MEDIUM CSRF /src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs: 97
detailsMethod at line 97 of /src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs gets a parameter from a user request from model. This...
Attack Vector
8 MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 534
detailsMethod at line 534 of /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs gets a parameter from a user request from model. This par...
Attack Vector
9 MEDIUM CSRF /src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs: 229
detailsMethod at line 229 of /src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs gets a parameter from a user request from model. Thi...
Attack Vector
10 MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1558
detailsMethod 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
11 MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1558
detailsMethod 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
12 MEDIUM CSRF /src/Api/Tools/Controllers/SendsController.cs: 73
detailsMethod at line 73 of /src/Api/Tools/Controllers/SendsController.cs gets a parameter from a user request from id. This parameter value flows thro...
Attack Vector
13 MEDIUM CSRF /src/Api/Billing/Controllers/VNext/AccountBillingVNextController.cs: 146
detailsMethod at line 146 of /src/Api/Billing/Controllers/VNext/AccountBillingVNextController.cs gets a parameter from a user request from request. Thi...
Attack Vector
14 MEDIUM CSRF /src/Api/Billing/Controllers/VNext/AccountBillingVNextController.cs: 145
detailsMethod at line 145 of /src/Api/Billing/Controllers/VNext/AccountBillingVNextController.cs gets a parameter from a user request from user. This p...
Attack Vector
15 MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 217
detailsMethod at line 217 of /src/Api/Auth/Controllers/AccountsController.cs gets a parameter from a user request from model. This parameter value flow...
Attack Vector
16 MEDIUM CSRF /src/Api/Public/Controllers/CollectionsController.cs: 91
detailsMethod at line 91 of /src/Api/Public/Controllers/CollectionsController.cs gets a parameter from a user request from model. This parameter value ...
Attack Vector
17 MEDIUM CSRF /src/Api/Public/Controllers/CollectionsController.cs: 91
detailsMethod at line 91 of /src/Api/Public/Controllers/CollectionsController.cs gets a parameter from a user request from model. This parameter value ...
Attack Vector
18 MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 176
detailsMethod at line 176 of /src/Api/Controllers/CollectionsController.cs gets a parameter from a user request from model. This parameter value flows ...
Attack Vector
19 MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 176
detailsMethod at line 176 of /src/Api/Controllers/CollectionsController.cs gets a parameter from a user request from model. This parameter value flows ...
Attack Vector
20 MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 176
detailsMethod at line 176 of /src/Api/Controllers/CollectionsController.cs gets a parameter from a user request from model. This parameter value flows ...
Attack Vector
21 MEDIUM CSRF /src/Api/Controllers/CollectionsController.cs: 176
detailsMethod at line 176 of /src/Api/Controllers/CollectionsController.cs gets a parameter from a user request from model. This parameter value flows ...
Attack Vector
22 MEDIUM CSRF /src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs: 173
detailsMethod at line 173 of /src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs gets a parameter from a user request from model. Thi...
Attack Vector
23 MEDIUM CSRF /src/Api/Auth/Controllers/AccountsController.cs: 452
detailsMethod 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
24 MEDIUM CSRF /src/Api/Dirt/Controllers/OrganizationReportsController.cs: 189
detailsMethod at line 189 of /src/Api/Dirt/Controllers/OrganizationReportsController.cs gets a parameter from a user request from request. This paramet...
Attack Vector
25 MEDIUM CSRF /src/Api/Billing/Controllers/VNext/AccountBillingVNextController.cs: 104
detailsMethod at line 104 of /src/Api/Billing/Controllers/VNext/AccountBillingVNextController.cs gets a parameter from a user request from user. This p...
Attack Vector
26 MEDIUM CSRF /src/Api/Billing/Controllers/VNext/OrganizationBillingVNextController.cs: 107
detailsMethod at line 107 of /src/Api/Billing/Controllers/VNext/OrganizationBillingVNextController.cs gets a parameter from a user request from organiza...
Attack Vector
27 MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1417
detailsMethod at line 1417 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ...
Attack Vector
28 MEDIUM CSRF /src/Api/Dirt/Controllers/OrganizationReportsController.cs: 286
detailsMethod at line 286 of /src/Api/Dirt/Controllers/OrganizationReportsController.cs gets a parameter from a user request from request. This paramet...
Attack Vector
29 MEDIUM CSRF /src/Api/Dirt/Controllers/OrganizationReportsController.cs: 233
detailsMethod at line 233 of /src/Api/Dirt/Controllers/OrganizationReportsController.cs gets a parameter from a user request from request. This paramet...
Attack Vector
30 MEDIUM CSRF /src/Api/Dirt/Controllers/OrganizationReportsController.cs: 189
detailsMethod at line 189 of /src/Api/Dirt/Controllers/OrganizationReportsController.cs gets a parameter from a user request from request. This paramet...
Attack Vector
31 MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1417
detailsMethod at line 1417 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ...
Attack Vector
32 MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1417
detailsMethod at line 1417 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ...
Attack Vector
33 MEDIUM CSRF /src/Api/AdminConsole/Controllers/GroupsController.cs: 289
detailsMethod at line 289 of /src/Api/AdminConsole/Controllers/GroupsController.cs gets a parameter from a user request from orgUserId. This parameter ...
Attack Vector
34 MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1385
detailsMethod 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
35 MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1446
detailsMethod at line 1446 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ...
Attack Vector
36 MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1148
detailsMethod at line 1148 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ...
Attack Vector
37 MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1032
detailsMethod at line 1032 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ...
Attack Vector

More results are available on the CxOne platform


Fixed Issues (1) Great job! The following issues were fixed in this Pull Request
Severity Issue Source File / Package
MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 287

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 31, 2026

Codecov Report

❌ Patch coverage is 96.89441% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.28%. Comparing base (0918bfd) to head (7d6818a).

Files with missing lines Patch % Lines
...PolicyEventHandlers/SingleOrgPolicyEventHandler.cs 73.33% 2 Missing and 2 partials ⚠️
...ures/Policies/Implementations/SavePolicyCommand.cs 99.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7364      +/-   ##
==========================================
- Coverage   62.43%   58.28%   -4.16%     
==========================================
  Files        2060     2059       -1     
  Lines       90974    90812     -162     
  Branches     8087     8073      -14     
==========================================
- Hits        56803    52933    -3870     
- Misses      32216    36007    +3791     
+ Partials     1955     1872      -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.

@r-tome r-tome added the ai-review-vnext Request a Claude code review using the vNext workflow label Apr 1, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 1, 2026

Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR completes the migration from the old policy validator pattern to the event-based policy validation pattern. It removes VNextSavePolicyCommand, IVNextSavePolicyCommand, IPolicyValidator, and IPostSavePolicySideEffect, promoting the VNext implementation as the canonical SavePolicyCommand. The PolicyValidators directory is renamed to PolicyEventHandlers, and all bridge/duplicate methods in handlers are collapsed into their event-based equivalents. All callers (controllers, services, domain commands) and tests are updated consistently with no behavioral changes.

Code Review Details

No findings. The refactoring is mechanical and preserves existing behavior. The new SavePolicyCommand is a direct adoption of the deleted VNextSavePolicyCommand with identical logic, and all interface consumers have been correctly migrated.

r-tome added 4 commits April 1, 2026 11:17
…ed methods and simplifying the implementation. Update corresponding tests to reflect these changes.
…iles and update using statements for consistency.
@r-tome r-tome marked this pull request as draft April 1, 2026 12:41
@r-tome r-tome marked this pull request as ready for review April 1, 2026 13:21
enmande
enmande previously approved these changes Apr 1, 2026
Copy link
Copy Markdown
Contributor

@enmande enmande left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auth changes LGTM. Thank you!

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 2, 2026

@r-tome r-tome requested a review from enmande April 2, 2026 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review-vnext Request a Claude code review using the vNext workflow needs-qa

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants