Skip to content

[PM-26383] Remove feature flag from server-side for autoconfirm#7402

Merged
JaredScar merged 11 commits intomainfrom
ac/pm-19934-implement-feature-flag-for-auto-confirm-users
Apr 9, 2026
Merged

[PM-26383] Remove feature flag from server-side for autoconfirm#7402
JaredScar merged 11 commits intomainfrom
ac/pm-19934-implement-feature-flag-for-auto-confirm-users

Conversation

@JaredScar
Copy link
Copy Markdown
Contributor

🎟️ Tracking

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

📔 Objective

Removes the feature flag logic from client side for autoconfirm flag

…ure flag checks. Updated related commands and tests to utilize policy requirements instead of feature service checks for automatic user confirmation. Cleaned up organization form view by simplifying checkbox rendering for automatic user confirmation.
@JaredScar JaredScar requested review from a team as code owners April 6, 2026 19:46
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 6, 2026

Logo
Checkmarx One – Scan Summary & Detailsd00d624c-668d-4157-af07-9f805ef4c84c


New Issues (1) Checkmarx found the following issues in this Pull Request
# Severity Issue Source File / Package Checkmarx Insight
1 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

@JaredScar JaredScar added the ai-review Request a Claude code review label Apr 6, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 6, 2026

Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR removes the AutomaticConfirmUsers feature flag from the server codebase, making the auto-confirm user policy enforcement permanent. The changes are consistent and mechanical across 8+ source files and their corresponding tests: removing IFeatureService dependencies, unwrapping conditional blocks, deleting "flag disabled" test scenarios, and adding mock setups for the now-always-active policy checks. The feature flag constant in Constants.cs is intentionally retained for older client compatibility.

Code Review Details
  • ♻️ DEBT: Three test method names still reference the removed feature flag and should be renamed for clarity
    • AcceptOrgUserCommandTests.cs:815 -- AcceptOrgUser_WithAutoConfirmFeatureFlagEnabled_SendsPushNotification
    • EmergencyAccessServiceTests.cs:124 -- InviteAsync_FeatureFlagEnabled_GrantorNotInAutoConfirmOrg_Succeeds
    • EmergencyAccessServiceTests.cs:438 -- AcceptUserAsync_FeatureFlagEnabled_GranteeNotInAutoConfirmOrg_Succeeds

Note: The EmergencyAccessServiceTests.cs names were previously flagged by JaredSnider-Bitwarden in a review thread.

JaredScar and others added 8 commits April 7, 2026 11:50
…omatic user confirmation. Updated logic to streamline policy requirement checks and cleaned up related tests by removing unnecessary feature flag assertions.
…dated multiple test classes to include checks for `AutomaticUserConfirmationPolicyRequirement`, ensuring no auto-confirm restrictions are applied by default. Refactored related assertions in `AcceptOrgUserCommandTests`, `ConfirmOrganizationUserCommandTests`, `RestoreOrganizationUserCommandTests`, and others to streamline compliance validation logic.
…test classes. Added checks for `AutomaticUserConfirmationPolicyRequirement` in `ConfirmOrganizationUserCommandTests`, `RestoreOrganizationUserCommandTests`, and `SelfHostedOrganizationSignUpCommandTests`, ensuring compliance validation logic is streamlined and consistent. Updated assertions to reflect new policy requirements.
…rs' of https://github.com/bitwarden/server into ac/pm-19934-implement-feature-flag-for-auto-confirm-users
…iderServiceTests to enhance test coverage for user confirmation policies.
…ationPolicyRequirement, enhancing test coverage for user acceptance scenarios.
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 8, 2026

Codecov Report

❌ Patch coverage is 89.87342% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.45%. Comparing base (aa1aa58) to head (4484e6a).
⚠️ Report is 25 commits behind head on main.

Files with missing lines Patch % Lines
...es/Organizations/CloudOrganizationSignUpCommand.cs 33.33% 3 Missing and 1 partial ⚠️
...ganizations/SelfHostedOrganizationSignUpCommand.cs 20.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7402      +/-   ##
==========================================
- Coverage   58.48%   58.45%   -0.03%     
==========================================
  Files        2063     2067       +4     
  Lines       91187    90997     -190     
  Branches     8129     8083      -46     
==========================================
- Hits        53332    53195     -137     
+ Misses      35957    35901      -56     
- Partials     1898     1901       +3     

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

… by removing feature flag references, improving readability and maintainability of the test suite.
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 9, 2026

@JaredScar JaredScar merged commit b3c8950 into main Apr 9, 2026
47 checks passed
@JaredScar JaredScar deleted the ac/pm-19934-implement-feature-flag-for-auto-confirm-users branch April 9, 2026 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants