From 6d9360c15085cc7cfcb3f7168d6194dc42ce7e2e Mon Sep 17 00:00:00 2001 From: Reactor Scram Date: Thu, 7 Dec 2023 12:28:28 -0600 Subject: [PATCH] windows: fix advanced settings loading, catch deep link error (#2811) Based / Blocked on #2795 Fixes #2807 where I accidentally bail out of the controller task if the settings file is missing or isn't valid in any way. --------- Signed-off-by: Reactor Scram Co-authored-by: Jamil --- rust/Cargo.lock | 2 + .../src/device_channel/device_channel_win.rs | 28 +++++-- rust/windows-client/src-tauri/Cargo.toml | 2 + rust/windows-client/src-tauri/src/gui.rs | 84 +++++++++++++------ rust/windows-client/src-tauri/src/settings.rs | 6 +- 5 files changed, 89 insertions(+), 33 deletions(-) diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 9afdb1953..2e27805b4 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -2013,6 +2013,7 @@ dependencies = [ "anyhow", "clap", "connlib-client-shared", + "connlib-shared", "firezone-cli-utils", "http-body-util", "hyper 1.0.1", @@ -2025,6 +2026,7 @@ dependencies = [ "tauri", "tauri-build", "tauri-plugin-deep-link", + "thiserror", "tokio", "tracing", "tracing-subscriber", diff --git a/rust/connlib/tunnel/src/device_channel/device_channel_win.rs b/rust/connlib/tunnel/src/device_channel/device_channel_win.rs index ef87e5e51..65a243206 100644 --- a/rust/connlib/tunnel/src/device_channel/device_channel_win.rs +++ b/rust/connlib/tunnel/src/device_channel/device_channel_win.rs @@ -4,27 +4,36 @@ use connlib_shared::{messages::Interface, Callbacks, Result}; use ip_network::IpNetwork; use std::task::{Context, Poll}; +// TODO: Fill all this out. These are just stubs to test the GUI. + pub(crate) struct DeviceIo; pub(crate) struct IfaceConfig; impl DeviceIo { pub fn poll_read(&self, _: &mut [u8], _: &mut Context<'_>) -> Poll> { - todo!() + // Incoming packets will never appear + Poll::Pending } - pub fn write(&self, _: Packet<'_>) -> std::io::Result { - todo!() + pub fn write(&self, packet: Packet<'_>) -> std::io::Result { + // All outgoing packets are successfully written to the void + match packet { + Packet::Ipv4(msg) => Ok(msg.len()), + Packet::Ipv6(msg) => Ok(msg.len()), + } } } +const BOGUS_MTU: usize = 1_500; + impl IfaceConfig { pub(crate) fn mtu(&self) -> usize { - todo!() + BOGUS_MTU } pub(crate) async fn refresh_mtu(&self) -> Result { - todo!() + Ok(BOGUS_MTU) } pub(crate) async fn add_route( @@ -32,10 +41,15 @@ impl IfaceConfig { _: IpNetwork, _: &impl Callbacks, ) -> Result> { - todo!() + let io = DeviceIo {}; + let config = IfaceConfig {}; + Ok(Some(Device { io, config })) } } pub(crate) async fn create_iface(_: &Interface, _: &impl Callbacks) -> Result { - todo!() + Ok(Device { + config: IfaceConfig {}, + io: DeviceIo {}, + }) } diff --git a/rust/windows-client/src-tauri/Cargo.toml b/rust/windows-client/src-tauri/Cargo.toml index e2f888a07..b0b88fd51 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 = [] } anyhow = { version = "1.0" } clap = { version = "4.4", features = ["derive", "env"] } connlib-client-shared = { workspace = true } +connlib-shared = { workspace = true } firezone-cli-utils = { workspace = true } http-body-util = "0.1.0" hyper = { version = "1.0.1", features = ["http1", "server"] } @@ -21,6 +22,7 @@ smbios-lib = "0.9" secrecy.workspace = true serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" +thiserror = { version = "1.0", default-features = false } tokio = { version = "1.33.0", features = ["time"] } tracing = { workspace = true } tracing-subscriber = { version = "0.3.17", features = ["env-filter"] } diff --git a/rust/windows-client/src-tauri/src/gui.rs b/rust/windows-client/src-tauri/src/gui.rs index a635f4a42..1f44eac82 100755 --- a/rust/windows-client/src-tauri/src/gui.rs +++ b/rust/windows-client/src-tauri/src/gui.rs @@ -13,7 +13,6 @@ use tauri::{ SystemTraySubmenu, }; use tokio::sync::{mpsc, oneshot}; -use url::Url; use ControllerRequest as Req; pub(crate) type CtlrTx = mpsc::Sender; @@ -100,12 +99,14 @@ pub(crate) fn run(params: crate::GuiParams) -> Result<()> { // From https://github.com/FabianLars/tauri-plugin-deep-link/blob/main/example/main.rs let handle = app.handle(); - tauri_plugin_deep_link::register(crate::DEEP_LINK_SCHEME, move |url| { + if let Err(e) = tauri_plugin_deep_link::register(crate::DEEP_LINK_SCHEME, move |url| { match handle_deep_link(&handle, url) { Ok(()) => {} Err(e) => tracing::error!("{e}"), } - })?; + }) { + tracing::error!("couldn't register deep link scheme: {e}"); + } Ok(()) }) .build(tauri::generate_context!())? @@ -218,12 +219,17 @@ fn keyring_entry() -> Result { #[derive(Clone)] struct CallbackHandler { ctlr_tx: CtlrTx, - handle: Option, + logger: Option, +} + +#[derive(thiserror::Error, Debug)] +enum CallbackError { + #[error(transparent)] + ControllerRequest(#[from] tokio::sync::mpsc::error::TrySendError), } impl connlib_client_shared::Callbacks for CallbackHandler { - // TODO: add thiserror type - type Error = std::convert::Infallible; + type Error = CallbackError; fn on_disconnect( &self, @@ -242,15 +248,15 @@ impl connlib_client_shared::Callbacks for CallbackHandler { &self, resources: Vec, ) -> Result<(), Self::Error> { - tracing::debug!("on_update_resources"); + tracing::trace!("on_update_resources"); + // TODO: Better error handling? self.ctlr_tx - .blocking_send(ControllerRequest::UpdateResources(resources)) - .unwrap(); + .try_send(ControllerRequest::UpdateResources(resources))?; Ok(()) } fn roll_log_file(&self) -> Option { - self.handle + self.logger .as_ref()? .roll_to_new_file() .unwrap_or_else(|e| { @@ -276,7 +282,9 @@ impl Controller { .ok_or_else(|| anyhow::anyhow!("can't get Managed object from Tauri"))? .ctlr_tx .clone(); - let advanced_settings = settings::load_advanced_settings(&app).await?; + let advanced_settings = settings::load_advanced_settings(&app) + .await + .unwrap_or_default(); tracing::trace!("re-loading token"); let token: Option = tokio::task::spawn_blocking(|| { @@ -318,7 +326,7 @@ impl Controller { ctlr_tx: CtlrTx, token: &SecretString, ) -> Result> { - let (layer, handle) = file_logger::layer(std::path::Path::new("logs")); + let (layer, logger) = file_logger::layer(std::path::Path::new("logs")); // TODO: How can I set up the tracing subscriber if the Session isn't ready yet? Check what other clients do. if false { // This helps the type inference @@ -332,7 +340,7 @@ impl Controller { crate::device_id::get(), CallbackHandler { ctlr_tx, - handle: Some(handle), + logger: Some(logger), }, )?) } @@ -342,7 +350,14 @@ async fn run_controller( app: tauri::AppHandle, mut rx: mpsc::Receiver, ) -> Result<()> { - let mut controller = Controller::new(app.clone()).await?; + let mut controller = match Controller::new(app.clone()).await { + Err(e) => { + // TODO: There must be a shorter way to write these? + tracing::error!("couldn't create controller: {e}"); + return Err(e); + } + Ok(x) => x, + }; tracing::debug!("GUI controller main loop start"); @@ -378,7 +393,12 @@ async fn run_controller( )?; } Req::UpdateResources(resources) => { - tracing::debug!("got {} resources", resources.len()); + tracing::debug!("controller got UpdateResources"); + let resources: Vec<_> = resources.into_iter().map(ResourceDisplay::from).collect(); + + // TODO: Save the user name between runs of the app + app.tray_handle() + .set_menu(signed_in_menu("placeholder", &resources))?; } } } @@ -424,29 +444,45 @@ fn parse_auth_callback(input: &SecretString) -> Result { } /// The information needed for the GUI to display a resource inside the Firezone VPN -struct _ResourceDisplay { - /// UUIDv4 (Fully random) - /// This should be stable over time even if the DNS / IP / name change, so we can use it for callbacks from the tray menu - id: String, +struct ResourceDisplay { + id: connlib_shared::messages::ResourceId, /// User-friendly name, e.g. "GitLab" name: String, /// What will be copied to the clipboard to paste into a web browser - url: Url, + pastable: String, } -fn _signed_in_menu(user_email: &str, resources: &[_ResourceDisplay]) -> SystemTrayMenu { +impl From for ResourceDisplay { + fn from(x: connlib_client_shared::ResourceDescription) -> Self { + match x { + connlib_client_shared::ResourceDescription::Dns(x) => Self { + id: x.id, + name: x.name, + pastable: x.address, + }, + connlib_client_shared::ResourceDescription::Cidr(x) => Self { + id: x.id, + name: x.name, + // TODO: CIDRs aren't URLs right? + pastable: x.address.to_string(), + }, + } + } +} + +fn signed_in_menu(user_name: &str, resources: &[ResourceDisplay]) -> SystemTrayMenu { let mut menu = SystemTrayMenu::new() .add_item( - CustomMenuItem::new("".to_string(), format!("Signed in as {user_email}")).disabled(), + CustomMenuItem::new("".to_string(), format!("Signed in as {user_name}")).disabled(), ) .add_item(CustomMenuItem::new("/sign_out".to_string(), "Sign out")) .add_native_item(SystemTrayMenuItem::Separator) .add_item(CustomMenuItem::new("".to_string(), "Resources").disabled()); - for _ResourceDisplay { id, name, url } in resources { + for ResourceDisplay { id, name, pastable } in resources { let submenu = SystemTrayMenu::new().add_item(CustomMenuItem::new( format!("/resource/{id}"), - url.to_string(), + pastable.to_string(), )); menu = menu.add_submenu(SystemTraySubmenu::new(name, submenu)); } diff --git a/rust/windows-client/src-tauri/src/settings.rs b/rust/windows-client/src-tauri/src/settings.rs index ec2e5e576..6f3caabfd 100755 --- a/rust/windows-client/src-tauri/src/settings.rs +++ b/rust/windows-client/src-tauri/src/settings.rs @@ -64,11 +64,13 @@ pub(crate) async fn get_advanced_settings( managed: tauri::State<'_, Managed>, ) -> StdResult { let (tx, rx) = oneshot::channel(); - managed + if let Err(e) = managed .ctlr_tx .send(ControllerRequest::GetAdvancedSettings(tx)) .await - .unwrap(); + { + tracing::error!("couldn't request advanced settings from controller task: {e}"); + } Ok(rx.await.unwrap()) }