fix(linux): TLS file checks, connect errors and keyring wording

linux/ARCHITECTURE.md

@@ -212,7 +212,9 @@ A connection's `TlsConfig` holds the `TlsMode`, an optional CA file, an optional
 - `Require`: encrypted, and the certificate is not checked.
 - `VerifyFull`: the certificate must chain to a trusted root and carry the server name.
 
-The name checked is `NetworkTarget::tls_server_name()`: the override when one is set, otherwise the host the user typed, never the address of a local SSH forward. `TlsConfig::validate` refuses a mode, file or override the driver cannot use on that route, and the connect dialog reads the chosen files up front, so a file with no certificate is named before anything connects.
+A CA file goes only with `VerifyFull`, the one mode that checks the certificate. libpq and the mysql client verify against a CA file given with their require mode, so taking one with `Require` and ignoring it would leave a user who expects a check with none. `validate` refuses the pair, the connect dialog shows the CA row only for Verify full, and a saved or imported connection that holds the pair, from a build that allowed it, loads as `VerifyFull` with its CA file: a server the file does not vouch for then fails to connect instead of connecting unchecked.
+
+The name checked is `NetworkTarget::tls_server_name()`: the override when one is set, otherwise the host the user typed, never the address of a local SSH forward. `TlsConfig::validate` refuses a mode, file or override the driver cannot use on that route, and the connect dialog reads the chosen files up front, so a file with no certificate is named before anything connects. A CA, certificate or key file is read only when it is a regular file of at most 1 MiB, and drivers read it on the blocking pool, so a device or a pipe named in an imported connection can neither fill memory nor hold a connect.
 
 Each engine's TLS stack takes trust in its own form, so the driver builds it:
 
@@ -230,7 +232,7 @@ On Linux the system store is the distribution's CA bundle. Under Flatpak it is t
 
 #### Choosing a CA file under Flatpak
 
-- For a server whose certificate comes from a private CA, choose the CA's PEM file in the connection's TLS settings, whatever the host trusts.
+- For a server whose certificate comes from a private CA, set Encryption to Verify full and choose the CA's PEM file, whatever the host trusts.
 - The development manifest grants the home folder and the mount points of removable drives (`/media`, `/run/media` and `/mnt`), so a file under any of them is read at its real path.
 - A file chosen from anywhere else comes back from the file chooser portal as a path under `/run/user/<uid>/doc/`. It works for as long as the portal keeps the grant. If it goes, the connect fails naming the CA file, and choosing the file again fixes it. Keeping a copy of the CA in your home folder avoids this.
 - The file is read at every connect, so a replaced CA applies on the next connect.

linux/crates/app/src/services/secret_save_report.rs

@@ -3,21 +3,37 @@ use tablepro_core::credentials::SecretError;
 /// The toast for a secret the keyring would not keep. The connection
 /// still works; the user is asked for the secret next time.
 pub(crate) fn not_saved_message(error: &SecretError) -> String {
-    crate::i18n::gettext_f(
-        "Password not saved in the keyring: {reason}",
-        &[("reason", &error.to_string())],
-    )
+    match error {
+        SecretError::ServiceUnavailable { .. } => crate::i18n::gettext(
+            "No keyring is running, so the password was not saved. It will be asked for next time.",
+        ),
+        SecretError::PortalUnavailable { .. } => crate::i18n::gettext(
+            "The secret portal is unavailable, so the password was not saved. It will be asked for next time.",
+        ),
+        SecretError::Locked | SecretError::UnlockDismissed => crate::i18n::gettext(
+            "The keyring stayed locked, so the password was not saved. It will be asked for next time.",
+        ),
+        SecretError::Backend { detail } => {
+            crate::i18n::gettext_f("The keyring did not save the password: {reason}", &[("reason", detail)])
+        }
+        SecretError::InvalidEncoding => crate::i18n::gettext("The keyring did not save the password."),
+    }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
 
     #[test]
-    fn the_message_names_the_reason() {
-        let message = not_saved_message(&SecretError::UnlockDismissed);
+    fn the_message_says_what_happened_in_words_not_the_error_text() {
+        let locked = not_saved_message(&SecretError::UnlockDismissed);
+        let backend = not_saved_message(&SecretError::Backend {
+            detail: "org.freedesktop.Secret.Error.NoSuchObject".to_owned(),
+        });
 
-        assert!(message.contains("Password not saved in the keyring"), "{message}");
-        assert!(message.contains("dismissed"), "{message}");
+        assert!(locked.contains("password was not saved"), "{locked}");
+        assert!(!locked.contains(&SecretError::UnlockDismissed.to_string()), "{locked}");
+        assert!(backend.contains("NoSuchObject"), "{backend}");
+        assert!(!backend.contains("the secret service reported"), "{backend}");
     }
 }

linux/crates/app/src/ui/connect_dialog/connect_form.rs

@@ -52,9 +52,9 @@ impl TlsFormState {
         }
     }
 
-    /// The settings a request carries. A row the driver does not show
-    /// is left out rather than cleared, so looking at another driver
-    /// and coming back keeps what was chosen.
+    /// The settings a request carries. A row the driver or the mode does
+    /// not show is left out rather than cleared, so looking at another
+    /// driver or mode and coming back keeps what was chosen.
     pub fn config(&self, caps: &TlsCapabilities, route: TransportClass) -> TlsConfig {
         if self.mode == TlsMode::Disable {
             return TlsConfig::disabled();
@@ -67,7 +67,10 @@ impl TlsFormState {
         });
         TlsConfig {
             mode: self.mode,
-            ca_file: self.ca_file.clone().filter(|_| caps.ca_file),
+            ca_file: self
+                .ca_file
+                .clone()
+                .filter(|_| caps.ca_file && self.mode == TlsMode::VerifyFull),
             client_identity,
             server_name: Some(self.server_name.trim().to_owned())
                 .filter(|name| !name.is_empty() && shows_server_name(caps, route)),
@@ -96,6 +99,14 @@ pub fn shows_server_name(caps: &TlsCapabilities, route: TransportClass) -> bool
     }
 }
 
+/// Whether the dialog, opened on a connection to `driver` over `route`,
+/// shows the server name row. A driver without TLS shows none.
+pub fn driver_shows_server_name(driver: Option<&DriverCapabilities>, route: TransportClass) -> bool {
+    driver
+        .and_then(|caps| caps.tls.as_ref())
+        .is_some_and(|tls| shows_server_name(tls, route))
+}
+
 /// What the connect dialog holds, read from its rows in one go so a
 /// connect works from a snapshot.
 #[derive(Debug, Clone)]
@@ -385,10 +396,10 @@ impl ConnectForm {
         }
     }
 
-    /// The entry a connect from the New Connection dialog writes over
-    /// `existing`, the entry it describes again. An empty Name row there
-    /// means no name was typed, so a name the user gave the entry stays;
-    /// one that was only where it connects follows the form.
+    /// The entry a connect writes over `existing`, the entry the dialog
+    /// was reopened on. An empty Name row there means no name was typed,
+    /// so a name the user gave the entry stays; one that was only where
+    /// it connects follows the form.
     pub fn submitted_connection(
         &self,
         caps: &DriverCapabilities,
@@ -563,36 +574,6 @@ fn passphrase_left_unused(original: &SavedConnection, edited: &SavedSshAuth) ->
     }
 }
 
-/// The saved entry the form describes again, so connecting twice to
-/// the same database updates one entry instead of adding a second.
-pub fn match_saved_connection(
-    saved: &[SavedConnection],
-    candidate: &SavedConnection,
-    file_based: bool,
-) -> Option<Uuid> {
-    saved
-        .iter()
-        .find(|entry| same_connection(entry, candidate, file_based))
-        .map(|entry| entry.id)
-}
-
-fn same_connection(entry: &SavedConnection, candidate: &SavedConnection, file_based: bool) -> bool {
-    if entry.driver_id != candidate.driver_id || entry.database != candidate.database {
-        return false;
-    }
-    // A file is reached by its path alone. Comparing the credentials
-    // there would strand every entry an older build wrote with the
-    // hidden Username row's leftover text.
-    if file_based {
-        return true;
-    }
-    entry.host == candidate.host
-        && entry.port == candidate.port
-        && entry.username == candidate.username
-        && entry.auth_mode == candidate.auth_mode
-        && entry.ssh == candidate.ssh
-}
-
 fn ssh_field(error: &SshInputError) -> FormField {
     match error {
         SshInputError::HostRequired | SshInputError::HostInvalid(_) => FormField::SshHost,
@@ -677,25 +658,6 @@ mod tests {
         }
     }
 
-    fn saved(driver_id: &str, username: &str, auth_mode: AuthMode) -> SavedConnection {
-        SavedConnection {
-            id: Uuid::new_v4(),
-            name: "saved".into(),
-            driver_id: driver_id.into(),
-            host: "sql.corp.example".into(),
-            port: 1433,
-            database: "sales".into(),
-            username: username.into(),
-            tls: TlsConfig::disabled(),
-            read_only: false,
-            auth_mode,
-            ssh: None,
-            last_opened_at: None,
-            color: None,
-            group: None,
-        }
-    }
-
     #[test]
     fn verify_full_is_default() {
         for caps in [postgres(), mysql(), mssql()] {
@@ -841,6 +803,15 @@ mod tests {
         assert_eq!(form.username, "app");
     }
 
+    #[test]
+    fn the_server_name_row_shows_where_the_driver_can_check_another_name() {
+        assert!(!driver_shows_server_name(Some(mysql()), TransportClass::DirectTcp));
+        assert!(driver_shows_server_name(Some(mysql()), TransportClass::SshForward));
+        assert!(driver_shows_server_name(Some(postgres()), TransportClass::DirectTcp));
+        assert!(!driver_shows_server_name(Some(sqlite()), TransportClass::DirectTcp));
+        assert!(!driver_shows_server_name(None, TransportClass::SshForward));
+    }
+
     #[test]
     fn hidden_fields_are_omitted_from_request() {
         let mut form = filled("mssql", mssql());
@@ -931,53 +902,6 @@ mod tests {
         assert_eq!(entry.ssh.as_ref().map(|ssh| &ssh.auth), Some(&SavedSshAuth::Agent));
     }
 
-    #[test]
-    fn match_saved_connection_by_path_for_files() {
-        let mut legacy = saved("sqlite", "postgres", AuthMode::Password);
-        legacy.database = "/srv/app.db".into();
-        let form = ConnectForm {
-            file: Some(FileEndpoint::open_existing(PathBuf::from("/srv/app.db")).expect("absolute")),
-            ..ConnectForm::new("sqlite", sqlite())
-        };
-        let valid = form.validate(sqlite()).expect("valid");
-        let candidate = form.saved_connection(sqlite(), &valid, Uuid::new_v4(), None);
-
-        assert_eq!(
-            match_saved_connection(std::slice::from_ref(&legacy), &candidate, true),
-            Some(legacy.id)
-        );
-    }
-
-    #[test]
-    fn match_saved_connection_distinguishes_user_and_auth() {
-        let entry = saved("mssql", "sa", AuthMode::Password);
-        let entries = std::slice::from_ref(&entry);
-        let candidate = |username: &str, auth_mode: AuthMode| SavedConnection {
-            id: Uuid::new_v4(),
-            username: username.into(),
-            auth_mode,
-            ..entry.clone()
-        };
-
-        assert_eq!(
-            match_saved_connection(entries, &candidate("sa", AuthMode::Password), false),
-            Some(entry.id)
-        );
-        assert_eq!(
-            match_saved_connection(entries, &candidate("other", AuthMode::Password), false),
-            None
-        );
-        assert_eq!(
-            match_saved_connection(entries, &candidate("", AuthMode::Kerberos), false),
-            None
-        );
-        let finance = SavedConnection {
-            database: "finance".into(),
-            ..candidate("sa", AuthMode::Password)
-        };
-        assert_eq!(match_saved_connection(entries, &finance, false), None);
-    }
-
     fn saved_from(form: &ConnectForm, caps: &DriverCapabilities) -> SavedConnection {
         let valid = form.validate(caps).expect("valid");
         form.saved_connection(caps, &valid, Uuid::new_v4(), None)
@@ -1035,18 +959,13 @@ mod tests {
     }
 
     #[test]
-    fn a_connect_from_new_connection_keeps_the_name_of_a_renamed_match() {
+    fn a_reopened_entry_keeps_the_name_the_user_gave_it() {
         let form = filled("postgres", postgres());
         let valid = form.validate(postgres()).expect("valid");
         let renamed = SavedConnection {
             name: "Shop".to_owned(),
             ..saved_from(&form, postgres())
         };
-        let candidate = form.saved_connection(postgres(), &valid, Uuid::new_v4(), None);
-        assert_eq!(
-            match_saved_connection(std::slice::from_ref(&renamed), &candidate, false),
-            Some(renamed.id)
-        );
 
         let submitted = form.submitted_connection(postgres(), &valid, renamed.id, Some(&renamed));
         assert_eq!(submitted.name, "Shop", "the connect undid the rename");

linux/crates/app/src/ui/connect_dialog/mod.rs

@@ -36,8 +36,8 @@ use tablepro_storage::{CredentialDemand, SavedConnection};
 use uuid::Uuid;
 
 pub use self::connect_dialog_mode::ConnectDialogMode;
-pub use self::connect_form::ConnectForm;
-use self::connect_form::{ValidatedForm, match_saved_connection};
+use self::connect_form::ValidatedForm;
+pub use self::connect_form::{ConnectForm, driver_shows_server_name};
 pub use self::connect_submission::ConnectSubmission;
 pub use self::form_errors::FormErrors;
 pub use self::form_field::FormField;
@@ -364,6 +364,11 @@ impl ConnectDialog {
         }
     }
 
+    fn shows_server_name(&self) -> bool {
+        let entry = self.current_entry();
+        driver_shows_server_name(entry.as_ref().map(DriverEntry::capabilities), self.route())
+    }
+
     fn route(&self) -> TransportClass {
         if self.imp().ssh_section.is_enabled() {
             TransportClass::SshForward
@@ -580,16 +585,14 @@ impl ConnectDialog {
             .services()
             .map(|services| services.saved_connections.clone())
             .unwrap_or_else(|| Arc::from(Vec::new()));
-        let candidate = form.saved_connection(caps, &valid, Uuid::new_v4(), None);
+        // Only the entry the dialog was reopened on is written over. One
+        // that merely reaches the same database may differ in its name
+        // or its read-only switch, and New Connection must not undo them.
         let reopened = self.imp().prefill_id.get();
         let existing = saved_connections
             .iter()
-            .find(|saved| Some(saved.id) == reopened && saved.driver_id == entry.id)
-            .or_else(|| {
-                match_saved_connection(&saved_connections, &candidate, caps.is_file_based())
-                    .and_then(|id| saved_connections.iter().find(|saved| saved.id == id))
-            });
-        let id = existing.map_or(candidate.id, |saved| saved.id);
+            .find(|saved| Some(saved.id) == reopened && saved.driver_id == entry.id);
+        let id = existing.map_or_else(Uuid::new_v4, |saved| saved.id);
         let connection = form.submitted_connection(caps, &valid, id, existing);
         let request = connect_request(&entry, &form, &valid, &connection);
         let pending_save = PendingSave {
@@ -767,7 +770,7 @@ impl ConnectDialog {
                 Ok(Err(ConnectError::Cancelled)) => {}
                 Ok(Err(error)) => dialog.show_toast(
                     &crate::ui::error_text::connect_error_message(&error),
-                    crate::ui::error_text::connect_remedy(&error),
+                    crate::ui::error_text::connect_remedy(&error, dialog.shows_server_name()),
                 ),
                 Err(failure) => dialog.show_toast(&crate::ui::error_text::task_failure_message(&failure), None),
             }