Skip to content

PM-39006: Bug: Update the setPassword flow for TDE users#7056

Merged
david-livefront merged 1 commit into
mainfrom
PM-39006-set-password-flow
Jun 16, 2026
Merged

PM-39006: Bug: Update the setPassword flow for TDE users#7056
david-livefront merged 1 commit into
mainfrom
PM-39006-set-password-flow

Conversation

@david-livefront

@david-livefront david-livefront commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

🎟️ Tracking

PM-39006

📔 Objective

This PR address some issues in the setPassword flow for TDE user.

Main changes:

  1. Wrap the entire setPassword function in a userStateTransaction to ensure no state-based navigation occurs when processing the change.
  2. Do not attempt to unlock the vault after setting the password, the vault is already unlocked and attempting to do so causes an error.
  3. Ensure the MasterPasswordUnlockData is set before trying to unlock the vault, that data is now required.
  4. Store the ForcePasswordResetReason on sync in order to ensure the correct flow is followed.

@david-livefront david-livefront requested a review from a team as a code owner June 12, 2026 21:49
@david-livefront david-livefront added the ai-review-vnext Request a Claude code review using the vNext workflow label Jun 12, 2026
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context t:bug Change Type - Bug labels Jun 12, 2026
@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the TDE setPassword flow changes in AuthRepositoryImpl and the sync-time profile updates in UserStateJsonExtensions. The change wraps setPassword in a userStateTransaction to suppress state-based navigation mid-flow, removes the redundant vault unlock for already-unlocked TDE users, makes MasterPasswordUnlockData required when persisting the password, and computes ForcePasswordResetReason on sync. Logic was traced across the TDE, admin-reset, weak-password, and no-permission paths; all behave correctly and are covered by new and updated tests.

Code Review Details

No blocking findings.

Notes considered and cleared during review:

  • State write in setUpdatedPassword.onSuccess runs after the server confirms the password is set and before enrollment — ordering is correct and consistent with the JIT V1 flow.
  • salt = profile.email matches the test fixtures (EMAIL / BASE_PROFILE_1.email).
  • getForcePasswordResetReason reads permissions.shouldManageResetPassword on a non-null Permissions, so no NPE risk; the ?: previousForcePasswordResetReason fallback preserves prior reasons correctly for master-password users.
  • The hasMasterPassword = false fallback when sync has no userDecryption is a deliberate behavioral change aligned with the PR intent and covered by an updated test.

@david-livefront david-livefront force-pushed the PM-39006-set-password-flow branch from d37fefc to 7ed2f16 Compare June 15, 2026 19:36

@aj-rosado aj-rosado left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@david-livefront

Copy link
Copy Markdown
Collaborator Author

Thanks @aj-rosado

@david-livefront david-livefront added this pull request to the merge queue Jun 16, 2026
Merged via the queue into main with commit fe8bf57 Jun 16, 2026
25 checks passed
@david-livefront david-livefront deleted the PM-39006-set-password-flow branch June 16, 2026 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review-vnext Request a Claude code review using the vNext workflow app:password-manager Bitwarden Password Manager app context t:bug Change Type - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants