diff --git a/rust/Cargo.lock b/rust/Cargo.lock index c35a80f2a..092ba66a5 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -172,12 +172,6 @@ dependencies = [ "x11rb", ] -[[package]] -name = "arc-swap" -version = "1.7.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "69f7f8c3906b62b754cd5326047894316021dcfe5a194c8ea52bdd94934a3457" - [[package]] name = "array-init" version = "0.0.4" @@ -1888,7 +1882,6 @@ version = "1.1.6" dependencies = [ "anyhow", "arboard", - "arc-swap", "atomicwrites", "chrono", "clap", diff --git a/rust/gui-client/src-tauri/Cargo.toml b/rust/gui-client/src-tauri/Cargo.toml index d421ae237..188add641 100644 --- a/rust/gui-client/src-tauri/Cargo.toml +++ b/rust/gui-client/src-tauri/Cargo.toml @@ -14,7 +14,6 @@ tauri-build = { version = "1.5", features = [] } [dependencies] arboard = { version = "3.4.0", default-features = false } anyhow = { version = "1.0" } -arc-swap = "1.7.0" atomicwrites = "0.4.3" chrono = { workspace = true } clap = { version = "4.5", features = ["derive", "env"] } diff --git a/rust/gui-client/src-tauri/src/client/gui.rs b/rust/gui-client/src-tauri/src/client/gui.rs index cc7debe28..6ac31faf7 100644 --- a/rust/gui-client/src-tauri/src/client/gui.rs +++ b/rust/gui-client/src-tauri/src/client/gui.rs @@ -4,20 +4,18 @@ //! The real macOS Client is in `swift/apple` use crate::client::{ - self, about, deep_link, - ipc::{self, CallbackHandler}, - logging, network_changes, + self, about, deep_link, ipc, logging, network_changes, settings::{self, AdvancedSettings}, Failure, }; use anyhow::{anyhow, bail, Context, Result}; -use arc_swap::ArcSwap; use connlib_client_shared::callbacks::ResourceDescription; +use firezone_headless_client::IpcServerMsg; use secrecy::{ExposeSecret, SecretString}; -use std::{path::PathBuf, str::FromStr, sync::Arc, time::Duration}; +use std::{path::PathBuf, str::FromStr, time::Duration}; use system_tray_menu::Event as TrayMenuEvent; use tauri::{Manager, SystemTray, SystemTrayEvent}; -use tokio::sync::{mpsc, oneshot, Notify}; +use tokio::sync::{mpsc, oneshot}; use tracing::instrument; use url::Url; @@ -410,10 +408,6 @@ pub(crate) enum ControllerRequest { ApplySettings(AdvancedSettings), /// Only used for smoke tests ClearLogs, - Disconnected { - error_msg: String, - is_authentication_error: bool, - }, /// The same as the arguments to `client::logging::export_logs_to` ExportLogs { path: PathBuf, @@ -421,10 +415,10 @@ pub(crate) enum ControllerRequest { }, Fail(Failure), GetAdvancedSettings(oneshot::Sender), + Ipc(IpcServerMsg), SchemeRequest(SecretString), SignIn, SystemTrayMenu(TrayMenuEvent), - TunnelReady, UpdateAvailable(crate::client::updates::Release), UpdateNotificationClicked(Url), } @@ -435,7 +429,7 @@ enum Status { /// Firezone is signing in and raising the tunnel. Connecting, /// Firezone is ready to use. - TunnelReady, + TunnelReady { resources: Vec }, } impl Default for Status { @@ -450,7 +444,7 @@ impl Status { match self { Self::Disconnected => false, Self::Connecting => true, - Self::TunnelReady => true, + Self::TunnelReady { .. } => true, } } } @@ -464,9 +458,6 @@ struct Controller { ctlr_tx: CtlrTx, ipc_client: ipc::Client, log_filter_reloader: logging::Reloader, - /// 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, - resources: Arc>>, status: Status, uptime: client::uptime::Tracker, } @@ -523,27 +514,6 @@ impl Controller { Req::ClearLogs => logging::clear_logs_inner() .await .context("Failed to clear logs")?, - Req::Disconnected { - error_msg, - is_authentication_error, - } => { - self.sign_out().await?; - if is_authentication_error { - tracing::info!(?error_msg, "Auth error"); - os::show_notification( - "Firezone disconnected", - "To access resources, sign in again.", - )?; - } else { - tracing::error!(?error_msg, "Disconnected"); - native_dialog::MessageDialog::new() - .set_title("Firezone Error") - .set_text(&error_msg) - .set_type(native_dialog::MessageType::Error) - .show_alert() - .context("Couldn't show Disconnected alert")?; - } - } Req::ExportLogs { path, stem } => logging::export_logs_to(path, stem) .await .context("Failed to export logs to zip")?, @@ -553,6 +523,9 @@ impl Controller { Req::GetAdvancedSettings(tx) => { tx.send(self.advanced_settings.clone()).ok(); } + Req::Ipc(msg) => if let Err(error) = self.handle_ipc(msg).await { + tracing::error!(?error, "Failed to handle IPC message"); + } Req::SchemeRequest(url) => { if let Err(error) = self.handle_deep_link(&url).await { tracing::error!(?error, "`handle_deep_link` failed"); @@ -597,7 +570,7 @@ impl Controller { ); self.sign_out().await?; } - Status::TunnelReady => tracing::error!("Can't cancel sign-in, the tunnel is already up. This is a logic error in the code."), + Status::TunnelReady{..} => tracing::error!("Can't cancel sign-in, the tunnel is already up. This is a logic error in the code."), } } Req::SystemTrayMenu(TrayMenuEvent::ShowWindow(window)) => { @@ -623,16 +596,6 @@ impl Controller { Req::SystemTrayMenu(TrayMenuEvent::Quit) => Err(anyhow!( "Impossible error: `Quit` should be handled before this" ))?, - Req::TunnelReady => { - if ! matches!(self.status, Status::TunnelReady) { - self.status = Status::TunnelReady; - os::show_notification( - "Firezone connected", - "You are now signed in and able to access resources.", - )?; - } - self.refresh_system_tray_menu()?; - } Req::UpdateAvailable(release) => { let title = format!("Firezone {} available for download", release.version); @@ -650,6 +613,53 @@ impl Controller { Ok(()) } + async fn handle_ipc(&mut self, msg: IpcServerMsg) -> Result<()> { + match msg { + IpcServerMsg::OnDisconnect { + error_msg, + is_authentication_error, + } => { + self.sign_out().await?; + if is_authentication_error { + tracing::info!(?error_msg, "Auth error"); + os::show_notification( + "Firezone disconnected", + "To access resources, sign in again.", + )?; + } else { + tracing::error!(?error_msg, "Disconnected"); + native_dialog::MessageDialog::new() + .set_title("Firezone Error") + .set_text(&error_msg) + .set_type(native_dialog::MessageType::Error) + .show_alert() + .context("Couldn't show Disconnected alert")?; + } + } + IpcServerMsg::OnUpdateResources(resources) => { + if self.auth.session().is_none() { + // This could happen if the user cancels the sign-in + // before it completes. This is because the state machine + // between the GUI, the IPC service, and connlib isn't perfectly synced. + tracing::error!("Got `UpdateResources` while signed out"); + return Ok(()); + } + tracing::debug!(len = resources.len(), "Got new Resources"); + if !matches!(self.status, Status::TunnelReady { .. }) { + os::show_notification( + "Firezone connected", + "You are now signed in and able to access resources.", + )?; + } + self.status = Status::TunnelReady { resources }; + if let Err(error) = self.refresh_system_tray_menu() { + tracing::error!(?error, "Failed to refresh Resource list"); + } + } + } + Ok(()) + } + /// Returns a new system tray menu fn build_system_tray_menu(&self) -> tauri::SystemTrayMenu { // TODO: Refactor this and the auth module so that "Are we logged in" @@ -663,8 +673,8 @@ impl Controller { system_tray_menu::signed_out() } Status::Connecting => system_tray_menu::signing_in("Signing In..."), - Status::TunnelReady => { - system_tray_menu::signed_in(&auth_session.actor_name, &self.resources.load()) + Status::TunnelReady { resources } => { + system_tray_menu::signed_in(&auth_session.actor_name, resources) } } } else if self.auth.ongoing_request().is_ok() { @@ -727,14 +737,7 @@ async fn run_controller( log_filter_reloader: logging::Reloader, ) -> Result<(), Error> { tracing::info!("Entered `run_controller`"); - let notify_controller = Arc::new(Notify::new()); - let resources = Default::default(); - let callback_handler = CallbackHandler { - ctlr_tx: ctlr_tx.clone(), - notify_controller: Arc::clone(¬ify_controller), - resources: Arc::clone(&resources), - }; - let ipc_client = ipc::Client::new(callback_handler).await?; + let ipc_client = ipc::Client::new(ctlr_tx.clone()).await?; let mut controller = Controller { advanced_settings, app: app.clone(), @@ -742,8 +745,6 @@ async fn run_controller( ctlr_tx, ipc_client, log_filter_reloader, - notify_controller, // TODO: Fix cancel-safety - resources, status: Default::default(), uptime: Default::default(), }; @@ -777,12 +778,6 @@ async fn run_controller( loop { tokio::select! { - () = controller.notify_controller.notified() => { - tracing::debug!("Controller notified of new resources"); - if let Err(error) = controller.refresh_system_tray_menu() { - tracing::error!(?error, "Failed to reload resource list"); - } - } () = com_worker.notified() => { let new_have_internet = network_changes::check_internet().context("Failed to check for internet")?; if new_have_internet != have_internet { diff --git a/rust/gui-client/src-tauri/src/client/ipc.rs b/rust/gui-client/src-tauri/src/client/ipc.rs index d2cf2c558..68186a7a7 100644 --- a/rust/gui-client/src-tauri/src/client/ipc.rs +++ b/rust/gui-client/src-tauri/src/client/ipc.rs @@ -1,47 +1,12 @@ use crate::client::gui::{ControllerRequest, CtlrTx}; use anyhow::{Context as _, Result}; -use arc_swap::ArcSwap; -use connlib_client_shared::callbacks::ResourceDescription; use firezone_headless_client::{ ipc::{self, Error}, - IpcClientMsg, IpcServerMsg, + IpcClientMsg, }; use futures::{SinkExt, StreamExt}; use secrecy::{ExposeSecret, SecretString}; -use std::{net::IpAddr, sync::Arc}; -use tokio::sync::Notify; - -#[derive(Clone)] -pub(crate) struct CallbackHandler { - pub notify_controller: Arc, - pub ctlr_tx: CtlrTx, - pub resources: Arc>>, -} - -// Almost but not quite implements `Callbacks` from connlib. -// Because of the IPC boundary, we can deviate. -impl CallbackHandler { - fn on_disconnect(&self, error_msg: String, is_authentication_error: bool) { - self.ctlr_tx - .try_send(ControllerRequest::Disconnected { - error_msg, - is_authentication_error, - }) - .expect("controller channel failed"); - } - - fn on_tunnel_ready(&self) { - self.ctlr_tx - .try_send(ControllerRequest::TunnelReady) - .expect("controller channel failed"); - } - - fn on_update_resources(&self, resources: Vec) { - tracing::debug!("on_update_resources"); - self.resources.store(resources.into()); - self.notify_controller.notify_one(); - } -} +use std::net::IpAddr; pub(crate) struct Client { task: tokio::task::JoinHandle>, @@ -57,7 +22,7 @@ impl Drop for Client { } impl Client { - pub(crate) async fn new(callback_handler: CallbackHandler) -> Result { + pub(crate) async fn new(ctlr_tx: CtlrTx) -> Result { tracing::info!( client_pid = std::process::id(), "Connecting to IPC service..." @@ -65,15 +30,7 @@ impl Client { let (mut rx, tx) = ipc::connect_to_service(ipc::ServiceId::Prod).await?; let task = tokio::task::spawn(async move { while let Some(msg) = rx.next().await.transpose()? { - match msg { - IpcServerMsg::Ok => {} - IpcServerMsg::OnDisconnect { - error_msg, - is_authentication_error, - } => callback_handler.on_disconnect(error_msg, is_authentication_error), - IpcServerMsg::OnTunnelReady => callback_handler.on_tunnel_ready(), - IpcServerMsg::OnUpdateResources(v) => callback_handler.on_update_resources(v), - } + ctlr_tx.send(ControllerRequest::Ipc(msg)).await?; } Ok(()) }); diff --git a/rust/headless-client/src/ipc_service.rs b/rust/headless-client/src/ipc_service.rs index 176e03e2b..313c6af79 100644 --- a/rust/headless-client/src/ipc_service.rs +++ b/rust/headless-client/src/ipc_service.rs @@ -253,10 +253,6 @@ impl Handler { InternalServerMsg::OnSetInterfaceConfig { ipv4, ipv6, dns } => { self.tun_device.set_ips(ipv4, ipv6).await?; self.dns_controller.set_dns(&dns).await?; - self.ipc_tx - .send(&IpcServerMsg::OnTunnelReady) - .await - .context("Error while sending `OnTunnelReady`")? } InternalServerMsg::OnUpdateRoutes { ipv4, ipv6 } => { self.tun_device.set_routes(ipv4, ipv6).await? diff --git a/rust/headless-client/src/ipc_service/ipc.rs b/rust/headless-client/src/ipc_service/ipc.rs index 9fe38c5e7..e2d3a9a17 100644 --- a/rust/headless-client/src/ipc_service/ipc.rs +++ b/rust/headless-client/src/ipc_service/ipc.rs @@ -207,7 +207,7 @@ mod tests { while let Some(req) = rx.next().await { let req = req.expect("Error while reading from IPC client"); ensure!(req == IpcClientMsg::Reconnect); - tx.send(&IpcServerMsg::OnTunnelReady) + tx.send(&IpcServerMsg::OnUpdateResources(vec![])) .await .expect("Error while writing to IPC client"); } @@ -232,7 +232,7 @@ mod tests { .await .expect("Should have gotten a reply from the IPC server") .expect("Error while reading from IPC server"); - ensure!(matches!(resp, IpcServerMsg::OnTunnelReady)); + ensure!(matches!(resp, IpcServerMsg::OnUpdateResources(_))); } } Ok(()) diff --git a/rust/headless-client/src/lib.rs b/rust/headless-client/src/lib.rs index cd9aef98d..720a11310 100644 --- a/rust/headless-client/src/lib.rs +++ b/rust/headless-client/src/lib.rs @@ -92,12 +92,10 @@ enum InternalServerMsg { /// Messages that we can send to IPC clients #[derive(Debug, serde::Deserialize, serde::Serialize)] pub enum IpcServerMsg { - Ok, OnDisconnect { error_msg: String, is_authentication_error: bool, }, - OnTunnelReady, OnUpdateResources(Vec), } diff --git a/rust/headless-client/src/standalone.rs b/rust/headless-client/src/standalone.rs index 34027ae3b..6e838ad2a 100644 --- a/rust/headless-client/src/standalone.rs +++ b/rust/headless-client/src/standalone.rs @@ -198,8 +198,6 @@ pub fn run_only_headless_client() -> Result<()> { error_msg, is_authentication_error: _, }) => return Err(anyhow!(error_msg).context("Firezone disconnected")), - InternalServerMsg::Ipc(IpcServerMsg::Ok) - | InternalServerMsg::Ipc(IpcServerMsg::OnTunnelReady) => {} InternalServerMsg::Ipc(IpcServerMsg::OnUpdateResources(_)) => { // On every resources update, flush DNS to mitigate dns_controller.flush()?;