From 4cf0008961a57dbb85175bdb046a384521ca92b4 Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Thu, 28 Sep 2023 12:51:26 +0200 Subject: [PATCH] UX improvements for `mullvad api-access` - Re-phrase help texts for a lot of `mullvad api-access` commands - Add to help texts for some `mullvad api-access` commands - Compact the output of `mullvad api-access test` - `mullvad api-access status` is changed to `mullvad api-access get` to align with other `mullvad` commands. - `mullvad api-access get` does not print the enabled/disabled status of the shown access method --- mullvad-cli/src/cmds/api_access.rs | 118 ++++++++++++++++++----------- mullvad-cli/src/main.rs | 17 ++++- 2 files changed, 88 insertions(+), 47 deletions(-) diff --git a/mullvad-cli/src/cmds/api_access.rs b/mullvad-cli/src/cmds/api_access.rs index c1b7552313dc..fecabb818dc5 100644 --- a/mullvad-cli/src/cmds/api_access.rs +++ b/mullvad-cli/src/cmds/api_access.rs @@ -8,27 +8,31 @@ use talpid_types::net::openvpn::SHADOWSOCKS_CIPHERS; #[derive(Subcommand, Debug, Clone)] pub enum ApiAccess { - /// List the configured API access methods - List, + /// Display the current API access method. + Get, /// Add a custom API access method #[clap(subcommand)] Add(AddCustomCommands), - /// Edit an API access method + /// Lists all API access methods + /// + /// * = Enabled + List, + /// Edit a custom API access method Edit(EditCustomCommands), - /// Remove an API access method + /// Remove a custom API access method Remove(SelectItem), /// Enable an API access method Enable(SelectItem), /// Disable an API access method Disable(SelectItem), - /// Test an API access method - Test(SelectItem), - /// Force the use of a specific API access method. + /// Try to use a specific API access method (If the API is unreachable, reverts back to the previous access method) + /// + /// Selecting "Direct" will connect to the Mullvad API without going through any proxy. This connection use https and is therefore encrypted. /// - /// Selecting "Mullvad Bridges" respects your current bridge settings. + /// Selecting "Mullvad Bridges" respects your current bridge settings Use(SelectItem), - /// Show which access method is currently used to access the Mullvad API. - Status, + /// Try to reach the Mullvad API using a specific access method + Test(SelectItem), } impl ApiAccess { @@ -54,8 +58,8 @@ impl ApiAccess { ApiAccess::Use(cmd) => { Self::set(cmd).await?; } - ApiAccess::Status => { - Self::status().await?; + ApiAccess::Get => { + Self::get().await?; } }; Ok(()) @@ -163,13 +167,11 @@ impl ApiAccess { let mut rpc = MullvadProxyClient::new().await?; let access_method = Self::get_access_method(&mut rpc, &item).await?; rpc.set_access_method(access_method.get_id()).await?; + println!("Testing access method \"{}\"", access_method.name); // Make the daemon perform an network request which involves talking to the Mullvad API. match rpc.get_api_addresses().await { - Ok(_) => println!("Connected to the Mullvad API!"), - Err(_) => println!( - "Could *not* connect to the Mullvad API using access method \"{}\"", - access_method.name - ), + Ok(_) => println!("Success!"), + Err(_) => println!("Failed"), } Ok(()) @@ -185,10 +187,12 @@ impl ApiAccess { Ok(()) } - async fn status() -> Result<()> { + async fn get() -> Result<()> { let mut rpc = MullvadProxyClient::new().await?; let current = rpc.get_current_api_access_method().await?; - println!("{}", pp::ApiAccessMethodFormatter::new(¤t)); + let mut access_method_formatter = pp::ApiAccessMethodFormatter::new(¤t); + access_method_formatter.settings.write_enabled = false; + println!("{}", access_method_formatter); Ok(()) } @@ -206,16 +210,16 @@ impl ApiAccess { #[derive(Subcommand, Debug, Clone)] pub enum AddCustomCommands { - /// Configure a local SOCKS5 proxy + /// Configure a SOCKS5 proxy #[clap(subcommand)] Socks5(AddSocks5Commands), - /// Configure Shadowsocks proxy + /// Configure a custom Shadowsocks proxy to use as an API access method Shadowsocks { /// An easy to remember name for this custom proxy name: String, - /// The IP of the remote Shadowsocks server + /// The IP of the remote Shadowsocks-proxy remote_ip: IpAddr, - /// The port of the remote Shadowsocks server + /// Port on which the remote Shadowsocks-proxy listens for traffic #[arg(default_value = "443")] remote_port: u16, /// Password for authentication @@ -237,9 +241,9 @@ pub enum AddSocks5Commands { Remote { /// An easy to remember name for this custom proxy name: String, - /// The IP of the remote proxy server + /// IP of the remote SOCKS5-proxy remote_ip: IpAddr, - /// The port of the remote proxy server + /// Port on which the remote SOCKS5-proxy listens for traffic remote_port: u16, #[clap(flatten)] authentication: Option, @@ -371,16 +375,15 @@ mod conversions { name: _, disabled: _, } => { - println!("Adding Local SOCKS5-proxy: localhost:{local_port} => {remote_ip}:{remote_port}"); - let socks_proxy = daemon_types::Socks5::Local( - daemon_types::Socks5Local::from_args( - remote_ip.to_string(), - remote_port, - local_port, - ) - .ok_or(anyhow!("Could not create a local Socks5 api proxy"))?, - ); - daemon_types::AccessMethod::from(socks_proxy) + println!("Adding SOCKS5-proxy: localhost:{local_port} => {remote_ip}:{remote_port}"); + daemon_types::Socks5Local::from_args( + remote_ip.to_string(), + remote_port, + local_port, + ) + .map(daemon_types::Socks5::Local) + .map(daemon_types::AccessMethod::from) + .ok_or(anyhow!("Could not create a local Socks5 access method"))? } AddSocks5Commands::Remote { remote_ip, @@ -409,7 +412,7 @@ mod conversions { } .map(daemon_types::Socks5::Remote) .map(daemon_types::AccessMethod::from) - .ok_or(anyhow!("Could not create a remote Socks5 api proxy"))? + .ok_or(anyhow!("Could not create a remote Socks5 access method"))? } }, AddCustomCommands::Shadowsocks { @@ -423,14 +426,14 @@ mod conversions { println!( "Adding Shadowsocks-proxy: {password} @ {remote_ip}:{remote_port} using {cipher}" ); - let shadowsocks_proxy = daemon_types::Shadowsocks::from_args( + daemon_types::Shadowsocks::from_args( remote_ip.to_string(), remote_port, cipher, password, ) - .ok_or(anyhow!("Could not create a Shadowsocks api proxy"))?; - daemon_types::AccessMethod::from(shadowsocks_proxy) + .map(daemon_types::AccessMethod::from) + .ok_or(anyhow!("Could not create a Shadowsocks access method"))? } }) } @@ -445,11 +448,29 @@ mod pp { pub struct ApiAccessMethodFormatter<'a> { api_access_method: &'a AccessMethodSetting, + pub settings: FormatterSettings, + } + + pub struct FormatterSettings { + /// If the formatter should print the enabled status of an + /// [`AcessMethodSetting`] (*) next to its name. + pub write_enabled: bool, + } + + impl Default for FormatterSettings { + fn default() -> Self { + Self { + write_enabled: true, + } + } } impl<'a> ApiAccessMethodFormatter<'a> { pub fn new(api_access_method: &'a AccessMethodSetting) -> ApiAccessMethodFormatter<'a> { - ApiAccessMethodFormatter { api_access_method } + ApiAccessMethodFormatter { + api_access_method, + settings: Default::default(), + } } } @@ -468,12 +489,17 @@ mod pp { match &self.api_access_method.access_method { AccessMethod::BuiltIn(method) => { write!(f, "{}", method.canonical_name())?; - write_status(f, self.api_access_method.enabled()) + if self.settings.write_enabled { + write_status(f, self.api_access_method.enabled())?; + } + Ok(()) } AccessMethod::Custom(method) => match &method { CustomAccessMethod::Shadowsocks(shadowsocks) => { write!(f, "{}", self.api_access_method.get_name())?; - write_status(f, self.api_access_method.enabled())?; + if self.settings.write_enabled { + write_status(f, self.api_access_method.enabled())?; + } writeln!(f)?; print_option!("Protocol", format!("Shadowsocks [{}]", shadowsocks.cipher)); print_option!("Peer", shadowsocks.peer); @@ -483,7 +509,9 @@ mod pp { CustomAccessMethod::Socks5(socks) => match socks { Socks5::Remote(remote) => { write!(f, "{}", self.api_access_method.get_name())?; - write_status(f, self.api_access_method.enabled())?; + if self.settings.write_enabled { + write_status(f, self.api_access_method.enabled())?; + } writeln!(f)?; print_option!("Protocol", "Socks5"); print_option!("Peer", remote.peer); @@ -498,7 +526,9 @@ mod pp { } Socks5::Local(local) => { write!(f, "{}", self.api_access_method.get_name())?; - write_status(f, self.api_access_method.enabled())?; + if self.settings.write_enabled { + write_status(f, self.api_access_method.enabled())?; + } writeln!(f)?; print_option!("Protocol", "Socks5 (local)"); print_option!("Peer", local.peer); diff --git a/mullvad-cli/src/main.rs b/mullvad-cli/src/main.rs index a79465f9658d..7a09a4eebdf7 100644 --- a/mullvad-cli/src/main.rs +++ b/mullvad-cli/src/main.rs @@ -71,9 +71,20 @@ enum Cli { #[clap(subcommand)] Relay(relay::Relay), - /// Manage use of access methods for reaching the Mullvad API. - /// Can be used to connect to the the Mullvad API via one of the - /// Mullvad bridge servers or a custom proxy (SOCKS5 & Shadowsocks). + /// Manage Mullvad API access methods. + /// + /// Access methods are used to connect to the the Mullvad API via one of + /// Mullvad's bridge servers or a custom proxy (SOCKS5 & Shadowsocks) when + /// and where establishing a direct connection does not work. + /// + /// If the Mullvad daemon is unable to connect to the Mullvad API, it will + /// automatically try to use any other configured access method and re-try + /// the API call. If it succeeds, all subsequent API calls are made using + /// the new access method. Otherwise it will re-try using yet another access + /// method. + /// + /// The Mullvad API is used for logging in, accessing the relay list, + /// rotating Wireguard keys and more. #[clap(subcommand)] ApiAccess(api_access::ApiAccess),