github-personal/firezone
1
0
Fork 0
mirror of https://github.com/outbackdingo/firezone.git synced 2026-01-27 10:18:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

chore(gui-client): show an error dialog if the IPC service isn't running (#5454)

Browse Source 
Closes #5377 

Windows
<img width="445" alt="image"
src="https://github.com/firezone/firezone/assets/13400041/d557cce1-e6c6-4a47-8645-777d1aa04f33">

KDE
<img width="521" alt="image"
src="https://github.com/firezone/firezone/assets/13400041/48b9d97a-e479-4d21-b717-230b0e27214e">

```[tasklist]
### Tasks
- [x] Fix broken smoke tests #5464
- [x] Linux manual sign-in
- [x] Linux auto sign-in
- [x] Windows manual sign-in
- [x] Window auto sign-in
- [x] Open for review
- [ ] Apply feedback
- [ ] Merge
```

---------

Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
This commit is contained in:
Reactor Scram
2024-06-25 21:07:43 +00:00
committed by GitHub
parent 4ffc49eef9
commit 144418f47c
8 changed files with 174 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
rust/gui-client/src-tauri/src/client.rs
View File

@@ -1,7 +1,6 @@
use anyhow::{bail, Context as _, Result};
use clap::{Args, Parser};
use connlib_client_shared::file_logger;
use firezone_headless_client::FIREZONE_GROUP;
use std::path::PathBuf;
use tracing::instrument;
use tracing_subscriber::EnvFilter;
@@ -74,7 +73,7 @@ pub(crate) fn run() -> Result<()> {
Ok(true) => run_gui(cli),
Ok(false) => bail!("The GUI should run as a normal user, not elevated"),
Err(error) => {
show_error_dialog(&error)?;
gui::show_error_dialog(&error)?;
Err(error.into())
}
}
@@ -125,7 +124,7 @@ fn run_gui(cli: Cli) -> Result<()> {
// Make sure errors get logged, at least to stderr
if let Err(error) = &result {
tracing::error!(?error, error_msg = %error);
show_error_dialog(error)?;
gui::show_error_dialog(error)?;
}
Ok(result?)
@@ -158,30 +157,6 @@ fn start_logging(directives: &str) -> Result<file_logger::Handle> {
Ok(logging_handles.logger)
}
#[instrument(skip_all)]
fn show_error_dialog(error: &gui::Error) -> Result<()> {
// Decision to put the error strings here: <https://github.com/firezone/firezone/pull/3464#discussion_r1473608415>
// This message gets shown to users in the GUI and could be localized, unlike
// messages in the log which only need to be used for `git grep`.
let user_friendly_error_msg = match error {
// TODO: Update this URL
gui::Error::WebViewNotInstalled => "Firezone cannot start because WebView2 is not installed. Follow the instructions at <https://www.firezone.dev/kb/user-guides/windows-client>.".to_string(),
gui::Error::DeepLink(deep_link::Error::CantListen) => "Firezone is already running. If it's not responding, force-stop it.".to_string(),
gui::Error::DeepLink(deep_link::Error::Other(error)) => error.to_string(),
gui::Error::Logging(_) => "Logging error".to_string(),
gui::Error::UserNotInFirezoneGroup => format!("You are not a member of the group `{FIREZONE_GROUP}`. Try `sudo usermod -aG {FIREZONE_GROUP} $USER` and then reboot"),
gui::Error::Other(error) => error.to_string(),
};
tracing::error!(?user_friendly_error_msg);
native_dialog::MessageDialog::new()
.set_title("Firezone Error")
.set_text(&user_friendly_error_msg)
.set_type(native_dialog::MessageType::Error)
.show_alert()?;
Ok(())
}
/// The debug / test flags like `crash_on_purpose` and `test_update_notification`
/// don't propagate when we use `RunAs` to elevate ourselves. So those must be run
/// from an admin terminal, or with "Run as administrator" in the right-click menu.

94
rust/gui-client/src-tauri/src/client/gui.rs
View File

@@ -10,7 +10,7 @@ use crate::client::{
settings::{self, AdvancedSettings},
Failure,
};
use anyhow::{bail, Context, Result};
use anyhow::{anyhow, bail, Context, Result};
use secrecy::{ExposeSecret, SecretString};
use std::{path::PathBuf, str::FromStr, sync::Arc, time::Duration};
use system_tray_menu::Event as TrayMenuEvent;
@@ -21,6 +21,7 @@ use url::Url;
use ControllerRequest as Req;
mod errors;
mod ran_before;
pub(crate) mod system_tray_menu;
@@ -40,6 +41,7 @@ mod os;
#[allow(clippy::unnecessary_wraps)]
mod os;
pub(crate) use errors::{show_error_dialog, Error};
pub(crate) use os::set_autostart;
pub(crate) type CtlrTx = mpsc::Sender<ControllerRequest>;
@@ -55,24 +57,6 @@ pub(crate) struct Managed {
pub inject_faults: bool,
}
// TODO: Replace with `anyhow` gradually per <https://github.com/firezone/firezone/pull/3546#discussion_r1477114789>
#[allow(dead_code)]
#[derive(Debug, thiserror::Error)]
pub(crate) enum Error {
#[error("Deep-link module error: {0}")]
DeepLink(#[from] deep_link::Error),
#[error("Logging module error: {0}")]
Logging(#[from] logging::Error),
// `client.rs` provides a more user-friendly message when showing the error dialog box for certain variants
#[error("UserNotInFirezoneGroup")]
UserNotInFirezoneGroup,
#[error("WebViewNotInstalled")]
WebViewNotInstalled,
#[error(transparent)]
Other(#[from] anyhow::Error),
}
/// Runs the Tauri GUI and returns on exit or unrecoverable error
///
/// Still uses `thiserror` so we can catch the deep_link `CantListen` error
@@ -236,6 +220,7 @@ pub(crate) fn run(
}
Ok(Err(error)) => {
tracing::error!(?error, "run_controller returned an error");
errors::show_error_dialog(&error).unwrap();
1
}
Ok(Ok(_)) => 0,
@@ -454,9 +439,9 @@ struct Session {
impl Controller {
// TODO: Figure out how re-starting sessions automatically will work
/// Pre-req: the auth module must be signed in
async fn start_session(&mut self, token: SecretString) -> Result<()> {
async fn start_session(&mut self, token: SecretString) -> Result<(), Error> {
if self.session.is_some() {
bail!("can't start session, we're already in a session");
Err(anyhow!("can't start session, we're already in a session"))?;
}
let callback_handler = CallbackHandler {
@@ -486,20 +471,21 @@ impl Controller {
Ok(())
}
async fn handle_deep_link(&mut self, url: &SecretString) -> Result<()> {
async fn handle_deep_link(&mut self, url: &SecretString) -> Result<(), Error> {
let auth_response =
client::deep_link::parse_auth_callback(url).context("Couldn't parse scheme request")?;
tracing::info!("Received deep link over IPC");
// Uses `std::fs`
let token = self.auth.handle_response(auth_response)?;
self.start_session(token)
.await
.context("Couldn't start connlib session")?;
let token = self
.auth
.handle_response(auth_response)
.context("Couldn't handle auth response")?;
self.start_session(token).await?;
Ok(())
}
async fn handle_request(&mut self, req: ControllerRequest) -> Result<()> {
async fn handle_request(&mut self, req: ControllerRequest) -> Result<(), Error> {
match req {
Req::ApplySettings(settings) => {
self.advanced_settings = settings;
@@ -529,36 +515,43 @@ impl Controller {
.set_title("Firezone Error")
.set_text(&error_msg)
.set_type(native_dialog::MessageType::Error)
.show_alert()?;
.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")?,
Req::Fail(_) => bail!("Impossible error: `Fail` should be handled before this"),
Req::Fail(_) => Err(anyhow!(
"Impossible error: `Fail` should be handled before this"
))?,
Req::GetAdvancedSettings(tx) => {
tx.send(self.advanced_settings.clone()).ok();
}
Req::SchemeRequest(url) => self
.handle_deep_link(&url)
.await
.context("Couldn't handle deep link")?,
Req::SchemeRequest(url) => self.handle_deep_link(&url).await?,
Req::SignIn | Req::SystemTrayMenu(TrayMenuEvent::SignIn) => {
if let Some(req) = self.auth.start_sign_in()? {
if let Some(req) = self
.auth
.start_sign_in()
.context("Couldn't start sign-in flow")?
{
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)?;
tauri::api::shell::open(&self.app.shell_scope(), url.expose_secret(), None)
.context("Couldn't open auth page")?;
self.app
.get_window("welcome")
.context("Couldn't get handle to Welcome window")?
.hide()?;
.hide()
.context("Couldn't hide Welcome window")?;
}
}
Req::SystemTrayMenu(TrayMenuEvent::AdminPortal) => tauri::api::shell::open(
&self.app.shell_scope(),
&self.advanced_settings.auth_base_url,
None,
)?,
)
.context("Couldn't open auth page")?,
Req::SystemTrayMenu(TrayMenuEvent::Copy(s)) => arboard::Clipboard::new()
.context("Couldn't access clipboard")?
.set_text(s)
@@ -595,11 +588,12 @@ impl Controller {
self.sign_out().await?;
}
Req::SystemTrayMenu(TrayMenuEvent::Url(url)) => {
tauri::api::shell::open(&self.app.shell_scope(), url, None)?
}
Req::SystemTrayMenu(TrayMenuEvent::Quit) => {
bail!("Impossible error: `Quit` should be handled before this")
tauri::api::shell::open(&self.app.shell_scope(), url, None)
.context("Couldn't open URL from system tray")?
}
Req::SystemTrayMenu(TrayMenuEvent::Quit) => Err(anyhow!(
"Impossible error: `Quit` should be handled before this"
))?,
Req::TunnelReady => {
if !self.tunnel_ready {
os::show_notification(
@@ -620,7 +614,8 @@ impl Controller {
}
Req::UpdateNotificationClicked(download_url) => {
tracing::info!("UpdateNotificationClicked in run_controller!");
tauri::api::shell::open(&self.app.shell_scope(), download_url, None)?;
tauri::api::shell::open(&self.app.shell_scope(), download_url, None)
.context("Couldn't open update page")?;
}
}
Ok(())
@@ -703,7 +698,7 @@ async fn run_controller(
ctlr_tx: CtlrTx,
mut rx: mpsc::Receiver<ControllerRequest>,
advanced_settings: AdvancedSettings,
) -> Result<()> {
) -> Result<(), Error> {
tracing::info!("Entered `run_controller`");
let mut controller = Controller {
advanced_settings,
@@ -721,10 +716,7 @@ async fn run_controller(
.token()
.context("Failed to load token from disk during app start")?
{
controller
.start_session(token)
.await
.context("Failed to restart session during app start")?;
controller.start_session(token).await?;
} else {
tracing::info!("No token / actor_name on disk, starting in signed-out state");
}
@@ -733,7 +725,7 @@ async fn run_controller(
let win = app
.get_window("welcome")
.context("Couldn't get handle to Welcome window")?;
win.show()?;
win.show().context("Couldn't show Welcome window")?;
}
let mut have_internet =
@@ -782,15 +774,13 @@ async fn run_controller(
tracing::error!("Crashing on purpose");
unsafe { sadness_generator::raise_segfault() }
},
Req::Fail(Failure::Error) => bail!("Test error"),
Req::Fail(Failure::Error) => Err(anyhow!("Test error"))?,
Req::Fail(Failure::Panic) => panic!("Test panic"),
Req::SystemTrayMenu(TrayMenuEvent::Quit) => {
tracing::info!("User clicked Quit in the menu");
break
}
req => if let Err(error) = controller.handle_request(req).await {
tracing::error!(?error, "Failed to handle a ControllerRequest");
}
req => controller.handle_request(req).await?,
}
},
}

58
rust/gui-client/src-tauri/src/client/gui/errors.rs Normal file
View File

@@ -0,0 +1,58 @@
use super::{deep_link, logging};
use anyhow::Result;
use firezone_headless_client::{ipc, FIREZONE_GROUP};
// TODO: Replace with `anyhow` gradually per <https://github.com/firezone/firezone/pull/3546#discussion_r1477114789>
#[allow(dead_code)]
#[derive(Debug, thiserror::Error)]
pub(crate) enum Error {
#[error("Deep-link module error: {0}")]
DeepLink(#[from] deep_link::Error),
#[error("Logging module error: {0}")]
Logging(#[from] logging::Error),
// `client.rs` provides a more user-friendly message when showing the error dialog box for certain variants
#[error("IPC")]
Ipc(#[from] ipc::Error),
#[error("UserNotInFirezoneGroup")]
UserNotInFirezoneGroup,
#[error("WebViewNotInstalled")]
WebViewNotInstalled,
#[error(transparent)]
Other(#[from] anyhow::Error),
}
/// Blocks the thread and shows an error dialog
///
/// Doesn't play well with async, only use this if we're bailing out of the
/// entire process.
pub(crate) fn show_error_dialog(error: &Error) -> Result<()> {
// Decision to put the error strings here: <https://github.com/firezone/firezone/pull/3464#discussion_r1473608415>
// This message gets shown to users in the GUI and could be localized, unlike
// messages in the log which only need to be used for `git grep`.
let user_friendly_error_msg = match error {
// TODO: Update this URL
Error::WebViewNotInstalled => "Firezone cannot start because WebView2 is not installed. Follow the instructions at <https://www.firezone.dev/kb/user-guides/windows-client>.".to_string(),
Error::DeepLink(deep_link::Error::CantListen) => "Firezone is already running. If it's not responding, force-stop it.".to_string(),
Error::DeepLink(deep_link::Error::Other(error)) => error.to_string(),
Error::Ipc(ipc::Error::NotFound(path)) => {
tracing::error!(?path, "Couldn't find Firezone IPC service");
"Couldn't find Firezone IPC service. Is the service running?".to_string()
}
Error::Ipc(ipc::Error::PermissionDenied) => "Permission denied for Firezone IPC service. This should only happen on dev systems.".to_string(),
Error::Ipc(ipc::Error::Other(error)) => error.to_string(),
Error::Logging(_) => "Logging error".to_string(),
Error::UserNotInFirezoneGroup => format!("You are not a member of the group `{FIREZONE_GROUP}`. Try `sudo usermod -aG {FIREZONE_GROUP} $USER` and then reboot"),
Error::Other(error) => error.to_string(),
};
tracing::error!(?user_friendly_error_msg);
// I tried the Tauri dialogs and for some reason they don't show our
// app icon.
native_dialog::MessageDialog::new()
.set_title("Firezone Error")
.set_text(&user_friendly_error_msg)
.set_type(native_dialog::MessageType::Error)
.show_alert()?;
Ok(())
}

12
rust/gui-client/src-tauri/src/client/ipc.rs
View File

@@ -2,7 +2,10 @@ 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, IpcClientMsg, IpcServerMsg};
use firezone_headless_client::{
ipc::{self, Error},
IpcClientMsg, IpcServerMsg,
};
use futures::{SinkExt, StreamExt};
use secrecy::{ExposeSecret, SecretString};
use std::{net::IpAddr, sync::Arc};
@@ -43,7 +46,7 @@ impl CallbackHandler {
pub(crate) struct Client {
task: tokio::task::JoinHandle<Result<()>>,
// Needed temporarily to avoid a big refactor. We can remove this in the future.
tx: firezone_headless_client::ipc::ClientWrite,
tx: ipc::ClientWrite,
}
impl Client {
@@ -69,7 +72,7 @@ impl Client {
token: SecretString,
callback_handler: CallbackHandler,
tokio_handle: tokio::runtime::Handle,
) -> Result<Self> {
) -> Result<Self, Error> {
tracing::info!(
client_pid = std::process::id(),
"Connecting to IPC service..."
@@ -99,7 +102,8 @@ impl Client {
token,
})
.await
.context("Couldn't send Connect message")?;
.context("Couldn't send Connect message")
.map_err(Error::Other)?;
Ok(client)
}

1
rust/headless-client/Cargo.toml
View File

@@ -21,6 +21,7 @@ ip_network = { version = "0.4", default-features = false }
secrecy = { workspace = true }
serde = { version = "1.0.203", features = ["derive"] }
serde_json = "1.0.117"
thiserror = { version = "1.0", default-features = false }
# This actually relies on many other features in Tokio, so this will probably
# fail to build outside the workspace. <https://github.com/firezone/firezone/pull/4328#discussion_r1540342142>
tokio = { version = "1.38.0", features = ["macros", "signal"] }

38
rust/headless-client/src/ipc.rs
View File

@@ -1,5 +1,5 @@
use crate::{IpcClientMsg, IpcServerMsg};
use anyhow::{anyhow, Context as _, Result};
use anyhow::{Context as _, Result};
use tokio::io::{ReadHalf, WriteHalf};
use tokio_util::{
bytes::BytesMut,
@@ -24,6 +24,18 @@ pub type ClientWrite = FramedWrite<WriteHalf<ClientStream>, Encoder<IpcClientMsg
pub(crate) type ServerRead = FramedRead<ReadHalf<ServerStream>, Decoder<IpcClientMsg>>;
pub(crate) type ServerWrite = FramedWrite<WriteHalf<ServerStream>, Encoder<IpcServerMsg>>;
// pub so that the GUI can display a human-friendly message
#[derive(Debug, thiserror::Error)]
pub enum Error {
#[error("Couldn't find IPC service `{0}`")]
NotFound(String),
#[error("Permission denied")]
PermissionDenied,
#[error(transparent)]
Other(anyhow::Error),
}
/// A name that both the server and client can use to find each other
///
/// In the platform-specific code, this is translated to a Unix Domain Socket
@@ -118,7 +130,11 @@ impl<E: serde::Serialize> tokio_util::codec::Encoder<&E> for Encoder<E> {
/// Connect to the IPC service
///
/// Public because the GUI Client will need it
pub async fn connect_to_service(id: ServiceId) -> Result<(ClientRead, ClientWrite)> {
pub async fn connect_to_service(id: ServiceId) -> Result<(ClientRead, ClientWrite), Error> {
// This is how ChatGPT recommended, and I couldn't think of any more clever
// way before I asked it.
let mut last_err = None;
for _ in 0..10 {
match platform::connect_to_service(id).await {
Ok(stream) => {
@@ -132,15 +148,14 @@ pub async fn connect_to_service(id: ServiceId) -> Result<(ClientRead, ClientWrit
?error,
"Couldn't connect to IPC service, will sleep and try again"
);
last_err = Some(error);
// This won't come up much for humans but it helps the automated
// tests pass
tokio::time::sleep(std::time::Duration::from_millis(100)).await;
}
}
}
Err(anyhow!(
"Failed to connect to IPC server after multiple attempts"
))
Err(last_err.expect("Impossible - Exhausted all retries but didn't get any errors"))
}
impl platform::Server {
@@ -156,11 +171,22 @@ impl platform::Server {
mod tests {
use super::{platform::Server, ServiceId};
use crate::{IpcClientMsg, IpcServerMsg};
use anyhow::{ensure, Context as _, Result};
use anyhow::{bail, ensure, Context as _, Result};
use futures::{SinkExt, StreamExt};
use std::time::Duration;
use tokio::{task::JoinHandle, time::timeout};
#[tokio::test]
async fn no_such_service() -> Result<()> {
let _ = tracing_subscriber::fmt().with_test_writer().try_init();
const ID: ServiceId = ServiceId::Test("H56FRXVH");
if super::connect_to_service(ID).await.is_ok() {
bail!("`connect_to_service` should have failed for a non-existent service");
}
Ok(())
}
/// Make sure the IPC client and server can exchange messages
#[tokio::test]
async fn smoke() -> Result<()> {

33
rust/headless-client/src/ipc/linux.rs
View File

@@ -1,6 +1,6 @@
use super::ServiceId;
use anyhow::{Context as _, Result};
use std::{os::unix::fs::PermissionsExt, path::PathBuf};
use super::{Error, ServiceId};
use anyhow::{anyhow, Context as _, Result};
use std::{io::ErrorKind, os::unix::fs::PermissionsExt, path::PathBuf};
use tokio::net::{UnixListener, UnixStream};
pub(crate) struct Server {
@@ -16,16 +16,25 @@ pub type ClientStream = UnixStream;
pub(crate) type ServerStream = UnixStream;
/// Connect to the IPC service
#[allow(clippy::unused_async)]
pub async fn connect_to_service(id: ServiceId) -> Result<ClientStream> {
#[allow(clippy::wildcard_enum_match_arm)]
pub async fn connect_to_service(id: ServiceId) -> Result<ClientStream, Error> {
let path = ipc_path(id);
let stream = UnixStream::connect(&path).await.with_context(|| {
format!(
"Couldn't connect to Unix domain socket at `{}`",
path.display()
)
})?;
let cred = stream.peer_cred()?;
let stream = UnixStream::connect(&path)
.await
.map_err(|error| match error.kind() {
ErrorKind::ConnectionRefused => {
Error::Other(anyhow!("ConnectionRefused by Unix domain socket"))
}
ErrorKind::NotFound => Error::NotFound(path.display().to_string()),
ErrorKind::PermissionDenied => Error::PermissionDenied,
_ => Error::Other(
anyhow!(error.to_string()).context("Couldn't connect to Unix domain socket"),
),
})?;
let cred = stream
.peer_cred()
.context("Couldn't get PID of UDS server")
.map_err(Error::Other)?;
tracing::info!(
uid = cred.uid(),
gid = cred.gid(),

15
rust/headless-client/src/ipc/windows.rs
View File

@@ -1,7 +1,7 @@
use super::ServiceId;
use super::{Error, ServiceId};
use anyhow::{bail, Context as _, Result};
use connlib_shared::BUNDLE_ID;
use std::{ffi::c_void, os::windows::io::AsRawHandle, time::Duration};
use std::{ffi::c_void, io::ErrorKind, os::windows::io::AsRawHandle, time::Duration};
use tokio::net::windows::named_pipe;
use windows::Win32::{
Foundation::HANDLE,
@@ -23,17 +23,22 @@ pub(crate) type ServerStream = named_pipe::NamedPipeServer;
///
/// This is async on Linux
#[allow(clippy::unused_async)]
pub(crate) async fn connect_to_service(id: ServiceId) -> Result<ClientStream> {
#[allow(clippy::wildcard_enum_match_arm)]
pub(crate) async fn connect_to_service(id: ServiceId) -> Result<ClientStream, Error> {
let path = ipc_path(id);
let stream = named_pipe::ClientOptions::new()
.open(&path)
.with_context(|| format!("Couldn't connect to named pipe server at `{path}`"))?;
.map_err(|error| match error.kind() {
ErrorKind::NotFound => Error::NotFound(path),
_ => Error::Other(error.into()),
})?;
let handle = HANDLE(stream.as_raw_handle() as isize);
let mut server_pid: u32 = 0;
// SAFETY: Windows doesn't store this pointer or handle, and we just got the handle
// from Tokio, so it should be valid.
unsafe { GetNamedPipeServerProcessId(handle, &mut server_pid) }
.context("Couldn't get PID of named pipe server")?;
.context("Couldn't get PID of named pipe server")
.map_err(Error::Other)?;
tracing::info!(?server_pid, "Made IPC connection");
Ok(stream)
}