diff --git a/Cargo.lock b/Cargo.lock index 4f6d5069f..8a2cadf3a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -678,6 +678,14 @@ dependencies = [ "tsify", ] +[[package]] +name = "bitwarden-state-migrations" +version = "1.0.0" +dependencies = [ + "bitwarden-state", + "bitwarden-vault", +] + [[package]] name = "bitwarden-test" version = "1.0.0" @@ -726,6 +734,7 @@ dependencies = [ "bitwarden-send", "bitwarden-ssh", "bitwarden-state", + "bitwarden-state-migrations", "bitwarden-uniffi-error", "bitwarden-vault", "chrono", @@ -816,6 +825,7 @@ dependencies = [ "bitwarden-ipc", "bitwarden-ssh", "bitwarden-state", + "bitwarden-state-migrations", "bitwarden-threading", "bitwarden-vault", "console_error_panic_hook", diff --git a/Cargo.toml b/Cargo.toml index 827edd519..93ff2770e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,7 @@ bitwarden-send = { path = "crates/bitwarden-send", version = "=1.0.0" } bitwarden-sm = { path = "bitwarden_license/bitwarden-sm", version = "=1.0.0" } bitwarden-ssh = { path = "crates/bitwarden-ssh", version = "=1.0.0" } bitwarden-state = { path = "crates/bitwarden-state", version = "=1.0.0" } +bitwarden-state-migrations = { path = "crates/bitwarden-state-migrations", version = "=1.0.0" } bitwarden-test = { path = "crates/bitwarden-test", version = "=1.0.0" } bitwarden-threading = { path = "crates/bitwarden-threading", version = "=1.0.0" } bitwarden-uniffi-error = { path = "crates/bitwarden-uniffi-error", version = "=1.0.0" } diff --git a/crates/bitwarden-core/src/platform/state_client.rs b/crates/bitwarden-core/src/platform/state_client.rs index 95b1b5cc8..819a57e14 100644 --- a/crates/bitwarden-core/src/platform/state_client.rs +++ b/crates/bitwarden-core/src/platform/state_client.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use bitwarden_state::{ registry::{RepositoryNotFoundError, StateRegistryError}, - repository::{Repository, RepositoryItem, RepositoryItemData}, + repository::{Repository, RepositoryItem, RepositoryMigrations}, DatabaseConfiguration, }; @@ -36,12 +36,12 @@ impl StateClient { pub async fn initialize_database( &self, configuration: DatabaseConfiguration, - repositories: Vec, + migrations: RepositoryMigrations, ) -> Result<(), StateRegistryError> { self.client .internal .repository_map - .initialize_database(configuration, repositories) + .initialize_database(configuration, migrations) .await } diff --git a/crates/bitwarden-state-migrations/Cargo.toml b/crates/bitwarden-state-migrations/Cargo.toml new file mode 100644 index 000000000..f0cadc8d0 --- /dev/null +++ b/crates/bitwarden-state-migrations/Cargo.toml @@ -0,0 +1,21 @@ +[package] +name = "bitwarden-state-migrations" +version.workspace = true +authors.workspace = true +edition.workspace = true +rust-version.workspace = true +homepage.workspace = true +repository.workspace = true +license-file.workspace = true +keywords.workspace = true + +[features] +uniffi = ["bitwarden-vault/uniffi"] +wasm = ["bitwarden-vault/wasm"] + +[dependencies] +bitwarden-state = { workspace = true } +bitwarden-vault = { workspace = true } + +[lints] +workspace = true diff --git a/crates/bitwarden-state-migrations/README.md b/crates/bitwarden-state-migrations/README.md new file mode 100644 index 000000000..a0fa7635f --- /dev/null +++ b/crates/bitwarden-state-migrations/README.md @@ -0,0 +1,4 @@ +# bitwarden-state-migrations + +This crate contains migrations for the SDK-managed state framework. It should only be imported by +the final application crates (`bitwarden-wasm-internal` and `bitwarden-uniffi`) diff --git a/crates/bitwarden-state-migrations/src/lib.rs b/crates/bitwarden-state-migrations/src/lib.rs new file mode 100644 index 000000000..b6585cac2 --- /dev/null +++ b/crates/bitwarden-state-migrations/src/lib.rs @@ -0,0 +1,15 @@ +#![doc = include_str!("../README.md")] + +use bitwarden_state::repository::{RepositoryItem, RepositoryMigrationStep, RepositoryMigrations}; +use bitwarden_vault::{Cipher, Folder}; + +/// Returns a list of all SDK-managed repository migrations. +pub fn get_sdk_managed_migrations() -> RepositoryMigrations { + use RepositoryMigrationStep::*; + RepositoryMigrations::new(vec![ + // Add any new migrations here. Note that order matters, and that removing a repository + // requires a separate migration step using `Remove(...)`. + Add(Cipher::data()), + Add(Folder::data()), + ]) +} diff --git a/crates/bitwarden-state/README.md b/crates/bitwarden-state/README.md index 4ae324985..bda45c114 100644 --- a/crates/bitwarden-state/README.md +++ b/crates/bitwarden-state/README.md @@ -182,65 +182,23 @@ getClient(userId = userId).platform().store().registerCipherStore(CipherStoreImp With `SDK-Managed State`, the SDK will be exclusively responsible for the data storage. This means that the clients don't need to make any changes themselves, as the implementation is internal to the -SDK. To add support for an SDK managed `Repository`, it needs to be added to the initialization code -for WASM and UniFFI. This example shows how to add support for `Cipher`s. +SDK. To add support for an SDK managed `Repository`, a new migration step needs to be added to the +`bitwarden-state-migrations` crate. -### How to initialize SDK-Managed State on WASM +### How to initialize SDK-Managed State -Go to `crates/bitwarden-wasm-internal/src/platform/mod.rs` and add a line with your type, as shown -below. In this example we're registering `Cipher` as both client and SDK managed to show how both -are done, but you can also just do one or the other. +Go to `crates/bitwarden-state-migrations/src/lib.rs` and add a line with your type, as shown below. +In this example we're registering `Cipher` and `Folder` as SDK managed. ```rust,ignore - pub async fn initialize_state( - &self, - cipher_repository: CipherRepository, - ) -> Result<(), bitwarden_state::registry::StateRegistryError> { - let cipher = cipher_repository.into_channel_impl(); - // Register the provided repository as client managed state - self.0.platform().state().register_client_managed(cipher); - - let sdk_managed_repositories = vec![ - // This should list all the SDK-managed repositories - ::data(), - // Add your type here - ]; - - self.0 - .platform() - .state() - .initialize_database(sdk_managed_repositories) - .await - } -``` - -### How to initialize SDK-Managed State on UniFFI - -Go to `crates/bitwarden-uniffi/src/platform/mod.rs` and add a line with your type, as shown below. -In this example we're registering `Cipher` as both client and SDK managed to show how both are done, -but you can also just do one or the other. - -```rust,ignore - pub async fn initialize_state( - &self, - cipher_repository: Arc, - ) -> Result<()> { - let cipher = UniffiRepositoryBridge::new(cipher_repository); - // Register the provided repository as client managed state - self.0.platform().state().register_client_managed(cipher); - - let sdk_managed_repositories = vec![ - // This should list all the SDK-managed repositories - ::data(), - // Add your type here - ]; - - self.0 - .platform() - .state() - .initialize_database(sdk_managed_repositories) - .await - .map_err(Error::StateRegistry)?; - Ok(()) - } +/// Returns a list of all SDK-managed repository migrations. +pub fn get_sdk_managed_migrations() -> RepositoryMigrations { + use RepositoryMigrationStep::*; + RepositoryMigrations::new(vec![ + // Add any new migrations here. Note that order matters, and that removing a repository + // requires a separate migration step using `Remove(...)`. + Add(Cipher::data()), + Add(Folder::data()), + ]) +} ``` diff --git a/crates/bitwarden-state/src/registry.rs b/crates/bitwarden-state/src/registry.rs index b2c8950d5..ae6d6500c 100644 --- a/crates/bitwarden-state/src/registry.rs +++ b/crates/bitwarden-state/src/registry.rs @@ -9,7 +9,7 @@ use serde::{de::DeserializeOwned, Serialize}; use thiserror::Error; use crate::{ - repository::{Repository, RepositoryItem, RepositoryItemData}, + repository::{Repository, RepositoryItem, RepositoryItemData, RepositoryMigrations}, sdk_managed::{Database, DatabaseConfiguration, SystemDatabase}, }; @@ -71,19 +71,19 @@ impl StateRegistry { pub async fn initialize_database( &self, configuration: DatabaseConfiguration, - repositories: Vec, + migrations: RepositoryMigrations, ) -> Result<(), StateRegistryError> { if self.database.get().is_some() { return Err(StateRegistryError::DatabaseAlreadyInitialized); } let _ = self .database - .set(SystemDatabase::initialize(configuration, &repositories).await?); + .set(SystemDatabase::initialize(configuration, migrations.clone()).await?); *self .sdk_managed .write() - .expect("RwLock should not be poisoned") = repositories.clone(); + .expect("RwLock should not be poisoned") = migrations.into_repository_items(); Ok(()) } diff --git a/crates/bitwarden-state/src/repository.rs b/crates/bitwarden-state/src/repository.rs index 7f956dc3e..01a8ce25d 100644 --- a/crates/bitwarden-state/src/repository.rs +++ b/crates/bitwarden-state/src/repository.rs @@ -100,6 +100,51 @@ pub const fn validate_registry_name(name: &str) -> bool { true } +/// Represents a set of migrations for multiple repositories in a database migration process. +#[derive(Debug, Clone)] +pub struct RepositoryMigrations { + pub(crate) steps: Vec, + // This is used only by indexedDB + #[allow(dead_code)] + pub(crate) version: u32, +} + +/// Represents a single step for a repository in a database migration process. +#[derive(Debug, Clone, Copy)] +pub enum RepositoryMigrationStep { + /// Add a new repository. + Add(RepositoryItemData), + /// Remove an existing repository. + Remove(RepositoryItemData), +} + +impl RepositoryMigrations { + /// Create a new `RepositoryMigrations` with the given steps. The version is derived from the + /// number of steps. + pub fn new(steps: Vec) -> Self { + Self { + version: steps.len() as u32, + steps, + } + } + + /// Converts the migration steps into a list of unique repository item data. + pub fn into_repository_items(self) -> Vec { + let mut map = std::collections::HashMap::new(); + for step in self.steps { + match step { + RepositoryMigrationStep::Add(data) => { + map.insert(data.type_id, data); + } + RepositoryMigrationStep::Remove(data) => { + map.remove(&data.type_id); + } + } + } + map.into_values().collect() + } +} + /// Register a type for use in a repository. The type must only be registered once in the crate /// where it's defined. The provided name must be unique and not be changed. #[macro_export] diff --git a/crates/bitwarden-state/src/sdk_managed/indexed_db.rs b/crates/bitwarden-state/src/sdk_managed/indexed_db.rs index ab75f746a..fe57de7fc 100644 --- a/crates/bitwarden-state/src/sdk_managed/indexed_db.rs +++ b/crates/bitwarden-state/src/sdk_managed/indexed_db.rs @@ -1,8 +1,9 @@ +use indexed_db::Error; use js_sys::JsString; use serde::{de::DeserializeOwned, ser::Serialize}; use crate::{ - repository::{RepositoryItem, RepositoryItemData}, + repository::{RepositoryItem, RepositoryMigrationStep, RepositoryMigrations}, sdk_managed::{Database, DatabaseConfiguration, DatabaseError}, }; @@ -22,7 +23,7 @@ pub struct IndexedDbDatabase( impl Database for IndexedDbDatabase { async fn initialize( configuration: DatabaseConfiguration, - registrations: &[RepositoryItemData], + migrations: RepositoryMigrations, ) -> Result { let DatabaseConfiguration::IndexedDb { db_name } = configuration else { return Err(DatabaseError::UnsupportedConfiguration(configuration)); @@ -30,19 +31,24 @@ impl Database for IndexedDbDatabase { let factory = indexed_db::Factory::get()?; - let registrations = registrations.to_vec(); - - // TODO: This version will be replaced by a proper migration system in a followup PR: - // https://github.com/bitwarden/sdk-internal/pull/410 - let version: u32 = 1; - // Open the database, creating it if needed let db = factory - .open(&db_name, version, async move |evt| { + .open(&db_name, migrations.version, async move |evt| { let db = evt.database(); - for reg in registrations { - db.build_object_store(reg.name()).create()?; + for step in &migrations.steps { + match step { + RepositoryMigrationStep::Add(data) => { + db.build_object_store(data.name()).create()?; + } + RepositoryMigrationStep::Remove(data) => { + match db.delete_object_store(data.name()) { + // If the store doesn't exist, we can ignore the error + Ok(_) | Err(Error::DoesNotExist) => {} + Err(e) => return Err(e), + } + } + } } Ok(()) diff --git a/crates/bitwarden-state/src/sdk_managed/mod.rs b/crates/bitwarden-state/src/sdk_managed/mod.rs index a5e6b9ddf..90f660fde 100644 --- a/crates/bitwarden-state/src/sdk_managed/mod.rs +++ b/crates/bitwarden-state/src/sdk_managed/mod.rs @@ -2,7 +2,7 @@ use bitwarden_error::bitwarden_error; use serde::{de::DeserializeOwned, ser::Serialize}; use thiserror::Error; -use crate::repository::{Repository, RepositoryError, RepositoryItem, RepositoryItemData}; +use crate::repository::{Repository, RepositoryError, RepositoryItem, RepositoryMigrations}; mod configuration; pub use configuration::DatabaseConfiguration; @@ -43,7 +43,7 @@ pub enum DatabaseError { pub trait Database { async fn initialize( configuration: DatabaseConfiguration, - registrations: &[RepositoryItemData], + registrations: RepositoryMigrations, ) -> Result where Self: Sized; diff --git a/crates/bitwarden-state/src/sdk_managed/sqlite.rs b/crates/bitwarden-state/src/sdk_managed/sqlite.rs index 190ace8e5..bcb2c3dfe 100644 --- a/crates/bitwarden-state/src/sdk_managed/sqlite.rs +++ b/crates/bitwarden-state/src/sdk_managed/sqlite.rs @@ -4,7 +4,9 @@ use serde::{de::DeserializeOwned, ser::Serialize}; use tokio::sync::Mutex; use crate::{ - repository::{validate_registry_name, RepositoryItem, RepositoryItemData}, + repository::{ + validate_registry_name, RepositoryItem, RepositoryMigrationStep, RepositoryMigrations, + }, sdk_managed::{Database, DatabaseConfiguration, DatabaseError}, }; @@ -25,24 +27,40 @@ fn validate_identifier(name: &'static str) -> Result<&'static str, DatabaseError impl SqliteDatabase { fn initialize_internal( mut db: rusqlite::Connection, - registrations: &[RepositoryItemData], + migrations: RepositoryMigrations, ) -> Result { // Set WAL mode for better concurrency db.pragma_update(None, "journal_mode", "WAL")?; let transaction = db.transaction()?; - for reg in registrations { - // SAFETY: SQLite tables cannot use ?, but `reg.name()` is not user controlled and - // is validated to only contain valid characters, so it's safe to - // interpolate here. - transaction.execute( - &format!( - "CREATE TABLE IF NOT EXISTS \"{}\" (key TEXT PRIMARY KEY, value TEXT NOT NULL);", - validate_identifier(reg.name())?, - ), - [], - )?; + for step in &migrations.steps { + match step { + RepositoryMigrationStep::Add(data) => { + // SAFETY: SQLite tables cannot use ?, but `reg.name()` is not user controlled + // and is validated to only contain valid characters, so + // it's safe to interpolate here. + transaction.execute( + &format!( + "CREATE TABLE IF NOT EXISTS \"{}\" (key TEXT PRIMARY KEY, value TEXT NOT NULL);", + validate_identifier(data.name())?, + ), + [], + )?; + } + RepositoryMigrationStep::Remove(data) => { + // SAFETY: SQLite tables cannot use ?, but `reg.name()` is not user controlled + // and is validated to only contain valid characters, so + // it's safe to interpolate here. + transaction.execute( + &format!( + "DROP TABLE IF EXISTS \"{}\";", + validate_identifier(data.name())?, + ), + [], + )?; + } + } } transaction.commit()?; @@ -53,7 +71,7 @@ impl SqliteDatabase { impl Database for SqliteDatabase { async fn initialize( configuration: DatabaseConfiguration, - registrations: &[RepositoryItemData], + migrations: RepositoryMigrations, ) -> Result { let DatabaseConfiguration::Sqlite { db_name, @@ -65,7 +83,7 @@ impl Database for SqliteDatabase { path.set_file_name(format!("{db_name}.sqlite")); let db = rusqlite::Connection::open(path)?; - Self::initialize_internal(db, registrations) + Self::initialize_internal(db, migrations) } async fn get( @@ -173,9 +191,21 @@ mod tests { struct TestA(usize); register_repository_item!(TestA, "TestItem_A"); - let registrations = vec![TestA::data()]; - - let db = SqliteDatabase::initialize_internal(db, ®istrations).unwrap(); + #[derive(Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] + struct TestB(usize); + register_repository_item!(TestB, "TestItem_B"); + + let steps = vec![ + // Test that deleting a table that doesn't exist is fine + RepositoryMigrationStep::Remove(TestB::data()), + RepositoryMigrationStep::Add(TestA::data()), + RepositoryMigrationStep::Add(TestB::data()), + // Test that deleting a table that does exist is also fine + RepositoryMigrationStep::Remove(TestB::data()), + ]; + let migrations = RepositoryMigrations::new(steps); + + let db = SqliteDatabase::initialize_internal(db, migrations).unwrap(); assert_eq!(db.list::().await.unwrap(), Vec::::new()); diff --git a/crates/bitwarden-uniffi/Cargo.toml b/crates/bitwarden-uniffi/Cargo.toml index 76a489869..bd9ffdbd7 100644 --- a/crates/bitwarden-uniffi/Cargo.toml +++ b/crates/bitwarden-uniffi/Cargo.toml @@ -30,6 +30,7 @@ bitwarden-generators = { workspace = true, features = ["uniffi"] } bitwarden-send = { workspace = true, features = ["uniffi"] } bitwarden-ssh = { workspace = true, features = ["uniffi"] } bitwarden-state = { workspace = true, features = ["uniffi"] } +bitwarden-state-migrations = { workspace = true, features = ["uniffi"] } bitwarden-uniffi-error = { workspace = true, features = ["uniffi"] } bitwarden-vault = { workspace = true, features = ["uniffi"] } chrono = { workspace = true, features = ["std"] } diff --git a/crates/bitwarden-uniffi/src/platform/mod.rs b/crates/bitwarden-uniffi/src/platform/mod.rs index 439bada77..bcf4d4a35 100644 --- a/crates/bitwarden-uniffi/src/platform/mod.rs +++ b/crates/bitwarden-uniffi/src/platform/mod.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use bitwarden_core::{platform::FingerprintRequest, Client}; use bitwarden_fido::ClientFido2Ext; -use bitwarden_state::{repository::RepositoryItem, DatabaseConfiguration}; +use bitwarden_state::DatabaseConfiguration; use bitwarden_vault::Cipher; use repository::UniffiRepositoryBridge; @@ -68,16 +68,14 @@ impl StateClient { self.0.platform().state().register_client_managed(cipher); } + /// Initialize the database for SDK managed repositories. pub async fn initialize_state(&self, configuration: SqliteConfiguration) -> Result<()> { - let sdk_managed_repositories = vec![ - // This should list all the SDK-managed repositories - ::data(), - ]; + let migrations = bitwarden_state_migrations::get_sdk_managed_migrations(); self.0 .platform() .state() - .initialize_database(configuration.into(), sdk_managed_repositories) + .initialize_database(configuration.into(), migrations) .await .map_err(Error::StateRegistry)?; Ok(()) diff --git a/crates/bitwarden-wasm-internal/Cargo.toml b/crates/bitwarden-wasm-internal/Cargo.toml index 9a2965750..ee6069078 100644 --- a/crates/bitwarden-wasm-internal/Cargo.toml +++ b/crates/bitwarden-wasm-internal/Cargo.toml @@ -26,6 +26,7 @@ bitwarden-generators = { workspace = true, features = ["wasm"] } bitwarden-ipc = { workspace = true, features = ["wasm"] } bitwarden-ssh = { workspace = true, features = ["wasm"] } bitwarden-state = { workspace = true, features = ["wasm"] } +bitwarden-state-migrations = { workspace = true, features = ["wasm"] } bitwarden-threading = { workspace = true } bitwarden-vault = { workspace = true, features = ["wasm"] } console_error_panic_hook = "0.1.7" diff --git a/crates/bitwarden-wasm-internal/src/platform/mod.rs b/crates/bitwarden-wasm-internal/src/platform/mod.rs index 316275660..8f49d2ac6 100644 --- a/crates/bitwarden-wasm-internal/src/platform/mod.rs +++ b/crates/bitwarden-wasm-internal/src/platform/mod.rs @@ -1,5 +1,5 @@ use bitwarden_core::Client; -use bitwarden_state::{repository::RepositoryItem, DatabaseConfiguration}; +use bitwarden_state::DatabaseConfiguration; use bitwarden_vault::{Cipher, Folder}; use serde::{Deserialize, Serialize}; use tsify::Tsify; @@ -64,14 +64,17 @@ impl StateClient { self.0.platform().state().register_client_managed(cipher); } + pub fn register_folder_repository(&self, store: FolderRepository) { + let store = store.into_channel_impl(); + self.0.platform().state().register_client_managed(store) + } + + /// Initialize the database for SDK managed repositories. pub async fn initialize_state( &self, configuration: IndexedDbConfiguration, ) -> Result<(), bitwarden_state::registry::StateRegistryError> { - let sdk_managed_repositories = vec![ - // This should list all the SDK-managed repositories - ::data(), - ]; + let sdk_managed_repositories = bitwarden_state_migrations::get_sdk_managed_migrations(); self.0 .platform() @@ -79,11 +82,6 @@ impl StateClient { .initialize_database(configuration.into(), sdk_managed_repositories) .await } - - pub fn register_folder_repository(&self, store: FolderRepository) { - let store = store.into_channel_impl(); - self.0.platform().state().register_client_managed(store) - } } impl From for DatabaseConfiguration {