Simon/feat/credential primary key wpb 22045

Cargo.toml

@@ -51,8 +51,9 @@ derive_more = { version = "2.0", features = [
 ] }
 futures-util = "0.3"
 hex = "0.4"
-idb = "0.6"
-indexmap = "2"
+
+# Until devashishdxt/idb#36 is resolved, we're keeping our fork
+idb = { git = "https://github.com/wireapp/idb", rev = "05573e5b29191fdeb442104ad1040346a3889822" }
 itertools = "0.14"
 log = { version = "0.4", features = ["kv_serde"] }
 log-reload = "0.1.3"

keystore/Cargo.toml

@@ -81,7 +81,6 @@ js-sys = "0.3"
 web-sys = { version = "0.3", features = ["console"] }
 wasm-bindgen = "0.2"
 serde-wasm-bindgen = "0.6"
-indexmap.workspace = true
 # Async WASM stuff
 wasm-bindgen-futures = "0.4"
 # Crypto stuff

keystore/src/connection/platform/generic/migrations/V20__credentials_new_primary_key.sql

@@ -0,0 +1,31 @@
+CREATE TABLE mls_credentials_new (
+    public_key_sha256 TEXT UNIQUE,
+    public_key BLOB NOT NULL,
+    id BLOB NOT NULL,
+    credential BLOB NOT NULL,
+    created_at DATETIME DEFAULT CURRENT_TIMESTAMP,
+    ciphersuite INT NOT NULL,
+    secret_key BLOB NOT NULL
+);
+
+INSERT INTO mls_credentials_new (
+    public_key_sha256,
+    public_key,
+    id,
+    credential,
+    created_at,
+    ciphersuite,
+    secret_key
+)
+SELECT sha256_blob(public_key),
+    public_key,
+    id,
+    credential,
+    created_at,
+    ciphersuite,
+    secret_key
+FROM mls_credentials;
+
+DROP TABLE mls_credentials;
+
+ALTER TABLE mls_credentials_new RENAME TO mls_credentials;

keystore/src/connection/platform/generic/mod.rs

@@ -437,13 +437,22 @@ mod migration_test {
             let db = Database::open(ConnectionType::Persistent(path), &new_key)
                 .await
                 .unwrap();
-            let deduplicated_count = db
+            let deduplicated_credentials = db
                 .find_all::<StoredCredential>(EntityFindParams::default())
                 .await
-                .expect("deduplicated credentials")
-                .len();
+                .expect("deduplicated credentials");
+
+            let deduplicated_count = deduplicated_credentials.len();
+
+            let deduplicated_credential = deduplicated_credentials.first().expect("first credential");
 
             assert_eq!(deduplicated_count, 1);
+
+            // In case of equal occurence, the credential with the numerically lower ciphersuite is kept.
+            assert_eq!(
+                deduplicated_credential.ciphersuite,
+                Ciphersuite::MLS_128_DHKEMX25519_AES128GCM_SHA256_Ed25519 as u16
+            );
         });
     }
 }

keystore/src/connection/platform/wasm/migrations/mod.rs

@@ -1,5 +1,4 @@
 mod db_key_type_to_bytes;
-mod metabuilder;
 mod pre_v4;
 mod v0;
 mod v2;
@@ -9,10 +8,10 @@ mod v5;
 mod v6;
 mod v7;
 mod v8;
+mod v9;
 
 pub(super) use db_key_type_to_bytes::migrate_db_key_type_to_bytes;
 use idb::{Database, Factory};
-use metabuilder::Metabuilder;
 
 use crate::{CryptoKeystoreError, CryptoKeystoreResult, connection::DatabaseKey};
 
@@ -34,11 +33,13 @@ const DB_VERSION_5: u32 = db_version_number(5);
 const DB_VERSION_6: u32 = db_version_number(6);
 const DB_VERSION_7: u32 = db_version_number(7);
 const DB_VERSION_8: u32 = db_version_number(8);
+const DB_VERSION_9: u32 = db_version_number(9);
+
+/// This must always be the latest version. Increment when adding a new migration.
+const TARGET_VERSION: u32 = DB_VERSION_9;
 
 /// Open an existing idb database with the given name, and migrate it if needed.
 pub(crate) async fn open_and_migrate(name: &str, key: &DatabaseKey) -> CryptoKeystoreResult<Database> {
-    /// Increment when adding a new migration.
-    const TARGET_VERSION: u32 = DB_VERSION_5;
     let factory = Factory::new()?;
 
     let open_existing = factory.open(name, None)?;
@@ -79,6 +80,7 @@ async fn do_migration_step(from: u32, name: &str, key: &DatabaseKey) -> CryptoKe
         DB_VERSION_5 => v6::migrate(name, key).await,
         DB_VERSION_6 => v7::migrate(name, key).await,
         DB_VERSION_7 => v8::migrate(name, key).await,
+        DB_VERSION_8 => v9::migrate(name, key).await,
         _ => Err(CryptoKeystoreError::MigrationNotSupported(from)),
     }
 }

keystore/src/connection/platform/wasm/migrations/v0.rs

@@ -1,9 +1,9 @@
 use idb::{
     KeyPath,
-    builder::{IndexBuilder, ObjectStoreBuilder},
+    builder::{DatabaseBuilder, IndexBuilder, ObjectStoreBuilder},
 };
 
-use super::{DB_VERSION_0, Metabuilder};
+use super::DB_VERSION_0;
 use crate::{
     entities::{
         E2eiAcmeCA, E2eiCrl, E2eiIntermediateCert, E2eiRefreshToken, EntityBase as _, MlsPendingMessage,
@@ -14,8 +14,8 @@ use crate::{
     migrations::{StoredSignatureKeypair, V5Credential},
 };
 
-pub(super) fn get_builder(name: &str) -> Metabuilder {
-    let idb_builder = Metabuilder::new(name)
+pub(super) fn get_builder(name: &str) -> DatabaseBuilder {
+    let idb_builder = DatabaseBuilder::new(name)
         .version(DB_VERSION_0)
         .add_object_store(
             ObjectStoreBuilder::new(V5Credential::COLLECTION_NAME)

keystore/src/connection/platform/wasm/migrations/v2.rs

@@ -1,9 +1,9 @@
 use idb::{
     KeyPath,
-    builder::{IndexBuilder, ObjectStoreBuilder},
+    builder::{DatabaseBuilder, IndexBuilder, ObjectStoreBuilder},
 };
 
-use super::{DB_VERSION_2, Metabuilder};
+use super::DB_VERSION_2;
 use crate::{
     CryptoKeystoreResult,
     entities::{ConsumerData, EntityBase as _},
@@ -18,7 +18,7 @@ pub(super) async fn migrate(name: &str) -> CryptoKeystoreResult<u32> {
 }
 
 /// Add a new object store for the ConsumerData struct.
-pub(super) fn get_builder(name: &str) -> Metabuilder {
+pub(super) fn get_builder(name: &str) -> DatabaseBuilder {
     let previous_builder = super::v0::get_builder(name);
     previous_builder.version(DB_VERSION_2).add_object_store(
         ObjectStoreBuilder::new(ConsumerData::COLLECTION_NAME)

keystore/src/connection/platform/wasm/migrations/v3.rs

@@ -1,9 +1,9 @@
 use idb::{
     KeyPath,
-    builder::{IndexBuilder, ObjectStoreBuilder},
+    builder::{DatabaseBuilder, IndexBuilder, ObjectStoreBuilder},
 };
 
-use super::{DB_VERSION_3, Metabuilder};
+use super::DB_VERSION_3;
 use crate::{
     CryptoKeystoreResult,
     entities::{EntityBase as _, StoredBufferedCommit},
@@ -18,7 +18,7 @@ pub(super) async fn migrate(name: &str) -> CryptoKeystoreResult<u32> {
 }
 
 /// Add a new object store for the StoredBufferedCommit struct.
-pub(super) fn get_builder(name: &str) -> Metabuilder {
+pub(super) fn get_builder(name: &str) -> DatabaseBuilder {
     let previous_builder = super::v2::get_builder(name);
     previous_builder.version(DB_VERSION_3).add_object_store(
         ObjectStoreBuilder::new(StoredBufferedCommit::COLLECTION_NAME)

keystore/src/connection/platform/wasm/migrations/v4.rs

@@ -1,4 +1,6 @@
-use super::{DB_VERSION_4, Metabuilder};
+use idb::builder::DatabaseBuilder;
+
+use super::DB_VERSION_4;
 use crate::CryptoKeystoreResult;
 
 /// Open IDB once with the new builder and close it, this will add the new object store.
@@ -10,6 +12,6 @@ pub(super) async fn migrate(name: &str) -> CryptoKeystoreResult<u32> {
 }
 
 /// Just initialize object stores.
-pub(super) fn get_builder(name: &str) -> Metabuilder {
+pub(super) fn get_builder(name: &str) -> DatabaseBuilder {
     super::v3::get_builder(name).version(DB_VERSION_4)
 }

keystore/src/connection/platform/wasm/migrations/v5.rs

@@ -1,4 +1,6 @@
-use super::{DB_VERSION_5, Metabuilder};
+use idb::builder::DatabaseBuilder;
+
+use super::DB_VERSION_5;
 use crate::{
     CryptoKeystoreResult,
     entities::{E2eiRefreshToken, EntityBase as _},
@@ -13,7 +15,7 @@ pub(super) async fn migrate(name: &str) -> CryptoKeystoreResult<u32> {
 }
 
 /// Just set up the builder for v5.
-pub(super) fn get_builder(name: &str) -> Metabuilder {
+pub(super) fn get_builder(name: &str) -> DatabaseBuilder {
     super::v4::get_builder(name)
         .version(DB_VERSION_5)
         .remove_object_store(E2eiRefreshToken::COLLECTION_NAME)

keystore/src/connection/platform/wasm/migrations/v6/mod.rs

@@ -1,6 +1,8 @@
 mod v5_entities;
 
-use super::{DB_VERSION_6, Metabuilder};
+use idb::builder::DatabaseBuilder;
+
+use super::DB_VERSION_6;
 use crate::{
     CryptoKeystoreResult, Database, DatabaseKey,
     connection::FetchFromDatabase,
@@ -41,7 +43,7 @@ pub(super) async fn migrate(name: &str, key: &DatabaseKey) -> CryptoKeystoreResu
 }
 
 /// Set up the builder for v6.
-pub(super) fn get_builder(name: &str) -> Metabuilder {
+pub(super) fn get_builder(name: &str) -> DatabaseBuilder {
     super::v5::get_builder(name)
         .version(DB_VERSION_6)
         .remove_object_store(StoredSignatureKeypair::COLLECTION_NAME)

keystore/src/connection/platform/wasm/migrations/v7/mod.rs

@@ -1,6 +1,8 @@
 mod v6_entities;
 
-use super::{DB_VERSION_7, Metabuilder};
+use idb::builder::DatabaseBuilder;
+
+use super::DB_VERSION_7;
 use crate::{
     CryptoKeystoreResult, Database, DatabaseKey,
     connection::FetchFromDatabase as _,
@@ -48,6 +50,6 @@ pub(super) async fn migrate(name: &str, key: &DatabaseKey) -> CryptoKeystoreResu
 }
 
 /// Set up the builder for v7.
-pub(super) fn get_builder(name: &str) -> Metabuilder {
+pub(super) fn get_builder(name: &str) -> DatabaseBuilder {
     super::v6::get_builder(name).version(DB_VERSION_7)
 }

keystore/src/connection/platform/wasm/migrations/v8.rs

@@ -1,8 +1,15 @@
-use super::{DB_VERSION_8, Metabuilder};
+//! This migration deduplicates credentials and creates a new object store, "mls_credentials_new".
+
+use idb::{
+    KeyPath,
+    builder::{DatabaseBuilder, IndexBuilder, ObjectStoreBuilder},
+};
+
+use super::DB_VERSION_8;
 use crate::{
     CryptoKeystoreResult, Database, DatabaseKey,
     connection::FetchFromDatabase as _,
-    entities::{EntityFindParams, PersistedMlsGroup, StoredCredential},
+    entities::{EntityBase, EntityFindParams, PersistedMlsGroup, StoredCredential},
     migrations::{detect_duplicate_credentials, make_least_used_ciphersuite},
 };
 
@@ -60,7 +67,13 @@ pub(super) async fn migrate(name: &str, key: &DatabaseKey) -> CryptoKeystoreResu
 }
 
 /// Set up the builder for v8.
-pub(super) fn get_builder(name: &str) -> Metabuilder {
-    super::v7::get_builder(name).version(DB_VERSION_8)
-    // TODO(SimonThormeyer): having de-deduplicated credentials, make the pk column the primary key.
+pub(super) fn get_builder(name: &str) -> DatabaseBuilder {
+    super::v7::get_builder(name).version(DB_VERSION_8).add_object_store(
+        ObjectStoreBuilder::new(&format!(
+            "{collection_name}_new",
+            collection_name = StoredCredential::COLLECTION_NAME
+        ))
+        .auto_increment(false)
+        .add_index(IndexBuilder::new("public_key".into(), KeyPath::new_single("public_key")).unique(true)),
+    )
 }