From 52d7000ac5b74c8f62aebd137cad4eb086571a23 Mon Sep 17 00:00:00 2001 From: _ Date: Mon, 11 Mar 2024 14:57:00 -0500 Subject: [PATCH 01/17] feat(linux): register deep links using a desktop entry --- rust/gui-client/build.sh | 13 ++--- rust/gui-client/docs/manual_testing.md | 8 ++- .../src-tauri/src/client/deep_link/linux.rs | 55 ++++++++++++++++--- rust/gui-client/src-tauri/src/client/gui.rs | 4 +- 4 files changed, 63 insertions(+), 17 deletions(-) diff --git a/rust/gui-client/build.sh b/rust/gui-client/build.sh index 2c81a14828..1de8286d83 100755 --- a/rust/gui-client/build.sh +++ b/rust/gui-client/build.sh @@ -1,18 +1,15 @@ -#!/bin/sh +#!/usr/bin/env bash -# The Windows client obviously doesn't build for *nix, but this -# script is helpful for doing UI work on those platforms for the -# Windows client. -set -e +set -euo pipefail # Copy frontend dependencies cp node_modules/flowbite/dist/flowbite.min.js src/ # Compile TypeScript -tsc +pnpm tsc # Compile CSS -tailwindcss -i src/input.css -o src/output.css +pnpm tailwindcss -i src/input.css -o src/output.css # Compile Rust and bundle -tauri build +pnpm tauri build diff --git a/rust/gui-client/docs/manual_testing.md b/rust/gui-client/docs/manual_testing.md index 341de7eaa1..14799ed801 100644 --- a/rust/gui-client/docs/manual_testing.md +++ b/rust/gui-client/docs/manual_testing.md @@ -80,7 +80,9 @@ If the client stops running while signed in, then the token may be stored in Win # Resetting state -This is a list of all the on-disk state that you need to delete / reset to test a first-time install / first-time run of the Firezone client. +This is a list of all the on-disk state that you need to delete / reset to test a first-time install / first-time run of the Firezone GUI client. + +## Windows - Dir `%LOCALAPPDATA%/dev.firezone.client/` (Config, logs, webview cache, wintun.dll etc.) - Dir `%PROGRAMDATA%/dev.firezone.client/` (Device ID file) @@ -89,6 +91,10 @@ This is a list of all the on-disk state that you need to delete / reset to test - Registry key `Computer\HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Interfaces\{e9245bc1-b8c1-44ca-ab1d-c6aad4f13b9c}` (IP address and DNS server for our tunnel interface) - Windows Credential Manager, "Windows Credentials", "Generic Credentials", `dev.firezone.client/token` +## Linux + +- Dir `$HOME/.local/share/applications` (.desktop file for deep links. This dir may not even exist by default on distros like Debian) + # Token storage ([#2740](https://github.com/firezone/firezone/issues/2740)) diff --git a/rust/gui-client/src-tauri/src/client/deep_link/linux.rs b/rust/gui-client/src-tauri/src/client/deep_link/linux.rs index 7272694632..5a4ba4f5e1 100644 --- a/rust/gui-client/src-tauri/src/client/deep_link/linux.rs +++ b/rust/gui-client/src-tauri/src/client/deep_link/linux.rs @@ -1,29 +1,70 @@ //! TODO: Not implemented for Linux yet use super::Error; +use anyhow::{bail, Context, Result}; use secrecy::SecretString; pub(crate) struct Server {} impl Server { pub(crate) fn new() -> Result { - tracing::warn!("Not implemented yet"); - tracing::trace!(scheme = super::FZ_SCHEME, "prevents dead code warning"); + tracing::error!("`deep_link::Server::new` not implemented yet"); Ok(Self {}) } pub(crate) async fn accept(self) -> Result { - tracing::warn!("Deep links not implemented yet on Linux"); + tracing::error!("Deep links not implemented yet on Linux"); futures::future::pending().await } } -pub(crate) async fn open(_url: &url::Url) -> Result<(), Error> { - tracing::warn!("Not implemented yet"); +pub(crate) async fn open(_url: &url::Url) -> Result<()> { + crate::client::logging::debug_command_setup()?; + tracing::error!("Deep link callback handling not implemented yet for Linux"); Ok(()) } -pub(crate) fn register() -> Result<(), Error> { - tracing::warn!("Not implemented yet"); +/// Register a URI scheme so that browser can deep link into our app for auth +/// +/// Performs blocking I/O (Waits on `xdg-desktop-menu` subprocess) +pub(crate) fn register() -> Result<()> { + // Write `$HOME/.local/share/applications/firezone-client.desktop` + // According to , that's the place to put + // per-user desktop entries. + let dir = dirs::data_local_dir() + .context("can't figure out where to put our desktop entry")? + .join("applications"); + std::fs::create_dir_all(&dir)?; + + // Don't use atomic writes here - If we lose power, we'll just rewrite this file on + // the next boot anyway. + let path = dir.join("firezone-client.desktop"); + let exe = std::env::current_exe().context("failed to find our own exe path")?; + let content = format!( + "[Desktop Entry] +Version=1.0 +Name=Firezone +Comment=Firezone GUI Client +Exec={} open-deep-link %U +Terminal=false +Type=Application +MimeType=x-scheme-handler/{} +Categories=Network; +", + exe.display(), + super::FZ_SCHEME + ); + std::fs::write(&path, content).context("failed to write desktop entry file")?; + + // Run `xdg-desktop-menu install` with that desktop file + let xdg_desktop_menu = "xdg-desktop-menu"; + let status = std::process::Command::new(xdg_desktop_menu) + .arg("install") + .arg(&path) + .status() + .with_context(|| format!("failed to run `{xdg_desktop_menu}`"))?; + if !status.success() { + bail!("failed to register our deep link scheme") + } Ok(()) } diff --git a/rust/gui-client/src-tauri/src/client/gui.rs b/rust/gui-client/src-tauri/src/client/gui.rs index 2c46d3ecda..6c65d08cb0 100644 --- a/rust/gui-client/src-tauri/src/client/gui.rs +++ b/rust/gui-client/src-tauri/src/client/gui.rs @@ -87,6 +87,8 @@ pub(crate) enum Error { // `client.rs` provides a more user-friendly message when showing the error dialog box #[error("WebViewNotInstalled")] WebViewNotInstalled, + #[error(transparent)] + Other(#[from] anyhow::Error), } /// Runs the Tauri GUI and returns on exit or unrecoverable error @@ -167,7 +169,7 @@ pub(crate) fn run(cli: &client::Cli) -> Result<(), Error> { // We know now we're the only instance on the computer, so register our exe // to handle deep links - deep_link::register()?; + deep_link::register().context("Failed to register deep link handler")?; tokio::spawn(accept_deep_links(server, ctlr_tx.clone())); let managed = Managed { From 1e8d85d5a83664a3377edaad7aadf976ff19de32 Mon Sep 17 00:00:00 2001 From: Not Applicable Date: Tue, 12 Mar 2024 15:04:06 +0000 Subject: [PATCH 02/17] it logged in --- rust/gui-client/src-tauri/src/client.rs | 5 +- .../src-tauri/src/client/deep_link.rs | 80 +++++++++++++------ .../src-tauri/src/client/deep_link/linux.rs | 65 ++++++++++++--- .../src-tauri/src/client/deep_link/windows.rs | 18 ++--- rust/gui-client/src-tauri/src/client/gui.rs | 10 ++- .../src-tauri/src/client/gui/os_linux.rs | 22 +++++ .../src-tauri/src/client/gui/os_windows.rs | 8 ++ .../src-tauri/src/client/resolvers.rs | 3 +- 8 files changed, 157 insertions(+), 54 deletions(-) diff --git a/rust/gui-client/src-tauri/src/client.rs b/rust/gui-client/src-tauri/src/client.rs index 724da9f736..c65ea2e82a 100644 --- a/rust/gui-client/src-tauri/src/client.rs +++ b/rust/gui-client/src-tauri/src/client.rs @@ -78,7 +78,9 @@ pub(crate) fn run() -> Result<()> { Some(Cmd::Elevated) => run_gui(cli), Some(Cmd::OpenDeepLink(deep_link)) => { let rt = tokio::runtime::Runtime::new()?; - rt.block_on(deep_link::open(&deep_link.url))?; + if let Err(error) = rt.block_on(deep_link::open(&deep_link.url)) { + tracing::error!(?error, "Error in `OpenDeepLink`"); + } Ok(()) } Some(Cmd::SmokeTest) => { @@ -212,6 +214,7 @@ pub enum Cmd { #[derive(Args)] pub struct DeepLink { + // TODO: Should be `Secret`? pub url: url::Url, } diff --git a/rust/gui-client/src-tauri/src/client/deep_link.rs b/rust/gui-client/src-tauri/src/client/deep_link.rs index b1079b9701..3077c343af 100644 --- a/rust/gui-client/src-tauri/src/client/deep_link.rs +++ b/rust/gui-client/src-tauri/src/client/deep_link.rs @@ -1,5 +1,6 @@ //! A module for registering, catching, and parsing deep links that are sent over to the app's already-running instance +use anyhow::{bail, Context, Result}; use crate::client::auth::Response as AuthResponse; use secrecy::{ExposeSecret, SecretString}; use std::io; @@ -54,14 +55,18 @@ pub enum Error { pub(crate) use imp::{open, register, Server}; -pub(crate) fn parse_auth_callback(url: &SecretString) -> Option { - let url = Url::parse(url.expose_secret()).ok()?; - match url.host() { - Some(url::Host::Domain("handle_client_sign_in_callback")) => {} - _ => return None, +pub(crate) fn parse_auth_callback(url: &SecretString) -> Result { + // TODO: Delete this before opening PR + tracing::debug!(secret = url.expose_secret(), "Parsing URL"); + let url = Url::parse(url.expose_secret())?; + if Some(url::Host::Domain("handle_client_sign_in_callback")) != url.host() { + bail!("URL host should be `handle_client_sign_in_callback`"); } - if url.path() != "/" { - return None; + // Sometimes I get an empty path, might be a glitch in Firefox Linux aarch64? + match url.path() { + "/" => {} + "" => {} + _ => bail!("URL path should be `/` or empty"), } let mut actor_name = None; @@ -72,22 +77,19 @@ pub(crate) fn parse_auth_callback(url: &SecretString) -> Option { match key.as_ref() { "actor_name" => { if actor_name.is_some() { - // actor_name must appear exactly once - return None; + bail!("`actor_name` should appear exactly once"); } actor_name = Some(value.to_string()); } "fragment" => { if fragment.is_some() { - // must appear exactly once - return None; + bail!("`fragment` should appear exactly once"); } fragment = Some(SecretString::new(value.to_string())); } "state" => { if state.is_some() { - // must appear exactly once - return None; + bail!("`state` should appear exactly once"); } state = Some(SecretString::new(value.to_string())); } @@ -95,31 +97,39 @@ pub(crate) fn parse_auth_callback(url: &SecretString) -> Option { } } - Some(AuthResponse { - actor_name: actor_name?, - fragment: fragment?, - state: state?, + Ok(AuthResponse { + actor_name: actor_name.context("URL should have `actor_name`")?, + fragment: fragment.context("URL should have `fragment`")?, + state: state.context("URL should have `state`")?, }) } #[cfg(test)] mod tests { - use anyhow::Result; + use anyhow::{Context, Result}; use secrecy::{ExposeSecret, SecretString}; #[test] fn parse_auth_callback() -> Result<()> { // Positive cases let input = "firezone://handle_client_sign_in_callback/?actor_name=Reactor+Scram&fragment=a_very_secret_string&state=a_less_secret_string&identity_provider_identifier=12345"; - let actual = parse_callback_wrapper(input).unwrap(); + let actual = parse_callback_wrapper(input)?; assert_eq!(actual.actor_name, "Reactor Scram"); assert_eq!(actual.fragment.expose_secret(), "a_very_secret_string"); assert_eq!(actual.state.expose_secret(), "a_less_secret_string"); + + let input = "firezone-fd0020211111://handle_client_sign_in_callback?account_name=Firezone&account_slug=firezone&actor_name=Reactor+Scram&fragment=a_very_secret_string&identity_provider_identifier=1234&state=a_less_secret_string"; + let actual = parse_callback_wrapper(input)?; + + assert_eq!(actual.actor_name, "Reactor Scram"); + assert_eq!(actual.fragment.expose_secret(), "a_very_secret_string"); + assert_eq!(actual.state.expose_secret(), "a_less_secret_string"); + // Empty string "" `actor_name` is fine let input = "firezone://handle_client_sign_in_callback/?actor_name=&fragment=&state=&identity_provider_identifier=12345"; - let actual = parse_callback_wrapper(input).unwrap(); + let actual = parse_callback_wrapper(input)?; assert_eq!(actual.actor_name, ""); assert_eq!(actual.fragment.expose_secret(), ""); @@ -130,22 +140,44 @@ mod tests { // URL host is wrong let input = "firezone://not_handle_client_sign_in_callback/?actor_name=Reactor+Scram&fragment=a_very_secret_string&state=a_less_secret_string&identity_provider_identifier=12345"; let actual = parse_callback_wrapper(input); - assert!(actual.is_none()); + assert!(actual.is_err()); // `actor_name` is not just blank but totally missing let input = "firezone://handle_client_sign_in_callback/?fragment=&state=&identity_provider_identifier=12345"; let actual = parse_callback_wrapper(input); - assert!(actual.is_none()); + assert!(actual.is_err()); // URL is nonsense let input = "?????????"; let actual_result = parse_callback_wrapper(input); - assert!(actual_result.is_none()); + assert!(actual_result.is_err()); Ok(()) } - fn parse_callback_wrapper(s: &str) -> Option { + fn parse_callback_wrapper(s: &str) -> Result { super::parse_auth_callback(&SecretString::new(s.to_owned())) } + + /// Tests the named pipe or Unix domain socket, doesn't test the URI scheme itself + /// + /// Will fail if any other Firezone Client instance is running + /// Will fail with permission error if Firezone already ran as sudo + #[tokio::test] + async fn socket_smoke_test() -> Result<()> { + let server = super::Server::new().context("Couldn't start Server")?; + let server_task = tokio::spawn(async move { + let bytes = server.accept().await?; + Ok::<_, anyhow::Error>(bytes) + }); + let id = uuid::Uuid::new_v4().to_string(); + let expected_url = url::Url::parse(&format!("bogus-test-schema://{id}"))?; + super::open(&expected_url).await?; + + let bytes = server_task.await??; + let s = std::str::from_utf8(bytes.expose_secret())?; + let url = url::Url::parse(s)?; + assert_eq!(url, expected_url); + Ok(()) + } } diff --git a/rust/gui-client/src-tauri/src/client/deep_link/linux.rs b/rust/gui-client/src-tauri/src/client/deep_link/linux.rs index 5a4ba4f5e1..c18aa761ef 100644 --- a/rust/gui-client/src-tauri/src/client/deep_link/linux.rs +++ b/rust/gui-client/src-tauri/src/client/deep_link/linux.rs @@ -1,26 +1,65 @@ -//! TODO: Not implemented for Linux yet - -use super::Error; use anyhow::{bail, Context, Result}; -use secrecy::SecretString; +use crate::client::known_dirs; +use secrecy::{ExposeSecret, Secret}; +use tokio::{io::{AsyncReadExt, AsyncWriteExt}, net::{UnixListener, UnixStream}}; + +const SOCK_NAME: &str = "deep_link.sock"; -pub(crate) struct Server {} +pub(crate) struct Server { + listener: UnixListener, +} impl Server { - pub(crate) fn new() -> Result { - tracing::error!("`deep_link::Server::new` not implemented yet"); - Ok(Self {}) + /// Create a new deep link server to make sure we're the only instance + pub(crate) fn new() -> Result { + let dir = known_dirs::runtime().context("couldn't find runtime dir")?; + let path = dir.join(SOCK_NAME); + // TODO: This breaks single instance. Can we enforce it some other way? + std::fs::remove_file(&path).ok(); + std::fs::create_dir_all(&dir).context("Can't create dir for deep link socket")?; + + let listener = UnixListener::bind(&path).context("Couldn't bind listener Unix socket")?; + + // Figure out who we were before `sudo`, if using sudo + if let Ok(username) = std::env::var("SUDO_USER") { + std::process::Command::new("chown") + .arg(username) + .arg(&path) + .status()?; + } + + Ok(Self { + listener, + }) } - pub(crate) async fn accept(self) -> Result { - tracing::error!("Deep links not implemented yet on Linux"); - futures::future::pending().await + /// Await one incoming deep link + /// + /// To match the Windows API, this consumes the `Server`. + pub(crate) async fn accept(self) -> Result>> { + tracing::debug!("deep_link::accept"); + let (mut stream, _) = self.listener.accept().await?; + tracing::debug!("Accepted Unix domain socket connection"); + + // TODO: Limit reads to 4,096 bytes. Partial reads will probably never happen + // since it's a local socket transferring very small data. + let mut bytes = vec![]; + stream.read_to_end(&mut bytes).await.context("failed to read incoming deep link over Unix socket stream")?; + let bytes = Secret::new(bytes); + tracing::debug!(len = bytes.expose_secret().len(), "Got data from Unix domain socket"); + Ok(bytes) } } -pub(crate) async fn open(_url: &url::Url) -> Result<()> { +pub(crate) async fn open(url: &url::Url) -> Result<()> { crate::client::logging::debug_command_setup()?; - tracing::error!("Deep link callback handling not implemented yet for Linux"); + + let dir = known_dirs::runtime().context("deep_link::open couldn't find runtime dir")?; + let path = dir.join(SOCK_NAME); + let mut stream = UnixStream::connect(&path).await?; + + stream.write_all(url.to_string().as_bytes()).await?; + Ok(()) } diff --git a/rust/gui-client/src-tauri/src/client/deep_link/windows.rs b/rust/gui-client/src-tauri/src/client/deep_link/windows.rs index 66b8c0d1ff..fdb4127eac 100644 --- a/rust/gui-client/src-tauri/src/client/deep_link/windows.rs +++ b/rust/gui-client/src-tauri/src/client/deep_link/windows.rs @@ -2,9 +2,10 @@ //! Based on reading some of the Windows code from , which is licensed "MIT OR Apache-2.0" use super::{Error, FZ_SCHEME}; +use anyhow::Result; use connlib_shared::BUNDLE_ID; -use secrecy::{ExposeSecret, Secret, SecretString}; -use std::{ffi::c_void, io, path::Path, str::FromStr}; +use secrecy::Secret; +use std::{ffi::c_void, io, path::Path}; use tokio::{io::AsyncReadExt, io::AsyncWriteExt, net::windows::named_pipe}; use windows::Win32::Security as WinSec; @@ -18,7 +19,7 @@ impl Server { /// Construct a server, but don't await client connections yet /// /// Panics if there is no Tokio runtime - pub(crate) fn new() -> Result { + pub(crate) fn new() -> Result { // This isn't air-tight - We recreate the whole server on each loop, // rather than binding 1 socket and accepting many streams like a normal socket API. // I can only assume Tokio is following Windows' underlying API. @@ -75,7 +76,7 @@ impl Server { /// I assume this is based on the underlying Windows API. /// I tried re-using the server and it acted strange. The official Tokio /// examples are not clear on this. - pub(crate) async fn accept(mut self) -> Result { + pub(crate) async fn accept(mut self) -> Result>> { self.inner .connect() .await @@ -93,14 +94,7 @@ impl Server { let bytes = Secret::new(bytes); self.inner.disconnect().ok(); - - tracing::debug!("Server read"); - let s = SecretString::from_str( - std::str::from_utf8(bytes.expose_secret()).map_err(Error::LinkNotUtf8)?, - ) - .expect("Infallible"); - - Ok(s) + Ok(bytes) } } diff --git a/rust/gui-client/src-tauri/src/client/gui.rs b/rust/gui-client/src-tauri/src/client/gui.rs index 6c65d08cb0..358afc5451 100644 --- a/rust/gui-client/src-tauri/src/client/gui.rs +++ b/rust/gui-client/src-tauri/src/client/gui.rs @@ -395,11 +395,15 @@ async fn check_for_updates(ctlr_tx: CtlrTx, always_show_update_notification: boo /// * `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: CtlrTx) -> Result<()> { loop { - if let Ok(url) = server.accept().await { + match server.accept().await { + Ok(bytes) => { + let url = SecretString::from_str(std::str::from_utf8(bytes.expose_secret()).context("Incoming deep link was not valid UTF-8")?).context("Impossible: can't wrap String into SecretString")?; + // Ignore errors from this, it would only happen if the app is shutting down, otherwise we would wait ctlr_tx .send(ControllerRequest::SchemeRequest(url)) .await - .ok(); + .ok();} + Err(error) => tracing::error!(?error, "error while accepting deep link"), } // We re-create the named pipe server every time we get a link, because of an oddity in the Windows API. server = deep_link::Server::new()?; @@ -647,7 +651,7 @@ impl Controller { if let Some(req) = self.auth.start_sign_in()? { let url = req.to_url(&self.advanced_settings.auth_base_url); self.refresh_system_tray_menu()?; - tauri::api::shell::open(&self.app.shell_scope(), url.expose_secret(), None)?; + os::open_url(&self.app, &url)?; } } Req::SystemTrayMenu(TrayMenuEvent::SignOut) => { diff --git a/rust/gui-client/src-tauri/src/client/gui/os_linux.rs b/rust/gui-client/src-tauri/src/client/gui/os_linux.rs index e56a4187b1..4270eb3d90 100644 --- a/rust/gui-client/src-tauri/src/client/gui/os_linux.rs +++ b/rust/gui-client/src-tauri/src/client/gui/os_linux.rs @@ -1,4 +1,26 @@ use super::{ControllerRequest, CtlrTx, Error}; +use anyhow::Result; +use secrecy::{ExposeSecret, SecretString}; + +/// Open a URL in the user's default browser +pub(crate) fn open_url(_app: &tauri::AppHandle, url: &SecretString) -> Result<()> { + // This is silly but it might work. + // Figure out who we were before `sudo` + let username = std::env::var("SUDO_USER")?; + std::process::Command::new("sudo") + // Make sure `XDG_RUNTIME_DIR` is preserved so we can find the Unix domain socket again + .arg("--preserve-env") + // Sudo back to our original username + .arg("--user") + .arg(username) + // Use the XDG wrapper for the user's default web browser (this will leak an `xdg-open` process if the user's first browser tab is the Firezone login. That process will end when the browser closes + .arg("xdg-open") + .arg(url.expose_secret()) + // Detach, since web browsers, and therefore xdg-open, typically block if there are no windows open and you're opening the first tab from CLI + .spawn()?; + + Ok(()) +} /// Show a notification in the bottom right of the screen pub(crate) fn show_notification(_title: &str, _body: &str) -> Result<(), Error> { diff --git a/rust/gui-client/src-tauri/src/client/gui/os_windows.rs b/rust/gui-client/src-tauri/src/client/gui/os_windows.rs index 0b8311c5fe..00a9dd9c13 100644 --- a/rust/gui-client/src-tauri/src/client/gui/os_windows.rs +++ b/rust/gui-client/src-tauri/src/client/gui/os_windows.rs @@ -1,5 +1,13 @@ use super::{ControllerRequest, CtlrTx, Error}; +use anyhow::Result; use connlib_shared::BUNDLE_ID; +use secrecy::{ExposeSecret, SecretString}; + +/// Open a URL in the user's default browser +pub(crate) fn open_url(app: &tauri::AppHandle, url: &SecretString) -> Result<()> { + tauri::api::shell::open(app.shell_scope(), url.expose_secret(), None)?; + Ok(()) +} /// Show a notification in the bottom right of the screen /// diff --git a/rust/gui-client/src-tauri/src/client/resolvers.rs b/rust/gui-client/src-tauri/src/client/resolvers.rs index 3f45823840..97477c1351 100644 --- a/rust/gui-client/src-tauri/src/client/resolvers.rs +++ b/rust/gui-client/src-tauri/src/client/resolvers.rs @@ -11,7 +11,8 @@ pub enum Error { #[cfg(target_os = "linux")] pub fn get() -> Result, Error> { - todo!() + tracing::error!("Resolvers module not yet implemented for Linux, returning empty Vec"); + Ok(Vec::default()) } #[cfg(target_os = "macos")] From 54d96c8c03dc72d89d9e4c1d5a75b35f92980242 Mon Sep 17 00:00:00 2001 From: Not Applicable Date: Tue, 12 Mar 2024 10:52:00 -0500 Subject: [PATCH 03/17] fix compiling on macos --- .../src-tauri/src/client/deep_link/macos.rs | 12 ++++++------ rust/gui-client/src-tauri/src/client/gui/os_macos.rs | 7 +++++++ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/rust/gui-client/src-tauri/src/client/deep_link/macos.rs b/rust/gui-client/src-tauri/src/client/deep_link/macos.rs index a3a88ee983..fd1ed0cbdd 100644 --- a/rust/gui-client/src-tauri/src/client/deep_link/macos.rs +++ b/rust/gui-client/src-tauri/src/client/deep_link/macos.rs @@ -1,26 +1,26 @@ //! Placeholder -use super::Error; -use secrecy::{Secret, SecretString}; +use anyhow::Result; +use secrecy::Secret; pub(crate) struct Server {} impl Server { - pub(crate) fn new() -> Result { + pub(crate) fn new() -> Result { tracing::warn!("This is not the actual Mac client"); tracing::trace!(scheme = super::FZ_SCHEME, "prevents dead code warning"); Ok(Self {}) } - pub(crate) async fn accept(self) -> Result { + pub(crate) async fn accept(self) -> Result>> { futures::future::pending().await } } -pub(crate) async fn open(_url: &url::Url) -> Result<(), Error> { +pub(crate) async fn open(_url: &url::Url) -> Result<()> { Ok(()) } -pub(crate) fn register() -> Result<(), Error> { +pub(crate) fn register() -> Result<()> { Ok(()) } diff --git a/rust/gui-client/src-tauri/src/client/gui/os_macos.rs b/rust/gui-client/src-tauri/src/client/gui/os_macos.rs index bb5125171a..467aff9ea4 100644 --- a/rust/gui-client/src-tauri/src/client/gui/os_macos.rs +++ b/rust/gui-client/src-tauri/src/client/gui/os_macos.rs @@ -1,6 +1,13 @@ //! This file is a stub only to do Tauri UI dev natively on a Mac. +use anyhow::Result; +use secrecy::SecretString; use super::{ControllerRequest, CtlrTx, Error}; +pub(crate) fn open_url(_app: &tauri::AppHandle, _url: &SecretString) -> +Result<()> { + unimplemented!() +} + /// Show a notification in the bottom right of the screen pub(crate) fn show_notification(_title: &str, _body: &str) -> Result<(), Error> { unimplemented!() From 3b1c9dd7fd9e8efbdee3b4785dc29562899a0424 Mon Sep 17 00:00:00 2001 From: Not Applicable Date: Tue, 12 Mar 2024 10:57:17 -0500 Subject: [PATCH 04/17] fix windows compile --- rust/gui-client/src-tauri/src/client/deep_link.rs | 3 --- rust/gui-client/src-tauri/src/client/gui/os_windows.rs | 3 ++- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/rust/gui-client/src-tauri/src/client/deep_link.rs b/rust/gui-client/src-tauri/src/client/deep_link.rs index 3077c343af..10640c2888 100644 --- a/rust/gui-client/src-tauri/src/client/deep_link.rs +++ b/rust/gui-client/src-tauri/src/client/deep_link.rs @@ -36,9 +36,6 @@ pub enum Error { /// Something went wrong finding the path to our own exe #[error(transparent)] CurrentExe(io::Error), - /// We got some data but it's not UTF-8 - #[error(transparent)] - LinkNotUtf8(std::str::Utf8Error), #[cfg(target_os = "windows")] #[error("Couldn't set up security descriptor for deep link server")] SecurityDescriptor, diff --git a/rust/gui-client/src-tauri/src/client/gui/os_windows.rs b/rust/gui-client/src-tauri/src/client/gui/os_windows.rs index 00a9dd9c13..88b7cc5ad1 100644 --- a/rust/gui-client/src-tauri/src/client/gui/os_windows.rs +++ b/rust/gui-client/src-tauri/src/client/gui/os_windows.rs @@ -2,10 +2,11 @@ use super::{ControllerRequest, CtlrTx, Error}; use anyhow::Result; use connlib_shared::BUNDLE_ID; use secrecy::{ExposeSecret, SecretString}; +use tauri::Manager; /// Open a URL in the user's default browser pub(crate) fn open_url(app: &tauri::AppHandle, url: &SecretString) -> Result<()> { - tauri::api::shell::open(app.shell_scope(), url.expose_secret(), None)?; + tauri::api::shell::open(&app.shell_scope(), url.expose_secret(), None)?; Ok(()) } From 07a7333ffce1190d08586f453c3f4161d3d9c3d3 Mon Sep 17 00:00:00 2001 From: _ Date: Tue, 12 Mar 2024 16:01:50 +0000 Subject: [PATCH 05/17] cargo fmt pass --- .../src-tauri/src/client/deep_link.rs | 11 +++--- .../src-tauri/src/client/deep_link/linux.rs | 37 +++++++++++-------- rust/gui-client/src-tauri/src/client/gui.rs | 17 ++++++--- .../src-tauri/src/client/gui/os_linux.rs | 2 +- .../src-tauri/src/client/gui/os_macos.rs | 7 ++-- 5 files changed, 42 insertions(+), 32 deletions(-) diff --git a/rust/gui-client/src-tauri/src/client/deep_link.rs b/rust/gui-client/src-tauri/src/client/deep_link.rs index 10640c2888..66186e313d 100644 --- a/rust/gui-client/src-tauri/src/client/deep_link.rs +++ b/rust/gui-client/src-tauri/src/client/deep_link.rs @@ -1,7 +1,7 @@ //! A module for registering, catching, and parsing deep links that are sent over to the app's already-running instance -use anyhow::{bail, Context, Result}; use crate::client::auth::Response as AuthResponse; +use anyhow::{bail, Context, Result}; use secrecy::{ExposeSecret, SecretString}; use std::io; use url::Url; @@ -116,14 +116,13 @@ mod tests { assert_eq!(actual.fragment.expose_secret(), "a_very_secret_string"); assert_eq!(actual.state.expose_secret(), "a_less_secret_string"); - let input = "firezone-fd0020211111://handle_client_sign_in_callback?account_name=Firezone&account_slug=firezone&actor_name=Reactor+Scram&fragment=a_very_secret_string&identity_provider_identifier=1234&state=a_less_secret_string"; let actual = parse_callback_wrapper(input)?; - + assert_eq!(actual.actor_name, "Reactor Scram"); assert_eq!(actual.fragment.expose_secret(), "a_very_secret_string"); assert_eq!(actual.state.expose_secret(), "a_less_secret_string"); - + // Empty string "" `actor_name` is fine let input = "firezone://handle_client_sign_in_callback/?actor_name=&fragment=&state=&identity_provider_identifier=12345"; let actual = parse_callback_wrapper(input)?; @@ -155,7 +154,7 @@ mod tests { fn parse_callback_wrapper(s: &str) -> Result { super::parse_auth_callback(&SecretString::new(s.to_owned())) } - + /// Tests the named pipe or Unix domain socket, doesn't test the URI scheme itself /// /// Will fail if any other Firezone Client instance is running @@ -170,7 +169,7 @@ mod tests { let id = uuid::Uuid::new_v4().to_string(); let expected_url = url::Url::parse(&format!("bogus-test-schema://{id}"))?; super::open(&expected_url).await?; - + let bytes = server_task.await??; let s = std::str::from_utf8(bytes.expose_secret())?; let url = url::Url::parse(s)?; diff --git a/rust/gui-client/src-tauri/src/client/deep_link/linux.rs b/rust/gui-client/src-tauri/src/client/deep_link/linux.rs index c18aa761ef..1f41ff0bac 100644 --- a/rust/gui-client/src-tauri/src/client/deep_link/linux.rs +++ b/rust/gui-client/src-tauri/src/client/deep_link/linux.rs @@ -1,7 +1,10 @@ -use anyhow::{bail, Context, Result}; use crate::client::known_dirs; +use anyhow::{bail, Context, Result}; use secrecy::{ExposeSecret, Secret}; -use tokio::{io::{AsyncReadExt, AsyncWriteExt}, net::{UnixListener, UnixStream}}; +use tokio::{ + io::{AsyncReadExt, AsyncWriteExt}, + net::{UnixListener, UnixStream}, +}; const SOCK_NAME: &str = "deep_link.sock"; @@ -17,9 +20,9 @@ impl Server { // TODO: This breaks single instance. Can we enforce it some other way? std::fs::remove_file(&path).ok(); std::fs::create_dir_all(&dir).context("Can't create dir for deep link socket")?; - + let listener = UnixListener::bind(&path).context("Couldn't bind listener Unix socket")?; - + // Figure out who we were before `sudo`, if using sudo if let Ok(username) = std::env::var("SUDO_USER") { std::process::Command::new("chown") @@ -27,39 +30,43 @@ impl Server { .arg(&path) .status()?; } - - Ok(Self { - listener, - }) + + Ok(Self { listener }) } /// Await one incoming deep link - /// + /// /// To match the Windows API, this consumes the `Server`. pub(crate) async fn accept(self) -> Result>> { tracing::debug!("deep_link::accept"); let (mut stream, _) = self.listener.accept().await?; tracing::debug!("Accepted Unix domain socket connection"); - + // TODO: Limit reads to 4,096 bytes. Partial reads will probably never happen // since it's a local socket transferring very small data. let mut bytes = vec![]; - stream.read_to_end(&mut bytes).await.context("failed to read incoming deep link over Unix socket stream")?; + stream + .read_to_end(&mut bytes) + .await + .context("failed to read incoming deep link over Unix socket stream")?; let bytes = Secret::new(bytes); - tracing::debug!(len = bytes.expose_secret().len(), "Got data from Unix domain socket"); + tracing::debug!( + len = bytes.expose_secret().len(), + "Got data from Unix domain socket" + ); Ok(bytes) } } pub(crate) async fn open(url: &url::Url) -> Result<()> { crate::client::logging::debug_command_setup()?; - + let dir = known_dirs::runtime().context("deep_link::open couldn't find runtime dir")?; let path = dir.join(SOCK_NAME); let mut stream = UnixStream::connect(&path).await?; - + stream.write_all(url.to_string().as_bytes()).await?; - + Ok(()) } diff --git a/rust/gui-client/src-tauri/src/client/gui.rs b/rust/gui-client/src-tauri/src/client/gui.rs index 358afc5451..9d0322ff59 100644 --- a/rust/gui-client/src-tauri/src/client/gui.rs +++ b/rust/gui-client/src-tauri/src/client/gui.rs @@ -397,12 +397,17 @@ async fn accept_deep_links(mut server: deep_link::Server, ctlr_tx: CtlrTx) -> Re loop { match server.accept().await { Ok(bytes) => { - let url = SecretString::from_str(std::str::from_utf8(bytes.expose_secret()).context("Incoming deep link was not valid UTF-8")?).context("Impossible: can't wrap String into SecretString")?; - // Ignore errors from this, it would only happen if the app is shutting down, otherwise we would wait - ctlr_tx - .send(ControllerRequest::SchemeRequest(url)) - .await - .ok();} + let url = SecretString::from_str( + std::str::from_utf8(bytes.expose_secret()) + .context("Incoming deep link was not valid UTF-8")?, + ) + .context("Impossible: can't wrap String into SecretString")?; + // Ignore errors from this, it would only happen if the app is shutting down, otherwise we would wait + ctlr_tx + .send(ControllerRequest::SchemeRequest(url)) + .await + .ok(); + } Err(error) => tracing::error!(?error, "error while accepting deep link"), } // We re-create the named pipe server every time we get a link, because of an oddity in the Windows API. diff --git a/rust/gui-client/src-tauri/src/client/gui/os_linux.rs b/rust/gui-client/src-tauri/src/client/gui/os_linux.rs index 4270eb3d90..037505a9d7 100644 --- a/rust/gui-client/src-tauri/src/client/gui/os_linux.rs +++ b/rust/gui-client/src-tauri/src/client/gui/os_linux.rs @@ -18,7 +18,7 @@ pub(crate) fn open_url(_app: &tauri::AppHandle, url: &SecretString) -> Result<() .arg(url.expose_secret()) // Detach, since web browsers, and therefore xdg-open, typically block if there are no windows open and you're opening the first tab from CLI .spawn()?; - + Ok(()) } diff --git a/rust/gui-client/src-tauri/src/client/gui/os_macos.rs b/rust/gui-client/src-tauri/src/client/gui/os_macos.rs index 467aff9ea4..18488facda 100644 --- a/rust/gui-client/src-tauri/src/client/gui/os_macos.rs +++ b/rust/gui-client/src-tauri/src/client/gui/os_macos.rs @@ -1,11 +1,10 @@ //! This file is a stub only to do Tauri UI dev natively on a Mac. +use super::{ControllerRequest, CtlrTx, Error}; use anyhow::Result; use secrecy::SecretString; -use super::{ControllerRequest, CtlrTx, Error}; -pub(crate) fn open_url(_app: &tauri::AppHandle, _url: &SecretString) -> -Result<()> { - unimplemented!() +pub(crate) fn open_url(_app: &tauri::AppHandle, _url: &SecretString) -> Result<()> { + unimplemented!() } /// Show a notification in the bottom right of the screen From 30ccd0c7155138ed3deb610f1673eb9f89c957a8 Mon Sep 17 00:00:00 2001 From: Not Applicable Date: Tue, 12 Mar 2024 11:19:45 -0500 Subject: [PATCH 06/17] remove unused error variants --- .../src-tauri/src/client/deep_link.rs | 22 +------------- .../src-tauri/src/client/deep_link/linux.rs | 4 ++- .../src-tauri/src/client/deep_link/windows.rs | 29 ++++++++++--------- rust/gui-client/src-tauri/src/client/gui.rs | 2 ++ 4 files changed, 21 insertions(+), 36 deletions(-) diff --git a/rust/gui-client/src-tauri/src/client/deep_link.rs b/rust/gui-client/src-tauri/src/client/deep_link.rs index 66186e313d..9308a0a003 100644 --- a/rust/gui-client/src-tauri/src/client/deep_link.rs +++ b/rust/gui-client/src-tauri/src/client/deep_link.rs @@ -3,7 +3,6 @@ use crate::client::auth::Response as AuthResponse; use anyhow::{bail, Context, Result}; use secrecy::{ExposeSecret, SecretString}; -use std::io; use url::Url; pub(crate) const FZ_SCHEME: &str = "firezone-fd0020211111"; @@ -27,27 +26,8 @@ mod imp; pub enum Error { #[error("named pipe server couldn't start listening, we are probably the second instance")] CantListen, - /// Error from client's POV #[error(transparent)] - ClientCommunications(io::Error), - /// Error while connecting to the server - #[error(transparent)] - Connect(io::Error), - /// Something went wrong finding the path to our own exe - #[error(transparent)] - CurrentExe(io::Error), - #[cfg(target_os = "windows")] - #[error("Couldn't set up security descriptor for deep link server")] - SecurityDescriptor, - /// Error from server's POV - #[error(transparent)] - ServerCommunications(io::Error), - #[error(transparent)] - UrlParse(#[from] url::ParseError), - /// Something went wrong setting up the registry - #[cfg(target_os = "windows")] - #[error(transparent)] - WindowsRegistry(io::Error), + Other(#[from] anyhow::Error), } pub(crate) use imp::{open, register, Server}; diff --git a/rust/gui-client/src-tauri/src/client/deep_link/linux.rs b/rust/gui-client/src-tauri/src/client/deep_link/linux.rs index 1f41ff0bac..4d4c9898e8 100644 --- a/rust/gui-client/src-tauri/src/client/deep_link/linux.rs +++ b/rust/gui-client/src-tauri/src/client/deep_link/linux.rs @@ -14,7 +14,9 @@ pub(crate) struct Server { impl Server { /// Create a new deep link server to make sure we're the only instance - pub(crate) fn new() -> Result { + /// + /// Still uses `thiserror` so we can catch the deep_link `CantListen` error + pub(crate) fn new() -> Result { let dir = known_dirs::runtime().context("couldn't find runtime dir")?; let path = dir.join(SOCK_NAME); // TODO: This breaks single instance. Can we enforce it some other way? diff --git a/rust/gui-client/src-tauri/src/client/deep_link/windows.rs b/rust/gui-client/src-tauri/src/client/deep_link/windows.rs index fdb4127eac..1d030c1d77 100644 --- a/rust/gui-client/src-tauri/src/client/deep_link/windows.rs +++ b/rust/gui-client/src-tauri/src/client/deep_link/windows.rs @@ -1,8 +1,8 @@ //! A module for registering, catching, and parsing deep links that are sent over to the app's already-running instance //! Based on reading some of the Windows code from , which is licensed "MIT OR Apache-2.0" -use super::{Error, FZ_SCHEME}; -use anyhow::Result; +use super::FZ_SCHEME; +use anyhow::{Context, Result}; use connlib_shared::BUNDLE_ID; use secrecy::Secret; use std::{ffi::c_void, io, path::Path}; @@ -19,7 +19,8 @@ impl Server { /// Construct a server, but don't await client connections yet /// /// Panics if there is no Tokio runtime - pub(crate) fn new() -> Result { + /// Still uses `thiserror` so we can catch the deep_link `CantListen` error + pub(crate) fn new() -> Result { // This isn't air-tight - We recreate the whole server on each loop, // rather than binding 1 socket and accepting many streams like a normal socket API. // I can only assume Tokio is following Windows' underlying API. @@ -43,9 +44,9 @@ impl Server { psd, windows::Win32::System::SystemServices::SECURITY_DESCRIPTOR_REVISION, ) - .map_err(|_| Error::SecurityDescriptor)?; + .context("InitializeSecurityDescriptor failed")?; WinSec::SetSecurityDescriptorDacl(psd, true, None, false) - .map_err(|_| Error::SecurityDescriptor)?; + .context("SetSecurityDescriptorDacl failed")?; } let mut sa = WinSec::SECURITY_ATTRIBUTES { @@ -65,7 +66,7 @@ impl Server { // or lifetime problems because we only pass pointers to our local vars to // Win32, and Win32 shouldn't save them anywhere. let server = unsafe { server_options.create_with_security_attributes_raw(path, sa_ptr) } - .map_err(|_| Error::CantListen)?; + .map_err(|_| super::Error::CantListen)?; tracing::debug!("server is bound"); Ok(Server { inner: server }) @@ -80,7 +81,7 @@ impl Server { self.inner .connect() .await - .map_err(Error::ServerCommunications)?; + .context("Couldn't accept connection from named pipe client")?; tracing::debug!("server got connection"); // TODO: Limit the read size here. Our typical callback is 350 bytes, so 4,096 bytes should be more than enough. @@ -90,7 +91,7 @@ impl Server { self.inner .read_to_end(&mut bytes) .await - .map_err(Error::ServerCommunications)?; + .context("Couldn't read bytes from named pipe client")?; let bytes = Secret::new(bytes); self.inner.disconnect().ok(); @@ -99,15 +100,15 @@ impl Server { } /// Open a deep link by sending it to the already-running instance of the app -pub async fn open(url: &url::Url) -> Result<(), Error> { +pub async fn open(url: &url::Url) -> Result<()> { let path = named_pipe_path(BUNDLE_ID); let mut client = named_pipe::ClientOptions::new() .open(path) - .map_err(Error::Connect)?; + .context("Couldn't connect to named pipe server")?; client .write_all(url.as_str().as_bytes()) .await - .map_err(Error::ClientCommunications)?; + .context("Couldn't write bytes to named pipe server")?; Ok(()) } @@ -115,14 +116,14 @@ pub async fn open(url: &url::Url) -> Result<(), Error> { /// /// This is copied almost verbatim from tauri-plugin-deep-link's `register` fn, with an improvement /// that we send the deep link to a subcommand so the URL won't confuse `clap` -pub fn register() -> Result<(), Error> { +pub fn register() -> Result<()> { let exe = tauri_utils::platform::current_exe() - .map_err(Error::CurrentExe)? + .context("Can't find our own exe path")? .display() .to_string() .replace("\\\\?\\", ""); - set_registry_values(BUNDLE_ID, &exe).map_err(Error::WindowsRegistry)?; + set_registry_values(BUNDLE_ID, &exe).context("Can't set Windows Registry values")?; Ok(()) } diff --git a/rust/gui-client/src-tauri/src/client/gui.rs b/rust/gui-client/src-tauri/src/client/gui.rs index 9d0322ff59..7c62f2e71b 100644 --- a/rust/gui-client/src-tauri/src/client/gui.rs +++ b/rust/gui-client/src-tauri/src/client/gui.rs @@ -92,6 +92,8 @@ pub(crate) enum Error { } /// Runs the Tauri GUI and returns on exit or unrecoverable error +/// +/// Still uses `thiserror` so we can catch the deep_link `CantListen` error pub(crate) fn run(cli: &client::Cli) -> Result<(), Error> { let advanced_settings = settings::load_advanced_settings().unwrap_or_default(); From f486bd5c847ace9fbb3c2a364b44d61ebeb95148 Mon Sep 17 00:00:00 2001 From: _ Date: Tue, 12 Mar 2024 16:22:00 +0000 Subject: [PATCH 07/17] fix --- rust/gui-client/src-tauri/src/client/deep_link/linux.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rust/gui-client/src-tauri/src/client/deep_link/linux.rs b/rust/gui-client/src-tauri/src/client/deep_link/linux.rs index 4d4c9898e8..81da1a1981 100644 --- a/rust/gui-client/src-tauri/src/client/deep_link/linux.rs +++ b/rust/gui-client/src-tauri/src/client/deep_link/linux.rs @@ -27,10 +27,13 @@ impl Server { // Figure out who we were before `sudo`, if using sudo if let Ok(username) = std::env::var("SUDO_USER") { + // chown so that when the non-privileged browser launches us, + // we can send a message to our privileged main process std::process::Command::new("chown") .arg(username) .arg(&path) - .status()?; + .status() + .context("couldn't chown Unix domain socket")?; } Ok(Self { listener }) From 4c588ca640451d17d9de05e61c3b467a1e1c74dd Mon Sep 17 00:00:00 2001 From: _ Date: Tue, 12 Mar 2024 16:35:58 +0000 Subject: [PATCH 08/17] remove URL logging only needed in dev --- rust/gui-client/src-tauri/src/client/deep_link.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/rust/gui-client/src-tauri/src/client/deep_link.rs b/rust/gui-client/src-tauri/src/client/deep_link.rs index 9308a0a003..9e829596a8 100644 --- a/rust/gui-client/src-tauri/src/client/deep_link.rs +++ b/rust/gui-client/src-tauri/src/client/deep_link.rs @@ -33,8 +33,6 @@ pub enum Error { pub(crate) use imp::{open, register, Server}; pub(crate) fn parse_auth_callback(url: &SecretString) -> Result { - // TODO: Delete this before opening PR - tracing::debug!(secret = url.expose_secret(), "Parsing URL"); let url = Url::parse(url.expose_secret())?; if Some(url::Host::Domain("handle_client_sign_in_callback")) != url.host() { bail!("URL host should be `handle_client_sign_in_callback`"); From c1f465b4c4c39e16f99020c6874911c234f663f8 Mon Sep 17 00:00:00 2001 From: _ Date: Tue, 12 Mar 2024 17:08:34 +0000 Subject: [PATCH 09/17] try to fix smoke test --- scripts/tests/smoke-test-gui-linux.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/tests/smoke-test-gui-linux.sh b/scripts/tests/smoke-test-gui-linux.sh index 17111264aa..ec444b7f6a 100755 --- a/scripts/tests/smoke-test-gui-linux.sh +++ b/scripts/tests/smoke-test-gui-linux.sh @@ -14,6 +14,11 @@ PACKAGE=firezone-gui-client export RUST_LOG=firezone_gui_client=debug,warn export WEBKIT_DISABLE_COMPOSITING_MODE=1 +# Contains `xdg-desktop-menu` for deep link registration. +# `ubuntu-desktop` has this by default. +sudo apt-get install xdg-utils +which xdg-desktop-menu + cargo build -p "$PACKAGE" function smoke_test() { From 9ff8c2838af6413b33e70ba380a021d009b9c646 Mon Sep 17 00:00:00 2001 From: _ Date: Tue, 12 Mar 2024 17:16:34 +0000 Subject: [PATCH 10/17] fix smoke test --- rust/gui-client/src-tauri/src/client/gui.rs | 22 +++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/rust/gui-client/src-tauri/src/client/gui.rs b/rust/gui-client/src-tauri/src/client/gui.rs index 7c62f2e71b..10e8f260a0 100644 --- a/rust/gui-client/src-tauri/src/client/gui.rs +++ b/rust/gui-client/src-tauri/src/client/gui.rs @@ -154,6 +154,11 @@ pub(crate) fn run(cli: &client::Cli) -> Result<(), Error> { } }); + // Make sure we're single-instance + // We register our deep links to call the `open-deep-link` subcommand, + // so if we're at this point, we know we've been launched manually + let server = deep_link::Server::new()?; + if let Some(client::Cmd::SmokeTest) = &cli.command { let ctlr_tx = ctlr_tx.clone(); tokio::spawn(async move { @@ -163,16 +168,13 @@ pub(crate) fn run(cli: &client::Cli) -> Result<(), Error> { } }); } - - // Make sure we're single-instance - // We register our deep links to call the `open-deep-link` subcommand, - // so if we're at this point, we know we've been launched manually - let server = deep_link::Server::new()?; - - // We know now we're the only instance on the computer, so register our exe - // to handle deep links - deep_link::register().context("Failed to register deep link handler")?; - tokio::spawn(accept_deep_links(server, ctlr_tx.clone())); + else { + // Don't register deep links for smoke tests yet + // The single-instance check is done, so register our exe + // to handle deep links + deep_link::register().context("Failed to register deep link handler")?; + tokio::spawn(accept_deep_links(server, ctlr_tx.clone())); + } let managed = Managed { ctlr_tx: ctlr_tx.clone(), From b1d79ffdcf280db3935353e18ba14f4afe4e947d Mon Sep 17 00:00:00 2001 From: _ Date: Tue, 12 Mar 2024 17:19:29 +0000 Subject: [PATCH 11/17] fmt --- rust/gui-client/src-tauri/src/client/gui.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rust/gui-client/src-tauri/src/client/gui.rs b/rust/gui-client/src-tauri/src/client/gui.rs index 10e8f260a0..ceca09ccd5 100644 --- a/rust/gui-client/src-tauri/src/client/gui.rs +++ b/rust/gui-client/src-tauri/src/client/gui.rs @@ -167,8 +167,7 @@ pub(crate) fn run(cli: &client::Cli) -> Result<(), Error> { std::process::exit(1); } }); - } - else { + } else { // Don't register deep links for smoke tests yet // The single-instance check is done, so register our exe // to handle deep links From 2e1c1cc7e82821b262c3dd7297d5d38f3a5986bd Mon Sep 17 00:00:00 2001 From: _ Date: Tue, 12 Mar 2024 17:31:11 +0000 Subject: [PATCH 12/17] disable deep links during CI --- rust/gui-client/src-tauri/src/client.rs | 3 +++ rust/gui-client/src-tauri/src/client/gui.rs | 5 +++-- scripts/tests/smoke-test-gui-linux.sh | 4 ++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/rust/gui-client/src-tauri/src/client.rs b/rust/gui-client/src-tauri/src/client.rs index c65ea2e82a..b98a3ddc91 100644 --- a/rust/gui-client/src-tauri/src/client.rs +++ b/rust/gui-client/src-tauri/src/client.rs @@ -163,6 +163,9 @@ struct Cli { /// If true, show a fake update notification that opens the Firezone release page when clicked #[arg(long, hide = true)] test_update_notification: bool, + /// Disable deep link registration and handling, for headless CI environments + #[arg(long, hide = true)] + no_deep_links: bool, } impl Cli { diff --git a/rust/gui-client/src-tauri/src/client/gui.rs b/rust/gui-client/src-tauri/src/client/gui.rs index ceca09ccd5..c5b2767308 100644 --- a/rust/gui-client/src-tauri/src/client/gui.rs +++ b/rust/gui-client/src-tauri/src/client/gui.rs @@ -167,8 +167,9 @@ pub(crate) fn run(cli: &client::Cli) -> Result<(), Error> { std::process::exit(1); } }); - } else { - // Don't register deep links for smoke tests yet + } + + if ! cli.no_deep_links { // The single-instance check is done, so register our exe // to handle deep links deep_link::register().context("Failed to register deep link handler")?; diff --git a/scripts/tests/smoke-test-gui-linux.sh b/scripts/tests/smoke-test-gui-linux.sh index ec444b7f6a..5ec96503af 100755 --- a/scripts/tests/smoke-test-gui-linux.sh +++ b/scripts/tests/smoke-test-gui-linux.sh @@ -41,7 +41,7 @@ function smoke_test() { sudo stat "$DEVICE_ID_PATH" # Run the test again and make sure the device ID is not changed - sudo --preserve-env xvfb-run --auto-servernum ../target/debug/"$PACKAGE" smoke-test + sudo --preserve-env xvfb-run --auto-servernum ../target/debug/"$PACKAGE" smoke-test --no-deep-links DEVICE_ID_2=$(cat "$DEVICE_ID_PATH") if [ "$DEVICE_ID_1" != "$DEVICE_ID_2" ] @@ -61,7 +61,7 @@ function crash_test() { sudo rm -f "$DUMP_PATH" # Fail if it returns success, this is supposed to crash - sudo --preserve-env xvfb-run --auto-servernum ../target/debug/"$PACKAGE" --crash && exit 1 + sudo --preserve-env xvfb-run --auto-servernum ../target/debug/"$PACKAGE" --crash --no-deep-links && exit 1 # Fail if the crash file wasn't written sudo stat "$DUMP_PATH" From cbeace1622d3ca5911064558b0cb57a8c3eec28b Mon Sep 17 00:00:00 2001 From: _ Date: Tue, 12 Mar 2024 17:56:01 +0000 Subject: [PATCH 13/17] fix script --- rust/gui-client/src-tauri/src/client/gui.rs | 2 +- scripts/tests/smoke-test-gui-linux.sh | 7 +------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/rust/gui-client/src-tauri/src/client/gui.rs b/rust/gui-client/src-tauri/src/client/gui.rs index e5a05cc4bd..2d8b217356 100644 --- a/rust/gui-client/src-tauri/src/client/gui.rs +++ b/rust/gui-client/src-tauri/src/client/gui.rs @@ -169,7 +169,7 @@ pub(crate) fn run(cli: &client::Cli) -> Result<(), Error> { }); } - if ! cli.no_deep_links { + if !cli.no_deep_links { // The single-instance check is done, so register our exe // to handle deep links deep_link::register().context("Failed to register deep link handler")?; diff --git a/scripts/tests/smoke-test-gui-linux.sh b/scripts/tests/smoke-test-gui-linux.sh index 5ec96503af..7340b5f8ba 100755 --- a/scripts/tests/smoke-test-gui-linux.sh +++ b/scripts/tests/smoke-test-gui-linux.sh @@ -14,11 +14,6 @@ PACKAGE=firezone-gui-client export RUST_LOG=firezone_gui_client=debug,warn export WEBKIT_DISABLE_COMPOSITING_MODE=1 -# Contains `xdg-desktop-menu` for deep link registration. -# `ubuntu-desktop` has this by default. -sudo apt-get install xdg-utils -which xdg-desktop-menu - cargo build -p "$PACKAGE" function smoke_test() { @@ -41,7 +36,7 @@ function smoke_test() { sudo stat "$DEVICE_ID_PATH" # Run the test again and make sure the device ID is not changed - sudo --preserve-env xvfb-run --auto-servernum ../target/debug/"$PACKAGE" smoke-test --no-deep-links + sudo --preserve-env xvfb-run --auto-servernum ../target/debug/"$PACKAGE" --no-deep-links smoke-test DEVICE_ID_2=$(cat "$DEVICE_ID_PATH") if [ "$DEVICE_ID_1" != "$DEVICE_ID_2" ] From 597fdf59829e610cb63af99a3fa9199f3245a8c3 Mon Sep 17 00:00:00 2001 From: _ Date: Tue, 12 Mar 2024 19:24:17 +0000 Subject: [PATCH 14/17] add trace --- rust/gui-client/src-tauri/src/client/gui.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/gui-client/src-tauri/src/client/gui.rs b/rust/gui-client/src-tauri/src/client/gui.rs index 2d8b217356..742d055336 100644 --- a/rust/gui-client/src-tauri/src/client/gui.rs +++ b/rust/gui-client/src-tauri/src/client/gui.rs @@ -169,6 +169,7 @@ pub(crate) fn run(cli: &client::Cli) -> Result<(), Error> { }); } + tracing::debug!(cli.no_deep_links); if !cli.no_deep_links { // The single-instance check is done, so register our exe // to handle deep links From e22ebeca8b8ccfe54208dc3acb10b8804f24f1bc Mon Sep 17 00:00:00 2001 From: _ Date: Tue, 12 Mar 2024 19:27:28 +0000 Subject: [PATCH 15/17] yeah turns out I just missed a spot --- scripts/tests/smoke-test-gui-linux.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tests/smoke-test-gui-linux.sh b/scripts/tests/smoke-test-gui-linux.sh index 7340b5f8ba..d2fd4e14f3 100755 --- a/scripts/tests/smoke-test-gui-linux.sh +++ b/scripts/tests/smoke-test-gui-linux.sh @@ -23,7 +23,7 @@ function smoke_test() { sudo stat "$DEVICE_ID_PATH" && exit 1 # Run the smoke test normally - sudo --preserve-env xvfb-run --auto-servernum ../target/debug/"$PACKAGE" smoke-test + sudo --preserve-env xvfb-run --auto-servernum ../target/debug/"$PACKAGE" --no-deep-links smoke-test # Note the device ID DEVICE_ID_1=$(cat "$DEVICE_ID_PATH") From d97d0ddeb8788d413ecc76d49747390094c3368e Mon Sep 17 00:00:00 2001 From: _ Date: Tue, 12 Mar 2024 21:37:23 +0000 Subject: [PATCH 16/17] chore: comment --- rust/gui-client/src-tauri/src/client/gui/os_linux.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rust/gui-client/src-tauri/src/client/gui/os_linux.rs b/rust/gui-client/src-tauri/src/client/gui/os_linux.rs index 037505a9d7..08aaf21dba 100644 --- a/rust/gui-client/src-tauri/src/client/gui/os_linux.rs +++ b/rust/gui-client/src-tauri/src/client/gui/os_linux.rs @@ -5,6 +5,9 @@ use secrecy::{ExposeSecret, SecretString}; /// Open a URL in the user's default browser pub(crate) fn open_url(_app: &tauri::AppHandle, url: &SecretString) -> Result<()> { // This is silly but it might work. + // Once closes, + // just go back to using Tauri's shell open like we do on Windows. + // // Figure out who we were before `sudo` let username = std::env::var("SUDO_USER")?; std::process::Command::new("sudo") From 5257c98e03d4d790c4e0ac4b70d364d1eb42c0f1 Mon Sep 17 00:00:00 2001 From: _ Date: Wed, 13 Mar 2024 16:33:43 +0000 Subject: [PATCH 17/17] rename for clarity --- rust/gui-client/src-tauri/src/client/deep_link.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/gui-client/src-tauri/src/client/deep_link.rs b/rust/gui-client/src-tauri/src/client/deep_link.rs index 9e829596a8..3615268271 100644 --- a/rust/gui-client/src-tauri/src/client/deep_link.rs +++ b/rust/gui-client/src-tauri/src/client/deep_link.rs @@ -32,8 +32,8 @@ pub enum Error { pub(crate) use imp::{open, register, Server}; -pub(crate) fn parse_auth_callback(url: &SecretString) -> Result { - let url = Url::parse(url.expose_secret())?; +pub(crate) fn parse_auth_callback(url_secret: &SecretString) -> Result { + let url = Url::parse(url_secret.expose_secret())?; if Some(url::Host::Domain("handle_client_sign_in_callback")) != url.host() { bail!("URL host should be `handle_client_sign_in_callback`"); }