From e1cb3d2308cca7d319c0267417db0e4778d5cb6b Mon Sep 17 00:00:00 2001 From: gnacho Date: Fri, 14 Aug 2026 18:03:42 +0200 Subject: [PATCH] fix(credentials): find and adopt Python-era keyring entries; resolve push by account id Python nextsync stored secrets with {server, username} attributes under schema io.github.gnacho.nextsync.Account; the Rust store only searched the account_id key, so migrated accounts had no credentials: empty remote-folder picker, syncs unable to authenticate. get_for_account now falls back to the legacy attributes and adopts the secret under account_id (legacy item left in place). The notify_push configure path also resolved the keyring with the login name instead of the account id and could never authenticate, even for wizard-created accounts. Closes #11 --- src/main.rs | 41 +++++++----- src/nextcloud/credentials.rs | 122 ++++++++++++++++++++++++++++++++++- src/nextcloud/sync_engine.rs | 6 +- src/ui/settings.rs | 8 ++- src/ui/setup.rs | 6 +- 5 files changed, 163 insertions(+), 20 deletions(-) diff --git a/src/main.rs b/src/main.rs index a44a7ba..fb97f76 100644 --- a/src/main.rs +++ b/src/main.rs @@ -84,23 +84,32 @@ fn main() { // handle is shared with the runtime, so configuring the clone // reaches the running push channel. { - let push_clients: Vec<(NotifyPushClient, String, String, bool)> = account_manager - .runtimes() - .values() - .filter_map(|runtime| { - runtime.push_client().map(|client| { - ( - client, - runtime.account.server_url.clone(), - runtime.account.login_name.clone(), - runtime.account.sync.remote_push_enabled, - ) + let push_clients: Vec<(NotifyPushClient, String, String, String, bool)> = + account_manager + .runtimes() + .values() + .filter_map(|runtime| { + runtime.push_client().map(|client| { + ( + client, + runtime.account.id.clone(), + runtime.account.server_url.clone(), + runtime.account.login_name.clone(), + runtime.account.sync.remote_push_enabled, + ) + }) }) - }) - .collect(); - for (client, server, username, enabled) in push_clients { - let keyring_user = username.clone(); - let task = gio::spawn_blocking(move || CredentialsStore::get(&keyring_user)); + .collect(); + for (client, account_id, server, username, enabled) in push_clients { + let server_for_lookup = server.clone(); + let username_for_lookup = username.clone(); + let task = gio::spawn_blocking(move || { + CredentialsStore::get_for_account( + &account_id, + &server_for_lookup, + &username_for_lookup, + ) + }); glib::spawn_future_local(async move { if let Ok(Ok(Some(password))) = task.await { client.configure(&server, &username, &password, enabled); diff --git a/src/nextcloud/credentials.rs b/src/nextcloud/credentials.rs index fe3b542..865cebd 100644 --- a/src/nextcloud/credentials.rs +++ b/src/nextcloud/credentials.rs @@ -2,7 +2,11 @@ //! //! Per-account passwords are stored in the default collection of the desktop //! Secret Service (`org.freedesktop.secrets`) keyed by `account_id`, so they -//! survive a reinstall and are shared with the Python `nextsync` v0.2.x. +//! survive a reinstall. Accounts created by the Python `nextsync` app stored +//! their secret with `{server, username}` attributes instead; those entries +//! are picked up through [`CredentialsStore::get_for_account`] and adopted +//! (re-stored under `account_id`, leaving the legacy item untouched for the +//! Python app). //! //! Uses `secret_service::blocking` (feature `rt-tokio-crypto-rust`, DH //! encrypted session). Blocking calls must not run on the async UI loop. @@ -18,6 +22,10 @@ pub type CredentialError = secret_service::Error; /// Attribute key used to index items by account id. const ATTR_ACCOUNT_ID: &str = "account_id"; +/// Attribute keys used by the legacy Python `nextsync` entries. +const ATTR_SERVER: &str = "server"; +const ATTR_USERNAME: &str = "username"; + /// Secret content type used for stored passwords. const CONTENT_TYPE: &str = "text/plain"; @@ -54,6 +62,36 @@ impl CredentialsStore { Ok(Some(String::from_utf8_lossy(&secret).into_owned())) } + /// Read the password for an account, falling back to the legacy entry. + /// + /// Tries the `account_id` item first; when absent, searches the legacy + /// Python `nextsync` attributes (`{server, username}`). A legacy hit is + /// adopted: the secret is re-stored under `account_id` so later lookups + /// hit the fast path, while the legacy item itself is left in place for + /// the Python app. Adoption failure is not fatal — the password is still + /// returned. + pub fn get_for_account( + account_id: &str, + server: &str, + login: &str, + ) -> Result, CredentialError> { + if let Some(password) = Self::get(account_id)? { + return Ok(Some(password)); + } + let service = SecretService::connect(EncryptionType::Dh)?; + let result = service.search_items(HashMap::from([ + (ATTR_SERVER, server), + (ATTR_USERNAME, login), + ]))?; + let Some(item) = result.unlocked.first() else { + return Ok(None); + }; + let secret = item.get_secret()?; + let password = String::from_utf8_lossy(&secret).into_owned(); + let _ = Self::set(account_id, &password); + Ok(Some(password)) + } + /// Delete the stored password for an account, if any. pub fn delete(account_id: &str) -> Result<(), CredentialError> { let service = SecretService::connect(EncryptionType::Dh)?; @@ -74,6 +112,8 @@ mod tests { const TEST_ACCOUNT: &str = "5fcc57b6eeae77370e1f1b1a1a608d97511bab8cf29c0e02beabeb3e9a393592"; const TEST_PASSWORD: &str = "correct-horse-battery-staple"; + const TEST_LEGACY_SERVER: &str = "https://legacy-unit-test.example.net"; + const TEST_LEGACY_LOGIN: &str = "legacy-unit-test@example.net"; /// Removes the test item even if the test panics. struct Cleanup; @@ -114,4 +154,84 @@ mod tests { let gone = CredentialsStore::get(TEST_ACCOUNT).expect("get should succeed"); assert!(gone.is_none()); } + + /// Removes the legacy-attribute test item even if the test panics. + struct LegacyCleanup; + + impl Drop for LegacyCleanup { + fn drop(&mut self) { + if let Ok(service) = SecretService::connect(EncryptionType::Dh) { + if let Ok(result) = service.search_items(HashMap::from([ + (ATTR_SERVER, TEST_LEGACY_SERVER), + (ATTR_USERNAME, TEST_LEGACY_LOGIN), + ])) { + for item in result.unlocked.iter().chain(result.locked.iter()) { + let _ = item.delete(); + } + } + } + let _ = CredentialsStore::delete(TEST_ACCOUNT); + } + } + + /// A legacy Python-era entry (`{server, username}` attributes) must be + /// found by `get_for_account`, returned, and adopted under `account_id`. + #[test] + fn legacy_python_entry_is_found_and_adopted() { + match SecretService::connect(EncryptionType::Dh) { + Ok(_) => {} + Err(_) => { + eprintln!("no Secret Service session bus available; skipping"); + return; + } + } + let _guard = LegacyCleanup; + + let _ = CredentialsStore::delete(TEST_ACCOUNT); + let service = SecretService::connect(EncryptionType::Dh).expect("connect"); + let collection = service.get_default_collection().expect("collection"); + collection + .create_item( + "NextSync — legacy test entry", + HashMap::from([ + (ATTR_SERVER, TEST_LEGACY_SERVER), + (ATTR_USERNAME, TEST_LEGACY_LOGIN), + ]), + TEST_PASSWORD.as_bytes(), + true, + CONTENT_TYPE, + ) + .expect("legacy item stored"); + + // No Rust-key item yet: the fallback must find the legacy secret. + let resolved = + CredentialsStore::get_for_account(TEST_ACCOUNT, TEST_LEGACY_SERVER, TEST_LEGACY_LOGIN) + .expect("get_for_account should succeed"); + assert_eq!(resolved.as_deref(), Some(TEST_PASSWORD)); + + // The legacy hit must have been adopted under `account_id`. + let adopted = CredentialsStore::get(TEST_ACCOUNT).expect("get should succeed"); + assert_eq!(adopted.as_deref(), Some(TEST_PASSWORD)); + + // The legacy item itself must survive for the Python app. + let still_there = service + .search_items(HashMap::from([ + (ATTR_SERVER, TEST_LEGACY_SERVER), + (ATTR_USERNAME, TEST_LEGACY_LOGIN), + ])) + .expect("legacy search"); + assert!( + !still_there.unlocked.is_empty() || !still_there.locked.is_empty(), + "legacy item must be left in place" + ); + + // Without any match (random account, unknown server) → Ok(None). + let missing = CredentialsStore::get_for_account( + "00003196e1aae95b1ef0cd15afaae5394c80144fcc6dd24f056d49bac25c6f3a", + "https://nonexistent.example.net", + "nobody@example.net", + ) + .expect("get_for_account should succeed"); + assert!(missing.is_none()); + } } diff --git a/src/nextcloud/sync_engine.rs b/src/nextcloud/sync_engine.rs index ea3d1e1..eb77d9d 100644 --- a/src/nextcloud/sync_engine.rs +++ b/src/nextcloud/sync_engine.rs @@ -73,7 +73,11 @@ pub struct KeyringCredentialSource; impl CredentialSource for KeyringCredentialSource { fn lookup(&self, account: &AccountConfig) -> CredentialLookup { - match CredentialsStore::get(&account.id) { + match CredentialsStore::get_for_account( + &account.id, + &account.server_url, + &account.login_name, + ) { Ok(Some(password)) => CredentialLookup::Found(password), Ok(None) => CredentialLookup::Missing, Err(secret_service::Error::Locked) => CredentialLookup::Locked, diff --git a/src/ui/settings.rs b/src/ui/settings.rs index 1c15147..2ecdc0e 100644 --- a/src/ui/settings.rs +++ b/src/ui/settings.rs @@ -1306,7 +1306,13 @@ fn populate_remote_picker( let list = list.clone(); let status = status.clone(); let handle = gio::spawn_blocking(move || -> RemoteFolderLookup { - let password = match CredentialsStore::get(&account_id) { + let server_for_lookup = server.clone(); + let username_for_lookup = username.clone(); + let password = match CredentialsStore::get_for_account( + &account_id, + &server_for_lookup, + &username_for_lookup, + ) { Ok(Some(password)) => Some(password), Ok(None) => None, Err(_) => None, diff --git a/src/ui/setup.rs b/src/ui/setup.rs index da7a614..ed863c1 100644 --- a/src/ui/setup.rs +++ b/src/ui/setup.rs @@ -1216,7 +1216,11 @@ fn start_syncing(ctx: &SetupContext) { let server_for_probe = server.clone(); let username_for_probe = username.clone(); let probe = gio::spawn_blocking(move || -> Result, ApiError> { - let password = match CredentialsStore::get(&account_id) { + let password = match CredentialsStore::get_for_account( + &account_id, + &server_for_probe, + &username_for_probe, + ) { Ok(Some(password)) => password, _ => return Ok(None), };