From 8fcc99ae14f062f31a4da7e9d0210af2e0f5b31e Mon Sep 17 00:00:00 2001 From: Reactor Scram Date: Fri, 15 Dec 2023 13:17:55 -0600 Subject: [PATCH] windows: fix deep links (#2903) Stacked on PR #2888 This will fix #2878 by setting some Windows security permissions to let non-admin clients connect to a named pipe server owned by an admin process. It should also give us a path to fix 2808 (If I link it, Github assumes this PR closes that issue?), since it'll be our own code instead of tauri-plugin-deep-link, and we can just choose not to panic. I'm using Tokio's `named_pipe` module where that panic happens, and Tokio is good about just returning Results instead of panicking. --------- Signed-off-by: Reactor Scram Co-authored-by: Jamil --- rust/Cargo.lock | 87 +-------- rust/windows-client/docs/manual_testing.md | 8 + rust/windows-client/src-tauri/Cargo.toml | 5 +- rust/windows-client/src-tauri/src/client.rs | 17 +- .../src-tauri/src/client/cli.rs | 10 +- .../src-tauri/src/client/debug_commands.rs | 58 +++++- .../src-tauri/src/client/deep_link.rs | 176 ++++++++++++++++++ .../src-tauri/src/client/gui.rs | 82 ++++---- 8 files changed, 304 insertions(+), 139 deletions(-) create mode 100755 rust/windows-client/src-tauri/src/client/deep_link.rs diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 320cae2ca..ab863e5de 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -1561,15 +1561,6 @@ dependencies = [ "subtle", ] -[[package]] -name = "dirs" -version = "5.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "44c45a9d03d6676652bcb5e724c7e988de1acad23a711b5217ab9cbecbec2225" -dependencies = [ - "dirs-sys", -] - [[package]] name = "dirs-next" version = "2.0.0" @@ -1580,18 +1571,6 @@ dependencies = [ "dirs-sys-next", ] -[[package]] -name = "dirs-sys" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "520f05a5cbd335fae5a99ff7a6ab8627577660ee5cfd6a94a6a929b52ff0321c" -dependencies = [ - "libc", - "option-ext", - "redox_users", - "windows-sys 0.48.0", -] - [[package]] name = "dirs-sys-next" version = "0.1.2" @@ -2036,7 +2015,7 @@ dependencies = [ "serde_json", "tauri", "tauri-build", - "tauri-plugin-deep-link", + "tauri-utils", "thiserror", "tokio", "tracing", @@ -2045,6 +2024,7 @@ dependencies = [ "url", "uuid", "windows 0.52.0", + "winreg 0.51.0", "wintun", "zip", ] @@ -3093,19 +3073,6 @@ dependencies = [ "webrtc-util", ] -[[package]] -name = "interprocess" -version = "1.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "81f2533f3be42fffe3b5e63b71aeca416c1c3bc33e4e27be018521e76b1f38fb" -dependencies = [ - "cfg-if", - "libc", - "rustc_version", - "to_method", - "winapi", -] - [[package]] name = "io-kit-sys" version = "0.1.0" @@ -3964,28 +3931,6 @@ dependencies = [ "objc_id", ] -[[package]] -name = "objc-sys" -version = "0.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c7c71324e4180d0899963fc83d9d241ac39e699609fc1025a850aadac8257459" - -[[package]] -name = "objc2" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "559c5a40fdd30eb5e344fbceacf7595a81e242529fb4e21cf5f43fb4f11ff98d" -dependencies = [ - "objc-sys", - "objc2-encode", -] - -[[package]] -name = "objc2-encode" -version = "3.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d079845b37af429bfe5dfa76e6d087d788031045b25cfc6fd898486fd9847666" - [[package]] name = "objc_exception" version = "0.1.2" @@ -4148,12 +4093,6 @@ dependencies = [ "tokio-stream", ] -[[package]] -name = "option-ext" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" - [[package]] name = "ordered-float" version = "3.9.2" @@ -6180,22 +6119,6 @@ dependencies = [ "tauri-utils", ] -[[package]] -name = "tauri-plugin-deep-link" -version = "0.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4536f5f6602e8fdfaa7b3b185076c2a0704f8eb7015f4e58461eb483ec3ed1f8" -dependencies = [ - "dirs", - "interprocess", - "log", - "objc2", - "once_cell", - "tauri-utils", - "windows-sys 0.48.0", - "winreg 0.50.0", -] - [[package]] name = "tauri-runtime" version = "0.14.1" @@ -6402,12 +6325,6 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" -[[package]] -name = "to_method" -version = "1.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c7c4ceeeca15c8384bbc3e011dbd8fccb7f068a440b752b7d9b32ceb0ca0e2e8" - [[package]] name = "tokio" version = "1.34.0" diff --git a/rust/windows-client/docs/manual_testing.md b/rust/windows-client/docs/manual_testing.md index 14b5e7e43..81a164298 100755 --- a/rust/windows-client/docs/manual_testing.md +++ b/rust/windows-client/docs/manual_testing.md @@ -52,6 +52,14 @@ If the client stops running while signed in, then the token may be stored in Win - [ ] Given the log tab is displayed, when you switch to another tab or close the window, then any ongoing computation will be canceled. - [ ] Given the log tab is computing log directory size, when the 1-minute refresh timer ticks, then the computation will time out and show an error e.g. "timed out while computing size" +# Resetting state + +This is a list of all the on-disk state that you need to reset to test a first-time install / first-time run of the Firezone client. + +- `AppData/Local/dev.firezone.client/` (Config, logs, webview cache, etc.) +- Registry key `Computer\HKEY_CURRENT_USER\Software\Classes\firezone-fd0020211111` (Deep link association) +- Token, in Windows Credential Manager + # Token storage ([#2740](https://github.com/firezone/firezone/issues/2740)) diff --git a/rust/windows-client/src-tauri/Cargo.toml b/rust/windows-client/src-tauri/Cargo.toml index 7ba85306e..3523f3534 100755 --- a/rust/windows-client/src-tauri/Cargo.toml +++ b/rust/windows-client/src-tauri/Cargo.toml @@ -36,14 +36,17 @@ zip = "0.6.6" [target.'cfg(windows)'.dependencies] # Tauri works fine on Linux, but it requires a lot of build-time deps like glib and gdk, so I've blocked it out for now. tauri = { version = "1.5", features = [ "dialog", "shell-open-api", "system-tray" ] } -tauri-plugin-deep-link = "0.1.2" +tauri-utils = "1.5.1" +winreg = "0.51.0" wintun = "0.3.2" [target.'cfg(windows)'.dependencies.windows] version = "0.52.0" features = [ "Win32_Foundation", + "Win32_Security", "Win32_System_LibraryLoader", + "Win32_System_SystemServices", "Win32_UI_Shell", "Win32_UI_WindowsAndMessaging", ] diff --git a/rust/windows-client/src-tauri/src/client.rs b/rust/windows-client/src-tauri/src/client.rs index 9d13e8bcb..3cf533bd3 100644 --- a/rust/windows-client/src-tauri/src/client.rs +++ b/rust/windows-client/src-tauri/src/client.rs @@ -4,6 +4,8 @@ use cli::CliCommands as Cmd; mod cli; mod debug_commands; +#[cfg(target_family = "windows")] +mod deep_link; mod device_id; #[cfg(target_family = "unix")] mod gui { @@ -38,16 +40,6 @@ pub(crate) struct GuiParams { pub(crate) struct AppLocalDataDir(std::path::PathBuf); pub(crate) fn run() -> Result<()> { - // Special case for app link URIs - if let Some(arg) = std::env::args().nth(1) { - let url = url::Url::parse(&arg)?; - if url.scheme() == DEEP_LINK_SCHEME { - return gui::run(GuiParams { - inject_faults: false, - }); - } - } - let cli = cli::Cli::parse(); match cli.command { @@ -58,9 +50,10 @@ pub(crate) fn run() -> Result<()> { println!("debug"); Ok(()) } + Some(Cmd::DebugPipeServer) => debug_commands::pipe_server(), Some(Cmd::DebugToken) => debug_commands::token(), Some(Cmd::DebugWintun) => debug_commands::wintun(cli), + Some(Cmd::OpenDeepLink(deep_link)) => debug_commands::open_deep_link(&deep_link.url), + Some(Cmd::RegisterDeepLink) => debug_commands::register_deep_link(), } } - -pub(crate) const DEEP_LINK_SCHEME: &str = "firezone-fd0020211111"; diff --git a/rust/windows-client/src-tauri/src/client/cli.rs b/rust/windows-client/src-tauri/src/client/cli.rs index 0303148b7..562e407c1 100755 --- a/rust/windows-client/src-tauri/src/client/cli.rs +++ b/rust/windows-client/src-tauri/src/client/cli.rs @@ -1,4 +1,4 @@ -use clap::Parser; +use clap::{Args, Parser}; #[derive(Parser)] #[command(author, version, about, long_about = None)] @@ -12,6 +12,14 @@ pub struct Cli { #[derive(clap::Subcommand)] pub enum CliCommands { Debug, + DebugPipeServer, DebugToken, DebugWintun, + OpenDeepLink(DeepLink), + RegisterDeepLink, +} + +#[derive(Args)] +pub struct DeepLink { + pub url: url::Url, } diff --git a/rust/windows-client/src-tauri/src/client/debug_commands.rs b/rust/windows-client/src-tauri/src/client/debug_commands.rs index 35b507ca2..6567de440 100644 --- a/rust/windows-client/src-tauri/src/client/debug_commands.rs +++ b/rust/windows-client/src-tauri/src/client/debug_commands.rs @@ -4,6 +4,10 @@ use crate::client::cli::Cli; use anyhow::Result; use keyring::Entry; +use tokio::runtime::Runtime; + +// TODO: In tauri-plugin-deep-link, this is the identifier in tauri.conf.json +const PIPE_NAME: &str = "dev.firezone.client"; /// Test encrypted credential storage pub fn token() -> Result<()> { @@ -29,14 +33,26 @@ pub fn token() -> Result<()> { Ok(()) } -pub use details::wintun; +pub use details::{open_deep_link, pipe_server, register_deep_link, wintun}; #[cfg(target_family = "unix")] mod details { use super::*; + pub fn open_deep_link(_: &url::Url) -> Result<()> { + unimplemented!() + } + + pub fn pipe_server(_: Cli) -> Result<()> { + unimplemented!() + } + + pub fn register_deep_link(_: Cli) -> Result<()> { + unimplemented!() + } + pub fn wintun(_: Cli) -> Result<()> { - panic!("Wintun not implemented for Linux."); + unimplemented!() } } @@ -45,6 +61,44 @@ mod details { use super::*; use std::sync::Arc; + pub fn open_deep_link(path: &url::Url) -> Result<()> { + let subscriber = tracing_subscriber::FmtSubscriber::builder() + .with_max_level(tracing::Level::TRACE) + .finish(); + tracing::subscriber::set_global_default(subscriber) + .expect("setting default subscriber failed"); + + let rt = Runtime::new()?; + rt.block_on(crate::client::deep_link::open(PIPE_NAME, path))?; + Ok(()) + } + + // Copied the named pipe idea from `interprocess` and `tauri-plugin-deep-link`, + // although I believe it's considered best practice on Windows to use named pipes for + // single-instance apps. + pub fn pipe_server() -> Result<()> { + let subscriber = tracing_subscriber::FmtSubscriber::builder() + .with_max_level(tracing::Level::TRACE) + .finish(); + tracing::subscriber::set_global_default(subscriber) + .expect("setting default subscriber failed"); + + let rt = Runtime::new()?; + rt.block_on(async { + loop { + let server = crate::client::deep_link::Server::new(PIPE_NAME)?; + server.accept().await?; + } + }) + } + + // 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_deep_link() -> Result<()> { + crate::client::deep_link::register(PIPE_NAME)?; + Ok(()) + } + pub fn wintun(_: Cli) -> Result<()> { for _ in 0..3 { println!("Creating adapter..."); diff --git a/rust/windows-client/src-tauri/src/client/deep_link.rs b/rust/windows-client/src-tauri/src/client/deep_link.rs new file mode 100755 index 000000000..1124d8d23 --- /dev/null +++ b/rust/windows-client/src-tauri/src/client/deep_link.rs @@ -0,0 +1,176 @@ +//! A module for registering 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 std::{ffi::c_void, io, path::Path}; +use tokio::{io::AsyncReadExt, io::AsyncWriteExt, net::windows::named_pipe}; +use windows::Win32::Security as WinSec; + +pub(crate) const FZ_SCHEME: &str = "firezone-fd0020211111"; + +#[derive(thiserror::Error, Debug)] +pub enum Error { + /// 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), + /// We got some data but it's not UTF-8 + #[error(transparent)] + LinkNotUtf8(std::string::FromUtf8Error), + /// This means we are probably the second instance + #[error("named pipe server couldn't start listening")] + Listen, + /// Error from server's POV + #[error(transparent)] + ServerCommunications(io::Error), + #[error(transparent)] + UrlParse(#[from] url::ParseError), + /// Something went wrong setting up the registry + #[error(transparent)] + WindowsRegistry(io::Error), +} + +/// A server for a named pipe, so we can receive deep links from other instances +/// of the client launched by web browsers +pub struct Server { + inner: named_pipe::NamedPipeServer, +} + +impl Server { + /// Construct a server, but don't await client connections yet + /// + /// Panics if there is no Tokio runtime + pub fn new(id: &str) -> 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. + + // We could instead pick an ephemeral TCP port and write that to a file, + // akin to how Unix processes will write their PID to a file to manage long-running instances + // But this doesn't require us to listen on TCP. + + let mut server_options = named_pipe::ServerOptions::new(); + server_options.first_pipe_instance(true); + + // This will allow non-admin clients to connect to us even if we're running as admin + let mut sd = WinSec::SECURITY_DESCRIPTOR::default(); + let psd = WinSec::PSECURITY_DESCRIPTOR(&mut sd as *mut _ as *mut c_void); + unsafe { + // ChatGPT pointed me to these functions, it's better than the official MS docs + WinSec::InitializeSecurityDescriptor( + psd, + windows::Win32::System::SystemServices::SECURITY_DESCRIPTOR_REVISION, + ) + .map_err(|_| Error::Listen)?; + WinSec::SetSecurityDescriptorDacl(psd, true, None, false).map_err(|_| Error::Listen)?; + } + + let mut sa = WinSec::SECURITY_ATTRIBUTES { + nLength: std::mem::size_of::() + .try_into() + .unwrap(), + lpSecurityDescriptor: psd.0, + bInheritHandle: false.into(), + }; + + let path = named_pipe_path(id); + let server = unsafe { + server_options + .create_with_security_attributes_raw(path, &mut sa as *mut _ as *mut c_void) + } + .map_err(|_| Error::Listen)?; + + tracing::debug!("server is bound"); + Ok(Server { inner: server }) + } + + /// Await one incoming deep link from a named pipe client + /// Tokio's API is strange, so this consumes the 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 async fn accept(mut self) -> Result { + self.inner + .connect() + .await + .map_err(Error::ServerCommunications)?; + 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. + // Also, I think `read_to_end` can do partial reads because this is a network socket, + // not a file. We might need a length-prefixed or newline-terminated format for IPC. + let mut bytes = vec![]; + self.inner + .read_to_end(&mut bytes) + .await + .map_err(Error::ServerCommunications)?; + + self.inner.disconnect().ok(); + + tracing::debug!("Server read"); + let s = String::from_utf8(bytes).map_err(Error::LinkNotUtf8)?; + tracing::info!("{}", s); + let url = url::Url::parse(&s)?; + + Ok(url) + } +} + +/// Open a deep link by sending it to the already-running instance of the app +pub async fn open(id: &str, url: &url::Url) -> Result<(), Error> { + let path = named_pipe_path(id); + let mut client = named_pipe::ClientOptions::new() + .open(path) + .map_err(Error::Connect)?; + client + .write_all(url.as_str().as_bytes()) + .await + .map_err(Error::ClientCommunications)?; + Ok(()) +} + +/// Registers the current exe as the handler for our deep link scheme. +/// +/// 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` +/// +/// * `id` A unique ID for the app, e.g. "com.contoso.todo-list" or "dev.firezone.client" +pub fn register(id: &str) -> Result<(), Error> { + let exe = tauri_utils::platform::current_exe() + .map_err(Error::CurrentExe)? + .display() + .to_string() + .replace("\\\\?\\", ""); + + set_registry_values(id, &exe).map_err(Error::WindowsRegistry)?; + + Ok(()) +} + +/// Set up the Windows registry to call the given exe when our deep link scheme is used +/// +/// All errors from this function are registry-related +fn set_registry_values(id: &str, exe: &str) -> Result<(), io::Error> { + let hkcu = winreg::RegKey::predef(winreg::enums::HKEY_CURRENT_USER); + let base = Path::new("Software").join("Classes").join(FZ_SCHEME); + + let (key, _) = hkcu.create_subkey(&base)?; + key.set_value("", &format!("URL:{}", id))?; + key.set_value("URL Protocol", &"")?; + + let (icon, _) = hkcu.create_subkey(base.join("DefaultIcon"))?; + icon.set_value("", &format!("{},0", &exe))?; + + let (cmd, _) = hkcu.create_subkey(base.join("shell").join("open").join("command"))?; + cmd.set_value("", &format!("{} open-deep-link \"%1\"", &exe))?; + + Ok(()) +} + +fn named_pipe_path(id: &str) -> String { + format!(r"\\.\pipe\{}", id) +} diff --git a/rust/windows-client/src-tauri/src/client/gui.rs b/rust/windows-client/src-tauri/src/client/gui.rs index c8eede905..98d4c175e 100755 --- a/rust/windows-client/src-tauri/src/client/gui.rs +++ b/rust/windows-client/src-tauri/src/client/gui.rs @@ -3,11 +3,11 @@ // TODO: `git grep` for unwraps before 1.0, especially this gui module -use crate::client::{self, AppLocalDataDir}; +use crate::client::{self, deep_link, AppLocalDataDir}; use anyhow::{anyhow, bail, Context, Result}; use client::settings::{self, AdvancedSettings}; use connlib_client_shared::file_logger; -use secrecy::SecretString; +use secrecy::{ExposeSecret, SecretString}; use std::{ net::{Ipv4Addr, Ipv6Addr}, path::PathBuf, @@ -36,19 +36,30 @@ pub(crate) struct Managed { pub inject_faults: bool, } +// TODO: We're supposed to get this from Tauri, but I'd need to move some things around first +const TAURI_ID: &str = "dev.firezone.client"; + /// Runs the Tauri GUI and returns on exit or unrecoverable error pub(crate) fn run(params: client::GuiParams) -> Result<()> { let client::GuiParams { inject_faults } = params; - // Make sure we're single-instance - // If another instance is already running, this call - // signals the other instance and then exits our process. - tauri_plugin_deep_link::prepare("dev.firezone"); - + // Needed for the deep link server let rt = tokio::runtime::Runtime::new()?; let _guard = rt.enter(); + // 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(TAURI_ID)?; + + // We know now we're the only instance on the computer, so register our exe + // to handle deep links + deep_link::register(TAURI_ID)?; + let (ctlr_tx, ctlr_rx) = mpsc::channel(5); + + tokio::spawn(accept_deep_links(server, ctlr_tx.clone())); + let managed = Managed { ctlr_tx, inject_faults, @@ -101,7 +112,6 @@ pub(crate) fn run(params: client::GuiParams) -> Result<()> { // Set up logger // It's hard to set it up before Tauri's setup, because Tauri knows where all the config and data go in AppData and I don't want to replicate their logic. - let logging_handles = client::logging::setup(&advanced_settings.log_filter)?; tracing::info!("started log"); @@ -114,16 +124,6 @@ pub(crate) fn run(params: client::GuiParams) -> Result<()> { } }); - // From https://github.com/FabianLars/tauri-plugin-deep-link/blob/main/example/main.rs - let handle = app.handle(); - if let Err(e) = tauri_plugin_deep_link::register(client::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!())? @@ -138,12 +138,22 @@ pub(crate) fn run(params: client::GuiParams) -> Result<()> { Ok(()) } -fn handle_deep_link(app: &tauri::AppHandle, url: String) -> Result<()> { - Ok(app - .try_state::() - .ok_or_else(|| anyhow!("can't get Managed object from Tauri"))? - .ctlr_tx - .blocking_send(ControllerRequest::SchemeRequest(SecretString::new(url)))?) +/// Worker task to accept deep links from a named pipe forever +/// +/// * `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: mpsc::Sender, +) -> Result<()> { + loop { + if let Ok(url) = server.accept().await { + ctlr_tx + .send(ControllerRequest::SchemeRequest(url)) + .await + .ok(); + } + server = deep_link::Server::new(TAURI_ID)?; + } } #[derive(Debug, PartialEq)] @@ -208,7 +218,10 @@ fn handle_system_tray_event(app: &tauri::AppHandle, event: TrayMenuEvent) -> Res .ok_or_else(|| anyhow!("getting ctlr_tx state"))? .ctlr_tx .blocking_send(ControllerRequest::SignIn)?, - TrayMenuEvent::SignOut => app.tray_handle().set_menu(signed_out_menu())?, + TrayMenuEvent::SignOut => { + keyring_entry()?.delete_password()?; + app.tray_handle().set_menu(signed_out_menu())?; + } TrayMenuEvent::Quit => app.exit(0), } Ok(()) @@ -217,8 +230,7 @@ fn handle_system_tray_event(app: &tauri::AppHandle, event: TrayMenuEvent) -> Res pub(crate) enum ControllerRequest { ExportLogs(PathBuf), GetAdvancedSettings(oneshot::Sender), - // Secret because it will have the token in it - SchemeRequest(SecretString), + SchemeRequest(url::Url), SignIn, UpdateResources(Vec), } @@ -419,10 +431,8 @@ async fn run_controller( Req::GetAdvancedSettings(tx) => { tx.send(controller.advanced_settings.clone()).ok(); } - Req::SchemeRequest(req) => { - use secrecy::ExposeSecret; - - if let Ok(auth) = parse_auth_callback(&req) { + Req::SchemeRequest(url) => { + if let Ok(auth) = parse_auth_callback(&url) { tracing::debug!("setting new token"); let entry = keyring_entry()?; entry.set_password(auth.token.expose_secret())?; @@ -474,11 +484,7 @@ pub(crate) struct AuthCallback { _identifier: SecretString, } -fn parse_auth_callback(input: &SecretString) -> Result { - use secrecy::ExposeSecret; - - let url = url::Url::parse(input.expose_secret())?; - +fn parse_auth_callback(url: &url::Url) -> Result { let mut actor_name = None; let mut token = None; let mut identifier = None; @@ -583,14 +589,14 @@ fn signed_out_menu() -> SystemTrayMenu { mod tests { use super::TrayMenuEvent; use anyhow::Result; - use secrecy::{ExposeSecret, SecretString}; + use secrecy::ExposeSecret; use std::str::FromStr; #[test] fn parse_auth_callback() -> Result<()> { let input = "firezone://handle_client_auth_callback/?actor_name=Reactor+Scram&client_auth_token=a_very_secret_string&identity_provider_identifier=12345"; - let actual = super::parse_auth_callback(&SecretString::from_str(input)?)?; + let actual = super::parse_auth_callback(&url::Url::parse(input)?)?; assert_eq!(actual.actor_name, "Reactor Scram"); assert_eq!(actual.token.expose_secret(), "a_very_secret_string");