Merge pull request #93 from narrrl/fix/78-reconnect-secret-service

fix(core): reconnect to the Secret Service after it restarts

crates/pdfs-core/src/auth.rs

@@ -123,13 +123,84 @@ fn client_config() -> ProtonClientConfiguration {
     ProtonClientConfiguration::new(app_version).with_user_agent(user_agent)
 }
 
-fn keyring_entry() -> Result<Entry> {
-    // Installed lazily rather than once at startup so a session bus that was not
-    // up yet on the first attempt is retried on the next one.
-    if keyring_core::get_default_store().is_none() {
-        keyring_core::set_default_store(dbus_secret_service_keyring_store::Store::new()?);
+/// Run `op` on the session's keyring entry.
+fn with_keyring<T>(op: impl Fn(&Entry) -> keyring_core::Result<T>) -> Result<T> {
+    with_keyring_entry(KEYRING_SERVICE, KEYRING_USER, op)
+}
+
+/// Run `op` on the entry `service`/`user`, reconnecting once if the Secret
+/// Service fails the call.
+///
+/// The store holds one D-Bus connection and one Secret Service session for the
+/// whole process. The session lives in the provider's memory: once the provider
+/// restarts (oo7's unit restarts it on failure, and both oo7 and gnome-keyring
+/// can be `--replace`d), every read and write on the old session fails with
+/// `NoSession`, for as long as this process runs. A daemon then keeps refreshed
+/// tokens only in memory, and a cold boot loses them (`docs/BUGS.md` B285). A
+/// platform failure therefore drops the store, and the call is made once more
+/// on a new one. A locked keyring, a dismissed prompt or a missing entry is
+/// not the connection's fault and comes back as it is.
+fn with_keyring_entry<T>(
+    service: &str,
+    user: &str,
+    op: impl Fn(&Entry) -> keyring_core::Result<T>,
+) -> Result<T> {
+    let store = keyring_store()?;
+    match op(&Entry::new(service, user)?) {
+        Err(keyring_core::Error::PlatformFailure(e)) => {
+            tracing::warn!(
+                error = %e,
+                "the Secret Service failed this process's connection; reconnecting"
+            );
+            drop_keyring_store(&store);
+            keyring_store()?;
+            Ok(op(&Entry::new(service, user)?)?)
+        }
+        result => Ok(result?),
+    }
+}
+
+/// Held while the default store is looked at and replaced, so that threads
+/// failing at the same time drop the broken store once and share its
+/// replacement.
+static KEYRING_STORE: parking_lot::Mutex<()> = parking_lot::Mutex::new(());
+
+/// The process's Secret Service store, connected now if there is none.
+///
+/// Connected lazily rather than once at startup so a session bus that was not
+/// up yet on the first attempt is tried again on the next one.
+fn keyring_store() -> Result<Arc<keyring_core::CredentialStore>> {
+    let _swap = KEYRING_STORE.lock();
+    if let Some(store) = keyring_core::get_default_store() {
+        return Ok(store);
+    }
+    let store = connect_keyring_store()?;
+    keyring_core::set_default_store(Arc::clone(&store));
+    Ok(store)
+}
+
+/// Forget `failed`, unless another thread has already replaced it.
+fn drop_keyring_store(failed: &Arc<keyring_core::CredentialStore>) {
+    let _swap = KEYRING_STORE.lock();
+    if keyring_core::get_default_store().is_some_and(|store| Arc::ptr_eq(&store, failed)) {
+        keyring_core::unset_default_store();
+    }
+}
+
+#[cfg(not(test))]
+fn connect_keyring_store() -> Result<Arc<keyring_core::CredentialStore>> {
+    Ok(dbus_secret_service_keyring_store::Store::new()?)
+}
+
+/// The tests' in-memory store when they set one, so a reconnect finds the
+/// same entries; the real Secret Service otherwise.
+#[cfg(test)]
+fn connect_keyring_store() -> Result<Arc<keyring_core::CredentialStore>> {
+    tests::KEYRING_CONNECTS.fetch_add(1, Ordering::SeqCst);
+    match tests::MOCK_STORE.lock().clone() {
+        Some(store) => Ok(store),
+        None => Ok(dbus_secret_service_keyring_store::Store::new()?),
     }
-    Ok(Entry::new(KEYRING_SERVICE, KEYRING_USER)?)
 }
 
 /// What the user answered when asked for the mailbox password.
@@ -421,17 +492,15 @@ fn classify_verification_gate(e: ProtonError) -> Error {
 
 fn write_stored(stored: &StoredSession) -> Result<()> {
     let json = Zeroizing::new(serde_json::to_string(stored)?);
-    keyring_entry()?.set_password(&json)?;
-    Ok(())
+    with_keyring(|entry| entry.set_password(&json))
 }
 
 /// Load the persisted session blob, or `Error::NotLoggedIn` if absent.
 pub fn load() -> Result<StoredSession> {
-    let entry = keyring_entry()?;
-    match entry.get_password() {
+    match with_keyring(|entry| entry.get_password()) {
         Ok(json) => Ok(serde_json::from_str(&json)?),
-        Err(keyring_core::Error::NoEntry) => Err(Error::NotLoggedIn),
-        Err(e) => Err(e.into()),
+        Err(Error::Keyring(keyring_core::Error::NoEntry)) => Err(Error::NotLoggedIn),
+        Err(e) => Err(e),
     }
 }
 
@@ -589,9 +658,9 @@ pub fn logout() -> Result<bool> {
         tracing::warn!(error = %e, "clearing the kernel-keyring unlock on logout failed");
     }
     let _ = kernelkey::UNSAVED_TOKENS.clear();
-    match keyring_entry()?.delete_credential() {
-        Ok(()) | Err(keyring_core::Error::NoEntry) => Ok(ended),
-        Err(e) => Err(e.into()),
+    match with_keyring(|entry| entry.delete_credential()) {
+        Ok(()) | Err(Error::Keyring(keyring_core::Error::NoEntry)) => Ok(ended),
+        Err(e) => Err(e),
     }
 }
 
@@ -1207,12 +1276,138 @@ mod tests {
     /// The keyring is one store per process, so the tests that use it,
     /// through `keyring-core`'s in-memory store, take turns.
     fn mock_keyring() -> parking_lot::MutexGuard<'static, ()> {
-        static KEYRING: parking_lot::Mutex<()> = parking_lot::Mutex::new(());
         let turn = KEYRING.lock();
-        keyring_core::set_default_store(keyring_core::mock::Store::new().unwrap());
+        let store: Arc<keyring_core::CredentialStore> = keyring_core::mock::Store::new().unwrap();
+        *MOCK_STORE.lock() = Some(Arc::clone(&store));
+        keyring_core::set_default_store(store);
         turn
     }
 
+    static KEYRING: parking_lot::Mutex<()> = parking_lot::Mutex::new(());
+
+    /// The in-memory store a reconnect hands out, so it finds the same
+    /// entries; `None` connects to the real Secret Service.
+    pub(super) static MOCK_STORE: parking_lot::Mutex<Option<Arc<keyring_core::CredentialStore>>> =
+        parking_lot::Mutex::new(None);
+
+    /// How many times a store was connected.
+    pub(super) static KEYRING_CONNECTS: std::sync::atomic::AtomicUsize =
+        std::sync::atomic::AtomicUsize::new(0);
+
+    /// The session's entry, to make its next call fail.
+    fn keyring_entry() -> Entry {
+        Entry::new(KEYRING_SERVICE, KEYRING_USER).unwrap()
+    }
+
+    /// Make the session entry's next call fail with `error`.
+    fn fail_next(error: keyring_core::Error) {
+        keyring_entry()
+            .as_any()
+            .downcast_ref::<keyring_core::mock::Cred>()
+            .unwrap()
+            .set_error(error);
+    }
+
+    fn no_session() -> keyring_core::Error {
+        keyring_core::Error::PlatformFailure(
+            std::io::Error::other("The session does not exist (NoSession)").into(),
+        )
+    }
+
+    /// B285: a restarted Secret Service no longer knows the process's
+    /// session, and every call on it failed until the process restarted.
+    #[test]
+    fn a_dropped_secret_service_session_is_reconnected_and_retried() {
+        let _turn = mock_keyring();
+        write_stored(&stored_with("r")).unwrap();
+        let connects = KEYRING_CONNECTS.load(Ordering::SeqCst);
+
+        fail_next(no_session());
+        assert_eq!(load().unwrap().refresh_token, "r");
+        assert_eq!(KEYRING_CONNECTS.load(Ordering::SeqCst), connects + 1);
+
+        fail_next(no_session());
+        write_stored(&stored_with("r2")).unwrap();
+        assert_eq!(load().unwrap().refresh_token, "r2");
+        assert_eq!(KEYRING_CONNECTS.load(Ordering::SeqCst), connects + 2);
+    }
+
+    #[test]
+    fn a_rotation_lands_at_once_after_the_keyring_restarts() {
+        let _turn = mock_keyring();
+        write_stored(&stored_with("r")).unwrap();
+        let sink = Arc::new(TokenSink::new("s", "r".into()));
+
+        fail_next(no_session());
+        sink.rotated(tokens("a2", "r2")).unwrap();
+        assert_eq!(load().unwrap().refresh_token, "r2");
+        assert!(sink.state.lock().unsaved.is_none());
+    }
+
+    /// A locked keyring or a dismissed prompt is no fault of the connection.
+    #[test]
+    fn a_locked_keyring_is_not_reconnected() {
+        let _turn = mock_keyring();
+        write_stored(&stored_with("r")).unwrap();
+        let connects = KEYRING_CONNECTS.load(Ordering::SeqCst);
+
+        fail_next(keyring_core::Error::NoStorageAccess(
+            std::io::Error::other("locked").into(),
+        ));
+        assert!(matches!(
+            load(),
+            Err(Error::Keyring(keyring_core::Error::NoStorageAccess(_)))
+        ));
+        assert_eq!(KEYRING_CONNECTS.load(Ordering::SeqCst), connects);
+        assert_eq!(load().unwrap().refresh_token, "r");
+    }
+
+    /// The real Secret Service, restarted between two calls on one store.
+    ///
+    /// Run inside a throwaway session bus, never against the desktop's
+    /// keyring. With gnome-keyring:
+    ///
+    /// ```sh
+    /// echo -n pw | gnome-keyring-daemon --unlock --components=secrets
+    /// PDFS_SECRET_SERVICE_RESTART_TEST=1 \
+    /// PDFS_RESTART_PROVIDER='echo -n pw | gnome-keyring-daemon --replace --unlock --components=secrets' \
+    /// cargo test -p pdfs-core real_secret_service -- --ignored
+    /// ```
+    #[test]
+    #[ignore = "needs a throwaway Secret Service; see the doc comment"]
+    fn real_secret_service_survives_a_provider_restart() {
+        if std::env::var_os("PDFS_SECRET_SERVICE_RESTART_TEST").is_none() {
+            return;
+        }
+        let restart = std::env::var("PDFS_RESTART_PROVIDER").expect("PDFS_RESTART_PROVIDER");
+        let _turn = KEYRING.lock();
+        *MOCK_STORE.lock() = None;
+        keyring_core::unset_default_store();
+        let (service, user) = ("pdfs-test", "restart");
+
+        with_keyring_entry(service, user, |e| e.set_password("before")).unwrap();
+        let status = std::process::Command::new("sh")
+            .args(["-c", &restart])
+            .status()
+            .unwrap();
+        assert!(status.success(), "restarting the provider failed");
+        std::thread::sleep(Duration::from_secs(1));
+
+        let connects = KEYRING_CONNECTS.load(Ordering::SeqCst);
+        let read = with_keyring_entry(service, user, |e| e.get_password()).unwrap();
+        assert_eq!(read, "before");
+        assert_eq!(
+            KEYRING_CONNECTS.load(Ordering::SeqCst),
+            connects + 1,
+            "the provider kept the old session, so nothing was tested"
+        );
+        with_keyring_entry(service, user, |e| e.set_password("after")).unwrap();
+        let read = with_keyring_entry(service, user, |e| e.get_password()).unwrap();
+        assert_eq!(read, "after");
+        with_keyring_entry(service, user, |e| e.delete_credential()).unwrap();
+        keyring_core::unset_default_store();
+    }
+
     fn stored_with(refresh: &str) -> StoredSession {
         serde_json::from_str(&blob("").replace(r#""r""#, &format!("{refresh:?}"))).unwrap()
     }
@@ -1267,12 +1462,7 @@ mod tests {
         write_stored(&stored_with("r")).unwrap();
         let sink = Arc::new(TokenSink::new("s", "r".into()));
 
-        let entry = keyring_entry().unwrap();
-        let cred = entry
-            .as_any()
-            .downcast_ref::<keyring_core::mock::Cred>()
-            .unwrap();
-        cred.set_error(keyring_core::Error::NoStorageAccess(
+        fail_next(keyring_core::Error::NoStorageAccess(
             std::io::Error::other("locked").into(),
         ));
         assert!(sink.rotated(tokens("a2", "r2")).is_err());
@@ -1307,12 +1497,7 @@ mod tests {
         let _turn = mock_keyring();
         write_stored(&stored_with("r")).unwrap();
         let sink = Arc::new(TokenSink::new("s", "r".into()));
-        let entry = keyring_entry().unwrap();
-        entry
-            .as_any()
-            .downcast_ref::<keyring_core::mock::Cred>()
-            .unwrap()
-            .set_error(keyring_core::Error::NoEntry);
+        fail_next(keyring_core::Error::NoEntry);
 
         assert!(sink.rotated(tokens("a2", "r2")).is_err());
         assert!(sink.state.lock().unsaved.is_some());

docs/BUGS.md

@@ -109,6 +109,7 @@ a live account since.
 
 | Entry | Title |
 |---|---|
+| [B285](#b285--a-restarted-secret-service-leaves-the-process-on-a-dead-session) | A restarted Secret Service leaves the process on a dead session |
 | [B284](#b284--a-keyring-entry-reported-missing-discards-refreshed-session-tokens) | A keyring entry reported missing discards refreshed session tokens |
 | [B283](#b283--videos-have-no-thumbnail-outside-photos-and-a-ts-recording-looks-like-source-code) | Videos have no thumbnail outside Photos, and a `.ts` recording looks like source code |
 | [B282](#b282--a-file-kept-offline-is-downloaded-again-after-a-change-made-elsewhere) | A file kept offline is downloaded again after a change made elsewhere |
@@ -387,6 +388,41 @@ a live account since.
 
 Newest first.
 
+### B285 — A restarted Secret Service leaves the process on a dead session
+
+**Status:** Fixed (unverified)
+**Found:** 2026-11-14, following up [issue #78](https://github.com/narrrl/proton-drive-linux/issues/78)
+(signed out after every cold boot on Fedora 45, which replaced gnome-keyring with oo7). See
+[the investigation](ISSUE-78.md#a-restarted-secret-service).
+**Where:** `crates/pdfs-core/src/auth.rs`, `with_keyring_entry`
+
+**Repro:** Write the session blob, restart the Secret Service provider
+(`gnome-keyring-daemon --replace`, or `oo7-daemon --replace`), then read or write it again from
+the same process. `dbus-secret-service-keyring-store` keeps one D-Bus connection and one Secret
+Service session for the life of the process, and pdfs installed it once. The session exists only
+in the provider's memory, so the new provider answers every `GetSecret` and `SetSecret` with
+`org.freedesktop.Secret.Error.NoSession`, for as long as the process runs. A fresh store in the
+same process reads the same entry fine. oo7's systemd unit restarts the daemon on failure, and
+both providers can be replaced, so the daemon could meet this mid-session. A token refresh after
+that point reached only memory and the kernel keyring: B61's retry kept asking on the dead
+session, and the next cold boot read the spent refresh token (`InvalidRefreshToken`).
+
+**Fix:** Every keyring call goes through `with_keyring_entry`. A platform failure (which is what
+`NoSession` and other D-Bus errors become) logs "the Secret Service failed this process's
+connection; reconnecting", drops the store unless another thread already replaced it, and makes
+the call once more on a new connection and session. A locked keyring, a dismissed prompt and a
+missing entry come back unchanged. Affects the daemon, the app and the tray alike.
+
+**Verified:** `auth::tests::a_dropped_secret_service_session_is_reconnected_and_retried`,
+`a_rotation_lands_at_once_after_the_keyring_restarts` and `a_locked_keyring_is_not_reconnected`
+on the in-memory store. The ignored `auth::tests::real_secret_service_survives_a_provider_restart`
+passes against gnome-keyring 51.1 and oo7 0.7.1 (built from its tag), each in a throwaway session
+bus and restarted with `--replace`; it checks that a reconnect actually happened. A standalone
+program holding one store, as before the fix, gets `NoSession` on read and write after the same
+restart under both providers. Whether the reporter's oo7 restarts during a session is not known.
+
+---
+
 ### B284 — A keyring entry reported missing discards refreshed session tokens
 
 **Status:** Fixed (unverified)

docs/CHANGELOG.md

@@ -124,6 +124,14 @@ Existing folders stay device backups. Older builds cannot open the upgraded data
   screen there, and with nothing to scroll no more were loaded. More now load whenever the photos
   don't fill the window, also after a zoom or a resize.
 
+- **Signed out after a cold boot on Fedora 45** (issue #78, B284, B285). Fedora 45 replaced the
+  GNOME keyring with oo7, and two things there could keep a refreshed sign-in from being saved:
+  oo7 reports the entries of a locked keyring as missing, which the client took for a sign-out,
+  and a restarted oo7 no longer knew the client's connection, which then failed every save until
+  the service restarted. The refreshed sign-in lived only in memory, and the next boot found the
+  spent one. The client now keeps such a sign-in and saves it once the keyring can be read, and
+  reconnects to a keyring service that restarted.
+
 - **Thumbnails appear in the folder you are looking at as soon as they are made.** During
   **Build thumbnails**, the folder on screen kept its file icons until you left it and opened it
   again. Its tiles now fill in while the build runs, and the same goes for any tile whose

docs/ISSUE-78.md

@@ -58,6 +58,44 @@ can resolve the default alias to a locked collection. Both have fixes in
 after the reporter's November 7 retest. Neither can be assigned as this report's cause without
 matching runtime evidence.
 
+## A restarted Secret Service
+
+A second path to the same symptom, independent of locking (B285). The keyring store pdfs used
+holds one D-Bus connection and one Secret Service session for the whole process, and was
+installed once. The session lives only in the provider's memory. When the provider restarts,
+the new one answers every `GetSecret` and `SetSecret` on the old session with
+`org.freedesktop.Secret.Error.NoSession`, until the pdfs process itself restarts. Reproduced
+with both gnome-keyring 51.1 and oo7 0.7.1 using `--replace`.
+
+gnome-keyring practically never restarts within a desktop session. oo7 can:
+`oo7-daemon.service` has `Restart=on-failure`, the daemon allows itself to be replaced, and it
+exits on a wrong credential password and is D-Bus-activated again. A daemon that met a restarted
+provider kept every later token refresh only in memory and the kernel keyring, and a cold boot
+then left the spent refresh token in the saved blob.
+
+Now a platform failure drops the store and the call is retried once on a new connection. The
+daemon log shows `the Secret Service failed this process's connection; reconnecting` when that
+happens. To check whether a machine meets this, look for oo7 restarts in the same boot:
+
+```sh
+journalctl --user -b -1 -u oo7-daemon.service | grep -E 'Started|Stopped|Starting oo7|Main process exited'
+journalctl --user -b -1 -u proton-drive.service | grep -iE 'keyring|reconnecting|refresh'
+```
+
+Reproduce in a throwaway session bus (never against the desktop's keyring; the runtime directory
+must be short, as it holds a Unix socket):
+
+```sh
+T=$(mktemp -d /tmp/ss.XXXX); mkdir -p $T/data $T/run; chmod 700 $T/run
+XDG_DATA_HOME=$T/data XDG_RUNTIME_DIR=$T/run PDFS_SECRET_SERVICE_RESTART_TEST=1 \
+  PDFS_RESTART_PROVIDER='echo -n pw | gnome-keyring-daemon --replace --unlock --components=secrets >/dev/null' \
+  dbus-run-session -- sh -c 'echo -n pw | gnome-keyring-daemon --unlock --components=secrets >/dev/null; sleep 1;
+    cargo test -p pdfs-core real_secret_service -- --ignored'
+```
+
+For oo7, start `oo7-daemon --login` with the password on stdin instead, and restart it with
+`--login --replace` in the background.
+
 ## Reproduce without a Proton account or desktop keyring
 
 The oo7 fixture uses fake tokens, a temporary disk keyring and a private peer-to-peer D-Bus