From 3bf4916254e7b98e2aba02bec5ebce5a8cd4890a Mon Sep 17 00:00:00 2001 From: JaredScar Date: Tue, 24 Feb 2026 16:34:33 -0500 Subject: [PATCH] Refactor secure storage implementation to utilize the 'dc-native' module directly in the renderer process, removing the need for the credential storage listener. Update TypeScript definitions and enhance error handling in password management functions. Adjust Cargo dependencies and versions for improved compatibility. --- .../electronRendererSecureStorage.service.ts | 60 +++++---- native/Cargo.lock | 116 +++++++++--------- native/Cargo.toml | 6 +- native/index.js | 15 ++- native/src/lib.rs | 108 ++++++++-------- src/main/credential-storage-listener.ts | 34 +---- src/services/nativeSecureStorage.service.ts | 19 ++- webpack.main.cjs | 1 - 8 files changed, 184 insertions(+), 175 deletions(-) diff --git a/jslib/electron/src/services/electronRendererSecureStorage.service.ts b/jslib/electron/src/services/electronRendererSecureStorage.service.ts index 38d0f012..f0c7ebdb 100644 --- a/jslib/electron/src/services/electronRendererSecureStorage.service.ts +++ b/jslib/electron/src/services/electronRendererSecureStorage.service.ts @@ -1,43 +1,53 @@ -import { ipcRenderer } from "electron"; import { StorageService } from "@/jslib/common/src/abstractions/storage.service"; import { StorageOptions } from "@/jslib/common/src/models/domain/storageOptions"; +import { passwords } from "dc-native"; + +const APPLICATION_NAME = "Bitwarden Directory Connector"; + export class ElectronRendererSecureStorageService implements StorageService { async get(key: string, options?: StorageOptions): Promise { - const val = ipcRenderer.sendSync("nativeSecureStorage", { - action: "getPassword", - key: key, - keySuffix: options?.keySuffix ?? "", - }); - return Promise.resolve(val != null ? (JSON.parse(val) as T) : null); + return passwords + .getPassword(this.buildServiceName(options), key) + .then((val: string) => JSON.parse(val) as T) + .catch((e: Error): T => { + if (e.message === passwords.PASSWORD_NOT_FOUND) { + return null; + } + throw e; + }); } async has(key: string, options?: StorageOptions): Promise { - const val = ipcRenderer.sendSync("nativeSecureStorage", { - action: "hasPassword", - key: key, - keySuffix: options?.keySuffix ?? "", - }); - return Promise.resolve(!!val); + return (await this.get(key, options)) != null; } async save(key: string, obj: any, options?: StorageOptions): Promise { - ipcRenderer.sendSync("nativeSecureStorage", { - action: "setPassword", - key: key, - keySuffix: options?.keySuffix ?? "", - value: JSON.stringify(obj), - }); - return Promise.resolve(); + if (!obj) { + return this.remove(key, options); + } + return passwords.setPassword( + this.buildServiceName(options), + key, + JSON.stringify(obj), + ); } async remove(key: string, options?: StorageOptions): Promise { - ipcRenderer.sendSync("nativeSecureStorage", { - action: "deletePassword", - key: key, - keySuffix: options?.keySuffix ?? "", + return passwords.deletePassword(this.buildServiceName(options), key).catch((e: Error) => { + if (e.message === passwords.PASSWORD_NOT_FOUND) { + return; + } + throw e; }); - return Promise.resolve(); + } + + private buildServiceName(options?: StorageOptions): string { + const suffix = options?.keySuffix; + if (suffix) { + return `${APPLICATION_NAME}_${suffix}`; + } + return APPLICATION_NAME; } } diff --git a/native/Cargo.lock b/native/Cargo.lock index 32fea738..ec3be8d1 100644 --- a/native/Cargo.lock +++ b/native/Cargo.lock @@ -38,6 +38,15 @@ dependencies = [ "subtle", ] +[[package]] +name = "aho-corasick" +version = "1.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ddd31a130427c27518df266943a5308ed92d4b226cc639f5a8f1002816174301" +dependencies = [ + "memchr", +] + [[package]] name = "anyhow" version = "1.0.100" @@ -430,9 +439,9 @@ checksum = "c2459377285ad874054d797f3ccebf984978aa39129f6eafde5cdc8315b612f8" [[package]] name = "convert_case" -version = "0.8.0" +version = "0.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "baaaa0ecca5b51987b9423ccdc971514dd8b0bb7b4060b983d3664dad3f1f89f" +checksum = "ec182b0ca2f35d8fc196cf3404988fd8b8c739a4d270ff118a398feb0cbec1ca" dependencies = [ "unicode-segmentation", ] @@ -493,20 +502,14 @@ dependencies = [ [[package]] name = "ctor" -version = "0.5.0" +version = "0.2.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67773048316103656a637612c4a62477603b777d91d9c62ff2290f9cde178fdb" +checksum = "32a2785755761f3ddc1492979ce1e48d2c00d09311c39e4466429188f3dd6501" dependencies = [ - "ctor-proc-macro", - "dtor", + "quote", + "syn", ] -[[package]] -name = "ctor-proc-macro" -version = "0.0.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e2931af7e13dc045d8e9d26afccc6fa115d64e115c9c84b1166288b46f6782c2" - [[package]] name = "ctr" version = "0.9.2" @@ -694,21 +697,6 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "75b325c5dbd37f80359721ad39aca5a29fb04c89279657cffdda8736d0c0b9d2" -[[package]] -name = "dtor" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "404d02eeb088a82cfd873006cb713fe411306c7d182c344905e101fb1167d301" -dependencies = [ - "dtor-proc-macro", -] - -[[package]] -name = "dtor-proc-macro" -version = "0.0.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f678cf4a922c215c63e0de95eb1ff08a958a81d47e485cf9da1e27bf6305cfa5" - [[package]] name = "ecdsa" version = "0.16.9" @@ -1287,9 +1275,9 @@ checksum = "37c93d8daa9d8a012fd8ab92f088405fb202ea0b6ab73ee2482ae66af4f42091" [[package]] name = "libloading" -version = "0.9.0" +version = "0.8.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "754ca22de805bb5744484a5b151a9e1a8e837d5dc232c2d7d8c2e3492edc8b60" +checksum = "d7c4b02199fee7c5d21a5ae7d8cfa79a6ef5bb2fc834d6e9058e89c825efdc55" dependencies = [ "cfg-if", "windows-link 0.2.1", @@ -1397,33 +1385,32 @@ dependencies = [ [[package]] name = "napi" -version = "3.3.0" +version = "2.16.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f1b74e3dce5230795bb4d2821b941706dee733c7308752507254b0497f39cad7" +checksum = "55740c4ae1d8696773c78fdafd5d0e5fe9bc9f1b071c7ba493ba5c413a9184f3" dependencies = [ "bitflags", "ctor", - "napi-build", + "napi-derive", "napi-sys", - "nohash-hasher", - "rustc-hash", + "once_cell", "tokio", ] [[package]] name = "napi-build" -version = "2.2.3" +version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dcae8ad5609d14afb3a3b91dee88c757016261b151e9dcecabf1b2a31a6cab14" +checksum = "ebd4419172727423cf30351406c54f6cc1b354a2cfb4f1dba3e6cd07f6d5522b" [[package]] name = "napi-derive" -version = "3.2.5" +version = "2.16.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7552d5a579b834614bbd496db5109f1b9f1c758f08224b0dee1e408333adf0d0" +checksum = "7cbe2585d8ac223f7d34f13701434b9d5f4eb9c332cccce8dee57ea18ab8ab0c" dependencies = [ + "cfg-if", "convert_case", - "ctor", "napi-derive-backend", "proc-macro2", "quote", @@ -1432,22 +1419,24 @@ dependencies = [ [[package]] name = "napi-derive-backend" -version = "2.2.0" +version = "1.0.75" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5f6a81ac7486b70f2532a289603340862c06eea5a1e650c1ffeda2ce1238516a" +checksum = "1639aaa9eeb76e91c6ae66da8ce3e89e921cd3885e99ec85f4abacae72fc91bf" dependencies = [ "convert_case", + "once_cell", "proc-macro2", "quote", + "regex", "semver", "syn", ] [[package]] name = "napi-sys" -version = "3.2.1" +version = "2.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8eb602b84d7c1edae45e50bbf1374696548f36ae179dfa667f577e384bb90c2b" +checksum = "427802e8ec3a734331fec1035594a210ce1ff4dc5bc1950530920ab717964ea3" dependencies = [ "libloading", ] @@ -1465,12 +1454,6 @@ dependencies = [ "memoffset", ] -[[package]] -name = "nohash-hasher" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2bf50223579dc7cdcfb3bfcacf7069ff68243f8c363f62ffa99cf000a6b9c451" - [[package]] name = "nom" version = "8.0.0" @@ -2090,6 +2073,35 @@ dependencies = [ "thiserror 2.0.17", ] +[[package]] +name = "regex" +version = "1.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e10754a14b9137dd7b1e3e5b0493cc9171fdd105e0ab477f51b72e7f3ac0e276" +dependencies = [ + "aho-corasick", + "memchr", + "regex-automata", + "regex-syntax", +] + +[[package]] +name = "regex-automata" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e1dd4122fc1595e8162618945476892eefca7b88c52820e74af6262213cae8f" +dependencies = [ + "aho-corasick", + "memchr", + "regex-syntax", +] + +[[package]] +name = "regex-syntax" +version = "0.8.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc897dd8d9e8bd1ed8cdad82b5966c3e0ecae09fb1907d58efaa013543185d0a" + [[package]] name = "rfc6979" version = "0.4.0" @@ -2131,12 +2143,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "rustc-hash" -version = "2.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "357703d41365b4b27c590e3ed91eabb1b663f07c4c084095e60cbed4362dff0d" - [[package]] name = "rustc_version" version = "0.4.1" diff --git a/native/Cargo.toml b/native/Cargo.toml index 48093cea..439065d5 100644 --- a/native/Cargo.toml +++ b/native/Cargo.toml @@ -12,8 +12,8 @@ name = "dc_native" [dependencies] anyhow = "=1.0.100" desktop_core = { git = "https://github.com/bitwarden/clients", rev = "00cf24972d944638bbd1adc00a0ae3eeabb6eb9a", package = "desktop_core" } -napi = { version = "=3.3.0", features = ["async"] } -napi-derive = "=3.2.5" +napi = { version = "2", features = ["async"] } +napi-derive = "2" [target.'cfg(windows)'.dependencies] scopeguard = "=1.2.0" @@ -24,4 +24,4 @@ windows = { version = "=0.61.1", features = [ ] } [build-dependencies] -napi-build = "=2.2.3" +napi-build = "1" diff --git a/native/index.js b/native/index.js index 67563200..0eed45f1 100644 --- a/native/index.js +++ b/native/index.js @@ -66,4 +66,17 @@ if (!nativeBinding) { throw new Error(`Failed to load dc-native binding`); } -module.exports = nativeBinding; +// Re-export flat native symbols as the `passwords` namespace so all +// TypeScript callers (and the existing index.d.ts declarations) work unchanged. +module.exports = { + passwords: { + getPassword: (service, account) => nativeBinding.getPassword(service, account), + setPassword: (service, account, password) => + nativeBinding.setPassword(service, account, password), + deletePassword: (service, account) => nativeBinding.deletePassword(service, account), + isAvailable: () => nativeBinding.isAvailable(), + migrateKeytarPassword: (service, account) => + nativeBinding.migrateKeytarPassword(service, account), + PASSWORD_NOT_FOUND: "Password not found.", + }, +}; diff --git a/native/src/lib.rs b/native/src/lib.rs index 053eecee..0b076919 100644 --- a/native/src/lib.rs +++ b/native/src/lib.rs @@ -1,68 +1,62 @@ #[macro_use] extern crate napi_derive; + +/// Fetch the stored password from the keychain. +/// Throws an Error with message PASSWORD_NOT_FOUND if the password does not exist. #[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; +pub async fn get_password(service: String, account: String) -> napi::Result { + desktop_core::password::get_password(&service, &account) + .await + .map_err(|e| napi::Error::from_reason(e.to_string())) +} - /// Fetch the stored password from the keychain. - /// Throws an Error with message 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) +/// Save the password to the keychain. Adds an entry if none exists, otherwise updates it. +#[napi] +pub async fn set_password( + service: String, + account: String, + password: String, +) -> napi::Result<()> { + desktop_core::password::set_password(&service, &account, &password) + .await + .map_err(|e| napi::Error::from_reason(e.to_string())) +} + +/// Delete the stored password from the keychain. +/// Throws an Error with message 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) + .await + .map_err(|e| napi::Error::from_reason(e.to_string())) +} + +/// Check if OS secure storage is available. +#[napi] +pub async fn is_available() -> napi::Result { + desktop_core::password::is_available() + .await + .map_err(|e| napi::Error::from_reason(e.to_string())) +} + +/// Migrate a credential that was stored by keytar (UTF-8 blob) to the new UTF-16 format +/// used by desktop_core on Windows. No-ops on non-Windows platforms. +/// +/// Returns true if a migration was performed, false if the credential was already in the +/// correct format or does not exist. +#[napi] +pub async fn migrate_keytar_password(service: String, account: String) -> napi::Result { + #[cfg(windows)] + { + migration::migrate_keytar_password(&service, &account) .await .map_err(|e| napi::Error::from_reason(e.to_string())) } - - /// Save the password to the keychain. Adds an entry if none exists, otherwise updates it. - #[napi] - pub async fn set_password( - service: String, - account: String, - password: String, - ) -> napi::Result<()> { - desktop_core::password::set_password(&service, &account, &password) - .await - .map_err(|e| napi::Error::from_reason(e.to_string())) - } - - /// Delete the stored password from the keychain. - /// Throws an Error with message 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) - .await - .map_err(|e| napi::Error::from_reason(e.to_string())) - } - - /// Check if OS secure storage is available. - #[napi] - pub async fn is_available() -> napi::Result { - desktop_core::password::is_available() - .await - .map_err(|e| napi::Error::from_reason(e.to_string())) - } - - /// Migrate a credential that was stored by keytar (UTF-8 blob) to the new UTF-16 format - /// used by desktop_core on Windows. No-ops on non-Windows platforms. - /// - /// Returns true if a migration was performed, false if the credential was already in the - /// correct format or does not exist. - #[napi] - pub async fn migrate_keytar_password(service: String, account: String) -> napi::Result { - #[cfg(windows)] - { - crate::migration::migrate_keytar_password(&service, &account) - .await - .map_err(|e| napi::Error::from_reason(e.to_string())) - } - #[cfg(not(windows))] - { - let _ = (service, account); - Ok(false) - } + #[cfg(not(windows))] + { + let _ = (service, account); + Ok(false) } } diff --git a/src/main/credential-storage-listener.ts b/src/main/credential-storage-listener.ts index cfe15e58..97fc01fb 100644 --- a/src/main/credential-storage-listener.ts +++ b/src/main/credential-storage-listener.ts @@ -1,35 +1,11 @@ -import { passwords } from "dc-native"; -import { ipcMain } from "electron"; - +// Secure storage is handled directly in the renderer process via dc-native, +// so this listener is no longer needed. Kept as a stub to avoid refactoring +// the main.ts bootstrap sequence. export class DCCredentialStorageListener { + constructor(private serviceName: string) {} init() { - ipcMain.on("nativeSecureStorage", async (event: any, message: any) => { - try { - let serviceName = this.serviceName; - message.keySuffix = "_" + (message.keySuffix ?? ""); - if (message.keySuffix !== "_") { - serviceName += message.keySuffix; - } - - let val: string | boolean = null; - if (message.action && message.key) { - if (message.action === "getPassword") { - val = await passwords.getPassword(serviceName, message.key); - } else if (message.action === "hasPassword") { - const result = await passwords.getPassword(serviceName, message.key); - val = result != null; - } else if (message.action === "setPassword" && message.value) { - await passwords.setPassword(serviceName, message.key, message.value); - } else if (message.action === "deletePassword") { - await passwords.deletePassword(serviceName, message.key); - } - } - event.returnValue = val; - } catch { - event.returnValue = null; - } - }); + // no-op: renderer calls dc-native directly } } diff --git a/src/services/nativeSecureStorage.service.ts b/src/services/nativeSecureStorage.service.ts index fd9edee1..27b09ee2 100644 --- a/src/services/nativeSecureStorage.service.ts +++ b/src/services/nativeSecureStorage.service.ts @@ -7,9 +7,15 @@ export class NativeSecureStorageService implements StorageService { constructor(private serviceName: string) {} get(key: string): Promise { - return passwords.getPassword(this.serviceName, key).then((val: string) => { - return JSON.parse(val) as T; - }); + return passwords + .getPassword(this.serviceName, key) + .then((val: string) => JSON.parse(val) as T) + .catch((e: Error): T => { + if (e.message === passwords.PASSWORD_NOT_FOUND) { + return null; + } + throw e; + }); } async has(key: string): Promise { @@ -24,6 +30,11 @@ export class NativeSecureStorageService implements StorageService { } remove(key: string): Promise { - return passwords.deletePassword(this.serviceName, key); + return passwords.deletePassword(this.serviceName, key).catch((e: Error) => { + if (e.message === passwords.PASSWORD_NOT_FOUND) { + return; + } + throw e; + }); } } diff --git a/webpack.main.cjs b/webpack.main.cjs index 256eff5a..070a6be9 100644 --- a/webpack.main.cjs +++ b/webpack.main.cjs @@ -58,7 +58,6 @@ const main = { ], externals: { "electron-reload": "commonjs2 electron-reload", - "dc-native": "commonjs2 dc-native", }, };