Upgrade accounts saved as http:// to https on launch

Before the previous commit, browser sign-in could store an account as
http://, and the client now refuses to send to one. Left alone, such a
library would fail to open with a configuration error, so its stored
endpoint is rewritten before anything reads it.

The endpoint is half of two keys, and both are handled:

- The keyring entry is filed under it. Rewriting only the record would
  strand the app password under the old key and sign the user out, so
  AccountStore::move_endpoint copies the secret across first, rewrites
  the record in place (the last record is the one resumed), and deletes
  the old entry only once nothing refers to it.
- namespace() is built from it and names the catalog directory. For an
  http to https rewrite it does not change, because the namespace strips
  either scheme. A move that would change it is refused, not performed,
  so no later rewrite can abandon a catalog either.

The rewrite is a new BackendProvider::upgrade_endpoint hook, which does
nothing by default, and not a second call to normalise_endpoint. The
folder connector's normalise_endpoint canonicalises the path and needs
it to exist, so running it on every launch would fail a library on an
unplugged disk, or rename one whose path now resolves differently. Only
Nextcloud implements the hook.

If the move fails (for example, a locked keyring), it is logged, the
account is left as it was, and the move is tried again on the next
launch.

Closes #65.

core/dr-sync/src/account.rs

@@ -29,7 +29,7 @@ use dr_plat::{SecretError, SecretRef, SecretStore};
 use dr_types::{Format, FormatFilter};
 use serde::{Deserialize, Serialize};
 
-use crate::RemoteError;
+use crate::{BackendRegistry, RemoteError};
 
 /// The connector every account had before there was a choice.
 ///
@@ -510,6 +510,95 @@ impl AccountStore {
         written
     }
 
+    /// TRACES: NFR-SEC-3
+    /// Rewrite the endpoints an older build stored in a form this one would
+    /// not, asking each account's connector
+    /// ([`upgrade_endpoint`](crate::BackendProvider::upgrade_endpoint)).
+    ///
+    /// Per account and best effort: one that cannot be moved — its keyring
+    /// locked, say — is logged and left as it was, and is tried again on the
+    /// next launch, rather than stopping the others or the launch. Returns
+    /// the accounts it rewrote.
+    pub fn upgrade_endpoints(&self, registry: &BackendRegistry) -> Vec<Account> {
+        let mut upgraded = Vec::new();
+        for account in self.list() {
+            let Ok(provider) = registry.for_account(&account) else {
+                continue;
+            };
+            let Some(endpoint) = provider.upgrade_endpoint(&account.endpoint) else {
+                continue;
+            };
+            if endpoint == account.endpoint {
+                continue;
+            }
+            match self.move_endpoint(&account, &endpoint) {
+                Ok(moved) => {
+                    log::info!("account {} moved to {endpoint}", account.describe());
+                    upgraded.push(moved);
+                }
+                Err(e) => log::warn!(
+                    "account {} could not be moved to {endpoint}: {e}",
+                    account.describe()
+                ),
+            }
+        }
+        upgraded
+    }
+
+    /// Move an account to a new endpoint, taking its credential with it.
+    ///
+    /// The endpoint is half of two keys, and both have to be dealt with. The
+    /// credential is filed under it ([`Account::secret_ref`]), so rewriting
+    /// the record alone would strand the app password under the old key and
+    /// sign the user out. And it feeds [`Account::namespace`], so a rewrite
+    /// that changed the namespace would abandon the catalog and everything
+    /// beside it; that is refused outright rather than left to the caller.
+    ///
+    /// Ordered so an interruption at any step leaves something that works:
+    /// the credential is copied before the record names the new key, and the
+    /// old copy is deleted only once nothing names the old one.
+    fn move_endpoint(&self, account: &Account, endpoint: &str) -> Result<Account, AccountError> {
+        let mut moved = account.clone();
+        moved.endpoint = endpoint.to_string();
+        if moved.namespace() != account.namespace() {
+            return Err(AccountError::WouldMoveData {
+                from: account.namespace(),
+                to: moved.namespace(),
+            });
+        }
+
+        let mut config = self.read_config();
+        // Already there — the user signed in again at the new address. That
+        // record and its credential are the newer, so the old one just goes.
+        let duplicate = config.sessions.iter().any(|a| a.is_same_as(&moved));
+
+        let old_ref = account.secret_ref();
+        let secret = match self.secrets.retrieve(&old_ref) {
+            Ok(s) => Some(s),
+            Err(SecretError::NotFound) => None,
+            Err(e) => return Err(e.into()),
+        };
+        if let (Some(s), false) = (&secret, duplicate) {
+            self.secrets.store(&moved.secret_ref(), s)?;
+        }
+
+        if duplicate {
+            config.sessions.retain(|a| !a.is_same_as(account));
+        } else {
+            // In place, not removed and pushed: the last record is the one
+            // the next launch resumes.
+            for a in config.sessions.iter_mut().filter(|a| a.is_same_as(account)) {
+                *a = moved.clone();
+            }
+        }
+        self.write_config(&config)?;
+
+        if secret.is_some() {
+            self.secrets.delete(&old_ref)?;
+        }
+        Ok(moved)
+    }
+
     fn read_config(&self) -> ConfigFile {
         std::fs::read_to_string(&self.config_path)
             .ok()
@@ -545,6 +634,11 @@ pub enum AccountError {
 
     #[error(transparent)]
     Remote(#[from] RemoteError),
+
+    /// A change that would give an account a different local data directory,
+    /// leaving its catalog and caches behind under the old one.
+    #[error("moving the account would leave its local data behind ({from} → {to})")]
+    WouldMoveData { from: String, to: String },
 }
 
 #[cfg(test)]
@@ -574,6 +668,134 @@ mod tests {
         d
     }
 
+    /// A connector that upgrades `http://` endpoints the way Nextcloud's does,
+    /// and every other one not at all.
+    struct Upgrading(&'static str);
+    impl crate::BackendProvider for Upgrading {
+        fn id(&self) -> &'static str {
+            self.0
+        }
+        fn display_name(&self) -> &'static str {
+            "U"
+        }
+        fn endpoint_label(&self) -> &'static str {
+            "Server"
+        }
+        fn endpoint_placeholder(&self) -> &'static str {
+            ""
+        }
+        fn sign_in(&self) -> crate::SignIn {
+            crate::SignIn::Browser
+        }
+        fn normalise_endpoint(&self, i: &str) -> Result<String, String> {
+            Ok(i.into())
+        }
+        fn upgrade_endpoint(&self, stored: &str) -> Option<String> {
+            stored
+                .strip_prefix("http://")
+                .map(|rest| format!("https://{rest}"))
+        }
+        fn connect(&self, _: &Connection) -> Result<Box<dyn crate::RemoteBackend>, RemoteError> {
+            Err(RemoteError::Unsupported("stub"))
+        }
+    }
+
+    fn upgrading(id: &'static str) -> BackendRegistry {
+        let mut r = BackendRegistry::new();
+        r.register(std::sync::Arc::new(Upgrading(id)));
+        r
+    }
+
+    /// TRACES: NFR-SEC-3
+    #[test]
+    fn an_http_account_is_upgraded_and_keeps_its_credential_and_data() {
+        let dir = tmpdir("upgrade");
+        let store = store_in(&dir);
+        let old =
+            Account::new(LEGACY_BACKEND, "http://cloud.example").with_login("duncan", "duncan");
+        store.save(&folder(), None).unwrap();
+        store
+            .save(&old, Some(&Secret::new("secret-token")))
+            .unwrap();
+
+        let moved = store.upgrade_endpoints(&upgrading(LEGACY_BACKEND));
+        assert_eq!(moved.len(), 1);
+
+        let now = store.current().expect("still the account resumed");
+        assert_eq!(now.endpoint, "https://cloud.example");
+        assert_eq!(
+            now.namespace(),
+            old.namespace(),
+            "the catalog directory must not move"
+        );
+        assert_eq!(
+            store
+                .connection(&now, true)
+                .unwrap()
+                .require_secret()
+                .unwrap()
+                .expose(),
+            "secret-token",
+            "the credential moves with the account"
+        );
+        assert!(
+            store.connection(&old, true).is_err(),
+            "nothing is left under the old key"
+        );
+        assert_eq!(store.list().len(), 2, "the folder library is untouched");
+
+        // And a second launch has nothing to do.
+        assert!(store
+            .upgrade_endpoints(&upgrading(LEGACY_BACKEND))
+            .is_empty());
+    }
+
+    /// TRACES: NFR-SEC-3
+    #[test]
+    fn a_move_that_would_change_the_data_directory_is_refused() {
+        // A scheme change keeps the namespace, which is what makes the upgrade
+        // safe; a host change does not, and would give the library a new,
+        // empty data directory. The account is left exactly as it was.
+        let dir = tmpdir("upgrade-refused");
+        let store = store_in(&dir);
+        let old = nextcloud();
+        store
+            .save(&old, Some(&Secret::new("secret-token")))
+            .unwrap();
+
+        assert!(matches!(
+            store.move_endpoint(&old, "https://elsewhere.example"),
+            Err(AccountError::WouldMoveData { .. })
+        ));
+        assert_eq!(store.current().unwrap().endpoint, old.endpoint);
+        assert!(store.connection(&old, true).is_ok());
+    }
+
+    /// TRACES: NFR-SEC-3
+    #[test]
+    fn an_upgrade_onto_an_existing_sign_in_keeps_the_newer_one() {
+        let dir = tmpdir("upgrade-duplicate");
+        let store = store_in(&dir);
+        let old =
+            Account::new(LEGACY_BACKEND, "http://cloud.example").with_login("duncan", "duncan");
+        let new =
+            Account::new(LEGACY_BACKEND, "https://cloud.example").with_login("duncan", "duncan");
+        store.save(&old, Some(&Secret::new("stale"))).unwrap();
+        store.save(&new, Some(&Secret::new("fresh"))).unwrap();
+
+        store.upgrade_endpoints(&upgrading(LEGACY_BACKEND));
+        assert_eq!(store.list().len(), 1);
+        assert_eq!(
+            store
+                .connection(&new, true)
+                .unwrap()
+                .require_secret()
+                .unwrap()
+                .expose(),
+            "fresh"
+        );
+    }
+
     #[test]
     fn a_saved_account_survives_reopening() {
         let dir = tmpdir("survives");

core/dr-sync/src/provider.rs

@@ -83,6 +83,20 @@ pub trait BackendProvider: Send + Sync {
     /// wrong rather than naming a type.
     fn normalise_endpoint(&self, input: &str) -> Result<String, String>;
 
+    /// The form a *stored* endpoint should take now, where an older build
+    /// wrote one this build would not.
+    ///
+    /// Not [`normalise_endpoint`](Self::normalise_endpoint) run again: that
+    /// judges what a person typed, and may touch the world to do it — a folder
+    /// is canonicalised and must exist — so rerunning it on every launch would
+    /// fail a library whose disk is unplugged, or rename one whose path now
+    /// resolves differently. This is a pure rewrite of the string, and `None`
+    /// means leave it alone, which is the answer for almost every connector.
+    fn upgrade_endpoint(&self, stored: &str) -> Option<String> {
+        let _ = stored;
+        None
+    }
+
     /// Build an account from a normalised endpoint alone.
     ///
     /// Only meaningful for [`SignIn::EndpointOnly`]; a browser flow produces