refactor(GUI clients): extract known_dirs module (#3734)

The CI tests aren't running for Linux just yet.
This organizes the well-known directories used on Linux and Windows for
logs, config, etc., and adds them to the (unused) Linux smoke test

Waiting on #3727

---------

Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>

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

@@ -10,6 +10,7 @@ mod deep_link;
 mod device_id;
 mod elevation;
 mod gui;
+mod known_dirs;
 mod logging;
 mod network_changes;
 mod resolvers;

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

@@ -1,6 +1,7 @@
 //! Fulfills <https://github.com/firezone/firezone/issues/2823>
 
-use connlib_shared::{control::SecureUrl, windows::app_local_data_dir};
+use crate::client::known_dirs;
+use connlib_shared::control::SecureUrl;
 use rand::{thread_rng, RngCore};
 use secrecy::{ExposeSecret, Secret, SecretString};
 use std::path::PathBuf;
@@ -25,8 +26,8 @@ const NONCE_LENGTH: usize = 32;
 pub(crate) enum Error {
     #[error("`actor_name_path` has no parent, this should be impossible")]
     ActorNamePathWrong,
-    #[error("`app_local_data_dir` failed")]
-    CantFindLocalAppDataFolder,
+    #[error("`known_dirs` failed")]
+    CantFindKnownDir,
     #[error("`create_dir_all` failed while writing `actor_name_path`")]
     CreateDirAll(std::io::Error),
     #[error("Couldn't delete actor_name from disk: {0}")]
@@ -233,11 +234,8 @@ impl Auth {
 ///
 /// Hopefully we don't need to save anything else, or there will be a migration step
 fn actor_name_path() -> Result<PathBuf> {
-    // Would it be better to break out another error type for `app_local_data_dir` here?
-    // It can only return one kind of error, unrelated to the other errors connlib can return
-    Ok(app_local_data_dir()
-        .map_err(|_| Error::CantFindLocalAppDataFolder)?
-        .join("data")
+    Ok(known_dirs::session()
+        .ok_or(Error::CantFindKnownDir)?
         .join("actor_name.txt"))
 }
 

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

@@ -13,8 +13,8 @@
 //! - Compile `minidump-stackwalk` with PR 891 merged
 //! - `minidump-stackwalker --symbols-path firezone.syms crash.dmp`
 
+use crate::client::known_dirs;
 use anyhow::{anyhow, bail, Context, Result};
-use connlib_shared::windows::app_local_data_dir;
 use crash_handler::CrashHandler;
 use std::{fs::File, io::Write, path::PathBuf};
 
@@ -66,9 +66,8 @@ fn start_server_and_connect() -> Result<(minidumper::Client, std::process::Child
     let exe = std::env::current_exe().context("unable to find our own exe path")?;
     // Path of a Unix domain socket for IPC with the crash handler server
     // <https://github.com/EmbarkStudios/crash-handling/issues/10>
-    let socket_path = app_local_data_dir()
-        .context("couldn't compute crash handler socket path")?
-        .join("data")
+    let socket_path = known_dirs::runtime()
+        .context("`known_dirs::runtime` failed")?
         .join("crash_handler_pipe");
 
     let mut server = None;
@@ -115,10 +114,8 @@ impl minidumper::ServerHandler for Handler {
     /// Called when a crash has been received and a backing file needs to be
     /// created to store it.
     fn create_minidump_file(&self) -> Result<(File, PathBuf), std::io::Error> {
-        let dump_path = app_local_data_dir()
-            .expect("app_local_data_dir() failed")
-            .join("data")
-            .join("logs")
+        let dump_path = known_dirs::logs()
+            .expect("Should be able to find logs dir to put crash dump in")
             .join("last_crash.dmp");
 
         if let Some(dir) = dump_path.parent() {

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

@@ -1,4 +1,3 @@
-use known_folders::{get_known_folder_path, KnownFolder};
 use tokio::fs;
 
 #[derive(thiserror::Error, Debug)]
@@ -16,18 +15,11 @@ pub(crate) enum Error {
 /// Per <https://github.com/firezone/firezone/issues/2697> and <https://github.com/firezone/firezone/issues/2711>,
 /// clients must generate their own random IDs and persist them to disk, to handle situations like VMs where a hardware ID is not unique or not available.
 ///
-/// # Arguments
-///
-/// * `identifier` - Our Tauri bundle identifier, e.g. "dev.firezone.client"
-///
 /// Returns: The UUID as a String, suitable for sending verbatim to `connlib_client_shared::Session::connect`.
 ///
 /// Errors: If the disk is unwritable when initially generating the ID, or unwritable when re-generating an invalid ID.
-pub(crate) async fn device_id(identifier: &str) -> Result<String, Error> {
-    let dir = get_known_folder_path(KnownFolder::ProgramData)
-        .ok_or(Error::KnownFolder)?
-        .join(identifier)
-        .join("config");
+pub(crate) async fn device_id() -> Result<String, Error> {
+    let dir = crate::client::known_dirs::device_id().ok_or(Error::KnownFolder)?;
     let path = dir.join("device_id.json");
 
     // Try to read it back from disk

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

@@ -4,7 +4,7 @@
 // TODO: `git grep` for unwraps before 1.0, especially this gui module <https://github.com/firezone/firezone/issues/3521>
 
 use crate::client::{
-    self, about, deep_link, logging, network_changes,
+    self, about, deep_link, known_dirs, logging, network_changes,
     settings::{self, AdvancedSettings},
     Failure,
 };
@@ -301,9 +301,8 @@ async fn smoke_test(ctlr_tx: CtlrTx) -> Result<()> {
     settings::apply_advanced_settings_inner(&settings::AdvancedSettings::default()).await?;
 
     // Test log exporting
-    let path = connlib_shared::windows::app_local_data_dir()
-        .context("`app_local_data_dir` failed")?
-        .join("data")
+    let path = known_dirs::session()
+        .context("`known_dirs::session` failed during smoke test")?
         .join("smoke_test_log_export.zip");
     let stem = "connlib-smoke-test".into();
     match tokio::fs::remove_file(&path).await {
@@ -742,7 +741,7 @@ async fn run_controller(
     advanced_settings: AdvancedSettings,
     notify_controller: Arc<Notify>,
 ) -> Result<()> {
-    let device_id = client::device_id::device_id(&app.config().tauri.bundle.identifier)
+    let device_id = client::device_id::device_id()
         .await
         .context("Failed to read / create device ID")?;
 

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

@@ -0,0 +1,120 @@
+//! An abstraction over well-known dirs like AppData/Local on Windows and $HOME/.config on Linux
+//!
+//! On Linux it uses `dirs` which is a convenience wrapper for getting XDG environment vars
+//!
+//! On Windows it uses `known_folders` which calls into Windows for forwards-compatibility
+//! We can't use `dirs` on Windows because we need to match connlib for when it opens wintun.dll.
+//!
+//! I wanted the ProgramData folder on Windows, which `dirs` alone doesn't provide.
+
+pub(crate) use imp::{device_id, logs, runtime, session, settings};
+
+#[cfg(target_os = "linux")]
+mod imp {
+    use connlib_shared::BUNDLE_ID;
+    use std::path::PathBuf;
+
+    /// e.g. `/home/alice/.config/dev.firezone.client/config`
+    ///
+    /// Device ID is stored here until <https://github.com/firezone/firezone/issues/3713> lands
+    ///
+    /// Linux has no direct equivalent to Window's `ProgramData` dir, `/var` doesn't seem
+    /// to be writable by normal users.
+    pub(crate) fn device_id() -> Option<PathBuf> {
+        Some(dirs::config_local_dir()?.join(BUNDLE_ID).join("config"))
+    }
+
+    /// e.g. `/home/alice/.cache/dev.firezone.client/data/logs`
+    ///
+    /// Logs are considered cache because they're not configs and it's technically okay
+    /// if the system / user deletes them to free up space
+    pub(crate) fn logs() -> Option<PathBuf> {
+        Some(dirs::cache_dir()?.join(BUNDLE_ID).join("data").join("logs"))
+    }
+
+    /// e.g. `/run/user/1000/dev.firezone.client/data`
+    ///
+    /// Crash handler socket and other temp files go here
+    pub(crate) fn runtime() -> Option<PathBuf> {
+        Some(dirs::runtime_dir()?.join(BUNDLE_ID).join("data"))
+    }
+
+    /// e.g. `/home/alice/.local/share/dev.firezone.client/data`
+    ///
+    /// Things like actor name are stored here because they're kind of config,
+    /// the system / user should not delete them to free up space, but they're not
+    /// really config since the program will rewrite them automatically to persist sessions.
+    pub(crate) fn session() -> Option<PathBuf> {
+        Some(dirs::data_local_dir()?.join(BUNDLE_ID).join("data"))
+    }
+
+    /// e.g. `/home/alice/.config/dev.firezone.client/config`
+    ///
+    /// See connlib docs for details
+    pub(crate) fn settings() -> Option<PathBuf> {
+        Some(dirs::config_local_dir()?.join(BUNDLE_ID).join("config"))
+    }
+}
+
+#[cfg(target_os = "windows")]
+mod imp {
+    use connlib_shared::BUNDLE_ID;
+    use known_folders::{get_known_folder_path, KnownFolder};
+    use std::path::PathBuf;
+
+    /// e.g. `C:\ProgramData\dev.firezone.client\config`
+    ///
+    /// Device ID is stored here until <https://github.com/firezone/firezone/issues/3712> lands
+    pub(crate) fn device_id() -> Option<PathBuf> {
+        Some(
+            get_known_folder_path(KnownFolder::ProgramData)?
+                .join(BUNDLE_ID)
+                .join("config"),
+        )
+    }
+
+    /// e.g. `C:\Users\Alice\AppData\Local\dev.firezone.client\data\logs`
+    ///
+    /// See connlib docs for details
+    pub(crate) fn logs() -> Option<PathBuf> {
+        Some(
+            connlib_shared::windows::app_local_data_dir()
+                .ok()?
+                .join("data")
+                .join("logs"),
+        )
+    }
+
+    /// e.g. `C:\Users\Alice\AppData\Local\dev.firezone.client\data`
+    ///
+    /// Crash handler socket and other temp files go here
+    pub(crate) fn runtime() -> Option<PathBuf> {
+        Some(
+            connlib_shared::windows::app_local_data_dir()
+                .ok()?
+                .join("data"),
+        )
+    }
+
+    /// e.g. `C:\Users\Alice\AppData\Local\dev.firezone.client\data`
+    ///
+    /// Things like actor name go here
+    pub(crate) fn session() -> Option<PathBuf> {
+        Some(
+            connlib_shared::windows::app_local_data_dir()
+                .ok()?
+                .join("data"),
+        )
+    }
+
+    /// e.g. `C:\Users\Alice\AppData\Local\dev.firezone.client\config`
+    ///
+    /// See connlib docs for details
+    pub(crate) fn settings() -> Option<PathBuf> {
+        Some(
+            connlib_shared::windows::app_local_data_dir()
+                .ok()?
+                .join("config"),
+        )
+    }
+}

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

@@ -1,9 +1,11 @@
 //! Everything for logging to files, zipping up the files for export, and counting the files
 
-use crate::client::gui::{ControllerRequest, CtlrTx, Managed};
+use crate::client::{
+    gui::{ControllerRequest, CtlrTx, Managed},
+    known_dirs,
+};
 use anyhow::{bail, Context, Result};
 use connlib_client_shared::file_logger;
-use connlib_shared::windows::app_local_data_dir;
 use serde::Serialize;
 use std::{fs, io, path::PathBuf, result::Result as StdResult, str::FromStr};
 use tokio::task::spawn_blocking;
@@ -179,13 +181,7 @@ pub(crate) async fn count_logs_inner() -> Result<FileCount> {
     Ok(file_count)
 }
 
-/// Returns the well-known log path
-///
-/// e.g. %LOCALAPPDATA%/dev.firezone.client/data/logs/
+/// Wrapper around `known_dirs::logs`
 pub(crate) fn log_path() -> Result<PathBuf, Error> {
-    let path = app_local_data_dir()
-        .map_err(|_| Error::CantFindLocalAppDataFolder)?
-        .join("data")
-        .join("logs");
-    Ok(path)
+    known_dirs::logs().ok_or(Error::CantFindLocalAppDataFolder)
 }

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

@@ -1,9 +1,11 @@
 //! Everything related to the Settings window, including
 //! advanced settings and code for manipulating diagnostic logs.
 
-use crate::client::gui::{ControllerRequest, Managed};
-use anyhow::Result;
-use connlib_shared::windows::app_local_data_dir;
+use crate::client::{
+    gui::{ControllerRequest, Managed},
+    known_dirs,
+};
+use anyhow::{Context, Result};
 use serde::{Deserialize, Serialize};
 use std::{path::PathBuf, time::Duration};
 use tokio::sync::oneshot;
@@ -38,15 +40,10 @@ impl Default for AdvancedSettings {
     }
 }
 
-struct DirAndPath {
-    dir: PathBuf,
-    path: PathBuf,
-}
-
-fn advanced_settings_path() -> Result<DirAndPath> {
-    let dir = app_local_data_dir()?.join("config");
-    let path = dir.join("advanced_settings.json");
-    Ok(DirAndPath { dir, path })
+fn advanced_settings_path() -> Result<PathBuf> {
+    Ok(known_dirs::settings()
+        .context("`known_dirs::settings` failed")?
+        .join("advanced_settings.json"))
 }
 
 #[tauri::command]
@@ -93,8 +90,11 @@ pub(crate) async fn get_advanced_settings(
 }
 
 pub(crate) async fn apply_advanced_settings_inner(settings: &AdvancedSettings) -> Result<()> {
-    let DirAndPath { dir, path } = advanced_settings_path()?;
-    tokio::fs::create_dir_all(&dir).await?;
+    let path = advanced_settings_path()?;
+    let dir = path
+        .parent()
+        .context("settings path should have a parent")?;
+    tokio::fs::create_dir_all(dir).await?;
     tokio::fs::write(path, serde_json::to_string(&settings)?).await?;
     Ok(())
 }
@@ -103,7 +103,7 @@ pub(crate) async fn apply_advanced_settings_inner(settings: &AdvancedSettings) -
 ///
 /// Uses std::fs, so stick it in `spawn_blocking` for async contexts
 pub(crate) fn load_advanced_settings() -> Result<AdvancedSettings> {
-    let DirAndPath { dir: _, path } = advanced_settings_path()?;
+    let path = advanced_settings_path()?;
     let text = std::fs::read_to_string(path)?;
     let settings = serde_json::from_str(&text)?;
     Ok(settings)

scripts/tests/smoke-test-gui-linux.sh

@@ -3,15 +3,25 @@
 set -euo pipefail
 
 BUNDLE_ID="dev.firezone.client"
-DUMP_PATH="$BUNDLE_ID/data/logs/last_crash.dmp"
+DUMP_PATH="$HOME/.cache/$BUNDLE_ID/data/logs/last_crash.dmp"
 export FIREZONE_DISABLE_SYSTRAY=true
 PACKAGE=firezone-gui-client
 export RUST_LOG=firezone_gui_client=debug,warn
 export WEBKIT_DISABLE_COMPOSITING_MODE=1
 
+# Make sure the files we want to check don't exist on the system yet
+stat "$HOME/.cache/$BUNDLE_ID" && exit 1
+stat "$HOME/.config/$BUNDLE_ID" && exit 1
+
 # Run the smoke test normally
 xvfb-run --auto-servernum cargo run -p "$PACKAGE" -- smoke-test
 
+# Make sure the files were written in the right paths
+# TODO: Inject some bogus sign-in sequence to test the actor_name file
+stat "$HOME/.config/$BUNDLE_ID/config/advanced_settings.json"
+stat "$HOME/.cache/$BUNDLE_ID/data/logs/"connlib*log
+stat "$HOME/.config/$BUNDLE_ID/config/device_id.json"
+
 # Delete the crash file if present
 rm -f "$DUMP_PATH"