[PM-35117] fix: Getting updated values from KDF before displaying update KDF prompt by aj-rosado · Pull Request #6802 · bitwarden/android

aj-rosado · 2026-04-15T18:06:25Z

🎟️ Tracking

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

📔 Objective

Avoid a loop state where users that updated the KDF information on a different client are stuck with after unlock.
Adding kdf info on state and calling sync to get the most recent data before displaying the prompt

…howing kdf update dialog

codecov · 2026-04-15T18:10:16Z

Codecov Report

❌ Patch coverage is 85.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.65%. Comparing base (7d2bfe1) to head (dd3e84a).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...ta/auth/repository/util/UserStateJsonExtensions.kt	55.55%	0 Missing and 4 partials ⚠️
...bitwarden/ui/vault/feature/vault/VaultViewModel.kt	93.54%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6802      +/-   ##
==========================================
+ Coverage   85.41%   85.65%   +0.23%     
==========================================
  Files         871      871              
  Lines       60064    60072       +8     
  Branches     8613     8621       +8     
==========================================
+ Hits        51304    51453     +149     
+ Misses       5775     5629     -146     
- Partials     2985     2990       +5

Flag	Coverage Δ
app-data	`17.21% <12.50%> (+0.01%)`	⬆️
app-ui-auth-tools	`20.62% <0.00%> (-0.01%)`	⬇️
app-ui-platform	`15.63% <0.00%> (+0.35%)`	⬆️
app-ui-vault	`26.56% <72.50%> (+0.01%)`	⬆️
authenticator	`6.55% <0.00%> (-0.01%)`	⬇️
lib-core-network-bridge	`4.28% <0.00%> (-0.01%)`	⬇️
lib-data-ui	`1.03% <0.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

github-actions · 2026-04-15T18:12:49Z

Checkmarx One – Scan Summary & Details – 265c4c2f-3e5e-45fb-84b8-c3fc197e510d

Great job! No new security vulnerabilities introduced in this pull request

david-livefront · 2026-04-15T18:24:02Z

+     * Tracks whether a forced sync was initiated to resolve a KDF update check. While true,
+     * the KDF update dialog is suppressed until the sync completes.
+     */
+    private var isAwaitingKdfSync = false


Can we put this in the state

david-livefront · 2026-04-15T18:26:23Z

+        if (authRepository.needsKdfUpdateToMinimums()) {
+            isAwaitingKdfSync = true
+            viewModelScope.launch {
+                try {


Why is there a ``tryhere,syncForResult` does not throw anything

Claude review tricked me here, it flagged this as necessary in case the syncForResult throws.
Although it should never throw.

david-livefront · 2026-04-15T18:26:59Z

        }
    }

+    @Suppress("MaxLineLength")


Can we move this suppression closer to the offending location.

Ping on this

david-livefront · 2026-04-15T18:28:20Z

+        verify(exactly = 0) { vaultRepository.syncIfNecessary() }
+    }
+
+    @Suppress("MaxLineLength")


This suppression is not needed

david-livefront · 2026-04-15T18:28:38Z

+            ),
+        )
+        val viewModel = createViewModel()
+        assertNull(viewModel.stateFlow.value.dialog)


Can you verify the entire state instead of just the dialog.

david-livefront · 2026-04-15T18:28:59Z

+        assertNull(viewModel.stateFlow.value.dialog)
+    }
+
+    @Suppress("MaxLineLength")


This suppression is not needed

david-livefront · 2026-04-15T18:29:16Z

+        runTest {
+            every { authRepository.needsKdfUpdateToMinimums() } returns true andThen false
+            val viewModel = createViewModel()
+            assertNull(viewModel.stateFlow.value.dialog)


Same thing, you should be validating the entire state.

david-livefront · 2026-04-15T18:29:26Z

+                        .the_new_recommended_encryption_settings_will_improve_your_account_desc_long
+                        .asText(),
+                ),
+                viewModel.stateFlow.value.dialog,


… tests and removed unnecessary MaxLineLength suppresions

david-livefront · 2026-04-16T16:19:54Z

+            )
+        }
+
+    @Suppress("MaxLineLength")


Can we move this closer to the offending line of code

github-actions · 2026-04-16T16:50:28Z

Code Review: PM-35117 fix: Getting updated values from KDF before displaying update KDF prompt

PR: #6802 | Author: aj-rosado | Reviewed by: Claude

Summary

This PR fixes a loop state where users who updated their KDF settings on another client would get stuck in a KDF update prompt after unlock. The fix has two parts:

Sync KDF from server: toUpdatedUserStateJson now extracts KDF settings (kdfType, kdfIterations, kdfMemory, kdfParallelism) from the sync response's userDecryption.masterPasswordUnlock.kdf, falling back to existing profile values when not present.
Force sync before showing dialog: VaultViewModel now forces a sync when needsKdfUpdateToMinimums() is true, using an isAwaitingKdfSync flag to suppress the KDF dialog until the sync completes. After sync, it re-checks whether the update is still needed before showing the dialog.

Findings

No issues found. The implementation is clean and well-tested:

The isAwaitingKdfSync guard correctly prevents the race condition where getDialogVaultLoaded could show the dialog before the sync response updates local KDF state.
The sync-then-recheck pattern in handleKdfSyncCompletedReceive properly handles both outcomes (KDF already updated on server vs. still needs updating).
Test coverage includes the key scenarios: forced sync trigger, dialog suppression during sync, dialog shown/hidden based on post-sync state.
Test fixtures (createMockUserDecryption, createMockMasterPasswordUnlock) are well-structured for reuse.
The default change in createMockSyncResponse (userDecryption = null -> createMockUserDecryption(number)) is reflected in updated test assertions.

Verdict: Approved ✅

aj-rosado added 2 commits April 15, 2026 18:47

Getting values for kdf iterations from sync and calling sync before s…

0ccb434

…howing kdf update dialog

removing unnecessary set on state copy

b93839a

aj-rosado requested review from a team and david-livefront as code owners April 15, 2026 18:06

github-actions Bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context labels Apr 15, 2026

david-livefront reviewed Apr 15, 2026

View reviewed changes

aj-rosado changed the title ~~Getting updated values from KDF before displaying update KDF prompt~~ [PM-35117] fix: Getting updated values from KDF before displaying update KDF prompt Apr 16, 2026

Added property to state, removed unnecessary try, validating state on…

a973137

… tests and removed unnecessary MaxLineLength suppresions

github-actions Bot added the t:bug Change Type - Bug label Apr 16, 2026

aj-rosado added the ai-review-vnext Request a Claude code review using the vNext workflow label Apr 16, 2026

aj-rosado requested a review from david-livefront April 16, 2026 11:24

david-livefront reviewed Apr 16, 2026

View reviewed changes

Taking suppress closer to offending line

dd3e84a

david-livefront approved these changes Apr 16, 2026

View reviewed changes

aj-rosado enabled auto-merge April 16, 2026 16:52

aj-rosado added this pull request to the merge queue Apr 16, 2026

Merged via the queue into main with commit 0143f93 Apr 16, 2026
26 checks passed

aj-rosado deleted the PM-35117/update-kdf-alert-loop branch April 16, 2026 17:16

Conversation

aj-rosado commented Apr 15, 2026

🎟️ Tracking

📔 Objective

Uh oh!

codecov Bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Great job! No new security vulnerabilities introduced in this pull request

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 16, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review: PM-35117 fix: Getting updated values from KDF before displaying update KDF prompt

Summary

Findings

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Apr 15, 2026 •

edited

Loading

github-actions Bot commented Apr 15, 2026 •

edited

Loading

github-actions Bot commented Apr 16, 2026 •

edited by atlassian Bot

Loading