From f759e62aeb9e10ac0bcc7aba6e892025e6ebdb9d Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Fri, 28 Mar 2025 14:17:18 -0400 Subject: [PATCH 001/104] fix(browser): restore timer based background syncs (#14031) * docs: fix a typo * fix(browser): restore timer-based background syncs The browser extension was not performing scheduled background syncs every 30 minutes as expected. This was due to missing task scheduling code that was accidentally removed during the web push implementation (PR #11346). This commit: - Creates a new BackgroundSyncService to manage sync scheduling - Properly initializes the sync interval in main.background.ts - Adds a test to ensure the sync initialization code isn't accidentally removed again - Organizes platform module structure to support the new service Fixes PM-19396 * review: remove unecassary await keyword --- .../src/background/main.background.spec.ts | 13 +++ .../browser/src/background/main.background.ts | 10 +- apps/browser/tsconfig.json | 1 + .../scheduling/task-scheduler.service.ts | 2 +- .../background-sync.service.spec.ts | 107 ++++++++++++++++++ .../background-sync.service.ts | 44 +++++++ libs/platform/src/background-sync/index.ts | 1 + libs/platform/src/index.ts | 1 + libs/platform/tsconfig.json | 4 +- tsconfig.json | 1 + 10 files changed, 178 insertions(+), 6 deletions(-) create mode 100644 apps/browser/src/background/main.background.spec.ts create mode 100644 libs/platform/src/background-sync/background-sync.service.spec.ts create mode 100644 libs/platform/src/background-sync/background-sync.service.ts create mode 100644 libs/platform/src/background-sync/index.ts diff --git a/apps/browser/src/background/main.background.spec.ts b/apps/browser/src/background/main.background.spec.ts new file mode 100644 index 00000000000..c2cd38b7a30 --- /dev/null +++ b/apps/browser/src/background/main.background.spec.ts @@ -0,0 +1,13 @@ +// This test skips all the initilization of the background script and just +// focuses on making sure we don't accidently delete the initilization of +// background vault syncing. This has happened before! +describe("MainBackground sync task scheduling", () => { + it("includes code to schedule the sync interval task", () => { + // Get the bootstrap method source code as string + const { default: MainBackground } = jest.requireActual("./main.background"); + const bootstrapSource = MainBackground.prototype.bootstrap.toString(); + + // Check that the source includes the critical sync interval scheduling code + expect(bootstrapSource).toContain("this.backgroundSyncService.init();"); + }); +}); diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index e1f0b8bfc64..cae554c872c 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -127,7 +127,6 @@ import { WebPushNotificationsApiService, WorkerWebPushConnectionService, } from "@bitwarden/common/platform/notifications/internal"; -import { ScheduledTaskNames } from "@bitwarden/common/platform/scheduling"; import { AppIdService } from "@bitwarden/common/platform/services/app-id.service"; import { ConfigApiService } from "@bitwarden/common/platform/services/config/config-api.service"; import { DefaultConfigService } from "@bitwarden/common/platform/services/config/default-config.service"; @@ -222,6 +221,7 @@ import { KdfConfigService, KeyService as KeyServiceAbstraction, } from "@bitwarden/key-management"; +import { BackgroundSyncService } from "@bitwarden/platform/background-sync"; import { IndividualVaultExportService, IndividualVaultExportServiceAbstraction, @@ -391,6 +391,7 @@ export default class MainBackground { offscreenDocumentService: OffscreenDocumentService; syncServiceListener: SyncServiceListener; browserInitialInstallService: BrowserInitialInstallService; + backgroundSyncService: BackgroundSyncService; webPushConnectionService: WorkerWebPushConnectionService | UnsupportedWebPushConnectionService; themeStateService: DefaultThemeStateService; @@ -585,9 +586,9 @@ export default class MainBackground { this.logService, this.stateProvider, ); - this.taskSchedulerService.registerTaskHandler(ScheduledTaskNames.scheduleNextSyncInterval, () => - this.fullSync(), - ); + + this.backgroundSyncService = new BackgroundSyncService(this.taskSchedulerService); + this.backgroundSyncService.register(() => this.fullSync()); this.environmentService = new BrowserEnvironmentService( this.logService, @@ -1368,6 +1369,7 @@ export default class MainBackground { setTimeout(async () => { await this.refreshBadge(); await this.fullSync(false); + this.backgroundSyncService.init(); this.notificationsService.startListening(); resolve(); }, 500); diff --git a/apps/browser/tsconfig.json b/apps/browser/tsconfig.json index e836af1327c..e24985f58af 100644 --- a/apps/browser/tsconfig.json +++ b/apps/browser/tsconfig.json @@ -29,6 +29,7 @@ "@bitwarden/key-management": ["../../libs/key-management/src"], "@bitwarden/key-management-ui": ["../../libs/key-management-ui/src"], "@bitwarden/platform": ["../../libs/platform/src"], + "@bitwarden/platform/*": ["../../libs/platform/src/*"], "@bitwarden/send-ui": ["../../libs/tools/send/send-ui/src"], "@bitwarden/tools-card": ["../../libs/tools/card/src"], "@bitwarden/ui-common": ["../../libs/ui/common/src"], diff --git a/libs/common/src/platform/scheduling/task-scheduler.service.ts b/libs/common/src/platform/scheduling/task-scheduler.service.ts index 35a577dda7d..3ea80c3ee0a 100644 --- a/libs/common/src/platform/scheduling/task-scheduler.service.ts +++ b/libs/common/src/platform/scheduling/task-scheduler.service.ts @@ -11,7 +11,7 @@ import { ScheduledTaskName } from "./scheduled-task-name.enum"; * in the future but the task that is ran is NOT the remainder of your RXJS pipeline. The * task you want ran must instead be registered in a location reachable on a service worker * startup (on browser). An example of an acceptible location is the constructor of a service - * you know is created in `MainBackground`. Uses of this API is other clients _can_ have the + * you know is created in `MainBackground`. Uses of this API in other clients _can_ have the * `registerTaskHandler` call in more places, but in order to have it work across clients * it is recommended to register it according to the rules of browser. * diff --git a/libs/platform/src/background-sync/background-sync.service.spec.ts b/libs/platform/src/background-sync/background-sync.service.spec.ts new file mode 100644 index 00000000000..1deb907b151 --- /dev/null +++ b/libs/platform/src/background-sync/background-sync.service.spec.ts @@ -0,0 +1,107 @@ +import { mock, MockProxy } from "jest-mock-extended"; + +import { TaskSchedulerService, ScheduledTaskNames } from "@bitwarden/common/platform/scheduling"; + +import { BackgroundSyncService, DEFAULT_SYNC_INTERVAL_MS } from "./background-sync.service"; + +describe("BackgroundSyncService", () => { + let taskSchedulerService: MockProxy; + let backgroundSyncService: BackgroundSyncService; + + beforeEach(() => { + taskSchedulerService = mock(); + backgroundSyncService = new BackgroundSyncService(taskSchedulerService); + }); + + describe("register", () => { + it("registers a task handler with the correct task name", () => { + // Arrange + const syncCallback = jest.fn().mockResolvedValue(undefined); + + // Act + backgroundSyncService.register(syncCallback); + + // Assert + expect(taskSchedulerService.registerTaskHandler).toHaveBeenCalledTimes(1); + expect(taskSchedulerService.registerTaskHandler).toHaveBeenCalledWith( + ScheduledTaskNames.scheduleNextSyncInterval, + syncCallback, + ); + }); + }); + + describe("init", () => { + it("schedules the sync interval task with default interval", () => { + // Act + backgroundSyncService.init(); + + // Assert + expect(taskSchedulerService.setInterval).toHaveBeenCalledTimes(1); + expect(taskSchedulerService.setInterval).toHaveBeenCalledWith( + ScheduledTaskNames.scheduleNextSyncInterval, + DEFAULT_SYNC_INTERVAL_MS, + ); + }); + + it("schedules the sync interval task with custom interval", () => { + // Arrange + const customInterval = 60000; // 1 minute + + // Act + backgroundSyncService.init(customInterval); + + // Assert + expect(taskSchedulerService.setInterval).toHaveBeenCalledTimes(1); + expect(taskSchedulerService.setInterval).toHaveBeenCalledWith( + ScheduledTaskNames.scheduleNextSyncInterval, + customInterval, + ); + }); + + it("correctly handles zero interval by using default", () => { + // Act + backgroundSyncService.init(0); + + // Assert + expect(taskSchedulerService.setInterval).toHaveBeenCalledTimes(1); + expect(taskSchedulerService.setInterval).toHaveBeenCalledWith( + ScheduledTaskNames.scheduleNextSyncInterval, + DEFAULT_SYNC_INTERVAL_MS, + ); + }); + + it("correctly handles negative interval by using default", () => { + // Act + backgroundSyncService.init(-1000); + + // Assert + expect(taskSchedulerService.setInterval).toHaveBeenCalledTimes(1); + expect(taskSchedulerService.setInterval).toHaveBeenCalledWith( + ScheduledTaskNames.scheduleNextSyncInterval, + DEFAULT_SYNC_INTERVAL_MS, + ); + }); + }); + + describe("full integration", () => { + it("registers and initializes correctly in sequence", () => { + // Arrange + const syncCallback = jest.fn().mockResolvedValue(undefined); + const customInterval = 45000; // 45 seconds + + // Act + backgroundSyncService.register(syncCallback); + backgroundSyncService.init(customInterval); + + // Assert + expect(taskSchedulerService.registerTaskHandler).toHaveBeenCalledWith( + ScheduledTaskNames.scheduleNextSyncInterval, + syncCallback, + ); + expect(taskSchedulerService.setInterval).toHaveBeenCalledWith( + ScheduledTaskNames.scheduleNextSyncInterval, + customInterval, + ); + }); + }); +}); diff --git a/libs/platform/src/background-sync/background-sync.service.ts b/libs/platform/src/background-sync/background-sync.service.ts new file mode 100644 index 00000000000..dc1b49d399e --- /dev/null +++ b/libs/platform/src/background-sync/background-sync.service.ts @@ -0,0 +1,44 @@ +import { TaskSchedulerService, ScheduledTaskNames } from "@bitwarden/common/platform/scheduling"; + +/** + * The default interval between background syncs. + * 300,000ms = 5 minutes + */ +export const DEFAULT_SYNC_INTERVAL_MS = 300000; + +/** + * Service responsible for registering and managing background synchronization for the browser extension. + * Handles scheduling of periodic sync operations using the task scheduler infrastructure. + */ + +export class BackgroundSyncService { + /** + * Creates a new instance of BackgroundSyncService. + * @param taskSchedulerService - Service that handles scheduling and execution of periodic tasks + */ + constructor(private taskSchedulerService: TaskSchedulerService) {} + + /** + * Registers a callback function to be executed when the sync interval task is triggered. + * This associates the sync task name with the provided callback in the task scheduler. + * + * @param syncCallback - The function to execute when the sync task is triggered + */ + register(syncCallback: () => Promise) { + this.taskSchedulerService.registerTaskHandler( + ScheduledTaskNames.scheduleNextSyncInterval, + syncCallback, + ); + } + + /** + * Initializes the background sync service by scheduling the sync interval task. + * This sets up a recurring timer that triggers the registered sync callback at regular intervals. + * + * @param intervalMs - The interval in milliseconds between sync operations (defaults to 300000ms/5 minutes) + */ + init(intervalMs: number = DEFAULT_SYNC_INTERVAL_MS) { + intervalMs = intervalMs < 1 ? DEFAULT_SYNC_INTERVAL_MS : intervalMs; + this.taskSchedulerService.setInterval(ScheduledTaskNames.scheduleNextSyncInterval, intervalMs); + } +} diff --git a/libs/platform/src/background-sync/index.ts b/libs/platform/src/background-sync/index.ts new file mode 100644 index 00000000000..adfeec608be --- /dev/null +++ b/libs/platform/src/background-sync/index.ts @@ -0,0 +1 @@ +export * from "./background-sync.service"; diff --git a/libs/platform/src/index.ts b/libs/platform/src/index.ts index f11ec102845..3fabe3fad1a 100644 --- a/libs/platform/src/index.ts +++ b/libs/platform/src/index.ts @@ -1 +1,2 @@ export * from "./services/browser-service"; +export * from "./background-sync"; diff --git a/libs/platform/tsconfig.json b/libs/platform/tsconfig.json index eaa021247d8..898f9e41c6a 100644 --- a/libs/platform/tsconfig.json +++ b/libs/platform/tsconfig.json @@ -1,7 +1,9 @@ { "extends": "../shared/tsconfig", "compilerOptions": { - "paths": {} + "paths": { + "@bitwarden/common/*": ["../common/src/*"] + } }, "include": ["src", "spec"], "exclude": ["node_modules", "dist"] diff --git a/tsconfig.json b/tsconfig.json index fb50f1e7033..c82851d50c8 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -35,6 +35,7 @@ "@bitwarden/key-management-ui": ["./libs/key-management-ui/src"], "@bitwarden/node/*": ["./libs/node/src/*"], "@bitwarden/platform": ["./libs/platform/src"], + "@bitwarden/platform/*": ["./libs/platform/src/*"], "@bitwarden/send-ui": ["./libs/tools/send/send-ui/src"], "@bitwarden/tools-card": ["./libs/tools/card/src"], "@bitwarden/ui-common": ["./libs/ui/common/src"], From 66a914badfd65046ce5969163e957ef8f919f5a8 Mon Sep 17 00:00:00 2001 From: Jason Ng Date: Fri, 28 Mar 2025 15:50:30 -0400 Subject: [PATCH 002/104] [PM-19654] add hideIcon option to extension anon layout (#14045) --- .../extension-anon-layout-wrapper.component.html | 1 + .../extension-anon-layout-wrapper.component.ts | 10 ++++++++++ .../extension-anon-layout-wrapper.stories.ts | 2 ++ .../anon-layout/anon-layout.component.html | 2 +- .../angular/anon-layout/anon-layout.component.ts | 1 + .../angular/anon-layout/anon-layout.stories.ts | 16 ++++++++++++++++ 6 files changed, 31 insertions(+), 1 deletion(-) diff --git a/apps/browser/src/auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper.component.html b/apps/browser/src/auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper.component.html index 88a3b1c3076..54cb5203a87 100644 --- a/apps/browser/src/auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper.component.html +++ b/apps/browser/src/auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper.component.html @@ -21,6 +21,7 @@ [hideLogo]="true" [maxWidth]="maxWidth" [hideFooter]="hideFooter" + [hideIcon]="hideIcon" > diff --git a/apps/browser/src/auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper.component.ts b/apps/browser/src/auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper.component.ts index 10ef65d0654..51dbb6503d7 100644 --- a/apps/browser/src/auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper.component.ts +++ b/apps/browser/src/auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper.component.ts @@ -26,6 +26,7 @@ export interface ExtensionAnonLayoutWrapperData extends AnonLayoutWrapperData { showBackButton?: boolean; showLogo?: boolean; hideFooter?: boolean; + hideIcon?: boolean; } @Component({ @@ -48,6 +49,7 @@ export class ExtensionAnonLayoutWrapperComponent implements OnInit, OnDestroy { protected showAcctSwitcher: boolean; protected showBackButton: boolean; protected showLogo: boolean = true; + protected hideIcon: boolean = false; protected pageTitle: string; protected pageSubtitle: string; @@ -129,6 +131,10 @@ export class ExtensionAnonLayoutWrapperComponent implements OnInit, OnDestroy { if (firstChildRouteData["showLogo"] !== undefined) { this.showLogo = Boolean(firstChildRouteData["showLogo"]); } + + if (firstChildRouteData["hideIcon"] !== undefined) { + this.hideIcon = Boolean(firstChildRouteData["hideIcon"]); + } } private listenForServiceDataChanges() { @@ -180,6 +186,10 @@ export class ExtensionAnonLayoutWrapperComponent implements OnInit, OnDestroy { if (data.showLogo !== undefined) { this.showLogo = data.showLogo; } + + if (data.hideIcon !== undefined) { + this.hideIcon = data.hideIcon; + } } private handleStringOrTranslation(value: string | Translation): string { diff --git a/apps/browser/src/auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper.stories.ts b/apps/browser/src/auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper.stories.ts index 841eefda0ad..a0990485d49 100644 --- a/apps/browser/src/auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper.stories.ts +++ b/apps/browser/src/auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper.stories.ts @@ -242,6 +242,7 @@ const initialData: ExtensionAnonLayoutWrapperData = { showAcctSwitcher: true, showBackButton: true, showLogo: true, + hideIcon: false, }; const changedData: ExtensionAnonLayoutWrapperData = { @@ -255,6 +256,7 @@ const changedData: ExtensionAnonLayoutWrapperData = { showAcctSwitcher: false, showBackButton: false, showLogo: false, + hideIcon: false, }; @Component({ diff --git a/libs/auth/src/angular/anon-layout/anon-layout.component.html b/libs/auth/src/angular/anon-layout/anon-layout.component.html index 4120ea59002..f31a5500b43 100644 --- a/libs/auth/src/angular/anon-layout/anon-layout.component.html +++ b/libs/auth/src/angular/anon-layout/anon-layout.component.html @@ -17,7 +17,7 @@ class="tw-text-center tw-mb-4 sm:tw-mb-6" [ngClass]="{ 'tw-max-w-md tw-mx-auto': titleAreaMaxWidth === 'md' }" > -
+
diff --git a/libs/auth/src/angular/anon-layout/anon-layout.component.ts b/libs/auth/src/angular/anon-layout/anon-layout.component.ts index 05ddb9614f1..1ca4ccd2432 100644 --- a/libs/auth/src/angular/anon-layout/anon-layout.component.ts +++ b/libs/auth/src/angular/anon-layout/anon-layout.component.ts @@ -39,6 +39,7 @@ export class AnonLayoutComponent implements OnInit, OnChanges { @Input() showReadonlyHostname: boolean; @Input() hideLogo: boolean = false; @Input() hideFooter: boolean = false; + @Input() hideIcon: boolean = false; /** * Max width of the title area content diff --git a/libs/auth/src/angular/anon-layout/anon-layout.stories.ts b/libs/auth/src/angular/anon-layout/anon-layout.stories.ts index c7e15d9dcfa..34d561d5210 100644 --- a/libs/auth/src/angular/anon-layout/anon-layout.stories.ts +++ b/libs/auth/src/angular/anon-layout/anon-layout.stories.ts @@ -163,6 +163,22 @@ export const WithCustomIcon: Story = { }), }; +export const HideIcon: Story = { + render: (args) => ({ + props: args, + template: + // Projected content (the
) and styling is just a sample and can be replaced with any content/styling. + ` + +
+
Primary Projected Content Area (customizable)
+
Lorem ipsum dolor sit amet consectetur adipisicing elit. Necessitatibus illum vero, placeat recusandae esse ratione eius minima veniam nemo, quas beatae! Impedit molestiae alias sapiente explicabo. Sapiente corporis ipsa numquam?
+
+
+ `, + }), +}; + export const HideLogo: Story = { render: (args) => ({ props: args, 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 003/104] 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 004/104] [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 005/104] 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 006/104] [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 007/104] [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 008/104] 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 009/104] 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 010/104] [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 011/104] [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 012/104] 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 013/104] 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 }} - - -