diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 5a1854465..c177998e9 100755 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -2026,6 +2026,7 @@ version = "1.0.0" dependencies = [ "anyhow", "arboard", + "arc-swap", "chrono", "clap", "connlib-client-shared", @@ -4338,9 +4339,9 @@ dependencies = [ [[package]] name = "pkg-config" -version = "0.3.27" +version = "0.3.28" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "26072860ba924cbfa98ea39c8c19b4dd6a4a25423dbdf219c1eca91aa0cf6964" +checksum = "69d3587f8a9e599cc7ec2c00e331f71c4e69a5f9a4b8a6efd5b07466b9736f9a" [[package]] name = "platforms" @@ -5281,9 +5282,9 @@ dependencies = [ [[package]] name = "serde_spanned" -version = "0.6.4" +version = "0.6.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "12022b835073e5b11e90a14f86838ceb1c8fb0325b72416845c487ac0fa95e80" +checksum = "eb3622f419d1296904700073ea6cc23ad690adbd66f13ea683df73298736f0c1" dependencies = [ "serde", ] @@ -5864,9 +5865,9 @@ checksum = "14c39fd04924ca3a864207c66fc2cd7d22d7c016007f9ce846cbb9326331930a" [[package]] name = "tauri" -version = "1.5.3" +version = "1.5.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32d563b672acde8d0cc4c1b1f5b855976923f67e8d6fe1eba51df0211e197be2" +checksum = "fd27c04b9543776a972c86ccf70660b517ecabbeced9fb58d8b961a13ad129af" dependencies = [ "anyhow", "cocoa", @@ -5912,9 +5913,9 @@ dependencies = [ [[package]] name = "tauri-build" -version = "1.5.0" +version = "1.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "defbfc551bd38ab997e5f8e458f87396d2559d05ce32095076ad6c30f7fc5f9c" +checksum = "e9914a4715e0b75d9f387a285c7e26b5bbfeb1249ad9f842675a82481565c532" dependencies = [ "anyhow", "cargo_toml", @@ -5931,9 +5932,9 @@ dependencies = [ [[package]] name = "tauri-codegen" -version = "1.4.1" +version = "1.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7b3475e55acec0b4a50fb96435f19631fb58cbcd31923e1a213de5c382536bbb" +checksum = "a1554c5857f65dbc377cefb6b97c8ac77b1cb2a90d30d3448114d5d6b48a77fc" dependencies = [ "base64 0.21.5", "brotli", @@ -5957,9 +5958,9 @@ dependencies = [ [[package]] name = "tauri-macros" -version = "1.4.2" +version = "1.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "acea6445eececebd72ed7720cfcca46eee3b5bad8eb408be8f7ef2e3f7411500" +checksum = "277abf361a3a6993ec16bcbb179de0d6518009b851090a01adfea12ac89fa875" dependencies = [ "heck 0.4.1", "proc-macro2", @@ -5971,9 +5972,9 @@ dependencies = [ [[package]] name = "tauri-runtime" -version = "0.14.1" +version = "0.14.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "07f8e9e53e00e9f41212c115749e87d5cd2a9eebccafca77a19722eeecd56d43" +checksum = "cf2d0652aa2891ff3e9caa2401405257ea29ab8372cce01f186a5825f1bd0e76" dependencies = [ "gtk", "http", @@ -5992,9 +5993,9 @@ dependencies = [ [[package]] name = "tauri-runtime-wry" -version = "0.14.2" +version = "0.14.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "803a01101bc611ba03e13329951a1bde44287a54234189b9024b78619c1bc206" +checksum = "6cae61fbc731f690a4899681c9052dde6d05b159b44563ace8186fc1bfb7d158" dependencies = [ "cocoa", "gtk", @@ -6012,9 +6013,9 @@ dependencies = [ [[package]] name = "tauri-utils" -version = "1.5.1" +version = "1.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a52165bb340e6f6a75f1f5eeeab1bb49f861c12abe3a176067d53642b5454986" +checksum = "ece74810b1d3d44f29f732a7ae09a63183d63949bbdd59c61f8ed2a1b70150db" dependencies = [ "brotli", "ctor", diff --git a/rust/connlib/tunnel/src/device_channel/tun_windows.rs b/rust/connlib/tunnel/src/device_channel/tun_windows.rs index df6d9310f..de1c7ed3f 100644 --- a/rust/connlib/tunnel/src/device_channel/tun_windows.rs +++ b/rust/connlib/tunnel/src/device_channel/tun_windows.rs @@ -113,6 +113,7 @@ impl Tun { }) } + // It's okay if this blocks until the route is added in the OS. pub fn add_route(&self, route: IpNetwork) -> Result<()> { tracing::debug!("add_route {route}"); let iface_idx = self.iface_idx; diff --git a/rust/windows-client/docs/manual_testing.md b/rust/windows-client/docs/manual_testing.md index 355c188ff..a651bc01d 100755 --- a/rust/windows-client/docs/manual_testing.md +++ b/rust/windows-client/docs/manual_testing.md @@ -49,6 +49,7 @@ If the client stops running while signed in, then the token may be stored in Win - [ ] Given the client is signed in, when you click "Sign Out", then the GUI will change to signed-out state, and the token will be wiped from the disk, and the client will continue running ([#2809](https://github.com/firezone/firezone/issues/2809)) - [ ] Given the client is signed in, when you click "Disconnect and Quit", then the client will stop running, and the token will stay on disk in Window's credential manager. ([#2809](https://github.com/firezone/firezone/issues/2809)) - [ ] Given the client was signed in when it stopped, when you start the client again, then the GUI will be in the signed-in state, and the user's name will be shown in the tray menu. ([#2712](https://github.com/firezone/firezone/issues/2712)) +- [ ] Given the client is signed out, when you sign in, then sign out, then sign in again, then the 2nd sign-in will work # Advanced settings diff --git a/rust/windows-client/src-tauri/Cargo.toml b/rust/windows-client/src-tauri/Cargo.toml index c2ce9363c..1fa6d8ea7 100755 --- a/rust/windows-client/src-tauri/Cargo.toml +++ b/rust/windows-client/src-tauri/Cargo.toml @@ -12,6 +12,7 @@ tauri-build = { version = "1.5", features = [] } [dependencies] arboard = { version = "3.3.0", default-features = false } anyhow = { version = "1.0" } +arc-swap = "1.6.0" chrono = { workspace = true } clap = { version = "4.4", features = ["derive", "env"] } connlib-client-shared = { workspace = true } diff --git a/rust/windows-client/src-tauri/src/client/gui.rs b/rust/windows-client/src-tauri/src/client/gui.rs index dd19af399..9ca4438d4 100755 --- a/rust/windows-client/src-tauri/src/client/gui.rs +++ b/rust/windows-client/src-tauri/src/client/gui.rs @@ -4,18 +4,22 @@ // TODO: `git grep` for unwraps before 1.0, especially this gui module use crate::client::{self, deep_link, AppLocalDataDir}; -use anyhow::{anyhow, Context, Result}; +use anyhow::{anyhow, bail, Context, Result}; +use arc_swap::ArcSwap; use client::{ logging, settings::{self, AdvancedSettings}, }; -use connlib_client_shared::file_logger; +use connlib_client_shared::{file_logger, ResourceDescription}; use connlib_shared::messages::ResourceId; use secrecy::{ExposeSecret, SecretString}; -use std::{net::IpAddr, path::PathBuf, str::FromStr, time::Duration}; +use std::{net::IpAddr, path::PathBuf, str::FromStr, sync::Arc, time::Duration}; use system_tray_menu::{Event as TrayMenuEvent, Resource as ResourceDisplay}; use tauri::{Manager, SystemTray, SystemTrayEvent}; -use tokio::sync::{mpsc, oneshot}; +use tokio::{ + sync::{mpsc, oneshot, Notify}, + task::spawn_blocking, +}; use ControllerRequest as Req; mod system_tray_menu; @@ -60,11 +64,12 @@ pub(crate) fn run(params: client::GuiParams) -> Result<()> { deep_link::register(TAURI_ID)?; let (ctlr_tx, ctlr_rx) = mpsc::channel(5); + let notify_controller = Arc::new(Notify::new()); tokio::spawn(accept_deep_links(server, ctlr_tx.clone())); let managed = Managed { - ctlr_tx, + ctlr_tx: ctlr_tx.clone(), inject_faults, }; @@ -123,10 +128,17 @@ pub(crate) fn run(params: client::GuiParams) -> Result<()> { let app_handle = app.handle(); let _ctlr_task = tokio::spawn(async move { - if let Err(e) = - run_controller(app_handle, ctlr_rx, logging_handles, advanced_settings).await + if let Err(e) = run_controller( + app_handle, + ctlr_tx, + ctlr_rx, + logging_handles, + advanced_settings, + notify_controller, + ) + .await { - tracing::error!("run_controller returned an error: {e}"); + tracing::error!("run_controller returned an error: {e:#?}"); } }); @@ -147,10 +159,7 @@ pub(crate) fn run(params: client::GuiParams) -> Result<()> { /// Worker task to accept deep links from a named pipe forever /// /// * `server` An initial named pipe server to consume before making new servers. This lets us also use the named pipe to enforce single-instance -async fn accept_deep_links( - mut server: deep_link::Server, - ctlr_tx: mpsc::Sender, -) -> Result<()> { +async fn accept_deep_links(mut server: deep_link::Server, ctlr_tx: CtlrTx) -> Result<()> { loop { if let Ok(url) = server.accept().await { ctlr_tx @@ -205,13 +214,13 @@ fn handle_system_tray_event(app: &tauri::AppHandle, event: TrayMenuEvent) -> Res pub(crate) enum ControllerRequest { CopyResource(String), + Disconnected, ExportLogs(PathBuf), GetAdvancedSettings(oneshot::Sender), SchemeRequest(url::Url), SignIn, StartStopLogCounting(bool), SignOut, - UpdateResources(Vec), } // TODO: Should these be keyed to the Google ID or email or something? @@ -226,18 +235,21 @@ fn keyring_entry() -> Result { #[derive(Clone)] struct CallbackHandler { + logger: file_logger::Handle, + notify_controller: Arc, ctlr_tx: CtlrTx, - logger: Option, + resources: Arc>>, } #[derive(thiserror::Error, Debug)] enum CallbackError { - #[error(transparent)] - ControllerRequest(#[from] tokio::sync::mpsc::error::TrySendError), #[error("system DNS resolver problem: {0}")] Resolvers(#[from] client::resolvers::Error), + #[error("can't send to controller task: {0}")] + SendError(#[from] mpsc::error::TrySendError), } +// Callbacks must all be non-blocking impl connlib_client_shared::Callbacks for CallbackHandler { type Error = CallbackError; @@ -245,8 +257,8 @@ impl connlib_client_shared::Callbacks for CallbackHandler { &self, error: Option<&connlib_client_shared::Error>, ) -> Result<(), Self::Error> { - // TODO: implement - tracing::error!("on_disconnect {error:?}"); + tracing::debug!("on_disconnect {error:?}"); + self.ctlr_tx.try_send(ControllerRequest::Disconnected)?; Ok(()) } @@ -261,14 +273,9 @@ impl connlib_client_shared::Callbacks for CallbackHandler { Ok(()) } - fn on_update_resources( - &self, - resources: Vec, - ) -> Result<(), Self::Error> { - tracing::trace!("on_update_resources"); - // TODO: Better error handling? - self.ctlr_tx - .try_send(ControllerRequest::UpdateResources(resources))?; + fn on_update_resources(&self, resources: Vec) -> Result<(), Self::Error> { + self.resources.store(resources.into()); + self.notify_controller.notify_one(); Ok(()) } @@ -277,64 +284,77 @@ impl connlib_client_shared::Callbacks for CallbackHandler { } fn roll_log_file(&self) -> Option { - self.logger - .as_ref()? - .roll_to_new_file() - .unwrap_or_else(|e| { - tracing::debug!("Failed to roll over to new file: {e}"); - let _ = self.on_error(&connlib_client_shared::Error::LogFileRollError(e)); + self.logger.roll_to_new_file().unwrap_or_else(|e| { + tracing::debug!("Failed to roll over to new file: {e}"); + let _ = self.on_error(&connlib_client_shared::Error::LogFileRollError(e)); - None - }) + None + }) } } struct Controller { /// Debugging-only settings like API URL, auth URL, log filter advanced_settings: AdvancedSettings, - /// mpsc sender to send things to the controller task ctlr_tx: CtlrTx, - /// connlib / tunnel session - connlib_session: Option>, + /// Session for the currently signed-in user, if there is one + session: Option, /// The UUIDv4 device ID persisted to disk /// Sent verbatim to Session::connect device_id: String, logging_handles: client::logging::Handles, - /// Info about currently signed-in user, if there is one - session: Option, + /// Tells us when to wake up and look for a new resource list. Tokio docs say that memory reads and writes are synchronized when notifying, so we don't need an extra mutex on the resources. + notify_controller: Arc, } -/// Information for a signed-in user session +/// Everything related to a signed-in user session struct Session { + auth_info: AuthInfo, + callback_handler: CallbackHandler, + connlib: connlib_client_shared::Session, +} + +/// Auth info that's persisted to disk if a session outlives an app instance +struct AuthInfo { /// User name, e.g. "John Doe", from the sign-in deep link actor_name: String, + /// Secret token to authenticate with the portal token: SecretString, } impl Controller { async fn new( app: tauri::AppHandle, + ctlr_tx: CtlrTx, logging_handles: client::logging::Handles, advanced_settings: AdvancedSettings, + notify_controller: Arc, ) -> Result { - let ctlr_tx = app - .try_state::() - .ok_or_else(|| anyhow::anyhow!("can't get Managed object from Tauri"))? - .ctlr_tx - .clone(); + let device_id = client::device_id::device_id(&app_local_data_dir(&app)?).await?; + + let mut this = Self { + advanced_settings, + ctlr_tx, + session: None, + device_id, + logging_handles, + notify_controller, + }; tracing::trace!("re-loading token"); - let session: Option = tokio::task::spawn_blocking(|| { + // spawn_blocking because accessing the keyring is I/O + if let Some(auth_info) = spawn_blocking(|| { let entry = keyring_entry()?; match entry.get_password() { Ok(token) => { let token = SecretString::new(token); tracing::debug!("re-loaded token from Windows credential manager"); - let session = Session { + let auth_info = AuthInfo { + // TODO: Reload actor name from disk here actor_name: "TODO".to_string(), token, }; - Ok(Some(session)) + Ok(Some(auth_info)) } Err(keyring::Error::NoEntry) => { tracing::debug!("no token in Windows credential manager"); @@ -343,145 +363,181 @@ impl Controller { Err(e) => Err(anyhow::Error::from(e)), } }) - .await??; + .await?? + { + // Connect immediately if we reloaded the token + if let Err(e) = this.start_session(auth_info) { + tracing::error!("couldn't restart session on app start: {e:#?}"); + } + } - let device_id = client::device_id::device_id(&app_local_data_dir(&app)?).await?; - - // Connect immediately if we reloaded the token - let connlib_session = if let Some(session) = session.as_ref() { - Some(Self::start_session( - &advanced_settings, - ctlr_tx.clone(), - device_id.clone(), - &session.token, - logging_handles.logger.clone(), - )?) - } else { - None - }; - - Ok(Self { - advanced_settings, - ctlr_tx, - connlib_session, - device_id, - logging_handles, - session, - }) + Ok(this) } - fn start_session( - advanced_settings: &settings::AdvancedSettings, - ctlr_tx: CtlrTx, - device_id: String, - token: &SecretString, - logger: file_logger::Handle, - ) -> Result> { - tracing::info!("Session::connect"); - Ok(connlib_client_shared::Session::connect( - advanced_settings.api_url.clone(), - token.clone(), - device_id, - CallbackHandler { - ctlr_tx, - logger: Some(logger), - }, + // TODO: Figure out how re-starting sessions automatically will work + fn start_session(&mut self, auth_info: AuthInfo) -> Result<()> { + if self.session.is_some() { + bail!("can't start session, we're already in a session"); + } + + let callback_handler = CallbackHandler { + ctlr_tx: self.ctlr_tx.clone(), + logger: self.logging_handles.logger.clone(), + notify_controller: Arc::clone(&self.notify_controller), + resources: Default::default(), + }; + + let connlib = connlib_client_shared::Session::connect( + self.advanced_settings.api_url.clone(), + auth_info.token.clone(), + self.device_id.clone(), + callback_handler.clone(), Duration::from_secs(5 * 60), - )?) + )?; + + self.session = Some(Session { + auth_info, + callback_handler, + connlib, + }); + + Ok(()) } } +// TODO: After PR #2960 lands, move some of this into `impl Controller` async fn run_controller( app: tauri::AppHandle, + ctlr_tx: CtlrTx, mut rx: mpsc::Receiver, logging_handles: client::logging::Handles, advanced_settings: AdvancedSettings, + notify_controller: Arc, ) -> Result<()> { - let mut controller = Controller::new(app.clone(), logging_handles, advanced_settings) - .await - .context("couldn't create Controller")?; + let mut controller = Controller::new( + app.clone(), + ctlr_tx, + logging_handles, + advanced_settings, + notify_controller, + ) + .await + .context("couldn't create Controller")?; let mut log_counting_task = None; - let mut resources: Vec = vec![]; - tracing::debug!("GUI controller main loop start"); - - while let Some(req) = rx.recv().await { - match req { - Req::CopyResource(id) => { - let id = ResourceId::from_str(&id)?; - let Some(res) = resources.iter().find(|r| r.id == id) else { + loop { + tokio::select! { + () = controller.notify_controller.notified() => { + let Some(session) = &controller.session else { + tracing::warn!("got notified to update resources but there is no session"); continue; }; - let mut clipboard = arboard::Clipboard::new()?; - clipboard.set_text(&res.pastable)?; - } - Req::ExportLogs(file_path) => logging::export_logs_to(file_path).await?, - Req::GetAdvancedSettings(tx) => { - tx.send(controller.advanced_settings.clone()).ok(); - } - Req::SchemeRequest(url) => { - if let Some(auth) = client::deep_link::parse_auth_callback(&url) { - tracing::debug!("setting new token"); - let entry = keyring_entry()?; - entry.set_password(auth.token.expose_secret())?; - controller.connlib_session = Some(Controller::start_session( - &controller.advanced_settings, - controller.ctlr_tx.clone(), - controller.device_id.clone(), - &auth.token, - controller.logging_handles.logger.clone(), - )?); - controller.session = Some(Session { - actor_name: auth.actor_name, - token: auth.token, - }); - } else { - tracing::warn!("couldn't handle scheme request"); - } - } - Req::SignIn => { - // TODO: Put the platform and local server callback in here - tauri::api::shell::open( - &app.shell_scope(), - &controller.advanced_settings.auth_base_url, - None, - )?; - } - Req::StartStopLogCounting(enable) => { - if enable { - if log_counting_task.is_none() { - let app = app.clone(); - log_counting_task = Some(tokio::spawn(logging::count_logs(app))); - tracing::debug!("started log counting"); - } - } else if let Some(t) = log_counting_task { - t.abort(); - log_counting_task = None; - tracing::debug!("cancelled log counting"); - } - } - Req::SignOut => { - keyring_entry()?.delete_password()?; - if let Some(mut session) = controller.connlib_session.take() { - // TODO: Needs testing - session.disconnect(None); - } - app.tray_handle().set_menu(system_tray_menu::signed_out())?; - } - Req::UpdateResources(r) => { - tracing::debug!("controller got UpdateResources"); - resources = r.into_iter().map(ResourceDisplay::from).collect(); - + let resources = session.callback_handler.resources.load().as_ref().clone(); + let resources: Vec<_> = resources.into_iter().map(ResourceDisplay::from).collect(); // TODO: Save the user name between runs of the app let actor_name = controller .session .as_ref() - .map(|x| x.actor_name.as_str()) + .map(|x| x.auth_info.actor_name.as_str()) .unwrap_or("TODO"); app.tray_handle() .set_menu(system_tray_menu::signed_in(actor_name, &resources))?; } + req = rx.recv() => { + let Some(req) = req else { + break; + }; + match req { + Req::CopyResource(id) => { + let Some(session) = &controller.session else { + tracing::warn!("got notified to update resources but there is no session"); + continue; + }; + let resources = session.callback_handler.resources.load(); + let id = ResourceId::from_str(&id)?; + let Some(res) = resources.iter().find(|r| r.id() == id) else { + continue; + }; + let mut clipboard = arboard::Clipboard::new()?; + // TODO: Make this a method on `ResourceDescription` + match res { + ResourceDescription::Dns(x) => clipboard.set_text(&x.address)?, + ResourceDescription::Cidr(x) => clipboard.set_text(&x.address.to_string())?, + } + } + Req::Disconnected => { + tracing::debug!("connlib disconnected, tearing down Session"); + if let Some(mut session) = controller.session.take() { + tracing::debug!("disconnecting connlib"); + session.connlib.disconnect(None); + } + }, + Req::ExportLogs(file_path) => logging::export_logs_to(file_path).await?, + Req::GetAdvancedSettings(tx) => { + tx.send(controller.advanced_settings.clone()).ok(); + } + Req::SchemeRequest(url) => { + let Some(auth) = client::deep_link::parse_auth_callback(&url) else { + tracing::error!("couldn't parse scheme request"); + // TODO: Move `run_controller` inside `Controller` and replace these `continue`s with `?` + continue; + }; + + let token = auth.token.clone(); + // spawn_blocking because keyring access is I/O + if let Err(e) = spawn_blocking(move || { + let entry = keyring_entry()?; + entry.set_password(token.expose_secret())?; + Ok::<_, anyhow::Error>(()) + }).await? { + tracing::warn!("couldn't save token to keyring: {e:#?}"); + } + + let auth_info = AuthInfo { + actor_name: auth.actor_name, + token: auth.token, + }; + if let Err(e) = controller.start_session(auth_info) { + tracing::error!("couldn't start session: {e:#?}"); + continue; + } + } + Req::SignIn => { + // TODO: Put the platform and local server callback in here + tauri::api::shell::open( + &app.shell_scope(), + &controller.advanced_settings.auth_base_url, + None, + )?; + } + Req::SignOut => { + // TODO: After we store the actor name on disk, clear the actor name here too. + keyring_entry()?.delete_password()?; + if let Some(mut session) = controller.session.take() { + tracing::debug!("disconnecting connlib"); + session.connlib.disconnect(None); + } + else { + tracing::error!("tried to sign out but there's no session"); + } + app.tray_handle().set_menu(system_tray_menu::signed_out())?; + } + Req::StartStopLogCounting(enable) => { + if enable { + if log_counting_task.is_none() { + let app = app.clone(); + log_counting_task = Some(tokio::spawn(logging::count_logs(app))); + tracing::debug!("started log counting"); + } + } else if let Some(t) = log_counting_task { + t.abort(); + log_counting_task = None; + tracing::debug!("cancelled log counting"); + } + } + } + } } } tracing::debug!("GUI controller task exiting cleanly");