Test the folder sign-in instead of trusting it

The whole of a folder library's sign-in lived inside a Slint callback,
which cannot run without a display server — so the one path that decides
whether a mistyped folder becomes a *stored* account had no test at all.
That failure is quiet and lasting: an account for a directory that is
not there skips the launch screen on the next start and reads as a
library that has lost its photographs.

`open_folder_library` is that logic, lifted out whole. Four tests: it
stores an account with no credential and reaches the keyring for
nothing, a typo is refused before anything is written, the messages read
as instructions because they go straight to the screen's error line, and
a file is not a library.
This commit is contained in:
2026-08-29 09:57:52 +02:00
parent cbe5c4fcde
commit 64462e9fd3
2 changed files with 121 additions and 42 deletions
+118 -39
View File
@@ -155,47 +155,13 @@ where
let ctl = controller.clone();
window.on_launch_use_folder(move |path| {
let Some(w) = weak.upgrade() else { return };
let provider = match crate::remote::registry().get(dr_sync_folder::BACKEND_ID) {
Some(p) => p.clone(),
None => {
ctl.model
.borrow_mut()
.fail("this build has no folder support");
render(&w, &ctl);
return;
match open_folder_library(&ctl.store, &path) {
Ok(account) => {
log::info!("using folder library at {}", account.endpoint);
ctl.model.borrow_mut().signed_in(account);
}
};
// The connector checks the directory before an account is written
// for it. A typo stored here would skip the launch screen next
// start and surface as a library that finds nothing.
let endpoint = match provider.normalise_endpoint(&path) {
Ok(e) => e,
Err(e) => {
ctl.model.borrow_mut().fail(e);
render(&w, &ctl);
return;
}
};
let account = match provider.account_for(&endpoint) {
Ok(a) => a,
Err(e) => {
ctl.model.borrow_mut().fail(e.to_string());
render(&w, &ctl);
return;
}
};
// `None`: there is no credential, and asking the keyring for one
// would fail on a machine with no secrets daemon — where a folder
// library is exactly the thing that should still work.
if let Err(e) = ctl.store.save(&account, None) {
log::warn!("persisting account: {e}");
Err(e) => ctl.model.borrow_mut().fail(e),
}
log::info!("using folder library at {}", account.endpoint);
ctl.model.borrow_mut().signed_in(account);
render(&w, &ctl);
});
}
@@ -347,6 +313,40 @@ where
render(window, &controller);
}
/// TRACES: FR-NC-13
/// Establish a folder library, returning the account to sign in as.
///
/// The whole of a [`SignIn::EndpointOnly`](dr_sync::SignIn) sign-in: check the
/// endpoint, build the account, persist it. Split out of the callback rather
/// than written inline because a Slint callback cannot be tested without a
/// display server, and this is the path that decides whether a mistyped folder
/// becomes a stored account — the failure that would then skip the launch
/// screen on the next start and surface as a library that finds nothing.
///
/// The error is a string because it goes straight to the screen's error line;
/// the connector wrote it to say what to fix.
fn open_folder_library(store: &AccountStore, path: &str) -> Result<Account, String> {
let provider = crate::remote::registry()
.get(dr_sync_folder::BACKEND_ID)
.ok_or("this build has no folder support")?;
// The connector checks the directory before an account is written for it.
let endpoint = provider.normalise_endpoint(path)?;
let account = provider.account_for(&endpoint).map_err(|e| e.to_string())?;
// `None`: there is no credential, and asking the keyring for one would
// fail on a machine with no secrets daemon — where a folder library is
// exactly the thing that should still work.
//
// A failure to persist is reported, not fatal: the library opens for this
// session and the user is asked again next launch, which is a great deal
// better than refusing to open a folder that is plainly there.
if let Err(e) = store.save(&account, None) {
log::warn!("persisting account: {e}");
}
Ok(account)
}
/// Run Login Flow v2 without blocking the UI thread.
///
/// Slint's event loop is single-threaded, so the network work happens on a
@@ -901,3 +901,82 @@ fn android_open_url(url: &str) -> Result<(), String> {
})
.map_err(|e: jni::errors::Error| e.to_string())
}
#[cfg(test)]
mod tests {
use super::*;
use dr_plat::EphemeralSecretStore;
fn store_in(dir: &std::path::Path) -> AccountStore {
AccountStore::open_at(
dir.join("sessions.json"),
Box::new(EphemeralSecretStore::new()),
)
}
fn tmpdir(name: &str) -> std::path::PathBuf {
let d = std::env::temp_dir().join(format!("dr-launch-folder-{name}"));
let _ = std::fs::remove_dir_all(&d);
std::fs::create_dir_all(&d).unwrap();
d
}
#[test]
fn opening_a_folder_stores_an_account_with_no_credential() {
// The whole sign-in, end to end through the registry: no browser, no
// keyring, no waiting state.
let dir = tmpdir("ok");
let library = dir.join("Photos");
std::fs::create_dir_all(&library).unwrap();
let store = store_in(&dir);
let account = open_folder_library(&store, &library.to_string_lossy()).unwrap();
assert_eq!(account.backend, dr_sync_folder::BACKEND_ID);
assert!(account.login.is_empty(), "a folder has nobody to name");
// And it survives, so the next launch skips the screen.
let reloaded = store.current().expect("persisted");
assert_eq!(reloaded.endpoint, account.endpoint);
// With nothing in the keyring — the machine may have no secrets daemon
// at all, which is precisely when a folder library matters.
assert!(store.connection(&reloaded, false).unwrap().secret.is_none());
}
#[test]
fn a_mistyped_folder_is_refused_rather_than_stored() {
// The failure this guards: a stored account for a folder that is not
// there skips the launch screen next start and reads as a library
// that has lost its photographs.
let dir = tmpdir("typo");
let store = store_in(&dir);
let err = open_folder_library(&store, &dir.join("Pictrues").to_string_lossy()).unwrap_err();
assert!(err.contains("No folder"), "{err}");
assert!(store.current().is_none(), "nothing may be persisted");
}
#[test]
fn the_error_says_what_to_fix() {
// It goes straight to the screen's error line, so it has to read as
// instruction rather than as a type name.
let dir = tmpdir("messages");
let store = store_in(&dir);
for (input, want) in [("", "Choose"), ("Pictures", "full path")] {
let err = open_folder_library(&store, input).unwrap_err();
assert!(err.contains(want), "{input:?} gave {err:?}");
}
}
#[test]
fn a_file_is_not_a_library() {
let dir = tmpdir("file");
let f = dir.join("a.CR2");
std::fs::write(&f, b"raw").unwrap();
let store = store_in(&dir);
let err = open_folder_library(&store, &f.to_string_lossy()).unwrap_err();
assert!(err.contains("not a folder"), "{err}");
}
}