Re-implement test procedure for access methods

Since the `AccessModeSelector` knows how to resolve endpoints on it's
own, we no longer have to re-use the existing `MullvadRestHandle` from
the daemon. Instead, we may spawn a completely new `ApiProxy` and except
the appropriate endpoint in the firewall without affecting the running
`MullvadRestHandle`.

2 files changed, 69 insertions, 122 deletions

diff --git a/mullvad-daemon/src/access_method.rs b/mullvad-daemon/src/access_method.rs
index d5c1bd80e4..4bdbd17f15 100644
--- a/mullvad-daemon/src/access_method.rs
+++ b/mullvad-daemon/src/access_method.rs
@@ -1,9 +1,9 @@
 use crate::{
-    api::{self, AccessModeSelectorHandle},
+    api,
     settings::{self, MadeChanges},
     Daemon, EventListener,
 };
-use mullvad_api::rest::{self, MullvadRestHandle};
+use mullvad_api::rest;
 use mullvad_types::{
     access_method::{self, AccessMethod, AccessMethodSetting},
     settings::Settings,
@@ -224,102 +224,4 @@ where
         };
         self
     }
-
-    /// The semantics of the [`Command`] datastructure.
-    async fn process_command(&mut self, command: Command) -> Result<(), Error> {
-        match command {
-            Command::Nothing => Ok(()),
-            Command::Rotate => self.force_api_endpoint_rotation().await,
-            Command::Set(id) => self.set_api_access_method(id).await,
-        }
-    }
-}
-
-/// 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.
-pub async fn test_access_method(
-    new_access_method: AccessMethodSetting,
-    access_mode_selector: AccessModeSelectorHandle,
-    rest_handle: MullvadRestHandle,
-) -> Result<bool, Error> {
-    // Setup test
-    let previous_access_method = access_mode_selector
-        .get_access_method()
-        .await
-        .map_err(Error::ConnectionMode)?;
-
-    let method_under_test = new_access_method.clone();
-    access_mode_selector
-        .set_access_method(new_access_method)
-        .await
-        .map_err(Error::ConnectionMode)?;
-
-    // We need to perform a rotation of API endpoint after a set action
-    let rotation_handle = rest_handle.clone();
-    rotation_handle
-        .service()
-        .next_api_endpoint()
-        .await
-        .map_err(|err| {
-            log::error!("Failed to rotate API endpoint: {err}");
-            Error::Rest(err)
-        })?;
-
-    // Set up the reset
-    //
-    // In case the API call fails, the next API endpoint will
-    // automatically be selected, which means that we need to set up
-    // with the previous API endpoint beforehand.
-    access_mode_selector
-        .set_access_method(previous_access_method)
-        .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::ConnectionMode(err)
-        })?;
-
-    // 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(rest_handle)
-        .api_addrs_available()
-        .await
-        .map_err(Error::Rest)?;
-
-    // We need to perform a rotation of API endpoint after a set action
-    // Note that this will be done automatically if the API call fails,
-    // so it only has to be done if the call succeeded ..
-    if result {
-        rotation_handle
-            .service()
-            .next_api_endpoint()
-            .await
-            .map_err(|err| {
-                log::error!("Failed to rotate API endpoint: {err}");
-                Error::Rest(err)
-            })?;
-    }
-
-    log::info!(
-        "The result of testing {method:?} is {result}",
-        method = method_under_test.access_method,
-        result = if result {
-            "success".to_string()
-        } else {
-            "failed".to_string()
-        }
-    );
-
-    Ok(result)
 }
diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs
index b760c46d86..0d02a068c4 100644
--- a/mullvad-daemon/src/lib.rs
+++ b/mullvad-daemon/src/lib.rs
@@ -1237,7 +1237,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,
-            TestApiAccessMethod(tx, method) => self.on_test_api_access_method(tx, method),
+            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"))]
@@ -2458,35 +2458,80 @@ where
         });
     }
 
-    fn on_test_api_access_method(
+    async fn on_test_api_access_method(
         &mut self,
         tx: ResponseTx<bool, 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 api_handle = self.api_handle.clone();
-        let handle = self.connection_modes_handler.clone();
-        let access_method_lookup = self
-            .get_api_access_method(access_method)
-            .map_err(Error::AccessMethodError);
+        let reply =
+            |response| Self::oneshot_send(tx, response, "on_test_api_access_method response");
 
-        match access_method_lookup {
-            Ok(access_method) => {
-                tokio::spawn(async move {
-                    let result =
-                        access_method::test_access_method(access_method, handle, api_handle)
-                            .await
-                            .map_err(Error::AccessMethodError);
-                    Self::oneshot_send(tx, result, "on_test_api_access_method response");
-                });
+        let access_method = match self.get_api_access_method(access_method) {
+            Ok(x) => x,
+            Err(err) => {
+                reply(Err(Error::AccessMethodError(err)));
+                return;
             }
+        };
+
+        let test_subject = match self.connection_modes_handler.resolve(access_method).await {
+            Ok(test_subject) => test_subject,
             Err(err) => {
-                Self::oneshot_send(tx, Err(err), "on_test_api_access_method response");
+                reply(Err(Error::ApiConnectionModeError(err)));
+                return;
             }
-        }
+        };
+
+        let test_subject_name = test_subject.setting.name.clone();
+        let proxy_provider = test_subject.connection_mode.clone().into_repeat();
+        let rest_handle = self.api_runtime.mullvad_rest_handle(proxy_provider).await;
+        let api_proxy = mullvad_api::ApiProxy::new(rest_handle);
+        let daemon_event_sender = self.tx.to_specialized_sender();
+        let access_method_selector = self.connection_modes_handler.clone();
+
+        tokio::spawn(async move {
+            let result = async move {
+                // Send an internal daemon event which will punch a hole in the firewall
+                // for the connection mode we are testing.
+                let _ = api::NewAccessMethodEvent::new(test_subject.setting, test_subject.endpoint)
+                    .announce(false)
+                    .send(daemon_event_sender.clone())
+                    .await;
+
+                // 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 = api_proxy
+                    .api_addrs_available()
+                    .await
+                    .map_err(Error::RestError);
+
+                // Tell the daemon to reset the hole we just punched to whatever was in
+                // place before.
+                let active = access_method_selector
+                    .get_current()
+                    .await
+                    .map_err(Error::ApiConnectionModeError)?;
+                let _ = api::NewAccessMethodEvent::new(active.setting, active.endpoint)
+                    .announce(false)
+                    .send(daemon_event_sender.clone())
+                    .await;
+
+                result
+            }
+            .await;
+
+            log::debug!(
+                "API access method {method} {verdict}",
+                method = test_subject_name,
+                verdict = match result {
+                    Ok(true) => "could successfully connect to the Mullvad API",
+                    _ => "could not connect to the Mullvad API",
+                }
+            );
+
+            reply(result);
+        });
     }
 
     fn on_get_settings(&self, tx: oneshot::Sender<Settings>) {