chore(gui-client): bump deps so we can get to zbus 4.x (#5957)

Yak shave for #5846

rust/gui-client/src-tauri/Cargo.toml

@@ -24,7 +24,6 @@ firezone-headless-client = { path = "../../headless-client" }
 futures = { version = "0.3", default-features = false }
 git-version = "0.3.9"
 hex = "0.4.3"
-keyring = "2.3.3"
 minidumper = "0.8.2"
 native-dialog = "0.7.0"
 output_vt100 = "0.1"
@@ -36,7 +35,7 @@ semver = { version = "1.0.22", features = ["serde"] }
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"
 subtle = "2.5.0"
-tauri = { version = "1.6.7", features = [ "dialog", "icon-png", "notification", "shell-open-api", "system-tray" ] }
+tauri = { version = "1.7.1", features = [ "dialog", "icon-png", "notification", "shell-open-api", "system-tray" ] }
 tauri-runtime = "0.14.2"
 tauri-utils = "1.6.0"
 thiserror = { version = "1.0", default-features = false }
@@ -51,6 +50,14 @@ url = { version = "2.5.0", features = ["serde"] }
 uuid = { version = "1.7.0", features = ["v4"] }
 zip = { version = "2", features = ["deflate", "time"], default-features = false }
 
+[dependencies.keyring]
+version = "3.0.3"
+features = [
+    "crypto-rust", # Don't rely on OpenSSL
+    "sync-secret-service", # Can't use Tokio because of <https://github.com/hwchen/keyring-rs/issues/132>
+    "windows-native", # Yes, really, we must actually explicitly ask for every platform. Otherwise it defaults to an in-memory mock store. Really. That's really how `keyring` 3.x is designed.
+]
+
 [target.'cfg(target_os = "linux")'.dependencies]
 dirs = "5.0.1"
 nix = { version = "0.28.0", features = ["user"] }

rust/gui-client/src-tauri/src/client/auth.rs

@@ -122,7 +122,7 @@ impl Auth {
     ///
     /// Performs I/O.
     pub fn sign_out(&mut self) -> Result<(), Error> {
-        if let Err(error) = self.token_store.delete_password() {
+        if let Err(error) = self.token_store.delete_credential() {
             tracing::warn!(?error, "Couldn't delete token while signing out");
         }
         if let Err(error) = std::fs::remove_file(actor_name_path()?) {
@@ -259,39 +259,58 @@ fn secure_equality(a: &SecretString, b: &SecretString) -> bool {
     a.ct_eq(b).into()
 }
 
-// The Linux CI is headless so it's hard to test keyrings in it
-// There is a trick, but it requires some setup outside of `cargo test`:
-// <https://github.com/hwchen/keyring-rs/blob/master/linux-test.sh>
-#[cfg(not(target_os = "linux"))]
 #[cfg(test)]
 mod tests {
     use super::*;
 
+    #[cfg(not(target_os = "linux"))]
     fn bogus_secret(x: &str) -> SecretString {
         SecretString::new(x.into())
     }
 
     #[test]
-    fn actor_name_path() {
-        assert!(super::actor_name_path()
+    fn actor_name() {
+        assert!(actor_name_path()
             .expect("`actor_name_path` should return Ok")
             .components()
             .any(|x| x == std::path::Component::Normal("dev.firezone.client".as_ref())));
     }
 
+    #[test]
+    #[cfg(target_os = "linux")]
+    fn keyring_is_persistent() {
+        assert!(matches!(
+            keyring::secret_service::default_credential_builder().persistence(),
+            keyring::credential::CredentialPersistence::UntilDelete
+        ));
+    }
+
+    #[test]
+    #[cfg(target_os = "windows")]
+    fn keyring_is_persistent() {
+        assert!(matches!(
+            keyring::windows::default_credential_builder().persistence(),
+            keyring::credential::CredentialPersistence::UntilDelete
+        ));
+    }
+
     /// Runs everything in one test so that `cargo test` can't multi-thread it
     /// This should work around a bug we had <https://github.com/firezone/firezone/issues/3256>
     #[test]
+    // The Linux CI is headless so it's hard to test keyrings in it
+    #[cfg(not(target_os = "linux"))]
     fn everything() {
         // Run `happy_path` first to make sure it reacts okay if our `data` dir is missing
         // TODO: Re-enable happy path tests once `keyring-rs` is working in CI tests
-        // happy_path("");
-        // happy_path("Jane Doe");
+        happy_path("");
+        happy_path("Jane Doe");
         utils();
         no_inflight_request();
         states_dont_match();
     }
 
+    // The Linux CI is headless so it's hard to test keyrings in it
+    #[cfg(not(target_os = "linux"))]
     #[test]
     fn keyring_rs() {
         // We used this test to find that `service` is not used on Windows - We have to namespace on our own.
@@ -299,25 +318,34 @@ mod tests {
         let name_1 = "dev.firezone.client/test_1/token";
         let name_2 = "dev.firezone.client/test_2/token";
 
-        // Accessing the same keys from different `Entry` instances doesn't work well,
-        // even within the same thread
+        let test_password_1 = "test_password_1";
+        let test_password_2 = "test_password_2";
+
         let entry = keyring::Entry::new_with_target(name_1, "", "").unwrap();
         entry.set_password("test_password_1").unwrap();
 
         {
             // In the middle of accessing one token, access another to make sure they don't interfere much
             let entry = keyring::Entry::new_with_target(name_2, "", "").unwrap();
-            entry.set_password("test_password_2").unwrap();
-            assert_eq!(entry.get_password().unwrap(), "test_password_2");
-            entry.delete_password().unwrap();
+            entry.set_password(test_password_2).unwrap();
+            assert_eq!(entry.get_password().unwrap(), test_password_2);
+        }
+
+        {
+            // Make sure that closing and re-opening the `Entry` on the same thread
+            // gives the correct result
+            let entry = keyring::Entry::new_with_target(name_2, "", "").unwrap();
+            assert_eq!(entry.get_password().unwrap(), test_password_2);
+            entry.delete_credential().unwrap();
             assert!(entry.get_password().is_err());
         }
 
-        assert_eq!(entry.get_password().unwrap(), "test_password_1");
-        entry.delete_password().unwrap();
+        assert_eq!(entry.get_password().unwrap(), test_password_1);
+        entry.delete_credential().unwrap();
         assert!(entry.get_password().is_err());
     }
 
+    #[cfg(not(target_os = "linux"))]
     fn utils() {
         // This doesn't test for constant-time properties, it just makes sure the function
         // gives the right result
@@ -342,8 +370,8 @@ mod tests {
         );
     }
 
-    // TODO: Re-enable
-    fn _happy_path(actor_name: &str) {
+    #[cfg(not(target_os = "linux"))]
+    fn happy_path(actor_name: &str) {
         // Key for credential manager. This is not what we use in production
         let key = "dev.firezone.client/test_DMRCZ67A_happy_path/token";
 
@@ -392,6 +420,7 @@ mod tests {
         }
     }
 
+    #[cfg(not(target_os = "linux"))]
     fn no_inflight_request() {
         // Start the program
         let mut state =
@@ -417,6 +446,7 @@ mod tests {
         state.sign_out().unwrap();
     }
 
+    #[cfg(not(target_os = "linux"))]
     fn states_dont_match() {
         // Start the program
         let mut state =

rust/gui-client/src-tauri/src/client/debug_commands.rs

@@ -6,6 +6,11 @@ use anyhow::Result;
 #[derive(clap::Subcommand)]
 pub(crate) enum Cmd {
     SetAutostart(SetAutostartArgs),
+
+    // Store and check a bogus debug token to make sure `keyring-rs`
+    // is behaving.
+    CheckToken(CheckTokenArgs),
+    StoreToken(StoreTokenArgs),
 }
 
 #[derive(clap::Parser)]
@@ -14,9 +19,33 @@ pub(crate) struct SetAutostartArgs {
     enabled: bool,
 }
 
+#[derive(clap::Parser)]
+pub(crate) struct CheckTokenArgs {
+    token: String,
+}
+
+#[derive(clap::Parser)]
+pub(crate) struct StoreTokenArgs {
+    token: String,
+}
+
+const CRED_NAME: &str = "dev.firezone.client/test_BYKPFT6P/token";
+
 pub fn run(cmd: Cmd) -> Result<()> {
     match cmd {
         Cmd::SetAutostart(SetAutostartArgs { enabled }) => set_autostart(enabled),
+
+        Cmd::CheckToken(CheckTokenArgs { token: expected }) => {
+            assert_eq!(
+                keyring::Entry::new_with_target(CRED_NAME, "", "")?.get_password()?,
+                expected
+            );
+            Ok(())
+        }
+        Cmd::StoreToken(StoreTokenArgs { token }) => {
+            keyring::Entry::new_with_target(CRED_NAME, "", "")?.set_password(&token)?;
+            Ok(())
+        }
     }
 }