From d60ec55e302f6acf0a4cbd711d0f434ffe2dfb1f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Wed, 3 Jul 2024 13:39:54 +0100 Subject: [PATCH] Crypto store: clear db before integ tests (#3644) It's currently possible for integ test results to leak from one test run to the next (for example, the indexeddb stores hang around in the browser), causing bad test results. Extend the test setup routine to clear out the store before the test starts. --- .../src/store/integration_tests.rs | 65 ++++++++++++++----- .../src/store/memorystore.rs | 22 +++++-- .../src/crypto_store/mod.rs | 34 ++++++++-- crates/matrix-sdk-sqlite/src/crypto_store.rs | 26 ++++++-- 4 files changed, 113 insertions(+), 34 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/store/integration_tests.rs b/crates/matrix-sdk-crypto/src/store/integration_tests.rs index d058e8f3c..b72fb2949 100644 --- a/crates/matrix-sdk-crypto/src/store/integration_tests.rs +++ b/crates/matrix-sdk-crypto/src/store/integration_tests.rs @@ -1,3 +1,34 @@ +/// A macro which will run the CryptoStore integration test suite. +/// +/// You need to provide a `async fn get_store() -> StoreResult` +/// providing a fresh store on the same level you invoke the macro. +/// +/// ## Usage Example: +/// ```no_run +/// # use matrix_sdk_crypto::store::{ +/// # MemoryStore as MyCryptoStore, +/// # }; +/// +/// #[cfg(test)] +/// mod tests { +/// use super::MyCryptoStore; +/// +/// async fn get_store( +/// name: &str, +/// passphrase: Option<&str>, +/// clear_data: bool, +/// ) -> MyCryptoStore { +/// let store = MyCryptoStore::new(); +/// if clear_data { +/// store.clear(); +/// } +/// store +/// } +/// +/// cryptostore_integration_tests!(); +/// } +/// ``` + #[allow(unused_macros)] #[macro_export] macro_rules! cryptostore_integration_tests { @@ -61,7 +92,7 @@ macro_rules! cryptostore_integration_tests { } pub async fn get_loaded_store(name: &str) -> (Account, impl CryptoStore) { - let store = get_store(name, None).await; + let store = get_store(name, None, true).await; let account = get_account(); store.save_pending_changes(PendingChanges { account: Some(account.deep_clone()), }).await.expect("Can't save account"); @@ -93,7 +124,7 @@ macro_rules! cryptostore_integration_tests { #[async_test] async fn save_account_via_generic_save() { - let store = get_store("save_account_via_generic", None).await; + let store = get_store("save_account_via_generic", None, true).await; assert!(store.get_static_account().is_none()); assert!(store.load_account().await.unwrap().is_none()); let account = get_account(); @@ -107,7 +138,7 @@ macro_rules! cryptostore_integration_tests { #[async_test] async fn save_account() { - let store = get_store("save_account", None).await; + let store = get_store("save_account", None, true).await; assert!(store.get_static_account().is_none()); assert!(store.load_account().await.unwrap().is_none()); let account = get_account(); @@ -121,7 +152,7 @@ macro_rules! cryptostore_integration_tests { #[async_test] async fn load_account() { - let store = get_store("load_account", None).await; + let store = get_store("load_account", None, true).await; let account = get_account(); store @@ -137,8 +168,8 @@ macro_rules! cryptostore_integration_tests { #[async_test] async fn load_account_with_passphrase() { - let store = - get_store("load_account_with_passphrase", Some("secret_passphrase")).await; + let passphrase = Some("secret_passphrase"); + let store = get_store("load_account_with_passphrase", passphrase, true).await; let account = get_account(); store @@ -154,7 +185,7 @@ macro_rules! cryptostore_integration_tests { #[async_test] async fn save_and_share_account() { - let store = get_store("save_and_share_account", None).await; + let store = get_store("save_and_share_account", None, true).await; let mut account = get_account(); store @@ -179,7 +210,7 @@ macro_rules! cryptostore_integration_tests { #[async_test] async fn load_sessions() { - let store = get_store("load_sessions", None).await; + let store = get_store("load_sessions", None, true).await; let (account, session) = get_account_and_session().await; store .save_pending_changes(PendingChanges { account: Some(account.deep_clone()) }) @@ -206,7 +237,7 @@ macro_rules! cryptostore_integration_tests { // Given we created a session and saved it in the store let (session_id, account, sender_key) = { - let store = get_store(store_name, None).await; + let store = get_store(store_name, None, true).await; let (account, session) = get_account_and_session().await; let sender_key = session.sender_key.to_base64(); let session_id = session.session_id().to_owned(); @@ -241,7 +272,7 @@ macro_rules! cryptostore_integration_tests { }; // When we reload the store - let store = get_store(store_name, None).await; + let store = get_store(store_name, None, false).await; // Then the same account and session info was reloaded let loaded_account = store.load_account().await.unwrap().unwrap(); @@ -293,7 +324,7 @@ macro_rules! cryptostore_integration_tests { } // When we reload the account - let store = get_store(dir, None).await; + let store = get_store(dir, None, false).await; store.load_account().await.unwrap(); // Then the saved session is restored @@ -512,7 +543,7 @@ macro_rules! cryptostore_integration_tests { drop(store); - let store = get_store(dir, None).await; + let store = get_store(dir, None, false).await; store.load_account().await.unwrap(); @@ -564,7 +595,7 @@ macro_rules! cryptostore_integration_tests { drop(store); - let store = get_store(dir.clone(), None).await; + let name = dir.clone();let store = get_store(name, None, false).await; let loaded = store.load_tracked_users().await.unwrap(); check_loaded_users(loaded); } @@ -615,7 +646,7 @@ macro_rules! cryptostore_integration_tests { drop(store); - let store = get_store(dir, None).await; + let store = get_store(dir, None, false).await; store.load_account().await.unwrap(); @@ -666,7 +697,7 @@ macro_rules! cryptostore_integration_tests { store.save_changes(changes).await.unwrap(); drop(store); - let store = get_store(dir, None).await; + let store = get_store(dir, None, false).await; store.load_account().await.unwrap(); @@ -683,7 +714,7 @@ macro_rules! cryptostore_integration_tests { let user_id = user_id!("@example:localhost"); let device_id: &DeviceId = "WSKKLTJZCL".into(); - let store = get_store(dir, None).await; + let store = get_store(dir, None, true).await; let account = Account::with_device_id(&user_id, device_id); @@ -705,7 +736,7 @@ macro_rules! cryptostore_integration_tests { drop(store); - let store = get_store(dir, None).await; + let store = get_store(dir, None, false).await; store.load_account().await.unwrap(); diff --git a/crates/matrix-sdk-crypto/src/store/memorystore.rs b/crates/matrix-sdk-crypto/src/store/memorystore.rs index f27c5129a..3acb77ffa 100644 --- a/crates/matrix-sdk-crypto/src/store/memorystore.rs +++ b/crates/matrix-sdk-crypto/src/store/memorystore.rs @@ -1132,7 +1132,11 @@ mod integration_tests { /// dropping this store won't destroy its data, since /// [PersistentMemoryStore] is a reference-counted smart pointer /// to an underlying [MemoryStore]. - async fn get_store(name: &str, _passphrase: Option<&str>) -> PersistentMemoryStore { + async fn get_store( + name: &str, + _passphrase: Option<&str>, + clear_data: bool, + ) -> PersistentMemoryStore { // Holds on to one [PersistentMemoryStore] per test, so even if the test drops // the store, we keep its data alive. This simulates the behaviour of // the other stores, which keep their data in a real DB, allowing us to @@ -1140,12 +1144,16 @@ mod integration_tests { static STORES: OnceLock>> = OnceLock::new(); let stores = STORES.get_or_init(|| Mutex::new(HashMap::new())); - stores - .lock() - .unwrap() - .entry(name.to_owned()) - .or_insert_with(PersistentMemoryStore::new) - .clone() + let mut stores = stores.lock().unwrap(); + + if clear_data { + // Create a new PersistentMemoryStore + let new_store = PersistentMemoryStore::new(); + stores.insert(name.to_owned(), new_store.clone()); + new_store + } else { + stores.entry(name.to_owned()).or_insert_with(PersistentMemoryStore::new).clone() + } } /// Forwards all methods to the underlying [MemoryStore]. diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs b/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs index f14c01930..78de43ac8 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store/mod.rs @@ -377,6 +377,14 @@ impl IndexeddbCryptoStore { IndexeddbCryptoStore::open_with_store_cipher(name, None).await } + /// Delete the IndexedDB databases for the given name. + #[cfg(test)] + pub fn delete_stores(prefix: &str) -> Result<()> { + IdbDatabase::delete_by_name(&format!("{prefix:0}::matrix-sdk-crypto-meta"))?; + IdbDatabase::delete_by_name(&format!("{prefix:0}::matrix-sdk-crypto"))?; + Ok(()) + } + fn get_static_account(&self) -> Option { self.static_account.read().unwrap().clone() } @@ -1735,7 +1743,14 @@ mod tests { wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser); - async fn get_store(name: &str, passphrase: Option<&str>) -> IndexeddbCryptoStore { + async fn get_store( + name: &str, + passphrase: Option<&str>, + clear_data: bool, + ) -> IndexeddbCryptoStore { + if clear_data { + IndexeddbCryptoStore::delete_stores(name).unwrap(); + } match passphrase { Some(pass) => IndexeddbCryptoStore::open_with_passphrase(name, pass) .await @@ -1748,7 +1763,7 @@ mod tests { #[async_test] async fn cache_cleared_after_device_update() { - let store = get_store("cache_cleared_after_device_update", None).await; + let store = get_store("cache_cleared_after_device_update", None, true).await; // Given we created a session and saved it in the store let (account, session) = cryptostore_integration_tests::get_account_and_session().await; let sender_key = session.sender_key.to_base64(); @@ -1800,11 +1815,17 @@ mod encrypted_tests { wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser); - async fn get_store(name: &str, passphrase: Option<&str>) -> IndexeddbCryptoStore { + async fn get_store( + name: &str, + passphrase: Option<&str>, + clear_data: bool, + ) -> IndexeddbCryptoStore { + if clear_data { + IndexeddbCryptoStore::delete_stores(name).unwrap(); + } + let pass = passphrase.unwrap_or(name); - // make sure to use a different store name than the equivalent unencrypted test - let store_name = name.to_owned() + "_enc"; - IndexeddbCryptoStore::open_with_passphrase(&store_name, pass) + IndexeddbCryptoStore::open_with_passphrase(&name, pass) .await .expect("Can't create a passphrase protected store") } @@ -1819,6 +1840,7 @@ mod encrypted_tests { let b64_passdata = base64_encode(passdata); // Initialise the store with some account data + IndexeddbCryptoStore::delete_stores(store_name).unwrap(); let store = IndexeddbCryptoStore::open_with_passphrase(&store_name, &b64_passdata) .await .expect("Can't create a passphrase-protected store"); diff --git a/crates/matrix-sdk-sqlite/src/crypto_store.rs b/crates/matrix-sdk-sqlite/src/crypto_store.rs index 426446d5c..83696f5c9 100644 --- a/crates/matrix-sdk-sqlite/src/crypto_store.rs +++ b/crates/matrix-sdk-sqlite/src/crypto_store.rs @@ -1326,14 +1326,23 @@ mod tests { use matrix_sdk_crypto::{cryptostore_integration_tests, cryptostore_integration_tests_time}; use once_cell::sync::Lazy; use tempfile::{tempdir, TempDir}; + use tokio::fs; use super::SqliteCryptoStore; static TMP_DIR: Lazy = Lazy::new(|| tempdir().unwrap()); - async fn get_store(name: &str, passphrase: Option<&str>) -> SqliteCryptoStore { + async fn get_store( + name: &str, + passphrase: Option<&str>, + clear_data: bool, + ) -> SqliteCryptoStore { let tmpdir_path = TMP_DIR.path().join(name); + if clear_data { + let _ = fs::remove_dir_all(&tmpdir_path).await; + } + SqliteCryptoStore::open(tmpdir_path.to_str().unwrap(), passphrase) .await .expect("Can't create a passphrase protected store") @@ -1353,15 +1362,24 @@ mod encrypted_tests { use matrix_sdk_test::async_test; use once_cell::sync::Lazy; use tempfile::{tempdir, TempDir}; + use tokio::fs; use super::SqliteCryptoStore; static TMP_DIR: Lazy = Lazy::new(|| tempdir().unwrap()); - async fn get_store(name: &str, passphrase: Option<&str>) -> SqliteCryptoStore { + async fn get_store( + name: &str, + passphrase: Option<&str>, + clear_data: bool, + ) -> SqliteCryptoStore { let tmpdir_path = TMP_DIR.path().join(name); let pass = passphrase.unwrap_or("default_test_password"); + if clear_data { + let _ = fs::remove_dir_all(&tmpdir_path).await; + } + SqliteCryptoStore::open(tmpdir_path.to_str().unwrap(), Some(pass)) .await .expect("Can't create a passphrase protected store") @@ -1369,7 +1387,7 @@ mod encrypted_tests { #[async_test] async fn cache_cleared() { - let store = get_store("cache_cleared", None).await; + let store = get_store("cache_cleared", None, true).await; // Given we created a session and saved it in the store let (account, session) = cryptostore_integration_tests::get_account_and_session().await; let sender_key = session.sender_key.to_base64(); @@ -1396,7 +1414,7 @@ mod encrypted_tests { #[async_test] async fn cache_cleared_after_device_update() { - let store = get_store("cache_cleared_after_device_update", None).await; + let store = get_store("cache_cleared_after_device_update", None, true).await; // Given we created a session and saved it in the store let (account, session) = cryptostore_integration_tests::get_account_and_session().await; let sender_key = session.sender_key.to_base64();