Refactor protobuf `ApiAccessMethod` definitions

- Replace rpcs `ReplaceApiAccessMethod` and `ToggleApiAccessMethod` in
favor of a commmon `UpdateApiAccessMethod` (which resembles
`ReplaceApiAccessMethod` in a lot of ways).
- `UpdateApiAccessMethod` works with unique identifiers instead of array
indices to pinpoint which API access method to update.
- Toggling an API access method to be enabled/disabled now happens via `UpdateApiAccessMethod`
- Add the useful `UUID` protobuf type definition, which
conveys more information that a raw string.
- Refactor `SetApiAccessMethod` to use API access method ID
- Update `ApiAcessMethod` messages to use `UUID` type for uuid values
- Use unique id for removing custom `ApiAccessMethods`

4 files changed, 165 insertions, 93 deletions

diff --git a/mullvad-management-interface/proto/management_interface.proto b/mullvad-management-interface/proto/management_interface.proto
index e923ec1043..ad982ea556 100644
--- a/mullvad-management-interface/proto/management_interface.proto
+++ b/mullvad-management-interface/proto/management_interface.proto
@@ -81,13 +81,10 @@ service ManagementService {
   rpc RemoveApiAccessMethod(ApiAccessMethod) returns (google.protobuf.Empty) {
     // Can I return something useful here instead of Empty?
   }
-  rpc ReplaceApiAccessMethod(ApiAccessMethodReplace) returns (google.protobuf.Empty) {
+  rpc UpdateApiAccessMethod(ApiAccessMethodUpdate) returns (google.protobuf.Empty) {
     // Can I return something useful here instead of Empty?
   }
-  rpc ToggleApiAccessMethod(ApiAccessMethodToggle) returns (google.protobuf.Empty) {
-    // Can I return something useful here instead of Empty?
-  }
-  rpc SetApiAccessMethod(ApiAccessMethod) returns (google.protobuf.Empty) {
+  rpc SetApiAccessMethod(UUID) returns (google.protobuf.Empty) {
     // Can I return something useful here instead of Empty?
   }
 
@@ -111,6 +108,8 @@ service ManagementService {
   rpc CheckVolumes(google.protobuf.Empty) returns (google.protobuf.Empty) {}
 }
 
+message UUID { string value = 1; }
+
 message RelaySettingsUpdate {
   oneof type {
     CustomRelaySettings custom = 1;
@@ -336,16 +335,6 @@ message ObfuscationSettings {
   Udp2TcpObfuscationSettings udp2tcp = 2;
 }
 
-message ApiAccessMethodReplace {
-  uint32 index = 1;
-  ApiAccessMethod access_method = 2;
-}
-
-message ApiAccessMethodToggle {
-  ApiAccessMethod access_method = 1;
-  bool enable = 2;
-}
-
 message CustomList {
   string id = 1;
   string name = 2;
@@ -358,31 +347,30 @@ message ApiAccessMethod {
   message Direct {}
   message Bridges {}
   message Socks5Local {
-    string id = 3;
     string ip = 4;
     uint32 port = 5;
     uint32 local_port = 6;
   }
   message Socks5Remote {
-    string id = 1;
     string ip = 2;
     uint32 port = 3;
   }
   message Shadowsocks {
-    string id = 1;
     string ip = 2;
     uint32 port = 3;
     string password = 4;
     string cipher = 5;
   }
-  string name = 1;
-  bool enabled = 2;
+
+  UUID id = 1;
+  string name = 2;
+  bool enabled = 3;
   oneof access_method {
-    Direct direct = 3;
-    Bridges bridges = 4;
-    Socks5Local socks5local = 5;
-    Socks5Remote socks5remote = 6;
-    Shadowsocks shadowsocks = 7;
+    Direct direct = 4;
+    Bridges bridges = 5;
+    Socks5Local socks5local = 6;
+    Socks5Remote socks5remote = 7;
+    Shadowsocks shadowsocks = 8;
   }
 }
 
@@ -390,6 +378,11 @@ message ApiAccessMethods { repeated ApiAccessMethod api_access_methods = 1; }
 
 message ApiAccessMethodSettings { repeated ApiAccessMethod api_access_methods = 1; }
 
+message ApiAccessMethodUpdate {
+  UUID id = 1;
+  ApiAccessMethod access_method = 2;
+}
+
 message Settings {
   RelaySettings relay_settings = 1;
   BridgeSettings bridge_settings = 2;
diff --git a/mullvad-management-interface/src/client.rs b/mullvad-management-interface/src/client.rs
index 315e3276fc..c79b4d2d80 100644
--- a/mullvad-management-interface/src/client.rs
+++ b/mullvad-management-interface/src/client.rs
@@ -3,10 +3,7 @@
 use crate::types;
 use futures::{Stream, StreamExt};
 use mullvad_types::{
-    access_method::{
-        daemon::{ApiAccessMethodReplace, ApiAccessMethodToggle},
-        ApiAccessMethod,
-    },
+    access_method::{daemon::ApiAccessMethodUpdate, ApiAccessMethod, ApiAccessMethodId},
     account::{AccountData, AccountToken, VoucherSubmission},
     custom_list::{CustomList, Id},
     device::{Device, DeviceEvent, DeviceId, DeviceState, RemoveDeviceEvent},
@@ -181,6 +178,26 @@ impl MullvadProxyClient {
             .collect()
     }
 
+    pub async fn get_api_access_method(
+        &mut self,
+        id: &ApiAccessMethodId,
+    ) -> Result<ApiAccessMethod> {
+        self.0
+            .get_api_access_methods(())
+            .await
+            .map_err(Error::Rpc)?
+            .into_inner()
+            .api_access_methods
+            .into_iter()
+            .map(|api_access_method| {
+                ApiAccessMethod::try_from(api_access_method)
+                    .map_err(Error::InvalidResponse)
+                    .expect("Failed to convert proto Api Access Method to daemon representation")
+            })
+            .find(|api_access_method| api_access_method.get_id() == *id)
+            .ok_or(Error::ApiAccessMethodNotFound)
+    }
+
     pub async fn get_api_addressess(&mut self) -> Result<()> {
         self.0.get_api_addressess(()).await.map_err(Error::Rpc)?;
         Ok(())
@@ -488,12 +505,37 @@ impl MullvadProxyClient {
             .map(drop)
     }
 
-    pub async fn toggle_access_method(
+    pub async fn enable_access_method(
         &mut self,
-        access_method_toggle: ApiAccessMethodToggle,
+        api_access_method_id: ApiAccessMethodId,
     ) -> Result<()> {
+        let mut new_api_access_method = self.get_api_access_method(&api_access_method_id).await?;
+        new_api_access_method.enable();
+        let update = ApiAccessMethodUpdate {
+            id: api_access_method_id,
+            access_method: new_api_access_method,
+        };
+
+        self.0
+            .update_api_access_method(types::ApiAccessMethodUpdate::from(update))
+            .await
+            .map_err(Error::Rpc)
+            .map(drop)
+    }
+
+    pub async fn disable_access_method(
+        &mut self,
+        api_access_method_id: ApiAccessMethodId,
+    ) -> Result<()> {
+        let mut new_api_access_method = self.get_api_access_method(&api_access_method_id).await?;
+        new_api_access_method.disable();
+        let update = ApiAccessMethodUpdate {
+            id: api_access_method_id,
+            access_method: new_api_access_method,
+        };
+
         self.0
-            .toggle_api_access_method(types::ApiAccessMethodToggle::from(access_method_toggle))
+            .update_api_access_method(types::ApiAccessMethodUpdate::from(update))
             .await
             .map_err(Error::Rpc)
             .map(drop)
@@ -507,12 +549,12 @@ impl MullvadProxyClient {
             .map(drop)
     }
 
-    pub async fn replace_access_method(
+    pub async fn update_access_method(
         &mut self,
-        access_method_replace: ApiAccessMethodReplace,
+        access_method_update: ApiAccessMethodUpdate,
     ) -> Result<()> {
         self.0
-            .replace_api_access_method(types::ApiAccessMethodReplace::from(access_method_replace))
+            .update_api_access_method(types::ApiAccessMethodUpdate::from(access_method_update))
             .await
             .map_err(Error::Rpc)
             .map(drop)
@@ -527,9 +569,9 @@ impl MullvadProxyClient {
     ///     method "randomly"
     ///
     /// [`ApiConnectionModeProvider`]: mullvad_daemon::api::ApiConnectionModeProvider
-    pub async fn set_access_method(&mut self, api_access_method: ApiAccessMethod) -> Result<()> {
+    pub async fn set_access_method(&mut self, api_access_method: ApiAccessMethodId) -> Result<()> {
         self.0
-            .set_api_access_method(types::ApiAccessMethod::from(api_access_method))
+            .set_api_access_method(types::Uuid::from(api_access_method))
             .await
             .map_err(Error::Rpc)
             .map(drop)
diff --git a/mullvad-management-interface/src/lib.rs b/mullvad-management-interface/src/lib.rs
index cf1a798878..39d5f5df9d 100644
--- a/mullvad-management-interface/src/lib.rs
+++ b/mullvad-management-interface/src/lib.rs
@@ -103,6 +103,9 @@ pub enum Error {
 
     #[error(display = "Location was not found in the custom list")]
     LocationNotFoundInCustomlist,
+
+    #[error(display = "An access method with that id does not exist")]
+    ApiAccessMethodNotFound,
 }
 
 #[deprecated(note = "Prefer MullvadProxyClient")]
diff --git a/mullvad-management-interface/src/types/conversions/api_access_method.rs b/mullvad-management-interface/src/types/conversions/api_access_method.rs
index 28859c5013..e72a4a1f97 100644
--- a/mullvad-management-interface/src/types/conversions/api_access_method.rs
+++ b/mullvad-management-interface/src/types/conversions/api_access_method.rs
@@ -37,55 +37,36 @@ mod settings {
         }
     }
 
-    impl From<access_method::daemon::ApiAccessMethodReplace> for proto::ApiAccessMethodReplace {
-        fn from(value: access_method::daemon::ApiAccessMethodReplace) -> Self {
-            let api_access_method = value.access_method;
-            proto::ApiAccessMethodReplace {
-                index: value.index as u32,
-                access_method: Some(proto::ApiAccessMethod::from(api_access_method)),
+    impl From<access_method::daemon::ApiAccessMethodUpdate> for proto::ApiAccessMethodUpdate {
+        fn from(value: access_method::daemon::ApiAccessMethodUpdate) -> Self {
+            proto::ApiAccessMethodUpdate {
+                id: Some(proto::Uuid::from(value.id)),
+                access_method: Some(proto::ApiAccessMethod::from(value.access_method)),
             }
         }
     }
 
-    impl TryFrom<proto::ApiAccessMethodReplace> for access_method::daemon::ApiAccessMethodReplace {
+    impl TryFrom<proto::ApiAccessMethodUpdate> for access_method::daemon::ApiAccessMethodUpdate {
         type Error = FromProtobufTypeError;
 
-        fn try_from(value: proto::ApiAccessMethodReplace) -> Result<Self, Self::Error> {
-            Ok(access_method::daemon::ApiAccessMethodReplace {
-                index: value.index as usize,
-                access_method: value
-                    .access_method
-                    .ok_or(FromProtobufTypeError::InvalidArgument(
-                        "Could not convert Access Method from protobuf",
-                    ))
-                    .and_then(access_method::ApiAccessMethod::try_from)?,
-            })
-        }
-    }
+        fn try_from(value: proto::ApiAccessMethodUpdate) -> Result<Self, Self::Error> {
+            let api_access_method = value
+                .access_method
+                .ok_or(FromProtobufTypeError::InvalidArgument(
+                    "Could not convert Access Method from protobuf",
+                ))
+                .and_then(access_method::ApiAccessMethod::try_from)?;
 
-    impl From<access_method::daemon::ApiAccessMethodToggle> for proto::ApiAccessMethodToggle {
-        fn from(value: access_method::daemon::ApiAccessMethodToggle) -> Self {
-            let api_access_method = value.access_method;
-            let enabled = api_access_method.enabled();
-            proto::ApiAccessMethodToggle {
-                access_method: Some(proto::ApiAccessMethod::from(api_access_method)),
-                enable: enabled,
-            }
-        }
-    }
+            let id = value
+                .id
+                .ok_or(FromProtobufTypeError::InvalidArgument(
+                    "Could not convert Access Method from protobuf",
+                ))
+                .map(access_method::ApiAccessMethodId::from)?;
 
-    impl TryFrom<proto::ApiAccessMethodToggle> for access_method::daemon::ApiAccessMethodToggle {
-        type Error = FromProtobufTypeError;
-
-        fn try_from(value: proto::ApiAccessMethodToggle) -> Result<Self, Self::Error> {
-            Ok(access_method::daemon::ApiAccessMethodToggle {
-                access_method: value
-                    .access_method
-                    .ok_or(FromProtobufTypeError::InvalidArgument(
-                        "Could not convert Access Method from protobuf",
-                    ))
-                    .and_then(access_method::ApiAccessMethod::try_from)?,
-                enable: value.enable,
+            Ok(access_method::daemon::ApiAccessMethodUpdate {
+                id,
+                access_method: api_access_method,
             })
         }
     }
@@ -97,8 +78,8 @@ mod settings {
 mod data {
     use crate::types::{proto, FromProtobufTypeError};
     use mullvad_types::access_method::{
-        AccessMethod, ApiAccessMethod, BuiltInAccessMethod, ObfuscationProtocol, Shadowsocks,
-        Socks5, Socks5Local, Socks5Remote,
+        AccessMethod, ApiAccessMethod, ApiAccessMethodId, BuiltInAccessMethod, CustomAccessMethod,
+        Shadowsocks, Socks5, Socks5Local, Socks5Remote,
     };
 
     impl TryFrom<proto::ApiAccessMethods> for Vec<ApiAccessMethod> {
@@ -117,8 +98,48 @@ mod data {
         type Error = FromProtobufTypeError;
 
         fn try_from(value: proto::ApiAccessMethod) -> Result<Self, Self::Error> {
+            let id = value
+                .id
+                .ok_or(FromProtobufTypeError::InvalidArgument(
+                    "Could not deserialize Access Method from protobuf",
+                ))
+                .and_then(ApiAccessMethodId::try_from)?;
             let name = value.name;
             let enabled = value.enabled;
+            let access_method = value
+                .access_method
+                .ok_or(FromProtobufTypeError::InvalidArgument(
+                    "Could not deserialize Access Method from protobuf",
+                ))
+                .and_then(AccessMethod::try_from)?;
+
+            Ok(AccessMethodSetting::with_id(
+                id,
+                name,
+                enabled,
+                access_method,
+            ))
+        }
+    }
+
+    impl From<AccessMethodSetting> for proto::ApiAccessMethod {
+        fn from(value: AccessMethodSetting) -> Self {
+            let id = proto::Uuid::from(value.get_id());
+            let name = value.get_name();
+            let enabled = value.enabled();
+            proto::ApiAccessMethod {
+                id: Some(id),
+                name,
+                enabled,
+                access_method: Some(proto::AccessMethod::from(value.access_method)),
+            }
+        }
+    }
+
+    impl TryFrom<proto::AccessMethod> for AccessMethod {
+        type Error = FromProtobufTypeError;
+
+        fn try_from(value: proto::AccessMethod) -> Result<Self, Self::Error> {
             let access_method =
                 value
                     .access_method
@@ -126,7 +147,7 @@ mod data {
                         "Could not deserialize Access Method from protobuf",
                     ))?;
 
-            let x = match access_method {
+            let access_method = match access_method {
                 proto::api_access_method::AccessMethod::Direct(
                     proto::api_access_method::Direct {},
                 ) => AccessMethod::from(BuiltInAccessMethod::Direct),
@@ -164,24 +185,38 @@ mod data {
                 }
             };
 
-            Ok(ApiAccessMethod {
-                name,
-                enabled,
-                access_method: x,
-            })
+            Ok(ApiAccessMethod::with_id(id, name, enabled, access_method))
+        }
+    }
+
+    impl From<ApiAccessMethodId> for proto::Uuid {
+        fn from(value: ApiAccessMethodId) -> Self {
+            proto::Uuid {
+                value: value.to_string(),
+            }
+        }
+    }
+
+    impl TryFrom<proto::Uuid> for ApiAccessMethodId {
+        type Error = FromProtobufTypeError;
+
+        fn try_from(value: proto::Uuid) -> Result<Self, Self::Error> {
+            Self::from_string(value.value).ok_or(FromProtobufTypeError::InvalidArgument(
+                "Could not parse UUID message from protobuf",
+            ))
         }
     }
 
     impl From<ApiAccessMethod> for proto::ApiAccessMethod {
         fn from(value: ApiAccessMethod) -> Self {
+            let id = proto::Uuid::from(value.get_id());
             let name = value.get_name();
             let enabled = value.enabled();
             let access_method = match value.access_method {
-                AccessMethod::Custom(value) => match value.access_method {
-                    ObfuscationProtocol::Shadowsocks(ss) => {
+                AccessMethod::Custom(value) => match value {
+                    CustomAccessMethod::Shadowsocks(ss) => {
                         proto::api_access_method::AccessMethod::Shadowsocks(
                             proto::api_access_method::Shadowsocks {
-                                id: value.id,
                                 ip: ss.peer.ip().to_string(),
                                 port: ss.peer.port() as u32,
                                 password: ss.password,
@@ -189,20 +224,18 @@ mod data {
                             },
                         )
                     }
-                    ObfuscationProtocol::Socks5(Socks5::Local(Socks5Local { peer, port })) => {
+                    CustomAccessMethod::Socks5(Socks5::Local(Socks5Local { peer, port })) => {
                         proto::api_access_method::AccessMethod::Socks5local(
                             proto::api_access_method::Socks5Local {
-                                id: value.id,
                                 ip: peer.ip().to_string(),
                                 port: peer.port() as u32,
                                 local_port: port as u32,
                             },
                         )
                     }
-                    ObfuscationProtocol::Socks5(Socks5::Remote(Socks5Remote { peer })) => {
+                    CustomAccessMethod::Socks5(Socks5::Remote(Socks5Remote { peer })) => {
                         proto::api_access_method::AccessMethod::Socks5remote(
                             proto::api_access_method::Socks5Remote {
-                                id: value.id,
                                 ip: peer.ip().to_string(),
                                 port: peer.port() as u32,
                             },
@@ -224,6 +257,7 @@ mod data {
             };
 
             proto::ApiAccessMethod {
+                id: Some(id),
                 name,
                 enabled,
                 access_method: Some(access_method),