Skip to content

[PM-38827] Remove errant org membership check#7929

Open
BTreston wants to merge 1 commit into
mainfrom
ac/pm-38827-accepted-member-with-weak-MP-not-updating
Open

[PM-38827] Remove errant org membership check#7929
BTreston wants to merge 1 commit into
mainfrom
ac/pm-38827-accepted-member-with-weak-MP-not-updating

Conversation

@BTreston

@BTreston BTreston commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

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

📔 Objective

Removes errant organization membership check since it is handled by the policy requirement now.

📸 Screenshots

@BTreston BTreston requested a review from a team as a code owner July 6, 2026 16:33
@BTreston BTreston requested a review from enmande July 6, 2026 16:33
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR removes the organization-membership short-circuit in GetMasterPasswordPolicyAsync so the master password policy is always resolved through PolicyRequirementQuery. This correctly ensures accepted-but-unconfirmed members are subject to MP policy enforcement, since the requirement factory already accounts for member state. The change mirrors the established pattern in AccountsController.cs, where GetAsyncVNext returns a non-null requirement and MasterPasswordPolicyResponseModel safely handles a null EnforcedOptions. Test coverage is added for both the no-policy and policy-applied paths.

Code Review Details
  • ♻️ : _organizationUserRepository is now assigned but never read after removing the membership check, making the field, constructor parameter, and assignment dead. Fully removing the injected IOrganizationUserRepository would also require updating the three subclass constructors that forward it to base(...). Not blocking.
    • src/Identity/IdentityServer/RequestValidators/BaseRequestValidator.cs:38

@BTreston BTreston requested review from a team and jrmccannon July 6, 2026 16:34
@BTreston BTreston added ai-review Request a Claude code review t:bugfix Change Type - Bugfix labels Jul 6, 2026
@BTreston BTreston changed the title [PM-38827] Remove errant org membership call [PM-38827] Remove errant org membership check Jul 6, 2026
@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.33%. Comparing base (b9876f7) to head (7fc31ac).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7929      +/-   ##
==========================================
- Coverage   61.34%   61.33%   -0.02%     
==========================================
  Files        2236     2242       +6     
  Lines       98517    98625     +108     
  Branches     8907     8910       +3     
==========================================
+ Hits        60437    60493      +56     
- Misses      35943    35997      +54     
+ Partials     2137     2135       -2     

☔ View full report in Codecov by Harness.
📢 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.

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 t:bugfix Change Type - Bugfix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant