From 907abc9dae3ecb5b994d9e6d852d60576ec64bdb Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Sun, 30 Mar 2025 15:49:52 -0400 Subject: [PATCH 01/17] Complete feature flag grouping by team (#14054) * Completed feature flag grouping * Added organization of default value section. * Clarified comment. --- libs/common/src/enums/feature-flag.enum.ts | 52 +++++++++++++--------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index c5119cd5206..1907f6539c5 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -2,6 +2,8 @@ * Feature flags. * * Flags MUST be short lived and SHALL be removed once enabled. + * + * Flags should be grouped by team to have visibility of ownership and cleanup. */ export enum FeatureFlag { /* Admin Console Team */ @@ -9,6 +11,11 @@ export enum FeatureFlag { VerifiedSsoDomainEndpoint = "pm-12337-refactor-sso-details-endpoint", LimitItemDeletion = "pm-15493-restrict-item-deletion-to-can-manage-permission", SsoExternalIdVisibility = "pm-18630-sso-external-id-visibility", + AccountDeprovisioningBanner = "pm-17120-account-deprovisioning-admin-console-banner", + + /* Auth */ + PM9112_DeviceApprovalPersistence = "pm-9112-device-approval-persistence", + UnauthenticatedExtensionUIRefresh = "unauth-ui-refresh", /* Autofill */ BlockBrowserInjectionsByDomain = "block-browser-injections-by-domain", @@ -21,6 +28,18 @@ export enum FeatureFlag { NotificationBarAddLoginImprovements = "notification-bar-add-login-improvements", NotificationRefresh = "notification-refresh", UseTreeWalkerApiForPageDetailsCollection = "use-tree-walker-api-for-page-details-collection", + MacOsNativeCredentialSync = "macos-native-credential-sync", + + /* Billing */ + TrialPaymentOptional = "PM-8163-trial-payment", + PM15179_AddExistingOrgsFromProviderPortal = "pm-15179-add-existing-orgs-from-provider-portal", + PM12276_BreadcrumbEventLogs = "pm-12276-breadcrumbing-for-business-features", + PM18794_ProviderPaymentMethod = "pm-18794-provider-payment-method", + + /* Key Management */ + PrivateKeyRegeneration = "pm-12241-private-key-regeneration", + UserKeyRotationV2 = "userkey-rotation-v2", + PM4154_BulkEncryptionService = "PM-4154-bulk-encryption-service", /* Tools */ ItemShare = "item-share", @@ -36,21 +55,7 @@ export enum FeatureFlag { NewDeviceVerificationPermanentDismiss = "new-device-permanent-dismiss", VaultBulkManagementAction = "vault-bulk-management-action", SecurityTasks = "security-tasks", - - /* Auth */ - PM9112_DeviceApprovalPersistence = "pm-9112-device-approval-persistence", - - UserKeyRotationV2 = "userkey-rotation-v2", - PM4154_BulkEncryptionService = "PM-4154-bulk-encryption-service", - UnauthenticatedExtensionUIRefresh = "unauth-ui-refresh", CipherKeyEncryption = "cipher-key-encryption", - TrialPaymentOptional = "PM-8163-trial-payment", - MacOsNativeCredentialSync = "macos-native-credential-sync", - PrivateKeyRegeneration = "pm-12241-private-key-regeneration", - AccountDeprovisioningBanner = "pm-17120-account-deprovisioning-admin-console-banner", - PM15179_AddExistingOrgsFromProviderPortal = "pm-15179-add-existing-orgs-from-provider-portal", - PM12276_BreadcrumbEventLogs = "pm-12276-breadcrumbing-for-business-features", - PM18794_ProviderPaymentMethod = "pm-18794-provider-payment-method", } export type AllowedFeatureFlagTypes = boolean | number | string; @@ -63,6 +68,8 @@ const FALSE = false as boolean; * * DO NOT enable previously disabled flags, REMOVE them instead. * We support true as a value as we prefer flags to "enable" not "disable". + * + * Flags should be grouped by team to have visibility of ownership and cleanup. */ export const DefaultFeatureFlagValue = { /* Admin Console Team */ @@ -70,6 +77,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.VerifiedSsoDomainEndpoint]: FALSE, [FeatureFlag.LimitItemDeletion]: FALSE, [FeatureFlag.SsoExternalIdVisibility]: FALSE, + [FeatureFlag.AccountDeprovisioningBanner]: FALSE, /* Autofill */ [FeatureFlag.BlockBrowserInjectionsByDomain]: FALSE, @@ -82,6 +90,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.NotificationBarAddLoginImprovements]: FALSE, [FeatureFlag.NotificationRefresh]: FALSE, [FeatureFlag.UseTreeWalkerApiForPageDetailsCollection]: FALSE, + [FeatureFlag.MacOsNativeCredentialSync]: FALSE, /* Tools */ [FeatureFlag.ItemShare]: FALSE, @@ -97,21 +106,22 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.NewDeviceVerificationPermanentDismiss]: FALSE, [FeatureFlag.VaultBulkManagementAction]: FALSE, [FeatureFlag.SecurityTasks]: FALSE, + [FeatureFlag.CipherKeyEncryption]: FALSE, /* Auth */ [FeatureFlag.PM9112_DeviceApprovalPersistence]: FALSE, - - [FeatureFlag.UserKeyRotationV2]: FALSE, - [FeatureFlag.PM4154_BulkEncryptionService]: FALSE, [FeatureFlag.UnauthenticatedExtensionUIRefresh]: FALSE, - [FeatureFlag.CipherKeyEncryption]: FALSE, + + /* Billing */ [FeatureFlag.TrialPaymentOptional]: FALSE, - [FeatureFlag.MacOsNativeCredentialSync]: FALSE, - [FeatureFlag.PrivateKeyRegeneration]: FALSE, - [FeatureFlag.AccountDeprovisioningBanner]: FALSE, [FeatureFlag.PM15179_AddExistingOrgsFromProviderPortal]: FALSE, [FeatureFlag.PM12276_BreadcrumbEventLogs]: FALSE, [FeatureFlag.PM18794_ProviderPaymentMethod]: FALSE, + + /* Key Management */ + [FeatureFlag.PrivateKeyRegeneration]: FALSE, + [FeatureFlag.UserKeyRotationV2]: FALSE, + [FeatureFlag.PM4154_BulkEncryptionService]: FALSE, } satisfies Record; export type DefaultFeatureFlagValueType = typeof DefaultFeatureFlagValue; From 56672a3568da7ef0a79de6d18f0ab6a6baab47b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ch=C4=99ci=C5=84ski?= Date: Mon, 31 Mar 2025 12:59:47 +0200 Subject: [PATCH 02/17] [BRE-714] Enhance TestFlight desktop publishing (#13871) * Update TestFlight deployment to use Fastlane for app uploads * Update TestFlight deployment to use Fastlane for app uploads * Fix * Fix create secret for fastlane * Fix create secret for fastlane * Fix create secret for fastlane * Install gsed to use sed on macos runner * Create test file * Fix test * Use actual token * Add TestFlight distribution option for QA testing * Update .github/workflows/build-desktop.yml Co-authored-by: MtnBurrit0 <77340197+mimartin12@users.noreply.github.com> * Add if to secret construction for fastlane --------- Co-authored-by: MtnBurrit0 <77340197+mimartin12@users.noreply.github.com> --- .github/workflows/build-desktop.yml | 42 ++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/.github/workflows/build-desktop.yml b/.github/workflows/build-desktop.yml index 48ecca540e8..72b60da97a1 100644 --- a/.github/workflows/build-desktop.yml +++ b/.github/workflows/build-desktop.yml @@ -33,6 +33,10 @@ on: description: "Custom SDK branch" required: false type: string + testflight_distribute: + description: "Force distribute to TestFlight regardless of branch (useful for QA testing on feature branches)" + type: boolean + default: true defaults: run: @@ -1208,21 +1212,45 @@ jobs: path: apps/desktop/dist/mas-universal/Bitwarden-${{ env._PACKAGE_VERSION }}-universal.pkg if-no-files-found: error + - name: Create secrets for Fastlane + if: | + github.event_name != 'pull_request_target' + && (inputs.testflight_distribute || github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc-desktop') + run: | + brew install gsed + + KEY_WITHOUT_NEWLINES=$(gsed -E ':a;N;$!ba;s/\r{0,1}\n/\\n/g' ~/private_keys/AuthKey_6TV9MKN3GP.p8) + + cat << EOF > ~/secrets/appstoreconnect-fastlane.json + { + "issuer_id": "${{ secrets.APP_STORE_CONNECT_TEAM_ISSUER }}", + "key_id": "6TV9MKN3GP", + "key": "$KEY_WITHOUT_NEWLINES" + } + EOF + - name: Deploy to TestFlight id: testflight-deploy if: | github.event_name != 'pull_request_target' - && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc-desktop') + && (inputs.testflight_distribute || github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc-desktop') env: APP_STORE_CONNECT_TEAM_ISSUER: ${{ secrets.APP_STORE_CONNECT_TEAM_ISSUER }} APP_STORE_CONNECT_AUTH_KEY: 6TV9MKN3GP + BRANCH: ${{ github.ref }} run: | - xcrun altool \ - --upload-app \ - --type macos \ - --file "$(find ./dist/mas-universal/Bitwarden*.pkg)" \ - --apiKey $APP_STORE_CONNECT_AUTH_KEY \ - --apiIssuer $APP_STORE_CONNECT_TEAM_ISSUER + + GIT_CHANGE="$(git show -s --format=%s)" + + BRANCH=$(echo $BRANCH | sed 's/refs\/heads\///') + + CHANGELOG="$BRANCH: $GIT_CHANGE" + + fastlane pilot upload \ + --app_identifier "com.bitwarden.desktop" \ + --changelog "$CHANGELOG" \ + --api_key_path $HOME/secrets/appstoreconnect-fastlane.json \ + --pkg "$(find ./dist/mas-universal/Bitwarden*.pkg)" - name: Post message to a Slack channel id: slack-message From 51bfbcf09056775e60fd24b11e37570e46835e85 Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Mon, 31 Mar 2025 09:11:47 -0400 Subject: [PATCH 03/17] chore(UI Refresh): [PM-19679] Remove unauth-ui-refresh flag from clients * Completed feature flag grouping * Added organization of default value section. * Clarified comment. * Removed flag * Removed merge error that duplicated comment. --- libs/common/src/enums/feature-flag.enum.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 1907f6539c5..798053c09d0 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -15,7 +15,6 @@ export enum FeatureFlag { /* Auth */ PM9112_DeviceApprovalPersistence = "pm-9112-device-approval-persistence", - UnauthenticatedExtensionUIRefresh = "unauth-ui-refresh", /* Autofill */ BlockBrowserInjectionsByDomain = "block-browser-injections-by-domain", @@ -110,7 +109,6 @@ export const DefaultFeatureFlagValue = { /* Auth */ [FeatureFlag.PM9112_DeviceApprovalPersistence]: FALSE, - [FeatureFlag.UnauthenticatedExtensionUIRefresh]: FALSE, /* Billing */ [FeatureFlag.TrialPaymentOptional]: FALSE, From 49924512b87098077440bcd702bd11ef318d19de Mon Sep 17 00:00:00 2001 From: Daniel James Smith <2670567+djsmith85@users.noreply.github.com> Date: Mon, 31 Mar 2025 15:55:20 +0200 Subject: [PATCH 04/17] [PM-19656] Fix zip option not being set correctly after navigating to Admin Console (#14058) * Simplify if to reduce nesting * Start subscribing to changes of the vaultSelector as soon as possible during ngOnInit --------- Co-authored-by: Daniel James Smith --- .../src/components/export.component.ts | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/libs/tools/export/vault-export/vault-export-ui/src/components/export.component.ts b/libs/tools/export/vault-export/vault-export-ui/src/components/export.component.ts index aecc6dcc330..c8efe093762 100644 --- a/libs/tools/export/vault-export/vault-export-ui/src/components/export.component.ts +++ b/libs/tools/export/vault-export/vault-export-ui/src/components/export.component.ts @@ -225,6 +225,20 @@ export class ExportComponent implements OnInit, OnDestroy, AfterViewInit { ), ); + combineLatest([ + this.exportForm.controls.vaultSelector.valueChanges, + this.isExportAttachmentsEnabled$, + ]) + .pipe(takeUntil(this.destroy$)) + .subscribe(([value, isExportAttachmentsEnabled]) => { + this.organizationId = value !== "myVault" ? value : undefined; + + this.formatOptions = this.formatOptions.filter((option) => option.value !== "zip"); + if (value === "myVault" && isExportAttachmentsEnabled) { + this.formatOptions.push({ name: ".zip (with attachments)", value: "zip" }); + } + }); + merge( this.exportForm.get("format").valueChanges, this.exportForm.get("fileEncryptionType").valueChanges, @@ -322,22 +336,6 @@ export class ExportComponent implements OnInit, OnDestroy, AfterViewInit { takeUntil(this.destroy$), ) .subscribe(); - - combineLatest([ - this.exportForm.controls.vaultSelector.valueChanges, - this.isExportAttachmentsEnabled$, - ]) - .pipe(takeUntil(this.destroy$)) - .subscribe(([value, isExportAttachmentsEnabled]) => { - this.organizationId = value !== "myVault" ? value : undefined; - if (value === "myVault" && isExportAttachmentsEnabled) { - if (!this.formatOptions.some((option) => option.value === "zip")) { - this.formatOptions.push({ name: ".zip (with attachments)", value: "zip" }); - } - } else { - this.formatOptions = this.formatOptions.filter((option) => option.value !== "zip"); - } - }); } ngAfterViewInit(): void { From 740d0251b8d57ceab78443f6023e86078ecfa481 Mon Sep 17 00:00:00 2001 From: cyprain-okeke <108260115+cyprain-okeke@users.noreply.github.com> Date: Mon, 31 Mar 2025 15:08:03 +0100 Subject: [PATCH 05/17] [PM-19368]Add new collection from individual vault is not displaying Upgrade option rather than Save (#13965) * Resolve the pop up issue and update button * Rename a method properly --- .../collection-dialog.component.ts | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts b/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts index 88746dc708b..1214c0ca411 100644 --- a/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts +++ b/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts @@ -13,6 +13,8 @@ import { Subject, switchMap, takeUntil, + tap, + filter, } from "rxjs"; import { first } from "rxjs/operators"; @@ -189,10 +191,29 @@ export class CollectionDialogComponent implements OnInit, OnDestroy { this.formGroup.updateValueAndValidity(); } - this.organizationSelected.valueChanges.pipe(takeUntil(this.destroy$)).subscribe((_) => { - this.organizationSelected.markAsTouched(); - this.formGroup.updateValueAndValidity(); - }); + this.organizationSelected.valueChanges + .pipe( + tap((_) => { + if (this.organizationSelected.errors?.cannotCreateCollections) { + this.buttonDisplayName = ButtonType.Upgrade; + } else { + this.buttonDisplayName = ButtonType.Save; + } + }), + filter(() => this.organizationSelected.errors?.cannotCreateCollections), + switchMap((value) => this.findOrganizationById(value)), + takeUntil(this.destroy$), + ) + .subscribe((org) => { + this.orgExceedingCollectionLimit = org; + this.organizationSelected.markAsTouched(); + this.formGroup.updateValueAndValidity(); + }); + } + + async findOrganizationById(orgId: string): Promise { + const organizations = await firstValueFrom(this.organizations$); + return organizations.find((org) => org.id === orgId); } async loadOrg(orgId: string) { From 646c7198aa2bb7953b6ee69bb619acf2ea9d1b8e Mon Sep 17 00:00:00 2001 From: cyprain-okeke <108260115+cyprain-okeke@users.noreply.github.com> Date: Mon, 31 Mar 2025 15:10:36 +0100 Subject: [PATCH 06/17] Changes to add validation (#13762) --- .../app/billing/organizations/change-plan-dialog.component.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/web/src/app/billing/organizations/change-plan-dialog.component.ts b/apps/web/src/app/billing/organizations/change-plan-dialog.component.ts index dc748e9ee41..a11858f3be8 100644 --- a/apps/web/src/app/billing/organizations/change-plan-dialog.component.ts +++ b/apps/web/src/app/billing/organizations/change-plan-dialog.component.ts @@ -733,6 +733,7 @@ export class ChangePlanDialogComponent implements OnInit, OnDestroy { submit = async () => { if (this.taxComponent !== undefined && !this.taxComponent.validate()) { + this.taxComponent.markAllAsTouched(); return; } From 0311681803f4e97d3ceb15c15efcd63655c3b95c Mon Sep 17 00:00:00 2001 From: Daniel James Smith <2670567+djsmith85@users.noreply.github.com> Date: Mon, 31 Mar 2025 16:55:04 +0200 Subject: [PATCH 07/17] Fix filename not including "_encrypted_" when selecting encrypted vault exports (#14066) Co-authored-by: Daniel James Smith --- .../src/services/individual-vault-export.service.ts | 2 +- .../vault-export-core/src/services/org-vault-export.service.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/tools/export/vault-export/vault-export-core/src/services/individual-vault-export.service.ts b/libs/tools/export/vault-export/vault-export-core/src/services/individual-vault-export.service.ts index 765de042d32..489a28b4c79 100644 --- a/libs/tools/export/vault-export/vault-export-core/src/services/individual-vault-export.service.ts +++ b/libs/tools/export/vault-export/vault-export-core/src/services/individual-vault-export.service.ts @@ -250,7 +250,7 @@ export class IndividualVaultExportService return { type: "text/plain", data: JSON.stringify(jsonDoc, null, " "), - fileName: ExportHelper.getFileName("", "json"), + fileName: ExportHelper.getFileName("", "encrypted_json"), } as ExportedVaultAsString; } diff --git a/libs/tools/export/vault-export/vault-export-core/src/services/org-vault-export.service.ts b/libs/tools/export/vault-export/vault-export-core/src/services/org-vault-export.service.ts index f9ecd778c23..8f6494edb70 100644 --- a/libs/tools/export/vault-export/vault-export-core/src/services/org-vault-export.service.ts +++ b/libs/tools/export/vault-export/vault-export-core/src/services/org-vault-export.service.ts @@ -109,7 +109,7 @@ export class OrganizationVaultExportService data: onlyManagedCollections ? await this.getEncryptedManagedExport(organizationId) : await this.getOrganizationEncryptedExport(organizationId), - fileName: ExportHelper.getFileName("org", "json"), + fileName: ExportHelper.getFileName("org", "encrypted_json"), } as ExportedVaultAsString; } From 22039d038d7d88c92247c856f9358fb8e11c018b Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Mon, 31 Mar 2025 16:58:02 +0200 Subject: [PATCH 08/17] [PM-3475] Remove deprecated keys (#13266) * Remove deprecated keys * Fix cli build * Fix build --- .../browser/src/background/main.background.ts | 2 - .../service-container/service-container.ts | 2 - .../src/services/jslib-services.module.ts | 2 - .../abstractions/pin.service.abstraction.ts | 13 +- .../pin/pin.service.implementation.ts | 171 ++---------------- .../common/services/pin/pin.service.spec.ts | 164 ++--------------- .../services/master-password.service.ts | 13 -- .../services/vault-timeout.service.ts | 1 - .../platform/abstractions/state.service.ts | 8 - .../src/platform/models/domain/account.ts | 2 - .../src/platform/services/state.service.ts | 41 ----- .../src/abstractions/key.service.ts | 8 - libs/key-management/src/key.service.spec.ts | 8 - libs/key-management/src/key.service.ts | 24 --- 14 files changed, 32 insertions(+), 427 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index cae554c872c..eec07b5b1ed 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -655,9 +655,7 @@ export default class MainBackground { this.kdfConfigService, this.keyGenerationService, this.logService, - this.masterPasswordService, this.stateProvider, - this.stateService, ); this.keyService = new DefaultKeyService( diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index 5bc07f63c32..6a4651bcd5a 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -436,9 +436,7 @@ export class ServiceContainer { this.kdfConfigService, this.keyGenerationService, this.logService, - this.masterPasswordService, this.stateProvider, - this.stateService, ); this.keyService = new KeyService( diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 37220b5195d..6334e5815d6 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -1173,9 +1173,7 @@ const safeProviders: SafeProvider[] = [ KdfConfigService, KeyGenerationServiceAbstraction, LogService, - MasterPasswordServiceAbstraction, StateProvider, - StateServiceAbstraction, ], }), safeProvider({ diff --git a/libs/auth/src/common/abstractions/pin.service.abstraction.ts b/libs/auth/src/common/abstractions/pin.service.abstraction.ts index 0d0f29dff40..16550888b94 100644 --- a/libs/auth/src/common/abstractions/pin.service.abstraction.ts +++ b/libs/auth/src/common/abstractions/pin.service.abstraction.ts @@ -1,4 +1,4 @@ -import { EncString, EncryptedString } from "@bitwarden/common/platform/models/domain/enc-string"; +import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { UserId } from "@bitwarden/common/types/guid"; import { PinKey, UserKey } from "@bitwarden/common/types/key"; import { KdfConfig } from "@bitwarden/key-management"; @@ -90,17 +90,6 @@ export abstract class PinServiceAbstraction { */ abstract clearUserKeyEncryptedPin(userId: UserId): Promise; - /** - * Gets the old MasterKey, encrypted by the PinKey (formerly called `pinProtected`). - * Deprecated and used for migration purposes only. - */ - abstract getOldPinKeyEncryptedMasterKey: (userId: UserId) => Promise; - - /** - * Clears the old MasterKey, encrypted by the PinKey. - */ - abstract clearOldPinKeyEncryptedMasterKey: (userId: UserId) => Promise; - /** * Makes a PinKey from the provided PIN. */ diff --git a/libs/auth/src/common/services/pin/pin.service.implementation.ts b/libs/auth/src/common/services/pin/pin.service.implementation.ts index 0f6ac05f381..99fb725c295 100644 --- a/libs/auth/src/common/services/pin/pin.service.implementation.ts +++ b/libs/auth/src/common/services/pin/pin.service.implementation.ts @@ -4,11 +4,9 @@ import { firstValueFrom, map } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; -import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service"; import { KeyGenerationService } from "@bitwarden/common/platform/abstractions/key-generation.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { EncString, EncryptedString } from "@bitwarden/common/platform/models/domain/enc-string"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { @@ -18,7 +16,7 @@ import { UserKeyDefinition, } from "@bitwarden/common/platform/state"; import { UserId } from "@bitwarden/common/types/guid"; -import { MasterKey, PinKey, UserKey } from "@bitwarden/common/types/key"; +import { PinKey, UserKey } from "@bitwarden/common/types/key"; import { KdfConfig, KdfConfigService } from "@bitwarden/key-management"; import { PinServiceAbstraction } from "../../abstractions/pin.service.abstraction"; @@ -73,19 +71,6 @@ export const USER_KEY_ENCRYPTED_PIN = new UserKeyDefinition( }, ); -/** - * The old MasterKey, encrypted by the PinKey (formerly called `pinProtected`). - * Deprecated and used for migration purposes only. - */ -export const OLD_PIN_KEY_ENCRYPTED_MASTER_KEY = new UserKeyDefinition( - PIN_DISK, - "oldPinKeyEncryptedMasterKey", - { - deserializer: (jsonValue) => jsonValue, - clearOn: ["logout"], - }, -); - export class PinService implements PinServiceAbstraction { constructor( private accountService: AccountService, @@ -94,9 +79,7 @@ export class PinService implements PinServiceAbstraction { private kdfConfigService: KdfConfigService, private keyGenerationService: KeyGenerationService, private logService: LogService, - private masterPasswordService: MasterPasswordServiceAbstraction, private stateProvider: StateProvider, - private stateService: StateService, ) {} async getPinKeyEncryptedUserKeyPersistent(userId: UserId): Promise { @@ -190,9 +173,7 @@ export class PinService implements PinServiceAbstraction { this.accountService.accounts$.pipe(map((accounts) => accounts[userId].email)), ); const kdfConfig = await this.kdfConfigService.getKdfConfig(); - const pinKey = await this.makePinKey(pin, email, kdfConfig); - return await this.encryptService.encrypt(userKey.key, pinKey); } @@ -242,20 +223,6 @@ export class PinService implements PinServiceAbstraction { return await this.encryptService.encrypt(pin, userKey); } - async getOldPinKeyEncryptedMasterKey(userId: UserId): Promise { - this.validateUserId(userId, "Cannot get oldPinKeyEncryptedMasterKey."); - - return await firstValueFrom( - this.stateProvider.getUserState$(OLD_PIN_KEY_ENCRYPTED_MASTER_KEY, userId), - ); - } - - async clearOldPinKeyEncryptedMasterKey(userId: UserId): Promise { - this.validateUserId(userId, "Cannot clear oldPinKeyEncryptedMasterKey."); - - await this.stateProvider.setUserState(OLD_PIN_KEY_ENCRYPTED_MASTER_KEY, null, userId); - } - async makePinKey(pin: string, salt: string, kdfConfig: KdfConfig): Promise { const pinKey = await this.keyGenerationService.deriveKeyFromPassword(pin, salt, kdfConfig); return (await this.keyGenerationService.stretchKey(pinKey)) as PinKey; @@ -264,23 +231,13 @@ export class PinService implements PinServiceAbstraction { async getPinLockType(userId: UserId): Promise { this.validateUserId(userId, "Cannot get PinLockType."); - /** - * We can't check the `userKeyEncryptedPin` (formerly called `protectedPin`) for both because old - * accounts only used it for MP on Restart - */ const aUserKeyEncryptedPinIsSet = !!(await this.getUserKeyEncryptedPin(userId)); const aPinKeyEncryptedUserKeyPersistentIsSet = !!(await this.getPinKeyEncryptedUserKeyPersistent(userId)); - const anOldPinKeyEncryptedMasterKeyIsSet = - !!(await this.getOldPinKeyEncryptedMasterKey(userId)); - if (aPinKeyEncryptedUserKeyPersistentIsSet || anOldPinKeyEncryptedMasterKeyIsSet) { + if (aPinKeyEncryptedUserKeyPersistentIsSet) { return "PERSISTENT"; - } else if ( - aUserKeyEncryptedPinIsSet && - !aPinKeyEncryptedUserKeyPersistentIsSet && - !anOldPinKeyEncryptedMasterKeyIsSet - ) { + } else if (aUserKeyEncryptedPinIsSet && !aPinKeyEncryptedUserKeyPersistentIsSet) { return "EPHEMERAL"; } else { return "DISABLED"; @@ -302,7 +259,7 @@ export class PinService implements PinServiceAbstraction { case "DISABLED": return false; case "PERSISTENT": - // The above getPinLockType call ensures that we have either a PinKeyEncryptedUserKey or OldPinKeyEncryptedMasterKey set. + // The above getPinLockType call ensures that we have either a PinKeyEncryptedUserKey set. return true; case "EPHEMERAL": { // The above getPinLockType call ensures that we have a UserKeyEncryptedPin set. @@ -326,31 +283,21 @@ export class PinService implements PinServiceAbstraction { try { const pinLockType = await this.getPinLockType(userId); - const requireMasterPasswordOnClientRestart = pinLockType === "EPHEMERAL"; - const { pinKeyEncryptedUserKey, oldPinKeyEncryptedMasterKey } = - await this.getPinKeyEncryptedKeys(pinLockType, userId); + const pinKeyEncryptedUserKey = await this.getPinKeyEncryptedKeys(pinLockType, userId); const email = await firstValueFrom( this.accountService.accounts$.pipe(map((accounts) => accounts[userId].email)), ); const kdfConfig = await this.kdfConfigService.getKdfConfig(); - let userKey: UserKey; - - if (oldPinKeyEncryptedMasterKey) { - userKey = await this.decryptAndMigrateOldPinKeyEncryptedMasterKey( - userId, - pin, - email, - kdfConfig, - requireMasterPasswordOnClientRestart, - oldPinKeyEncryptedMasterKey, - ); - } else { - userKey = await this.decryptUserKey(userId, pin, email, kdfConfig, pinKeyEncryptedUserKey); - } - + const userKey: UserKey = await this.decryptUserKey( + userId, + pin, + email, + kdfConfig, + pinKeyEncryptedUserKey, + ); if (!userKey) { this.logService.warning(`User key null after pin key decryption.`); return null; @@ -394,109 +341,23 @@ export class PinService implements PinServiceAbstraction { } /** - * Creates a new `pinKeyEncryptedUserKey` and clears the `oldPinKeyEncryptedMasterKey`. - * @returns UserKey - */ - private async decryptAndMigrateOldPinKeyEncryptedMasterKey( - userId: UserId, - pin: string, - email: string, - kdfConfig: KdfConfig, - requireMasterPasswordOnClientRestart: boolean, - oldPinKeyEncryptedMasterKey: EncString, - ): Promise { - this.validateUserId(userId, "Cannot decrypt and migrate oldPinKeyEncryptedMasterKey."); - - const masterKey = await this.decryptMasterKeyWithPin( - userId, - pin, - email, - kdfConfig, - oldPinKeyEncryptedMasterKey, - ); - - const encUserKey = await this.stateService.getEncryptedCryptoSymmetricKey({ userId: userId }); - - const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey( - masterKey, - userId, - encUserKey ? new EncString(encUserKey) : undefined, - ); - - const pinKeyEncryptedUserKey = await this.createPinKeyEncryptedUserKey(pin, userKey, userId); - await this.storePinKeyEncryptedUserKey( - pinKeyEncryptedUserKey, - requireMasterPasswordOnClientRestart, - userId, - ); - - const userKeyEncryptedPin = await this.createUserKeyEncryptedPin(pin, userKey); - await this.setUserKeyEncryptedPin(userKeyEncryptedPin, userId); - - await this.clearOldPinKeyEncryptedMasterKey(userId); - - return userKey; - } - - // Only for migration purposes - private async decryptMasterKeyWithPin( - userId: UserId, - pin: string, - salt: string, - kdfConfig: KdfConfig, - oldPinKeyEncryptedMasterKey?: EncString, - ): Promise { - this.validateUserId(userId, "Cannot decrypt master key with PIN."); - - if (!oldPinKeyEncryptedMasterKey) { - const oldPinKeyEncryptedMasterKeyString = await this.getOldPinKeyEncryptedMasterKey(userId); - - if (oldPinKeyEncryptedMasterKeyString == null) { - throw new Error("No oldPinKeyEncrytedMasterKey found."); - } - - oldPinKeyEncryptedMasterKey = new EncString(oldPinKeyEncryptedMasterKeyString); - } - - const pinKey = await this.makePinKey(pin, salt, kdfConfig); - const masterKey = await this.encryptService.decryptToBytes(oldPinKeyEncryptedMasterKey, pinKey); - - return new SymmetricCryptoKey(masterKey) as MasterKey; - } - - /** - * Gets the user's `pinKeyEncryptedUserKey` (persistent or ephemeral) and `oldPinKeyEncryptedMasterKey` + * Gets the user's `pinKeyEncryptedUserKey` (persistent or ephemeral) * (if one exists) based on the user's PinLockType. * - * @remarks The `oldPinKeyEncryptedMasterKey` (formerly `pinProtected`) is only used for migration and - * will be null for all migrated accounts. * @throws If PinLockType is 'DISABLED' or if userId is not provided */ private async getPinKeyEncryptedKeys( pinLockType: PinLockType, userId: UserId, - ): Promise<{ pinKeyEncryptedUserKey: EncString; oldPinKeyEncryptedMasterKey?: EncString }> { + ): Promise { this.validateUserId(userId, "Cannot get PinKey encrypted keys."); switch (pinLockType) { case "PERSISTENT": { - const pinKeyEncryptedUserKey = await this.getPinKeyEncryptedUserKeyPersistent(userId); - const oldPinKeyEncryptedMasterKey = await this.getOldPinKeyEncryptedMasterKey(userId); - - return { - pinKeyEncryptedUserKey, - oldPinKeyEncryptedMasterKey: oldPinKeyEncryptedMasterKey - ? new EncString(oldPinKeyEncryptedMasterKey) - : undefined, - }; + return await this.getPinKeyEncryptedUserKeyPersistent(userId); } case "EPHEMERAL": { - const pinKeyEncryptedUserKey = await this.getPinKeyEncryptedUserKeyEphemeral(userId); - - return { - pinKeyEncryptedUserKey, - oldPinKeyEncryptedMasterKey: undefined, // Going forward, we only migrate non-ephemeral version - }; + return await this.getPinKeyEncryptedUserKeyEphemeral(userId); } case "DISABLED": throw new Error("Pin is disabled"); diff --git a/libs/auth/src/common/services/pin/pin.service.spec.ts b/libs/auth/src/common/services/pin/pin.service.spec.ts index 794d08b63b2..ebdc36219ef 100644 --- a/libs/auth/src/common/services/pin/pin.service.spec.ts +++ b/libs/auth/src/common/services/pin/pin.service.spec.ts @@ -1,11 +1,9 @@ import { mock } from "jest-mock-extended"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; -import { FakeMasterPasswordService } from "@bitwarden/common/key-management/master-password/services/fake-master-password.service"; import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service"; import { KeyGenerationService } from "@bitwarden/common/platform/abstractions/key-generation.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; @@ -15,14 +13,13 @@ import { mockAccountServiceWith, } from "@bitwarden/common/spec"; import { UserId } from "@bitwarden/common/types/guid"; -import { MasterKey, PinKey, UserKey } from "@bitwarden/common/types/key"; +import { PinKey, UserKey } from "@bitwarden/common/types/key"; import { DEFAULT_KDF_CONFIG, KdfConfigService } from "@bitwarden/key-management"; import { PinService, PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, PIN_KEY_ENCRYPTED_USER_KEY_EPHEMERAL, - OLD_PIN_KEY_ENCRYPTED_MASTER_KEY, USER_KEY_ENCRYPTED_PIN, PinLockType, } from "./pin.service.implementation"; @@ -31,7 +28,6 @@ describe("PinService", () => { let sut: PinService; let accountService: FakeAccountService; - let masterPasswordService: FakeMasterPasswordService; let stateProvider: FakeStateProvider; const cryptoFunctionService = mock(); @@ -39,11 +35,9 @@ describe("PinService", () => { const kdfConfigService = mock(); const keyGenerationService = mock(); const logService = mock(); - const stateService = mock(); const mockUserId = Utils.newGuid() as UserId; const mockUserKey = new SymmetricCryptoKey(randomBytes(64)) as UserKey; - const mockMasterKey = new SymmetricCryptoKey(randomBytes(32)) as MasterKey; const mockPinKey = new SymmetricCryptoKey(randomBytes(32)) as PinKey; const mockUserEmail = "user@example.com"; const mockPin = "1234"; @@ -57,15 +51,10 @@ describe("PinService", () => { "2.fb5kOEZvh9zPABbP8WRmSQ==|Yi6ZAJY+UtqCKMUSqp1ahY9Kf8QuneKXs6BMkpNsakLVOzTYkHHlilyGABMF7GzUO8QHyZi7V/Ovjjg+Naf3Sm8qNhxtDhibITv4k8rDnM0=|TFkq3h2VNTT1z5BFbebm37WYuxyEHXuRo0DZJI7TQnw=", ); - const oldPinKeyEncryptedMasterKeyPostMigration: any = null; - const oldPinKeyEncryptedMasterKeyPreMigrationPersistent = - "2.fb5kOEZvh9zPABbP8WRmSQ==|Yi6ZAJY+UtqCKMUSqp1ahY9Kf8QuneKXs6BMkpNsakLVOzTYkHHlilyGABMF7GzUO8QHyZi7V/Ovjjg+Naf3Sm8qNhxtDhibITv4k8rDnM0=|TFkq3h2VNTT1z5BFbebm37WYuxyEHXuRo0DZJI7TQnw="; - beforeEach(() => { jest.clearAllMocks(); accountService = mockAccountServiceWith(mockUserId, { email: mockUserEmail }); - masterPasswordService = new FakeMasterPasswordService(); stateProvider = new FakeStateProvider(accountService); sut = new PinService( @@ -75,9 +64,7 @@ describe("PinService", () => { kdfConfigService, keyGenerationService, logService, - masterPasswordService, stateProvider, - stateService, ); }); @@ -111,12 +98,6 @@ describe("PinService", () => { await expect(sut.clearUserKeyEncryptedPin(undefined)).rejects.toThrow( "User ID is required. Cannot clear userKeyEncryptedPin.", ); - await expect(sut.getOldPinKeyEncryptedMasterKey(undefined)).rejects.toThrow( - "User ID is required. Cannot get oldPinKeyEncryptedMasterKey.", - ); - await expect(sut.clearOldPinKeyEncryptedMasterKey(undefined)).rejects.toThrow( - "User ID is required. Cannot clear oldPinKeyEncryptedMasterKey.", - ); await expect( sut.createPinKeyEncryptedUserKey(mockPin, mockUserKey, undefined), ).rejects.toThrow("User ID is required. Cannot create pinKeyEncryptedUserKey."); @@ -288,31 +269,6 @@ describe("PinService", () => { }); }); - describe("oldPinKeyEncryptedMasterKey methods", () => { - describe("getOldPinKeyEncryptedMasterKey()", () => { - it("should get the oldPinKeyEncryptedMasterKey of the specified userId", async () => { - await sut.getOldPinKeyEncryptedMasterKey(mockUserId); - - expect(stateProvider.mock.getUserState$).toHaveBeenCalledWith( - OLD_PIN_KEY_ENCRYPTED_MASTER_KEY, - mockUserId, - ); - }); - }); - - describe("clearOldPinKeyEncryptedMasterKey()", () => { - it("should clear the oldPinKeyEncryptedMasterKey of the specified userId", async () => { - await sut.clearOldPinKeyEncryptedMasterKey(mockUserId); - - expect(stateProvider.mock.setUserState).toHaveBeenCalledWith( - OLD_PIN_KEY_ENCRYPTED_MASTER_KEY, - null, - mockUserId, - ); - }); - }); - }); - describe("makePinKey()", () => { it("should make a PinKey", async () => { // Arrange @@ -346,26 +302,10 @@ describe("PinService", () => { expect(result).toBe("PERSISTENT"); }); - it("should return 'PERSISTENT' if an old oldPinKeyEncryptedMasterKey is found", async () => { - // Arrange - sut.getUserKeyEncryptedPin = jest.fn().mockResolvedValue(null); - sut.getPinKeyEncryptedUserKeyPersistent = jest.fn().mockResolvedValue(null); - sut.getOldPinKeyEncryptedMasterKey = jest - .fn() - .mockResolvedValue(oldPinKeyEncryptedMasterKeyPreMigrationPersistent); - - // Act - const result = await sut.getPinLockType(mockUserId); - - // Assert - expect(result).toBe("PERSISTENT"); - }); - - it("should return 'EPHEMERAL' if neither a pinKeyEncryptedUserKey (persistent version) nor an old oldPinKeyEncryptedMasterKey are found, but a userKeyEncryptedPin is found", async () => { + it("should return 'EPHEMERAL' if a pinKeyEncryptedUserKey (persistent version) is not found but a userKeyEncryptedPin is found", async () => { // Arrange sut.getUserKeyEncryptedPin = jest.fn().mockResolvedValue(mockUserKeyEncryptedPin); sut.getPinKeyEncryptedUserKeyPersistent = jest.fn().mockResolvedValue(null); - sut.getOldPinKeyEncryptedMasterKey = jest.fn().mockResolvedValue(null); // Act const result = await sut.getPinLockType(mockUserId); @@ -374,11 +314,10 @@ describe("PinService", () => { expect(result).toBe("EPHEMERAL"); }); - it("should return 'DISABLED' if ALL three of these are NOT found: userKeyEncryptedPin, pinKeyEncryptedUserKey (persistent version), oldPinKeyEncryptedMasterKey", async () => { + it("should return 'DISABLED' if both of these are NOT found: userKeyEncryptedPin, pinKeyEncryptedUserKey (persistent version)", async () => { // Arrange sut.getUserKeyEncryptedPin = jest.fn().mockResolvedValue(null); sut.getPinKeyEncryptedUserKeyPersistent = jest.fn().mockResolvedValue(null); - sut.getOldPinKeyEncryptedMasterKey = jest.fn().mockResolvedValue(null); // Act const result = await sut.getPinLockType(mockUserId); @@ -476,46 +415,20 @@ describe("PinService", () => { }); describe("decryptUserKeyWithPin()", () => { - async function setupDecryptUserKeyWithPinMocks( - pinLockType: PinLockType, - migrationStatus: "PRE" | "POST" = "POST", - ) { + async function setupDecryptUserKeyWithPinMocks(pinLockType: PinLockType) { sut.getPinLockType = jest.fn().mockResolvedValue(pinLockType); - mockPinEncryptedKeyDataByPinLockType(pinLockType, migrationStatus); + mockPinEncryptedKeyDataByPinLockType(pinLockType); kdfConfigService.getKdfConfig.mockResolvedValue(DEFAULT_KDF_CONFIG); - if (pinLockType === "PERSISTENT" && migrationStatus === "PRE") { - await mockDecryptAndMigrateOldPinKeyEncryptedMasterKeyFn(); - } else { - mockDecryptUserKeyFn(); - } + mockDecryptUserKeyFn(); sut.getUserKeyEncryptedPin = jest.fn().mockResolvedValue(mockUserKeyEncryptedPin); encryptService.decryptToUtf8.mockResolvedValue(mockPin); cryptoFunctionService.compareFast.calledWith(mockPin, "1234").mockResolvedValue(true); } - async function mockDecryptAndMigrateOldPinKeyEncryptedMasterKeyFn() { - sut.makePinKey = jest.fn().mockResolvedValue(mockPinKey); - encryptService.decryptToBytes.mockResolvedValue(mockMasterKey.key); - - stateService.getEncryptedCryptoSymmetricKey.mockResolvedValue(mockUserKey.keyB64); - masterPasswordService.mock.decryptUserKeyWithMasterKey.mockResolvedValue(mockUserKey); - - sut.createPinKeyEncryptedUserKey = jest - .fn() - .mockResolvedValue(pinKeyEncryptedUserKeyPersistant); - - await sut.storePinKeyEncryptedUserKey(pinKeyEncryptedUserKeyPersistant, false, mockUserId); - - sut.createUserKeyEncryptedPin = jest.fn().mockResolvedValue(mockUserKeyEncryptedPin); - await sut.setUserKeyEncryptedPin(mockUserKeyEncryptedPin, mockUserId); - - await sut.clearOldPinKeyEncryptedMasterKey(mockUserId); - } - function mockDecryptUserKeyFn() { sut.getPinKeyEncryptedUserKeyPersistent = jest .fn() @@ -524,26 +437,12 @@ describe("PinService", () => { encryptService.decryptToBytes.mockResolvedValue(mockUserKey.key); } - function mockPinEncryptedKeyDataByPinLockType( - pinLockType: PinLockType, - migrationStatus: "PRE" | "POST" = "POST", - ) { + function mockPinEncryptedKeyDataByPinLockType(pinLockType: PinLockType) { switch (pinLockType) { case "PERSISTENT": sut.getPinKeyEncryptedUserKeyPersistent = jest .fn() .mockResolvedValue(pinKeyEncryptedUserKeyPersistant); - - if (migrationStatus === "PRE") { - sut.getOldPinKeyEncryptedMasterKey = jest - .fn() - .mockResolvedValue(oldPinKeyEncryptedMasterKeyPreMigrationPersistent); - } else { - sut.getOldPinKeyEncryptedMasterKey = jest - .fn() - .mockResolvedValue(oldPinKeyEncryptedMasterKeyPostMigration); // null - } - break; case "EPHEMERAL": sut.getPinKeyEncryptedUserKeyEphemeral = jest @@ -557,49 +456,16 @@ describe("PinService", () => { } } - const testCases: { pinLockType: PinLockType; migrationStatus: "PRE" | "POST" }[] = [ - { pinLockType: "PERSISTENT", migrationStatus: "PRE" }, - { pinLockType: "PERSISTENT", migrationStatus: "POST" }, - { pinLockType: "EPHEMERAL", migrationStatus: "POST" }, + const testCases: { pinLockType: PinLockType }[] = [ + { pinLockType: "PERSISTENT" }, + { pinLockType: "EPHEMERAL" }, ]; - testCases.forEach(({ pinLockType, migrationStatus }) => { - describe(`given a ${pinLockType} PIN (${migrationStatus} migration)`, () => { - if (pinLockType === "PERSISTENT" && migrationStatus === "PRE") { - it("should clear the oldPinKeyEncryptedMasterKey from state", async () => { - // Arrange - await setupDecryptUserKeyWithPinMocks(pinLockType, migrationStatus); - - // Act - await sut.decryptUserKeyWithPin(mockPin, mockUserId); - - // Assert - expect(stateProvider.mock.setUserState).toHaveBeenCalledWith( - OLD_PIN_KEY_ENCRYPTED_MASTER_KEY, - null, - mockUserId, - ); - }); - - it("should set the new pinKeyEncrypterUserKeyPersistent to state", async () => { - // Arrange - await setupDecryptUserKeyWithPinMocks(pinLockType, migrationStatus); - - // Act - await sut.decryptUserKeyWithPin(mockPin, mockUserId); - - // Assert - expect(stateProvider.mock.setUserState).toHaveBeenCalledWith( - PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, - pinKeyEncryptedUserKeyPersistant.encryptedString, - mockUserId, - ); - }); - } - + testCases.forEach(({ pinLockType }) => { + describe(`given a ${pinLockType} PIN)`, () => { it(`should successfully decrypt and return user key when using a valid PIN`, async () => { // Arrange - await setupDecryptUserKeyWithPinMocks(pinLockType, migrationStatus); + await setupDecryptUserKeyWithPinMocks(pinLockType); // Act const result = await sut.decryptUserKeyWithPin(mockPin, mockUserId); @@ -610,7 +476,7 @@ describe("PinService", () => { it(`should return null when PIN is incorrect and user key cannot be decrypted`, async () => { // Arrange - await setupDecryptUserKeyWithPinMocks(pinLockType, migrationStatus); + await setupDecryptUserKeyWithPinMocks(pinLockType); sut.decryptUserKeyWithPin = jest.fn().mockResolvedValue(null); // Act @@ -623,7 +489,7 @@ describe("PinService", () => { // not sure if this is a realistic scenario but going to test it anyway it(`should return null when PIN doesn't match after successful user key decryption`, async () => { // Arrange - await setupDecryptUserKeyWithPinMocks(pinLockType, migrationStatus); + await setupDecryptUserKeyWithPinMocks(pinLockType); encryptService.decryptToUtf8.mockResolvedValue("9999"); // non matching PIN // Act diff --git a/libs/common/src/key-management/master-password/services/master-password.service.ts b/libs/common/src/key-management/master-password/services/master-password.service.ts index 72987b13827..9ed01cf0c83 100644 --- a/libs/common/src/key-management/master-password/services/master-password.service.ts +++ b/libs/common/src/key-management/master-password/services/master-password.service.ts @@ -163,19 +163,6 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr throw new Error("No master key found."); } - // Try one more way to get the user key if it still wasn't found. - if (userKey == null) { - const deprecatedKey = await this.stateService.getEncryptedCryptoSymmetricKey({ - userId: userId, - }); - - if (deprecatedKey == null) { - throw new Error("No encrypted user key found."); - } - - userKey = new EncString(deprecatedKey); - } - let decUserKey: Uint8Array; if (userKey.encryptionType === EncryptionType.AesCbc256_B64) { diff --git a/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.ts b/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.ts index 1762b9156db..d71b8972727 100644 --- a/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.ts +++ b/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.ts @@ -147,7 +147,6 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { await this.masterPasswordService.clearMasterKey(lockingUserId); await this.stateService.setUserKeyAutoUnlock(null, { userId: lockingUserId }); - await this.stateService.setCryptoMasterKeyAuto(null, { userId: lockingUserId }); await this.cipherService.clearCache(lockingUserId); diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index 3b0ce07623b..e4dbe76d7e4 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -50,14 +50,6 @@ export abstract class StateService { value: boolean, options?: StorageOptions, ) => Promise; - /** - * @deprecated For migration purposes only, use getUserKeyMasterKey instead - */ - getEncryptedCryptoSymmetricKey: (options?: StorageOptions) => Promise; - /** - * @deprecated For migration purposes only, use setUserKeyAuto instead - */ - setCryptoMasterKeyAuto: (value: string | null, options?: StorageOptions) => Promise; getDuckDuckGoSharedKey: (options?: StorageOptions) => Promise; setDuckDuckGoSharedKey: (value: string, options?: StorageOptions) => Promise; diff --git a/libs/common/src/platform/models/domain/account.ts b/libs/common/src/platform/models/domain/account.ts index 9873e5c8574..b9d10f47e97 100644 --- a/libs/common/src/platform/models/domain/account.ts +++ b/libs/common/src/platform/models/domain/account.ts @@ -48,8 +48,6 @@ export class EncryptionPair { export class AccountKeys { publicKey?: Uint8Array; - /** @deprecated July 2023, left for migration purposes*/ - cryptoMasterKeyAuto?: string; /** @deprecated July 2023, left for migration purposes*/ cryptoSymmetricKey?: EncryptionPair = new EncryptionPair< string, diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index a78a9b37a8c..284c8a7f2dc 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -222,45 +222,6 @@ export class StateService< await this.saveSecureStorageKey(partialKeys.userBiometricKey, value, options); } - /** - * @deprecated Use UserKeyAuto instead - */ - async setCryptoMasterKeyAuto(value: string | null, options?: StorageOptions): Promise { - options = this.reconcileOptions( - this.reconcileOptions(options, { keySuffix: "auto" }), - await this.defaultSecureStorageOptions(), - ); - if (options?.userId == null) { - return; - } - await this.saveSecureStorageKey(partialKeys.autoKey, value, options); - } - - /** - * @deprecated I don't see where this is even used - */ - async getCryptoMasterKeyB64(options?: StorageOptions): Promise { - options = this.reconcileOptions(options, await this.defaultSecureStorageOptions()); - if (options?.userId == null) { - return null; - } - return await this.secureStorageService.get( - `${options?.userId}${partialKeys.masterKey}`, - options, - ); - } - - /** - * @deprecated I don't see where this is even used - */ - async setCryptoMasterKeyB64(value: string, options?: StorageOptions): Promise { - options = this.reconcileOptions(options, await this.defaultSecureStorageOptions()); - if (options?.userId == null) { - return; - } - await this.saveSecureStorageKey(partialKeys.masterKey, value, options); - } - async getDuckDuckGoSharedKey(options?: StorageOptions): Promise { options = this.reconcileOptions(options, await this.defaultSecureStorageOptions()); if (options?.userId == null) { @@ -619,8 +580,6 @@ export class StateService< await this.setUserKeyAutoUnlock(null, { userId: userId }); await this.setUserKeyBiometric(null, { userId: userId }); - await this.setCryptoMasterKeyAuto(null, { userId: userId }); - await this.setCryptoMasterKeyB64(null, { userId: userId }); } protected async removeAccountFromMemory(userId: string = null): Promise { diff --git a/libs/key-management/src/abstractions/key.service.ts b/libs/key-management/src/abstractions/key.service.ts index 659dd1bbb29..4b257fbbebd 100644 --- a/libs/key-management/src/abstractions/key.service.ts +++ b/libs/key-management/src/abstractions/key.service.ts @@ -390,14 +390,6 @@ export abstract class KeyService { publicKey: string; privateKey: EncString; }>; - /** - * Previously, the master key was used for any additional key like the biometrics or pin key. - * We have switched to using the user key for these purposes. This method is for clearing the state - * of the older keys on logout or post migration. - * @param keySuffix The desired type of key to clear - * @param userId The desired user - */ - abstract clearDeprecatedKeys(keySuffix: KeySuffixOptions, userId?: string): Promise; /** * Retrieves all the keys needed for decrypting Ciphers diff --git a/libs/key-management/src/key.service.spec.ts b/libs/key-management/src/key.service.spec.ts index 2eab9de7487..a0afc160794 100644 --- a/libs/key-management/src/key.service.spec.ts +++ b/libs/key-management/src/key.service.spec.ts @@ -252,14 +252,6 @@ describe("keyService", () => { userId: mockUserId, }); }); - - it("clears the old deprecated Auto key whenever a User Key is set", async () => { - await keyService.setUserKey(mockUserKey, mockUserId); - - expect(stateService.setCryptoMasterKeyAuto).toHaveBeenCalledWith(null, { - userId: mockUserId, - }); - }); }); it("throws if key is null", async () => { diff --git a/libs/key-management/src/key.service.ts b/libs/key-management/src/key.service.ts index b18405a4200..870513f3913 100644 --- a/libs/key-management/src/key.service.ts +++ b/libs/key-management/src/key.service.ts @@ -254,16 +254,10 @@ export class DefaultKeyService implements KeyServiceAbstraction { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises this.stateService.setUserKeyAutoUnlock(null, { userId: userId }); - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.clearDeprecatedKeys(KeySuffixOptions.Auto, userId); } if (keySuffix === KeySuffixOptions.Pin && userId != null) { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. this.pinService.clearPinKeyEncryptedUserKeyEphemeral(userId); - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.clearDeprecatedKeys(KeySuffixOptions.Pin, userId); } } @@ -565,7 +559,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { await this.pinService.clearPinKeyEncryptedUserKeyPersistent(userId); await this.pinService.clearPinKeyEncryptedUserKeyEphemeral(userId); await this.pinService.clearUserKeyEncryptedPin(userId); - await this.clearDeprecatedKeys(KeySuffixOptions.Pin, userId); } async makeSendKey(keyMaterial: CsprngArray): Promise { @@ -726,7 +719,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { } else { await this.stateService.setUserKeyAutoUnlock(null, { userId: userId }); } - await this.clearDeprecatedKeys(KeySuffixOptions.Auto, userId); const storePin = await this.shouldStoreKey(KeySuffixOptions.Pin, userId); if (storePin) { @@ -749,9 +741,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { noPreExistingPersistentKey, userId, ); - // We can't always clear deprecated keys because the pin is only - // migrated once used to unlock - await this.clearDeprecatedKeys(KeySuffixOptions.Pin, userId); } else { await this.pinService.clearPinKeyEncryptedUserKeyPersistent(userId); await this.pinService.clearPinKeyEncryptedUserKeyEphemeral(userId); @@ -835,19 +824,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { return [new SymmetricCryptoKey(newSymKey) as T, protectedSymKey]; } - // --LEGACY METHODS-- - // We previously used the master key for additional keys, but now we use the user key. - // These methods support migrating the old keys to the new ones. - // TODO: Remove after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3475) - - async clearDeprecatedKeys(keySuffix: KeySuffixOptions, userId?: UserId) { - if (keySuffix === KeySuffixOptions.Auto) { - await this.stateService.setCryptoMasterKeyAuto(null, { userId: userId }); - } else if (keySuffix === KeySuffixOptions.Pin && userId != null) { - await this.pinService.clearOldPinKeyEncryptedMasterKey(userId); - } - } - userKey$(userId: UserId): Observable { return this.stateProvider.getUser(userId, USER_KEY).state$; } From 15738f16aee4d23e1e32d3c4634340b3bb8be46c Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Mon, 31 Mar 2025 16:59:01 +0200 Subject: [PATCH 09/17] [PM-18038] Fix safari using outdated biometrics protocol (#13287) * Fix safari using outdated biometrics protocol * Remove logging * Remove log * Move canEnableBiometricUnlock to biometric service * Fix build * Add tests * Fix type error * Attempt to fix build * Fix build * Fix test failure --- .../settings/account-security.component.ts | 13 ++-- .../browser/src/background/main.background.ts | 17 +++--- .../background/nativeMessaging.background.ts | 17 ++++-- .../src/background/runtime.background.ts | 4 ++ ...kground-browser-biometrics.service.spec.ts | 61 +++++++++++++++++++ .../background-browser-biometrics.service.ts | 12 ++++ .../foreground-browser-biometrics.spec.ts | 60 ++++++++++++++++++ .../foreground-browser-biometrics.ts | 20 ++++++ .../src/popup/services/services.module.ts | 6 +- .../safari/SafariWebExtensionHandler.swift | 8 +-- .../key-management/cli-biometrics-service.ts | 3 + .../app/accounts/settings.component.spec.ts | 1 + .../src/app/accounts/settings.component.ts | 16 +---- .../biometrics/main-biometrics.service.ts | 4 ++ .../renderer-biometrics.service.spec.ts | 44 +++++++++++++ .../biometrics/renderer-biometrics.service.ts | 9 +++ .../key-management/web-biometric.service.ts | 4 ++ .../src/biometrics/biometric.service.ts | 1 + .../src/biometrics/biometrics-commands.ts | 3 + 19 files changed, 264 insertions(+), 39 deletions(-) create mode 100644 apps/browser/src/key-management/biometrics/background-browser-biometrics.service.spec.ts create mode 100644 apps/browser/src/key-management/biometrics/foreground-browser-biometrics.spec.ts create mode 100644 apps/desktop/src/key-management/biometrics/renderer-biometrics.service.spec.ts diff --git a/apps/browser/src/auth/popup/settings/account-security.component.ts b/apps/browser/src/auth/popup/settings/account-security.component.ts index 75b59b8efdc..66e5b0bb214 100644 --- a/apps/browser/src/auth/popup/settings/account-security.component.ts +++ b/apps/browser/src/auth/popup/settings/account-security.component.ts @@ -233,11 +233,7 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { .pipe( switchMap(async () => { const status = await this.biometricsService.getBiometricsStatusForUser(activeAccount.id); - const biometricSettingAvailable = - !(await BrowserApi.permissionsGranted(["nativeMessaging"])) || - (status !== BiometricsStatus.DesktopDisconnected && - status !== BiometricsStatus.NotEnabledInConnectedDesktopApp) || - (await this.vaultTimeoutSettingsService.isBiometricLockSet()); + const biometricSettingAvailable = await this.biometricsService.canEnableBiometricUnlock(); if (!biometricSettingAvailable) { this.form.controls.biometric.disable({ emitEvent: false }); } else { @@ -256,6 +252,13 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { "biometricsStatusHelptextNotEnabledInDesktop", activeAccount.email, ); + } else if ( + status === BiometricsStatus.HardwareUnavailable && + !biometricSettingAvailable + ) { + this.biometricUnavailabilityReason = this.i18nService.t( + "biometricsStatusHelptextHardwareUnavailable", + ); } else { this.biometricUnavailabilityReason = ""; } diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index eec07b5b1ed..5cc964c2c2d 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -672,14 +672,6 @@ export default class MainBackground { this.kdfConfigService, ); - this.biometricsService = new BackgroundBrowserBiometricsService( - runtimeNativeMessagingBackground, - this.logService, - this.keyService, - this.biometricStateService, - this.messagingService, - ); - this.appIdService = new AppIdService(this.storageService, this.logService); this.userDecryptionOptionsService = new UserDecryptionOptionsService(this.stateProvider); @@ -699,6 +691,15 @@ export default class MainBackground { VaultTimeoutStringType.OnRestart, // default vault timeout ); + this.biometricsService = new BackgroundBrowserBiometricsService( + runtimeNativeMessagingBackground, + this.logService, + this.keyService, + this.biometricStateService, + this.messagingService, + this.vaultTimeoutSettingsService, + ); + this.apiService = new ApiService( this.tokenService, this.platformUtilsService, diff --git a/apps/browser/src/background/nativeMessaging.background.ts b/apps/browser/src/background/nativeMessaging.background.ts index 8100ff3cffa..69521228bc5 100644 --- a/apps/browser/src/background/nativeMessaging.background.ts +++ b/apps/browser/src/background/nativeMessaging.background.ts @@ -120,9 +120,15 @@ export class NativeMessagingBackground { this.connecting = true; const connectedCallback = () => { - this.logService.info( - "[Native Messaging IPC] Connection to Bitwarden Desktop app established!", - ); + if (!this.platformUtilsService.isSafari()) { + this.logService.info( + "[Native Messaging IPC] Connection to Bitwarden Desktop app established!", + ); + } else { + this.logService.info( + "[Native Messaging IPC] Connection to Safari swift module established!", + ); + } this.connected = true; this.connecting = false; resolve(); @@ -131,6 +137,7 @@ export class NativeMessagingBackground { // Safari has a bundled native component which is always available, no need to // check if the desktop app is running. if (this.platformUtilsService.isSafari()) { + this.isConnectedToOutdatedDesktopClient = false; connectedCallback(); } @@ -428,7 +435,9 @@ export class NativeMessagingBackground { } if (this.callbacks.has(messageId)) { - this.callbacks.get(messageId)!.resolver(message); + const callback = this.callbacks!.get(messageId)!; + this.callbacks.delete(messageId); + callback.resolver(message); } else { this.logService.info("[Native Messaging IPC] Received message without a callback", message); } diff --git a/apps/browser/src/background/runtime.background.ts b/apps/browser/src/background/runtime.background.ts index 7db72f38139..d31ccdd97b0 100644 --- a/apps/browser/src/background/runtime.background.ts +++ b/apps/browser/src/background/runtime.background.ts @@ -78,6 +78,7 @@ export default class RuntimeBackground { BiometricsCommands.GetBiometricsStatus, BiometricsCommands.UnlockWithBiometricsForUser, BiometricsCommands.GetBiometricsStatusForUser, + BiometricsCommands.CanEnableBiometricUnlock, "getUseTreeWalkerApiForPageDetailsCollectionFeatureFlag", "getInlineMenuFieldQualificationFeatureFlag", "getUserPremiumStatus", @@ -201,6 +202,9 @@ export default class RuntimeBackground { case BiometricsCommands.GetBiometricsStatusForUser: { return await this.main.biometricsService.getBiometricsStatusForUser(msg.userId); } + case BiometricsCommands.CanEnableBiometricUnlock: { + return await this.main.biometricsService.canEnableBiometricUnlock(); + } case "getUseTreeWalkerApiForPageDetailsCollectionFeatureFlag": { return await this.configService.getFeatureFlag( FeatureFlag.UseTreeWalkerApiForPageDetailsCollection, diff --git a/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.spec.ts b/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.spec.ts new file mode 100644 index 00000000000..4017953ee28 --- /dev/null +++ b/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.spec.ts @@ -0,0 +1,61 @@ +import { mock } from "jest-mock-extended"; + +import { VaultTimeoutSettingsService } from "@bitwarden/common/key-management/vault-timeout"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { KeyService, BiometricStateService, BiometricsStatus } from "@bitwarden/key-management"; + +import { NativeMessagingBackground } from "../../background/nativeMessaging.background"; + +import { BackgroundBrowserBiometricsService } from "./background-browser-biometrics.service"; + +describe("background browser biometrics service tests", function () { + let service: BackgroundBrowserBiometricsService; + + const nativeMessagingBackground = mock(); + const logService = mock(); + const keyService = mock(); + const biometricStateService = mock(); + const messagingService = mock(); + const vaultTimeoutSettingsService = mock(); + + beforeEach(() => { + jest.resetAllMocks(); + service = new BackgroundBrowserBiometricsService( + () => nativeMessagingBackground, + logService, + keyService, + biometricStateService, + messagingService, + vaultTimeoutSettingsService, + ); + }); + + describe("canEnableBiometricUnlock", () => { + const table: [BiometricsStatus, boolean, boolean][] = [ + // status, already enabled, expected + + // if the setting is not already on, it should only be possible to enable it if biometrics are available + [BiometricsStatus.Available, false, true], + [BiometricsStatus.HardwareUnavailable, false, false], + [BiometricsStatus.NotEnabledInConnectedDesktopApp, false, false], + [BiometricsStatus.DesktopDisconnected, false, false], + + // if the setting is already on, it should always be possible to disable it + [BiometricsStatus.Available, true, true], + [BiometricsStatus.HardwareUnavailable, true, true], + [BiometricsStatus.NotEnabledInConnectedDesktopApp, true, true], + [BiometricsStatus.DesktopDisconnected, true, true], + ]; + test.each(table)( + "status: %s, already enabled: %s, expected: %s", + async (status, alreadyEnabled, expected) => { + service.getBiometricsStatus = jest.fn().mockResolvedValue(status); + vaultTimeoutSettingsService.isBiometricLockSet.mockResolvedValue(alreadyEnabled); + const result = await service.canEnableBiometricUnlock(); + + expect(result).toBe(expected); + }, + ); + }); +}); diff --git a/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.ts b/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.ts index 3031134dc34..a8a89d45274 100644 --- a/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.ts +++ b/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.ts @@ -1,5 +1,6 @@ import { Injectable } from "@angular/core"; +import { VaultTimeoutSettingsService } from "@bitwarden/common/key-management/vault-timeout"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; @@ -25,6 +26,7 @@ export class BackgroundBrowserBiometricsService extends BiometricsService { private keyService: KeyService, private biometricStateService: BiometricStateService, private messagingService: MessagingService, + private vaultTimeoutSettingsService: VaultTimeoutSettingsService, ) { super(); } @@ -169,4 +171,14 @@ export class BackgroundBrowserBiometricsService extends BiometricsService { } async setShouldAutopromptNow(value: boolean): Promise {} + async canEnableBiometricUnlock(): Promise { + const status = await this.getBiometricsStatus(); + const isBiometricsAlreadyEnabled = await this.vaultTimeoutSettingsService.isBiometricLockSet(); + const statusAllowsBiometric = + status !== BiometricsStatus.DesktopDisconnected && + status !== BiometricsStatus.NotEnabledInConnectedDesktopApp && + status !== BiometricsStatus.HardwareUnavailable; + + return statusAllowsBiometric || isBiometricsAlreadyEnabled; + } } diff --git a/apps/browser/src/key-management/biometrics/foreground-browser-biometrics.spec.ts b/apps/browser/src/key-management/biometrics/foreground-browser-biometrics.spec.ts new file mode 100644 index 00000000000..672eec8c1fc --- /dev/null +++ b/apps/browser/src/key-management/biometrics/foreground-browser-biometrics.spec.ts @@ -0,0 +1,60 @@ +import { mock } from "jest-mock-extended"; + +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; + +import { BrowserApi } from "../../platform/browser/browser-api"; + +import { ForegroundBrowserBiometricsService } from "./foreground-browser-biometrics"; + +jest.mock("../../platform/browser/browser-api", () => ({ + BrowserApi: { + sendMessageWithResponse: jest.fn(), + permissionsGranted: jest.fn(), + }, +})); + +describe("foreground browser biometrics service tests", function () { + const platformUtilsService = mock(); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + describe("canEnableBiometricUnlock", () => { + const table: [boolean, boolean, boolean, boolean][] = [ + // canEnableBiometricUnlock from background, native permission granted, isSafari, expected + + // needs permission prompt; always allowed + [true, false, false, true], + [false, false, false, true], + + // is safari; depends on the status that the background service reports + [false, false, true, false], + [true, false, true, true], + + // native permissions granted; depends on the status that the background service reports + [false, true, false, false], + [true, true, false, true], + + // should never happen since safari does not use the permissions + [false, true, true, false], + [true, true, true, true], + ]; + test.each(table)( + "canEnableBiometric: %s, native permission granted: %s, isSafari: %s, expected: %s", + async (canEnableBiometricUnlockBackground, granted, isSafari, expected) => { + const service = new ForegroundBrowserBiometricsService(platformUtilsService); + + (BrowserApi.permissionsGranted as jest.Mock).mockResolvedValue(granted); + (BrowserApi.sendMessageWithResponse as jest.Mock).mockResolvedValue({ + result: canEnableBiometricUnlockBackground, + }); + platformUtilsService.isSafari.mockReturnValue(isSafari); + + const result = await service.canEnableBiometricUnlock(); + + expect(result).toBe(expected); + }, + ); + }); +}); diff --git a/apps/browser/src/key-management/biometrics/foreground-browser-biometrics.ts b/apps/browser/src/key-management/biometrics/foreground-browser-biometrics.ts index d248a630cc6..b6e84fee31a 100644 --- a/apps/browser/src/key-management/biometrics/foreground-browser-biometrics.ts +++ b/apps/browser/src/key-management/biometrics/foreground-browser-biometrics.ts @@ -1,3 +1,4 @@ +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { UserId } from "@bitwarden/common/types/guid"; import { UserKey } from "@bitwarden/common/types/key"; @@ -8,6 +9,10 @@ import { BrowserApi } from "../../platform/browser/browser-api"; export class ForegroundBrowserBiometricsService extends BiometricsService { shouldAutopromptNow = true; + constructor(private platformUtilsService: PlatformUtilsService) { + super(); + } + async authenticateWithBiometrics(): Promise { const response = await BrowserApi.sendMessageWithResponse<{ result: boolean; @@ -52,4 +57,19 @@ export class ForegroundBrowserBiometricsService extends BiometricsService { async setShouldAutopromptNow(value: boolean): Promise { this.shouldAutopromptNow = value; } + + async canEnableBiometricUnlock(): Promise { + const needsPermissionPrompt = + !(await BrowserApi.permissionsGranted(["nativeMessaging"])) && + !this.platformUtilsService.isSafari(); + return ( + needsPermissionPrompt || + ( + await BrowserApi.sendMessageWithResponse<{ + result: boolean; + error: string; + }>(BiometricsCommands.CanEnableBiometricUnlock) + ).result + ); + } } diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 42a05f14007..26e84ea964c 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -316,10 +316,8 @@ const safeProviders: SafeProvider[] = [ }), safeProvider({ provide: BiometricsService, - useFactory: () => { - return new ForegroundBrowserBiometricsService(); - }, - deps: [], + useClass: ForegroundBrowserBiometricsService, + deps: [PlatformUtilsService], }), safeProvider({ provide: SyncService, diff --git a/apps/browser/src/safari/safari/SafariWebExtensionHandler.swift b/apps/browser/src/safari/safari/SafariWebExtensionHandler.swift index d4ce360c32a..54e91611325 100644 --- a/apps/browser/src/safari/safari/SafariWebExtensionHandler.swift +++ b/apps/browser/src/safari/safari/SafariWebExtensionHandler.swift @@ -152,7 +152,7 @@ class SafariWebExtensionHandler: NSObject, NSExtensionRequestHandling { response.userInfo = [ SFExtensionMessageKey: [ "message": [ - "command": "biometricUnlock", + "command": "unlockWithBiometricsForUser", "response": false, "timestamp": Int64(NSDate().timeIntervalSince1970 * 1000), "messageId": messageId, @@ -177,7 +177,7 @@ class SafariWebExtensionHandler: NSObject, NSExtensionRequestHandling { response.userInfo = [ SFExtensionMessageKey: [ "message": [ - "command": "biometricUnlock", + "command": "unlockWithBiometricsForUser", "response": false, "timestamp": Int64(NSDate().timeIntervalSince1970 * 1000), "messageId": messageId, @@ -209,7 +209,7 @@ class SafariWebExtensionHandler: NSObject, NSExtensionRequestHandling { response.userInfo = [ SFExtensionMessageKey: [ "message": [ - "command": "biometricUnlock", + "command": "unlockWithBiometricsForUser", "response": true, "timestamp": Int64(NSDate().timeIntervalSince1970 * 1000), "userKeyB64": result!.replacingOccurrences(of: "\"", with: ""), @@ -220,7 +220,7 @@ class SafariWebExtensionHandler: NSObject, NSExtensionRequestHandling { response.userInfo = [ SFExtensionMessageKey: [ "message": [ - "command": "biometricUnlock", + "command": "unlockWithBiometricsForUser", "response": true, "timestamp": Int64(NSDate().timeIntervalSince1970 * 1000), "messageId": messageId, diff --git a/apps/cli/src/key-management/cli-biometrics-service.ts b/apps/cli/src/key-management/cli-biometrics-service.ts index bda8fe82895..b4f802eb053 100644 --- a/apps/cli/src/key-management/cli-biometrics-service.ts +++ b/apps/cli/src/key-management/cli-biometrics-service.ts @@ -24,4 +24,7 @@ export class CliBiometricsService extends BiometricsService { } async setShouldAutopromptNow(value: boolean): Promise {} + async canEnableBiometricUnlock(): Promise { + return false; + } } diff --git a/apps/desktop/src/app/accounts/settings.component.spec.ts b/apps/desktop/src/app/accounts/settings.component.spec.ts index d29147c1823..b05d90b7e1c 100644 --- a/apps/desktop/src/app/accounts/settings.component.spec.ts +++ b/apps/desktop/src/app/accounts/settings.component.spec.ts @@ -248,6 +248,7 @@ describe("SettingsComponent", () => { describe("biometrics enabled", () => { beforeEach(() => { desktopBiometricsService.getBiometricsStatus.mockResolvedValue(BiometricsStatus.Available); + desktopBiometricsService.canEnableBiometricUnlock.mockResolvedValue(true); vaultTimeoutSettingsService.isBiometricLockSet.mockResolvedValue(true); }); diff --git a/apps/desktop/src/app/accounts/settings.component.ts b/apps/desktop/src/app/accounts/settings.component.ts index 20b6d509f4d..a592e542d58 100644 --- a/apps/desktop/src/app/accounts/settings.component.ts +++ b/apps/desktop/src/app/accounts/settings.component.ts @@ -388,24 +388,12 @@ export class SettingsComponent implements OnInit, OnDestroy { } }); - this.supportsBiometric = this.shouldAllowBiometricSetup( - await this.biometricsService.getBiometricsStatus(), - ); + this.supportsBiometric = await this.biometricsService.canEnableBiometricUnlock(); this.timerId = setInterval(async () => { - this.supportsBiometric = this.shouldAllowBiometricSetup( - await this.biometricsService.getBiometricsStatus(), - ); + this.supportsBiometric = await this.biometricsService.canEnableBiometricUnlock(); }, 1000); } - private shouldAllowBiometricSetup(biometricStatus: BiometricsStatus): boolean { - return [ - BiometricsStatus.Available, - BiometricsStatus.AutoSetupNeeded, - BiometricsStatus.ManualSetupNeeded, - ].includes(biometricStatus); - } - async saveVaultTimeout(newValue: VaultTimeout) { if (newValue === VaultTimeoutStringType.Never) { const confirmed = await this.dialogService.openSimpleDialog({ diff --git a/apps/desktop/src/key-management/biometrics/main-biometrics.service.ts b/apps/desktop/src/key-management/biometrics/main-biometrics.service.ts index dd2e15a2fe8..cf80fa5f7f3 100644 --- a/apps/desktop/src/key-management/biometrics/main-biometrics.service.ts +++ b/apps/desktop/src/key-management/biometrics/main-biometrics.service.ts @@ -163,4 +163,8 @@ export class MainBiometricsService extends DesktopBiometricsService { async getShouldAutopromptNow(): Promise { return this.shouldAutoPrompt; } + + async canEnableBiometricUnlock(): Promise { + return true; + } } diff --git a/apps/desktop/src/key-management/biometrics/renderer-biometrics.service.spec.ts b/apps/desktop/src/key-management/biometrics/renderer-biometrics.service.spec.ts new file mode 100644 index 00000000000..7a3f00c7c44 --- /dev/null +++ b/apps/desktop/src/key-management/biometrics/renderer-biometrics.service.spec.ts @@ -0,0 +1,44 @@ +import { BiometricsStatus } from "@bitwarden/key-management"; + +import { RendererBiometricsService } from "./renderer-biometrics.service"; + +describe("renderer biometrics service tests", function () { + beforeEach(() => { + (global as any).ipc = { + keyManagement: { + biometric: { + authenticateWithBiometrics: jest.fn(), + getBiometricsStatus: jest.fn(), + unlockWithBiometricsForUser: jest.fn(), + getBiometricsStatusForUser: jest.fn(), + deleteBiometricUnlockKeyForUser: jest.fn(), + setupBiometrics: jest.fn(), + setClientKeyHalfForUser: jest.fn(), + getShouldAutoprompt: jest.fn(), + setShouldAutoprompt: jest.fn(), + }, + }, + }; + }); + + describe("canEnableBiometricUnlock", () => { + const table: [BiometricsStatus, boolean][] = [ + [BiometricsStatus.Available, true], + [BiometricsStatus.AutoSetupNeeded, true], + [BiometricsStatus.ManualSetupNeeded, true], + + [BiometricsStatus.UnlockNeeded, false], + [BiometricsStatus.HardwareUnavailable, false], + [BiometricsStatus.PlatformUnsupported, false], + [BiometricsStatus.NotEnabledLocally, false], + ]; + test.each(table)("canEnableBiometricUnlock(%s) === %s", async (status, expected) => { + const service = new RendererBiometricsService(); + (global as any).ipc.keyManagement.biometric.getBiometricsStatus.mockResolvedValue(status); + + const result = await service.canEnableBiometricUnlock(); + + expect(result).toBe(expected); + }); + }); +}); diff --git a/apps/desktop/src/key-management/biometrics/renderer-biometrics.service.ts b/apps/desktop/src/key-management/biometrics/renderer-biometrics.service.ts index 2a0b1282778..db17ee480cb 100644 --- a/apps/desktop/src/key-management/biometrics/renderer-biometrics.service.ts +++ b/apps/desktop/src/key-management/biometrics/renderer-biometrics.service.ts @@ -51,4 +51,13 @@ export class RendererBiometricsService extends DesktopBiometricsService { async setShouldAutopromptNow(value: boolean): Promise { return await ipc.keyManagement.biometric.setShouldAutoprompt(value); } + + async canEnableBiometricUnlock(): Promise { + const biometricStatus = await this.getBiometricsStatus(); + return [ + BiometricsStatus.Available, + BiometricsStatus.AutoSetupNeeded, + BiometricsStatus.ManualSetupNeeded, + ].includes(biometricStatus); + } } diff --git a/apps/web/src/app/key-management/web-biometric.service.ts b/apps/web/src/app/key-management/web-biometric.service.ts index 0c58c0da759..64fa0d243cd 100644 --- a/apps/web/src/app/key-management/web-biometric.service.ts +++ b/apps/web/src/app/key-management/web-biometric.service.ts @@ -24,4 +24,8 @@ export class WebBiometricsService extends BiometricsService { } async setShouldAutopromptNow(value: boolean): Promise {} + + async canEnableBiometricUnlock(): Promise { + return false; + } } diff --git a/libs/key-management/src/biometrics/biometric.service.ts b/libs/key-management/src/biometrics/biometric.service.ts index 3543185b632..119d89e0c9f 100644 --- a/libs/key-management/src/biometrics/biometric.service.ts +++ b/libs/key-management/src/biometrics/biometric.service.ts @@ -39,4 +39,5 @@ export abstract class BiometricsService { abstract getShouldAutopromptNow(): Promise; abstract setShouldAutopromptNow(value: boolean): Promise; + abstract canEnableBiometricUnlock(): Promise; } diff --git a/libs/key-management/src/biometrics/biometrics-commands.ts b/libs/key-management/src/biometrics/biometrics-commands.ts index 2309e8d30bc..81f0ea747e4 100644 --- a/libs/key-management/src/biometrics/biometrics-commands.ts +++ b/libs/key-management/src/biometrics/biometrics-commands.ts @@ -8,6 +8,9 @@ export enum BiometricsCommands { /** Get biometric status for a specific user account. This includes both information about availability of cryptographic material (is the user configured for biometric unlock? is a masterpassword unlock needed? But also information about the biometric system's availability in a single status) */ GetBiometricsStatusForUser = "getBiometricsStatusForUser", + /** Checks whether the biometric unlock can be enabled. */ + CanEnableBiometricUnlock = "canEnableBiometricUnlock", + // legacy Unlock = "biometricUnlock", IsAvailable = "biometricUnlockAvailable", From 7992e0247e8de7121f14b8efc7a2ca0aebf65048 Mon Sep 17 00:00:00 2001 From: Daniel James Smith <2670567+djsmith85@users.noreply.github.com> Date: Mon, 31 Mar 2025 17:06:27 +0200 Subject: [PATCH 10/17] Fix wrong file extension being set when exporting with attachments (#14067) Co-authored-by: Daniel James Smith --- .../src/services/individual-vault-export.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/tools/export/vault-export/vault-export-core/src/services/individual-vault-export.service.ts b/libs/tools/export/vault-export/vault-export-core/src/services/individual-vault-export.service.ts index 489a28b4c79..1fcdb84d375 100644 --- a/libs/tools/export/vault-export/vault-export-core/src/services/individual-vault-export.service.ts +++ b/libs/tools/export/vault-export/vault-export-core/src/services/individual-vault-export.service.ts @@ -126,7 +126,7 @@ export class IndividualVaultExportService return { type: "application/zip", data: blobData, - fileName: ExportHelper.getFileName("", "json"), + fileName: ExportHelper.getFileName("", "zip"), } as ExportedVaultAsBlob; } From 2381d6b3cdb729c01d61be3f4229f5b06cadc89b Mon Sep 17 00:00:00 2001 From: Brandon Treston Date: Mon, 31 Mar 2025 11:07:02 -0400 Subject: [PATCH 11/17] migrate provider clients component (#14030) --- .../providers/clients/clients.component.html | 38 +++++++++---------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.html b/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.html index e6b5ae122f3..668d59b8830 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.html +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.html @@ -21,7 +21,7 @@ @@ -31,35 +31,31 @@

{{ "noClientsInList" | i18n }}

- + {{ "name" | i18n }} {{ "numberOfUsers" | i18n }} {{ "billingPlan" | i18n }} - - + {{ row.organizationName }} - + {{ row.userCount }} / {{ row.seats }} - + {{ row.plan }} - - -
+
-
- - - {{ "remove" | i18n }} - -
+ + +
From 78c74f7acddd0ef34f4da4c5a30b8e5c20952fc5 Mon Sep 17 00:00:00 2001 From: Brandon Treston Date: Mon, 31 Mar 2025 11:11:17 -0400 Subject: [PATCH 12/17] fix qa bug (#14047) --- .../app/vault/components/vault-items/vault-items.component.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/web/src/app/vault/components/vault-items/vault-items.component.ts b/apps/web/src/app/vault/components/vault-items/vault-items.component.ts index 2fcaa8d74af..138c7514eb6 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-items.component.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-items.component.ts @@ -138,6 +138,7 @@ export class VaultItemsComponent { return canRestore$; }), + map((canRestore) => canRestore && this.showBulkTrashOptions), ); } From 753875219a80945fb05e869cebbdb44407751170 Mon Sep 17 00:00:00 2001 From: Brandon Treston Date: Mon, 31 Mar 2025 11:12:24 -0400 Subject: [PATCH 13/17] migrate setup-provider.component (#14026) --- .../providers/providers.module.ts | 3 +- .../setup/setup-provider.component.html | 38 +++++++------------ .../setup/setup-provider.component.ts | 6 +++ 3 files changed, 22 insertions(+), 25 deletions(-) diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/providers.module.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/providers.module.ts index 9b5559af946..b8fe60814a8 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/providers.module.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/providers.module.ts @@ -4,7 +4,7 @@ import { NgModule } from "@angular/core"; import { FormsModule } from "@angular/forms"; import { JslibModule } from "@bitwarden/angular/jslib.module"; -import { SearchModule } from "@bitwarden/components"; +import { CardComponent, SearchModule } from "@bitwarden/components"; import { DangerZoneComponent } from "@bitwarden/web-vault/app/auth/settings/account/danger-zone.component"; import { OrganizationPlansComponent } from "@bitwarden/web-vault/app/billing"; import { VerifyBankAccountComponent } from "@bitwarden/web-vault/app/billing/shared/verify-bank-account/verify-bank-account.component"; @@ -51,6 +51,7 @@ import { VerifyRecoverDeleteProviderComponent } from "./verify-recover-delete-pr DangerZoneComponent, ScrollingModule, VerifyBankAccountComponent, + CardComponent, ], declarations: [ AcceptProviderComponent, diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup-provider.component.html b/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup-provider.component.html index a3e2a180c39..1fa2fd3415a 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup-provider.component.html +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup-provider.component.html @@ -1,9 +1,9 @@ -
+
- -

+ +

@@ -11,25 +11,15 @@

-
-
-
-

{{ "setupProvider" | i18n }}

-
-
-

{{ "setupProviderLoginDesc" | i18n }}

-
-
- - {{ "logIn" | i18n }} - -
-
-
-
+
+
+

{{ "setupProvider" | i18n }}

+ +

{{ "setupProviderLoginDesc" | i18n }}

+
+ +
diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup-provider.component.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup-provider.component.ts index 0490b6c6ec0..a41c7cba362 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup-provider.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup-provider.component.ts @@ -1,6 +1,7 @@ import { Component } from "@angular/core"; import { Params } from "@angular/router"; +import { BitwardenLogo } from "@bitwarden/auth/angular"; import { BaseAcceptComponent } from "@bitwarden/web-vault/app/common/base.accept.component"; @Component({ @@ -8,6 +9,7 @@ import { BaseAcceptComponent } from "@bitwarden/web-vault/app/common/base.accept templateUrl: "setup-provider.component.html", }) export class SetupProviderComponent extends BaseAcceptComponent { + protected logo = BitwardenLogo; failedShortMessage = "inviteAcceptFailedShort"; failedMessage = "inviteAcceptFailed"; @@ -20,4 +22,8 @@ export class SetupProviderComponent extends BaseAcceptComponent { async unauthedHandler(qParams: Params) { // Empty } + + login() { + this.router.navigate(["/login"], { queryParams: { email: this.email } }); + } } From 6849d3aa98d6b42b5069bc2fdd9a4ef9f9e02ea1 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Mon, 31 Mar 2025 18:16:11 +0200 Subject: [PATCH 14/17] [PM-2199] Implement userkey rotation for all TDE devices (#13576) * Implement key rotation v2 * Pass through masterpassword hint * Properly split old and new code * Mark legacy rotation as deprecated * Throw when data is null * Cleanup * Add tests * Fix build * Update libs/key-management/src/key.service.spec.ts Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> * Update apps/web/src/app/auth/settings/change-password.component.ts Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> * Add documentation * Centralize loading logic * Add proof-of-concept for tde rotation * Fix build * Only include trusted devices in rotation request * Undo featureflag change * Fix tests * Prettier format * Fix build * Undo changes to migrate legacy component * Address feedback & add tests --------- Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> --- .../request/unlock-data.request.ts | 4 + .../key-rotation/user-key-rotation.service.ts | 16 ++-- .../devices-api.service.abstraction.ts | 6 +- .../request/update-devices-trust.request.ts | 2 +- .../response/protected-device.response.ts | 10 +++ .../devices-api.service.implementation.ts | 8 +- .../device-trust.service.abstraction.ts | 7 ++ .../device-trust.service.implementation.ts | 53 +++++++++-- .../services/device-trust.service.spec.ts | 88 +++++++++++++++++-- 9 files changed, 164 insertions(+), 30 deletions(-) diff --git a/apps/web/src/app/key-management/key-rotation/request/unlock-data.request.ts b/apps/web/src/app/key-management/key-rotation/request/unlock-data.request.ts index 5cdb56a3e23..6faf0c29401 100644 --- a/apps/web/src/app/key-management/key-rotation/request/unlock-data.request.ts +++ b/apps/web/src/app/key-management/key-rotation/request/unlock-data.request.ts @@ -1,4 +1,5 @@ import { OrganizationUserResetPasswordWithIdRequest } from "@bitwarden/admin-console/common"; +import { DeviceKeysUpdateRequest } from "@bitwarden/common/auth/models/request/update-devices-trust.request"; import { WebauthnRotateCredentialRequest } from "@bitwarden/common/auth/models/request/webauthn-rotate-credential.request"; import { EmergencyAccessWithIdRequest } from "../../../auth/emergency-access/request/emergency-access-update.request"; @@ -11,16 +12,19 @@ export class UnlockDataRequest { emergencyAccessUnlockData: EmergencyAccessWithIdRequest[]; organizationAccountRecoveryUnlockData: OrganizationUserResetPasswordWithIdRequest[]; passkeyUnlockData: WebauthnRotateCredentialRequest[]; + deviceKeyUnlockData: DeviceKeysUpdateRequest[]; constructor( masterPasswordUnlockData: MasterPasswordUnlockDataRequest, emergencyAccessUnlockData: EmergencyAccessWithIdRequest[], organizationAccountRecoveryUnlockData: OrganizationUserResetPasswordWithIdRequest[], passkeyUnlockData: WebauthnRotateCredentialRequest[], + deviceTrustUnlockData: DeviceKeysUpdateRequest[], ) { this.masterPasswordUnlockData = masterPasswordUnlockData; this.emergencyAccessUnlockData = emergencyAccessUnlockData; this.organizationAccountRecoveryUnlockData = organizationAccountRecoveryUnlockData; this.passkeyUnlockData = passkeyUnlockData; + this.deviceKeyUnlockData = deviceTrustUnlockData; } } diff --git a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts index c1b7a04d62b..9dc844c0104 100644 --- a/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts +++ b/apps/web/src/app/key-management/key-rotation/user-key-rotation.service.ts @@ -180,11 +180,19 @@ export class UserKeyRotationService { newUnencryptedUserKey, user.id, ); + + const trustedDeviceUnlockData = await this.deviceTrustService.getRotatedData( + originalUserKey, + newUnencryptedUserKey, + user.id, + ); + const unlockDataRequest = new UnlockDataRequest( masterPasswordUnlockData, emergencyAccessUnlockData, organizationAccountRecoveryUnlockData, passkeyUnlockData, + trustedDeviceUnlockData, ); const request = new RotateUserAccountKeysRequest( @@ -198,14 +206,6 @@ export class UserKeyRotationService { await this.apiService.postUserKeyUpdateV2(request); this.logService.info("[Userkey rotation] Userkey rotation request posted to server"); - // TODO PM-2199: Add device trust rotation support to the user key rotation endpoint - this.logService.info("[Userkey rotation] Rotating device trust..."); - await this.deviceTrustService.rotateDevicesTrust( - user.id, - newUnencryptedUserKey, - newMasterKeyAuthenticationHash, - ); - this.logService.info("[Userkey rotation] Device trust rotation completed"); this.toastService.showToast({ variant: "success", title: this.i18nService.t("rotationCompletedTitle"), diff --git a/libs/common/src/auth/abstractions/devices-api.service.abstraction.ts b/libs/common/src/auth/abstractions/devices-api.service.abstraction.ts index 92f0ebf1667..f5d58e3fc6b 100644 --- a/libs/common/src/auth/abstractions/devices-api.service.abstraction.ts +++ b/libs/common/src/auth/abstractions/devices-api.service.abstraction.ts @@ -2,7 +2,6 @@ // @ts-strict-ignore import { ListResponse } from "../../models/response/list.response"; import { DeviceResponse } from "../abstractions/devices/responses/device.response"; -import { SecretVerificationRequest } from "../models/request/secret-verification.request"; import { UpdateDevicesTrustRequest } from "../models/request/update-devices-trust.request"; import { ProtectedDeviceResponse } from "../models/response/protected-device.response"; @@ -25,10 +24,7 @@ export abstract class DevicesApiServiceAbstraction { deviceIdentifier: string, ) => Promise; - getDeviceKeys: ( - deviceIdentifier: string, - secretVerificationRequest: SecretVerificationRequest, - ) => Promise; + getDeviceKeys: (deviceIdentifier: string) => Promise; /** * Notifies the server that the device has a device key, but didn't receive any associated decryption keys. diff --git a/libs/common/src/auth/models/request/update-devices-trust.request.ts b/libs/common/src/auth/models/request/update-devices-trust.request.ts index 21fe0f600dc..f3cee00f948 100644 --- a/libs/common/src/auth/models/request/update-devices-trust.request.ts +++ b/libs/common/src/auth/models/request/update-devices-trust.request.ts @@ -13,5 +13,5 @@ export class DeviceKeysUpdateRequest { } export class OtherDeviceKeysUpdateRequest extends DeviceKeysUpdateRequest { - id: string; + deviceId: string; } diff --git a/libs/common/src/auth/models/response/protected-device.response.ts b/libs/common/src/auth/models/response/protected-device.response.ts index 14662236197..3cc3a3e0792 100644 --- a/libs/common/src/auth/models/response/protected-device.response.ts +++ b/libs/common/src/auth/models/response/protected-device.response.ts @@ -2,6 +2,8 @@ // @ts-strict-ignore import { Jsonify } from "type-fest"; +import { RotateableKeySet } from "@bitwarden/auth/common"; + import { DeviceType } from "../../../enums"; import { BaseResponse } from "../../../models/response/base.response"; import { EncString } from "../../../platform/models/domain/enc-string"; @@ -38,4 +40,12 @@ export class ProtectedDeviceResponse extends BaseResponse { * This enabled a user to rotate the keys for all of their devices. */ encryptedPublicKey: EncString; + + getRotateableKeyset(): RotateableKeySet { + return new RotateableKeySet(this.encryptedUserKey, this.encryptedPublicKey); + } + + isTrusted(): boolean { + return this.encryptedUserKey != null && this.encryptedPublicKey != null; + } } diff --git a/libs/common/src/auth/services/devices-api.service.implementation.ts b/libs/common/src/auth/services/devices-api.service.implementation.ts index cf760effbdf..9830a8c1ffd 100644 --- a/libs/common/src/auth/services/devices-api.service.implementation.ts +++ b/libs/common/src/auth/services/devices-api.service.implementation.ts @@ -5,7 +5,6 @@ import { ListResponse } from "../../models/response/list.response"; import { Utils } from "../../platform/misc/utils"; import { DeviceResponse } from "../abstractions/devices/responses/device.response"; import { DevicesApiServiceAbstraction } from "../abstractions/devices-api.service.abstraction"; -import { SecretVerificationRequest } from "../models/request/secret-verification.request"; import { UpdateDevicesTrustRequest } from "../models/request/update-devices-trust.request"; import { ProtectedDeviceResponse } from "../models/response/protected-device.response"; @@ -90,14 +89,11 @@ export class DevicesApiServiceImplementation implements DevicesApiServiceAbstrac ); } - async getDeviceKeys( - deviceIdentifier: string, - secretVerificationRequest: SecretVerificationRequest, - ): Promise { + async getDeviceKeys(deviceIdentifier: string): Promise { const result = await this.apiService.send( "POST", `/devices/${deviceIdentifier}/retrieve-keys`, - secretVerificationRequest, + null, true, true, ); diff --git a/libs/common/src/key-management/device-trust/abstractions/device-trust.service.abstraction.ts b/libs/common/src/key-management/device-trust/abstractions/device-trust.service.abstraction.ts index 882625fa231..407ae007622 100644 --- a/libs/common/src/key-management/device-trust/abstractions/device-trust.service.abstraction.ts +++ b/libs/common/src/key-management/device-trust/abstractions/device-trust.service.abstraction.ts @@ -2,6 +2,8 @@ // @ts-strict-ignore import { Observable } from "rxjs"; +import { DeviceKeysUpdateRequest } from "@bitwarden/common/auth/models/request/update-devices-trust.request"; + import { DeviceResponse } from "../../../auth/abstractions/devices/responses/device.response"; import { EncString } from "../../../platform/models/domain/enc-string"; import { UserId } from "../../../types/guid"; @@ -55,4 +57,9 @@ export abstract class DeviceTrustServiceAbstraction { * Note: For debugging purposes only. */ recordDeviceTrustLoss: () => Promise; + getRotatedData: ( + oldUserKey: UserKey, + newUserKey: UserKey, + userId: UserId, + ) => Promise; } diff --git a/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts b/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts index 579fe9360a6..501c5cfabc8 100644 --- a/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts +++ b/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts @@ -2,7 +2,7 @@ // @ts-strict-ignore import { firstValueFrom, map, Observable, Subject } from "rxjs"; -import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; +import { RotateableKeySet, UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; import { KeyService } from "@bitwarden/key-management"; import { DeviceResponse } from "../../../auth/abstractions/devices/responses/device.response"; @@ -10,6 +10,7 @@ import { DevicesApiServiceAbstraction } from "../../../auth/abstractions/devices import { SecretVerificationRequest } from "../../../auth/models/request/secret-verification.request"; import { DeviceKeysUpdateRequest, + OtherDeviceKeysUpdateRequest, UpdateDevicesTrustRequest, } from "../../../auth/models/request/update-devices-trust.request"; import { AppIdService } from "../../../platform/abstractions/app-id.service"; @@ -187,6 +188,51 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { return deviceResponse; } + async getRotatedData( + oldUserKey: UserKey, + newUserKey: UserKey, + userId: UserId, + ): Promise { + if (!userId) { + throw new Error("UserId is required. Cannot get rotated data."); + } + if (!oldUserKey) { + throw new Error("Old user key is required. Cannot get rotated data."); + } + if (!newUserKey) { + throw new Error("New user key is required. Cannot get rotated data."); + } + + const devices = await this.devicesApiService.getDevices(); + return await Promise.all( + devices.data + .filter((device) => device.isTrusted) + .map(async (device) => { + const deviceWithKeys = await this.devicesApiService.getDeviceKeys(device.identifier); + const publicKey = await this.encryptService.decryptToBytes( + deviceWithKeys.encryptedPublicKey, + oldUserKey, + ); + const newEncryptedPublicKey = await this.encryptService.encrypt(publicKey, newUserKey); + const newEncryptedUserKey = await this.encryptService.rsaEncrypt( + newUserKey.key, + publicKey, + ); + + const newRotateableKeySet = new RotateableKeySet( + newEncryptedUserKey, + newEncryptedPublicKey, + ); + + const request = new OtherDeviceKeysUpdateRequest(); + request.encryptedPublicKey = newRotateableKeySet.encryptedPublicKey.encryptedString; + request.encryptedUserKey = newRotateableKeySet.encryptedUserKey.encryptedString; + request.deviceId = device.id; + return request; + }), + ); + } + async rotateDevicesTrust( userId: UserId, newUserKey: UserKey, @@ -216,10 +262,7 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { secretVerificationRequest.masterPasswordHash = masterPasswordHash; // Get the keys that are used in rotating a devices keys from the server - const currentDeviceKeys = await this.devicesApiService.getDeviceKeys( - deviceIdentifier, - secretVerificationRequest, - ); + const currentDeviceKeys = await this.devicesApiService.getDeviceKeys(deviceIdentifier); // Decrypt the existing device public key with the old user key const decryptedDevicePublicKey = await this.encryptService.decryptToBytes( diff --git a/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts b/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts index 1893b097ec6..2f3034e67ba 100644 --- a/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts +++ b/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts @@ -7,6 +7,7 @@ import { UserDecryptionOptionsServiceAbstraction, UserDecryptionOptions, } from "@bitwarden/auth/common"; +import { ListResponse } from "@bitwarden/common/models/response/list.response"; import { KeyService } from "@bitwarden/key-management"; import { FakeAccountService, mockAccountServiceWith } from "../../../../spec/fake-account-service"; @@ -655,6 +656,86 @@ describe("deviceTrustService", () => { }); }); + describe("getRotatedData", () => { + let fakeNewUserKey: UserKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey; + let fakeOldUserKey: UserKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey; + const userId: UserId = Utils.newGuid() as UserId; + + it("throws an error when a null user id is passed in", async () => { + await expect( + deviceTrustService.getRotatedData(fakeOldUserKey, fakeNewUserKey, null), + ).rejects.toThrow("UserId is required. Cannot get rotated data."); + }); + + it("throws an error when a null old user key is passed in", async () => { + await expect( + deviceTrustService.getRotatedData(null, fakeNewUserKey, userId), + ).rejects.toThrow("Old user key is required. Cannot get rotated data."); + }); + + it("throws an error when a null new user key is passed in", async () => { + await expect( + deviceTrustService.getRotatedData(fakeOldUserKey, null, userId), + ).rejects.toThrow("New user key is required. Cannot get rotated data."); + }); + + it("returns the expected data when all required parameters are provided", async () => { + const deviceResponse = { + id: "", + userId: "", + name: "", + identifier: "", + type: DeviceType.Android, + creationDate: "", + revisionDate: "", + isTrusted: true, + }; + devicesApiService.getDevices.mockResolvedValue( + new ListResponse( + { + data: [deviceResponse], + }, + DeviceResponse, + ), + ); + encryptService.decryptToBytes.mockResolvedValue(new Uint8Array(64)); + encryptService.encrypt.mockResolvedValue(new EncString("test_encrypted_data")); + encryptService.rsaEncrypt.mockResolvedValue(new EncString("test_encrypted_data")); + + const protectedDeviceResponse = new ProtectedDeviceResponse({ + id: "", + creationDate: "", + identifier: "test_device_identifier", + name: "Firefox", + type: DeviceType.FirefoxBrowser, + encryptedPublicKey: "", + encryptedUserKey: "", + }); + devicesApiService.getDeviceKeys.mockResolvedValue(protectedDeviceResponse); + const fakeOldUserKeyData = new Uint8Array(64); + fakeOldUserKeyData.fill(5, 0, 1); + fakeOldUserKey = new SymmetricCryptoKey(fakeOldUserKeyData) as UserKey; + + const fakeNewUserKeyData = new Uint8Array(64); + fakeNewUserKeyData.fill(1, 0, 1); + fakeNewUserKey = new SymmetricCryptoKey(fakeNewUserKeyData) as UserKey; + + const result = await deviceTrustService.getRotatedData( + fakeOldUserKey, + fakeNewUserKey, + userId, + ); + + expect(result).toEqual([ + { + deviceId: "", + encryptedUserKey: "test_encrypted_data", + encryptedPublicKey: "test_encrypted_data", + }, + ]); + }); + }); + describe("rotateDevicesTrust", () => { let fakeNewUserKey: UserKey = null; @@ -708,11 +789,8 @@ describe("deviceTrustService", () => { appIdService.getAppId.mockResolvedValue("test_device_identifier"); - devicesApiService.getDeviceKeys.mockImplementation((deviceIdentifier, secretRequest) => { - if ( - deviceIdentifier !== "test_device_identifier" || - secretRequest.masterPasswordHash !== "my_password_hash" - ) { + devicesApiService.getDeviceKeys.mockImplementation((deviceIdentifier) => { + if (deviceIdentifier !== "test_device_identifier") { return Promise.resolve(null); } From 4e413283a8fdf7fbef53c2c7bd2fa8fa57449596 Mon Sep 17 00:00:00 2001 From: Miles Blackwood Date: Mon, 31 Mar 2025 12:36:31 -0400 Subject: [PATCH 15/17] [PM-16635] Remove InlineMenuFieldQualification feature flag references. (#13230) * Remove InlineMenuFieldQualification feature flag references. Removes all business logic related to InlineMenuFieldQualification Feature Flag, while leaving the flag in enum itself intact. No corresponding pull on server, where there exists no current references. * Removes the feature flag from enum. * Remove inlineMenuFieldQualificationFlagSet from InlineMenuFieldQualificationService. * Removes deprecated isFieldForLoginFormFallback method. --- ...e-menu-field-qualification.service.spec.ts | 1 - ...inline-menu-field-qualification.service.ts | 20 ------------------- .../src/background/runtime.background.ts | 4 ---- libs/common/src/enums/feature-flag.enum.ts | 2 -- 4 files changed, 27 deletions(-) diff --git a/apps/browser/src/autofill/services/inline-menu-field-qualification.service.spec.ts b/apps/browser/src/autofill/services/inline-menu-field-qualification.service.spec.ts index f3047947c82..6bedb939c30 100644 --- a/apps/browser/src/autofill/services/inline-menu-field-qualification.service.spec.ts +++ b/apps/browser/src/autofill/services/inline-menu-field-qualification.service.spec.ts @@ -17,7 +17,6 @@ describe("InlineMenuFieldQualificationService", () => { fields: [], }); inlineMenuFieldQualificationService = new InlineMenuFieldQualificationService(); - inlineMenuFieldQualificationService["inlineMenuFieldQualificationFlagSet"] = true; }); describe("isFieldForLoginForm", () => { diff --git a/apps/browser/src/autofill/services/inline-menu-field-qualification.service.ts b/apps/browser/src/autofill/services/inline-menu-field-qualification.service.ts index 046381d956c..9b16a0cfbdd 100644 --- a/apps/browser/src/autofill/services/inline-menu-field-qualification.service.ts +++ b/apps/browser/src/autofill/services/inline-menu-field-qualification.service.ts @@ -150,7 +150,6 @@ export class InlineMenuFieldQualificationService this.identityPostalCodeAutocompleteValue, ]); private totpFieldAutocompleteValue = "one-time-code"; - private inlineMenuFieldQualificationFlagSet = false; private premiumEnabled = false; constructor() { @@ -158,7 +157,6 @@ export class InlineMenuFieldQualificationService sendExtensionMessage("getInlineMenuFieldQualificationFeatureFlag"), sendExtensionMessage("getUserPremiumStatus"), ]).then(([fieldQualificationFlag, premiumStatus]) => { - this.inlineMenuFieldQualificationFlagSet = !!fieldQualificationFlag?.result; this.premiumEnabled = !!premiumStatus?.result; }); } @@ -170,10 +168,6 @@ export class InlineMenuFieldQualificationService * @param pageDetails - The details of the page that the field is on. */ isFieldForLoginForm(field: AutofillField, pageDetails: AutofillPageDetails): boolean { - if (!this.inlineMenuFieldQualificationFlagSet) { - return this.isFieldForLoginFormFallback(field); - } - /** * Totp inline menu is available only for premium users. */ @@ -1223,18 +1217,4 @@ export class InlineMenuFieldQualificationService return false; } - - /** - * This method represents the previous rudimentary approach to qualifying fields for login forms. - * - * @param field - The field to validate - * @deprecated - This method will only be used when the fallback flag is set to true. - */ - private isFieldForLoginFormFallback(field: AutofillField): boolean { - if (field.type === "password") { - return true; - } - - return this.isUsernameField(field); - } } diff --git a/apps/browser/src/background/runtime.background.ts b/apps/browser/src/background/runtime.background.ts index d31ccdd97b0..c1719abbb3a 100644 --- a/apps/browser/src/background/runtime.background.ts +++ b/apps/browser/src/background/runtime.background.ts @@ -80,7 +80,6 @@ export default class RuntimeBackground { BiometricsCommands.GetBiometricsStatusForUser, BiometricsCommands.CanEnableBiometricUnlock, "getUseTreeWalkerApiForPageDetailsCollectionFeatureFlag", - "getInlineMenuFieldQualificationFeatureFlag", "getUserPremiumStatus", ]; @@ -210,9 +209,6 @@ export default class RuntimeBackground { FeatureFlag.UseTreeWalkerApiForPageDetailsCollection, ); } - case "getInlineMenuFieldQualificationFeatureFlag": { - return await this.configService.getFeatureFlag(FeatureFlag.InlineMenuFieldQualification); - } case "getUserPremiumStatus": { const activeUserId = await firstValueFrom( this.accountService.activeAccount$.pipe(map((a) => a?.id)), diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 798053c09d0..88a52f2948d 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -22,7 +22,6 @@ export enum FeatureFlag { EnableNewCardCombinedExpiryAutofill = "enable-new-card-combined-expiry-autofill", GenerateIdentityFillScriptRefactor = "generate-identity-fill-script-refactor", IdpAutoSubmitLogin = "idp-auto-submit-login", - InlineMenuFieldQualification = "inline-menu-field-qualification", InlineMenuPositioningImprovements = "inline-menu-positioning-improvements", NotificationBarAddLoginImprovements = "notification-bar-add-login-improvements", NotificationRefresh = "notification-refresh", @@ -84,7 +83,6 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.EnableNewCardCombinedExpiryAutofill]: FALSE, [FeatureFlag.GenerateIdentityFillScriptRefactor]: FALSE, [FeatureFlag.IdpAutoSubmitLogin]: FALSE, - [FeatureFlag.InlineMenuFieldQualification]: FALSE, [FeatureFlag.InlineMenuPositioningImprovements]: FALSE, [FeatureFlag.NotificationBarAddLoginImprovements]: FALSE, [FeatureFlag.NotificationRefresh]: FALSE, From 80e58427fb926b1c521dcbfb25d380ce7b6ac112 Mon Sep 17 00:00:00 2001 From: Miles Blackwood Date: Mon, 31 Mar 2025 12:36:52 -0400 Subject: [PATCH 16/17] Use bit-select (+ReactiveForms) in Autofill on page load settings. (#13593) * Use bit-select (+ReactiveForms) in Autofill on page load settings. * Use ReactiveForms for Additional options. * Disable margin reinclude from rebase. --- .../popup/settings/autofill.component.html | 218 +++++++++--------- .../popup/settings/autofill.component.ts | 139 ++++++++--- 2 files changed, 219 insertions(+), 138 deletions(-) diff --git a/apps/browser/src/autofill/popup/settings/autofill.component.html b/apps/browser/src/autofill/popup/settings/autofill.component.html index c690eb3d2ca..90a536ad25d 100644 --- a/apps/browser/src/autofill/popup/settings/autofill.component.html +++ b/apps/browser/src/autofill/popup/settings/autofill.component.html @@ -154,121 +154,115 @@ - -

{{ "enableAutoFillOnPageLoadSectionTitle" | i18n }}

- - - - {{ "enableAutoFillOnPageLoadDesc" | i18n }} - {{ "warningCapitalized" | i18n }}: {{ "experimentalFeature" | i18n }} - - {{ "learnMoreAboutAutofillOnPageLoadLinkText" | i18n }} - - - - - {{ "enableAutoFillOnPageLoad" | i18n }} - {{ - "enterprisePolicyRequirementsApplied" | i18n - }} - - - {{ "defaultAutoFillOnPageLoad" | i18n }} - - - {{ "defaultAutoFillOnPageLoadDesc" | i18n }} +
+ + +

{{ "enableAutoFillOnPageLoadSectionTitle" | i18n }}

+ + + + + {{ "enableAutoFillOnPageLoadDesc" | i18n }} + {{ "warningCapitalized" | i18n }}: {{ "experimentalFeature" | i18n }} + + {{ "learnMoreAboutAutofillOnPageLoadLinkText" | i18n }} + - - + + + {{ "enableAutoFillOnPageLoad" | i18n }} + {{ + "enterprisePolicyRequirementsApplied" | i18n + }} + + + {{ "defaultAutoFillOnPageLoad" | i18n }} + + + + + + {{ "defaultAutoFillOnPageLoadDesc" | i18n }} + + + +
- -

{{ "additionalOptions" | i18n }}

- - - - - {{ "enableContextMenuItem" | i18n }} - - - - {{ "enableAutoTotpCopy" | i18n }} - - - {{ "clearClipboard" | i18n }} - - - {{ "clearClipboardDesc" | i18n }} - - - - {{ "defaultUriMatchDetection" | i18n }} - - - {{ "defaultUriMatchDetectionDesc" | i18n }} - - - +
+ +

{{ "additionalOptions" | i18n }}

+ + + + + {{ "enableContextMenuItem" | i18n }} + + + + {{ "enableAutoTotpCopy" | i18n }} + + + {{ "clearClipboard" | i18n }} + + + + + {{ "clearClipboardDesc" | i18n }} + + + + {{ "defaultUriMatchDetection" | i18n }} + + + + + {{ "defaultUriMatchDetectionDesc" | i18n }} + + + +
diff --git a/apps/browser/src/autofill/popup/settings/autofill.component.ts b/apps/browser/src/autofill/popup/settings/autofill.component.ts index 7bd6c93bb64..eb18ea9f23e 100644 --- a/apps/browser/src/autofill/popup/settings/autofill.component.ts +++ b/apps/browser/src/autofill/popup/settings/autofill.component.ts @@ -1,8 +1,15 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { CommonModule } from "@angular/common"; -import { Component, OnInit } from "@angular/core"; -import { FormsModule } from "@angular/forms"; +import { Component, DestroyRef, OnInit } from "@angular/core"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; +import { + FormsModule, + ReactiveFormsModule, + FormBuilder, + FormGroup, + FormControl, +} from "@angular/forms"; import { RouterModule } from "@angular/router"; import { firstValueFrom } from "rxjs"; @@ -73,6 +80,7 @@ import { PopupPageComponent } from "../../../platform/popup/layout/popup-page.co SectionHeaderComponent, SelectModule, TypographyModule, + ReactiveFormsModule, ], }) export class AutofillComponent implements OnInit { @@ -94,6 +102,18 @@ export class AutofillComponent implements OnInit { protected autofillOnPageLoadFromPolicy$ = this.autofillSettingsService.activateAutofillOnPageLoadFromPolicy$; + protected autofillOnPageLoadForm = new FormGroup({ + autofillOnPageLoad: new FormControl(), + defaultAutofill: new FormControl(), + }); + + protected additionalOptionsForm = new FormGroup({ + enableContextMenuItem: new FormControl(), + enableAutoTotpCopy: new FormControl(), + clearClipboard: new FormControl(), + defaultUriMatch: new FormControl(), + }); + enableAutofillOnPageLoad: boolean = false; enableInlineMenu: boolean = false; enableInlineMenuOnIconSelect: boolean = false; @@ -121,10 +141,12 @@ export class AutofillComponent implements OnInit { private messagingService: MessagingService, private vaultSettingsService: VaultSettingsService, private configService: ConfigService, + private formBuilder: FormBuilder, + private destroyRef: DestroyRef, ) { this.autofillOnPageLoadOptions = [ - { name: i18nService.t("autoFillOnPageLoadYes"), value: true }, - { name: i18nService.t("autoFillOnPageLoadNo"), value: false }, + { name: this.i18nService.t("autoFillOnPageLoadYes"), value: true }, + { name: this.i18nService.t("autoFillOnPageLoadNo"), value: false }, ]; this.clearClipboardOptions = [ { name: i18nService.t("never"), value: ClearClipboardDelay.Never }, @@ -181,27 +203,106 @@ export class AutofillComponent implements OnInit { this.inlineMenuVisibility === AutofillOverlayVisibility.OnFieldFocus || this.enableInlineMenuOnIconSelect; + this.autofillSettingsService.activateAutofillOnPageLoadFromPolicy$ + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((value) => { + value + ? this.autofillOnPageLoadForm.controls.autofillOnPageLoad.disable({ emitEvent: false }) + : this.autofillOnPageLoadForm.controls.autofillOnPageLoad.enable({ emitEvent: false }); + }); + this.enableAutofillOnPageLoad = await firstValueFrom( this.autofillSettingsService.autofillOnPageLoad$, ); + this.autofillOnPageLoadForm.controls.autofillOnPageLoad.patchValue( + this.enableAutofillOnPageLoad, + { emitEvent: false }, + ); + this.autofillOnPageLoadDefault = await firstValueFrom( this.autofillSettingsService.autofillOnPageLoadDefault$, ); + if (this.enableAutofillOnPageLoad === false) { + this.autofillOnPageLoadForm.controls.defaultAutofill.disable(); + } + + this.autofillOnPageLoadForm.controls.defaultAutofill.patchValue( + this.autofillOnPageLoadDefault, + { emitEvent: false }, + ); + + this.autofillOnPageLoadForm.controls.autofillOnPageLoad.valueChanges + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((value) => { + void this.autofillSettingsService.setAutofillOnPageLoad(value); + this.enableDefaultAutofillControl(value); + }); + + this.autofillOnPageLoadForm.controls.defaultAutofill.valueChanges + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((value) => { + void this.autofillSettingsService.setAutofillOnPageLoadDefault(value); + }); + + /** Additional options form */ + this.enableContextMenuItem = await firstValueFrom( this.autofillSettingsService.enableContextMenu$, ); + this.additionalOptionsForm.controls.enableContextMenuItem.patchValue( + this.enableContextMenuItem, + { emitEvent: false }, + ); + this.enableAutoTotpCopy = await firstValueFrom(this.autofillSettingsService.autoCopyTotp$); + this.additionalOptionsForm.controls.enableAutoTotpCopy.patchValue(this.enableAutoTotpCopy, { + emitEvent: false, + }); + this.clearClipboard = await firstValueFrom(this.autofillSettingsService.clearClipboardDelay$); + this.additionalOptionsForm.controls.clearClipboard.patchValue(this.clearClipboard, { + emitEvent: false, + }); + const defaultUriMatch = await firstValueFrom( this.domainSettingsService.defaultUriMatchStrategy$, ); this.defaultUriMatch = defaultUriMatch == null ? UriMatchStrategy.Domain : defaultUriMatch; + this.additionalOptionsForm.controls.defaultUriMatch.patchValue(this.defaultUriMatch, { + emitEvent: false, + }); + + this.additionalOptionsForm.controls.enableContextMenuItem.valueChanges + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((value) => { + void this.autofillSettingsService.setEnableContextMenu(value); + this.messagingService.send("bgUpdateContextMenu"); + }); + + this.additionalOptionsForm.controls.enableAutoTotpCopy.valueChanges + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((value) => { + void this.autofillSettingsService.setAutoCopyTotp(value); + }); + + this.additionalOptionsForm.controls.clearClipboard.valueChanges + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((value) => { + void this.autofillSettingsService.setClearClipboardDelay(value); + }); + + this.additionalOptionsForm.controls.defaultUriMatch.valueChanges + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((value) => { + void this.domainSettingsService.setDefaultUriMatchStrategy(value); + }); + const command = await this.platformUtilsService.getAutofillKeyboardShortcut(); await this.setAutofillKeyboardHelperText(command); @@ -230,17 +331,16 @@ export class AutofillComponent implements OnInit { await this.requestPrivacyPermission(); } } - - async updateAutofillOnPageLoad() { - await this.autofillSettingsService.setAutofillOnPageLoad(this.enableAutofillOnPageLoad); + async getAutofillOnPageLoadFromPolicy() { + await firstValueFrom(this.autofillOnPageLoadFromPolicy$); } - async updateAutofillOnPageLoadDefault() { - await this.autofillSettingsService.setAutofillOnPageLoadDefault(this.autofillOnPageLoadDefault); - } - - async saveDefaultUriMatch() { - await this.domainSettingsService.setDefaultUriMatchStrategy(this.defaultUriMatch); + enableDefaultAutofillControl(enable: boolean = true) { + if (enable) { + this.autofillOnPageLoadForm.controls.defaultAutofill.enable(); + } else { + this.autofillOnPageLoadForm.controls.defaultAutofill.disable(); + } } private async setAutofillKeyboardHelperText(command: string) { @@ -388,19 +488,6 @@ export class AutofillComponent implements OnInit { return await BrowserApi.permissionsGranted(["privacy"]); } - async updateContextMenuItem() { - await this.autofillSettingsService.setEnableContextMenu(this.enableContextMenuItem); - this.messagingService.send("bgUpdateContextMenu"); - } - - async updateAutoTotpCopy() { - await this.autofillSettingsService.setAutoCopyTotp(this.enableAutoTotpCopy); - } - - async saveClearClipboard() { - await this.autofillSettingsService.setClearClipboardDelay(this.clearClipboard); - } - async updateShowCardsCurrentTab() { await this.vaultSettingsService.setShowCardsCurrentTab(this.showCardsCurrentTab); } From fa17bd7a3af16000e4e199712163c9dd2b530804 Mon Sep 17 00:00:00 2001 From: Conner Turnbull <133619638+cturnbull-bitwarden@users.noreply.github.com> Date: Mon, 31 Mar 2025 15:20:01 -0400 Subject: [PATCH 17/17] [PM-17526] Updated to use tailwind (#13685) * Updated to use tailwind * Removed "tw-w-full" and "tw-border-collapse" --- .../billing/providers/clients/manage-clients.component.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-clients.component.html b/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-clients.component.html index 077aeb6c124..85309e55bd9 100644 --- a/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-clients.component.html +++ b/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-clients.component.html @@ -38,7 +38,7 @@ @@ -46,7 +46,7 @@ - + {{ "client" | i18n }} {{ "assigned" | i18n }}