diff --git a/apps/browser/src/auth/popup/settings/account-security.component.spec.ts b/apps/browser/src/auth/popup/settings/account-security.component.spec.ts index fe9c8c1bf06..15c4dbee98b 100644 --- a/apps/browser/src/auth/popup/settings/account-security.component.spec.ts +++ b/apps/browser/src/auth/popup/settings/account-security.component.spec.ts @@ -25,6 +25,7 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; import { MessageSender } from "@bitwarden/common/platform/messaging"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { StateProvider } from "@bitwarden/common/platform/state"; @@ -34,6 +35,8 @@ import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.servi import { DialogService, ToastService } from "@bitwarden/components"; import { BiometricStateService, BiometricsService, KeyService } from "@bitwarden/key-management"; +import { BrowserApi } from "../../../platform/browser/browser-api"; +import BrowserPopupUtils from "../../../platform/popup/browser-popup-utils"; import { PopOutComponent } from "../../../platform/popup/components/pop-out.component"; import { PopupRouterCacheService } from "../../../platform/popup/view-cache/popup-router-cache.service"; @@ -55,6 +58,10 @@ describe("AccountSecurityComponent", () => { const biometricStateService = mock(); const policyService = mock(); const pinServiceAbstraction = mock(); + const keyService = mock(); + const validationService = mock(); + const dialogService = mock(); + const platformUtilsService = mock(); beforeEach(async () => { await TestBed.configureTestingModule({ @@ -63,13 +70,13 @@ describe("AccountSecurityComponent", () => { { provide: AccountSecurityComponent, useValue: mock() }, { provide: BiometricsService, useValue: mock() }, { provide: BiometricStateService, useValue: biometricStateService }, - { provide: DialogService, useValue: mock() }, + { provide: DialogService, useValue: dialogService }, { provide: EnvironmentService, useValue: mock() }, { provide: I18nService, useValue: mock() }, { provide: MessageSender, useValue: mock() }, - { provide: KeyService, useValue: mock() }, + { provide: KeyService, useValue: keyService }, { provide: PinServiceAbstraction, useValue: pinServiceAbstraction }, - { provide: PlatformUtilsService, useValue: mock() }, + { provide: PlatformUtilsService, useValue: platformUtilsService }, { provide: PolicyService, useValue: policyService }, { provide: PopupRouterCacheService, useValue: mock() }, { provide: StateService, useValue: mock() }, @@ -84,14 +91,17 @@ describe("AccountSecurityComponent", () => { { provide: OrganizationService, useValue: mock() }, { provide: CollectionService, useValue: mock() }, { provide: ConfigService, useValue: mock() }, + { provide: ValidationService, useValue: validationService }, ], }) .overrideComponent(AccountSecurityComponent, { remove: { imports: [PopOutComponent], + providers: [DialogService], }, add: { imports: [MockPopOutComponent], + providers: [{ provide: DialogService, useValue: dialogService }], }, }) .compileComponents(); @@ -106,10 +116,17 @@ describe("AccountSecurityComponent", () => { vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$.mockReturnValue( of(VaultTimeoutAction.Lock), ); + vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$.mockReturnValue( + of(VaultTimeoutAction.Lock), + ); biometricStateService.promptAutomatically$ = of(false); pinServiceAbstraction.isPinSet.mockResolvedValue(false); }); + afterEach(() => { + jest.resetAllMocks(); + }); + it("pin enabled when RemoveUnlockWithPin policy is not set", async () => { // @ts-strict-ignore policyService.policiesByType$.mockReturnValue(of([null])); @@ -211,4 +228,136 @@ describe("AccountSecurityComponent", () => { const pinInputElement = fixture.debugElement.query(By.css("#pin")); expect(pinInputElement).toBeNull(); }); + + describe("updateBiometric", () => { + let browserApiSpy: jest.SpyInstance; + + beforeEach(() => { + policyService.policiesByType$.mockReturnValue(of([null])); + browserApiSpy = jest.spyOn(BrowserApi, "requestPermission"); + browserApiSpy.mockResolvedValue(true); + }); + + describe("updating to false", () => { + it("calls biometricStateService methods with false when false", async () => { + await component.ngOnInit(); + await component.updateBiometric(false); + + expect(biometricStateService.setBiometricUnlockEnabled).toHaveBeenCalledWith(false); + expect(biometricStateService.setFingerprintValidated).toHaveBeenCalledWith(false); + }); + }); + + describe("updating to true", () => { + let trySetupBiometricsSpy: jest.SpyInstance; + + beforeEach(() => { + trySetupBiometricsSpy = jest.spyOn(component, "trySetupBiometrics"); + }); + + it("displays permission error dialog when nativeMessaging permission is not granted", async () => { + browserApiSpy.mockResolvedValue(false); + + await component.ngOnInit(); + await component.updateBiometric(true); + + expect(dialogService.openSimpleDialog).toHaveBeenCalledWith({ + title: { key: "nativeMessaginPermissionErrorTitle" }, + content: { key: "nativeMessaginPermissionErrorDesc" }, + acceptButtonText: { key: "ok" }, + cancelButtonText: null, + type: "danger", + }); + expect(component.form.controls.biometric.value).toBe(false); + expect(trySetupBiometricsSpy).not.toHaveBeenCalled(); + }); + + it("displays a specific sidebar dialog when nativeMessaging permissions throws an error on firefox + sidebar", async () => { + browserApiSpy.mockRejectedValue(new Error("Permission denied")); + platformUtilsService.isFirefox.mockReturnValue(true); + jest.spyOn(BrowserPopupUtils, "inSidebar").mockReturnValue(true); + + await component.ngOnInit(); + await component.updateBiometric(true); + + expect(dialogService.openSimpleDialog).toHaveBeenCalledWith({ + title: { key: "nativeMessaginPermissionSidebarTitle" }, + content: { key: "nativeMessaginPermissionSidebarDesc" }, + acceptButtonText: { key: "ok" }, + cancelButtonText: null, + type: "info", + }); + expect(component.form.controls.biometric.value).toBe(false); + expect(trySetupBiometricsSpy).not.toHaveBeenCalled(); + }); + + test.each([ + [false, false], + [false, true], + [true, false], + ])( + "displays a generic dialog when nativeMessaging permissions throws an error and isFirefox is %s and onSidebar is %s", + async (isFirefox, inSidebar) => { + browserApiSpy.mockRejectedValue(new Error("Permission denied")); + platformUtilsService.isFirefox.mockReturnValue(isFirefox); + jest.spyOn(BrowserPopupUtils, "inSidebar").mockReturnValue(inSidebar); + + await component.ngOnInit(); + await component.updateBiometric(true); + + expect(dialogService.openSimpleDialog).toHaveBeenCalledWith({ + title: { key: "nativeMessaginPermissionErrorTitle" }, + content: { key: "nativeMessaginPermissionErrorDesc" }, + acceptButtonText: { key: "ok" }, + cancelButtonText: null, + type: "danger", + }); + expect(component.form.controls.biometric.value).toBe(false); + expect(trySetupBiometricsSpy).not.toHaveBeenCalled(); + }, + ); + + it("refreshes additional keys and attempts to setup biometrics when enabled with nativeMessaging permission", async () => { + const setupBiometricsResult = true; + trySetupBiometricsSpy.mockResolvedValue(setupBiometricsResult); + + await component.ngOnInit(); + await component.updateBiometric(true); + + expect(keyService.refreshAdditionalKeys).toHaveBeenCalledWith(mockUserId); + expect(biometricStateService.setBiometricUnlockEnabled).toHaveBeenCalledWith( + setupBiometricsResult, + ); + expect(component.form.controls.biometric.value).toBe(setupBiometricsResult); + }); + + it("handles failed biometrics setup", async () => { + const setupBiometricsResult = false; + trySetupBiometricsSpy.mockResolvedValue(setupBiometricsResult); + + await component.ngOnInit(); + await component.updateBiometric(true); + + expect(biometricStateService.setBiometricUnlockEnabled).toHaveBeenCalledWith( + setupBiometricsResult, + ); + expect(biometricStateService.setFingerprintValidated).toHaveBeenCalledWith( + setupBiometricsResult, + ); + expect(component.form.controls.biometric.value).toBe(setupBiometricsResult); + }); + + it("handles error during biometrics setup", async () => { + // Simulate an error during biometrics setup + keyService.refreshAdditionalKeys.mockRejectedValue(new Error("UserId is required")); + + await component.ngOnInit(); + await component.updateBiometric(true); + + expect(validationService.showError).toHaveBeenCalledWith(new Error("UserId is required")); + expect(component.form.controls.biometric.value).toBe(false); + expect(trySetupBiometricsSpy).not.toHaveBeenCalled(); + }); + }); + }); }); 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 af716ee2301..61937a30e8f 100644 --- a/apps/browser/src/auth/popup/settings/account-security.component.ts +++ b/apps/browser/src/auth/popup/settings/account-security.component.ts @@ -45,6 +45,7 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; import { DialogRef, CardComponent, @@ -153,6 +154,7 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { private toastService: ToastService, private biometricsService: BiometricsService, private vaultNudgesService: NudgesService, + private validationService: ValidationService, ) {} async ngOnInit() { @@ -520,13 +522,19 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { return; } - await this.keyService.refreshAdditionalKeys(); + try { + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + await this.keyService.refreshAdditionalKeys(userId); - const successful = await this.trySetupBiometrics(); - this.form.controls.biometric.setValue(successful); - await this.biometricStateService.setBiometricUnlockEnabled(successful); - if (!successful) { - await this.biometricStateService.setFingerprintValidated(false); + const successful = await this.trySetupBiometrics(); + this.form.controls.biometric.setValue(successful); + await this.biometricStateService.setBiometricUnlockEnabled(successful); + if (!successful) { + await this.biometricStateService.setFingerprintValidated(false); + } + } catch (error) { + this.form.controls.biometric.setValue(false); + this.validationService.showError(error); } } else { await this.biometricStateService.setBiometricUnlockEnabled(false); diff --git a/apps/desktop/src/app/accounts/settings.component.spec.ts b/apps/desktop/src/app/accounts/settings.component.spec.ts index 6348ec30a97..55bc09b7c95 100644 --- a/apps/desktop/src/app/accounts/settings.component.spec.ts +++ b/apps/desktop/src/app/accounts/settings.component.spec.ts @@ -22,17 +22,20 @@ import { import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; import { ThemeType } from "@bitwarden/common/platform/enums"; import { MessageSender } from "@bitwarden/common/platform/messaging"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { ThemeStateService } from "@bitwarden/common/platform/theming/theme-state.service"; import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; import { UserId } from "@bitwarden/common/types/guid"; -import { DialogService } from "@bitwarden/components"; +import { DialogRef, DialogService } from "@bitwarden/components"; import { BiometricStateService, BiometricsStatus, KeyService } from "@bitwarden/key-management"; +import { SetPinComponent } from "../../auth/components/set-pin.component"; import { SshAgentPromptType } from "../../autofill/models/ssh-agent-setting"; import { DesktopAutofillSettingsService } from "../../autofill/services/desktop-autofill-settings.service"; import { DesktopBiometricsService } from "../../key-management/biometrics/desktop.biometrics.service"; @@ -60,6 +63,11 @@ describe("SettingsComponent", () => { const pinServiceAbstraction = mock(); const desktopBiometricsService = mock(); const platformUtilsService = mock(); + const logService = mock(); + const validationService = mock(); + const messagingService = mock(); + const keyService = mock(); + const dialogService = mock(); beforeEach(async () => { originalIpc = (global as any).ipc; @@ -95,15 +103,15 @@ describe("SettingsComponent", () => { { provide: DesktopBiometricsService, useValue: desktopBiometricsService }, { provide: DesktopSettingsService, useValue: desktopSettingsService }, { provide: DomainSettingsService, useValue: domainSettingsService }, - { provide: DialogService, useValue: mock() }, + { provide: DialogService, useValue: dialogService }, { provide: I18nService, useValue: i18nService }, - { provide: LogService, useValue: mock() }, + { provide: LogService, useValue: logService }, { provide: MessageSender, useValue: mock() }, { provide: NativeMessagingManifestService, useValue: mock(), }, - { provide: KeyService, useValue: mock() }, + { provide: KeyService, useValue: keyService }, { provide: PinServiceAbstraction, useValue: pinServiceAbstraction }, { provide: PlatformUtilsService, useValue: platformUtilsService }, { provide: PolicyService, useValue: policyService }, @@ -111,6 +119,8 @@ describe("SettingsComponent", () => { { provide: ThemeStateService, useValue: themeStateService }, { provide: UserVerificationService, useValue: mock() }, { provide: VaultTimeoutSettingsService, useValue: vaultTimeoutSettingsService }, + { provide: ValidationService, useValue: validationService }, + { provide: MessagingService, useValue: messagingService }, ], schemas: [NO_ERRORS_SCHEMA], }).compileComponents(); @@ -324,4 +334,261 @@ describe("SettingsComponent", () => { expect(textNodes).toContain("Require password on app start"); }); }); + + describe("updatePinHandler", () => { + afterEach(() => { + jest.resetAllMocks(); + }); + + test.each([true, false])(`handles thrown errors when updated pin to %s`, async (update) => { + const error = new Error("Test error"); + jest.spyOn(component, "updatePin").mockRejectedValue(error); + + await component.ngOnInit(); + await component.updatePinHandler(update); + + expect(logService.error).toHaveBeenCalled(); + expect(component.form.controls.pin.value).toBe(!update); + expect(validationService.showError).toHaveBeenCalledWith(error); + expect(messagingService.send).toHaveBeenCalledWith("redrawMenu"); + }); + + describe("when updating to true", () => { + it("sets pin form control to false when the PIN dialog is cancelled", async () => { + jest.spyOn(SetPinComponent, "open").mockReturnValue(null); + + await component.ngOnInit(); + await component.updatePinHandler(true); + + expect(component.form.controls.pin.value).toBe(false); + expect(vaultTimeoutSettingsService.clear).not.toHaveBeenCalled(); + expect(messagingService.send).toHaveBeenCalledWith("redrawMenu"); + }); + + test.each([true, false])( + `sets the pin form control to the dialog result`, + async (dialogResult) => { + const mockDialogRef = { + closed: of(dialogResult), + } as DialogRef; + jest.spyOn(SetPinComponent, "open").mockReturnValue(mockDialogRef); + + await component.ngOnInit(); + await component.updatePinHandler(true); + + expect(component.form.controls.pin.value).toBe(dialogResult); + expect(vaultTimeoutSettingsService.clear).not.toHaveBeenCalled(); + expect(messagingService.send).toHaveBeenCalledWith("redrawMenu"); + }, + ); + }); + + describe("when updating to false", () => { + let updateRequirePasswordOnStartSpy: jest.SpyInstance; + + beforeEach(() => { + updateRequirePasswordOnStartSpy = jest + .spyOn(component, "updateRequirePasswordOnStart") + .mockImplementation(() => Promise.resolve()); + }); + + it("updates requires password on start when the user doesn't have a MP and has requirePasswordOnStart on", async () => { + await component.ngOnInit(); + component.form.controls.requirePasswordOnStart.setValue(true, { emitEvent: false }); + component.userHasMasterPassword = false; + await component.updatePinHandler(false); + + expect(component.form.controls.pin.value).toBe(false); + expect(component.form.controls.requirePasswordOnStart.value).toBe(false); + expect(updateRequirePasswordOnStartSpy).toHaveBeenCalled(); + expect(vaultTimeoutSettingsService.clear).toHaveBeenCalled(); + expect(messagingService.send).toHaveBeenCalledWith("redrawMenu"); + }); + + test.each([ + [true, true], + [false, true], + [false, false], + ])( + `doesn't updates requires password on start when the user's requirePasswordOnStart is %s and userHasMasterPassword is %s`, + async (requirePasswordOnStart, userHasMasterPassword) => { + await component.ngOnInit(); + component.form.controls.requirePasswordOnStart.setValue(requirePasswordOnStart, { + emitEvent: false, + }); + component.userHasMasterPassword = userHasMasterPassword; + await component.updatePinHandler(false); + + expect(component.form.controls.pin.value).toBe(false); + expect(component.form.controls.requirePasswordOnStart.value).toBe(requirePasswordOnStart); + expect(updateRequirePasswordOnStartSpy).not.toHaveBeenCalled(); + expect(vaultTimeoutSettingsService.clear).toHaveBeenCalled(); + expect(messagingService.send).toHaveBeenCalledWith("redrawMenu"); + }, + ); + }); + }); + + describe("updateBiometricHandler", () => { + afterEach(() => { + jest.resetAllMocks(); + }); + + test.each([true, false])( + `handles thrown errors when updated biometrics to %s`, + async (update) => { + const error = new Error("Test error"); + jest.spyOn(component, "updateBiometric").mockRejectedValue(error); + + await component.ngOnInit(); + await component.updateBiometricHandler(update); + + expect(logService.error).toHaveBeenCalled(); + expect(component.form.controls.biometric.value).toBe(false); + expect(validationService.showError).toHaveBeenCalledWith(error); + expect(messagingService.send).toHaveBeenCalledWith("redrawMenu"); + }, + ); + + describe("when updating to true", () => { + beforeEach(async () => { + await component.ngOnInit(); + component.supportsBiometric = true; + }); + + it("calls services to clear biometrics when supportsBiometric is false", async () => { + component.supportsBiometric = false; + await component.updateBiometricHandler(true); + + expect(component.form.controls.biometric.value).toBe(false); + expect(biometricStateService.setBiometricUnlockEnabled).toHaveBeenLastCalledWith(false); + expect(keyService.refreshAdditionalKeys).toHaveBeenCalled(); + expect(messagingService.send).toHaveBeenCalledWith("redrawMenu"); + }); + + test.each([true, false])( + `launches a dialog and exits when man setup is needed, dialog result is %s`, + async (dialogResult) => { + dialogService.openSimpleDialog.mockResolvedValue(dialogResult); + desktopBiometricsService.getBiometricsStatus.mockResolvedValue( + BiometricsStatus.ManualSetupNeeded, + ); + + await component.updateBiometricHandler(true); + + expect(biometricStateService.setBiometricUnlockEnabled).not.toHaveBeenCalled(); + expect(keyService.refreshAdditionalKeys).not.toHaveBeenCalled(); + expect(messagingService.send).toHaveBeenCalledWith("redrawMenu"); + + if (dialogResult) { + expect(platformUtilsService.launchUri).toHaveBeenCalledWith( + "https://bitwarden.com/help/biometrics/", + ); + } else { + expect(platformUtilsService.launchUri).not.toHaveBeenCalled(); + } + }, + ); + + it("sets up biometrics when auto setup is needed", async () => { + desktopBiometricsService.getBiometricsStatus.mockResolvedValue( + BiometricsStatus.AutoSetupNeeded, + ); + desktopBiometricsService.getBiometricsStatusForUser.mockResolvedValue( + BiometricsStatus.Available, + ); + + await component.updateBiometricHandler(true); + + expect(desktopBiometricsService.setupBiometrics).toHaveBeenCalled(); + expect(biometricStateService.setBiometricUnlockEnabled).toHaveBeenCalledWith(true); + expect(component.form.controls.biometric.value).toBe(true); + expect(keyService.refreshAdditionalKeys).toHaveBeenCalledWith(mockUserId); + expect(messagingService.send).toHaveBeenCalledWith("redrawMenu"); + }); + + it("handles windows case", async () => { + desktopBiometricsService.getBiometricsStatus.mockResolvedValue(BiometricsStatus.Available); + desktopBiometricsService.getBiometricsStatusForUser.mockResolvedValue( + BiometricsStatus.Available, + ); + + component.isWindows = true; + component.isLinux = false; + await component.updateBiometricHandler(true); + + expect(biometricStateService.setBiometricUnlockEnabled).toHaveBeenCalledWith(true); + expect(component.form.controls.requirePasswordOnStart.value).toBe(true); + expect(component.form.controls.autoPromptBiometrics.value).toBe(false); + expect(biometricStateService.setPromptAutomatically).toHaveBeenCalledWith(false); + expect(biometricStateService.setRequirePasswordOnStart).toHaveBeenCalledWith(true); + expect(biometricStateService.setDismissedRequirePasswordOnStartCallout).toHaveBeenCalled(); + expect(keyService.refreshAdditionalKeys).toHaveBeenCalledWith(mockUserId); + expect(component.form.controls.biometric.value).toBe(true); + expect(messagingService.send).toHaveBeenCalledWith("redrawMenu"); + }); + + it("handles linux case", async () => { + desktopBiometricsService.getBiometricsStatus.mockResolvedValue(BiometricsStatus.Available); + desktopBiometricsService.getBiometricsStatusForUser.mockResolvedValue( + BiometricsStatus.Available, + ); + + component.isWindows = false; + component.isLinux = true; + await component.updateBiometricHandler(true); + + expect(biometricStateService.setBiometricUnlockEnabled).toHaveBeenCalledWith(true); + expect(component.form.controls.requirePasswordOnStart.value).toBe(true); + expect(component.form.controls.autoPromptBiometrics.value).toBe(false); + expect(biometricStateService.setPromptAutomatically).toHaveBeenCalledWith(false); + expect(biometricStateService.setRequirePasswordOnStart).toHaveBeenCalledWith(true); + expect(biometricStateService.setDismissedRequirePasswordOnStartCallout).toHaveBeenCalled(); + expect(keyService.refreshAdditionalKeys).toHaveBeenCalledWith(mockUserId); + expect(component.form.controls.biometric.value).toBe(true); + expect(messagingService.send).toHaveBeenCalledWith("redrawMenu"); + }); + + test.each([ + BiometricsStatus.UnlockNeeded, + BiometricsStatus.HardwareUnavailable, + BiometricsStatus.AutoSetupNeeded, + BiometricsStatus.ManualSetupNeeded, + BiometricsStatus.PlatformUnsupported, + BiometricsStatus.DesktopDisconnected, + BiometricsStatus.NotEnabledLocally, + BiometricsStatus.NotEnabledInConnectedDesktopApp, + BiometricsStatus.NativeMessagingPermissionMissing, + ])( + `disables biometric when biometrics status check for the user returns %s`, + async (status) => { + desktopBiometricsService.getBiometricsStatus.mockResolvedValue( + BiometricsStatus.Available, + ); + desktopBiometricsService.getBiometricsStatusForUser.mockResolvedValue(status); + + await component.updateBiometricHandler(true); + + expect(keyService.refreshAdditionalKeys).toHaveBeenCalledWith(mockUserId); + expect(component.form.controls.biometric.value).toBe(false); + expect(biometricStateService.setBiometricUnlockEnabled).toHaveBeenCalledWith(true); + expect(biometricStateService.setBiometricUnlockEnabled).toHaveBeenCalledTimes(2); + expect(biometricStateService.setBiometricUnlockEnabled).toHaveBeenLastCalledWith(false); + expect(messagingService.send).toHaveBeenCalledWith("redrawMenu"); + }, + ); + }); + + describe("when updating to false", () => { + it("calls services to clear biometrics", async () => { + await component.ngOnInit(); + await component.updateBiometricHandler(false); + + expect(component.form.controls.biometric.value).toBe(false); + expect(biometricStateService.setBiometricUnlockEnabled).toHaveBeenLastCalledWith(false); + expect(keyService.refreshAdditionalKeys).toHaveBeenCalled(); + expect(messagingService.send).toHaveBeenCalledWith("redrawMenu"); + }); + }); + }); }); diff --git a/apps/desktop/src/app/accounts/settings.component.ts b/apps/desktop/src/app/accounts/settings.component.ts index fd0585e805e..76c257efad7 100644 --- a/apps/desktop/src/app/accounts/settings.component.ts +++ b/apps/desktop/src/app/accounts/settings.component.ts @@ -37,6 +37,7 @@ import { LogService } from "@bitwarden/common/platform/abstractions/log.service" import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; import { Theme, ThemeTypes } from "@bitwarden/common/platform/enums/theme-type.enum"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { ThemeStateService } from "@bitwarden/common/platform/theming/theme-state.service"; @@ -162,6 +163,7 @@ export class SettingsComponent implements OnInit, OnDestroy { private logService: LogService, private nativeMessagingManifestService: NativeMessagingManifestService, private configService: ConfigService, + private validationService: ValidationService, ) { const isMac = this.platformUtilsService.getDevice() === DeviceType.MacOsDesktop; @@ -379,7 +381,7 @@ export class SettingsComponent implements OnInit, OnDestroy { this.form.controls.pin.valueChanges .pipe( concatMap(async (value) => { - await this.updatePin(value); + await this.updatePinHandler(value); this.refreshTimeoutSettings$.next(); }), takeUntil(this.destroy$), @@ -389,7 +391,7 @@ export class SettingsComponent implements OnInit, OnDestroy { this.form.controls.biometric.valueChanges .pipe( concatMap(async (enabled) => { - await this.updateBiometric(enabled); + await this.updateBiometricHandler(enabled); this.refreshTimeoutSettings$.next(); }), takeUntil(this.destroy$), @@ -485,6 +487,18 @@ export class SettingsComponent implements OnInit, OnDestroy { ); } + async updatePinHandler(value: boolean) { + try { + await this.updatePin(value); + } catch (error) { + this.logService.error("Error updating unlock with PIN: ", error); + this.form.controls.pin.setValue(!value, { emitEvent: false }); + this.validationService.showError(error); + } finally { + this.messagingService.send("redrawMenu"); + } + } + async updatePin(value: boolean) { if (value) { const dialogRef = SetPinComponent.open(this.dialogService); @@ -509,8 +523,18 @@ export class SettingsComponent implements OnInit, OnDestroy { const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); await this.vaultTimeoutSettingsService.clear(userId); } + } - this.messagingService.send("redrawMenu"); + async updateBiometricHandler(value: boolean) { + try { + await this.updateBiometric(value); + } catch (error) { + this.logService.error("Error updating unlock with biometrics: ", error); + this.form.controls.biometric.setValue(false, { emitEvent: false }); + this.validationService.showError(error); + } finally { + this.messagingService.send("redrawMenu"); + } } async updateBiometric(enabled: boolean) { @@ -519,61 +543,55 @@ export class SettingsComponent implements OnInit, OnDestroy { // The bug should resolve itself once the angular issue is resolved. // See: https://github.com/angular/angular/issues/13063 - try { - if (!enabled || !this.supportsBiometric) { - this.form.controls.biometric.setValue(false, { emitEvent: false }); - await this.biometricStateService.setBiometricUnlockEnabled(false); - await this.keyService.refreshAdditionalKeys(); - return; - } + const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + if (!enabled || !this.supportsBiometric) { + this.form.controls.biometric.setValue(false, { emitEvent: false }); + await this.biometricStateService.setBiometricUnlockEnabled(false); + await this.keyService.refreshAdditionalKeys(activeUserId); + return; + } - const status = await this.biometricsService.getBiometricsStatus(); + const status = await this.biometricsService.getBiometricsStatus(); - if (status === BiometricsStatus.AutoSetupNeeded) { - await this.biometricsService.setupBiometrics(); - } else if (status === BiometricsStatus.ManualSetupNeeded) { - const confirmed = await this.dialogService.openSimpleDialog({ - title: { key: "biometricsManualSetupTitle" }, - content: { key: "biometricsManualSetupDesc" }, - type: "warning", - }); - if (confirmed) { - this.platformUtilsService.launchUri("https://bitwarden.com/help/biometrics/"); - } - return; + if (status === BiometricsStatus.AutoSetupNeeded) { + await this.biometricsService.setupBiometrics(); + } else if (status === BiometricsStatus.ManualSetupNeeded) { + const confirmed = await this.dialogService.openSimpleDialog({ + title: { key: "biometricsManualSetupTitle" }, + content: { key: "biometricsManualSetupDesc" }, + type: "warning", + }); + if (confirmed) { + this.platformUtilsService.launchUri("https://bitwarden.com/help/biometrics/"); } + return; + } - await this.biometricStateService.setBiometricUnlockEnabled(true); - if (this.isWindows) { - // Recommended settings for Windows Hello - this.form.controls.requirePasswordOnStart.setValue(true); - this.form.controls.autoPromptBiometrics.setValue(false); - await this.biometricStateService.setPromptAutomatically(false); - await this.biometricStateService.setRequirePasswordOnStart(true); - await this.biometricStateService.setDismissedRequirePasswordOnStartCallout(); - } else if (this.isLinux) { - // Similar to Windows - this.form.controls.requirePasswordOnStart.setValue(true); - this.form.controls.autoPromptBiometrics.setValue(false); - await this.biometricStateService.setPromptAutomatically(false); - await this.biometricStateService.setRequirePasswordOnStart(true); - await this.biometricStateService.setDismissedRequirePasswordOnStartCallout(); - } - await this.keyService.refreshAdditionalKeys(); + await this.biometricStateService.setBiometricUnlockEnabled(true); + if (this.isWindows) { + // Recommended settings for Windows Hello + this.form.controls.requirePasswordOnStart.setValue(true); + this.form.controls.autoPromptBiometrics.setValue(false); + await this.biometricStateService.setPromptAutomatically(false); + await this.biometricStateService.setRequirePasswordOnStart(true); + await this.biometricStateService.setDismissedRequirePasswordOnStartCallout(); + } else if (this.isLinux) { + // Similar to Windows + this.form.controls.requirePasswordOnStart.setValue(true); + this.form.controls.autoPromptBiometrics.setValue(false); + await this.biometricStateService.setPromptAutomatically(false); + await this.biometricStateService.setRequirePasswordOnStart(true); + await this.biometricStateService.setDismissedRequirePasswordOnStartCallout(); + } + await this.keyService.refreshAdditionalKeys(activeUserId); - const activeUserId = await firstValueFrom( - this.accountService.activeAccount$.pipe(map((a) => a?.id)), - ); - // Validate the key is stored in case biometrics fail. - const biometricSet = - (await this.biometricsService.getBiometricsStatusForUser(activeUserId)) === - BiometricsStatus.Available; - this.form.controls.biometric.setValue(biometricSet, { emitEvent: false }); - if (!biometricSet) { - await this.biometricStateService.setBiometricUnlockEnabled(false); - } - } finally { - this.messagingService.send("redrawMenu"); + // Validate the key is stored in case biometrics fail. + const biometricSet = + (await this.biometricsService.getBiometricsStatusForUser(activeUserId)) === + BiometricsStatus.Available; + this.form.controls.biometric.setValue(biometricSet, { emitEvent: false }); + if (!biometricSet) { + await this.biometricStateService.setBiometricUnlockEnabled(false); } } @@ -599,7 +617,8 @@ export class SettingsComponent implements OnInit, OnDestroy { await this.biometricStateService.setRequirePasswordOnStart(false); } await this.biometricStateService.setDismissedRequirePasswordOnStartCallout(); - await this.keyService.refreshAdditionalKeys(); + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + await this.keyService.refreshAdditionalKeys(userId); } async saveFavicons() { diff --git a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts index 16e38ae0b52..b3ed2165ed9 100644 --- a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts +++ b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts @@ -92,7 +92,7 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA clientSecret, ]); - await this.keyService.refreshAdditionalKeys(); + await this.keyService.refreshAdditionalKeys(userId); } availableVaultTimeoutActions$(userId?: string): Observable { diff --git a/libs/key-management/src/abstractions/key.service.ts b/libs/key-management/src/abstractions/key.service.ts index 7a9c076a8bb..4a3fca16515 100644 --- a/libs/key-management/src/abstractions/key.service.ts +++ b/libs/key-management/src/abstractions/key.service.ts @@ -83,8 +83,11 @@ export abstract class KeyService { * Gets the user key from memory and sets it again, * kicking off a refresh of any additional keys * (such as auto, biometrics, or pin) + * @param userId The target user to refresh keys for. + * @throws Error when userId is null or undefined. + * @throws When userKey doesn't exist in memory for the target user. */ - abstract refreshAdditionalKeys(): Promise; + abstract refreshAdditionalKeys(userId: UserId): Promise; /** * Observable value that returns whether or not the user has ever had a userKey, diff --git a/libs/key-management/src/key.service.spec.ts b/libs/key-management/src/key.service.spec.ts index cd5458e9a1f..7d30af23372 100644 --- a/libs/key-management/src/key.service.spec.ts +++ b/libs/key-management/src/key.service.spec.ts @@ -90,6 +90,35 @@ describe("keyService", () => { expect(keyService).not.toBeFalsy(); }); + describe("refreshAdditionalKeys", () => { + test.each([null as unknown as UserId, undefined as unknown as UserId])( + "throws when the provided userId is %s", + async (userId) => { + await expect(keyService.refreshAdditionalKeys(userId)).rejects.toThrow( + "UserId is required", + ); + }, + ); + + it("throws error if user key not found", async () => { + stateProvider.singleUser.getFake(mockUserId, USER_KEY).nextState(null); + + await expect(keyService.refreshAdditionalKeys(mockUserId)).rejects.toThrow( + "No user key found for: " + mockUserId, + ); + }); + + it("refreshes additional keys when user key is available", async () => { + const mockUserKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey; + stateProvider.singleUser.getFake(mockUserId, USER_KEY).nextState(mockUserKey); + const setUserKeySpy = jest.spyOn(keyService, "setUserKey"); + + await keyService.refreshAdditionalKeys(mockUserId); + + expect(setUserKeySpy).toHaveBeenCalledWith(mockUserKey, mockUserId); + }); + }); + describe("getUserKey", () => { let mockUserKey: UserKey; diff --git a/libs/key-management/src/key.service.ts b/libs/key-management/src/key.service.ts index e09cabcfae2..6cbb1fbcc03 100644 --- a/libs/key-management/src/key.service.ts +++ b/libs/key-management/src/key.service.ts @@ -122,15 +122,17 @@ export class DefaultKeyService implements KeyServiceAbstraction { await this.setPrivateKey(encPrivateKey, userId); } - async refreshAdditionalKeys(): Promise { - const activeUserId = await firstValueFrom(this.stateProvider.activeUserId$); - - if (activeUserId == null) { - throw new Error("Can only refresh keys while there is an active user."); + async refreshAdditionalKeys(userId: UserId): Promise { + if (userId == null) { + throw new Error("UserId is required."); } - const key = await this.getUserKey(activeUserId); - await this.setUserKey(key, activeUserId); + const key = await firstValueFrom(this.userKey$(userId)); + if (key == null) { + throw new Error("No user key found for: " + userId); + } + + await this.setUserKey(key, userId); } everHadUserKey$(userId: UserId): Observable {