Auth/PM-38811 - KM - Update RotateUserAccountKeysCommand to use MasterPasswordService#7804
Draft
JaredSnider-Bitwarden wants to merge 3 commits into
Draft
Conversation
…rite UpdateUserKeyAndEncryptedDataV2Async enumerated a fixed column list and omitted LastPasswordChangeDate, silently dropping the field on PostgreSQL/MySQL/SQLite even when callers set it. The MSSQL sproc User_Update already persists this column, so this aligns EF with the existing Dapper behavior.
Wires PasswordChangeAndRotateUserAccountKeysAsync to IMasterPasswordService.PrepareUpdateExistingMasterPasswordAsync (Prepare* tier from PM-35392), replacing the inline master password mutation block. RefreshStamp is false so the existing BaseRotateUserAccountKeysAsync SecurityStamp + V2UpgradeToken logic remains the sole owner of session-invalidation behavior. The hint is sourced from the request because a password change can update it. Closes the parity gap where LastPasswordChangeDate was not set on this path even though the master password is changing. Other rotation variants (master-password-only, TDE, Key Connector) are untouched. Unit tests cover delegation, OneOf failure mapping, and short-circuit on old-password mismatch.
Extends the existing happy-path integration test to verify the master-key-wrapped user key, master password hint, master password hash (rewritten and verifies against the new authentication hash), and LastPasswordChangeDate are persisted as expected after a password-change-and-rotate call.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7804 +/- ##
==========================================
+ Coverage 61.15% 65.54% +4.39%
==========================================
Files 2175 2175
Lines 96784 96794 +10
Branches 8730 8731 +1
==========================================
+ Hits 59187 63447 +4260
+ Misses 35487 31146 -4341
- Partials 2110 2201 +91 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-38811
Awaiting merge of #7768 before continuing to testing.
📔 Objective
To finish consolidating KM usages of the
MasterPasswordServicewhich centralizes MP change logic in one location as part of the ongoing separation of MP salt & email work.📸 Screenshots
TODO