Cache TOTP discovery and report progress (#74)

crates/storage/tests/mobile_totp.rs

@@ -2,13 +2,20 @@
 
 mod support;
 
-use std::collections::{BTreeMap, BTreeSet};
+use std::{
+    collections::{BTreeMap, BTreeSet},
+    fs,
+};
 
 use ironstorage::{
     crypto::{KeyInfo, KeyStore, SecretProvider, SecretProviderError},
-    mobile_totp::{MobileTotpService, MobileWatchSnapshotState},
+    mobile_totp::{
+        MobileTotpDiscoveryPhase, MobileTotpError, MobileTotpOperation, MobileTotpService,
+        MobileWatchSnapshotState,
+    },
+    otp::OtpError,
     recipient::RecipientPolicyManager,
-    repository::{EntryPath, Repository, SecretBytes},
+    repository::{EncryptedEntry, EntryPath, Repository, SecretBytes},
 };
 use support::compatibility::{FixtureSet, TestResult};
 
@@ -42,6 +49,63 @@ impl SecretProvider for FixtureSecrets {
     }
 }
 
+struct CountingSecrets {
+    inner: FixtureSecrets,
+    requests: usize,
+}
+
+impl CountingSecrets {
+    fn all(fixture: &FixtureSet) -> Self {
+        Self {
+            inner: FixtureSecrets::all(fixture),
+            requests: 0,
+        }
+    }
+}
+
+impl SecretProvider for CountingSecrets {
+    fn secret_for(&mut self, key: &KeyInfo) -> Result<SecretBytes, SecretProviderError> {
+        self.requests += 1;
+        self.inner.secret_for(key)
+    }
+}
+
+struct CancellingSecrets<'a> {
+    inner: FixtureSecrets,
+    operation: &'a MobileTotpOperation,
+    requests: usize,
+}
+
+struct MutatingSecrets<'a> {
+    inner: FixtureSecrets,
+    repository: &'a Repository,
+    path: EntryPath,
+    replacement: Option<EncryptedEntry>,
+}
+
+impl SecretProvider for MutatingSecrets<'_> {
+    fn secret_for(&mut self, key: &KeyInfo) -> Result<SecretBytes, SecretProviderError> {
+        let secret = self.inner.secret_for(key)?;
+        if let Some(replacement) = self.replacement.take() {
+            self.repository
+                .write_entry(&self.path, &replacement)
+                .expect("fixture mutation succeeds");
+        }
+        Ok(secret)
+    }
+}
+
+impl SecretProvider for CancellingSecrets<'_> {
+    fn secret_for(&mut self, key: &KeyInfo) -> Result<SecretBytes, SecretProviderError> {
+        self.requests += 1;
+        let secret = self.inner.secret_for(key)?;
+        if self.requests == 1 {
+            self.operation.cancel();
+        }
+        Ok(secret)
+    }
+}
+
 #[test]
 fn mobile_totp_catalog_details_and_watch_selection_are_storage_owned() -> TestResult {
     let fixture = FixtureSet::load()?;
@@ -99,6 +163,218 @@ fn mobile_totp_catalog_details_and_watch_selection_are_storage_owned() -> TestRe
     Ok(())
 }
 
+#[test]
+fn totp_cache_reuses_ciphertext_hashes_and_removes_deleted_entries() -> TestResult {
+    let fixture = FixtureSet::load()?;
+    let store = fixture.materialize_store("basic")?;
+    let repository = Repository::open(store.path())?;
+    let keys = KeyStore::load(fixture.path("keys"))?;
+    write_plaintext(
+        &repository,
+        &keys,
+        "otp/alice",
+        b"password\notpauth://totp/Acme:alice@example.com?secret=GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ&issuer=Acme\n",
+    )?;
+    for path in ["team-a", "team-a/shared", "team-b/shared"] {
+        write_plaintext(
+            &repository,
+            &keys,
+            path,
+            b"otpauth://totp/Shared?secret=GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ\n",
+        )?;
+    }
+    write_plaintext(&repository, &keys, "ordinary", b"password\nlogin: alice\n")?;
+    let cache_directory = tempfile::tempdir()?;
+    let cache = cache_directory.path().join("totp-catalog.toml");
+    let service = MobileTotpService::new(&repository, &keys);
+
+    let cold = MobileTotpOperation::default();
+    let mut cold_secrets = CountingSecrets::all(&fixture);
+    let page = service.discover(&BTreeSet::new(), &mut cold_secrets, &cache, &cold)?;
+    assert!(page.rows().iter().any(|row| row.path() == "otp/alice"));
+    assert!(page.rows().iter().any(|row| row.path() == "team-a"));
+    assert!(page.rows().iter().any(|row| row.path() == "team-a/shared"));
+    assert!(page.rows().iter().any(|row| row.path() == "team-b/shared"));
+    assert_eq!(cold.progress().phase(), MobileTotpDiscoveryPhase::Complete);
+    assert_eq!(cold.progress().inspected(), cold.progress().total());
+    assert!(cold_secrets.requests > 0);
+
+    let encoded = fs::read_to_string(&cache)?;
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt as _;
+
+        assert_eq!(fs::metadata(&cache)?.permissions().mode() & 0o777, 0o600);
+    }
+    assert!(encoded.contains("is_totp = true"));
+    assert!(encoded.contains("ciphertext_hash ="));
+    assert!(encoded.contains("otp/alice"));
+    for secret in [
+        "otpauth://",
+        "secret=",
+        "alice@example.com",
+        "Acme",
+        "password",
+    ] {
+        assert!(!encoded.contains(secret), "cache leaked {secret}");
+    }
+
+    let warm = MobileTotpOperation::default();
+    let mut warm_secrets = CountingSecrets::all(&fixture);
+    let warm_page = service.discover(&BTreeSet::new(), &mut warm_secrets, &cache, &warm)?;
+    assert_eq!(warm_secrets.requests, 0);
+    assert_eq!(warm.progress().cache_hits(), warm.progress().total());
+    assert_eq!(warm_page.rows(), page.rows());
+    assert_eq!(
+        service
+            .cached_page(&BTreeSet::new(), &cache)
+            .expect("created cache")
+            .rows(),
+        page.rows()
+    );
+
+    write_plaintext(
+        &repository,
+        &keys,
+        "ordinary",
+        b"otpauth://totp/New:new@example.com?secret=GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ&issuer=New\n",
+    )?;
+    let changed = MobileTotpOperation::default();
+    let mut changed_secrets = CountingSecrets::all(&fixture);
+    let changed_page =
+        service.discover(&BTreeSet::new(), &mut changed_secrets, &cache, &changed)?;
+    assert_eq!(changed_secrets.requests, 1);
+    assert_eq!(
+        changed.progress().cache_hits() + 1,
+        changed.progress().total()
+    );
+    assert!(
+        changed_page
+            .rows()
+            .iter()
+            .any(|row| row.path() == "ordinary")
+    );
+
+    repository.remove_entry(&EntryPath::parse("otp/alice")?)?;
+    let deleted = MobileTotpOperation::default();
+    let mut deleted_secrets = CountingSecrets::all(&fixture);
+    let deleted_page =
+        service.discover(&BTreeSet::new(), &mut deleted_secrets, &cache, &deleted)?;
+    assert_eq!(deleted_secrets.requests, 0);
+    assert!(
+        deleted_page
+            .rows()
+            .iter()
+            .all(|row| row.path() != "otp/alice")
+    );
+    assert!(
+        service
+            .cached_page(&BTreeSet::new(), &cache)
+            .expect("updated cache")
+            .rows()
+            .iter()
+            .all(|row| row.path() != "otp/alice")
+    );
+
+    let cancelled = MobileTotpOperation::default();
+    cancelled.cancel();
+    let mut cancelled_secrets = CountingSecrets::all(&fixture);
+    assert!(matches!(
+        service.discover(&BTreeSet::new(), &mut cancelled_secrets, &cache, &cancelled,),
+        Err(MobileTotpError::Otp(OtpError::Cancelled))
+    ));
+    assert_eq!(
+        cancelled.progress().phase(),
+        MobileTotpDiscoveryPhase::Cancelled
+    );
+
+    fs::write(&cache, "not a TOTP catalog")?;
+    let recovery = MobileTotpOperation::default();
+    let mut recovery_secrets = CountingSecrets::all(&fixture);
+    let recovered = service.discover(&BTreeSet::new(), &mut recovery_secrets, &cache, &recovery)?;
+    assert!(recovered.cache_notice().is_some());
+    assert!(service.cached_page(&BTreeSet::new(), &cache).is_some());
+
+    let other_store = fixture.materialize_store("nested")?;
+    let other_repository = Repository::open(other_store.path())?;
+    assert!(
+        MobileTotpService::new(&other_repository, &keys)
+            .cached_page(&BTreeSet::new(), &cache)
+            .is_none()
+    );
+    Ok(())
+}
+
+#[test]
+fn cancelled_discovery_checkpoints_completed_entries() -> TestResult {
+    let fixture = FixtureSet::load()?;
+    let store = fixture.materialize_store("basic")?;
+    let repository = Repository::open(store.path())?;
+    let keys = KeyStore::load(fixture.path("keys"))?;
+    let cache_directory = tempfile::tempdir()?;
+    let cache = cache_directory.path().join("totp-catalog.toml");
+    let service = MobileTotpService::new(&repository, &keys);
+    let interrupted = MobileTotpOperation::default();
+    let mut secrets = CancellingSecrets {
+        inner: FixtureSecrets::all(&fixture),
+        operation: &interrupted,
+        requests: 0,
+    };
+
+    assert!(matches!(
+        service.discover(&BTreeSet::new(), &mut secrets, &cache, &interrupted,),
+        Err(MobileTotpError::Otp(OtpError::Cancelled))
+    ));
+    assert!(cache.is_file());
+
+    let resumed = MobileTotpOperation::default();
+    let mut resumed_secrets = CountingSecrets::all(&fixture);
+    service.discover(&BTreeSet::new(), &mut resumed_secrets, &cache, &resumed)?;
+    assert!(resumed.progress().cache_hits() >= 1);
+    assert!(resumed_secrets.requests < resumed.progress().total() as usize);
+    Ok(())
+}
+
+#[test]
+fn discovery_rejects_a_result_when_ciphertext_changes_mid_scan() -> TestResult {
+    let fixture = FixtureSet::load()?;
+    let store = fixture.materialize_store("basic")?;
+    let repository = Repository::open(store.path())?;
+    let keys = KeyStore::load(fixture.path("keys"))?;
+    let path = EntryPath::parse("ordinary")?;
+    write_plaintext(&repository, &keys, "ordinary", b"password\n")?;
+    let recipients =
+        RecipientPolicyManager::new(&repository, &keys).resolve_for_entry(&path, None)?;
+    let replacement = keys.encrypt(
+        SecretBytes::new(b"changed password\n".to_vec()),
+        recipients.recipients(),
+    )?;
+    let cache_directory = tempfile::tempdir()?;
+    let cache = cache_directory.path().join("totp-catalog.toml");
+    let operation = MobileTotpOperation::default();
+    let mut secrets = MutatingSecrets {
+        inner: FixtureSecrets::all(&fixture),
+        repository: &repository,
+        path,
+        replacement: Some(replacement),
+    };
+
+    assert!(matches!(
+        MobileTotpService::new(&repository, &keys).discover(
+            &BTreeSet::new(),
+            &mut secrets,
+            &cache,
+            &operation,
+        ),
+        Err(MobileTotpError::ConcurrentModification)
+    ));
+    assert_ne!(
+        operation.progress().phase(),
+        MobileTotpDiscoveryPhase::Complete
+    );
+    Ok(())
+}
+
 fn write_plaintext(
     repository: &Repository,
     keys: &KeyStore,

crates/storage/tests/repository_core.rs

@@ -24,7 +24,6 @@ fn ordinary_nested_and_unicode_pass_trees_are_discovered() -> TestResult {
     assert_eq!(paths.len(), 6);
     assert!(paths.contains(&Path::new("email/personal").to_owned()));
     assert!(paths.contains(&Path::new("unicode/咖啡").to_owned()));
-    assert_eq!(snapshot.collisions().len(), 0);
     assert_eq!(snapshot.auxiliary_files().len(), 0);
     assert_eq!(snapshot.recipient_policies().len(), 3);
 
@@ -86,38 +85,54 @@ fn innermost_git_repository_is_selected_without_scanning_git_objects() -> TestRe
 }
 
 #[test]
-fn entry_and_directory_ambiguity_requires_explicit_directory_syntax() -> TestResult {
+fn entry_and_directory_with_the_same_logical_path_remain_independent() -> TestResult {
     let temporary = tempfile::tempdir()?;
     fs::create_dir(temporary.path().join("ambiguous"))?;
     fs::write(temporary.path().join("ambiguous.gpg"), b"ciphertext")?;
+    fs::write(
+        temporary.path().join("ambiguous/child.gpg"),
+        b"child ciphertext",
+    )?;
     let repository = Repository::open(temporary.path())?;
     let snapshot = repository.snapshot()?;
 
-    assert_eq!(
-        snapshot.resolve("ambiguous").expect_err("ambiguous path"),
-        RepositoryError::AmbiguousPath {
-            path: Path::new("ambiguous").to_owned()
-        }
-    );
+    assert!(matches!(
+        snapshot.resolve("ambiguous")?,
+        ResolvedObject::Entry(entry) if entry.path().as_path() == Path::new("ambiguous")
+    ));
     assert!(matches!(
         snapshot.resolve("ambiguous/")?,
         ResolvedObject::Directory(directory)
             if directory.path().as_path() == Path::new("ambiguous")
     ));
     assert_eq!(
-        snapshot.collisions().collect::<Vec<_>>(),
-        [&Path::new("ambiguous").to_owned()]
+        snapshot
+            .entries()
+            .map(|entry| entry.path().as_path())
+            .collect::<Vec<_>>(),
+        [Path::new("ambiguous"), Path::new("ambiguous/child")]
     );
 
-    let original = fs::read(temporary.path().join("ambiguous.gpg"))?;
-    assert!(matches!(
-        repository.write_entry(
-            &EntryPath::parse("ambiguous")?,
-            &EncryptedEntry::new(b"replacement".to_vec())
-        ),
-        Err(RepositoryError::Collision { .. })
-    ));
-    assert_eq!(fs::read(temporary.path().join("ambiguous.gpg"))?, original);
+    repository.write_entry(
+        &EntryPath::parse("ambiguous")?,
+        &EncryptedEntry::new(b"replacement".to_vec()),
+    )?;
+    repository.write_entry(
+        &EntryPath::parse("ambiguous/new-child")?,
+        &EncryptedEntry::new(b"new child".to_vec()),
+    )?;
+    assert_eq!(
+        repository
+            .read_entry(&EntryPath::parse("ambiguous")?)?
+            .as_bytes(),
+        b"replacement"
+    );
+    assert_eq!(
+        repository
+            .read_entry(&EntryPath::parse("ambiguous/new-child")?)?
+            .as_bytes(),
+        b"new child"
+    );
     Ok(())
 }
 

crates/storage/tests/tree_mutations.rs

@@ -320,7 +320,7 @@ fn entry_collisions_require_confirmation_unless_forced() -> TestResult {
 }
 
 #[test]
-fn mutation_rejects_ambiguous_sources_and_same_object_targets() -> TestResult {
+fn mutation_prefers_an_entry_over_its_same_named_directory() -> TestResult {
     let fixture = FixtureSet::load()?;
     let store = fixture.materialize_store("basic")?;
     fs::create_dir(store.path().join("email/personal"))?;
@@ -329,22 +329,26 @@ fn mutation_rejects_ambiguous_sources_and_same_object_targets() -> TestResult {
     let mut provider = Secrets::all(&fixture);
     let mutator = TreeMutator::new(&repository, &keys);
 
-    assert!(matches!(
-        mutator.copy(
-            &CopyRequest {
-                source: "email/personal".into(),
-                destination: "elsewhere".into(),
-                force: false,
-            },
-            OverwriteDecision::Allow,
-            None,
+    mutator.copy(
+        &CopyRequest {
+            source: "email/personal".into(),
+            destination: "elsewhere".into(),
+            force: false,
+        },
+        OverwriteDecision::Allow,
+        None,
+        &mut provider,
+        &mut Committer::default(),
+    )?;
+    assert!(store.path().join("email/personal").is_dir());
+    assert_eq!(
+        keys.decrypt(
+            &repository.read_entry(&EntryPath::parse("elsewhere")?)?,
             &mut provider,
-            &mut Committer::default()
-        ),
-        Err(MutationError::Repository(
-            ironstorage::repository::RepositoryError::AmbiguousPath { .. }
-        ))
-    ));
+        )?
+        .expose(),
+        fixture.read("expected/basic/email/personal.txt")?
+    );
     assert!(matches!(
         mutator.copy(
             &CopyRequest {