Skip to content

Commit a3d748a

Browse files
authored
[PM-19479] Client-managed Repository traits (#213)
## 🎟️ Tracking https://bitwarden.atlassian.net/browse/PM-19479 ## 📔 Objective Implement a generic trait for accessing the client application's data storage directly. Because we want the store access to be typed, but `bitwarden_core` isn't aware of the models, `bitwarden_core` only implements a generic way to set and retrieve generic `impl Repository<T>` instances, somewhat like dependency injection. Then it's up to each team/feature crates to define which models and which stores are available. This feature is created in a new `bitwarden-state`, which will be expanded by a separate PR with the addition of SDK-managed state (Sqlite+IndexedDB). At the moment this crate contains: - A `Repository` trait which will be implemented by the clients (and the SDK in the future), and a `RepositoryItem` marker trait which will be used to mark which models are meant to be used with the repositories. - A `StateRegistry` which stores all the client-managed Repositories, and in the future will also handle the SDK-managed repositories. - A new `StateClient` subclient under platform that will be used by the applications to register their Repositories. Both the WASM and UniFFI crates also need some conversion code to implement the `Repository` traits. I've tried to simplify it as much as possible, and hide it behind a macro when that wasn't possible. Some limitations on the current design: - The current integration with web clients requires the State Provider definition to be `UserKeyDefinition<Record<string, T>>` to match the key-value pattern in `Repository`. This usually matches with what is being used for vault (encrypted ciphers/folders, etc), but it might fall short on other domains, like profile data, or user keys. - There's no great way of ensuring that all the client-managed Repositories have been registered, other than keeping a list. I've tried to use the `inventory` crate to keep a global list of all the existing implementations and then validating that they've all been registered, but that doesn't work for WASM. We might be able to use the `inventory` crate and just run it in tests though. For some documentation on how to use it, you can check the README: https://github.com/bitwarden/sdk-internal/blob/ps/state-traits/crates/bitwarden-state/README.md The continuation of this is in #301, which contains SDK managed repository support. The client implementation of both these PRs is in bitwarden/clients#14839 ## ⏰ Reminders before review - Contributor guidelines followed - All formatters and local linters executed and passed - Written new unit and / or integration tests where applicable - Protected functional changes with optionality (feature flags) - Used internationalization (i18n) for all UI strings - CI builds passed - Communicated to DevOps any deployment requirements - Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team ## 🦮 Reviewer guidelines <!-- Suggested interactions but feel free to use (or not) as you desire! --> - 👍 (`:+1:`) or similar for great changes - 📝 (`:memo:`) or ℹ️ (`:information_source:`) for notes or general info - ❓ (`:question:`) for questions - 🤔 (`:thinking:`) or 💭 (`:thought_balloon:`) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion - 🎨 (`:art:`) for suggestions / improvements - ❌ (`:x:`) or ⚠️ (`:warning:`) for more significant problems or concerns needing attention - 🌱 (`:seedling:`) or ♻️ (`:recycle:`) for future improvements or indications of technical debt - ⛏ (`:pick:`) for minor or nitpick changes
1 parent 6b19644 commit a3d748a

File tree

25 files changed

+843
-4
lines changed

25 files changed

+843
-4
lines changed

Cargo.lock

Lines changed: 17 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ bitwarden-fido = { path = "crates/bitwarden-fido", version = "=1.0.0" }
3131
bitwarden-generators = { path = "crates/bitwarden-generators", version = "=1.0.0" }
3232
bitwarden-ipc = { path = "crates/bitwarden-ipc", version = "=1.0.0" }
3333
bitwarden-send = { path = "crates/bitwarden-send", version = "=1.0.0" }
34+
bitwarden-state = { path = "crates/bitwarden-state", version = "=1.0.0" }
3435
bitwarden-threading = { path = "crates/bitwarden-threading", version = "=1.0.0" }
3536
bitwarden-sm = { path = "bitwarden_license/bitwarden-sm", version = "=1.0.0" }
3637
bitwarden-ssh = { path = "crates/bitwarden-ssh", version = "=1.0.0" }
@@ -39,6 +40,7 @@ bitwarden-uuid-macro = { path = "crates/bitwarden-uuid-macro", version = "=1.0.0
3940
bitwarden-vault = { path = "crates/bitwarden-vault", version = "=1.0.0" }
4041

4142
# External crates that are expected to maintain a consistent version across all crates
43+
async-trait = ">=0.1.80, <0.2"
4244
chrono = { version = ">=0.4.26, <0.5", features = [
4345
"clock",
4446
"serde",

crates/bitwarden-core/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ bitwarden-api-api = { workspace = true }
3434
bitwarden-api-identity = { workspace = true }
3535
bitwarden-crypto = { workspace = true }
3636
bitwarden-error = { workspace = true }
37+
bitwarden-state = { workspace = true }
3738
bitwarden-uuid = { workspace = true }
3839
chrono = { workspace = true, features = ["std"] }
3940
# We don't use this directly (it's used by rand), but we need it here to enable WASM support

crates/bitwarden-core/src/client/client.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use std::sync::{Arc, OnceLock, RwLock};
22

33
use bitwarden_crypto::KeyStore;
4+
#[cfg(feature = "internal")]
5+
use bitwarden_state::registry::StateRegistry;
46
use reqwest::header::{self, HeaderValue};
57

68
use super::internal::InternalClient;
@@ -90,6 +92,8 @@ impl Client {
9092
})),
9193
external_client,
9294
key_store: KeyStore::default(),
95+
#[cfg(feature = "internal")]
96+
repository_map: StateRegistry::new(),
9397
}),
9498
}
9599
}

crates/bitwarden-core/src/client/internal.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ use bitwarden_crypto::KeyStore;
55
use bitwarden_crypto::SymmetricCryptoKey;
66
#[cfg(feature = "internal")]
77
use bitwarden_crypto::{EncString, Kdf, MasterKey, PinKey, UnsignedSharedKey};
8+
#[cfg(feature = "internal")]
9+
use bitwarden_state::registry::StateRegistry;
810
use chrono::Utc;
911
use uuid::Uuid;
1012

@@ -62,6 +64,9 @@ pub struct InternalClient {
6264
pub(crate) external_client: reqwest::Client,
6365

6466
pub(super) key_store: KeyStore<KeyIds>,
67+
68+
#[cfg(feature = "internal")]
69+
pub(crate) repository_map: StateRegistry,
6570
}
6671

6772
impl InternalClient {

crates/bitwarden-core/src/platform/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ mod generate_fingerprint;
66
mod get_user_api_key;
77
mod platform_client;
88
mod secret_verification_request;
9+
mod state_client;
910

1011
pub use generate_fingerprint::{
1112
FingerprintError, FingerprintRequest, FingerprintResponse, UserFingerprintError,
@@ -14,3 +15,4 @@ pub(crate) use get_user_api_key::get_user_api_key;
1415
pub use get_user_api_key::{UserApiKeyError, UserApiKeyResponse};
1516
pub use platform_client::PlatformClient;
1617
pub use secret_verification_request::SecretVerificationRequest;
18+
pub use state_client::StateClient;

crates/bitwarden-core/src/platform/platform_client.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@ impl PlatformClient {
3333
) -> Result<UserApiKeyResponse, UserApiKeyError> {
3434
get_user_api_key(&self.client, &input).await
3535
}
36+
37+
/// Access to state functionality.
38+
pub fn state(&self) -> super::StateClient {
39+
super::StateClient {
40+
client: self.client.clone(),
41+
}
42+
}
3643
}
3744

3845
impl Client {
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
use std::sync::Arc;
2+
3+
use bitwarden_state::repository::{Repository, RepositoryItem};
4+
5+
use crate::Client;
6+
7+
/// Wrapper for state specific functionality.
8+
pub struct StateClient {
9+
pub(crate) client: Client,
10+
}
11+
12+
impl StateClient {
13+
/// Register a client managed state repository for a specific type.
14+
pub fn register_client_managed<T: 'static + Repository<V>, V: RepositoryItem>(
15+
&self,
16+
store: Arc<T>,
17+
) {
18+
self.client
19+
.internal
20+
.repository_map
21+
.register_client_managed(store)
22+
}
23+
24+
/// Get a client managed state repository for a specific type, if it exists.
25+
pub fn get_client_managed<T: RepositoryItem>(&self) -> Option<Arc<dyn Repository<T>>> {
26+
self.client.internal.repository_map.get_client_managed()
27+
}
28+
}

crates/bitwarden-fido/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ keywords.workspace = true
1818
uniffi = ["dep:uniffi", "bitwarden-core/uniffi", "bitwarden-vault/uniffi"]
1919

2020
[dependencies]
21-
async-trait = ">=0.1.80, <0.2"
21+
async-trait = { workspace = true }
2222
base64 = ">=0.22.1, <0.23"
2323
bitwarden-core = { workspace = true }
2424
bitwarden-crypto = { workspace = true }

crates/bitwarden-state/Cargo.toml

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
[package]
2+
name = "bitwarden-state"
3+
version.workspace = true
4+
authors.workspace = true
5+
edition.workspace = true
6+
rust-version.workspace = true
7+
homepage.workspace = true
8+
repository.workspace = true
9+
license-file.workspace = true
10+
keywords.workspace = true
11+
12+
[features]
13+
uniffi = []
14+
wasm = []
15+
16+
[dependencies]
17+
async-trait = { workspace = true }
18+
thiserror = { workspace = true }
19+
20+
[dev-dependencies]
21+
tokio = { workspace = true, features = ["rt"] }
22+
23+
[lints]
24+
workspace = true

0 commit comments

Comments
 (0)