Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Profile snapshot id payload #214

Merged
merged 8 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apple/Sources/Sargon/SargonOS/SargonOS+Static+Shared.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
9 changes: 2 additions & 7 deletions apple/Sources/Sargon/SargonOS/TestOS.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

}
}

Expand Down
8 changes: 4 additions & 4 deletions apple/Sources/Sargon/Util/EventPublisher.swift
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion apple/Tests/IntegrationTests/SargonOS/SargonOSTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
)
Expand Down
2 changes: 2 additions & 0 deletions crates/sargon/src/profile/v100/header/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand All @@ -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::*;
1 change: 0 additions & 1 deletion crates/sargon/src/profile/v100/header/profile_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
37 changes: 37 additions & 0 deletions crates/sargon/src/profile/v100/header/profile_id_uniffi_fn.rs
Original file line number Diff line number Diff line change
@@ -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
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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}"))
}

//======
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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)
);
}
Expand All @@ -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
);
}

Expand All @@ -273,7 +298,7 @@ mod tests {

assert_eq!(
sut.load_unwrap_or::<Profile>(
SecureStorageKey::ProfileSnapshot,
SecureStorageKey::load_profile_snapshot(),
Profile::sample_other()
)
.await,
Expand Down Expand Up @@ -350,7 +375,7 @@ mod tests {
let (sut, _) = SecureStorageClient::ephemeral();
assert_eq!(
sut.save(
SecureStorageKey::ProfileSnapshot,
SecureStorageKey::load_profile_snapshot(),
&AlwaysFailSerialize {}
)
.await,
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -19,13 +67,22 @@ 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 {
let profile_id_noop = ProfileID::sample();
GhenadieVP marked this conversation as resolved.
Show resolved Hide resolved
SecureStorageKey::ProfileSnapshot {
profile_id: profile_id_noop,
}
}
}

#[uniffi::export]
pub fn secure_storage_key_identifier(key: &SecureStorageKey) -> String {
key.identifier()
Expand All @@ -45,7 +102,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"
);
}
Expand All @@ -57,7 +114,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)
Expand Down
7 changes: 6 additions & 1 deletion crates/sargon/src/system/sargon_os/sargon_os_profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ internal class AndroidStorageDriver(

private fun SecureStorageKey.mapping() = when (this) {
is SecureStorageKey.ProfileSnapshot -> ProfileSnapshotKeyMapping(
key = this,
encryptedStorage = encryptedPreferencesDatastore
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down
Loading
Loading