Profile snapshot id payload (#214)

* wip * wip * fix test * wip * fix ios tests * test * update * wip
radixdlt · Sep 30, 2024 · 3d8d347 · 3d8d347
1 parent 7d6a571
commit 3d8d347
Show file tree

Hide file tree

Showing 17 changed files with 164 additions and 46 deletions.
diff --git a/apple/Sources/Sargon/SargonOS/SargonOS+Static+Shared.swift b/apple/Sources/Sargon/SargonOS/SargonOS+Static+Shared.swift
@@ -47,7 +47,7 @@ extension SargonOS {
 		if !isEmulatingFreshInstall, _shared != nil {
 			throw SargonOSAlreadyBooted()
 		}
-		let shared = try await SargonOS.boot(bios: bios)
+		let shared = await SargonOS.boot(bios: bios)
 		Self._shared = shared
 		return shared
 	}

diff --git a/apple/Sources/Sargon/SargonOS/TestOS.swift b/apple/Sources/Sargon/SargonOS/TestOS.swift
@@ -42,14 +42,9 @@ extension TestOS: SargonOSProtocol {}
 
 // MARK: Private
 extension TestOS {
-	private func nextAccountName() throws -> DisplayName {
-		do {
-			let index = try accountsForDisplayOnCurrentNetwork.count
+	private func nextAccountName() -> DisplayName {
+			let index = (try? accountsForDisplayOnCurrentNetwork.count) ?? 0
 			return DisplayName(value: "Unnamed \(index)")
-		} catch CommonError.NoNetworkInProfile(_) {
-			return DisplayName(value: "Unnamed 0")
-		}
-
 	}
 }
 

diff --git a/apple/Sources/Sargon/Util/EventPublisher.swift b/apple/Sources/Sargon/Util/EventPublisher.swift
@@ -1,11 +1,11 @@
 import AsyncExtensions
 
 public final actor EventPublisher<Element: Sendable> {
-    public typealias Subject = AsyncPassthroughSubject<Element>
-    public typealias Stream = AsyncThrowingPassthroughSubject<Element, any Error>
+    public typealias Subject = AsyncReplaySubject<Element>
+    public typealias Stream = AsyncThrowingReplaySubject<Element, any Error>
 
-    let stream = Stream()
-    let subject = Subject()
+    let stream = Stream(bufferSize: 1)
+    let subject = Subject(bufferSize: 1)
 
     public func eventStream() -> AsyncMulticastSequence<Subject, Stream> {
         subject

diff --git a/apple/Tests/IntegrationTests/DriversTests/DriversTests.swift b/apple/Tests/IntegrationTests/DriversTests/DriversTests.swift
@@ -29,6 +29,6 @@ final class DriversTests: TestCase {
 	}
 
     func test_bios_insecure() async throws {
-        let _ = try await SargonOS.boot(bios: BIOS.insecure())
+        let _ = await SargonOS.boot(bios: BIOS.insecure())
     }
 }
diff --git a/apple/Tests/IntegrationTests/DriversTests/InsecureStorageTests.swift b/apple/Tests/IntegrationTests/DriversTests/InsecureStorageTests.swift
@@ -9,7 +9,7 @@ class InsecureStorageDriverTests: DriverTest<Insecure︕！TestOnly︕！Ephemer
     func test() async throws {
         let sut = SUT.init(keychainService: "test")
         let data = Data.sampleAced
-        let key = SUT.Key.profileSnapshot
+        let key = SUT.Key.profileSnapshot(profileId: newProfileIdSample())
         try await sut.saveData(key: key, data: data)
         let loaded = try await sut.loadData(key: key)
         XCTAssertEqual(loaded, data)

diff --git a/apple/Tests/IntegrationTests/SargonOS/SargonOSTests.swift b/apple/Tests/IntegrationTests/SargonOS/SargonOSTests.swift
@@ -13,7 +13,7 @@ final class SargonOSTests: OSTest {
 	}
 
 	func test() async throws {
-		let _ = try await SUT.boot(
+		let _ = await SUT.boot(
 			bios: .init(
 				drivers: .test()
 			)

diff --git a/crates/sargon/src/profile/v100/header/mod.rs b/crates/sargon/src/profile/v100/header/mod.rs
@@ -7,6 +7,7 @@ mod device_info_uniffi_fn;
 mod header;
 mod header_uniffi_fn;
 mod profile_id;
+mod profile_id_uniffi_fn;
 
 pub use content_hint::*;
 pub use device_id::*;
@@ -17,3 +18,4 @@ pub use device_info_uniffi_fn::*;
 pub use header::*;
 pub use header_uniffi_fn::*;
 pub use profile_id::*;
+pub use profile_id_uniffi_fn::*;
diff --git a/crates/sargon/src/profile/v100/header/profile_id.rs b/crates/sargon/src/profile/v100/header/profile_id.rs
@@ -14,7 +14,6 @@ use crate::prelude::*;
 )]
 #[serde(transparent)]
 pub struct ProfileID(pub(crate) Uuid);
-uniffi::custom_newtype!(ProfileID, Uuid);
 
 impl FromStr for ProfileID {
     type Err = CommonError;

diff --git a/crates/sargon/src/profile/v100/header/profile_id_uniffi_fn.rs b/crates/sargon/src/profile/v100/header/profile_id_uniffi_fn.rs
@@ -0,0 +1,37 @@
+use crate::prelude::*;
+
+uniffi::custom_newtype!(ProfileID, Uuid);
+
+#[uniffi::export]
+pub fn new_profile_id_sample() -> ProfileID {
+    ProfileID::sample()
+}
+
+#[uniffi::export]
+pub fn new_profile_id_sample_other() -> ProfileID {
+    ProfileID::sample_other()
+}
+
+#[cfg(test)]
+mod uniffi_test {
+
+    use super::*;
+
+    #[allow(clippy::upper_case_acronyms)]
+    type SUT = ProfileID;
+
+    #[test]
+    fn hash_of_samples() {
+        assert_eq!(
+            HashSet::<SUT>::from_iter([
+                new_profile_id_sample(),
+                new_profile_id_sample_other(),
+                // duplicates should get removed
+                new_profile_id_sample(),
+                new_profile_id_sample_other(),
+            ])
+            .len(),
+            2
+        );
+    }
+}
diff --git a/crates/sargon/src/system/clients/client/secure_storage_client/secure_storage_client.rs b/crates/sargon/src/system/clients/client/secure_storage_client/secure_storage_client.rs
@@ -94,7 +94,7 @@ impl SecureStorageClient {
     /// Loads the Profile.
     pub async fn load_profile(&self) -> Result<Option<Profile>> {
         debug!("Loading profile");
-        self.load(SecureStorageKey::ProfileSnapshot)
+        self.load(SecureStorageKey::load_profile_snapshot())
             .await
             .inspect(|some_profile| {
                 if some_profile.is_some() {
@@ -110,10 +110,15 @@ impl SecureStorageClient {
     pub async fn save_profile(&self, profile: &Profile) -> Result<()> {
         let profile_id = profile.id();
         debug!("Saving profile with id: {}", profile_id);
-        self.save(SecureStorageKey::ProfileSnapshot, profile)
-            .await
-            .inspect(|_| debug!("Saved profile with id {}", profile_id))
-            .inspect_err(|e| error!("Failed to save profile, error {e}"))
+        self.save(
+            SecureStorageKey::ProfileSnapshot {
+                profile_id: profile.id(),
+            },
+            profile,
+        )
+        .await
+        .inspect(|_| debug!("Saved profile with id {}", profile_id))
+        .inspect_err(|e| error!("Failed to save profile, error {e}"))
     }
 
     //======
@@ -200,7 +205,9 @@ impl SecureStorageClient {
     pub async fn delete_profile(&self, id: ProfileID) -> Result<()> {
         warn!("Deleting profile with id: {}", id);
         self.driver
-            .delete_data_for_key(SecureStorageKey::ProfileSnapshot)
+            .delete_data_for_key(SecureStorageKey::ProfileSnapshot {
+                profile_id: id,
+            })
             .await
     }
 }
@@ -230,7 +237,8 @@ mod tests {
     async fn load_ok_when_none() {
         let sut = make_sut();
         assert_eq!(
-            sut.load::<Profile>(SecureStorageKey::ProfileSnapshot).await,
+            sut.load::<Profile>(SecureStorageKey::load_profile_snapshot())
+                .await,
             Ok(None)
         );
     }
@@ -239,31 +247,48 @@ mod tests {
     async fn load_successful() {
         let sut = make_sut();
 
+        let profile = Profile::sample();
         assert!(sut
-            .save(SecureStorageKey::ProfileSnapshot, &Profile::sample())
+            .save(
+                SecureStorageKey::ProfileSnapshot {
+                    profile_id: profile.id()
+                },
+                &profile
+            )
             .await
             .is_ok());
         assert_eq!(
-            sut.load::<Profile>(SecureStorageKey::ProfileSnapshot).await,
-            Ok(Some(Profile::sample()))
+            sut.load::<Profile>(SecureStorageKey::ProfileSnapshot {
+                profile_id: profile.id()
+            })
+            .await,
+            Ok(Some(profile))
         );
     }
 
     #[actix_rt::test]
     async fn load_unwrap_or_some_default_not_used() {
         let sut = make_sut();
 
+        let profile = Profile::sample();
         assert!(sut
-            .save(SecureStorageKey::ProfileSnapshot, &Profile::sample())
+            .save(
+                SecureStorageKey::ProfileSnapshot {
+                    profile_id: profile.id()
+                },
+                &profile
+            )
             .await
             .is_ok());
         assert_eq!(
             sut.load_unwrap_or::<Profile>(
-                SecureStorageKey::ProfileSnapshot,
-                Profile::sample_other()
+                SecureStorageKey::ProfileSnapshot {
+                    profile_id: profile.id()
+                },
+                profile.clone()
             )
             .await,
-            Profile::sample()
+            profile
         );
     }
 
@@ -273,7 +298,7 @@ mod tests {
 
         assert_eq!(
             sut.load_unwrap_or::<Profile>(
-                SecureStorageKey::ProfileSnapshot,
+                SecureStorageKey::load_profile_snapshot(),
                 Profile::sample_other()
             )
             .await,
@@ -350,7 +375,7 @@ mod tests {
         let (sut, _) = SecureStorageClient::ephemeral();
         assert_eq!(
             sut.save(
-                SecureStorageKey::ProfileSnapshot,
+                SecureStorageKey::load_profile_snapshot(),
                 &AlwaysFailSerialize {}
             )
             .await,

diff --git a/crates/sargon/src/system/drivers/secure_storage_driver/support/secure_storage_key.rs b/crates/sargon/src/system/drivers/secure_storage_driver/support/secure_storage_key.rs
@@ -1,12 +1,60 @@
 use crate::prelude::*;
+use std::hash::{Hash, Hasher};
 
-#[derive(Debug, Clone, PartialEq, Eq, Hash, uniffi::Enum)]
+#[derive(Debug, Clone, Eq, uniffi::Enum)]
 pub enum SecureStorageKey {
     HostID,
     DeviceFactorSourceMnemonic {
         factor_source_id: FactorSourceIDFromHash,
     },
-    ProfileSnapshot,
+    ProfileSnapshot {
+        // Note:
+        // `profile_id` is only meant to be used by the iOS Host for backward compatibility.
+        // iOS Host stores multiple profiles in the secure storage uniquely identified by `profile_id`,
+        // while Android Host stores only one profile in the secure storage.
+        profile_id: ProfileID,
+    },
+}
+
+impl PartialEq<SecureStorageKey> for SecureStorageKey {
+    fn eq(&self, other: &SecureStorageKey) -> bool {
+        match (self, other) {
+            (SecureStorageKey::HostID, SecureStorageKey::HostID) => true,
+            (
+                SecureStorageKey::DeviceFactorSourceMnemonic {
+                    factor_source_id: a,
+                },
+                SecureStorageKey::DeviceFactorSourceMnemonic {
+                    factor_source_id: b,
+                },
+            ) => a == b,
+            (
+                SecureStorageKey::ProfileSnapshot { .. },
+                SecureStorageKey::ProfileSnapshot { .. },
+            ) => true, // Note: `profile_id` is not used for comparison, as it is only forwarded as additional payload to the iOS Host.
+            _ => false,
+        }
+    }
+}
+
+impl Hash for SecureStorageKey {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        match self {
+            SecureStorageKey::HostID => {
+                "host_id".hash(state);
+            }
+            SecureStorageKey::DeviceFactorSourceMnemonic {
+                factor_source_id,
+            } => {
+                "device_factor_source".hash(state);
+                factor_source_id.hash(state);
+            }
+            // Note: `profile_id` is not used for computing the hash, as it is only forwarded as additional payload to the iOS Host.
+            SecureStorageKey::ProfileSnapshot { .. } => {
+                "profile_snapshot".hash(state);
+            }
+        }
+    }
 }
 
 impl SecureStorageKey {
@@ -19,13 +67,24 @@ impl SecureStorageKey {
                 SecureStorageKey::DeviceFactorSourceMnemonic {
                     factor_source_id,
                 } => format!("device_factor_source_{}", factor_source_id),
-                SecureStorageKey::ProfileSnapshot =>
+                SecureStorageKey::ProfileSnapshot { .. } =>
                     "profile_snapshot".to_owned(),
             }
         )
     }
 }
 
+impl SecureStorageKey {
+    pub fn load_profile_snapshot() -> Self {
+        // This id will not be used to load the profile snapshot.
+        // It is only a stub to conform to the SecureStorageKey definition.
+        let dummy_id = ProfileID(Uuid::from_bytes([0x00; 16]));
+        SecureStorageKey::ProfileSnapshot {
+            profile_id: dummy_id,
+        }
+    }
+}
+
 #[uniffi::export]
 pub fn secure_storage_key_identifier(key: &SecureStorageKey) -> String {
     key.identifier()
@@ -45,7 +104,7 @@ mod tests {
             "secure_storage_key_device_factor_source_device:f1a93d324dd0f2bff89963ab81ed6e0c2ee7e18c0827dc1d3576b2d9f26bbd0a"
         );
         assert_eq!(
-            SecureStorageKey::ProfileSnapshot.identifier(),
+            SecureStorageKey::load_profile_snapshot().identifier(),
             "secure_storage_key_profile_snapshot"
         );
     }
@@ -57,7 +116,7 @@ mod uniffi_tests {
 
     #[test]
     fn identifier() {
-        let key = SecureStorageKey::ProfileSnapshot;
+        let key = SecureStorageKey::load_profile_snapshot();
         assert_eq!(
             key.clone().identifier(),
             secure_storage_key_identifier(&key)

diff --git a/crates/sargon/src/system/sargon_os/sargon_os_profile.rs b/crates/sargon/src/system/sargon_os/sargon_os_profile.rs
@@ -164,7 +164,12 @@ impl SargonOS {
         let secure_storage = &self.secure_storage;
 
         secure_storage
-            .save(SecureStorageKey::ProfileSnapshot, profile)
+            .save(
+                SecureStorageKey::ProfileSnapshot {
+                    profile_id: profile.id(),
+                },
+                profile,
+            )
             .await?;
 
         self.event_bus

diff --git a/jvm/sargon-android/src/main/java/com/radixdlt/sargon/os/driver/AndroidStorageDriver.kt b/jvm/sargon-android/src/main/java/com/radixdlt/sargon/os/driver/AndroidStorageDriver.kt
@@ -61,7 +61,6 @@ internal class AndroidStorageDriver(
 
     private fun SecureStorageKey.mapping() = when (this) {
         is SecureStorageKey.ProfileSnapshot -> ProfileSnapshotKeyMapping(
-            key = this,
             encryptedStorage = encryptedPreferencesDatastore
         )
 

diff --git a/...gon-android/src/main/java/com/radixdlt/sargon/os/storage/key/ProfileSnapshotKeyMapping.kt b/...gon-android/src/main/java/com/radixdlt/sargon/os/storage/key/ProfileSnapshotKeyMapping.kt
@@ -16,7 +16,6 @@ import kotlinx.coroutines.delay
 import java.io.IOException
 
 internal class ProfileSnapshotKeyMapping(
-    private val key: SecureStorageKey.ProfileSnapshot,
     private val encryptedStorage: DataStore<Preferences>
 ) : DatastoreKeyMapping {
 

diff --git a/jvm/sargon-android/src/test/java/com/radixdlt/sargon/SecureStorageKeyTest.kt b/jvm/sargon-android/src/test/java/com/radixdlt/sargon/SecureStorageKeyTest.kt
@@ -23,7 +23,7 @@ class SecureStorageKeyTest {
 
         assertEquals(
             "secure_storage_key_profile_snapshot",
-            SecureStorageKey.ProfileSnapshot.identifier
+            SecureStorageKey.ProfileSnapshot(profileId = newProfileIdSample()).identifier
         )
     }