Merge branch 'fix/rename-ip-and-port-socks5local'

6 files changed, 114 insertions, 143 deletions

diff --git a/mullvad-api/src/https_client_with_sni.rs b/mullvad-api/src/https_client_with_sni.rs
index 17d9f7f0d8..7d559faa7a 100644
--- a/mullvad-api/src/https_client_with_sni.rs
+++ b/mullvad-api/src/https_client_with_sni.rs
@@ -255,7 +255,10 @@ impl TryFrom<ApiConnectionMode> for InnerConnectionMode {
                 ProxyConfig::Socks(config) => match config {
                     access_method::Socks5::Local(config) => {
                         InnerConnectionMode::Socks5(SocksConfig {
-                            peer: SocketAddr::new(IpAddr::from(Ipv4Addr::LOCALHOST), config.port),
+                            peer: SocketAddr::new(
+                                IpAddr::from(Ipv4Addr::LOCALHOST),
+                                config.local_port,
+                            ),
                             authentication: SocksAuth::None,
                         })
                     }
diff --git a/mullvad-api/src/proxy.rs b/mullvad-api/src/proxy.rs
index 44a2309587..3193fdc19b 100644
--- a/mullvad-api/src/proxy.rs
+++ b/mullvad-api/src/proxy.rs
@@ -46,7 +46,7 @@ impl ProxyConfig {
         match self {
             ProxyConfig::Shadowsocks(ss) => ss.peer,
             ProxyConfig::Socks(socks) => match socks {
-                access_method::Socks5::Local(s) => s.peer,
+                access_method::Socks5::Local(s) => s.remote_peer,
                 access_method::Socks5::Remote(s) => s.peer,
             },
         }
@@ -60,7 +60,11 @@ impl fmt::Display for ProxyConfig {
             ProxyConfig::Shadowsocks(ss) => write!(f, "Shadowsocks {}/TCP", ss.peer),
             ProxyConfig::Socks(socks) => match socks {
                 access_method::Socks5::Local(s) => {
-                    write!(f, "Socks5 {}/TCP via localhost:{}", s.peer, s.port)
+                    write!(
+                        f,
+                        "Socks5 {}/TCP via localhost:{}",
+                        s.remote_peer, s.local_port
+                    )
                 }
                 access_method::Socks5::Remote(s) => write!(f, "Socks5 {}/TCP", s.peer),
             },
diff --git a/mullvad-cli/src/cmds/api_access.rs b/mullvad-cli/src/cmds/api_access.rs
index dc736c71c6..55c4a9b516 100644
--- a/mullvad-cli/src/cmds/api_access.rs
+++ b/mullvad-cli/src/cmds/api_access.rs
@@ -83,7 +83,7 @@ impl ApiAccess {
         let mut rpc = MullvadProxyClient::new().await?;
         let name = cmd.name().to_string();
         let enabled = cmd.enabled();
-        let access_method = AccessMethod::try_from(cmd)?;
+        let access_method = AccessMethod::from(cmd);
         rpc.add_access_method(name, enabled, access_method).await?;
         Ok(())
     }
@@ -99,6 +99,9 @@ impl ApiAccess {
 
     /// Edit the data of an API access method.
     async fn edit(cmd: EditCustomCommands) -> Result<()> {
+        use mullvad_types::access_method::{
+            Shadowsocks, Socks5, Socks5Local, Socks5Remote, SocksAuth,
+        };
         let mut rpc = MullvadProxyClient::new().await?;
         let mut api_access_method = Self::get_access_method(&mut rpc, &cmd.item).await?;
 
@@ -107,38 +110,31 @@ impl ApiAccess {
             None => return Err(anyhow!("Can not edit built-in access method")),
             Some(x) => match x.clone() {
                 CustomAccessMethod::Shadowsocks(shadowsocks) => {
-                    let ip = cmd.params.ip.unwrap_or(shadowsocks.peer.ip()).to_string();
+                    let ip = cmd.params.ip.unwrap_or(shadowsocks.peer.ip());
                     let port = cmd.params.port.unwrap_or(shadowsocks.peer.port());
                     let password = cmd.params.password.unwrap_or(shadowsocks.password);
                     let cipher = cmd.params.cipher.unwrap_or(shadowsocks.cipher);
-                    mullvad_types::access_method::Shadowsocks::from_args(ip, port, cipher, password)
-                        .map(AccessMethod::from)
+                    AccessMethod::from(Shadowsocks::new((ip, port), cipher, password))
                 }
                 CustomAccessMethod::Socks5(socks) => match socks {
-                    mullvad_types::access_method::Socks5::Local(local) => {
-                        let ip = cmd.params.ip.unwrap_or(local.peer.ip()).to_string();
-                        let port = cmd.params.port.unwrap_or(local.peer.port());
-                        let local_port = cmd.params.local_port.unwrap_or(local.port);
-                        mullvad_types::access_method::Socks5Local::from_args(ip, port, local_port)
-                            .map(AccessMethod::from)
+                    Socks5::Local(local) => {
+                        let remote_ip = cmd.params.ip.unwrap_or(local.remote_peer.ip());
+                        let remote_port = cmd.params.port.unwrap_or(local.remote_peer.port());
+                        let local_port = cmd.params.local_port.unwrap_or(local.local_port);
+                        AccessMethod::from(Socks5Local::new((remote_ip, remote_port), local_port))
                     }
-                    mullvad_types::access_method::Socks5::Remote(remote) => {
-                        let ip = cmd.params.ip.unwrap_or(remote.peer.ip()).to_string();
+                    Socks5::Remote(remote) => {
+                        let ip = cmd.params.ip.unwrap_or(remote.peer.ip());
                         let port = cmd.params.port.unwrap_or(remote.peer.port());
-                        match remote.authentication {
-                            None => mullvad_types::access_method::Socks5Remote::from_args(ip, port),
-                            Some(mullvad_types::access_method::SocksAuth {
-                                username,
-                                password,
-                            }) => {
+                        AccessMethod::from(match remote.authentication {
+                            None => Socks5Remote::new((ip, port)),
+                            Some(SocksAuth { username, password }) => {
                                 let username = cmd.params.username.unwrap_or(username);
                                 let password = cmd.params.password.unwrap_or(password);
-                                mullvad_types::access_method::Socks5Remote::from_args_with_password(
-                                    ip, port, username, password,
-                                )
+                                let auth = SocksAuth { username, password };
+                                Socks5Remote::new_with_authentication((ip, port), auth)
                             }
-                        }
-                        .map(AccessMethod::from)
+                        })
                     }
                 },
             },
@@ -147,9 +143,7 @@ impl ApiAccess {
         if let Some(name) = cmd.params.name {
             api_access_method.name = name;
         };
-        if let Some(access_method) = access_method {
-            api_access_method.access_method = access_method;
-        }
+        api_access_method.access_method = access_method;
 
         rpc.update_access_method(api_access_method).await?;
 
@@ -223,9 +217,8 @@ impl ApiAccess {
                 rpc.set_access_method(previous_access_method.get_id())
                     .await?;
                 return Err(anyhow!(
-                    "Could not reach the Mullvad API using access method \"{}\". Rolling back to \"{}\"",
+                    "Could not reach the Mullvad API using access method \"{}\"",
                     new_access_method.get_name(),
-                    previous_access_method.get_name()
                 ));
             }
         };
@@ -412,16 +405,12 @@ pub struct EditParams {
 /// Since these are not supposed to be used outside of the CLI,
 /// we define them in a hidden-away module.
 mod conversions {
-    use anyhow::{anyhow, Error};
-    use mullvad_types::access_method as daemon_types;
-
     use super::{AddCustomCommands, AddSocks5Commands, SocksAuthentication};
+    use mullvad_types::access_method as daemon_types;
 
-    impl TryFrom<AddCustomCommands> for daemon_types::AccessMethod {
-        type Error = Error;
-
-        fn try_from(value: AddCustomCommands) -> Result<Self, Self::Error> {
-            Ok(match value {
+    impl From<AddCustomCommands> for daemon_types::AccessMethod {
+        fn from(value: AddCustomCommands) -> Self {
+            match value {
                 AddCustomCommands::Socks5(socks) => match socks {
                     AddSocks5Commands::Local {
                         local_port,
@@ -431,14 +420,7 @@ mod conversions {
                         disabled: _,
                     } => {
                         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"))?
+                        daemon_types::Socks5Local::new((remote_ip, remote_port), local_port).into()
                     }
                     AddSocks5Commands::Remote {
                         remote_ip,
@@ -446,29 +428,23 @@ mod conversions {
                         authentication,
                         name: _,
                         disabled: _,
-                    } => {
+                    } => daemon_types::AccessMethod::from(daemon_types::Socks5::Remote(
                         match authentication {
                             Some(SocksAuthentication { username, password }) => {
                                 println!("Adding SOCKS5-proxy: {username}:{password}@{remote_ip}:{remote_port}");
-                                daemon_types::Socks5Remote::from_args_with_password(
-                                    remote_ip.to_string(),
-                                    remote_port,
-                                    username,
-                                    password
+                                let auth =
+                                    mullvad_types::access_method::SocksAuth { username, password };
+                                daemon_types::Socks5Remote::new_with_authentication(
+                                    (remote_ip, remote_port),
+                                    auth,
                                 )
                             }
                             None => {
                                 println!("Adding SOCKS5-proxy: {remote_ip}:{remote_port}");
-                                daemon_types::Socks5Remote::from_args(
-                                    remote_ip.to_string(),
-                                    remote_port,
-                                )
+                                daemon_types::Socks5Remote::new((remote_ip, remote_port))
                             }
-                        }
-                        .map(daemon_types::Socks5::Remote)
-                        .map(daemon_types::AccessMethod::from)
-                        .ok_or(anyhow!("Could not create a remote Socks5 access method"))?
-                    }
+                        },
+                    )),
                 },
                 AddCustomCommands::Shadowsocks {
                     remote_ip,
@@ -481,16 +457,13 @@ mod conversions {
                     println!(
                 "Adding Shadowsocks-proxy: {password} @ {remote_ip}:{remote_port} using {cipher}"
                     );
-                    daemon_types::Shadowsocks::from_args(
-                        remote_ip.to_string(),
-                        remote_port,
+                    daemon_types::AccessMethod::from(daemon_types::Shadowsocks::new(
+                        (remote_ip, remote_port),
                         cipher,
                         password,
-                    )
-                    .map(daemon_types::AccessMethod::from)
-                    .ok_or(anyhow!("Could not create a Shadowsocks access method"))?
+                    ))
                 }
-            })
+            }
         }
     }
 }
@@ -586,8 +559,8 @@ mod pp {
                             }
                             writeln!(f)?;
                             print_option!("Protocol", "Socks5 (local)");
-                            print_option!("Peer", local.peer);
-                            print_option!("Local port", local.port);
+                            print_option!("Peer", local.remote_peer);
+                            print_option!("Local port", local.local_port);
                             Ok(())
                         }
                     },
diff --git a/mullvad-management-interface/proto/management_interface.proto b/mullvad-management-interface/proto/management_interface.proto
index 35417584b3..057718bdb1 100644
--- a/mullvad-management-interface/proto/management_interface.proto
+++ b/mullvad-management-interface/proto/management_interface.proto
@@ -333,8 +333,8 @@ message AccessMethod {
   message Direct {}
   message Bridges {}
   message Socks5Local {
-    string ip = 1;
-    uint32 port = 2;
+    string remote_ip = 1;
+    uint32 remote_port = 2;
     uint32 local_port = 3;
   }
   message SocksAuth {
diff --git a/mullvad-management-interface/src/types/conversions/access_method.rs b/mullvad-management-interface/src/types/conversions/access_method.rs
index 8907c4da29..b4a4547fdf 100644
--- a/mullvad-management-interface/src/types/conversions/access_method.rs
+++ b/mullvad-management-interface/src/types/conversions/access_method.rs
@@ -42,6 +42,8 @@ mod settings {
 /// [`crate::types::proto::ApiAccessMethod`] type to the internal
 /// [`mullvad_types::access_method::AccessMethodSetting`] data type.
 mod data {
+    use std::net::Ipv4Addr;
+
     use crate::types::{proto, FromProtobufTypeError};
     use mullvad_types::access_method::{
         AccessMethod, AccessMethodSetting, BuiltInAccessMethod, CustomAccessMethod, Id,
@@ -142,11 +144,15 @@ mod data {
         type Error = FromProtobufTypeError;
 
         fn try_from(value: proto::access_method::Socks5Local) -> Result<Self, Self::Error> {
-            Socks5Local::from_args(value.ip, value.port as u16, value.local_port as u16)
-                .ok_or(FromProtobufTypeError::InvalidArgument(
+            let remote_ip = value.remote_ip.parse::<Ipv4Addr>().map_err(|_| {
+                FromProtobufTypeError::InvalidArgument(
                     "Could not parse Socks5 (local) message from protobuf",
-                ))
-                .map(AccessMethod::from)
+                )
+            })?;
+            Ok(AccessMethod::from(Socks5Local::new(
+                (remote_ip, value.remote_port as u16),
+                value.local_port as u16,
+            )))
         }
     }
 
@@ -159,19 +165,19 @@ mod data {
                 port,
                 authentication,
             } = value;
-            let port = port as u16;
-            match authentication.map(SocksAuth::from) {
-                Some(SocksAuth { username, password }) => {
-                    Socks5Remote::from_args_with_password(ip, port, username, password)
-                }
-                None => Socks5Remote::from_args(ip, port),
-            }
-            .ok_or({
+            let ip = ip.parse::<Ipv4Addr>().map_err(|_| {
                 FromProtobufTypeError::InvalidArgument(
                     "Could not parse Socks5 (remote) message from protobuf",
                 )
-            })
-            .map(AccessMethod::from)
+            })?;
+            let port = port as u16;
+
+            Ok(AccessMethod::from(
+                match authentication.map(SocksAuth::from) {
+                    Some(auth) => Socks5Remote::new_with_authentication((ip, port), auth),
+                    None => Socks5Remote::new((ip, port)),
+                },
+            ))
         }
     }
 
@@ -179,11 +185,17 @@ mod data {
         type Error = FromProtobufTypeError;
 
         fn try_from(value: proto::access_method::Shadowsocks) -> Result<Self, Self::Error> {
-            Shadowsocks::from_args(value.ip, value.port as u16, value.cipher, value.password)
-                .ok_or(FromProtobufTypeError::InvalidArgument(
-                    "Could not parse Shadowsocks message from protobuf",
-                ))
-                .map(AccessMethod::from)
+            let ip = value.ip.parse::<Ipv4Addr>().map_err(|_| {
+                FromProtobufTypeError::InvalidArgument(
+                    "Could not parse Socks5 (remote) message from protobuf",
+                )
+            })?;
+
+            Ok(AccessMethod::from(Shadowsocks::new(
+                (ip, value.port as u16),
+                value.cipher,
+                value.password,
+            )))
         }
     }
 
@@ -216,15 +228,16 @@ mod data {
                         },
                     )
                 }
-                CustomAccessMethod::Socks5(Socks5::Local(Socks5Local { peer, port })) => {
-                    proto::access_method::AccessMethod::Socks5local(
-                        proto::access_method::Socks5Local {
-                            ip: peer.ip().to_string(),
-                            port: peer.port() as u32,
-                            local_port: port as u32,
-                        },
-                    )
-                }
+                CustomAccessMethod::Socks5(Socks5::Local(Socks5Local {
+                    remote_peer: peer,
+                    local_port: port,
+                })) => proto::access_method::AccessMethod::Socks5local(
+                    proto::access_method::Socks5Local {
+                        remote_ip: peer.ip().to_string(),
+                        remote_port: peer.port() as u32,
+                        local_port: port as u32,
+                    },
+                ),
                 CustomAccessMethod::Socks5(Socks5::Remote(Socks5Remote {
                     peer,
                     authentication,
diff --git a/mullvad-types/src/access_method.rs b/mullvad-types/src/access_method.rs
index 249c097ba1..f4a76392fa 100644
--- a/mullvad-types/src/access_method.rs
+++ b/mullvad-types/src/access_method.rs
@@ -1,7 +1,7 @@
 use std::str::FromStr;
 
 use serde::{Deserialize, Serialize};
-use std::net::{IpAddr, Ipv4Addr, SocketAddr, SocketAddrV4};
+use std::net::SocketAddr;
 
 /// Daemon settings for API access methods.
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
@@ -205,9 +205,9 @@ pub struct Shadowsocks {
 
 #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Hash)]
 pub struct Socks5Local {
-    pub peer: SocketAddr,
+    pub remote_peer: SocketAddr,
     /// Port on localhost where the SOCKS5-proxy listens to.
-    pub port: u16,
+    pub local_port: u16,
 }
 
 #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Hash)]
@@ -241,62 +241,40 @@ impl BuiltInAccessMethod {
 }
 
 impl Shadowsocks {
-    pub fn new(peer: SocketAddr, cipher: String, password: String) -> Self {
+    pub fn new<I: Into<SocketAddr>>(peer: I, cipher: String, password: String) -> Self {
         Shadowsocks {
-            peer,
+            peer: peer.into(),
             password,
             cipher,
         }
     }
-
-    /// Like [new()], but tries to parse `ip` and `port` into a [`std::net::SocketAddr`] for you.
-    /// If `ip` or `port` are valid [`Some(Socks5Local)`] is returned, otherwise [`None`].
-    pub fn from_args(ip: String, port: u16, cipher: String, password: String) -> Option<Self> {
-        let peer = SocketAddrV4::new(Ipv4Addr::from_str(&ip).ok()?, port).into();
-        Some(Self::new(peer, cipher, password))
-    }
 }
 
 impl Socks5Local {
-    pub fn new(peer: SocketAddr, port: u16) -> Self {
-        Self { peer, port }
-    }
-
-    /// Like [new()], but tries to parse `ip` and `port` into a [`std::net::SocketAddr`] for you.
-    /// If `ip` or `port` are valid [`Some(Socks5Local)`] is returned, otherwise [`None`].
-    pub fn from_args(ip: String, port: u16, localport: u16) -> Option<Self> {
-        let peer_ip = IpAddr::V4(Ipv4Addr::from_str(&ip).ok()?);
-        let peer = SocketAddr::new(peer_ip, port);
-        Some(Self::new(peer, localport))
+    pub fn new<I: Into<SocketAddr>>(remote_peer: I, local_port: u16) -> Self {
+        Self {
+            remote_peer: remote_peer.into(),
+            local_port,
+        }
     }
 }
 
 impl Socks5Remote {
-    pub fn new(peer: SocketAddr) -> Self {
+    pub fn new<I: Into<SocketAddr>>(peer: I) -> Self {
         Self {
-            peer,
+            peer: peer.into(),
             authentication: None,
         }
     }
 
-    /// Like [new()], but tries to parse `ip` and `port` into a [`std::net::SocketAddr`] for you.
-    /// If `ip` or `port` are valid [`Some(Socks5Remote)`] is returned, otherwise [`None`].
-    pub fn from_args(ip: String, port: u16) -> Option<Self> {
-        let peer_ip = IpAddr::V4(Ipv4Addr::from_str(&ip).ok()?);
-        let peer = SocketAddr::new(peer_ip, port);
-        Some(Self::new(peer))
-    }
-
-    /// Like [from_args()], but with authentication.
-    pub fn from_args_with_password(
-        ip: String,
-        port: u16,
-        username: String,
-        password: String,
-    ) -> Option<Self> {
-        let mut socks = Self::from_args(ip, port)?;
-        socks.authentication = Some(SocksAuth { username, password });
-        Some(socks)
+    pub fn new_with_authentication<I: Into<SocketAddr>>(
+        peer: I,
+        authentication: SocksAuth,
+    ) -> Self {
+        Self {
+            peer: peer.into(),
+            authentication: Some(authentication),
+        }
     }
 }