From 6b5d953c9488c6f146bbc80afc19afcdbdd7163b Mon Sep 17 00:00:00 2001 From: JaredScar Date: Wed, 12 Aug 2026 16:41:12 -0400 Subject: [PATCH] Implement CollectionsClient for managing collection encryption and decryption - Added `CollectionsClient` to handle encrypting and decrypting collections. - Introduced `CollectionEncryptError` for managing encryption errors. - Updated `Cargo.toml` to include `tokio` and `bitwarden-collections` as dependencies. - Enhanced `bitwarden-pm` to expose `CollectionsClient` for top-level access. - Updated relevant modules and tests to support new functionality. This change improves the SDK's capability to manage collections securely and efficiently. --- Cargo.lock | 2 + crates/bitwarden-collections/Cargo.toml | 4 + .../src/collection_client.rs | 86 +++++++++---------- crates/bitwarden-collections/src/error.rs | 13 +++ crates/bitwarden-collections/src/lib.rs | 5 ++ crates/bitwarden-pm/Cargo.toml | 3 + crates/bitwarden-pm/src/lib.rs | 10 +++ crates/bitwarden-uniffi/src/error.rs | 6 ++ crates/bitwarden-uniffi/src/lib.rs | 9 ++ .../bitwarden-uniffi/src/vault/collections.rs | 6 +- crates/bitwarden-vault/src/lib.rs | 2 - crates/bitwarden-vault/src/vault_client.rs | 10 ++- crates/bitwarden-wasm-internal/src/client.rs | 9 ++ 13 files changed, 113 insertions(+), 52 deletions(-) rename crates/{bitwarden-vault => bitwarden-collections}/src/collection_client.rs (77%) diff --git a/Cargo.lock b/Cargo.lock index c5eb55f8c4..ad206539e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -601,6 +601,7 @@ dependencies = [ "serde", "serde_repr", "thiserror 2.0.19", + "tokio", "tsify", "uniffi", "uuid", @@ -1065,6 +1066,7 @@ version = "3.0.0" dependencies = [ "async-trait", "bitwarden-auth", + "bitwarden-collections", "bitwarden-commercial-vault", "bitwarden-core", "bitwarden-crypto", diff --git a/crates/bitwarden-collections/Cargo.toml b/crates/bitwarden-collections/Cargo.toml index 8afd135cd4..689e912bfa 100644 --- a/crates/bitwarden-collections/Cargo.toml +++ b/crates/bitwarden-collections/Cargo.toml @@ -32,5 +32,9 @@ uniffi = { workspace = true, optional = true } uuid = { workspace = true } wasm-bindgen = { workspace = true, optional = true } +[dev-dependencies] +bitwarden-core = { workspace = true, features = ["internal", "test-fixtures"] } +tokio = { workspace = true, features = ["rt"] } + [lints] workspace = true diff --git a/crates/bitwarden-vault/src/collection_client.rs b/crates/bitwarden-collections/src/collection_client.rs similarity index 77% rename from crates/bitwarden-vault/src/collection_client.rs rename to crates/bitwarden-collections/src/collection_client.rs index 90fd9788eb..f569f23e6b 100644 --- a/crates/bitwarden-vault/src/collection_client.rs +++ b/crates/bitwarden-collections/src/collection_client.rs @@ -1,10 +1,6 @@ use std::collections::HashMap; -use bitwarden_collections::{ - collection::{Collection, CollectionId, CollectionView}, - tree::{NodeItem, Tree}, -}; -use bitwarden_core::Client; +use bitwarden_core::{Client, FromClient}; #[cfg(feature = "wasm")] use serde::{Deserialize, Serialize}; #[cfg(feature = "wasm")] @@ -12,7 +8,11 @@ use tsify::Tsify; #[cfg(feature = "wasm")] use wasm_bindgen::prelude::wasm_bindgen; -use crate::{DecryptError, EncryptError}; +use crate::{ + collection::{Collection, CollectionId, CollectionView}, + error::{CollectionDecryptError, CollectionEncryptError}, + tree::{NodeItem, Tree}, +}; #[allow(missing_docs)] #[cfg_attr(feature = "wasm", wasm_bindgen)] @@ -21,10 +21,21 @@ pub struct CollectionsClient { pub(crate) client: Client, } +impl FromClient for CollectionsClient { + fn from_client(client: &Client) -> Self { + Self { + client: client.clone(), + } + } +} + #[cfg_attr(feature = "wasm", wasm_bindgen)] impl CollectionsClient { /// Encrypts a [CollectionView] into an encrypted [Collection] using the organization key. - pub fn encrypt(&self, collection_view: CollectionView) -> Result { + pub fn encrypt( + &self, + collection_view: CollectionView, + ) -> Result { let key_store = self.client.internal.get_key_store(); let collection = key_store.encrypt(collection_view)?; Ok(collection) @@ -35,14 +46,17 @@ impl CollectionsClient { pub fn encrypt_list( &self, collection_views: Vec, - ) -> Result, EncryptError> { + ) -> Result, CollectionEncryptError> { let key_store = self.client.internal.get_key_store(); let collections = key_store.encrypt_list(&collection_views)?; Ok(collections) } #[allow(missing_docs)] - pub fn decrypt(&self, collection: Collection) -> Result { + pub fn decrypt( + &self, + collection: Collection, + ) -> Result { let key_store = self.client.internal.get_key_store(); let view = key_store.decrypt(&collection)?; Ok(view) @@ -52,7 +66,7 @@ impl CollectionsClient { pub fn decrypt_list( &self, collections: Vec, - ) -> Result, DecryptError> { + ) -> Result, CollectionDecryptError> { let key_store = self.client.internal.get_key_store(); let views = key_store.decrypt_list(&collections)?; Ok(views) @@ -144,11 +158,10 @@ impl CollectionViewTree { #[cfg(test)] mod tests { - use bitwarden_collections::collection::CollectionType; use bitwarden_core::client::test_accounts::test_bitwarden_com_account; use super::*; - use crate::VaultClientExt; + use crate::collection::CollectionType; fn test_collection() -> Collection { Collection { @@ -164,49 +177,42 @@ mod tests { } } + async fn test_collections_client() -> CollectionsClient { + let client = Client::init_test_account(test_bitwarden_com_account()).await; + CollectionsClient::from_client(&client) + } + #[tokio::test] async fn test_decrypt_list() { - let client = Client::init_test_account(test_bitwarden_com_account()).await; + let collections = test_collections_client().await; - let dec = client - .vault() - .collections() - .decrypt_list(vec![test_collection()]) - .unwrap(); + let dec = collections.decrypt_list(vec![test_collection()]).unwrap(); assert_eq!(dec[0].name, "Default collection"); } #[tokio::test] async fn test_decrypt() { - let client = Client::init_test_account(test_bitwarden_com_account()).await; + let collections = test_collections_client().await; - let dec = client - .vault() - .collections() - .decrypt(test_collection()) - .unwrap(); + let dec = collections.decrypt(test_collection()).unwrap(); assert_eq!(dec.name, "Default collection"); } #[tokio::test] async fn test_encrypt_decrypt_roundtrip() { - let client = Client::init_test_account(test_bitwarden_com_account()).await; + let collections = test_collections_client().await; - let view = client - .vault() - .collections() - .decrypt(test_collection()) - .unwrap(); + let view = collections.decrypt(test_collection()).unwrap(); assert_eq!(view.name, "Default collection"); // Re-encrypt the decrypted view, then decrypt again let expected_id = view.id; let expected_org_id = view.organization_id; - let re_encrypted = client.vault().collections().encrypt(view).unwrap(); - let re_decrypted = client.vault().collections().decrypt(re_encrypted).unwrap(); + let re_encrypted = collections.encrypt(view).unwrap(); + let re_decrypted = collections.decrypt(re_encrypted).unwrap(); assert_eq!(re_decrypted.name, "Default collection"); assert_eq!(re_decrypted.id, expected_id); @@ -215,13 +221,9 @@ mod tests { #[tokio::test] async fn test_encrypt_list_decrypt_list_roundtrip() { - let client = Client::init_test_account(test_bitwarden_com_account()).await; + let collections = test_collections_client().await; - let views = client - .vault() - .collections() - .decrypt_list(vec![test_collection()]) - .unwrap(); + let views = collections.decrypt_list(vec![test_collection()]).unwrap(); assert_eq!(views.len(), 1); assert_eq!(views[0].name, "Default collection"); @@ -229,15 +231,11 @@ mod tests { let expected_id = views[0].id; let expected_org_id = views[0].organization_id; - let re_encrypted = client.vault().collections().encrypt_list(views).unwrap(); + let re_encrypted = collections.encrypt_list(views).unwrap(); assert_eq!(re_encrypted.len(), 1); - let re_decrypted = client - .vault() - .collections() - .decrypt_list(re_encrypted) - .unwrap(); + let re_decrypted = collections.decrypt_list(re_encrypted).unwrap(); assert_eq!(re_decrypted.len(), 1); assert_eq!(re_decrypted[0].name, "Default collection"); diff --git a/crates/bitwarden-collections/src/error.rs b/crates/bitwarden-collections/src/error.rs index 638aa75b1e..0780d2678d 100644 --- a/crates/bitwarden-collections/src/error.rs +++ b/crates/bitwarden-collections/src/error.rs @@ -9,6 +9,19 @@ pub enum CollectionDecryptError { Crypto(#[from] bitwarden_crypto::CryptoError), } +/// Generic error type for collection encryption errors. +/// +/// This intentionally mirrors `bitwarden_vault::EncryptError` rather than depending on it, to +/// avoid creating a circular dependency between the `bitwarden-collections` and `bitwarden-vault` +/// crates. +#[allow(missing_docs)] +#[bitwarden_error(flat)] +#[derive(Debug, Error)] +pub enum CollectionEncryptError { + #[error(transparent)] + Crypto(#[from] bitwarden_crypto::CryptoError), +} + #[allow(missing_docs)] #[derive(Debug, Error)] pub enum CollectionsParseError { diff --git a/crates/bitwarden-collections/src/lib.rs b/crates/bitwarden-collections/src/lib.rs index 9a44e04a63..f6b368305d 100644 --- a/crates/bitwarden-collections/src/lib.rs +++ b/crates/bitwarden-collections/src/lib.rs @@ -10,6 +10,11 @@ mod uniffi_support; /// Encryptable, TryFrom, and TreeItem pub mod collection; /// +/// Module containing the [CollectionsClient](collection_client::CollectionsClient), which exposes +/// encrypt/decrypt operations for collections. +#[allow(missing_docs)] +pub mod collection_client; +/// /// Module containing the error types. pub mod error; /// diff --git a/crates/bitwarden-pm/Cargo.toml b/crates/bitwarden-pm/Cargo.toml index 3eb310d7ee..2669949597 100644 --- a/crates/bitwarden-pm/Cargo.toml +++ b/crates/bitwarden-pm/Cargo.toml @@ -18,6 +18,7 @@ keywords.workspace = true cli = ["bitwarden-unlock/cli"] no-memory-hardening = ["bitwarden-core/no-memory-hardening"] uniffi = [ + "bitwarden-collections/uniffi", "bitwarden-core/uniffi", "bitwarden-crypto-cipher-suite/uniffi", "bitwarden-crypto-sync-handler/uniffi", @@ -33,6 +34,7 @@ uniffi = [ ] wasm = [ "bitwarden-auth/wasm", + "bitwarden-collections/wasm", "bitwarden-commercial-vault/wasm", "bitwarden-core/wasm", "bitwarden-crypto-cipher-suite/wasm", @@ -56,6 +58,7 @@ bitwarden-license = ["dep:bitwarden-commercial-vault", "dep:bitwarden-pam"] [dependencies] async-trait = { workspace = true } bitwarden-auth = { workspace = true } +bitwarden-collections = { workspace = true } bitwarden-commercial-vault = { workspace = true, optional = true } bitwarden-core = { workspace = true, features = ["internal"] } bitwarden-crypto = { workspace = true } diff --git a/crates/bitwarden-pm/src/lib.rs b/crates/bitwarden-pm/src/lib.rs index 8b4b71876d..6e18863d4b 100644 --- a/crates/bitwarden-pm/src/lib.rs +++ b/crates/bitwarden-pm/src/lib.rs @@ -31,6 +31,7 @@ uniffi::setup_scaffolding!(); /// Re-export subclients for easier access pub mod clients { pub use bitwarden_auth::AuthClient; + pub use bitwarden_collections::collection_client::CollectionsClient; pub use bitwarden_core::key_management::CryptoClient; pub use bitwarden_crypto_cipher_suite::CryptoCipherSuiteClient; pub use bitwarden_crypto_sync_handler::CryptoSyncHandlerClient; @@ -147,6 +148,15 @@ impl PasswordManagerClient { self.0.vault() } + /// Collection related operations. + /// + /// This is registered directly on the top-level client in addition to being nested under + /// [`vault`](Self::vault); once all consumers have migrated to this accessor, the nested one + /// will be removed. + pub fn collections(&self) -> bitwarden_collections::collection_client::CollectionsClient { + bitwarden_collections::collection_client::CollectionsClient::from_client(&self.0) + } + /// Exporter operations pub fn exporters(&self) -> bitwarden_exporters::ExporterClient { self.0.exporters() diff --git a/crates/bitwarden-uniffi/src/error.rs b/crates/bitwarden-uniffi/src/error.rs index d43b0b63cf..4aeafc7559 100644 --- a/crates/bitwarden-uniffi/src/error.rs +++ b/crates/bitwarden-uniffi/src/error.rs @@ -68,6 +68,12 @@ pub enum BitwardenError { #[error(transparent)] EncryptFile(#[from] bitwarden_vault::EncryptFileError), + // Collections + #[error(transparent)] + CollectionDecrypt(#[from] bitwarden_collections::error::CollectionDecryptError), + #[error(transparent)] + CollectionEncrypt(#[from] bitwarden_collections::error::CollectionEncryptError), + // Send #[error(transparent)] SendDecrypt(#[from] bitwarden_send::SendDecryptError), diff --git a/crates/bitwarden-uniffi/src/lib.rs b/crates/bitwarden-uniffi/src/lib.rs index 40213b9626..0bdbecddd6 100644 --- a/crates/bitwarden-uniffi/src/lib.rs +++ b/crates/bitwarden-uniffi/src/lib.rs @@ -92,6 +92,15 @@ impl Client { VaultClient(self.0.vault()) } + /// Collection related operations. + /// + /// This is registered directly on the top-level client in addition to being nested under + /// [`vault`](Self::vault). Once mobile clients have migrated to this accessor, the nested one + /// will be removed. + pub fn collections(&self) -> vault::collections::CollectionsClient { + vault::collections::CollectionsClient(self.0.collections()) + } + #[allow(missing_docs)] pub fn platform(&self) -> PlatformClient { PlatformClient(self.0.0.clone()) diff --git a/crates/bitwarden-uniffi/src/vault/collections.rs b/crates/bitwarden-uniffi/src/vault/collections.rs index 5e63cc30c2..e9d9c346af 100644 --- a/crates/bitwarden-uniffi/src/vault/collections.rs +++ b/crates/bitwarden-uniffi/src/vault/collections.rs @@ -2,15 +2,17 @@ use std::sync::Arc; use bitwarden_collections::{ collection::{Collection, CollectionId, CollectionView}, + collection_client::AncestorMap, tree::{NodeItem, Tree}, }; -use bitwarden_vault::collection_client::AncestorMap; use crate::Result; #[allow(missing_docs)] #[derive(uniffi::Object)] -pub struct CollectionsClient(pub(crate) bitwarden_vault::collection_client::CollectionsClient); +pub struct CollectionsClient( + pub(crate) bitwarden_collections::collection_client::CollectionsClient, +); #[uniffi::export] impl CollectionsClient { diff --git a/crates/bitwarden-vault/src/lib.rs b/crates/bitwarden-vault/src/lib.rs index cb632d03ef..223f740fe8 100644 --- a/crates/bitwarden-vault/src/lib.rs +++ b/crates/bitwarden-vault/src/lib.rs @@ -26,8 +26,6 @@ pub use error::{DecryptError, EncryptError, VaultParseError}; mod vault_client; pub use vault_client::{VaultClient, VaultClientExt}; -#[allow(missing_docs)] -pub mod collection_client; mod totp_client; pub use totp_client::TotpClient; diff --git a/crates/bitwarden-vault/src/vault_client.rs b/crates/bitwarden-vault/src/vault_client.rs index 8b60a9fdbb..9340f62001 100644 --- a/crates/bitwarden-vault/src/vault_client.rs +++ b/crates/bitwarden-vault/src/vault_client.rs @@ -1,10 +1,11 @@ +use bitwarden_collections::collection_client::CollectionsClient; use bitwarden_core::{Client, FromClient}; #[cfg(feature = "wasm")] use wasm_bindgen::prelude::*; use crate::{ AttachmentsClient, CipherRiskClient, CiphersClient, FoldersClient, PasswordHistoryClient, - TotpClient, collection_client::CollectionsClient, + TotpClient, }; #[allow(missing_docs)] @@ -52,10 +53,11 @@ impl VaultClient { } /// Collection related operations. + /// + /// This nested accessor is kept for backwards compatibility. New callers should prefer the + /// `collections()` accessor registered directly on the top-level Password Manager client. pub fn collections(&self) -> CollectionsClient { - CollectionsClient { - client: self.client.clone(), - } + CollectionsClient::from_client(&self.client) } /// Cipher risk evaluation operations. diff --git a/crates/bitwarden-wasm-internal/src/client.rs b/crates/bitwarden-wasm-internal/src/client.rs index 8a7d61652e..fe1848b124 100644 --- a/crates/bitwarden-wasm-internal/src/client.rs +++ b/crates/bitwarden-wasm-internal/src/client.rs @@ -99,6 +99,15 @@ impl PasswordManagerClient { self.0.vault() } + /// Collection related operations. + /// + /// This is registered directly on the top-level client in addition to being nested under + /// [`vault`](Self::vault). Once consumers have migrated to this accessor, the nested one will + /// be removed. + pub fn collections(&self) -> CollectionsClient { + self.0.collections() + } + /// Constructs a specific client for platform-specific functionality pub fn platform(&self) -> PlatformClient { PlatformClient::new(self.0.0.clone())