Skip to content

[PM-31902] Remove m2 flagged logic#7351

Merged
cturnbull-bitwarden merged 9 commits intomainfrom
billing/PM-31902/remove-m2-flagged-logic-server
Apr 1, 2026
Merged

[PM-31902] Remove m2 flagged logic#7351
cturnbull-bitwarden merged 9 commits intomainfrom
billing/PM-31902/remove-m2-flagged-logic-server

Conversation

@cturnbull-bitwarden
Copy link
Copy Markdown
Contributor

@cturnbull-bitwarden cturnbull-bitwarden commented Mar 30, 2026

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-31902
https://bitwarden.atlassian.net/browse/PM-31891

📔 Objective

Remove pm-23341-milestone-2 feature flag logic from the server. The flag is fully rolled out so we keep the "on" path everywhere:

  • Always call AlignPremiumUsersSubscriptionConcernsAsync in UpcomingInvoiceHandler
  • Remove includeMilestone2Discount parameter from SubscriptionResponseModel constructors; rename helper to ShouldIncludeDiscount
  • Remove IFeatureService from AccountsController constructor
  • Update tests: remove flag mocking, delete flag-disabled test cases

…rUserAsync

Always call AlignPremiumUsersSubscriptionConcernsAsync instead of gating
it behind the feature flag.
…tionResponseModel

Remove includeMilestone2Discount parameter from both SubscriptionResponseModel
constructors. Rename ShouldIncludeMilestone2Discount to ShouldIncludeDiscount
and always evaluate the discount (coupon ID match + active check).
Remove feature flag mock setup lines and delete test methods that
verified flag-disabled behavior. Rename tests to remove flag references.
Remove includeMilestone2Discount parameter from all constructor calls.
Delete tests that verified flag-disabled behavior.
Remove all feature flag mock setup lines for PM23341_Milestone_2.
The constructor parameter became unused after removing the flag check.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 30, 2026

Logo
Checkmarx One – Scan Summary & Details2a82d306-8b14-41fa-9448-aafde557e9f1


New Issues (121) 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: 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
14 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
15 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
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/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
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/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
22 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
23 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
24 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
25 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
26 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
27 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
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: 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
30 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
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/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
33 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
34 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
35 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
36 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
37 MEDIUM CSRF /src/Api/Vault/Controllers/CiphersController.cs: 1281
detailsMethod at line 1281 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from organizationId. This parameter ...
Attack Vector
38 MEDIUM CSRF /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 394
detailsMethod at line 394 of /src/Api/AdminConsole/Controllers/OrganizationUsersController.cs gets a parameter from a user request from model. This par...
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

@cturnbull-bitwarden cturnbull-bitwarden changed the title Remove pm-23341-milestone-2 flagged logic from server [PM-31902] Remove m2 flagged logic Mar 30, 2026
After removing the m2 flag guard, AlignPremiumUsersSubscriptionConcernsAsync
is always called and needs ListPremiumPlans to return a non-null value.
Add a default empty list in the test constructor setup.
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.22%. Comparing base (4934c41) to head (d0b70bd).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7351   +/-   ##
=======================================
  Coverage   58.22%   58.22%           
=======================================
  Files        2060     2060           
  Lines       90951    90945    -6     
  Branches     8081     8080    -1     
=======================================
- Hits        52954    52953    -1     
+ Misses      36124    36120    -4     
+ Partials     1873     1872    -1     

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

@cturnbull-bitwarden cturnbull-bitwarden added ai-review-vnext Request a Claude code review using the vNext workflow ai-review Request a Claude code review and removed ai-review-vnext Request a Claude code review using the vNext workflow labels Mar 31, 2026
@cturnbull-bitwarden cturnbull-bitwarden marked this pull request as ready for review March 31, 2026 15:54
@cturnbull-bitwarden cturnbull-bitwarden requested a review from a team as a code owner March 31, 2026 15:54
Resolve conflicts with PM-33891 (Migrate Cancel and Reinstate Paths).
Keep IFeatureService in AccountsController constructor since the new
reinstate method uses it for PM32645 flag check.
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 31, 2026

Claude finished @cturnbull-bitwarden's task in 1m 42s —— View job


Code Review: PM-31902 Remove m2 flagged logic

  • Gather context and review PR diff
  • Review source code changes
  • Review test changes
  • Post review summary

Review Summary

This is a clean, mechanical removal of the fully-rolled-out pm-23341-milestone-2 feature flag. No issues found.

Files reviewed (6):

File Change
src/Api/Billing/Controllers/AccountsController.cs Removed PM23341_Milestone_2 flag check. IFeatureService correctly remains in the constructor since it's still used for PM32645_DeferPriceMigrationToRenewal.
src/Api/Models/Response/SubscriptionResponseModel.cs Removed includeMilestone2Discount parameter from both constructors; renamed ShouldIncludeMilestone2Discount to ShouldIncludeDiscount; discount logic now always evaluated.
src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs Removed feature flag conditional; AlignPremiumUsersSubscriptionConcernsAsync is now always called.
test/Api.Test/Billing/Controllers/AccountsControllerTests.cs Removed flag mocking, deleted flag-disabled and flag-toggle test cases.
test/Api.Test/Models/Response/SubscriptionResponseModelTests.cs Removed includeMilestone2Discount parameter from test calls, deleted flag-disabled test cases.
test/Billing.Test/Services/UpcomingInvoiceHandlerTests.cs Removed flag mocking; added default empty list return for ListPremiumPlans() to prevent null references now that alignment is always invoked.

Result: No issues found. All changes consistently remove the flag-gated code paths and retain the "on" behavior, with tests properly updated to match.

@cturnbull-bitwarden cturnbull-bitwarden removed the ai-review Request a Claude code review label Mar 31, 2026
@cturnbull-bitwarden cturnbull-bitwarden enabled auto-merge (squash) April 1, 2026 17:49
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 1, 2026

@cturnbull-bitwarden cturnbull-bitwarden merged commit aced376 into main Apr 1, 2026
38 checks passed
@cturnbull-bitwarden cturnbull-bitwarden deleted the billing/PM-31902/remove-m2-flagged-logic-server branch April 1, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants