chore(gui-client/linux): show an error if the user doesn't belong to the firezone group (#4822)

Ran into this during testing. For some reason Ubuntu 20.04, maybe
others, requires a reboot to add the desktop user to a group. I thought
logging out and back in should be enough but it's not.

<img width="527" alt="image"
src="https://github.com/firezone/firezone/assets/13400041/4f7c2551-c7aa-4ecc-be55-66c6e6ac32a0">

---------

Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>

rust/Cargo.lock

@@ -1909,6 +1909,7 @@ dependencies = [
  "keyring",
  "minidumper",
  "native-dialog",
+ "nix 0.28.0",
  "output_vt100",
  "rand 0.8.5",
  "reqwest",

rust/gui-client/src-tauri/Cargo.toml

@@ -55,6 +55,7 @@ zip = { version = "0.6.6", features = ["deflate", "time"], default-features = fa
 dirs = "5.0.1"
 # Used for infinite `pending` on not-yet-implemented functions
 futures = "0.3.30"
+nix = { version =  "0.28.0", features = ["user"] }
 tokio-util = { version = "0.7.10", features = ["codec"] }
 
 [target.'cfg(target_os = "macos")'.dependencies]

rust/gui-client/src-tauri/src/client.rs

@@ -66,13 +66,18 @@ pub(crate) fn run() -> Result<()> {
 
     match cli.command {
         None => {
-            if elevation::check()? {
+            match elevation::check() {
                 // We're already elevated, just run the GUI
-                run_gui(cli)
-            } else {
-                // We're not elevated, ask Powershell / sudo to re-launch us, then exit
-                elevation::elevate()?;
-                Ok(())
+                Ok(true) => run_gui(cli),
+                Ok(false) => {
+                    // We're not elevated, ask Powershell to re-launch us, then exit. On Linux this is completely different.
+                    elevation::elevate()?;
+                    Ok(())
+                }
+                Err(error) => {
+                    show_error_dialog(&error)?;
+                    Err(error.into())
+                }
             }
         }
         Some(Cmd::CrashHandlerServer { socket_path }) => crash_handling::server(socket_path),
@@ -122,12 +127,15 @@ fn run_gui(cli: Cli) -> Result<()> {
 }
 
 fn show_error_dialog(error: &gui::Error) -> Result<()> {
-    #[allow(clippy::wildcard_enum_match_arm)]
+    // Decision to put the error strings here: <https://github.com/firezone/firezone/pull/3464#discussion_r1473608415>
     let 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(),
-        error => error.to_string(),
+        gui::Error::DeepLink(deep_link::Error::Other(error)) => error.to_string(),
+        gui::Error::Logging(_) => "Logging error".to_string(),
+        gui::Error::UserNotInFirezoneGroup => "You are not a member of the group `firezone`. Try `sudo adduser $USER firezone` and then reboot".to_string(),
+        gui::Error::Other(error) => error.to_string(),
     };
 
     native_dialog::MessageDialog::new()

rust/gui-client/src-tauri/src/client/elevation.rs

@@ -2,17 +2,24 @@ pub(crate) use imp::{check, elevate};
 
 #[cfg(target_os = "linux")]
 mod imp {
+    use crate::client::gui::Error;
     use anyhow::{Context, Result};
 
     #[allow(clippy::print_stderr)]
-    pub(crate) fn check() -> Result<bool> {
+    pub(crate) fn check() -> Result<bool, Error> {
         // Must use `eprintln` here because `tracing` won't be initialized yet.
-
         let user = std::env::var("USER").context("USER env var should be set")?;
         if user == "root" {
             eprintln!("Firezone must run as a normal user, not with `sudo` or as root");
             return Ok(false);
         }
+
+        let fz_gid = firezone_headless_client::imp::firezone_group()?.gid;
+        let groups = nix::unistd::getgroups().context("`nix::unistd::getgroups`")?;
+        if !groups.contains(&fz_gid) {
+            return Err(Error::UserNotInFirezoneGroup);
+        }
+
         Ok(true)
     }
 
@@ -26,7 +33,7 @@ mod imp {
 mod imp {
     use anyhow::Result;
 
-    pub(crate) fn check() -> Result<bool> {
+    pub(crate) fn check() -> Result<bool, crate::client::gui::Error> {
         Ok(true)
     }
 
@@ -37,14 +44,14 @@ mod imp {
 
 #[cfg(target_os = "windows")]
 mod imp {
-    use crate::client::wintun_install;
+    use crate::client::{gui::Error, wintun_install};
     use anyhow::{Context, Result};
     use std::{os::windows::process::CommandExt, str::FromStr};
 
     /// Check if we have elevated privileges, extract wintun.dll if needed.
     ///
     /// Returns true if already elevated, false if not elevated, error if we can't be sure
-    pub(crate) fn check() -> Result<bool> {
+    pub(crate) fn check() -> Result<bool, Error> {
         // Almost the same as the code in tun_windows.rs in connlib
         const TUNNEL_UUID: &str = "72228ef4-cb84-4ca5-a4e6-3f8636e75757";
         const TUNNEL_NAME: &str = "Firezone Elevation Check";
@@ -52,7 +59,11 @@ mod imp {
         let path = match wintun_install::ensure_dll() {
             Ok(x) => x,
             Err(wintun_install::Error::PermissionDenied) => return Ok(false),
-            Err(e) => return Err(e).context("Failed to ensure wintun.dll is installed"),
+            Err(e) => {
+                return Err(e)
+                    .context("Failed to ensure wintun.dll is installed")
+                    .map_err(Error::Other)
+            }
         };
 
         // SAFETY: Unsafe needed because we're loading a DLL from disk and it has arbitrary C code in it.

rust/gui-client/src-tauri/src/client/gui.rs

@@ -80,7 +80,7 @@ impl Managed {
 }
 
 // TODO: Replace with `anyhow` gradually per <https://github.com/firezone/firezone/pull/3546#discussion_r1477114789>
-#[cfg_attr(target_os = "macos", allow(dead_code))]
+#[allow(dead_code)]
 #[derive(Debug, thiserror::Error)]
 pub(crate) enum Error {
     #[error("Deep-link module error: {0}")]
@@ -88,7 +88,9 @@ pub(crate) enum Error {
     #[error("Logging module error: {0}")]
     Logging(#[from] logging::Error),
 
-    // `client.rs` provides a more user-friendly message when showing the error dialog box
+    // `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)]

rust/gui-client/src-tauri/src/main.rs

@@ -1,4 +1,4 @@
-//! The Firezone GUI client for Windows
+//! The Firezone GUI client for Linux and Windows
 
 // Prevents additional console window on Windows in release, DO NOT REMOVE!!
 #![cfg_attr(not(debug_assertions), windows_subsystem = "windows")]

rust/headless-client/src/imp_linux.rs

@@ -184,6 +184,13 @@ pub(crate) fn run_ipc_service(cli: Cli) -> Result<()> {
     rt.block_on(async { ipc_listen(cli).await })
 }
 
+pub fn firezone_group() -> Result<nix::unistd::Group> {
+    let group = nix::unistd::Group::from_name("firezone")
+        .context("can't get group by name")?
+        .context("firezone group must exist on the system")?;
+    Ok(group)
+}
+
 async fn ipc_listen(cli: Cli) -> Result<()> {
     // Remove the socket if a previous run left it there
     let sock_path = sock_path();

scripts/tests/smoke-test-gui-linux.sh

@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 
-source "./scripts/tests/lib.sh"
+set -euox pipefail
 
 BUNDLE_ID="dev.firezone.client"
 
@@ -22,6 +22,17 @@ cargo install --quiet --locked dump_syms minidump-stackwalk
 dump_syms ../target/debug/firezone-gui-client --output "$SYMS_PATH"
 ls -lash ../target/debug
 
+sudo groupadd --force firezone
+sudo adduser "$USER" firezone
+
+function run_fz_gui() {
+    pwd
+    # Does what it says
+    sudo --preserve-env \
+    su --login "$USER" --command \
+    "xvfb-run --auto-servernum $PWD/../target/debug/$PACKAGE $*"
+}
+
 function smoke_test() {
     # Make sure the files we want to check don't exist on the system yet
     stat "$LOGS_PATH" && exit 1
@@ -31,7 +42,7 @@ function smoke_test() {
     stat "$RAN_BEFORE_PATH" && exit 1
 
     # Run the smoke test normally
-    if ! xvfb-run --auto-servernum ../target/debug/"$PACKAGE" --no-deep-links smoke-test
+    if ! run_fz_gui --no-deep-links smoke-test
     then
         minidump-stackwalk --symbols-path "$SYMS_PATH" "$DUMP_PATH"
         exit 1
@@ -49,7 +60,7 @@ function smoke_test() {
     stat "$RAN_BEFORE_PATH"
 
     # Run the test again and make sure the device ID is not changed
-    xvfb-run --auto-servernum ../target/debug/"$PACKAGE"  --no-deep-links smoke-test
+    run_fz_gui --no-deep-links smoke-test
     # DEVICE_ID_2=$(cat "$DEVICE_ID_PATH")
 
     #if [ "$DEVICE_ID_1" != "$DEVICE_ID_2" ]
@@ -70,7 +81,7 @@ function crash_test() {
     rm -f "$DUMP_PATH"
 
     # Fail if it returns success, this is supposed to crash
-    xvfb-run --auto-servernum ../target/debug/"$PACKAGE" --crash --no-deep-links && exit 1
+    run_fz_gui --crash --no-deep-links && exit 1
 
     # Fail if the crash file wasn't written
     stat "$DUMP_PATH"