fix(gui-client): don't reset favourites when settings change (#9211)

The GUI client currently has a bug that resets the favourites and the
status of the Internet Resource every time the advanced settings are
saved. This happens because those fields are annotated with
`#[serde(default)]` and are thus initialised to their default value when
the struct is deserialised from the frontend.

To mitigate this, we introduce a new `GeneralSettings` struct that holds
the status of the Internet Resource and the list of favourites. When a
client starts up, it will try to migrate the existing advanced settings
into the new split of general and advanced settings.

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

@@ -11,7 +11,7 @@ use clap::{Args, Parser};
 use controller::Failure;
 use firezone_gui_client::{controller, deep_link, elevation, gui, logging, settings};
 use firezone_telemetry::Telemetry;
-use settings::AdvancedSettings;
+use settings::AdvancedSettingsLegacy;
 use tracing_subscriber::EnvFilter;
 
 fn main() -> ExitCode {
@@ -28,7 +28,7 @@ fn main() -> ExitCode {
         .install_default()
         .expect("Calling `install_default` only once per process should always succeed");
 
-    let settings = settings::load_advanced_settings().unwrap_or_default();
+    let settings = settings::load_advanced_settings::<AdvancedSettingsLegacy>().unwrap_or_default();
 
     let mut telemetry = Telemetry::default();
     telemetry.start(
@@ -61,7 +61,11 @@ fn main() -> ExitCode {
     }
 }
 
-fn try_main(cli: Cli, rt: &tokio::runtime::Runtime, mut settings: AdvancedSettings) -> Result<()> {
+fn try_main(
+    cli: Cli,
+    rt: &tokio::runtime::Runtime,
+    mut settings: AdvancedSettingsLegacy,
+) -> Result<()> {
     let config = gui::RunConfig {
         inject_faults: cli.inject_faults,
         debug_update_check: cli.debug_update_check,
@@ -186,11 +190,11 @@ fn try_main(cli: Cli, rt: &tokio::runtime::Runtime, mut settings: AdvancedSettin
 }
 
 /// Parse the log filter from settings, showing an error and fixing it if needed
-fn fix_log_filter(settings: &mut AdvancedSettings) -> Result<()> {
+fn fix_log_filter(settings: &mut AdvancedSettingsLegacy) -> Result<()> {
     if EnvFilter::try_new(&settings.log_filter).is_ok() {
         return Ok(());
     }
-    settings.log_filter = AdvancedSettings::default().log_filter;
+    settings.log_filter = AdvancedSettingsLegacy::default().log_filter;
 
     native_dialog::MessageDialog::new()
         .set_title("Log filter error")

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

@@ -3,7 +3,7 @@ use crate::{
     gui::{self, system_tray},
     ipc::{self, SocketId},
     logging, service,
-    settings::{self, AdvancedSettings},
+    settings::{self, AdvancedSettings, GeneralSettings},
     updates, uptime,
 };
 use anyhow::{Context, Result, anyhow, bail};
@@ -32,7 +32,7 @@ mod ran_before;
 pub type CtlrTx = mpsc::Sender<ControllerRequest>;
 
 pub struct Controller<I: GuiIntegration> {
-    /// Debugging-only settings like API URL, auth URL, log filter
+    general_settings: GeneralSettings,
     advanced_settings: AdvancedSettings,
     // Sign-in state with the portal / deep links
     auth: auth::Auth,
@@ -205,10 +205,12 @@ enum EventloopTick {
 pub struct FailedToReceiveHello(anyhow::Error);
 
 impl<I: GuiIntegration> Controller<I> {
+    #[expect(clippy::too_many_arguments, reason = "We don't care.")]
     pub(crate) async fn start(
         ctlr_tx: CtlrTx,
         integration: I,
         rx: mpsc::Receiver<ControllerRequest>,
+        general_settings: GeneralSettings,
         advanced_settings: AdvancedSettings,
         log_filter_reloader: FilterReloadHandle,
         updates_rx: mpsc::Receiver<Option<updates::Notification>>,
@@ -227,6 +229,7 @@ impl<I: GuiIntegration> Controller<I> {
         let network_notifier = new_network_notifier().await?.boxed();
 
         let controller = Controller {
+            general_settings,
             advanced_settings,
             auth: auth::Auth::new()?,
             clear_logs_callback: None,
@@ -470,7 +473,7 @@ impl<I: GuiIntegration> Controller<I> {
                 self.advanced_settings = *settings;
 
                 // Save to disk
-                settings::save(&self.advanced_settings).await?;
+                settings::save_advanced(&self.advanced_settings).await?;
 
                 // Tell tunnel about new log level
                 self.send_ipc(&service::ClientMsg::ApplyLogFilter {
@@ -524,9 +527,7 @@ impl<I: GuiIntegration> Controller<I> {
                     .set_welcome_window_visible(false, self.auth.session())?;
             }
             SystemTrayMenu(system_tray::Event::AddFavorite(resource_id)) => {
-                self.advanced_settings
-                    .favorite_resources
-                    .insert(resource_id);
+                self.general_settings.favorite_resources.insert(resource_id);
                 self.refresh_favorite_resources().await?;
             }
             SystemTrayMenu(system_tray::Event::AdminPortal) => self
@@ -556,7 +557,7 @@ impl<I: GuiIntegration> Controller<I> {
                 }
             },
             SystemTrayMenu(system_tray::Event::RemoveFavorite(resource_id)) => {
-                self.advanced_settings
+                self.general_settings
                     .favorite_resources
                     .remove(&resource_id);
                 self.refresh_favorite_resources().await?;
@@ -565,11 +566,11 @@ impl<I: GuiIntegration> Controller<I> {
                 self.try_retry_connection().await?
             }
             SystemTrayMenu(system_tray::Event::EnableInternetResource) => {
-                self.advanced_settings.internet_resource_enabled = Some(true);
+                self.general_settings.internet_resource_enabled = Some(true);
                 self.update_disabled_resources().await?;
             }
             SystemTrayMenu(system_tray::Event::DisableInternetResource) => {
-                self.advanced_settings.internet_resource_enabled = Some(false);
+                self.general_settings.internet_resource_enabled = Some(false);
                 self.update_disabled_resources().await?;
             }
             SystemTrayMenu(system_tray::Event::ShowWindow(window)) => {
@@ -804,7 +805,7 @@ impl<I: GuiIntegration> Controller<I> {
     }
 
     async fn update_disabled_resources(&mut self) -> Result<()> {
-        settings::save(&self.advanced_settings).await?;
+        settings::save_general(&self.general_settings).await?;
 
         let Some(internet_resource) = self.status.internet_resource() else {
             return Ok(());
@@ -812,7 +813,7 @@ impl<I: GuiIntegration> Controller<I> {
 
         let mut disabled_resources = BTreeSet::new();
 
-        if !self.advanced_settings.internet_resource_enabled() {
+        if !self.general_settings.internet_resource_enabled() {
             disabled_resources.insert(internet_resource.id());
         }
 
@@ -827,7 +828,7 @@ impl<I: GuiIntegration> Controller<I> {
 
     /// Saves the current settings (including favorites) to disk and refreshes the tray menu
     async fn refresh_favorite_resources(&mut self) -> Result<()> {
-        settings::save(&self.advanced_settings).await?;
+        settings::save_general(&self.general_settings).await?;
         self.refresh_system_tray_menu();
         Ok(())
     }
@@ -847,8 +848,8 @@ impl<I: GuiIntegration> Controller<I> {
                 Status::TunnelReady { resources } => {
                     system_tray::ConnlibState::SignedIn(system_tray::SignedIn {
                         actor_name: auth_session.actor_name.clone(),
-                        favorite_resources: self.advanced_settings.favorite_resources.clone(),
-                        internet_resource_enabled: self.advanced_settings.internet_resource_enabled,
+                        favorite_resources: self.general_settings.favorite_resources.clone(),
+                        internet_resource_enabled: self.general_settings.internet_resource_enabled,
                         resources: resources.clone(),
                     })
                 }

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

@@ -9,7 +9,7 @@ use crate::{
     deep_link,
     ipc::{self, ClientRead, ClientWrite, SocketId},
     logging,
-    settings::{self, AdvancedSettings},
+    settings::{self, AdvancedSettings, AdvancedSettingsLegacy},
     updates,
 };
 use anyhow::{Context, Result, bail};
@@ -188,7 +188,7 @@ pub enum ServerMsg {
 pub fn run(
     rt: &tokio::runtime::Runtime,
     config: RunConfig,
-    advanced_settings: AdvancedSettings,
+    advanced_settings: AdvancedSettingsLegacy,
     reloader: firezone_logging::FilterReloadHandle,
 ) -> Result<()> {
     // Needed for the deep link server
@@ -205,6 +205,9 @@ pub fn run(
         }
     };
 
+    let (general_settings, advanced_settings) =
+        rt.block_on(settings::migrate_legacy_settings(advanced_settings));
+
     let (ctlr_tx, ctlr_rx) = mpsc::channel(5);
     let (ready_tx, mut ready_rx) = mpsc::channel::<tauri::AppHandle>(1);
 
@@ -329,11 +332,13 @@ pub fn run(
             tray,
         };
 
+
         // Spawn the controller
         let ctrl_task = tokio::spawn(Controller::start(
             ctlr_tx,
             integration,
             ctlr_rx,
+            general_settings,
             advanced_settings,
             reloader,
             updates_rx,
@@ -431,7 +436,7 @@ async fn smoke_test(ctlr_tx: CtlrTx) -> Result<()> {
     tokio::time::sleep_until(quit_time).await;
 
     // Write the settings so we can check the path for those
-    settings::save(&AdvancedSettings::default()).await?;
+    settings::save_advanced(&AdvancedSettings::default()).await?;
 
     // Check results of tests
     let zip_len = tokio::fs::metadata(&path)

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

@@ -5,6 +5,7 @@ use crate::gui::Managed;
 use anyhow::{Context as _, Result};
 use connlib_model::ResourceId;
 use firezone_bin_shared::known_dirs;
+use serde::de::DeserializeOwned;
 use serde::{Deserialize, Serialize};
 use std::time::Duration;
 use std::{collections::HashSet, path::PathBuf};
@@ -40,7 +41,7 @@ pub(crate) async fn reset_advanced_settings(
 }
 
 #[derive(Clone, Deserialize, Serialize)]
-pub struct AdvancedSettings {
+pub struct AdvancedSettingsLegacy {
     pub auth_base_url: Url,
     pub api_url: Url,
     #[serde(default)]
@@ -50,6 +51,21 @@ pub struct AdvancedSettings {
     pub log_filter: String,
 }
 
+#[derive(Clone, Deserialize, Serialize)]
+pub struct AdvancedSettings {
+    pub auth_base_url: Url,
+    pub api_url: Url,
+    pub log_filter: String,
+}
+
+#[derive(Clone, Deserialize, Serialize)]
+pub struct GeneralSettings {
+    #[serde(default)]
+    pub favorite_resources: HashSet<ResourceId>,
+    #[serde(default)]
+    pub internet_resource_enabled: Option<bool>,
+}
+
 #[cfg(debug_assertions)]
 mod defaults {
     pub(crate) const AUTH_BASE_URL: &str = "https://app.firez.one";
@@ -64,7 +80,7 @@ mod defaults {
     pub(crate) const LOG_FILTER: &str = "info";
 }
 
-impl Default for AdvancedSettings {
+impl Default for AdvancedSettingsLegacy {
     fn default() -> Self {
         Self {
             auth_base_url: Url::parse(defaults::AUTH_BASE_URL).expect("static URL is a valid URL"),
@@ -76,20 +92,70 @@ impl Default for AdvancedSettings {
     }
 }
 
-impl AdvancedSettings {
+impl GeneralSettings {
     pub fn internet_resource_enabled(&self) -> bool {
         self.internet_resource_enabled.is_some_and(|v| v)
     }
 }
 
+impl Default for AdvancedSettings {
+    fn default() -> Self {
+        Self {
+            auth_base_url: Url::parse(defaults::AUTH_BASE_URL).expect("static URL is a valid URL"),
+            api_url: Url::parse(defaults::API_URL).expect("static URL is a valid URL"),
+            log_filter: defaults::LOG_FILTER.to_string(),
+        }
+    }
+}
+
 pub fn advanced_settings_path() -> Result<PathBuf> {
     Ok(known_dirs::settings()
         .context("`known_dirs::settings` failed")?
         .join("advanced_settings.json"))
 }
 
-/// Saves the settings to disk
-pub async fn save(settings: &AdvancedSettings) -> Result<()> {
+pub fn general_settings_path() -> Result<PathBuf> {
+    Ok(known_dirs::settings()
+        .context("`known_dirs::settings` failed")?
+        .join("general_settings.json"))
+}
+
+pub async fn migrate_legacy_settings(
+    legacy: AdvancedSettingsLegacy,
+) -> (GeneralSettings, AdvancedSettings) {
+    let general_settings = load_general_settings();
+
+    let advanced = AdvancedSettings {
+        auth_base_url: legacy.auth_base_url,
+        api_url: legacy.api_url,
+        log_filter: legacy.log_filter,
+    };
+
+    if let Ok(general) = general_settings {
+        return (general, advanced);
+    }
+
+    let general = GeneralSettings {
+        favorite_resources: legacy.favorite_resources,
+        internet_resource_enabled: legacy.internet_resource_enabled,
+    };
+
+    if let Err(e) = save_general(&general).await {
+        tracing::error!("Failed to write new general settings: {e:#}");
+        return (general, advanced);
+    }
+    if let Err(e) = save_advanced(&advanced).await {
+        tracing::error!("Failed to write new advanced settings: {e:#}");
+        return (general, advanced);
+    }
+
+    tracing::info!("Successfully migrate settings");
+
+    (general, advanced)
+}
+
+/// Saves the advanced settings to disk
+pub async fn save_advanced(settings: &AdvancedSettings) -> Result<()> {
     let path = advanced_settings_path()?;
     let dir = path
         .parent()
@@ -103,16 +169,44 @@ pub async fn save(settings: &AdvancedSettings) -> Result<()> {
     Ok(())
 }
 
+/// Saves the general settings to disk
+pub async fn save_general(settings: &GeneralSettings) -> Result<()> {
+    let path = general_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?;
+
+    tracing::debug!(?path, "Saved settings");
+
+    Ok(())
+}
+
 /// Return advanced settings if they're stored on disk
 ///
 /// Uses std::fs, so stick it in `spawn_blocking` for async contexts
-pub fn load_advanced_settings() -> Result<AdvancedSettings> {
+pub fn load_advanced_settings<T>() -> Result<T>
+where
+    T: DeserializeOwned,
+{
     let path = advanced_settings_path()?;
     let text = std::fs::read_to_string(path)?;
     let settings = serde_json::from_str(&text)?;
     Ok(settings)
 }
 
+/// Return general settings if they're stored on disk
+///
+/// Uses std::fs, so stick it in `spawn_blocking` for async contexts
+pub fn load_general_settings() -> Result<GeneralSettings> {
+    let path = general_settings_path()?;
+    let text = std::fs::read_to_string(path)?;
+    let settings = serde_json::from_str(&text)?;
+    Ok(settings)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

website/src/components/Changelog/GUI.tsx

@@ -8,7 +8,12 @@ export default function GUI({ os }: { os: OS }) {
   return (
     <Entries downloadLinks={downloadLinks(os)} title={title(os)}>
       {/* When you cut a release, remove any solved issues from the "known issues" lists over in `client-apps`. This must not be done when the issue's PR merges. */}
-      <Unreleased></Unreleased>
+      <Unreleased>
+        <ChangeItem pull="9211">
+          Fixes an issue where changing the Advanced settings would reset
+          the favourited resources.
+        </ChangeItem>
+      </Unreleased>
       <Entry version="1.4.14" date={new Date("2025-05-21")}>
         <ChangeItem pull="9147">
           Fixes an issue where connections failed to establish on machines