diff --git a/apps/desktop/desktop_native/core/src/password/macos.rs b/apps/desktop/desktop_native/core/src/password/macos.rs index 1af005acb4d..3075dce3b88 100644 --- a/apps/desktop/desktop_native/core/src/password/macos.rs +++ b/apps/desktop/desktop_native/core/src/password/macos.rs @@ -1,10 +1,12 @@ +use crate::password::PASSWORD_NOT_FOUND; use anyhow::Result; use security_framework::passwords::{ delete_generic_password, get_generic_password, set_generic_password, }; pub async fn get_password(service: &str, account: &str) -> Result { - let result = String::from_utf8(get_generic_password(service, account)?)?; + let password = get_generic_password(service, account).map_err(convert_error)?; + let result = String::from_utf8(password)?; Ok(result) } @@ -14,7 +16,7 @@ pub async fn set_password(service: &str, account: &str, password: &str) -> Resul } pub async fn delete_password(service: &str, account: &str) -> Result<()> { - delete_generic_password(service, account)?; + delete_generic_password(service, account).map_err(convert_error)?; Ok(()) } @@ -22,6 +24,15 @@ pub async fn is_available() -> Result { Ok(true) } +fn convert_error(e: security_framework::base::Error) -> anyhow::Error { + match e.code() { + security_framework_sys::base::errSecItemNotFound => { + anyhow::anyhow!(PASSWORD_NOT_FOUND) + } + _ => anyhow::anyhow!(e), + } +} + #[cfg(test)] mod tests { use super::*; @@ -44,10 +55,7 @@ mod tests { // Ensure password is deleted match get_password("BitwardenTest", "BitwardenTest").await { Ok(_) => panic!("Got a result"), - Err(e) => assert_eq!( - "The specified item could not be found in the keychain.", - e.to_string() - ), + Err(e) => assert_eq!(PASSWORD_NOT_FOUND, e.to_string()), } } @@ -55,10 +63,7 @@ mod tests { async fn test_error_no_password() { match get_password("Unknown", "Unknown").await { Ok(_) => panic!("Got a result"), - Err(e) => assert_eq!( - "The specified item could not be found in the keychain.", - e.to_string() - ), + Err(e) => assert_eq!(PASSWORD_NOT_FOUND, e.to_string()), } } } diff --git a/apps/desktop/desktop_native/core/src/password/mod.rs b/apps/desktop/desktop_native/core/src/password/mod.rs index 351e896c40e..93c91072dd2 100644 --- a/apps/desktop/desktop_native/core/src/password/mod.rs +++ b/apps/desktop/desktop_native/core/src/password/mod.rs @@ -1,3 +1,5 @@ +pub const PASSWORD_NOT_FOUND: &str = "Password not found."; + #[allow(clippy::module_inception)] #[cfg_attr(target_os = "linux", path = "unix.rs")] #[cfg_attr(target_os = "windows", path = "windows.rs")] diff --git a/apps/desktop/desktop_native/core/src/password/unix.rs b/apps/desktop/desktop_native/core/src/password/unix.rs index a40f4d8ed43..0b1c59e7724 100644 --- a/apps/desktop/desktop_native/core/src/password/unix.rs +++ b/apps/desktop/desktop_native/core/src/password/unix.rs @@ -1,3 +1,4 @@ +use crate::password::PASSWORD_NOT_FOUND; use anyhow::{anyhow, Result}; use oo7::dbus::{self}; use std::collections::HashMap; @@ -20,7 +21,7 @@ async fn get_password_new(service: &str, account: &str) -> Result { let secret = res.secret().await?; Ok(String::from_utf8(secret.to_vec())?) } - None => Err(anyhow!("no result")), + None => Err(anyhow!(PASSWORD_NOT_FOUND)), } } @@ -46,7 +47,7 @@ async fn get_password_legacy(service: &str, account: &str) -> Result { set_password(service, account, &secret_string).await?; Ok(secret_string) } - None => Err(anyhow!("no result")), + None => Err(anyhow!(PASSWORD_NOT_FOUND)), } } @@ -152,7 +153,7 @@ mod tests { Ok(_) => { panic!("Got a result") } - Err(e) => assert_eq!("no result", e.to_string()), + Err(e) => assert_eq!(PASSWORD_NOT_FOUND, e.to_string()), } } @@ -160,7 +161,7 @@ mod tests { async fn test_error_no_password() { match get_password("Unknown", "Unknown").await { Ok(_) => panic!("Got a result"), - Err(e) => assert_eq!("no result", e.to_string()), + Err(e) => assert_eq!(PASSWORD_NOT_FOUND, e.to_string()), } } } diff --git a/apps/desktop/desktop_native/core/src/password/windows.rs b/apps/desktop/desktop_native/core/src/password/windows.rs index f0dcc3fb1eb..ee2361a832f 100644 --- a/apps/desktop/desktop_native/core/src/password/windows.rs +++ b/apps/desktop/desktop_native/core/src/password/windows.rs @@ -1,3 +1,4 @@ +use crate::password::PASSWORD_NOT_FOUND; use anyhow::{anyhow, Result}; use widestring::{U16CString, U16String}; use windows::{ @@ -79,7 +80,9 @@ pub async fn set_password(service: &str, account: &str, password: &str) -> Resul pub async fn delete_password(service: &str, account: &str) -> Result<()> { let target_name = U16CString::from_str(target_name(service, account))?; - unsafe { CredDeleteW(PCWSTR(target_name.as_ptr()), CRED_TYPE_GENERIC, None)? }; + let result = unsafe { CredDeleteW(PCWSTR(target_name.as_ptr()), CRED_TYPE_GENERIC, None) }; + + result.map_err(|e| anyhow!(convert_error(e)))?; Ok(()) } @@ -95,7 +98,7 @@ fn target_name(service: &str, account: &str) -> String { // Convert the internal WIN32 errors to descriptive messages fn convert_error(e: windows::core::Error) -> String { if e == ERROR_NOT_FOUND.into() { - return "Password not found.".to_string(); + return PASSWORD_NOT_FOUND.to_string(); } e.to_string() } @@ -122,7 +125,7 @@ mod tests { // Ensure password is deleted match get_password("BitwardenTest", "BitwardenTest").await { Ok(_) => panic!("Got a result"), - Err(e) => assert_eq!("Password not found.", e.to_string()), + Err(e) => assert_eq!(PASSWORD_NOT_FOUND, e.to_string()), } } @@ -130,7 +133,7 @@ mod tests { async fn test_error_no_password() { match get_password("BitwardenTest", "BitwardenTest").await { Ok(_) => panic!("Got a result"), - Err(e) => assert_eq!("Password not found.", e.to_string()), + Err(e) => assert_eq!(PASSWORD_NOT_FOUND, e.to_string()), } } } diff --git a/apps/desktop/desktop_native/napi/index.d.ts b/apps/desktop/desktop_native/napi/index.d.ts index b3c6f715e98..cb9430290e3 100644 --- a/apps/desktop/desktop_native/napi/index.d.ts +++ b/apps/desktop/desktop_native/napi/index.d.ts @@ -4,18 +4,31 @@ /* auto-generated by NAPI-RS */ export declare namespace passwords { - /** Fetch the stored password from the keychain. */ + /** The error message returned when a password is not found during retrieval or deletion. */ + export const PASSWORD_NOT_FOUND: string + /** + * Fetch the stored password from the keychain. + * Throws {@link Error} with message {@link PASSWORD_NOT_FOUND} if the password does not exist. + */ export function getPassword(service: string, account: string): Promise /** Save the password to the keychain. Adds an entry if none exists otherwise updates the existing entry. */ export function setPassword(service: string, account: string, password: string): Promise - /** Delete the stored password from the keychain. */ + /** + * Delete the stored password from the keychain. + * Throws {@link Error} with message {@link PASSWORD_NOT_FOUND} if the password does not exist. + */ export function deletePassword(service: string, account: string): Promise + /** Checks if the os secure storage is available */ export function isAvailable(): Promise } export declare namespace biometrics { export function prompt(hwnd: Buffer, message: string): Promise export function available(): Promise export function setBiometricSecret(service: string, account: string, secret: string, keyMaterial: KeyMaterial | undefined | null, ivB64: string): Promise + /** + * Retrieves the biometric secret for the given service and account. + * Throws Error with message [`passwords::PASSWORD_NOT_FOUND`] if the secret does not exist. + */ export function getBiometricSecret(service: string, account: string, keyMaterial?: KeyMaterial | undefined | null): Promise /** * Derives key material from biometric data. Returns a string encoded with a diff --git a/apps/desktop/desktop_native/napi/src/lib.rs b/apps/desktop/desktop_native/napi/src/lib.rs index 079872a3b03..e538dc8d432 100644 --- a/apps/desktop/desktop_native/napi/src/lib.rs +++ b/apps/desktop/desktop_native/napi/src/lib.rs @@ -6,7 +6,12 @@ mod registry; #[napi] pub mod passwords { + /// The error message returned when a password is not found during retrieval or deletion. + #[napi] + pub const PASSWORD_NOT_FOUND: &str = desktop_core::password::PASSWORD_NOT_FOUND; + /// Fetch the stored password from the keychain. + /// Throws {@link Error} with message {@link PASSWORD_NOT_FOUND} if the password does not exist. #[napi] pub async fn get_password(service: String, account: String) -> napi::Result { desktop_core::password::get_password(&service, &account) @@ -27,6 +32,7 @@ pub mod passwords { } /// Delete the stored password from the keychain. + /// Throws {@link Error} with message {@link PASSWORD_NOT_FOUND} if the password does not exist. #[napi] pub async fn delete_password(service: String, account: String) -> napi::Result<()> { desktop_core::password::delete_password(&service, &account) @@ -34,7 +40,7 @@ pub mod passwords { .map_err(|e| napi::Error::from_reason(e.to_string())) } - // Checks if the os secure storage is available + /// Checks if the os secure storage is available #[napi] pub async fn is_available() -> napi::Result { desktop_core::password::is_available() @@ -84,6 +90,8 @@ pub mod biometrics { .map_err(|e| napi::Error::from_reason(e.to_string())) } + /// Retrieves the biometric secret for the given service and account. + /// Throws Error with message [`passwords::PASSWORD_NOT_FOUND`] if the secret does not exist. #[napi] pub async fn get_biometric_secret( service: String, diff --git a/apps/desktop/src/key-management/biometrics/main-biometrics-ipc.listener.ts b/apps/desktop/src/key-management/biometrics/main-biometrics-ipc.listener.ts index e270c4cc50f..d4ce01f53f4 100644 --- a/apps/desktop/src/key-management/biometrics/main-biometrics-ipc.listener.ts +++ b/apps/desktop/src/key-management/biometrics/main-biometrics-ipc.listener.ts @@ -55,7 +55,7 @@ export class MainBiometricsIPCListener { return; } } catch (e) { - this.logService.info(e); + this.logService.error("[Main Biometrics IPC Listener] %s failed", message.action, e); } }); } 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 a6a0e532655..1de8e3cd12d 100644 --- a/apps/desktop/src/key-management/biometrics/main-biometrics.service.ts +++ b/apps/desktop/src/key-management/biometrics/main-biometrics.service.ts @@ -40,7 +40,7 @@ export class MainBiometricsService extends DesktopBiometricsService { } else if (platform === "darwin") { // eslint-disable-next-line const OsBiometricsServiceMac = require("./os-biometrics-mac.service").default; - this.osBiometricsService = new OsBiometricsServiceMac(this.i18nService); + this.osBiometricsService = new OsBiometricsServiceMac(this.i18nService, this.logService); } else if (platform === "linux") { // eslint-disable-next-line const OsBiometricsServiceLinux = require("./os-biometrics-linux.service").default; @@ -48,6 +48,7 @@ export class MainBiometricsService extends DesktopBiometricsService { this.biometricStateService, this.encryptService, this.cryptoFunctionService, + this.logService, ); } else { throw new Error("Unsupported platform"); diff --git a/apps/desktop/src/key-management/biometrics/os-biometrics-linux.service.spec.ts b/apps/desktop/src/key-management/biometrics/os-biometrics-linux.service.spec.ts new file mode 100644 index 00000000000..64af0cc625e --- /dev/null +++ b/apps/desktop/src/key-management/biometrics/os-biometrics-linux.service.spec.ts @@ -0,0 +1,86 @@ +import { mock } from "jest-mock-extended"; + +import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; +import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { UserId } from "@bitwarden/common/types/guid"; +import { passwords } from "@bitwarden/desktop-napi"; +import { BiometricStateService } from "@bitwarden/key-management"; + +import OsBiometricsServiceLinux from "./os-biometrics-linux.service"; + +jest.mock("@bitwarden/desktop-napi", () => ({ + biometrics: { + setBiometricSecret: jest.fn(), + getBiometricSecret: jest.fn(), + deleteBiometricSecret: jest.fn(), + prompt: jest.fn(), + available: jest.fn(), + deriveKeyMaterial: jest.fn(), + }, + passwords: { + deletePassword: jest.fn(), + getPassword: jest.fn(), + isAvailable: jest.fn(), + PASSWORD_NOT_FOUND: "Password not found", + }, +})); + +describe("OsBiometricsServiceLinux", () => { + let service: OsBiometricsServiceLinux; + let logService: LogService; + + const mockUserId = "test-user-id" as UserId; + + beforeEach(() => { + const biometricStateService = mock(); + const encryptService = mock(); + const cryptoFunctionService = mock(); + logService = mock(); + service = new OsBiometricsServiceLinux( + biometricStateService, + encryptService, + cryptoFunctionService, + logService, + ); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe("deleteBiometricKey", () => { + const serviceName = "Bitwarden_biometric"; + const keyName = "test-user-id_user_biometric"; + + it("should delete biometric key successfully", async () => { + await service.deleteBiometricKey(mockUserId); + + expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, keyName); + }); + + it("should not throw error if key not found", async () => { + passwords.deletePassword = jest + .fn() + .mockRejectedValueOnce(new Error(passwords.PASSWORD_NOT_FOUND)); + + await service.deleteBiometricKey(mockUserId); + + expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, keyName); + expect(logService.debug).toHaveBeenCalledWith( + "[OsBiometricService] Biometric key %s not found for service %s.", + keyName, + serviceName, + ); + }); + + it("should throw error for unexpected errors", async () => { + const error = new Error("Unexpected error"); + passwords.deletePassword = jest.fn().mockRejectedValueOnce(error); + + await expect(service.deleteBiometricKey(mockUserId)).rejects.toThrow(error); + + expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, keyName); + }); + }); +}); diff --git a/apps/desktop/src/key-management/biometrics/os-biometrics-linux.service.ts b/apps/desktop/src/key-management/biometrics/os-biometrics-linux.service.ts index 8d3c8e9795f..3f13682f1b7 100644 --- a/apps/desktop/src/key-management/biometrics/os-biometrics-linux.service.ts +++ b/apps/desktop/src/key-management/biometrics/os-biometrics-linux.service.ts @@ -2,6 +2,7 @@ import { spawn } from "child_process"; import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.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"; @@ -42,6 +43,7 @@ export default class OsBiometricsServiceLinux implements OsBiometricService { private biometricStateService: BiometricStateService, private encryptService: EncryptService, private cryptoFunctionService: CryptoFunctionService, + private logService: LogService, ) {} private _iv: string | null = null; // Use getKeyMaterial helper instead of direct access @@ -62,7 +64,19 @@ export default class OsBiometricsServiceLinux implements OsBiometricService { ); } async deleteBiometricKey(userId: UserId): Promise { - await passwords.deletePassword(SERVICE, getLookupKeyForUser(userId)); + try { + await passwords.deletePassword(SERVICE, getLookupKeyForUser(userId)); + } catch (e) { + if (e instanceof Error && e.message === passwords.PASSWORD_NOT_FOUND) { + this.logService.debug( + "[OsBiometricService] Biometric key %s not found for service %s.", + getLookupKeyForUser(userId), + SERVICE, + ); + } else { + throw e; + } + } } async getBiometricKey(userId: UserId): Promise { diff --git a/apps/desktop/src/key-management/biometrics/os-biometrics-mac.service.spec.ts b/apps/desktop/src/key-management/biometrics/os-biometrics-mac.service.spec.ts new file mode 100644 index 00000000000..6d20095d8bb --- /dev/null +++ b/apps/desktop/src/key-management/biometrics/os-biometrics-mac.service.spec.ts @@ -0,0 +1,78 @@ +import { mock } from "jest-mock-extended"; + +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { UserId } from "@bitwarden/common/types/guid"; +import { passwords } from "@bitwarden/desktop-napi"; + +import OsBiometricsServiceMac from "./os-biometrics-mac.service"; + +jest.mock("@bitwarden/desktop-napi", () => ({ + biometrics: { + setBiometricSecret: jest.fn(), + getBiometricSecret: jest.fn(), + deleteBiometricSecret: jest.fn(), + prompt: jest.fn(), + available: jest.fn(), + deriveKeyMaterial: jest.fn(), + }, + passwords: { + deletePassword: jest.fn(), + getPassword: jest.fn(), + isAvailable: jest.fn(), + PASSWORD_NOT_FOUND: "Password not found", + }, +})); + +describe("OsBiometricsServiceMac", () => { + let service: OsBiometricsServiceMac; + let i18nService: I18nService; + let logService: LogService; + + const mockUserId = "test-user-id" as UserId; + + beforeEach(() => { + i18nService = mock(); + logService = mock(); + service = new OsBiometricsServiceMac(i18nService, logService); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe("deleteBiometricKey", () => { + const serviceName = "Bitwarden_biometric"; + const keyName = "test-user-id_user_biometric"; + + it("should delete biometric key successfully", async () => { + await service.deleteBiometricKey(mockUserId); + + expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, keyName); + }); + + it("should not throw error if key not found", async () => { + passwords.deletePassword = jest + .fn() + .mockRejectedValueOnce(new Error(passwords.PASSWORD_NOT_FOUND)); + + await service.deleteBiometricKey(mockUserId); + + expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, keyName); + expect(logService.debug).toHaveBeenCalledWith( + "[OsBiometricService] Biometric key %s not found for service %s.", + keyName, + serviceName, + ); + }); + + it("should throw error for unexpected errors", async () => { + const error = new Error("Unexpected error"); + passwords.deletePassword = jest.fn().mockRejectedValueOnce(error); + + await expect(service.deleteBiometricKey(mockUserId)).rejects.toThrow(error); + + expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, keyName); + }); + }); +}); diff --git a/apps/desktop/src/key-management/biometrics/os-biometrics-mac.service.ts b/apps/desktop/src/key-management/biometrics/os-biometrics-mac.service.ts index 004495b6da9..1dc64f1bcd5 100644 --- a/apps/desktop/src/key-management/biometrics/os-biometrics-mac.service.ts +++ b/apps/desktop/src/key-management/biometrics/os-biometrics-mac.service.ts @@ -1,6 +1,7 @@ import { systemPreferences } from "electron"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { UserId } from "@bitwarden/common/types/guid"; import { passwords } from "@bitwarden/desktop-napi"; @@ -14,7 +15,10 @@ function getLookupKeyForUser(userId: UserId): string { } export default class OsBiometricsServiceMac implements OsBiometricService { - constructor(private i18nservice: I18nService) {} + constructor( + private i18nservice: I18nService, + private logService: LogService, + ) {} async supportsBiometrics(): Promise { return systemPreferences.canPromptTouchID(); @@ -52,7 +56,19 @@ export default class OsBiometricsServiceMac implements OsBiometricService { } async deleteBiometricKey(user: UserId): Promise { - return await passwords.deletePassword(SERVICE, getLookupKeyForUser(user)); + try { + return await passwords.deletePassword(SERVICE, getLookupKeyForUser(user)); + } catch (e) { + if (e instanceof Error && e.message === passwords.PASSWORD_NOT_FOUND) { + this.logService.debug( + "[OsBiometricService] Biometric key %s not found for service %s.", + getLookupKeyForUser(user), + SERVICE, + ); + } else { + throw e; + } + } } private async valueUpToDate(user: UserId, key: SymmetricCryptoKey): Promise { diff --git a/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.spec.ts b/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.spec.ts index d0fd8682f2a..674d97bf696 100644 --- a/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.spec.ts +++ b/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.spec.ts @@ -6,8 +6,11 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { UserId } from "@bitwarden/common/types/guid"; +import { passwords } from "@bitwarden/desktop-napi"; import { BiometricsStatus, BiometricStateService } from "@bitwarden/key-management"; +import { WindowMain } from "../../main/window.main"; + import OsBiometricsServiceWindows from "./os-biometrics-windows.service"; jest.mock("@bitwarden/desktop-napi", () => ({ @@ -15,28 +18,37 @@ jest.mock("@bitwarden/desktop-napi", () => ({ available: jest.fn(), setBiometricSecret: jest.fn(), getBiometricSecret: jest.fn(), - deriveKeyMaterial: jest.fn(), + deleteBiometricSecret: jest.fn(), prompt: jest.fn(), + deriveKeyMaterial: jest.fn(), }, passwords: { getPassword: jest.fn(), deletePassword: jest.fn(), + isAvailable: jest.fn(), + PASSWORD_NOT_FOUND: "Password not found", }, })); describe("OsBiometricsServiceWindows", () => { let service: OsBiometricsServiceWindows; + let i18nService: I18nService; + let windowMain: WindowMain; + let logService: LogService; let biometricStateService: BiometricStateService; + const mockUserId = "test-user-id" as UserId; + beforeEach(() => { - const i18nService = mock(); - const logService = mock(); + i18nService = mock(); + windowMain = mock(); + logService = mock(); biometricStateService = mock(); const encryptionService = mock(); const cryptoFunctionService = mock(); service = new OsBiometricsServiceWindows( i18nService, - null, + windowMain, logService, biometricStateService, encryptionService, @@ -81,7 +93,7 @@ describe("OsBiometricsServiceWindows", () => { cryptoFunctionService = mock(); service = new OsBiometricsServiceWindows( mock(), - null, + windowMain, mock(), biometricStateService, encryptionService, @@ -140,4 +152,97 @@ describe("OsBiometricsServiceWindows", () => { expect((service as any).clientKeyHalves.get(userId.toString())).toBeNull(); }); }); + + describe("deleteBiometricKey", () => { + const serviceName = "Bitwarden_biometric"; + const keyName = "test-user-id_user_biometric"; + const witnessKeyName = "test-user-id_user_biometric_witness"; + + it("should delete biometric key successfully", async () => { + await service.deleteBiometricKey(mockUserId); + + expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, keyName); + expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, witnessKeyName); + }); + + it.each([ + [false, false], + [false, true], + [true, false], + ])( + "should not throw error if key found: %s and witness key found: %s", + async (keyFound, witnessKeyFound) => { + passwords.deletePassword = jest.fn().mockImplementation((_, account) => { + if (account === keyName) { + if (!keyFound) { + throw new Error(passwords.PASSWORD_NOT_FOUND); + } + return Promise.resolve(); + } + if (account === witnessKeyName) { + if (!witnessKeyFound) { + throw new Error(passwords.PASSWORD_NOT_FOUND); + } + return Promise.resolve(); + } + throw new Error("Unexpected key"); + }); + + await service.deleteBiometricKey(mockUserId); + + expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, keyName); + expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, witnessKeyName); + if (!keyFound) { + expect(logService.debug).toHaveBeenCalledWith( + "[OsBiometricService] Biometric key %s not found for service %s.", + keyName, + serviceName, + ); + } + if (!witnessKeyFound) { + expect(logService.debug).toHaveBeenCalledWith( + "[OsBiometricService] Biometric witness key %s not found for service %s.", + witnessKeyName, + serviceName, + ); + } + }, + ); + + it("should throw error when deletePassword for key throws unexpected errors", async () => { + const error = new Error("Unexpected error"); + passwords.deletePassword = jest.fn().mockImplementation((_, account) => { + if (account === keyName) { + throw error; + } + if (account === witnessKeyName) { + return Promise.resolve(); + } + throw new Error("Unexpected key"); + }); + + await expect(service.deleteBiometricKey(mockUserId)).rejects.toThrow(error); + + expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, keyName); + expect(passwords.deletePassword).not.toHaveBeenCalledWith(serviceName, witnessKeyName); + }); + + it("should throw error when deletePassword for witness key throws unexpected errors", async () => { + const error = new Error("Unexpected error"); + passwords.deletePassword = jest.fn().mockImplementation((_, account) => { + if (account === keyName) { + return Promise.resolve(); + } + if (account === witnessKeyName) { + throw error; + } + throw new Error("Unexpected key"); + }); + + await expect(service.deleteBiometricKey(mockUserId)).rejects.toThrow(error); + + expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, keyName); + expect(passwords.deletePassword).toHaveBeenCalledWith(serviceName, witnessKeyName); + }); + }); }); diff --git a/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.ts b/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.ts index dc4f8674d7f..b1726644b0a 100644 --- a/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.ts +++ b/apps/desktop/src/key-management/biometrics/os-biometrics-windows.service.ts @@ -116,8 +116,32 @@ export default class OsBiometricsServiceWindows implements OsBiometricService { } async deleteBiometricKey(userId: UserId): Promise { - await passwords.deletePassword(SERVICE, getLookupKeyForUser(userId)); - await passwords.deletePassword(SERVICE, getLookupKeyForUser(userId) + KEY_WITNESS_SUFFIX); + try { + await passwords.deletePassword(SERVICE, getLookupKeyForUser(userId)); + } catch (e) { + if (e instanceof Error && e.message === passwords.PASSWORD_NOT_FOUND) { + this.logService.debug( + "[OsBiometricService] Biometric key %s not found for service %s.", + getLookupKeyForUser(userId), + SERVICE, + ); + } else { + throw e; + } + } + try { + await passwords.deletePassword(SERVICE, getLookupKeyForUser(userId) + KEY_WITNESS_SUFFIX); + } catch (e) { + if (e instanceof Error && e.message === passwords.PASSWORD_NOT_FOUND) { + this.logService.debug( + "[OsBiometricService] Biometric witness key %s not found for service %s.", + getLookupKeyForUser(userId) + KEY_WITNESS_SUFFIX, + SERVICE, + ); + } else { + throw e; + } + } } async authenticateBiometric(): Promise { @@ -227,8 +251,19 @@ export default class OsBiometricsServiceWindows implements OsBiometricService { storageKey + KEY_WITNESS_SUFFIX, witnessKeyMaterial, ); - } catch { - this.logService.debug("Error retrieving witness key, assuming value is not up to date."); + } catch (e) { + if (e instanceof Error && e.message === passwords.PASSWORD_NOT_FOUND) { + this.logService.debug( + "[OsBiometricService] Biometric witness key %s not found for service %s, value is not up to date.", + storageKey + KEY_WITNESS_SUFFIX, + service, + ); + } else { + this.logService.error( + "[OsBiometricService] Error retrieving witness key, assuming value is not up to date.", + e, + ); + } return false; } diff --git a/apps/desktop/src/platform/main/desktop-credential-storage-listener.ts b/apps/desktop/src/platform/main/desktop-credential-storage-listener.ts index ca4d9a2d3ca..6922911e367 100644 --- a/apps/desktop/src/platform/main/desktop-credential-storage-listener.ts +++ b/apps/desktop/src/platform/main/desktop-credential-storage-listener.ts @@ -35,13 +35,13 @@ export class DesktopCredentialStorageListener { } return val; } catch (e) { - if ( - e.message === "Password not found." || - e.message === "The specified item could not be found in the keychain." - ) { + if (e instanceof Error && e.message === passwords.PASSWORD_NOT_FOUND) { + if (message.action === "hasPassword") { + return false; + } return null; } - this.logService.info(e); + this.logService.error("[Credential Storage Listener] %s failed", message.action, e); } }); }