[PM-37487] Move collections client ownership to Admin Console - #1380
[PM-37487] Move collections client ownership to Admin Console#1380JaredScar wants to merge 1 commit into
Conversation
…cryption - 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.
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE This PR moves Code Review Details
Dependency Changes
All three are existing first-party or already-approved workspace dependencies, so no AppSec dependency review is required. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| /// 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), | ||
| } |
There was a problem hiding this comment.
Details and impact
CollectionsClient::{encrypt,encrypt_list} moved from bitwarden_vault::EncryptError to CollectionEncryptError, and {decrypt,decrypt_list} from bitwarden_vault::DecryptError to CollectionDecryptError. Because #[bitwarden_error(flat)] derives the binding surface from the enum name, this is consumer-visible on both bindings:
- WASM/TS: the thrown error's
namebecomes"CollectionEncryptError"/"CollectionDecryptError", and the generated guard is nowisCollectionEncryptError()/isCollectionDecryptError(). ExistingisEncryptError(e)/isDecryptError(e)checks around collection calls now returnfalsewith no compile error. - UniFFI:
bitwarden-uniffi/src/error.rsroutes these through newBitwardenError::CollectionEncrypt/CollectionDecryptvariants, socatch (e: BitwardenException.Encrypt)/BitwardenException.Decryptin Kotlin/Swift stops matching — also without a compile error.
The accessor move in this PR keeps vault().collections() around for a staged migration, but the error rename has no equivalent shim, so client-side error handling breaks the moment this version is consumed. Renaming to keep EncryptError via export_as is not viable (it would collide with bitwarden_vault::EncryptError in the same wasm module), so this likely needs a coordinated client change — worth calling out in the PR description / linked ticket so web and mobile pick it up.
🔍 SDK Breaking Change DetectionSDK Version:
Breaking change detection uses the build of the SDK from this branch, including any incompatibities pre-existing on or merged into this branch. Check the workflow logs to confirm. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1380 +/- ##
==========================================
- Coverage 86.28% 86.27% -0.02%
==========================================
Files 500 500
Lines 73417 73419 +2
==========================================
- Hits 63349 63339 -10
- Misses 10068 10080 +12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2 participants
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-37487
📔 Objective
Move the collections client ownership over to Admin Console