Move access method testing logic to `mullvad-daemon`

Move access method testing logic to `mullvad-daemon`, which means that
the implementation details of how the test works is opaque to whatever
frontend which wants to issue a test of some (configured) access method.

7 files changed, 95 insertions, 70 deletions

diff --git a/mullvad-api/src/lib.rs b/mullvad-api/src/lib.rs
index c0024b22ee..5427f51aad 100644
--- a/mullvad-api/src/lib.rs
+++ b/mullvad-api/src/lib.rs
@@ -620,4 +620,24 @@ impl ApiProxy {
         let response = self.handle.service.request(request).await?;
         response.deserialize().await
     }
+
+    /// Check the availablility of `{APP_URL_PREFIX}/api-addrs`.
+    pub async fn api_addrs_available(&self) -> Result<(), rest::Error> {
+        let service = self.handle.service.clone();
+
+        rest::send_request(
+            &self.handle.factory,
+            service,
+            &format!("{APP_URL_PREFIX}/api-addrs"),
+            Method::HEAD,
+            None,
+            &[StatusCode::OK],
+        )
+        .await?;
+
+        // NOTE: A HEAD request should *not* have a body:
+        // https://developer.mozilla.org/en-US/docs/web/http/methods/head
+        // I.e., no need to deserialize the result.
+        Ok(())
+    }
 }
diff --git a/mullvad-cli/src/cmds/api_access.rs b/mullvad-cli/src/cmds/api_access.rs
index 9ad481c4ef..a826c32b45 100644
--- a/mullvad-cli/src/cmds/api_access.rs
+++ b/mullvad-cli/src/cmds/api_access.rs
@@ -182,25 +182,16 @@ impl ApiAccess {
     /// Test an access method to see if it successfully reaches the Mullvad API.
     async fn test(item: SelectItem) -> Result<()> {
         let mut rpc = MullvadProxyClient::new().await?;
-        // Retrieve the currently used access method. We will reset to this
-        // after we are done testing.
-        let previous_access_method = rpc.get_current_api_access_method().await?;
         let access_method = Self::get_access_method(&mut rpc, &item).await?;
 
         println!("Testing access method \"{}\"", access_method.name);
-        rpc.set_access_method(access_method.get_id()).await?;
-        // Make the daemon perform an network request which involves talking to the Mullvad API.
-        let result = match rpc.get_api_addresses().await {
+        match rpc.test_api_access_method(access_method.get_id()).await {
             Ok(_) => {
                 println!("Success!");
                 Ok(())
             }
-            Err(_) => Err(anyhow!("Could not reach the Mullvad API")),
-        };
-        // In any case, switch back to the previous access method.
-        rpc.set_access_method(previous_access_method.get_id())
-            .await?;
-        result
+            Err(_) => Err(anyhow!("Could not reach the Mullvad API.")),
+        }
     }
 
     /// Try to use of a specific [`AccessMethodSetting`] for subsequent calls to
@@ -217,30 +208,24 @@ impl ApiAccess {
     /// configured ones.
     async fn set(item: SelectItem) -> Result<()> {
         let mut rpc = MullvadProxyClient::new().await?;
-        let previous_access_method = rpc.get_current_api_access_method().await?;
         let mut new_access_method = Self::get_access_method(&mut rpc, &item).await?;
+        let current_access_method = rpc.get_current_api_access_method().await?;
         // Try to reach the API with the newly selected access method.
+        rpc.test_api_access_method(new_access_method.get_id())
+            .await
+            .map_err(|_| {
+                anyhow!("Could not reach the Mullvad API using access method \"{}\". Rolling back to \"{}\"", new_access_method.get_name(), current_access_method.get_name())
+            })?
+
+            ;
+        // If the test succeeded, the new access method should be used from now on.
         rpc.set_access_method(new_access_method.get_id()).await?;
-        match rpc.get_api_addresses().await {
-            Ok(_) => (),
-            Err(_) => {
-                // Roll-back to the previous access method
-                rpc.set_access_method(previous_access_method.get_id())
-                    .await?;
-                return Err(anyhow!(
-                    "Could not reach the Mullvad API using access method \"{}\"",
-                    new_access_method.get_name(),
-                ));
-            }
-        };
-        // It worked! Let the daemon keep using this access method.
-        let display_name = new_access_method.get_name();
+        println!("Using access method \"{}\"", new_access_method.get_name());
         // Toggle the enabled status if needed
         if !new_access_method.enabled() {
             new_access_method.enable();
             rpc.update_access_method(new_access_method).await?;
         }
-        println!("Using access method \"{}\"", display_name);
         Ok(())
     }
 
diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs
index 65f2c56654..c1891bda4d 100644
--- a/mullvad-daemon/src/lib.rs
+++ b/mullvad-daemon/src/lib.rs
@@ -293,8 +293,8 @@ pub enum DaemonCommand {
     UpdateApiAccessMethod(ResponseTx<(), Error>, AccessMethodSetting),
     /// Get the currently used API access method
     GetCurrentAccessMethod(ResponseTx<AccessMethodSetting, Error>),
-    /// Get the addresses of all known API endpoints
-    GetApiAddresses(ResponseTx<Vec<std::net::SocketAddr>, Error>),
+    /// Test an API access method
+    TestApiAccessMethod(ResponseTx<(), Error>, mullvad_types::access_method::Id),
     /// Get information about the currently running and latest app versions
     GetVersionInfo(oneshot::Sender<Option<AppVersionInfo>>),
     /// Return whether the daemon is performing post-upgrade tasks
@@ -1151,7 +1151,7 @@ where
             UpdateApiAccessMethod(tx, method) => self.on_update_api_access_method(tx, method).await,
             GetCurrentAccessMethod(tx) => self.on_get_current_api_access_method(tx),
             SetApiAccessMethod(tx, method) => self.on_set_api_access_method(tx, method).await,
-            GetApiAddresses(tx) => self.on_get_api_addresses(tx).await,
+            TestApiAccessMethod(tx, method) => self.on_test_api_access_method(tx, method).await,
             IsPerformingPostUpgrade(tx) => self.on_is_performing_post_upgrade(tx),
             GetCurrentVersion(tx) => self.on_get_current_version(tx),
             #[cfg(not(target_os = "android"))]
@@ -2381,11 +2381,57 @@ where
         Self::oneshot_send(tx, result, "get_current_api_access_method response");
     }
 
-    async fn on_get_api_addresses(&mut self, tx: ResponseTx<Vec<std::net::SocketAddr>, Error>) {
-        let api_proxy = mullvad_api::ApiProxy::new(self.api_handle.clone());
-        let result = api_proxy.get_api_addrs().await.map_err(Error::RestError);
+    /// Try to reach the Mullvad API using a specific access method, returning
+    /// an [`Error`] in the case where the test fails to reach the API.
+    ///
+    /// Ephemerally sets a new access method (associated with `access_method`)
+    /// to be used for subsequent API calls, before performing an API call and
+    /// switching back to the previously active access method. The previous
+    /// access method is *always* reset.
+    async fn on_test_api_access_method(
+        &mut self,
+        tx: ResponseTx<(), Error>,
+        access_method: mullvad_types::access_method::Id,
+    ) {
+        // NOTE: Preferably we would block all new API calls until the test is
+        // done and the previous access method is reset. Otherwise we run the
+        // risk of errounously triggering a rotation of the currently in-use
+        // access method.
+        let result = async {
+            // Setup test
+            let previous_access_method = self
+                .get_current_access_method()
+                .map_err(Error::AccessMethodError)?;
+
+            self.set_api_access_method(access_method)
+                .await
+                .map_err(Error::AccessMethodError)?;
+            // Perform test
+            //
+            // Send a HEAD request to some Mullvad API endpoint. We issue a HEAD
+            // request because we are *only* concerned with if we get a reply from
+            // the API, and not with the actual data that the endpoint returns.
+            let result = mullvad_api::ApiProxy::new(self.api_handle.clone())
+                .api_addrs_available()
+                .await
+                .map_err(Error::RestError);
+
+            // Reset test
+            self.set_api_access_method(previous_access_method.get_id())
+                .await
+                .map_err(|err| {
+                    log::error!(
+                        "Could not reset to previous access
+            method after API reachability test was carried out. This should only
+            happen if the previous access method was removed in the meantime."
+                    );
+                    Error::AccessMethodError(err)
+                })?;
+
+            result
+        };
 
-        Self::oneshot_send(tx, result, "on_get_api_adressess response");
+        Self::oneshot_send(tx, result.await, "on_test_api_access_method response");
     }
 
     fn on_get_settings(&self, tx: oneshot::Sender<Settings>) {
diff --git a/mullvad-daemon/src/management_interface.rs b/mullvad-daemon/src/management_interface.rs
index f042a923e5..633bcc0945 100644
--- a/mullvad-daemon/src/management_interface.rs
+++ b/mullvad-daemon/src/management_interface.rs
@@ -693,13 +693,13 @@ impl ManagementService for ManagementServiceImpl {
             .map_err(map_daemon_error)
     }
 
-    async fn get_api_addresses(&self, _: Request<()>) -> ServiceResult<types::ApiAddresses> {
-        log::debug!("get_api_addresses");
+    async fn test_api_access_method(&self, request: Request<types::Uuid>) -> ServiceResult<()> {
+        log::debug!("test_api_access_method");
         let (tx, rx) = oneshot::channel();
-        self.send_command_to_daemon(DaemonCommand::GetApiAddresses(tx))?;
+        let api_access_method = mullvad_types::access_method::Id::try_from(request.into_inner())?;
+        self.send_command_to_daemon(DaemonCommand::TestApiAccessMethod(tx, api_access_method))?;
         self.wait_for_result(rx)
             .await?
-            .map(types::ApiAddresses::from)
             .map(Response::new)
             .map_err(map_daemon_error)
     }
diff --git a/mullvad-management-interface/proto/management_interface.proto b/mullvad-management-interface/proto/management_interface.proto
index a27698f317..bb426afdea 100644
--- a/mullvad-management-interface/proto/management_interface.proto
+++ b/mullvad-management-interface/proto/management_interface.proto
@@ -22,7 +22,6 @@ service ManagementService {
 
   rpc GetCurrentVersion(google.protobuf.Empty) returns (google.protobuf.StringValue) {}
   rpc GetVersionInfo(google.protobuf.Empty) returns (AppVersionInfo) {}
-  rpc GetApiAddresses(google.protobuf.Empty) returns (ApiAddresses) {}
 
   rpc IsPerformingPostUpgrade(google.protobuf.Empty) returns (google.protobuf.BoolValue) {}
 
@@ -82,6 +81,7 @@ service ManagementService {
   rpc SetApiAccessMethod(UUID) returns (google.protobuf.Empty) {}
   rpc UpdateApiAccessMethod(AccessMethodSetting) returns (google.protobuf.Empty) {}
   rpc GetCurrentApiAccessMethod(google.protobuf.Empty) returns (AccessMethodSetting) {}
+  rpc TestApiAccessMethod(UUID) returns (google.protobuf.Empty) {}
 
   // Split tunneling (Linux)
   rpc GetSplitTunnelProcesses(google.protobuf.Empty) returns (stream google.protobuf.Int32Value) {}
@@ -110,8 +110,6 @@ message AccountData { google.protobuf.Timestamp expiry = 1; }
 
 message AccountHistory { google.protobuf.StringValue token = 1; }
 
-message ApiAddresses { repeated google.protobuf.StringValue api_addresses = 1; }
-
 message VoucherSubmission {
   uint64 seconds_added = 1;
   google.protobuf.Timestamp new_expiry = 2;
diff --git a/mullvad-management-interface/src/client.rs b/mullvad-management-interface/src/client.rs
index 64ee088d18..c24f30260e 100644
--- a/mullvad-management-interface/src/client.rs
+++ b/mullvad-management-interface/src/client.rs
@@ -208,15 +208,12 @@ impl MullvadProxyClient {
             })
     }
 
-    pub async fn get_api_addresses(&mut self) -> Result<Vec<std::net::SocketAddr>> {
+    pub async fn test_api_access_method(&mut self, id: access_method::Id) -> Result<()> {
         self.0
-            .get_api_addresses(())
+            .test_api_access_method(types::Uuid::from(id))
             .await
-            .map_err(Error::Rpc)
-            .map(tonic::Response::into_inner)
-            .and_then(|api_addresses| {
-                Vec::<std::net::SocketAddr>::try_from(api_addresses).map_err(Error::InvalidResponse)
-            })
+            .map_err(Error::Rpc)?;
+        Ok(())
     }
 
     pub async fn update_relay_locations(&mut self) -> Result<()> {
diff --git a/mullvad-management-interface/src/types/conversions/net.rs b/mullvad-management-interface/src/types/conversions/net.rs
index 7b24a8f2b4..e2df5553a0 100644
--- a/mullvad-management-interface/src/types/conversions/net.rs
+++ b/mullvad-management-interface/src/types/conversions/net.rs
@@ -163,27 +163,6 @@ impl From<proto::IpVersion> for talpid_types::net::IpVersion {
     }
 }
 
-impl TryFrom<proto::ApiAddresses> for Vec<SocketAddr> {
-    type Error = FromProtobufTypeError;
-
-    fn try_from(value: proto::ApiAddresses) -> Result<Self, Self::Error> {
-        value
-            .api_addresses
-            .iter()
-            .map(|api_address| api_address.parse::<SocketAddr>())
-            .collect::<Result<_, _>>()
-            .map_err(|_| FromProtobufTypeError::InvalidArgument("Invalid socket address"))
-    }
-}
-
-impl From<Vec<SocketAddr>> for proto::ApiAddresses {
-    fn from(value: Vec<SocketAddr>) -> Self {
-        Self {
-            api_addresses: value.iter().map(SocketAddr::to_string).collect(),
-        }
-    }
-}
-
 pub fn try_tunnel_type_from_i32(
     tunnel_type: i32,
 ) -> Result<talpid_types::net::TunnelType, FromProtobufTypeError> {