Open
Conversation
Contributor
|
New Issues (3)Checkmarx found the following issues in this Pull Request
Fixed Issues (3)Great job! The following issues were fixed in this Pull Request
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7201 +/- ##
==========================================
+ Coverage 57.53% 57.55% +0.01%
==========================================
Files 2032 2032
Lines 89486 89504 +18
Branches 7957 7954 -3
==========================================
+ Hits 51487 51510 +23
+ Misses 36154 36149 -5
Partials 1845 1845 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Thomas-Avery
commented
Mar 12, 2026
|
|
||
| [HttpPost("key-management/rotate-user-account-keys")] | ||
| public async Task RotateUserAccountKeysAsync([FromBody] RotateUserAccountKeysAndDataRequestModel model) | ||
| public async Task PasswordChangeAndRotateUserAccountKeysAsync([FromBody] RotateUserAccountKeysAndDataRequestModel model) |
Contributor
Author
There was a problem hiding this comment.
KM team, I've realized changing this method name will effect the SDK name of the generated API bindings. Do we think that is worth it or should I just revert it back to the original name?
Contributor
There was a problem hiding this comment.
I'm OK with it, it just means we need to fix the breaking API bindings PR. I.e we run the API bindings automation on the SDK repo and do a rename on that branch, and review. Should be fairly low effort?
|
Contributor
|
quexten
reviewed
Mar 17, 2026
quexten
approved these changes
Mar 17, 2026
Contributor
quexten
left a comment
quexten
left a comment
There was a problem hiding this comment.
Looks good from my side; Only one question
ike-kottlowski
approved these changes
Mar 17, 2026
src/Api/KeyManagement/Models/Requests/MasterPasswordUnlockDataAndAuthenticationModel.cs
Show resolved
Hide resolved
ike-kottlowski
approved these changes
Mar 17, 2026
enmande
approved these changes
Mar 18, 2026
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
5 participants
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-33162
📔 Objective
The objective of this PR is to refactor the current change password and user key rotation endpoint to use, via composition, a new base data model. This is preparation of adding a new endpoint to support none password change userKey rotation that will share the same base data model and processing logic.
Note to Auth reviewers:
The only auth code changes was moving
MasterPasswordUnlockAndAuthenticationDataModelto KM ownership.MasterPasswordUnlockAndAuthenticationDataModelthe request model is only used on the KM key rotation endpoint. Let me know if you don't agree and want to still retain ownership.