[PM-23278] Upgrade user KDF settings to minimums #5955

andrebispo5 · 2025-09-29T09:17:36Z

Note

Depends on PR: #5944

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-23278
https://bitwarden.atlassian.net/browse/PM-23577

📔 Objective

This pull request introduces functionality to ensure user KDF (Key Derivation Function) settings meet minimum security requirements. It adds logic to check if an account's KDF configuration is outdated and, if so, updates it automatically after password unlock, provided the relevant feature flag is enabled. The changes also include necessary model, service, and dependency updates to support this workflow.

KDF Minimums Enforcement and Update Workflow

Added new methods to AuthRepository and AuthRepositoryImpl to check if a user's KDF settings are below the minimum and to update them if needed, including integration with a feature flag (ForceUpdateKdfSettings). [1] [2]
Automatically triggers KDF update after password unlock if the user's settings are outdated and the feature flag is enabled.

Model and SDK Support

Introduced UpdateKdfMinimumsResult sealed class to model the result of updating KDF settings.
Added makeUpdateKdf method to VaultSdkSource and its implementation to generate updated KDF data using the SDK. [1] [2]
Added extension function toKdfRequestModel to convert SDK KDF objects to network request models.

Dependency Injection and Feature Flag Integration

Updated DI module (AuthRepositoryModule) to provide FeatureFlagManager to AuthRepositoryImpl. [1] [2] [3]

📸 Screenshots

Screen.Recording.2025-10-02.at.09.24.01.mov

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

…imums

…tory.

# Conflicts: # app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt # app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/debugmenu/components/FeatureFlagListItems.kt # app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt # app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/handlers/VaultHandlers.kt # app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModelTest.kt # core/src/main/kotlin/com/bitwarden/core/data/manager/model/FlagKey.kt # network/src/main/kotlin/com/bitwarden/network/service/AccountsService.kt # network/src/main/kotlin/com/bitwarden/network/service/AccountsServiceImpl.kt # network/src/test/kotlin/com/bitwarden/network/service/AccountsServiceTest.kt # ui/src/main/res/values/strings.xml

…re update.

…responses

…esponses, and save to profile.

…minimum # Conflicts: # app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/util/UserStateJsonExtensions.kt # core/src/main/kotlin/com/bitwarden/core/data/manager/model/FlagKey.kt

…minimum

github-actions · 2025-09-29T09:53:46Z

Checkmarx One – Scan Summary & Details – 521a5494-cb5a-41c4-a00a-8337bd69207c

New Issues (2)

Checkmarx found the following issues in this Pull Request

Severity	Issue	Source File / Package	Checkmarx Insight
	Use_of_Hardcoded_Password	/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModelTest.kt: 3050	details The application uses the hard-coded password "mock_password" for authentication purposes, either using it to verify users' identities, or to acce... `ID: n7npCwWTHuohDSgD5OTttlWiV6Q%3D` Attack Vector
	Use_of_Hardcoded_Password	/app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreenTest.kt: 816	details The application uses the hard-coded password "test_password" for authentication purposes, either using it to verify users' identities, or to acce... `ID: hF9bsk8oPhG6%2F0nqhQxopbQP1D4%3D` Attack Vector

app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultScreenTest.kt

app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModelTest.kt

SaintPatrck · 2025-10-08T14:13:37Z

app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt

+                .updateKdfToMinimumsIfNeeded(password = password)
+                .also { result ->
+                    if (result is UpdateKdfMinimumsResult.Error) {
+                        Timber.e("Failed to silent update KDF settings: ${result.error}")


Can we just do Timber.e(result.error, "Failed to silently update KDF settings.")?

SaintPatrck · 2025-10-08T14:17:05Z

app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultLockManagerImpl.kt

+                )
+                .also { result ->
+                    if (result is UpdateKdfMinimumsResult.Error) {
+                        Timber.e("Failed to silent update KDF settings: ${result.error}")


Can we do

Timber.e(result.error, "Failed to silently update KDF settings.")

SaintPatrck · 2025-10-08T14:19:37Z

...in/kotlin/com/x8bit/bitwarden/ui/platform/components/dialog/BitwardenMasterPasswordDialog.kt

 * @param onDismissRequest called when the user attempts to dismiss the dialog (for example by
 * tapping outside of it).
 */
+@Suppress("LongMethod")


Can you double check if this is needed? I removed it and detekt didn't complain. 🤔

SaintPatrck · 2025-10-08T14:24:39Z

app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt

+                        .asText(),
+                    error = result.error,
+                )
+                Timber.e(message = "Failed to update kdf to minimums: ${result.error}")


Another place we can pass the error as an arg instead.

Timber.e(result.error, "Failed to update kdf to minimums.")

🤔 Is this actually needed since we're logging errors in the data layer?

SaintPatrck · 2025-10-08T14:32:38Z

ui/src/main/res/values/strings.xml

+    <string name="kdf_update_failed_active_account_not_found">"Kdf update failed, active account not found. Please try again or contact us."</string>
+    <string name="an_error_occurred_while_trying_to_update_your_kdf_settings">"An error occurred while trying to update your kdf settings. Please try again or contact us."</string>


Should these be surrounded by quotes? If so, we should be using the “ and ” chars for opening and closing quotes.

Outer quotes like this do not actually appear in the displayed string, I actually just removed a bunch of these. I think they should be removed

nice catch!

SaintPatrck

Left a few minor comments. Overall, it's looking good. Great job!

SaintPatrck · 2025-10-08T17:14:03Z

app/src/main/kotlin/com/x8bit/bitwarden/data/auth/manager/KdfManagerImpl.kt

+                kdf = Kdf.Pbkdf2(iterations = DEFAULT_PBKDF2_ITERATIONS.toUInt()),
+            )
+            .fold(
+                onSuccess = { updateKdfOnServer(createUpdateKdfRequest(it)) },


SaintPatrck · 2025-10-08T17:28:46Z

app/src/main/kotlin/com/x8bit/bitwarden/data/auth/manager/KdfManagerImpl.kt

+    private suspend fun updateKdfOnServer(request: UpdateKdfJsonRequest): UpdateKdfMinimumsResult {
+        return accountsService
+            .updateKdf(body = request)
+            .fold(
+                onSuccess = {
+                    authDiskSource.userState = authDiskSource.userState
+                        ?.toUserStateJsonKdfUpdatedMinimums()
+                    UpdateKdfMinimumsResult.Success
+                },
+                onFailure = { UpdateKdfMinimumsResult.Error(error = it) },
+            )
+    }


I like that request creation is extracted, but I think this introduces an unnecessary fold operation. vaultSdkSource.makeUpdateKdf().flatMap will handle mapping a successful result to the result of accountsService.updateKdf(), then you only have to handle failures once, in the final fold().

return vaultSdkSource .makeUpdateKdf() .flatMap { response -> accountsService.updateKdf( request = createUpdateKdfRequest(response) ) } .fold( onSuccess = { authDiskSource.userState = authDiskSource.userState ?.toUserStateJsonKdfUpdatedMinimums() UpdateKdfMinimumsResult.Success }, onFailure = { UpdateKdsMinimumsResult.Error(error = it) }, )

SaintPatrck

Awesome! Thanks for that. LGTM

# Conflicts: # app/src/main/kotlin/com/x8bit/bitwarden/data/auth/manager/di/AuthManagerModule.kt # app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModel.kt # app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/vault/VaultViewModelTest.kt

andrebispo5 added 24 commits September 9, 2025 09:58

[PM-23278] Add SDK makeUpdateKdf call

5ff44d6

[PM-23278] Add updateKdf service call and test

a0bbc1a

[PM-23278] Add func to update to minimum kdf settings to AuthRepository

0ff6bb4

[PM-23278] Remove unused imports

b200af3

[PM-23278] Add customisation to BitwardenMasterPasswordDialog

6300d5d

[PM-23278] Add dialog to input password to update KDF settings to min…

98d0361

…imums

Merge branch 'main' into pm-23278/kdf-upgrade-minimum

d07e8f9

[PM-23278] Check and show if user needs KDF update on vault load.

7c5a6ac

[PM-23278] Try to update KDF to minimums after password unlock.

c4078cd

[PM-23278] Update tests.

03a09ef

[PM-23278] Add pm-18021-force-update-kdf-settings feature flag.

d0e7297

[PM-23278] Add feature flag to needsKdfUpdateToMinimums in AuthReposi…

6e832ae

…tory.

[PM-23278] Small fixes

ca97ab0

[PM-23278] Add KDF update to minimums tests to VaultViewModelTest

cc77cc9

[PM-23278] Check if user has decryption options and has password befo…

d069048

…re update.

[PM-23278] Add MasterPasswordUnlock data to sync and identity server …

e40b01f

…responses

[PM-23280] Add MasterPasswordUnlockData to sync and identity server r…

871e2a8

…esponses, and save to profile.

[PM-23280] Fix broken test.

f586b28

Merge branch 'pm-23280/kdf_changes_logout' into pm-23278/kdf-upgrade-…

3d977c3

…minimum # Conflicts: # app/src/main/kotlin/com/x8bit/bitwarden/data/auth/repository/util/UserStateJsonExtensions.kt # core/src/main/kotlin/com/bitwarden/core/data/manager/model/FlagKey.kt

[PM-23280] Fix test.

bc094a1

Merge branch 'main' into pm-23280/kdf_changes_logout

91296b7

Merge branch 'pm-23280/kdf_changes_logout' into pm-23278/kdf-upgrade-…

d153314

…minimum

[PM-23278] Fix tests.

091b35f

github-advanced-security bot found potential problems Sep 29, 2025

View reviewed changes

andrebispo5 added 3 commits September 30, 2025 09:25

[PM-23278] Update account profile KDF settings after server call.

38e9c83

[PM-23278] Remove unnecessary AccountNotFound error.

b4943e1

[PM-23278] Fix MasterPasswordUnlockDataJson serial name.

96d4747

andrebispo5 changed the base branch from main to pm-23280/kdf_changes_logout October 1, 2025 09:27

SaintPatrck reviewed Oct 8, 2025

View reviewed changes

SaintPatrck requested changes Oct 8, 2025

View reviewed changes

[PM-23278] Add PR suggestions

9a8c3c2

andrebispo5 dismissed david-livefront’s stale review via 9a8c3c2 October 8, 2025 17:04

SaintPatrck reviewed Oct 8, 2025

View reviewed changes

andrebispo5 requested a review from SaintPatrck October 8, 2025 17:20

andrebispo5 enabled auto-merge October 8, 2025 17:26

SaintPatrck reviewed Oct 8, 2025

View reviewed changes

[PM-23278] Refactor

1c8e3c3

SaintPatrck previously approved these changes Oct 8, 2025

View reviewed changes

david-livefront previously approved these changes Oct 8, 2025

View reviewed changes

andrebispo5 dismissed stale reviews from david-livefront and SaintPatrck via 14c4700 October 8, 2025 19:08

david-livefront and others added 2 commits October 8, 2025 16:25

Fix failing test

68f80fe

Merge branch 'main' into pm-23278/kdf-upgrade-minimum

5d3f729

david-livefront previously approved these changes Oct 8, 2025

View reviewed changes

andrebispo5 dismissed david-livefront’s stale review via 5d3f729 October 8, 2025 22:04

andrebispo5 requested review from SaintPatrck and david-livefront October 9, 2025 06:05

Merge branch 'main' into pm-23278/kdf-upgrade-minimum

9bdecf0

vvolkgang approved these changes Oct 9, 2025

View reviewed changes

andrebispo5 added this pull request to the merge queue Oct 9, 2025

Merged via the queue into main with commit d98ff64 Oct 9, 2025
9 checks passed

andrebispo5 deleted the pm-23278/kdf-upgrade-minimum branch October 9, 2025 08:21

		<string name="kdf_update_failed_active_account_not_found">"Kdf update failed, active account not found. Please try again or contact us."</string>
		<string name="an_error_occurred_while_trying_to_update_your_kdf_settings">"An error occurred while trying to update your kdf settings. Please try again or contact us."</string>

[PM-23278] Upgrade user KDF settings to minimums #5955

[PM-23278] Upgrade user KDF settings to minimums #5955

Uh oh!

Conversation

andrebispo5 commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎟️ Tracking

📔 Objective

📸 Screenshots

⏰ Reminders before review

🦮 Reviewer guidelines

Uh oh!

github-actions bot commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

david-livefront Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SaintPatrck left a comment

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!

SaintPatrck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

andrebispo5 commented Sep 29, 2025 •

edited

Loading

github-actions bot commented Sep 29, 2025 •

edited

Loading

david-livefront Oct 8, 2025 •

edited

Loading