From 1fb0f81185260656a6822b2dee74069cb31c541b Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Sun, 21 Sep 2025 08:40:11 +0100 Subject: [PATCH 01/14] move change-kdf into KM ownership --- .../src/app/auth/settings/security/security-keys.component.ts | 2 +- .../change-kdf/change-kdf-confirmation.component.html | 0 .../change-kdf/change-kdf-confirmation.component.ts | 0 .../change-kdf/change-kdf.component.html | 0 .../change-kdf/change-kdf.component.ts | 0 .../security => key-management}/change-kdf/change-kdf.module.ts | 2 +- 6 files changed, 2 insertions(+), 2 deletions(-) rename apps/web/src/app/{auth/settings/security => key-management}/change-kdf/change-kdf-confirmation.component.html (100%) rename apps/web/src/app/{auth/settings/security => key-management}/change-kdf/change-kdf-confirmation.component.ts (100%) rename apps/web/src/app/{auth/settings/security => key-management}/change-kdf/change-kdf.component.html (100%) rename apps/web/src/app/{auth/settings/security => key-management}/change-kdf/change-kdf.component.ts (100%) rename apps/web/src/app/{auth/settings/security => key-management}/change-kdf/change-kdf.module.ts (90%) diff --git a/apps/web/src/app/auth/settings/security/security-keys.component.ts b/apps/web/src/app/auth/settings/security/security-keys.component.ts index c77109936eee..9d16d4380ebc 100644 --- a/apps/web/src/app/auth/settings/security/security-keys.component.ts +++ b/apps/web/src/app/auth/settings/security/security-keys.component.ts @@ -8,10 +8,10 @@ import { AccountService } from "@bitwarden/common/auth/abstractions/account.serv import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { DialogService } from "@bitwarden/components"; +import { ChangeKdfModule } from "../../../key-management/change-kdf/change-kdf.module"; import { SharedModule } from "../../../shared"; import { ApiKeyComponent } from "./api-key.component"; -import { ChangeKdfModule } from "./change-kdf/change-kdf.module"; @Component({ templateUrl: "security-keys.component.html", diff --git a/apps/web/src/app/auth/settings/security/change-kdf/change-kdf-confirmation.component.html b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.html similarity index 100% rename from apps/web/src/app/auth/settings/security/change-kdf/change-kdf-confirmation.component.html rename to apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.html diff --git a/apps/web/src/app/auth/settings/security/change-kdf/change-kdf-confirmation.component.ts b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts similarity index 100% rename from apps/web/src/app/auth/settings/security/change-kdf/change-kdf-confirmation.component.ts rename to apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts diff --git a/apps/web/src/app/auth/settings/security/change-kdf/change-kdf.component.html b/apps/web/src/app/key-management/change-kdf/change-kdf.component.html similarity index 100% rename from apps/web/src/app/auth/settings/security/change-kdf/change-kdf.component.html rename to apps/web/src/app/key-management/change-kdf/change-kdf.component.html diff --git a/apps/web/src/app/auth/settings/security/change-kdf/change-kdf.component.ts b/apps/web/src/app/key-management/change-kdf/change-kdf.component.ts similarity index 100% rename from apps/web/src/app/auth/settings/security/change-kdf/change-kdf.component.ts rename to apps/web/src/app/key-management/change-kdf/change-kdf.component.ts diff --git a/apps/web/src/app/auth/settings/security/change-kdf/change-kdf.module.ts b/apps/web/src/app/key-management/change-kdf/change-kdf.module.ts similarity index 90% rename from apps/web/src/app/auth/settings/security/change-kdf/change-kdf.module.ts rename to apps/web/src/app/key-management/change-kdf/change-kdf.module.ts index 3ef1fef03f83..342ad43e3687 100644 --- a/apps/web/src/app/auth/settings/security/change-kdf/change-kdf.module.ts +++ b/apps/web/src/app/key-management/change-kdf/change-kdf.module.ts @@ -1,7 +1,7 @@ import { CommonModule } from "@angular/common"; import { NgModule } from "@angular/core"; -import { SharedModule } from "../../../../shared"; +import { SharedModule } from "../../shared"; import { ChangeKdfConfirmationComponent } from "./change-kdf-confirmation.component"; import { ChangeKdfComponent } from "./change-kdf.component"; From b92cd271100cde27678dd0f670376df712205348 Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Sun, 21 Sep 2025 10:41:45 +0100 Subject: [PATCH 02/14] Change kdf component update for Forced KDF update --- .../change-kdf-confirmation.component.html | 12 +- .../change-kdf-confirmation.component.ts | 33 +++- .../change-kdf/change-kdf.component.html | 156 ++++++++++-------- .../change-kdf/change-kdf.component.ts | 12 +- .../change-kdf/change-kdf.module.ts | 4 +- apps/web/src/locales/en/messages.json | 140 ++++++++-------- 6 files changed, 202 insertions(+), 155 deletions(-) diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.html b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.html index 9f21b28f1906..d5d1127feaa0 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.html +++ b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.html @@ -1,11 +1,13 @@
- {{ "changeKdf" | i18n }} + {{ "updateYourEncryptionSettings" | i18n }} - {{ "kdfSettingsChangeLogoutWarning" | i18n }} + @if (!forceUpdateKDFSettingsFeatureFlag$) { + {{ "kdfSettingsChangeLogoutWarning" | i18n }} + } {{ "masterPass" | i18n }} @@ -18,12 +20,12 @@ > {{ "confirmIdentity" | i18n }} - + + {{ "encKeySettings" | i18n }} > - - {{ "kdfMemory" | i18n }} - -
-
- + @if (isPBKDF2(kdfConfig)) { + {{ "kdfIterations" | i18n }} - - - {{ "encKeySettings" | i18n }} /> {{ "kdfIterationRecommends" | i18n }} - - - - {{ "kdfIterations" | i18n }} - - - - - - {{ "kdfParallelism" | i18n }} - - - - -
+ } @else if (isArgon2(kdfConfig)) { + + {{ "kdfMemory" | i18n }} + + + }
+ @if (isArgon2(kdfConfig)) { +
+ + + {{ "kdfIterations" | i18n }} + + + +
+
+ + + {{ "kdfParallelism" | i18n }} + + + +
+ } + + +
    +
  • {{ "encryptionKeySettingsAlgorithmPopoverPBKDF2" | i18n }}
    • +
  • {{ "encryptionKeySettingsAlgorithmPopoverArgon2Id" | i18n }}
    • +
+
+ + {{ "learnMore" | i18n }} + + +
+ + diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf.component.ts b/apps/web/src/app/key-management/change-kdf/change-kdf.component.ts index a059ede77b4f..69201473418f 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf.component.ts +++ b/apps/web/src/app/key-management/change-kdf/change-kdf.component.ts @@ -2,10 +2,12 @@ // @ts-strict-ignore import { Component, OnDestroy, OnInit } from "@angular/core"; import { FormBuilder, FormControl, ValidatorFn, Validators } from "@angular/forms"; -import { Subject, firstValueFrom, takeUntil } from "rxjs"; +import { Subject, firstValueFrom, takeUntil, Observable } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { DialogService } from "@bitwarden/components"; import { KdfConfigService, @@ -31,7 +33,7 @@ export class ChangeKdfComponent implements OnInit, OnDestroy { protected formGroup = this.formBuilder.group({ kdf: new FormControl(KdfType.PBKDF2_SHA256, [Validators.required]), kdfConfig: this.formBuilder.group({ - iterations: [this.kdfConfig.iterations], + iterations: new FormControl(this.kdfConfig.iterations, [Validators.required]), memory: [null as number], parallelism: [null as number], }), @@ -43,16 +45,22 @@ export class ChangeKdfComponent implements OnInit, OnDestroy { protected ARGON2_MEMORY = Argon2KdfConfig.MEMORY; protected ARGON2_PARALLELISM = Argon2KdfConfig.PARALLELISM; + forceUpdateKDFSettingsFeatureFlag$: Observable; + constructor( private dialogService: DialogService, private kdfConfigService: KdfConfigService, private accountService: AccountService, private formBuilder: FormBuilder, + configService: ConfigService, ) { this.kdfOptions = [ { name: "PBKDF2 SHA-256", value: KdfType.PBKDF2_SHA256 }, { name: "Argon2id", value: KdfType.Argon2id }, ]; + this.forceUpdateKDFSettingsFeatureFlag$ = configService.getFeatureFlag$( + FeatureFlag.ForceUpdateKDFSettings, + ); } async ngOnInit() { diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf.module.ts b/apps/web/src/app/key-management/change-kdf/change-kdf.module.ts index 342ad43e3687..4c9cd00e79db 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf.module.ts +++ b/apps/web/src/app/key-management/change-kdf/change-kdf.module.ts @@ -1,13 +1,15 @@ import { CommonModule } from "@angular/common"; import { NgModule } from "@angular/core"; +import { PopoverModule } from "@bitwarden/components"; + import { SharedModule } from "../../shared"; import { ChangeKdfConfirmationComponent } from "./change-kdf-confirmation.component"; import { ChangeKdfComponent } from "./change-kdf.component"; @NgModule({ - imports: [CommonModule, SharedModule], + imports: [CommonModule, SharedModule, PopoverModule], declarations: [ChangeKdfComponent, ChangeKdfConfirmationComponent], exports: [ChangeKdfComponent, ChangeKdfConfirmationComponent], }) diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index ccb9e6b3520e..304e8a4f45d5 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -11,7 +11,7 @@ "criticalApplications": { "message": "Critical applications" }, - "noCriticalAppsAtRisk":{ + "noCriticalAppsAtRisk": { "message": "No critical applications at risk" }, "accessIntelligence": { @@ -134,7 +134,7 @@ "atRiskMembers": { "message": "At-risk members" }, - "membersAtRiskActivityDescription":{ + "membersAtRiskActivityDescription": { "message": "Members with edit access to at-risk items for critical applications" }, "membersAtRisk": { @@ -1558,7 +1558,6 @@ } } }, - "dontAskAgainOnThisDeviceFor30Days": { "message": "Don't ask again on this device for 30 days" }, @@ -1929,9 +1928,6 @@ "encKeySettings": { "message": "Encryption key settings" }, - "kdfAlgorithm": { - "message": "KDF algorithm" - }, "kdfIterations": { "message": "KDF iterations" }, @@ -1966,9 +1962,6 @@ "argon2Desc": { "message": "Higher KDF iterations, memory, and parallelism can help protect your master password from being brute forced by an attacker." }, - "changeKdf": { - "message": "Change KDF" - }, "encKeySettingsChanged": { "message": "Encryption key settings saved" }, @@ -1985,22 +1978,22 @@ "message": "Proceeding will also log you out of your current session, requiring you to log back in. You will also be prompted for two-step login again, if set up. Active sessions on other devices may continue to remain active for up to one hour." }, "newDeviceLoginProtection": { - "message":"New device login" + "message": "New device login" }, "turnOffNewDeviceLoginProtection": { - "message":"Turn off new device login protection" + "message": "Turn off new device login protection" }, "turnOnNewDeviceLoginProtection": { - "message":"Turn on new device login protection" + "message": "Turn on new device login protection" }, "turnOffNewDeviceLoginProtectionModalDesc": { - "message":"Proceed below to turn off the verification emails bitwarden sends when you login from a new device." + "message": "Proceed below to turn off the verification emails bitwarden sends when you login from a new device." }, "turnOnNewDeviceLoginProtectionModalDesc": { - "message":"Proceed below to have bitwarden send you verification emails when you login from a new device." + "message": "Proceed below to have bitwarden send you verification emails when you login from a new device." }, "turnOffNewDeviceLoginProtectionWarning": { - "message":"With new device login protection turned off, anyone with your master password can access your account from any device. To protect your account without verification emails, set up two-step login." + "message": "With new device login protection turned off, anyone with your master password can access your account from any device. To protect your account without verification emails, set up two-step login." }, "accountNewDeviceLoginProtectionSaved": { "message": "New device login protection changes saved" @@ -2136,7 +2129,7 @@ "selectImportCollection": { "message": "Select a collection" }, - "importTargetHintCollection": { + "importTargetHintCollection": { "message": "Select this option if you want the imported file contents moved to a collection" }, "importTargetHintFolder": { @@ -5526,26 +5519,26 @@ "message": "All items will be owned and saved to the organization, enabling organization-wide controls, visibility, and reporting. When turned on, a default collection be available for each member to store items. Learn more about managing the ", "description": "This will be used as part of a larger sentence, broken up to include links. The full sentence will read 'All items will be owned and saved to the organization, enabling organization-wide controls, visibility, and reporting. When turned on, a default collection be available for each member to store items. Learn more about managing the credential lifecycle.'" }, - "organizationDataOwnershipContentAnchor":{ + "organizationDataOwnershipContentAnchor": { "message": "credential lifecycle", "description": "This will be used as a hyperlink" }, - "organizationDataOwnershipWarningTitle":{ + "organizationDataOwnershipWarningTitle": { "message": "Are you sure you want to proceed?" }, - "organizationDataOwnershipWarning1":{ + "organizationDataOwnershipWarning1": { "message": "will remain accessible to members" }, - "organizationDataOwnershipWarning2":{ + "organizationDataOwnershipWarning2": { "message": "will not be automatically selected when creating new items" }, - "organizationDataOwnershipWarning3":{ + "organizationDataOwnershipWarning3": { "message": "cannot be managed from the Admin Console until the user is offboarded" }, - "organizationDataOwnershipWarningContentTop":{ + "organizationDataOwnershipWarningContentTop": { "message": "By turning this policy off, the default collection: " }, - "organizationDataOwnershipWarningContentBottom":{ + "organizationDataOwnershipWarningContentBottom": { "message": "Learn more about the ", "description": "This will be used as part of a larger sentence, broken up to include links. The full sentence will read 'Learn more about the credential lifecycle.'" }, @@ -8420,7 +8413,7 @@ } }, "accessedSecret": { - "message": "Accessed secret $SECRET_ID$.", + "message": "Accessed secret $SECRET_ID$.", "placeholders": { "secret_id": { "content": "$1", @@ -8428,7 +8421,7 @@ } } }, - "editedSecretWithId": { + "editedSecretWithId": { "message": "Edited a secret with identifier: $SECRET_ID$", "placeholders": { "secret_id": { @@ -8437,7 +8430,7 @@ } } }, - "deletedSecretWithId": { + "deletedSecretWithId": { "message": "Deleted a secret with identifier: $SECRET_ID$", "placeholders": { "secret_id": { @@ -8455,7 +8448,7 @@ } } }, - "restoredSecretWithId": { + "restoredSecretWithId": { "message": "Restored a secret with identifier: $SECRET_ID$", "placeholders": { "secret_id": { @@ -8464,7 +8457,7 @@ } } }, - "createdSecretWithId": { + "createdSecretWithId": { "message": "Created a new secret with identifier: $SECRET_ID$", "placeholders": { "secret_id": { @@ -8474,7 +8467,7 @@ } }, "accessedProjectWithId": { - "message": "Accessed a project with Id: $PROJECT_ID$.", + "message": "Accessed a project with Id: $PROJECT_ID$.", "placeholders": { "project_id": { "content": "$1", @@ -8483,7 +8476,7 @@ } }, "nameUnavailableProjectDeleted": { - "message": "Deleted project Id: $PROJECT_ID$", + "message": "Deleted project Id: $PROJECT_ID$", "placeholders": { "project_id": { "content": "$1", @@ -8492,7 +8485,7 @@ } }, "nameUnavailableSecretDeleted": { - "message": "Deleted secret Id: $SECRET_ID$", + "message": "Deleted secret Id: $SECRET_ID$", "placeholders": { "secret_id": { "content": "$1", @@ -8500,7 +8493,7 @@ } } }, - "editedProjectWithId": { + "editedProjectWithId": { "message": "Edited a project with identifier: $PROJECT_ID$", "placeholders": { "project_id": { @@ -8509,7 +8502,7 @@ } } }, - "deletedProjectWithId": { + "deletedProjectWithId": { "message": "Deleted a project with identifier: $PROJECT_ID$", "placeholders": { "project_id": { @@ -8518,7 +8511,7 @@ } } }, - "createdProjectWithId": { + "createdProjectWithId": { "message": "Created a new project with identifier: $PROJECT_ID$", "placeholders": { "project_id": { @@ -9236,15 +9229,15 @@ "message": "Common formats", "description": "Label indicating the most common import formats" }, - "uriMatchDefaultStrategyHint": { + "uriMatchDefaultStrategyHint": { "message": "URI match detection is how Bitwarden identifies autofill suggestions.", "description": "Explains to the user that URI match detection determines how Bitwarden suggests autofill options, and clarifies that this default strategy applies when no specific match detection is set for a login item." }, - "regExAdvancedOptionWarning": { + "regExAdvancedOptionWarning": { "message": "\"Regular expression\" is an advanced option with increased risk of exposing credentials.", "description": "Content for dialog which warns a user when selecting 'regular expression' matching strategy as a cipher match strategy" }, - "startsWithAdvancedOptionWarning": { + "startsWithAdvancedOptionWarning": { "message": "\"Starts with\" is an advanced option with increased risk of exposing credentials.", "description": "Content for dialog which warns a user when selecting 'starts with' matching strategy as a cipher match strategy" }, @@ -9252,11 +9245,11 @@ "message": "More about match detection", "description": "Link to match detection docs on warning dialog for advance match strategy" }, - "uriAdvancedOption":{ + "uriAdvancedOption": { "message": "Advanced options", "description": "Advanced option placeholder for uri option component" }, - "warningCapitalized": { + "warningCapitalized": { "message": "Warning", "description": "Warning (should maintain locale-relevant capitalization)" }, @@ -10087,27 +10080,9 @@ "memberAccessReportAuthenticationEnabledFalse": { "message": "Off" }, - "higherKDFIterations": { - "message": "Higher KDF iterations can help protect your master password from being brute forced by an attacker." - }, - "incrementsOf100,000": { - "message": "increments of 100,000" - }, - "smallIncrements": { - "message": "small increments" - }, "kdfIterationRecommends": { "message": "We recommend 600,000 or more" }, - "kdfToHighWarningIncreaseInIncrements": { - "message": "For older devices, setting your KDF too high may lead to performance issues. Increase the value in $VALUE$ and test your devices.", - "placeholders": { - "value": { - "content": "$1", - "example": "increments of 100,000" - } - } - }, "providerReinstate": { "message": " Contact Customer Support to reinstate your subscription." }, @@ -10783,7 +10758,7 @@ "orgTrustWarning1": { "message": "This organization has an Enterprise policy that will enroll you in account recovery. Enrollment will allow organization administrators to change your password. Only proceed if you recognize this organization and the fingerprint phrase displayed below matches the organization's fingerprint." }, - "trustUser":{ + "trustUser": { "message": "Trust user" }, "sshKeyWrongPassword": { @@ -10819,7 +10794,7 @@ "openingExtension": { "message": "Opening the Bitwarden browser extension" }, - "somethingWentWrong":{ + "somethingWentWrong": { "message": "Something went wrong..." }, "openingExtensionError": { @@ -10906,7 +10881,7 @@ } } }, - "accountDeprovisioningNotification" : { + "accountDeprovisioningNotification": { "message": "Administrators now have the ability to delete member accounts that belong to a claimed domain." }, "deleteManagedUserWarningDesc": { @@ -10997,14 +10972,14 @@ "upgradeForFullEventsMessage": { "message": "Event logs are not stored for your organization. Upgrade to a Teams or Enterprise plan to get full access to organization event logs." }, - "upgradeEventLogTitleMessage" : { - "message" : "Upgrade to see event logs from your organization." + "upgradeEventLogTitleMessage": { + "message": "Upgrade to see event logs from your organization." }, - "upgradeEventLogMessage":{ - "message" : "These events are examples only and do not reflect real events within your Bitwarden organization." + "upgradeEventLogMessage": { + "message": "These events are examples only and do not reflect real events within your Bitwarden organization." }, - "viewEvents":{ - "message" : "View Events" + "viewEvents": { + "message": "View Events" }, "cannotCreateCollection": { "message": "Free organizations may have up to 2 collections. Upgrade to a paid plan to add more collections." @@ -11265,14 +11240,14 @@ } } }, - "unlimitedSecretsAndProjects": { + "unlimitedSecretsAndProjects": { "message": "Unlimited secrets and projects" }, - "providersubscriptionCanceled": { + "providersubscriptionCanceled": { "message": "Subscription canceled" }, "providersubCanceledmessage": { - "message" : "To resubscribe, contact Bitwarden Customer Support." + "message": "To resubscribe, contact Bitwarden Customer Support." }, "showMore": { "message": "Show more" @@ -11341,5 +11316,32 @@ }, "verifyNow": { "message": "Verify now." + }, + "updateEncryptionSettings": { + "message": "Update encryption settings" + }, + "updateYourEncryptionSettings": { + "message": "Update your encryption settings" + }, + "updateSettings": { + "message": "Update settings" + }, + "algorithm": { + "message": "Algorithm" + }, + "encryptionKeySettingsHowShouldWeEncryptYourData": { + "message": "Choose how Bitwarden should encrypt your vault data. All options are secure, but stronger methods offer better protection-especially against brute-force attacks. Bitwarden recommend the default setting for most users." + }, + "encryptionKeySettingsIncreaseImproveSecurity": { + "message": "Increasing the values above the default will improve security, but your vault may take longer to unlock as a result." + }, + "encryptionKeySettingsAlgorithmPopoverTitle": { + "message": "About encryption algorithms" + }, + "encryptionKeySettingsAlgorithmPopoverPBKDF2": { + "message": "PBKDF2-SHA256 is a well-tested encryption method that balances security and performance. Good for all users." + }, + "encryptionKeySettingsAlgorithmPopoverArgon2Id": { + "message": "Argon2id offers stronger protection against modern attacks. Best for advanced users with powerful devices." } } From 06915792c95c620261ccc8317d2bbcd7272e38f5 Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Sun, 21 Sep 2025 10:59:57 +0100 Subject: [PATCH 03/14] correct validators load on init --- .../change-kdf/change-kdf.component.ts | 55 ++++++++++--------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf.component.ts b/apps/web/src/app/key-management/change-kdf/change-kdf.component.ts index 69201473418f..9444627a8eeb 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf.component.ts +++ b/apps/web/src/app/key-management/change-kdf/change-kdf.component.ts @@ -33,7 +33,7 @@ export class ChangeKdfComponent implements OnInit, OnDestroy { protected formGroup = this.formBuilder.group({ kdf: new FormControl(KdfType.PBKDF2_SHA256, [Validators.required]), kdfConfig: this.formBuilder.group({ - iterations: new FormControl(this.kdfConfig.iterations, [Validators.required]), + iterations: [null as number], memory: [null as number], parallelism: [null as number], }), @@ -68,62 +68,65 @@ export class ChangeKdfComponent implements OnInit, OnDestroy { this.kdfConfig = await this.kdfConfigService.getKdfConfig(userId); this.formGroup.get("kdf").setValue(this.kdfConfig.kdfType); this.setFormControlValues(this.kdfConfig); + this.setFormValidators(this.kdfConfig.kdfType); this.formGroup .get("kdf") .valueChanges.pipe(takeUntil(this.destroy$)) - .subscribe((newValue) => { + .subscribe((newValue: KdfType) => { this.updateKdfConfig(newValue); }); } private updateKdfConfig(newValue: KdfType) { let config: KdfConfig; - const validators: { [key: string]: ValidatorFn[] } = { - iterations: [], - memory: [], - parallelism: [], - }; switch (newValue) { case KdfType.PBKDF2_SHA256: config = new PBKDF2KdfConfig(); - validators.iterations = [ + break; + case KdfType.Argon2id: + config = new Argon2KdfConfig(); + break; + default: + throw new Error("Unknown KDF type."); + } + + this.kdfConfig = config; + this.setFormValidators(newValue); + this.setFormControlValues(this.kdfConfig); + } + + private setFormValidators(kdfType: KdfType) { + switch (kdfType) { + case KdfType.PBKDF2_SHA256: + this.setValidators("kdfConfig.iterations", [ Validators.required, Validators.min(PBKDF2KdfConfig.ITERATIONS.min), Validators.max(PBKDF2KdfConfig.ITERATIONS.max), - ]; + ]); + this.setValidators("kdfConfig.memory", []); + this.setValidators("kdfConfig.parallelism", []); break; case KdfType.Argon2id: - config = new Argon2KdfConfig(); - validators.iterations = [ + this.setValidators("kdfConfig.iterations", [ Validators.required, Validators.min(Argon2KdfConfig.ITERATIONS.min), Validators.max(Argon2KdfConfig.ITERATIONS.max), - ]; - validators.memory = [ + ]); + this.setValidators("kdfConfig.memory", [ Validators.required, Validators.min(Argon2KdfConfig.MEMORY.min), Validators.max(Argon2KdfConfig.MEMORY.max), - ]; - validators.parallelism = [ + ]); + this.setValidators("kdfConfig.parallelism", [ Validators.required, Validators.min(Argon2KdfConfig.PARALLELISM.min), Validators.max(Argon2KdfConfig.PARALLELISM.max), - ]; + ]); break; default: throw new Error("Unknown KDF type."); } - - this.kdfConfig = config; - this.setFormValidators(validators); - this.setFormControlValues(this.kdfConfig); - } - - private setFormValidators(validators: { [key: string]: ValidatorFn[] }) { - this.setValidators("kdfConfig.iterations", validators.iterations); - this.setValidators("kdfConfig.memory", validators.memory); - this.setValidators("kdfConfig.parallelism", validators.parallelism); } private setValidators(controlName: string, validators: ValidatorFn[]) { const control = this.formGroup.get(controlName); From a20f0525663af6ce4049c5432e29aca67554496e Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Sun, 21 Sep 2025 14:16:52 +0100 Subject: [PATCH 04/14] incorrect feature flag observable check --- .../change-kdf/change-kdf-confirmation.component.html | 2 +- .../src/app/key-management/change-kdf/change-kdf.component.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.html b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.html index d5d1127feaa0..3a6a12dfc27a 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.html +++ b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.html @@ -5,7 +5,7 @@ - @if (!forceUpdateKDFSettingsFeatureFlag$) { + @if (!(forceUpdateKDFSettingsFeatureFlag$ | async)) { {{ "kdfSettingsChangeLogoutWarning" | i18n }} } diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf.component.html b/apps/web/src/app/key-management/change-kdf/change-kdf.component.html index fd444f852eb4..9d274a994552 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf.component.html +++ b/apps/web/src/app/key-management/change-kdf/change-kdf.component.html @@ -1,7 +1,7 @@

{{ "encKeySettings" | i18n }}

-@if (!forceUpdateKDFSettingsFeatureFlag$) { +@if (!(forceUpdateKDFSettingsFeatureFlag$ | async)) { {{ "kdfSettingsChangeLogoutWarning" | i18n }} }

From 5f8443cef5f9ae3992520f0447df5c68de6d2f71 Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Sun, 21 Sep 2025 14:52:42 +0100 Subject: [PATCH 05/14] unit test coverage --- .../change-kdf-confirmation.component.ts | 8 +- .../change-kdf/change-kdf.component.spec.ts | 385 ++++++++++++++++++ 2 files changed, 388 insertions(+), 5 deletions(-) create mode 100644 apps/web/src/app/key-management/change-kdf/change-kdf.component.spec.ts diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts index 20180ef5c161..2360a500a1de 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts +++ b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts @@ -23,10 +23,9 @@ export class ChangeKdfConfirmationComponent { kdfConfig: KdfConfig; form = new FormGroup({ - masterPassword: new FormControl(null, Validators.required), + masterPassword: new FormControl(null, Validators.required), }); showPassword = false; - masterPassword: string; loading = false; forceUpdateKDFSettingsFeatureFlag$: Observable; @@ -43,7 +42,6 @@ export class ChangeKdfConfirmationComponent { configService: ConfigService, ) { this.kdfConfig = params.kdfConfig; - this.masterPassword = null; this.forceUpdateKDFSettingsFeatureFlag$ = configService.getFeatureFlag$( FeatureFlag.ForceUpdateKDFSettings, ); @@ -54,7 +52,7 @@ export class ChangeKdfConfirmationComponent { return; } this.loading = true; - await this.makeKeyAndSaveAsync(); + await this.makeKeyAndSave(); if (await firstValueFrom(this.forceUpdateKDFSettingsFeatureFlag$)) { this.toastService.showToast({ variant: "success", @@ -72,7 +70,7 @@ export class ChangeKdfConfirmationComponent { this.loading = false; }; - private async makeKeyAndSaveAsync() { + async makeKeyAndSave() { const activeAccount = await firstValueFrom(this.accountService.activeAccount$); if (activeAccount == null) { throw new Error("No active account found."); diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf.component.spec.ts b/apps/web/src/app/key-management/change-kdf/change-kdf.component.spec.ts new file mode 100644 index 000000000000..8a03c94ffbb8 --- /dev/null +++ b/apps/web/src/app/key-management/change-kdf/change-kdf.component.spec.ts @@ -0,0 +1,385 @@ +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { FormBuilder, FormGroup } from "@angular/forms"; +import { mock, MockProxy } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; +import { UserId } from "@bitwarden/common/types/guid"; +import { DialogService, PopoverModule, CalloutModule } from "@bitwarden/components"; +import { + KdfConfigService, + Argon2KdfConfig, + PBKDF2KdfConfig, + KdfType, +} from "@bitwarden/key-management"; + +import { SharedModule } from "../../shared"; + +import { ChangeKdfComponent } from "./change-kdf.component"; + +describe("ChangeKdfComponent", () => { + let component: ChangeKdfComponent; + let fixture: ComponentFixture; + + // Mock Services + let mockDialogService: MockProxy; + let mockKdfConfigService: MockProxy; + let mockConfigService: MockProxy; + let mockI18nService: MockProxy; + let accountService: FakeAccountService; + let formBuilder: FormBuilder; + + const mockUserId = "user-id" as UserId; + + // Helper functions for validation testing + function expectPBKDF2Validation(formGroup: FormGroup): void { + const iterationsControl = formGroup.get("kdfConfig.iterations"); + const memoryControl = formGroup.get("kdfConfig.memory"); + const parallelismControl = formGroup.get("kdfConfig.parallelism"); + + // Assert current validators state + expect(iterationsControl?.hasError("required")).toBe(false); + expect(iterationsControl?.hasError("min")).toBe(false); + expect(iterationsControl?.hasError("max")).toBe(false); + expect(memoryControl?.validator).toBeNull(); + expect(parallelismControl?.validator).toBeNull(); + + // Test validation boundaries + iterationsControl?.setValue(PBKDF2KdfConfig.ITERATIONS.min - 1); + expect(iterationsControl?.hasError("min")).toBe(true); + + iterationsControl?.setValue(PBKDF2KdfConfig.ITERATIONS.max + 1); + expect(iterationsControl?.hasError("max")).toBe(true); + } + + function expectArgon2Validation(formGroup: FormGroup): void { + const iterationsControl = formGroup.get("kdfConfig.iterations"); + const memoryControl = formGroup.get("kdfConfig.memory"); + const parallelismControl = formGroup.get("kdfConfig.parallelism"); + + // Assert current validators state + expect(iterationsControl?.hasError("required")).toBe(false); + expect(memoryControl?.hasError("required")).toBe(false); + expect(parallelismControl?.hasError("required")).toBe(false); + + // Test validation boundaries - min values + iterationsControl?.setValue(Argon2KdfConfig.ITERATIONS.min - 1); + expect(iterationsControl?.hasError("min")).toBe(true); + + memoryControl?.setValue(Argon2KdfConfig.MEMORY.min - 1); + expect(memoryControl?.hasError("min")).toBe(true); + + parallelismControl?.setValue(Argon2KdfConfig.PARALLELISM.min - 1); + expect(parallelismControl?.hasError("min")).toBe(true); + + // Test validation boundaries - max values + iterationsControl?.setValue(Argon2KdfConfig.ITERATIONS.max + 1); + expect(iterationsControl?.hasError("max")).toBe(true); + + memoryControl?.setValue(Argon2KdfConfig.MEMORY.max + 1); + expect(memoryControl?.hasError("max")).toBe(true); + + parallelismControl?.setValue(Argon2KdfConfig.PARALLELISM.max + 1); + expect(parallelismControl?.hasError("max")).toBe(true); + } + + beforeEach(() => { + mockDialogService = mock(); + mockKdfConfigService = mock(); + mockConfigService = mock(); + mockI18nService = mock(); + accountService = mockAccountServiceWith(mockUserId); + formBuilder = new FormBuilder(); + + mockConfigService.getFeatureFlag$.mockReturnValue(of(false)); + + // Mock i18n service with switch statement for all keys + mockI18nService.t.mockImplementation((key: string) => { + switch (key) { + case "encKeySettings": + return "Encryption Key Settings"; + case "kdfSettingsChangeLogoutWarning": + return "Proceeding will log you out of all active sessions. You will need to log back in and complete two-step login, if any. We recommend exporting your vault before changing your encryption settings to prevent data loss."; + case "encryptionKeySettingsHowShouldWeEncryptYourData": + return "How should we encrypt your data?"; + case "encryptionKeySettingsIncreaseImproveSecurity": + return "Increase the number of iterations to improve security at the cost of slower login times."; + case "algorithm": + return "Algorithm"; + case "kdfIterations": + return "KDF Iterations"; + case "kdfIterationRecommends": + return "Minimum 100,000 iterations recommended."; + case "kdfMemory": + return "Memory (MB)"; + case "kdfParallelism": + return "Parallelism"; + case "updateEncryptionSettings": + return "Update Encryption Settings"; + case "encryptionKeySettingsAlgorithmPopoverTitle": + return "Encryption Key Settings"; + case "encryptionKeySettingsAlgorithmPopoverPBKDF2": + return "PBKDF2 SHA-256 is the default algorithm and is supported by all Bitwarden applications."; + case "encryptionKeySettingsAlgorithmPopoverArgon2Id": + return "Argon2id is a newer algorithm that is more secure but may not be supported by older Bitwarden applications."; + case "learnMoreAboutEncryptionAlgorithms": + return "Learn more about encryption algorithms"; + case "learnMore": + return "Learn more"; + default: + return key; + } + }); + + TestBed.configureTestingModule({ + declarations: [ChangeKdfComponent], + imports: [SharedModule, PopoverModule, CalloutModule], + providers: [ + { provide: DialogService, useValue: mockDialogService }, + { provide: KdfConfigService, useValue: mockKdfConfigService }, + { provide: AccountService, useValue: accountService }, + { provide: FormBuilder, useValue: formBuilder }, + { provide: ConfigService, useValue: mockConfigService }, + { provide: I18nService, useValue: mockI18nService }, + ], + }); + }); + + describe("Component Initialization", () => { + describe("given PBKDF2 configuration", () => { + it("should initialize form with PBKDF2 values and validators when component loads", async () => { + // Arrange + const mockPBKDF2Config = new PBKDF2KdfConfig(600_000); + mockKdfConfigService.getKdfConfig.mockResolvedValue(mockPBKDF2Config); + + // Act + fixture = TestBed.createComponent(ChangeKdfComponent); + component = fixture.componentInstance; + await component.ngOnInit(); + + // Extract form controls + const formGroup = component["formGroup"]; + + // Assert form values + expect(formGroup.get("kdf")?.value).toBe(KdfType.PBKDF2_SHA256); + expect(formGroup.get("kdfConfig.iterations")?.value).toBe(600_000); + expect(formGroup.get("kdfConfig.memory")?.value).toBeNull(); + expect(formGroup.get("kdfConfig.parallelism")?.value).toBeNull(); + expect(component.kdfConfig).toEqual(mockPBKDF2Config); + + // Assert validators + expectPBKDF2Validation(formGroup); + }); + }); + + describe("given Argon2id configuration", () => { + it("should initialize form with Argon2id values and validators when component loads", async () => { + // Arrange + const mockArgon2Config = new Argon2KdfConfig(3, 64, 4); + mockKdfConfigService.getKdfConfig.mockResolvedValue(mockArgon2Config); + + // Act + fixture = TestBed.createComponent(ChangeKdfComponent); + component = fixture.componentInstance; + await component.ngOnInit(); + + // Extract form controls + const formGroup = component["formGroup"]; + + // Assert form values + expect(formGroup.get("kdf")?.value).toBe(KdfType.Argon2id); + expect(formGroup.get("kdfConfig.iterations")?.value).toBe(3); + expect(formGroup.get("kdfConfig.memory")?.value).toBe(64); + expect(formGroup.get("kdfConfig.parallelism")?.value).toBe(4); + expect(component.kdfConfig).toEqual(mockArgon2Config); + + // Assert validators + expectArgon2Validation(formGroup); + }); + }); + + it.each([ + [true, false], + [false, true], + ])( + "should show log out banner = %s when feature flag observable is %s", + async (showLogOutBanner, forceUpgradeKdfFeatureFlag) => { + // Arrange + const mockPBKDF2Config = new PBKDF2KdfConfig(600_000); + mockKdfConfigService.getKdfConfig.mockResolvedValue(mockPBKDF2Config); + mockConfigService.getFeatureFlag$.mockReturnValue(of(forceUpgradeKdfFeatureFlag)); + + // Act + fixture = TestBed.createComponent(ChangeKdfComponent); + component = fixture.componentInstance; + await component.ngOnInit(); + fixture.detectChanges(); + + // Assert + const calloutElement = fixture.debugElement.query((el) => + el.nativeElement.textContent?.includes("Proceeding will log you out"), + ); + + if (showLogOutBanner) { + expect(calloutElement).not.toBeNull(); + expect(calloutElement.nativeElement.textContent).toContain( + "Proceeding will log you out of all active sessions", + ); + } else { + expect(calloutElement).toBeNull(); + } + }, + ); + }); + + describe("KDF Type Switching", () => { + describe("switching from PBKDF2 to Argon2id", () => { + beforeEach(async () => { + // Setup component with initial PBKDF2 configuration + const mockPBKDF2Config = new PBKDF2KdfConfig(600_001); + mockKdfConfigService.getKdfConfig.mockResolvedValue(mockPBKDF2Config); + + fixture = TestBed.createComponent(ChangeKdfComponent); + component = fixture.componentInstance; + await component.ngOnInit(); + }); + + it("should update form structure and default values when KDF type changes to Argon2id", () => { + // Arrange + const formGroup = component["formGroup"]; + const kdfControl = formGroup.get("kdf"); + + // Act - change KDF type to Argon2id + kdfControl?.setValue(KdfType.Argon2id); + + // Assert form values update to Argon2id defaults + expect(formGroup.get("kdf")?.value).toBe(KdfType.Argon2id); + expect(formGroup.get("kdfConfig.iterations")?.value).toBe(3); // Argon2id default + expect(formGroup.get("kdfConfig.memory")?.value).toBe(64); // Argon2id default + expect(formGroup.get("kdfConfig.parallelism")?.value).toBe(4); // Argon2id default + }); + + it("should update validators when KDF type changes to Argon2id", () => { + // Arrange + const formGroup = component["formGroup"]; + const kdfControl = formGroup.get("kdf"); + + // Act - change KDF type to Argon2id + kdfControl?.setValue(KdfType.Argon2id); + + // Assert validators update to Argon2id validation rules + expectArgon2Validation(formGroup); + }); + }); + + describe("switching from Argon2id to PBKDF2", () => { + beforeEach(async () => { + // Setup component with initial Argon2id configuration + const mockArgon2IdConfig = new Argon2KdfConfig(4, 65, 5); + mockKdfConfigService.getKdfConfig.mockResolvedValue(mockArgon2IdConfig); + + fixture = TestBed.createComponent(ChangeKdfComponent); + component = fixture.componentInstance; + await component.ngOnInit(); + }); + + it("should update form structure and default values when KDF type changes to PBKDF2", () => { + // Arrange + const formGroup = component["formGroup"]; + const kdfControl = formGroup.get("kdf"); + + // Act - change KDF type back to PBKDF2 + kdfControl?.setValue(KdfType.PBKDF2_SHA256); + + // Assert form values update to PBKDF2 defaults + expect(formGroup.get("kdf")?.value).toBe(KdfType.PBKDF2_SHA256); + expect(formGroup.get("kdfConfig.iterations")?.value).toBe(600_000); // PBKDF2 default + expect(formGroup.get("kdfConfig.memory")?.value).toBeNull(); // PBKDF2 doesn't use memory + expect(formGroup.get("kdfConfig.parallelism")?.value).toBeNull(); // PBKDF2 doesn't use parallelism + }); + + it("should update validators when KDF type changes to PBKDF2", () => { + // Arrange + const formGroup = component["formGroup"]; + const kdfControl = formGroup.get("kdf"); + + // Act - change KDF type back to PBKDF2 + kdfControl?.setValue(KdfType.PBKDF2_SHA256); + + // Assert validators update to PBKDF2 validation rules + expectPBKDF2Validation(formGroup); + }); + }); + }); + + describe("openConfirmationModal", () => { + describe("when form is valid", () => { + it("should open confirmation modal with PBKDF2 config when form is submitted", async () => { + // Arrange + const mockPBKDF2Config = new PBKDF2KdfConfig(600_001); + mockKdfConfigService.getKdfConfig.mockResolvedValue(mockPBKDF2Config); + + fixture = TestBed.createComponent(ChangeKdfComponent); + component = fixture.componentInstance; + await component.ngOnInit(); + + // Act + await component.openConfirmationModal(); + + // Assert + expect(mockDialogService.open).toHaveBeenCalledWith( + expect.any(Function), + expect.objectContaining({ + data: expect.objectContaining({ + kdfConfig: mockPBKDF2Config, + }), + }), + ); + }); + + it("should open confirmation modal with Argon2id config when form is submitted", async () => { + // Arrange + const mockArgon2Config = new Argon2KdfConfig(4, 65, 5); + mockKdfConfigService.getKdfConfig.mockResolvedValue(mockArgon2Config); + + fixture = TestBed.createComponent(ChangeKdfComponent); + component = fixture.componentInstance; + await component.ngOnInit(); + + // Act + await component.openConfirmationModal(); + + // Assert + expect(mockDialogService.open).toHaveBeenCalledWith( + expect.any(Function), + expect.objectContaining({ + data: expect.objectContaining({ + kdfConfig: mockArgon2Config, + }), + }), + ); + }); + }); + + describe("when form is invalid", () => { + it("should not open modal when form is invalid", async () => { + // Arrange + const mockPBKDF2Config = new PBKDF2KdfConfig(PBKDF2KdfConfig.ITERATIONS.min - 1); + mockKdfConfigService.getKdfConfig.mockResolvedValue(mockPBKDF2Config); + + fixture = TestBed.createComponent(ChangeKdfComponent); + component = fixture.componentInstance; + await component.ngOnInit(); + + // Act + await component.openConfirmationModal(); + + // Assert + expect(mockDialogService.open).not.toHaveBeenCalled(); + }); + }); + }); +}); From 7b06cd6b396cd3781df6fa701c9c2979bf036a52 Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Sun, 21 Sep 2025 19:59:38 +0100 Subject: [PATCH 06/14] unit test coverage --- .../change-kdf-confirmation.component.spec.ts | 345 ++++++++++++++++++ 1 file changed, 345 insertions(+) create mode 100644 apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.spec.ts diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.spec.ts b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.spec.ts new file mode 100644 index 000000000000..9037196d9b5b --- /dev/null +++ b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.spec.ts @@ -0,0 +1,345 @@ +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { FormControl } from "@angular/forms"; +import { mock, MockProxy } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { + FakeAccountService, + makeEncString, + makeSymmetricCryptoKey, + mockAccountServiceWith, +} from "@bitwarden/common/spec"; +import { UserId } from "@bitwarden/common/types/guid"; +import { MasterKey, UserKey } from "@bitwarden/common/types/key"; +import { DIALOG_DATA, DialogRef, ToastService } from "@bitwarden/components"; +import { KdfType, KeyService, PBKDF2KdfConfig, Argon2KdfConfig } from "@bitwarden/key-management"; + +import { SharedModule } from "../../shared"; + +import { ChangeKdfConfirmationComponent } from "./change-kdf-confirmation.component"; + +import SpyInstance = jest.SpyInstance; + +describe("ChangeKdfConfirmationComponent", () => { + let component: ChangeKdfConfirmationComponent; + let fixture: ComponentFixture; + + // Mock Services + let mockApiService: MockProxy; + let mockI18nService: MockProxy; + let mockKeyService: MockProxy; + let mockMessagingService: MockProxy; + let mockToastService: MockProxy; + let mockDialogRef: MockProxy>; + let mockConfigService: MockProxy; + let accountService: FakeAccountService; + + const mockUserId = "user-id" as UserId; + const mockEmail = "email"; + const mockMasterPassword = "master-password"; + const mockDialogData = jest.fn(); + + beforeEach(() => { + mockApiService = mock(); + mockI18nService = mock(); + mockKeyService = mock(); + mockMessagingService = mock(); + mockToastService = mock(); + mockDialogRef = mock>(); + mockConfigService = mock(); + accountService = mockAccountServiceWith(mockUserId, { email: mockEmail }); + + // Mock i18n service + mockI18nService.t.mockImplementation((key: string) => { + switch (key) { + case "encKeySettingsChanged": + return "Encryption key settings changed"; + case "logBackIn": + return "Please log back in"; + default: + return key; + } + }); + + // Mock config service feature flag + mockConfigService.getFeatureFlag$.mockReturnValue(of(false)); + + mockDialogData.mockReturnValue({ + kdf: KdfType.PBKDF2_SHA256, + kdfConfig: new PBKDF2KdfConfig(600_000), + }); + + TestBed.configureTestingModule({ + declarations: [ChangeKdfConfirmationComponent], + imports: [SharedModule], + providers: [ + { provide: ApiService, useValue: mockApiService }, + { provide: I18nService, useValue: mockI18nService }, + { provide: KeyService, useValue: mockKeyService }, + { provide: MessagingService, useValue: mockMessagingService }, + { provide: AccountService, useValue: accountService }, + { provide: ToastService, useValue: mockToastService }, + { provide: DialogRef, useValue: mockDialogRef }, + { provide: ConfigService, useValue: mockConfigService }, + { + provide: DIALOG_DATA, + useFactory: mockDialogData, + }, + ], + }); + }); + + describe("Component Initialization", () => { + it("should create component with PBKDF2 config", () => { + mockDialogData.mockReturnValue({ + kdf: KdfType.PBKDF2_SHA256, + kdfConfig: new PBKDF2KdfConfig(600_001), + }); + + const fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); + const component = fixture.componentInstance; + + expect(component).toBeTruthy(); + expect(component.kdfConfig).toBeInstanceOf(PBKDF2KdfConfig); + expect(component.kdfConfig.iterations).toBe(600_001); + }); + + it("should create component with Argon2id config", () => { + mockDialogData.mockReturnValue({ + kdf: KdfType.Argon2id, + kdfConfig: new Argon2KdfConfig(4, 65, 5), + }); + + const fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); + const component = fixture.componentInstance; + + expect(component).toBeTruthy(); + expect(component.kdfConfig).toBeInstanceOf(Argon2KdfConfig); + const kdfConfig = component.kdfConfig as Argon2KdfConfig; + expect(kdfConfig.iterations).toBe(4); + expect(kdfConfig.memory).toBe(65); + expect(kdfConfig.parallelism).toBe(5); + }); + + it("should initialize form with required master password field", () => { + const fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); + const component = fixture.componentInstance; + + expect(component.form.get("masterPassword")?.value).toEqual(null); + expect(component.form.get("masterPassword")).toBeInstanceOf(FormControl); + expect(component.form.get("masterPassword")?.hasError("required")).toBe(true); + }); + }); + + describe("Form Validation", () => { + beforeEach(() => { + fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); + component = fixture.componentInstance; + }); + + it("should be invalid when master password is empty", () => { + component.form.get("masterPassword")?.setValue(""); + expect(component.form.invalid).toBe(true); + }); + + it("should be valid when master password is provided", () => { + component.form.get("masterPassword")?.setValue(mockMasterPassword); + expect(component.form.valid).toBe(true); + }); + }); + + describe("submit method", () => { + let makeKeyAndSaveSpy: SpyInstance>; + beforeEach(() => { + fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); + component = fixture.componentInstance; + + makeKeyAndSaveSpy = jest.spyOn(component, "makeKeyAndSave"); + makeKeyAndSaveSpy.mockImplementation(); + }); + + describe("when form is invalid", () => { + it("should return early without processing", async () => { + // Arrange + component.form.get("masterPassword")?.setValue(""); + expect(component.form.invalid).toBe(true); + + // Act + await component.submit(); + + // Assert + expect(makeKeyAndSaveSpy).not.toHaveBeenCalled(); + }); + }); + + describe("when form is valid", () => { + beforeEach(() => { + component.form.get("masterPassword")?.setValue(mockMasterPassword); + }); + + it("should set loading to true during submission", async () => { + // Arrange + let loadingDuringExecution = false; + makeKeyAndSaveSpy.mockImplementation(async () => { + loadingDuringExecution = component.loading; + }); + + // Act + await component.submit(); + + expect(loadingDuringExecution).toBe(true); + expect(component.loading).toBe(false); + }); + + it("should call makeKeyAndSaveAsync", async () => { + // Act + await component.submit(); + + // Assert + expect(makeKeyAndSaveSpy).toHaveBeenCalledTimes(1); + }); + + describe("when ForceUpdateKDFSettings feature flag is enabled", () => { + it("should show success toast and close dialog", async () => { + // Arrange - reset mocks and create component with feature flag enabled + mockConfigService.getFeatureFlag$.mockReturnValue(of(true)); + + const fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); + const component = fixture.componentInstance; + + makeKeyAndSaveSpy = jest.spyOn(component, "makeKeyAndSave"); + makeKeyAndSaveSpy.mockImplementation(); + + component.form.get("masterPassword")?.setValue(mockMasterPassword); + + // Act + await component.submit(); + + // Assert + expect(mockToastService.showToast).toHaveBeenCalledWith({ + variant: "success", + message: "Encryption key settings changed", + }); + expect(mockDialogRef.close).toHaveBeenCalled(); + expect(mockMessagingService.send).not.toHaveBeenCalled(); + }); + }); + + describe("when ForceUpdateKDFSettings feature flag is disabled", () => { + it("should show toast with logout message and send logout", async () => { + // Act + await component.submit(); + + // Assert + expect(mockToastService.showToast).toHaveBeenCalledWith({ + variant: "success", + title: "Encryption key settings changed", + message: "Please log back in", + }); + expect(mockMessagingService.send).toHaveBeenCalledWith("logout"); + expect(mockDialogRef.close).not.toHaveBeenCalled(); + }); + }); + }); + }); + + describe("makeKeyAndSaveAsync", () => { + const kdfConfig = new PBKDF2KdfConfig(); + const validateKdfConfigForSetting = jest.spyOn(kdfConfig, "validateKdfConfigForSetting"); + const mockMasterKey = makeSymmetricCryptoKey(64) as MasterKey; + const mockNewMasterKey = makeSymmetricCryptoKey(64) as MasterKey; + const mockNewUserKey = makeSymmetricCryptoKey(64) as UserKey; + const mockNewUserKeyEncrypted = makeEncString("user-key"); + const mockMasterPasswordHash = "master-password-hash"; + const mockNewMasterPasswordHash = "new-master-password-hash"; + + beforeEach(() => { + fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); + component = fixture.componentInstance; + + component.kdfConfig = kdfConfig; + + component.form.get("masterPassword")?.setValue(mockMasterPassword); + + mockKeyService.getOrDeriveMasterKey.mockResolvedValue(mockMasterKey); + mockKeyService.hashMasterKey + .mockResolvedValueOnce(mockMasterPasswordHash) + .mockResolvedValueOnce(mockNewMasterPasswordHash); + mockKeyService.makeMasterKey.mockResolvedValue(mockNewMasterKey); + mockKeyService.encryptUserKeyWithMasterKey.mockResolvedValue([ + mockNewUserKey, + mockNewUserKeyEncrypted, + ]); + }); + + it("should throw error when no active account", async () => { + accountService.activeAccount$ = of(null); + + await expect(component.makeKeyAndSave()).rejects.toThrow("No active account found."); + }); + + it("should throw error when KDF validation failed", async () => { + validateKdfConfigForSetting.mockImplementation(() => { + throw new Error("KDF config invalid"); + }); + component.kdfConfig = kdfConfig; + + await expect(component.makeKeyAndSave()).rejects.toThrow("KDF config invalid"); + }); + + it.each([new PBKDF2KdfConfig(600_001), new Argon2KdfConfig(4, 65, 5)])( + "should post KDF request to API when kdf = %s", + async (kdfConfig) => { + // Arrange + component.kdfConfig = kdfConfig; + const expectedRequest = { + kdf: kdfConfig.kdfType, + kdfIterations: kdfConfig.iterations, + kdfMemory: kdfConfig instanceof Argon2KdfConfig ? kdfConfig.memory : undefined, + kdfParallelism: kdfConfig instanceof Argon2KdfConfig ? kdfConfig.parallelism : undefined, + masterPasswordHash: mockMasterPasswordHash, + newMasterPasswordHash: mockNewMasterPasswordHash, + key: mockNewUserKeyEncrypted.encryptedString, + }; + if (kdfConfig instanceof PBKDF2KdfConfig) { + delete expectedRequest.kdfMemory; + delete expectedRequest.kdfParallelism; + } + + // Act + await component.makeKeyAndSave(); + + // Assert + expect(validateKdfConfigForSetting).toHaveBeenCalled(); + expect(mockKeyService.getOrDeriveMasterKey).toHaveBeenCalledWith( + mockMasterPassword, + mockUserId, + ); + expect(mockKeyService.hashMasterKey).toHaveBeenNthCalledWith( + 1, + mockMasterPassword, + mockMasterKey, + ); + expect(mockKeyService.makeMasterKey).toHaveBeenCalledWith( + mockMasterPassword, + mockEmail, + kdfConfig, + ); + expect(mockKeyService.hashMasterKey).toHaveBeenNthCalledWith( + 2, + mockMasterPassword, + mockNewMasterKey, + ); + expect(mockKeyService.encryptUserKeyWithMasterKey).toHaveBeenCalledWith(mockNewMasterKey); + expect(mockApiService.postAccountKdf).toHaveBeenCalledWith( + expect.objectContaining(expectedRequest), + ); + }, + ); + }); +}); From 97c3eed2323f5bf6a9dbf6225d85ccc52ad5aa3d Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Mon, 22 Sep 2025 21:27:10 +0100 Subject: [PATCH 07/14] remove Close button, wrong icon --- .../app/key-management/change-kdf/change-kdf.component.html | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf.component.html b/apps/web/src/app/key-management/change-kdf/change-kdf.component.html index 9d274a994552..d12d75d93716 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf.component.html +++ b/apps/web/src/app/key-management/change-kdf/change-kdf.component.html @@ -20,7 +20,6 @@

type="button" class="tw-border-none tw-bg-transparent tw-text-primary-600 tw-p-0" [bitPopoverTriggerFor]="algorithmPopover" - #triggerRef="popoverTrigger" aria-label="Open about encryption algorithms popover" title="Open about encryption algorithms popover" bitLink @@ -122,8 +121,7 @@

appA11yTitle="{{ 'learnMoreAboutEncryptionAlgorithms' | i18n }}" > {{ "learnMore" | i18n }} - + - From 5a64c34a33c4a9b45c918372fcfc50f67de66527 Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Mon, 29 Sep 2025 21:08:19 +0100 Subject: [PATCH 08/14] change to `pm-23995-no-logout-on-kdf-change` feature flag --- .../change-kdf/change-kdf-confirmation.component.html | 2 +- .../change-kdf/change-kdf-confirmation.component.ts | 8 ++++---- .../key-management/change-kdf/change-kdf.component.html | 2 +- .../app/key-management/change-kdf/change-kdf.component.ts | 6 +++--- libs/common/src/enums/feature-flag.enum.ts | 2 ++ 5 files changed, 11 insertions(+), 9 deletions(-) diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.html b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.html index 3a6a12dfc27a..11818b5a62aa 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.html +++ b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.html @@ -5,7 +5,7 @@ - @if (!(forceUpdateKDFSettingsFeatureFlag$ | async)) { + @if (!(noLogoutOnKdfChangeFeatureFlag$ | async)) { {{ "kdfSettingsChangeLogoutWarning" | i18n }} } diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts index 9a0eaede6041..418d68092c41 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts +++ b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts @@ -28,7 +28,7 @@ export class ChangeKdfConfirmationComponent { showPassword = false; loading = false; - forceUpdateKDFSettingsFeatureFlag$: Observable; + noLogoutOnKdfChangeFeatureFlag$: Observable; constructor( private i18nService: I18nService, @@ -41,8 +41,8 @@ export class ChangeKdfConfirmationComponent { configService: ConfigService, ) { this.kdfConfig = params.kdfConfig; - this.forceUpdateKDFSettingsFeatureFlag$ = configService.getFeatureFlag$( - FeatureFlag.ForceUpdateKDFSettings, + this.noLogoutOnKdfChangeFeatureFlag$ = configService.getFeatureFlag$( + FeatureFlag.NoLogoutOnKdfChange, ); } @@ -52,7 +52,7 @@ export class ChangeKdfConfirmationComponent { } this.loading = true; await this.makeKeyAndSave(); - if (await firstValueFrom(this.forceUpdateKDFSettingsFeatureFlag$)) { + if (await firstValueFrom(this.noLogoutOnKdfChangeFeatureFlag$)) { this.toastService.showToast({ variant: "success", message: this.i18nService.t("encKeySettingsChanged"), diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf.component.html b/apps/web/src/app/key-management/change-kdf/change-kdf.component.html index d12d75d93716..8b8bd603e701 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf.component.html +++ b/apps/web/src/app/key-management/change-kdf/change-kdf.component.html @@ -1,7 +1,7 @@

{{ "encKeySettings" | i18n }}

-@if (!(forceUpdateKDFSettingsFeatureFlag$ | async)) { +@if (!(noLogoutOnKdfChangeFeatureFlag$ | async)) { {{ "kdfSettingsChangeLogoutWarning" | i18n }} }

diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf.component.ts b/apps/web/src/app/key-management/change-kdf/change-kdf.component.ts index 9444627a8eeb..70d74c7b9fc0 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf.component.ts +++ b/apps/web/src/app/key-management/change-kdf/change-kdf.component.ts @@ -45,7 +45,7 @@ export class ChangeKdfComponent implements OnInit, OnDestroy { protected ARGON2_MEMORY = Argon2KdfConfig.MEMORY; protected ARGON2_PARALLELISM = Argon2KdfConfig.PARALLELISM; - forceUpdateKDFSettingsFeatureFlag$: Observable; + noLogoutOnKdfChangeFeatureFlag$: Observable; constructor( private dialogService: DialogService, @@ -58,8 +58,8 @@ export class ChangeKdfComponent implements OnInit, OnDestroy { { name: "PBKDF2 SHA-256", value: KdfType.PBKDF2_SHA256 }, { name: "Argon2id", value: KdfType.Argon2id }, ]; - this.forceUpdateKDFSettingsFeatureFlag$ = configService.getFeatureFlag$( - FeatureFlag.ForceUpdateKDFSettings, + this.noLogoutOnKdfChangeFeatureFlag$ = configService.getFeatureFlag$( + FeatureFlag.NoLogoutOnKdfChange, ); } diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 18134fee2c37..b18afe2c1b4a 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -33,6 +33,7 @@ export enum FeatureFlag { PrivateKeyRegeneration = "pm-12241-private-key-regeneration", EnrollAeadOnKeyRotation = "enroll-aead-on-key-rotation", ForceUpdateKDFSettings = "pm-18021-force-update-kdf-settings", + NoLogoutOnKdfChange = "pm-23995-no-logout-on-kdf-change", /* Tools */ DesktopSendUIRefresh = "desktop-send-ui-refresh", @@ -112,6 +113,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.PrivateKeyRegeneration]: FALSE, [FeatureFlag.EnrollAeadOnKeyRotation]: FALSE, [FeatureFlag.ForceUpdateKDFSettings]: FALSE, + [FeatureFlag.NoLogoutOnKdfChange]: FALSE, /* Platform */ [FeatureFlag.IpcChannelFramework]: FALSE, From 11e716c583538a818c915a98759e2644ec623b7e Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Mon, 29 Sep 2025 23:02:48 +0100 Subject: [PATCH 09/14] updated unit tests --- .../change-kdf-confirmation.component.spec.ts | 244 ++++++------------ .../change-kdf-confirmation.component.ts | 5 +- 2 files changed, 77 insertions(+), 172 deletions(-) diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.spec.ts b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.spec.ts index 9037196d9b5b..b668523a43d3 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.spec.ts +++ b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.spec.ts @@ -3,56 +3,47 @@ import { FormControl } from "@angular/forms"; import { mock, MockProxy } from "jest-mock-extended"; import { of } from "rxjs"; -import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { ChangeKdfService } from "@bitwarden/common/key-management/kdf/change-kdf-service.abstraction"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; -import { - FakeAccountService, - makeEncString, - makeSymmetricCryptoKey, - mockAccountServiceWith, -} from "@bitwarden/common/spec"; +import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; import { UserId } from "@bitwarden/common/types/guid"; -import { MasterKey, UserKey } from "@bitwarden/common/types/key"; import { DIALOG_DATA, DialogRef, ToastService } from "@bitwarden/components"; -import { KdfType, KeyService, PBKDF2KdfConfig, Argon2KdfConfig } from "@bitwarden/key-management"; +import { KdfType, PBKDF2KdfConfig, Argon2KdfConfig } from "@bitwarden/key-management"; import { SharedModule } from "../../shared"; import { ChangeKdfConfirmationComponent } from "./change-kdf-confirmation.component"; -import SpyInstance = jest.SpyInstance; - describe("ChangeKdfConfirmationComponent", () => { let component: ChangeKdfConfirmationComponent; let fixture: ComponentFixture; // Mock Services - let mockApiService: MockProxy; let mockI18nService: MockProxy; - let mockKeyService: MockProxy; let mockMessagingService: MockProxy; let mockToastService: MockProxy; let mockDialogRef: MockProxy>; let mockConfigService: MockProxy; let accountService: FakeAccountService; + let mockChangeKdfService: MockProxy; const mockUserId = "user-id" as UserId; const mockEmail = "email"; const mockMasterPassword = "master-password"; const mockDialogData = jest.fn(); + const kdfConfig = new PBKDF2KdfConfig(600_001); beforeEach(() => { - mockApiService = mock(); mockI18nService = mock(); - mockKeyService = mock(); mockMessagingService = mock(); mockToastService = mock(); mockDialogRef = mock>(); mockConfigService = mock(); accountService = mockAccountServiceWith(mockUserId, { email: mockEmail }); + mockChangeKdfService = mock(); // Mock i18n service mockI18nService.t.mockImplementation((key: string) => { @@ -71,21 +62,20 @@ describe("ChangeKdfConfirmationComponent", () => { mockDialogData.mockReturnValue({ kdf: KdfType.PBKDF2_SHA256, - kdfConfig: new PBKDF2KdfConfig(600_000), + kdfConfig, }); TestBed.configureTestingModule({ declarations: [ChangeKdfConfirmationComponent], imports: [SharedModule], providers: [ - { provide: ApiService, useValue: mockApiService }, { provide: I18nService, useValue: mockI18nService }, - { provide: KeyService, useValue: mockKeyService }, { provide: MessagingService, useValue: mockMessagingService }, { provide: AccountService, useValue: accountService }, { provide: ToastService, useValue: mockToastService }, { provide: DialogRef, useValue: mockDialogRef }, { provide: ConfigService, useValue: mockConfigService }, + { provide: ChangeKdfService, useValue: mockChangeKdfService }, { provide: DIALOG_DATA, useFactory: mockDialogData, @@ -96,11 +86,6 @@ describe("ChangeKdfConfirmationComponent", () => { describe("Component Initialization", () => { it("should create component with PBKDF2 config", () => { - mockDialogData.mockReturnValue({ - kdf: KdfType.PBKDF2_SHA256, - kdfConfig: new PBKDF2KdfConfig(600_001), - }); - const fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); const component = fixture.componentInstance; @@ -154,17 +139,15 @@ describe("ChangeKdfConfirmationComponent", () => { }); describe("submit method", () => { - let makeKeyAndSaveSpy: SpyInstance>; - beforeEach(() => { - fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); - component = fixture.componentInstance; + describe("should not update kdf and not show success toast", () => { + beforeEach(() => { + fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); + component = fixture.componentInstance; - makeKeyAndSaveSpy = jest.spyOn(component, "makeKeyAndSave"); - makeKeyAndSaveSpy.mockImplementation(); - }); + component.form.get("masterPassword")?.setValue(mockMasterPassword); + }); - describe("when form is invalid", () => { - it("should return early without processing", async () => { + it("when form is invalid", async () => { // Arrange component.form.get("masterPassword")?.setValue(""); expect(component.form.invalid).toBe(true); @@ -173,173 +156,98 @@ describe("ChangeKdfConfirmationComponent", () => { await component.submit(); // Assert - expect(makeKeyAndSaveSpy).not.toHaveBeenCalled(); + expect(mockChangeKdfService.updateUserKdfParams).not.toHaveBeenCalled(); }); - }); - describe("when form is valid", () => { - beforeEach(() => { - component.form.get("masterPassword")?.setValue(mockMasterPassword); + it("when no active account", async () => { + accountService.activeAccount$ = of(null); + + await expect(component.submit()).rejects.toThrow("Null or undefined account"); + + expect(mockChangeKdfService.updateUserKdfParams).not.toHaveBeenCalled(); + }); + + it("when kdf is invalid", async () => { + // Arrange + component.kdfConfig = new PBKDF2KdfConfig(1); + + // Act + await expect(component.submit).rejects.toThrow(); + + expect(mockChangeKdfService.updateUserKdfParams).not.toHaveBeenCalled(); }); + }); + describe("should update kdf and show success toast", () => { it("should set loading to true during submission", async () => { // Arrange let loadingDuringExecution = false; - makeKeyAndSaveSpy.mockImplementation(async () => { + mockChangeKdfService.updateUserKdfParams.mockImplementation(async () => { loadingDuringExecution = component.loading; }); - // Act - await component.submit(); + const fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); + const component = fixture.componentInstance; - expect(loadingDuringExecution).toBe(true); - expect(component.loading).toBe(false); - }); + component.form.get("masterPassword")?.setValue(mockMasterPassword); - it("should call makeKeyAndSaveAsync", async () => { // Act await component.submit(); - // Assert - expect(makeKeyAndSaveSpy).toHaveBeenCalledTimes(1); + expect(loadingDuringExecution).toBe(true); + expect(component.loading).toBe(false); }); - describe("when ForceUpdateKDFSettings feature flag is enabled", () => { - it("should show success toast and close dialog", async () => { - // Arrange - reset mocks and create component with feature flag enabled - mockConfigService.getFeatureFlag$.mockReturnValue(of(true)); - - const fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); - const component = fixture.componentInstance; - - makeKeyAndSaveSpy = jest.spyOn(component, "makeKeyAndSave"); - makeKeyAndSaveSpy.mockImplementation(); + it("not logout and close dialog when feature flag is enabled", async () => { + // Arrange + mockConfigService.getFeatureFlag$.mockReturnValue(of(true)); - component.form.get("masterPassword")?.setValue(mockMasterPassword); + const fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); + const component = fixture.componentInstance; - // Act - await component.submit(); + component.form.get("masterPassword")?.setValue(mockMasterPassword); - // Assert - expect(mockToastService.showToast).toHaveBeenCalledWith({ - variant: "success", - message: "Encryption key settings changed", - }); - expect(mockDialogRef.close).toHaveBeenCalled(); - expect(mockMessagingService.send).not.toHaveBeenCalled(); - }); - }); + // Act + await component.submit(); - describe("when ForceUpdateKDFSettings feature flag is disabled", () => { - it("should show toast with logout message and send logout", async () => { - // Act - await component.submit(); - - // Assert - expect(mockToastService.showToast).toHaveBeenCalledWith({ - variant: "success", - title: "Encryption key settings changed", - message: "Please log back in", - }); - expect(mockMessagingService.send).toHaveBeenCalledWith("logout"); - expect(mockDialogRef.close).not.toHaveBeenCalled(); + // Assert + expect(mockChangeKdfService.updateUserKdfParams).toHaveBeenCalledWith( + mockMasterPassword, + kdfConfig, + mockUserId, + ); + expect(mockToastService.showToast).toHaveBeenCalledWith({ + variant: "success", + message: "Encryption key settings changed", }); + expect(mockDialogRef.close).toHaveBeenCalled(); + expect(mockMessagingService.send).not.toHaveBeenCalled(); }); - }); - }); - - describe("makeKeyAndSaveAsync", () => { - const kdfConfig = new PBKDF2KdfConfig(); - const validateKdfConfigForSetting = jest.spyOn(kdfConfig, "validateKdfConfigForSetting"); - const mockMasterKey = makeSymmetricCryptoKey(64) as MasterKey; - const mockNewMasterKey = makeSymmetricCryptoKey(64) as MasterKey; - const mockNewUserKey = makeSymmetricCryptoKey(64) as UserKey; - const mockNewUserKeyEncrypted = makeEncString("user-key"); - const mockMasterPasswordHash = "master-password-hash"; - const mockNewMasterPasswordHash = "new-master-password-hash"; - - beforeEach(() => { - fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); - component = fixture.componentInstance; - - component.kdfConfig = kdfConfig; - - component.form.get("masterPassword")?.setValue(mockMasterPassword); - - mockKeyService.getOrDeriveMasterKey.mockResolvedValue(mockMasterKey); - mockKeyService.hashMasterKey - .mockResolvedValueOnce(mockMasterPasswordHash) - .mockResolvedValueOnce(mockNewMasterPasswordHash); - mockKeyService.makeMasterKey.mockResolvedValue(mockNewMasterKey); - mockKeyService.encryptUserKeyWithMasterKey.mockResolvedValue([ - mockNewUserKey, - mockNewUserKeyEncrypted, - ]); - }); - - it("should throw error when no active account", async () => { - accountService.activeAccount$ = of(null); - - await expect(component.makeKeyAndSave()).rejects.toThrow("No active account found."); - }); - it("should throw error when KDF validation failed", async () => { - validateKdfConfigForSetting.mockImplementation(() => { - throw new Error("KDF config invalid"); - }); - component.kdfConfig = kdfConfig; - - await expect(component.makeKeyAndSave()).rejects.toThrow("KDF config invalid"); - }); - - it.each([new PBKDF2KdfConfig(600_001), new Argon2KdfConfig(4, 65, 5)])( - "should post KDF request to API when kdf = %s", - async (kdfConfig) => { + it("with logout message and send logout when feature flag is enabled", async () => { // Arrange - component.kdfConfig = kdfConfig; - const expectedRequest = { - kdf: kdfConfig.kdfType, - kdfIterations: kdfConfig.iterations, - kdfMemory: kdfConfig instanceof Argon2KdfConfig ? kdfConfig.memory : undefined, - kdfParallelism: kdfConfig instanceof Argon2KdfConfig ? kdfConfig.parallelism : undefined, - masterPasswordHash: mockMasterPasswordHash, - newMasterPasswordHash: mockNewMasterPasswordHash, - key: mockNewUserKeyEncrypted.encryptedString, - }; - if (kdfConfig instanceof PBKDF2KdfConfig) { - delete expectedRequest.kdfMemory; - delete expectedRequest.kdfParallelism; - } + const fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); + const component = fixture.componentInstance; + + component.form.get("masterPassword")?.setValue(mockMasterPassword); // Act - await component.makeKeyAndSave(); + await component.submit(); // Assert - expect(validateKdfConfigForSetting).toHaveBeenCalled(); - expect(mockKeyService.getOrDeriveMasterKey).toHaveBeenCalledWith( - mockMasterPassword, - mockUserId, - ); - expect(mockKeyService.hashMasterKey).toHaveBeenNthCalledWith( - 1, + expect(mockChangeKdfService.updateUserKdfParams).toHaveBeenCalledWith( mockMasterPassword, - mockMasterKey, - ); - expect(mockKeyService.makeMasterKey).toHaveBeenCalledWith( - mockMasterPassword, - mockEmail, kdfConfig, + mockUserId, ); - expect(mockKeyService.hashMasterKey).toHaveBeenNthCalledWith( - 2, - mockMasterPassword, - mockNewMasterKey, - ); - expect(mockKeyService.encryptUserKeyWithMasterKey).toHaveBeenCalledWith(mockNewMasterKey); - expect(mockApiService.postAccountKdf).toHaveBeenCalledWith( - expect.objectContaining(expectedRequest), - ); - }, - ); + expect(mockToastService.showToast).toHaveBeenCalledWith({ + variant: "success", + title: "Encryption key settings changed", + message: "Please log back in", + }); + expect(mockMessagingService.send).toHaveBeenCalledWith("logout"); + expect(mockDialogRef.close).not.toHaveBeenCalled(); + }); + }); }); }); diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts index 418d68092c41..d8f93a5eee88 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts +++ b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts @@ -69,11 +69,8 @@ export class ChangeKdfConfirmationComponent { this.loading = false; }; - async makeKeyAndSave() { + private async makeKeyAndSave() { const activeAccountId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); - if (activeAccountId == null) { - throw new Error("No active account found."); - } const masterPassword = this.form.value.masterPassword; From 2b1d0d3a3d493be78cf5e742213535fa64afa492 Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Mon, 29 Sep 2025 23:07:03 +0100 Subject: [PATCH 10/14] revert bad merge Signed-off-by: Maciej Zieniuk --- .../system-notifications/browser-system-notification.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts b/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts index 33e60894a543..b835c711853d 100644 --- a/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts +++ b/apps/browser/src/platform/system-notifications/browser-system-notification.service.ts @@ -70,7 +70,7 @@ export class BrowserSystemNotificationService implements SystemNotificationsServ } async clear(clearInfo: SystemNotificationClearInfo): Promise { - + // eslint-disable-next-line @typescript-eslint/no-floating-promises chrome.notifications.clear(clearInfo.id); } From 40708ea85c25b6a6a7607160b67b0321ce6b90b9 Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Fri, 3 Oct 2025 12:31:26 +0100 Subject: [PATCH 11/14] updated wording, TS strict enabled, use form controls, updated tests --- .../change-kdf-confirmation.component.spec.ts | 26 ++-- .../change-kdf-confirmation.component.ts | 6 +- .../change-kdf/change-kdf.component.spec.ts | 146 ++++++++++-------- .../change-kdf/change-kdf.component.ts | 68 ++++---- apps/web/src/locales/en/messages.json | 2 +- 5 files changed, 130 insertions(+), 118 deletions(-) diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.spec.ts b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.spec.ts index b668523a43d3..f23f0487cf9d 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.spec.ts +++ b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.spec.ts @@ -115,9 +115,9 @@ describe("ChangeKdfConfirmationComponent", () => { const fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); const component = fixture.componentInstance; - expect(component.form.get("masterPassword")?.value).toEqual(null); - expect(component.form.get("masterPassword")).toBeInstanceOf(FormControl); - expect(component.form.get("masterPassword")?.hasError("required")).toBe(true); + expect(component.form.controls.masterPassword).toBeInstanceOf(FormControl); + expect(component.form.controls.masterPassword.value).toEqual(null); + expect(component.form.controls.masterPassword.hasError("required")).toBe(true); }); }); @@ -128,12 +128,12 @@ describe("ChangeKdfConfirmationComponent", () => { }); it("should be invalid when master password is empty", () => { - component.form.get("masterPassword")?.setValue(""); + component.form.controls.masterPassword.setValue(""); expect(component.form.invalid).toBe(true); }); it("should be valid when master password is provided", () => { - component.form.get("masterPassword")?.setValue(mockMasterPassword); + component.form.controls.masterPassword.setValue(mockMasterPassword); expect(component.form.valid).toBe(true); }); }); @@ -144,12 +144,12 @@ describe("ChangeKdfConfirmationComponent", () => { fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); component = fixture.componentInstance; - component.form.get("masterPassword")?.setValue(mockMasterPassword); + component.form.controls.masterPassword.setValue(mockMasterPassword); }); it("when form is invalid", async () => { // Arrange - component.form.get("masterPassword")?.setValue(""); + component.form.controls.masterPassword.setValue(""); expect(component.form.invalid).toBe(true); // Act @@ -172,7 +172,7 @@ describe("ChangeKdfConfirmationComponent", () => { component.kdfConfig = new PBKDF2KdfConfig(1); // Act - await expect(component.submit).rejects.toThrow(); + await expect(component.submit()).rejects.toThrow(); expect(mockChangeKdfService.updateUserKdfParams).not.toHaveBeenCalled(); }); @@ -189,7 +189,7 @@ describe("ChangeKdfConfirmationComponent", () => { const fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); const component = fixture.componentInstance; - component.form.get("masterPassword")?.setValue(mockMasterPassword); + component.form.controls.masterPassword.setValue(mockMasterPassword); // Act await component.submit(); @@ -198,14 +198,14 @@ describe("ChangeKdfConfirmationComponent", () => { expect(component.loading).toBe(false); }); - it("not logout and close dialog when feature flag is enabled", async () => { + it("doesn't logout and closes the dialog when feature flag is enabled", async () => { // Arrange mockConfigService.getFeatureFlag$.mockReturnValue(of(true)); const fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); const component = fixture.componentInstance; - component.form.get("masterPassword")?.setValue(mockMasterPassword); + component.form.controls.masterPassword.setValue(mockMasterPassword); // Act await component.submit(); @@ -224,12 +224,12 @@ describe("ChangeKdfConfirmationComponent", () => { expect(mockMessagingService.send).not.toHaveBeenCalled(); }); - it("with logout message and send logout when feature flag is enabled", async () => { + it("sends a logout and displays a log back in toast when feature flag is disabled", async () => { // Arrange const fixture = TestBed.createComponent(ChangeKdfConfirmationComponent); const component = fixture.componentInstance; - component.form.get("masterPassword")?.setValue(mockMasterPassword); + component.form.controls.masterPassword.setValue(mockMasterPassword); // Act await component.submit(); diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts index d8f93a5eee88..0598c0214467 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts +++ b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts @@ -1,5 +1,3 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { Component, Inject } from "@angular/core"; import { FormGroup, FormControl, Validators } from "@angular/forms"; import { firstValueFrom, Observable } from "rxjs"; @@ -23,7 +21,7 @@ export class ChangeKdfConfirmationComponent { kdfConfig: KdfConfig; form = new FormGroup({ - masterPassword: new FormControl(null, Validators.required), + masterPassword: new FormControl(null, Validators.required), }); showPassword = false; loading = false; @@ -72,7 +70,7 @@ export class ChangeKdfConfirmationComponent { private async makeKeyAndSave() { const activeAccountId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); - const masterPassword = this.form.value.masterPassword; + const masterPassword = this.form.value.masterPassword!; // Ensure the KDF config is valid. this.kdfConfig.validateKdfConfigForSetting(); diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf.component.spec.ts b/apps/web/src/app/key-management/change-kdf/change-kdf.component.spec.ts index 8a03c94ffbb8..9c4b22f3cf0d 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf.component.spec.ts +++ b/apps/web/src/app/key-management/change-kdf/change-kdf.component.spec.ts @@ -1,5 +1,5 @@ import { ComponentFixture, TestBed } from "@angular/core/testing"; -import { FormBuilder, FormGroup } from "@angular/forms"; +import { FormBuilder, FormControl } from "@angular/forms"; import { mock, MockProxy } from "jest-mock-extended"; import { of } from "rxjs"; @@ -35,55 +35,55 @@ describe("ChangeKdfComponent", () => { const mockUserId = "user-id" as UserId; // Helper functions for validation testing - function expectPBKDF2Validation(formGroup: FormGroup): void { - const iterationsControl = formGroup.get("kdfConfig.iterations"); - const memoryControl = formGroup.get("kdfConfig.memory"); - const parallelismControl = formGroup.get("kdfConfig.parallelism"); - + function expectPBKDF2Validation( + iterationsControl: FormControl, + memoryControl: FormControl, + parallelismControl: FormControl, + ) { // Assert current validators state - expect(iterationsControl?.hasError("required")).toBe(false); - expect(iterationsControl?.hasError("min")).toBe(false); - expect(iterationsControl?.hasError("max")).toBe(false); - expect(memoryControl?.validator).toBeNull(); - expect(parallelismControl?.validator).toBeNull(); + expect(iterationsControl.hasError("required")).toBe(false); + expect(iterationsControl.hasError("min")).toBe(false); + expect(iterationsControl.hasError("max")).toBe(false); + expect(memoryControl.validator).toBeNull(); + expect(parallelismControl.validator).toBeNull(); // Test validation boundaries - iterationsControl?.setValue(PBKDF2KdfConfig.ITERATIONS.min - 1); - expect(iterationsControl?.hasError("min")).toBe(true); + iterationsControl.setValue(PBKDF2KdfConfig.ITERATIONS.min - 1); + expect(iterationsControl.hasError("min")).toBe(true); - iterationsControl?.setValue(PBKDF2KdfConfig.ITERATIONS.max + 1); - expect(iterationsControl?.hasError("max")).toBe(true); + iterationsControl.setValue(PBKDF2KdfConfig.ITERATIONS.max + 1); + expect(iterationsControl.hasError("max")).toBe(true); } - function expectArgon2Validation(formGroup: FormGroup): void { - const iterationsControl = formGroup.get("kdfConfig.iterations"); - const memoryControl = formGroup.get("kdfConfig.memory"); - const parallelismControl = formGroup.get("kdfConfig.parallelism"); - + function expectArgon2Validation( + iterationsControl: FormControl, + memoryControl: FormControl, + parallelismControl: FormControl, + ) { // Assert current validators state - expect(iterationsControl?.hasError("required")).toBe(false); - expect(memoryControl?.hasError("required")).toBe(false); - expect(parallelismControl?.hasError("required")).toBe(false); + expect(iterationsControl.hasError("required")).toBe(false); + expect(memoryControl.hasError("required")).toBe(false); + expect(parallelismControl.hasError("required")).toBe(false); // Test validation boundaries - min values - iterationsControl?.setValue(Argon2KdfConfig.ITERATIONS.min - 1); - expect(iterationsControl?.hasError("min")).toBe(true); + iterationsControl.setValue(Argon2KdfConfig.ITERATIONS.min - 1); + expect(iterationsControl.hasError("min")).toBe(true); - memoryControl?.setValue(Argon2KdfConfig.MEMORY.min - 1); - expect(memoryControl?.hasError("min")).toBe(true); + memoryControl.setValue(Argon2KdfConfig.MEMORY.min - 1); + expect(memoryControl.hasError("min")).toBe(true); - parallelismControl?.setValue(Argon2KdfConfig.PARALLELISM.min - 1); - expect(parallelismControl?.hasError("min")).toBe(true); + parallelismControl.setValue(Argon2KdfConfig.PARALLELISM.min - 1); + expect(parallelismControl.hasError("min")).toBe(true); // Test validation boundaries - max values - iterationsControl?.setValue(Argon2KdfConfig.ITERATIONS.max + 1); - expect(iterationsControl?.hasError("max")).toBe(true); + iterationsControl.setValue(Argon2KdfConfig.ITERATIONS.max + 1); + expect(iterationsControl.hasError("max")).toBe(true); - memoryControl?.setValue(Argon2KdfConfig.MEMORY.max + 1); - expect(memoryControl?.hasError("max")).toBe(true); + memoryControl.setValue(Argon2KdfConfig.MEMORY.max + 1); + expect(memoryControl.hasError("max")).toBe(true); - parallelismControl?.setValue(Argon2KdfConfig.PARALLELISM.max + 1); - expect(parallelismControl?.hasError("max")).toBe(true); + parallelismControl.setValue(Argon2KdfConfig.PARALLELISM.max + 1); + expect(parallelismControl.hasError("max")).toBe(true); } beforeEach(() => { @@ -164,14 +164,19 @@ describe("ChangeKdfComponent", () => { const formGroup = component["formGroup"]; // Assert form values - expect(formGroup.get("kdf")?.value).toBe(KdfType.PBKDF2_SHA256); - expect(formGroup.get("kdfConfig.iterations")?.value).toBe(600_000); - expect(formGroup.get("kdfConfig.memory")?.value).toBeNull(); - expect(formGroup.get("kdfConfig.parallelism")?.value).toBeNull(); + expect(formGroup.controls.kdf.value).toBe(KdfType.PBKDF2_SHA256); + const kdfConfigFormGroup = formGroup.controls.kdfConfig; + expect(kdfConfigFormGroup.controls.iterations.value).toBe(600_000); + expect(kdfConfigFormGroup.controls.memory.value).toBeNull(); + expect(kdfConfigFormGroup.controls.parallelism.value).toBeNull(); expect(component.kdfConfig).toEqual(mockPBKDF2Config); // Assert validators - expectPBKDF2Validation(formGroup); + expectPBKDF2Validation( + kdfConfigFormGroup.controls.iterations, + kdfConfigFormGroup.controls.memory, + kdfConfigFormGroup.controls.parallelism, + ); }); }); @@ -190,14 +195,19 @@ describe("ChangeKdfComponent", () => { const formGroup = component["formGroup"]; // Assert form values - expect(formGroup.get("kdf")?.value).toBe(KdfType.Argon2id); - expect(formGroup.get("kdfConfig.iterations")?.value).toBe(3); - expect(formGroup.get("kdfConfig.memory")?.value).toBe(64); - expect(formGroup.get("kdfConfig.parallelism")?.value).toBe(4); + expect(formGroup.controls.kdf.value).toBe(KdfType.Argon2id); + const kdfConfigFormGroup = formGroup.controls.kdfConfig; + expect(kdfConfigFormGroup.controls.iterations.value).toBe(3); + expect(kdfConfigFormGroup.controls.memory.value).toBe(64); + expect(kdfConfigFormGroup.controls.parallelism.value).toBe(4); expect(component.kdfConfig).toEqual(mockArgon2Config); // Assert validators - expectArgon2Validation(formGroup); + expectArgon2Validation( + kdfConfigFormGroup.controls.iterations, + kdfConfigFormGroup.controls.memory, + kdfConfigFormGroup.controls.parallelism, + ); }); }); @@ -250,28 +260,32 @@ describe("ChangeKdfComponent", () => { it("should update form structure and default values when KDF type changes to Argon2id", () => { // Arrange const formGroup = component["formGroup"]; - const kdfControl = formGroup.get("kdf"); // Act - change KDF type to Argon2id - kdfControl?.setValue(KdfType.Argon2id); + formGroup.controls.kdf.setValue(KdfType.Argon2id); // Assert form values update to Argon2id defaults - expect(formGroup.get("kdf")?.value).toBe(KdfType.Argon2id); - expect(formGroup.get("kdfConfig.iterations")?.value).toBe(3); // Argon2id default - expect(formGroup.get("kdfConfig.memory")?.value).toBe(64); // Argon2id default - expect(formGroup.get("kdfConfig.parallelism")?.value).toBe(4); // Argon2id default + expect(formGroup.controls.kdf.value).toBe(KdfType.Argon2id); + const kdfConfigFormGroup = formGroup.controls.kdfConfig; + expect(kdfConfigFormGroup.controls.iterations.value).toBe(3); // Argon2id default + expect(kdfConfigFormGroup.controls.memory.value).toBe(64); // Argon2id default + expect(kdfConfigFormGroup.controls.parallelism.value).toBe(4); // Argon2id default }); it("should update validators when KDF type changes to Argon2id", () => { // Arrange const formGroup = component["formGroup"]; - const kdfControl = formGroup.get("kdf"); // Act - change KDF type to Argon2id - kdfControl?.setValue(KdfType.Argon2id); + formGroup.controls.kdf.setValue(KdfType.Argon2id); // Assert validators update to Argon2id validation rules - expectArgon2Validation(formGroup); + const kdfConfigFormGroup = formGroup.controls.kdfConfig; + expectArgon2Validation( + kdfConfigFormGroup.controls.iterations, + kdfConfigFormGroup.controls.memory, + kdfConfigFormGroup.controls.parallelism, + ); }); }); @@ -289,28 +303,32 @@ describe("ChangeKdfComponent", () => { it("should update form structure and default values when KDF type changes to PBKDF2", () => { // Arrange const formGroup = component["formGroup"]; - const kdfControl = formGroup.get("kdf"); // Act - change KDF type back to PBKDF2 - kdfControl?.setValue(KdfType.PBKDF2_SHA256); + formGroup.controls.kdf.setValue(KdfType.PBKDF2_SHA256); // Assert form values update to PBKDF2 defaults - expect(formGroup.get("kdf")?.value).toBe(KdfType.PBKDF2_SHA256); - expect(formGroup.get("kdfConfig.iterations")?.value).toBe(600_000); // PBKDF2 default - expect(formGroup.get("kdfConfig.memory")?.value).toBeNull(); // PBKDF2 doesn't use memory - expect(formGroup.get("kdfConfig.parallelism")?.value).toBeNull(); // PBKDF2 doesn't use parallelism + expect(formGroup.controls.kdf.value).toBe(KdfType.PBKDF2_SHA256); + const kdfConfigFormGroup = formGroup.controls.kdfConfig; + expect(kdfConfigFormGroup.controls.iterations.value).toBe(600_000); // PBKDF2 default + expect(kdfConfigFormGroup.controls.memory.value).toBeNull(); // PBKDF2 doesn't use memory + expect(kdfConfigFormGroup.controls.parallelism.value).toBeNull(); // PBKDF2 doesn't use parallelism }); it("should update validators when KDF type changes to PBKDF2", () => { // Arrange const formGroup = component["formGroup"]; - const kdfControl = formGroup.get("kdf"); // Act - change KDF type back to PBKDF2 - kdfControl?.setValue(KdfType.PBKDF2_SHA256); + formGroup.controls.kdf.setValue(KdfType.PBKDF2_SHA256); // Assert validators update to PBKDF2 validation rules - expectPBKDF2Validation(formGroup); + const kdfConfigFormGroup = formGroup.controls.kdfConfig; + expectPBKDF2Validation( + kdfConfigFormGroup.controls.iterations, + kdfConfigFormGroup.controls.memory, + kdfConfigFormGroup.controls.parallelism, + ); }); }); }); @@ -362,9 +380,7 @@ describe("ChangeKdfComponent", () => { }), ); }); - }); - describe("when form is invalid", () => { it("should not open modal when form is invalid", async () => { // Arrange const mockPBKDF2Config = new PBKDF2KdfConfig(PBKDF2KdfConfig.ITERATIONS.min - 1); diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf.component.ts b/apps/web/src/app/key-management/change-kdf/change-kdf.component.ts index 70d74c7b9fc0..0dce72902ef0 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf.component.ts +++ b/apps/web/src/app/key-management/change-kdf/change-kdf.component.ts @@ -1,7 +1,5 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { Component, OnDestroy, OnInit } from "@angular/core"; -import { FormBuilder, FormControl, ValidatorFn, Validators } from "@angular/forms"; +import { FormBuilder, FormControl, Validators } from "@angular/forms"; import { Subject, firstValueFrom, takeUntil, Observable } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; @@ -31,11 +29,11 @@ export class ChangeKdfComponent implements OnInit, OnDestroy { private destroy$ = new Subject(); protected formGroup = this.formBuilder.group({ - kdf: new FormControl(KdfType.PBKDF2_SHA256, [Validators.required]), + kdf: new FormControl(KdfType.PBKDF2_SHA256, [Validators.required]), kdfConfig: this.formBuilder.group({ - iterations: [null as number], - memory: [null as number], - parallelism: [null as number], + iterations: new FormControl(null), + memory: new FormControl(null), + parallelism: new FormControl(null), }), }); @@ -66,15 +64,14 @@ export class ChangeKdfComponent implements OnInit, OnDestroy { async ngOnInit() { const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); this.kdfConfig = await this.kdfConfigService.getKdfConfig(userId); - this.formGroup.get("kdf").setValue(this.kdfConfig.kdfType); + this.formGroup.controls.kdf.setValue(this.kdfConfig.kdfType); this.setFormControlValues(this.kdfConfig); this.setFormValidators(this.kdfConfig.kdfType); - this.formGroup - .get("kdf") - .valueChanges.pipe(takeUntil(this.destroy$)) - .subscribe((newValue: KdfType) => { - this.updateKdfConfig(newValue); + this.formGroup.controls.kdf.valueChanges + .pipe(takeUntil(this.destroy$)) + .subscribe((newValue) => { + this.updateKdfConfig(newValue!); }); } private updateKdfConfig(newValue: KdfType) { @@ -97,28 +94,29 @@ export class ChangeKdfComponent implements OnInit, OnDestroy { } private setFormValidators(kdfType: KdfType) { + const kdfConfigFormGroup = this.formGroup.controls.kdfConfig; switch (kdfType) { case KdfType.PBKDF2_SHA256: - this.setValidators("kdfConfig.iterations", [ + kdfConfigFormGroup.controls.iterations.setValidators([ Validators.required, Validators.min(PBKDF2KdfConfig.ITERATIONS.min), Validators.max(PBKDF2KdfConfig.ITERATIONS.max), ]); - this.setValidators("kdfConfig.memory", []); - this.setValidators("kdfConfig.parallelism", []); + kdfConfigFormGroup.controls.memory.setValidators([]); + kdfConfigFormGroup.controls.parallelism.setValidators([]); break; case KdfType.Argon2id: - this.setValidators("kdfConfig.iterations", [ + kdfConfigFormGroup.controls.iterations.setValidators([ Validators.required, Validators.min(Argon2KdfConfig.ITERATIONS.min), Validators.max(Argon2KdfConfig.ITERATIONS.max), ]); - this.setValidators("kdfConfig.memory", [ + kdfConfigFormGroup.controls.memory.setValidators([ Validators.required, Validators.min(Argon2KdfConfig.MEMORY.min), Validators.max(Argon2KdfConfig.MEMORY.max), ]); - this.setValidators("kdfConfig.parallelism", [ + kdfConfigFormGroup.controls.parallelism.setValidators([ Validators.required, Validators.min(Argon2KdfConfig.PARALLELISM.min), Validators.max(Argon2KdfConfig.PARALLELISM.max), @@ -127,22 +125,20 @@ export class ChangeKdfComponent implements OnInit, OnDestroy { default: throw new Error("Unknown KDF type."); } + kdfConfigFormGroup.controls.iterations.updateValueAndValidity(); + kdfConfigFormGroup.controls.memory.updateValueAndValidity(); + kdfConfigFormGroup.controls.parallelism.updateValueAndValidity(); } - private setValidators(controlName: string, validators: ValidatorFn[]) { - const control = this.formGroup.get(controlName); - if (control) { - control.setValidators(validators); - control.updateValueAndValidity(); - } - } + private setFormControlValues(kdfConfig: KdfConfig) { - this.formGroup.get("kdfConfig").reset(); + const kdfConfigFormGroup = this.formGroup.controls.kdfConfig; + kdfConfigFormGroup.reset(); if (kdfConfig.kdfType === KdfType.PBKDF2_SHA256) { - this.formGroup.get("kdfConfig.iterations").setValue(kdfConfig.iterations); + kdfConfigFormGroup.controls.iterations.setValue(kdfConfig.iterations); } else if (kdfConfig.kdfType === KdfType.Argon2id) { - this.formGroup.get("kdfConfig.iterations").setValue(kdfConfig.iterations); - this.formGroup.get("kdfConfig.memory").setValue(kdfConfig.memory); - this.formGroup.get("kdfConfig.parallelism").setValue(kdfConfig.parallelism); + kdfConfigFormGroup.controls.iterations.setValue(kdfConfig.iterations); + kdfConfigFormGroup.controls.memory.setValue(kdfConfig.memory); + kdfConfigFormGroup.controls.parallelism.setValue(kdfConfig.parallelism); } } @@ -164,12 +160,14 @@ export class ChangeKdfComponent implements OnInit, OnDestroy { if (this.formGroup.invalid) { return; } + + const kdfConfigFormGroup = this.formGroup.controls.kdfConfig; if (this.kdfConfig.kdfType === KdfType.PBKDF2_SHA256) { - this.kdfConfig.iterations = this.formGroup.get("kdfConfig.iterations").value; + this.kdfConfig.iterations = kdfConfigFormGroup.controls.iterations.value!; } else if (this.kdfConfig.kdfType === KdfType.Argon2id) { - this.kdfConfig.iterations = this.formGroup.get("kdfConfig.iterations").value; - this.kdfConfig.memory = this.formGroup.get("kdfConfig.memory").value; - this.kdfConfig.parallelism = this.formGroup.get("kdfConfig.parallelism").value; + this.kdfConfig.iterations = kdfConfigFormGroup.controls.iterations.value!; + this.kdfConfig.memory = kdfConfigFormGroup.controls.memory.value!; + this.kdfConfig.parallelism = kdfConfigFormGroup.controls.parallelism.value!; } this.dialogService.open(ChangeKdfConfirmationComponent, { data: { diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 292b6f715ae2..ecc1d709c315 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -11660,7 +11660,7 @@ "message": "Algorithm" }, "encryptionKeySettingsHowShouldWeEncryptYourData": { - "message": "Choose how Bitwarden should encrypt your vault data. All options are secure, but stronger methods offer better protection-especially against brute-force attacks. Bitwarden recommend the default setting for most users." + "message": "Choose how Bitwarden should encrypt your vault data. All options are secure, but stronger methods offer better protection - especially against brute-force attacks. Bitwarden recommends the default setting for most users." }, "encryptionKeySettingsIncreaseImproveSecurity": { "message": "Increasing the values above the default will improve security, but your vault may take longer to unlock as a result." From c03fa9271e089e510e7f0926b35944f27b72794a Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Fri, 3 Oct 2025 14:54:14 +0100 Subject: [PATCH 12/14] use localisation for button label --- .../app/key-management/change-kdf/change-kdf.component.html | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf.component.html b/apps/web/src/app/key-management/change-kdf/change-kdf.component.html index 8b8bd603e701..7ac34d7f3e58 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf.component.html +++ b/apps/web/src/app/key-management/change-kdf/change-kdf.component.html @@ -20,8 +20,7 @@

type="button" class="tw-border-none tw-bg-transparent tw-text-primary-600 tw-p-0" [bitPopoverTriggerFor]="algorithmPopover" - aria-label="Open about encryption algorithms popover" - title="Open about encryption algorithms popover" + appA11yTitle="{{ 'encryptionKeySettingsAlgorithmPopoverTitle' | i18n }}" bitLink > From 6f7ca6ab97013c4fa5da58c595be7fa4cbef574a Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Sat, 4 Oct 2025 12:16:46 +0100 Subject: [PATCH 13/14] small margin in confirmation dialog --- .../change-kdf/change-kdf-confirmation.component.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.html b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.html index 11818b5a62aa..88c6c8b9aca9 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.html +++ b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.html @@ -8,7 +8,7 @@ @if (!(noLogoutOnKdfChangeFeatureFlag$ | async)) { {{ "kdfSettingsChangeLogoutWarning" | i18n }} } - + {{ "masterPass" | i18n }}