From 3e3d0e43471b31bbc9c6a198e9373d6484fc8b2c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20L=C3=B6nnhager?= <david.l@mullvad.net>
Date: Wed, 29 Nov 2023 12:03:12 +0100
Subject: [PATCH 01/14] Move patch feature to unreleased section

---
 CHANGELOG.md | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 072c56d007cf..aafcbc0f27f9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -22,6 +22,9 @@ Line wrap the file at 100 chars.                                              Th
 * **Security**: in case of vulnerabilities.
 
 ## [Unreleased]
+### Added
+- Add CLI support for applying patches to the settings with `mullvad import-settings`.
+
 ### Changed
 #### Android
 - Migrate welcome view to compose.
@@ -51,7 +54,6 @@ Line wrap the file at 100 chars.                                              Th
   `SOCKS5`.
 - Add social media content blocker.
 - Add ability to override server IPs to the CLI.
-- Add CLI support for applying patches to the settings with `mullvad import-settings`.
 
 ### Changed
 - Update Electron from 25.2.0 to 26.3.0.

From b4bff3b51a0576fc3789f8376f3b741dcebf006d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20L=C3=B6nnhager?= <david.l@mullvad.net>
Date: Wed, 29 Nov 2023 12:03:31 +0100
Subject: [PATCH 02/14] Check whether changelog entries are correct in CI

---
 .github/workflows/check-changelog.yml | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 .github/workflows/check-changelog.yml

diff --git a/.github/workflows/check-changelog.yml b/.github/workflows/check-changelog.yml
new file mode 100644
index 000000000000..838629e685dc
--- /dev/null
+++ b/.github/workflows/check-changelog.yml
@@ -0,0 +1,18 @@
+---
+name: Check changelog format
+on:
+  pull_request:
+    paths:
+      - 'CHANGELOG.md'
+env:
+  LINE_LIMIT: 100
+jobs:
+  check-changelog:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v3
+      - name: No lines must exceed ${{ env.LINE_LIMIT }} characters
+        run: |
+          awk 'length($0) > '$LINE_LIMIT' { print NR ": Line exceeds '$LINE_LIMIT' chars: " $0; found=1 } \
+            END { if(found) exit 1 }' CHANGELOG.md

From 2550a7db09fc0de087000badc21e72880b3c2d34 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20L=C3=B6nnhager?= <david.l@mullvad.net>
Date: Wed, 29 Nov 2023 12:18:50 +0100
Subject: [PATCH 03/14] Trim overly long entries in changelog

---
 CHANGELOG.md | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index aafcbc0f27f9..3ce9628a1a02 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -46,7 +46,8 @@ Line wrap the file at 100 chars.                                              Th
 
 ## [2023.6-beta1] - 2023-11-23
 ### Added
-- Add customizable relay lists to the CLI on desktop. Custom lists can be managed through `mullvad custom-lists` and can be selected through `mullvad relay set` and `mullvad bridge set`.
+- Add customizable relay lists to the CLI on desktop. Custom lists can be managed through
+  `mullvad custom-lists` and can be selected through `mullvad relay set` and `mullvad bridge set`.
 - Add custom lists to location selector in desktop app.
 - Add custom API access methods to the CLI on desktop. Custom API access methods allow the user to
   proxy API traffic through a peer before connecting to a tunnel. They are managed through
@@ -57,7 +58,9 @@ Line wrap the file at 100 chars.                                              Th
 
 ### Changed
 - Update Electron from 25.2.0 to 26.3.0.
-- CLI command `mullvad relay set tunnel wireguard entry-location` changed to `mullvad relay set tunnel wireguard entry location`, as the `location` subcommand can now be swapped for `custom-list` to select entry relays using a custom list.
+- CLI command `mullvad relay set tunnel wireguard entry-location` changed to
+  `mullvad relay set tunnel wireguard entry location`, as the `location` subcommand can now be
+  swapped for `custom-list` to select entry relays using a custom list.
 
 #### Linux
 - Don't block forwarding of traffic when the split tunnel mark (ct mark) is set.
@@ -67,7 +70,8 @@ Line wrap the file at 100 chars.                                              Th
 - Remove wireguard-go (userspace WireGuard) support.
 
 ### Fixed
-- Validate that hostname matches correct server type for CLI commands `mullvad relay set location`, `mullvad bridge set location` and `mullvad relay set tunnel wireguard entry location`
+- Validate that hostname matches correct server type for CLI commands `mullvad relay set location`,
+  `mullvad bridge set location`, and `mullvad relay set tunnel wireguard entry location`.
 - Show correct endpoint in CLI for custom relays.
 - Lower risk of being rate limited.
 - Fix error dialog when failing to write to console by handling the thrown error.
@@ -489,8 +493,8 @@ Identical to android/2022.2-beta2 except for updated translations.
 
 ### Security
 - When the system service is being shut down and the target state is _secured_, maintain the
-  blocking firewall rules. Unless it's possible to deduce that the system isn't shutting down and the
-  system service is being stopped by the user intentionally. This is to prevent leaks that might
+  blocking firewall rules. Unless it's possible to deduce that the system isn't shutting down and
+  the system service is being stopped by the user intentionally. This is to prevent leaks that might
   occur during system shutdown. Fixes 2022 Mullvad app audit issue item `MUL22-02`.
 
 #### Windows
@@ -901,8 +905,8 @@ properly after initialization.
 - Move window after dragging tray icon to new position.
 
 #### Linux
-- Greatly simplify behavior around custom DNS when using systemd-resolved, by not setting DNS config on interfaces other
-  than our tunnel interface.
+- Greatly simplify behavior around custom DNS when using systemd-resolved, by not setting DNS config
+  on interfaces other than our tunnel interface.
 
 
 ## [2021.5] - 2021-10-25
@@ -1352,7 +1356,8 @@ This release is for Android only.
 - Fix issues managing DNS when dnsmasq is used with NetworkManager.
 - Fix issues with managing kernel WireGuard device via NetworkManager.
 - Disable NetworkManager's connectivity check before applying firewall rules to avoid triggering
-  NetworkManager's [bug](https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/312#note_453724)
+  NetworkManager's
+  [bug](https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/312#note_453724)
 
 ### Security
 - Restore the last target state if the daemon crashes. Previously, if auto-connect and

From 0d638dbbde30141a2da5ac64556e192fc98b633e Mon Sep 17 00:00:00 2001
From: Markus Pettersson <markus.pettersson@mullvad.net>
Date: Thu, 26 Oct 2023 16:10:45 +0200
Subject: [PATCH 04/14] 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.
---
 mullvad-api/src/lib.rs                        | 20 +++++++
 mullvad-cli/src/cmds/api_access.rs            | 41 ++++---------
 mullvad-daemon/src/lib.rs                     | 60 ++++++++++++++++---
 mullvad-daemon/src/management_interface.rs    |  8 +--
 .../proto/management_interface.proto          |  4 +-
 mullvad-management-interface/src/client.rs    | 11 ++--
 .../src/types/conversions/net.rs              | 21 -------
 7 files changed, 95 insertions(+), 70 deletions(-)

diff --git a/mullvad-api/src/lib.rs b/mullvad-api/src/lib.rs
index c0024b22eea3..5427f51aad45 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 9ad481c4efa6..a826c32b4559 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 65f2c56654e2..c1891bda4d0e 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 f042a923e51e..633bcc09454b 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 a27698f3176b..bb426afdea18 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 64ee088d1877..c24f30260e32 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 7b24a8f2b4ca..e2df5553a04f 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> {

From 4b7ed46ae1945a03bd080e12e8c9ab376dd1cf07 Mon Sep 17 00:00:00 2001
From: Markus Pettersson <markus.pettersson@mullvad.net>
Date: Tue, 14 Nov 2023 12:04:48 +0100
Subject: [PATCH 05/14] Perform testing of access methods asynchronously

Perform testing of access methods asynchronously in a separate `tokio`
task as to not block the daemon from handling other daemon events during
the testing window
---
 mullvad-api/src/lib.rs                        |  24 +-
 mullvad-api/src/rest.rs                       |   4 +
 mullvad-cli/src/cmds/api_access.rs            |   4 +-
 mullvad-daemon/src/access_method.rs           |  67 +++--
 mullvad-daemon/src/api.rs                     | 264 +++++++++++++-----
 mullvad-daemon/src/lib.rs                     | 146 +++++++---
 mullvad-daemon/src/management_interface.rs    |   2 +-
 .../proto/management_interface.proto          |   2 +-
 mullvad-management-interface/src/client.rs    |   7 +-
 9 files changed, 367 insertions(+), 153 deletions(-)

diff --git a/mullvad-api/src/lib.rs b/mullvad-api/src/lib.rs
index 5427f51aad45..c8765ec2b2b1 100644
--- a/mullvad-api/src/lib.rs
+++ b/mullvad-api/src/lib.rs
@@ -622,22 +622,14 @@ impl ApiProxy {
     }
 
     /// 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?;
+    pub async fn api_addrs_available(&self) -> Result<bool, rest::Error> {
+        let request = self
+            .handle
+            .factory
+            .head(&format!("{APP_URL_PREFIX}/api-addrs"))?
+            .expected_status(&[StatusCode::OK]);
 
-        // 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(())
+        let response = self.handle.service.request(request).await?;
+        Ok(response.status().is_success())
     }
 }
diff --git a/mullvad-api/src/rest.rs b/mullvad-api/src/rest.rs
index 2484bec64b01..559ddd4b4e53 100644
--- a/mullvad-api/src/rest.rs
+++ b/mullvad-api/src/rest.rs
@@ -524,6 +524,10 @@ impl RequestFactory {
         self.request(path, Method::DELETE)
     }
 
+    pub fn head(&self, path: &str) -> Result<Request> {
+        self.request(path, Method::HEAD)
+    }
+
     pub fn post_json<S: serde::Serialize>(&self, path: &str, body: &S) -> Result<Request> {
         self.json_request(Method::POST, path, body)
     }
diff --git a/mullvad-cli/src/cmds/api_access.rs b/mullvad-cli/src/cmds/api_access.rs
index a826c32b4559..c6e01c52d689 100644
--- a/mullvad-cli/src/cmds/api_access.rs
+++ b/mullvad-cli/src/cmds/api_access.rs
@@ -186,11 +186,11 @@ impl ApiAccess {
 
         println!("Testing access method \"{}\"", access_method.name);
         match rpc.test_api_access_method(access_method.get_id()).await {
-            Ok(_) => {
+            Ok(true) => {
                 println!("Success!");
                 Ok(())
             }
-            Err(_) => Err(anyhow!("Could not reach the Mullvad API.")),
+            Ok(false) | Err(_) => Err(anyhow!("Could not reach the Mullvad API.")),
         }
     }
 
diff --git a/mullvad-daemon/src/access_method.rs b/mullvad-daemon/src/access_method.rs
index 4584aa374aa1..51a0f0ae4a05 100644
--- a/mullvad-daemon/src/access_method.rs
+++ b/mullvad-daemon/src/access_method.rs
@@ -1,4 +1,5 @@
 use crate::{
+    api,
     settings::{self, MadeChanges},
     Daemon, EventListener,
 };
@@ -25,6 +26,9 @@ pub enum Error {
     /// Access method could not be rotate
     #[error(display = "Access method could not be rotated")]
     RotationError,
+    /// Daemon API error
+    #[error(display = "Daemon API handling error")]
+    Api(#[error(source)] api::Error),
     /// Access methods settings error
     #[error(display = "Settings error")]
     Settings(#[error(source)] settings::Error),
@@ -81,7 +85,9 @@ where
             Some(api_access_method) => {
                 if api_access_method.is_builtin() {
                     Err(Error::RemoveBuiltIn)
-                } else if api_access_method.get_id() == self.get_current_access_method()?.get_id() {
+                } else if api_access_method.get_id()
+                    == self.get_current_access_method().await?.get_id()
+                {
                     Ok(Command::Rotate)
                 } else {
                     Ok(Command::Nothing)
@@ -108,15 +114,10 @@ where
         &mut self,
         access_method: access_method::Id,
     ) -> Result<(), Error> {
-        let access_method = self
-            .settings
-            .api_access_methods
-            .find(&access_method)
-            .ok_or(Error::NoSuchMethod(access_method))?;
-        {
-            let mut connection_modes = self.connection_modes.lock().unwrap();
-            connection_modes.set_access_method(access_method.clone());
-        }
+        let access_method = self.get_api_access_method(access_method)?;
+        self.connection_modes_handler
+            .set_access_method(access_method)
+            .await?;
         // Force a rotation of Access Methods.
         //
         // This is not a call to `process_command` due to the restrictions on
@@ -124,6 +125,17 @@ where
         self.force_api_endpoint_rotation().await
     }
 
+    pub fn get_api_access_method(
+        &mut self,
+        access_method: access_method::Id,
+    ) -> Result<AccessMethodSetting, Error> {
+        self.settings
+            .api_access_methods
+            .find(&access_method)
+            .ok_or(Error::NoSuchMethod(access_method))
+            .cloned()
+    }
+
     /// "Updates" an [`AccessMethodSetting`] by replacing the existing entry
     /// with the argument `access_method_update` if an existing entry with
     /// matching [`access_method::Id`] is found.
@@ -140,7 +152,7 @@ where
         // in the daemon's settings. Therefore, we have to safeguard against
         // this by explicitly checking for & disallow any update which would
         // cause the last enabled access method to become disabled.
-        let current = self.get_current_access_method()?;
+        let current = self.get_current_access_method().await?;
         let mut command = Command::Nothing;
         let settings_update = |settings: &mut Settings| {
             if let Some(access_method) = settings
@@ -165,9 +177,8 @@ where
 
     /// Return the [`AccessMethodSetting`] which is currently used to access the
     /// Mullvad API.
-    pub fn get_current_access_method(&self) -> Result<AccessMethodSetting, Error> {
-        let connections_modes = self.connection_modes.lock().unwrap();
-        Ok(connections_modes.peek())
+    pub async fn get_current_access_method(&self) -> Result<AccessMethodSetting, Error> {
+        Ok(self.connection_modes_handler.get_access_method().await?)
     }
 
     /// Change which [`AccessMethodSetting`] which will be used to figure out
@@ -189,7 +200,8 @@ where
             self.event_listener
                 .notify_settings(self.settings.to_settings());
 
-            let access_methods: Vec<_> = self
+            let handle = self.connection_modes_handler.clone();
+            let new_access_methods = self
                 .settings
                 .api_access_methods
                 .access_method_settings
@@ -198,20 +210,19 @@ where
                 .cloned()
                 .collect();
 
-            let mut connection_modes = self.connection_modes.lock().unwrap();
-            match connection_modes.update_access_methods(access_methods) {
-                Ok(_) => (),
-                Err(crate::api::Error::NoAccessMethods) => {
-                    // `access_methods` was empty! This implies that the user
-                    // disabled all access methods. If we ever get into this
-                    // state, we should default to using the direct access
-                    // method.
-                    let default = access_method::Settings::direct();
-                    connection_modes
-                        .update_access_methods(vec![default])
-                        .expect("Failed to create the data structure responsible for managing access methods");
+            tokio::spawn(async move {
+                match handle.update_access_methods(new_access_methods).await {
+                    Ok(_) => (),
+                    Err(crate::api::Error::NoAccessMethods) => {
+                        // `access_methods` was empty! This implies that the user
+                        // disabled all access methods. If we ever get into this
+                        // state, we should default to using the direct access
+                        // method.
+                        let default = access_method::Settings::direct();
+                        handle.update_access_methods(vec![default]).await.expect("Failed to create the data structure responsible for managing access methods");
+                    }
                 }
-            }
+            });
         };
         self
     }
diff --git a/mullvad-daemon/src/api.rs b/mullvad-daemon/src/api.rs
index d5099ae74a20..d309854473ac 100644
--- a/mullvad-daemon/src/api.rs
+++ b/mullvad-daemon/src/api.rs
@@ -2,7 +2,8 @@
 use crate::{DaemonCommand, DaemonEventSender};
 use futures::{
     channel::{mpsc, oneshot},
-    Future, Stream, StreamExt,
+    stream::unfold,
+    Stream, StreamExt,
 };
 use mullvad_api::{
     availability::ApiAvailabilityHandle,
@@ -13,17 +14,205 @@ use mullvad_relay_selector::RelaySelector;
 use mullvad_types::access_method::{self, AccessMethod, AccessMethodSetting, BuiltInAccessMethod};
 use std::{
     path::PathBuf,
-    pin::Pin,
     sync::{Arc, Mutex, Weak},
-    task::Poll,
 };
 #[cfg(target_os = "android")]
 use talpid_core::mpsc::Sender;
 use talpid_core::tunnel_state_machine::TunnelCommand;
-use talpid_types::{
-    net::{openvpn::ProxySettings, AllowedEndpoint, Endpoint},
-    ErrorExt,
-};
+use talpid_types::net::{openvpn::ProxySettings, AllowedEndpoint, Endpoint};
+
+// TODO(markus): Remove text
+/// Here, a new agent was born.
+
+pub enum Message {
+    Get(ResponseTx<AccessMethodSetting>),
+    Set(ResponseTx<()>, AccessMethodSetting),
+    Next(ResponseTx<ApiConnectionMode>),
+    Update(ResponseTx<()>, Vec<AccessMethodSetting>),
+}
+
+#[derive(err_derive::Error, Debug)]
+pub enum Error {
+    /// Oddly specific.
+    #[error(display = "Very Generic error.")]
+    Generic,
+}
+
+#[derive(Clone)]
+pub struct Ehandle {
+    cmd_tx: mpsc::UnboundedSender<Message>,
+}
+
+impl Ehandle {
+    pub fn new(
+        cache_dir: PathBuf,
+        relay_selector: RelaySelector,
+        connection_modes: Vec<AccessMethodSetting>,
+    ) -> Self {
+        let (cmd_tx, cmd_rx) = mpsc::unbounded();
+
+        let mut actor = EActor {
+            cmd_rx,
+            state: ApiConnectionModeProvider::new(cache_dir, relay_selector, connection_modes),
+        };
+        tokio::spawn(async move { actor.run().await });
+        Self { cmd_tx }
+    }
+
+    async fn send_command<T>(&self, make_cmd: impl FnOnce(ResponseTx<T>) -> Message) -> Result<T> {
+        let (tx, rx) = oneshot::channel();
+        // TODO(markus): Error handling
+        self.cmd_tx.unbounded_send(make_cmd(tx)).unwrap();
+        // TODO(markus): Error handling
+        rx.await.unwrap()
+    }
+
+    pub async fn get_access_method(&self) -> Result<AccessMethodSetting> {
+        self.send_command(Message::Get).await.map_err(|err| {
+            log::error!("Failed to get current access method!");
+            err
+        })
+    }
+
+    pub async fn set_access_method(&self, value: AccessMethodSetting) -> Result<()> {
+        self.send_command(|tx| Message::Set(tx, value))
+            .await
+            .map_err(|err| {
+                log::error!("Failed to set new access method!");
+                err
+            })
+    }
+
+    pub async fn update_access_methods(&self, values: Vec<AccessMethodSetting>) -> Result<()> {
+        self.send_command(|tx| Message::Update(tx, values))
+            .await
+            .map_err(|err| {
+                log::error!("Failed to update new access methods!");
+                err
+            })
+    }
+
+    pub async fn next(&self) -> Result<ApiConnectionMode> {
+        self.send_command(Message::Next).await.map_err(|err| {
+            log::error!("Failed to update new access methods!");
+            err
+        })
+    }
+
+    /// Stream the connection modes of this actor.
+    pub fn as_stream(&self) -> impl Stream<Item = ApiConnectionMode> {
+        let handle = self.clone();
+        unfold(handle, |handle| async move {
+            let connection_mode = handle
+                .next()
+                .await
+                .expect("It should always be safe to `unwrap` a new API connection mode");
+            Some((connection_mode, handle))
+        })
+    }
+}
+
+pub struct EActor {
+    cmd_rx: mpsc::UnboundedReceiver<Message>,
+    state: ApiConnectionModeProvider,
+}
+
+impl EActor {
+    async fn run(&mut self) {
+        while let Some(cmd) = self.cmd_rx.next().await {
+            let _ = match cmd {
+                Message::Get(tx) => self.on_get_access_method(tx),
+                Message::Set(tx, value) => self.on_set_access_method(tx, value),
+                Message::Next(tx) => self.on_next_connection_mode(tx),
+                Message::Update(tx, values) => self.on_update_access_methods(tx, values),
+            }
+            .map_err(|err| {
+                log::error!("todo(markus): Some error occured {err}");
+                err
+            });
+        }
+    }
+
+    fn reply<T>(&self, tx: ResponseTx<T>, value: T) -> Result<()> {
+        // TODO(markus): The error probably should come from the value/tx
+        tx.send(Ok(value)).map_err(|_| Error::Generic)
+    }
+
+    fn on_get_access_method(&mut self, tx: ResponseTx<AccessMethodSetting>) -> Result<()> {
+        let value = self.get_access_method()?;
+        self.reply(tx, value)
+    }
+
+    fn get_access_method(&mut self) -> Result<AccessMethodSetting> {
+        let connections_modes = self.state.connection_modes.lock().unwrap();
+        Ok(connections_modes.peek())
+    }
+
+    fn on_set_access_method(
+        &mut self,
+        tx: ResponseTx<()>,
+        value: AccessMethodSetting,
+    ) -> Result<()> {
+        self.set_access_method(value)?;
+        self.reply(tx, ())
+    }
+
+    fn set_access_method(&mut self, value: AccessMethodSetting) -> Result<()> {
+        let mut connections_modes = self.state.connection_modes.lock().unwrap();
+        connections_modes.set_access_method(value);
+        Ok(())
+    }
+
+    fn on_next_connection_mode(&mut self, tx: ResponseTx<ApiConnectionMode>) -> Result<()> {
+        let next = self.next_connection_mode();
+        // Save the new connection mode to cache!
+        {
+            let cache_dir = self.state.cache_dir.clone();
+            let next = next.clone();
+            tokio::spawn(async move {
+                if next.save(&cache_dir).await.is_err() {
+                    log::warn!(
+                        "Failed to save {connection_mode} to cache",
+                        connection_mode = next
+                    )
+                }
+            });
+        }
+        self.reply(tx, next)
+    }
+
+    fn next_connection_mode(&mut self) -> ApiConnectionMode {
+        let access_method = {
+            let mut connection_modes = self.state.connection_modes.lock().unwrap();
+            connection_modes
+                .next()
+                .map(|access_method_setting| access_method_setting.access_method)
+                .unwrap_or(AccessMethod::from(BuiltInAccessMethod::Direct))
+        };
+
+        let connection_mode = self.state.from(access_method);
+        log::info!("New API connection mode selected: {}", connection_mode);
+        connection_mode
+    }
+
+    fn on_update_access_methods(
+        &mut self,
+        tx: ResponseTx<()>,
+        values: Vec<AccessMethodSetting>,
+    ) -> Result<()> {
+        self.update_access_methods(values)?;
+        self.reply(tx, ())
+    }
+
+    fn update_access_methods(&mut self, values: Vec<AccessMethodSetting>) -> Result<()> {
+        let mut connection_modes = self.state.connection_modes.lock().unwrap();
+        connection_modes.update_access_methods(values);
+        Ok(())
+    }
+}
+
+type ResponseTx<T> = oneshot::Sender<Result<T>>;
+type Result<T> = std::result::Result<T, Error>;
 
 /// A stream that returns the next API connection mode to use for reaching the API.
 ///
@@ -38,45 +227,9 @@ pub struct ApiConnectionModeProvider {
     cache_dir: PathBuf,
     /// Used for selecting a Bridge when the `Mullvad Bridges` access method is used.
     relay_selector: RelaySelector,
-    current_task: Option<Pin<Box<dyn Future<Output = ApiConnectionMode> + Send>>>,
     connection_modes: Arc<Mutex<ConnectionModesIterator>>,
 }
 
-impl Stream for ApiConnectionModeProvider {
-    type Item = ApiConnectionMode;
-
-    fn poll_next(
-        mut self: Pin<&mut Self>,
-        cx: &mut std::task::Context<'_>,
-    ) -> Poll<Option<Self::Item>> {
-        // Poll the current task
-        if let Some(task) = self.current_task.as_mut() {
-            return match task.as_mut().poll(cx) {
-                Poll::Ready(mode) => {
-                    self.current_task = None;
-                    Poll::Ready(Some(mode))
-                }
-                Poll::Pending => Poll::Pending,
-            };
-        }
-
-        let connection_mode = self.new_connection_mode();
-
-        let cache_dir = self.cache_dir.clone();
-        self.current_task = Some(Box::pin(async move {
-            if let Err(error) = connection_mode.save(&cache_dir).await {
-                log::debug!(
-                    "{}",
-                    error.display_chain_with_msg("Failed to save API endpoint")
-                );
-            }
-            connection_mode
-        }));
-
-        self.poll_next(cx)
-    }
-}
-
 impl ApiConnectionModeProvider {
     pub(crate) fn new(
         cache_dir: PathBuf,
@@ -87,35 +240,10 @@ impl ApiConnectionModeProvider {
         Ok(Self {
             cache_dir,
             relay_selector,
-            current_task: None,
             connection_modes: Arc::new(Mutex::new(connection_modes_iterator)),
         })
     }
 
-    /// Return a pointer to the underlying iterator over [`AccessMethod`].
-    /// Having access to this iterator allow you to influence , e.g. by calling
-    /// [`ConnectionModesIterator::set_access_method()`] or
-    /// [`ConnectionModesIterator::update_access_methods()`].
-    pub(crate) fn handle(&self) -> Arc<Mutex<ConnectionModesIterator>> {
-        self.connection_modes.clone()
-    }
-
-    /// Return a new connection mode to be used for the API connection.
-    fn new_connection_mode(&mut self) -> ApiConnectionMode {
-        log::debug!("Rotating Access mode!");
-        let access_method = {
-            let mut access_methods_picker = self.connection_modes.lock().unwrap();
-            access_methods_picker
-                .next()
-                .map(|access_method_setting| access_method_setting.access_method)
-                .unwrap_or(AccessMethod::from(BuiltInAccessMethod::Direct))
-        };
-
-        let connection_mode = self.from(access_method);
-        log::info!("New API connection mode selected: {}", connection_mode);
-        connection_mode
-    }
-
     /// Ad-hoc version of [`std::convert::From::from`], but since some
     /// [`ApiConnectionMode`]s require extra logic/data from
     /// [`ApiConnectionModeProvider`] the standard [`std::convert::From`] trait
diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs
index c1891bda4d0e..629959785844 100644
--- a/mullvad-daemon/src/lib.rs
+++ b/mullvad-daemon/src/lib.rs
@@ -66,7 +66,7 @@ use std::{
     mem,
     path::PathBuf,
     pin::Pin,
-    sync::{Arc, Mutex, Weak},
+    sync::{Arc, Weak},
     time::Duration,
 };
 #[cfg(any(target_os = "linux", windows))]
@@ -179,6 +179,9 @@ pub enum Error {
     #[error(display = "Access method error")]
     AccessMethodError(#[error(source)] access_method::Error),
 
+    #[error(display = "API connection mode error")]
+    ApiConnectionModeError(#[error(source)] api::Error),
+
     #[cfg(target_os = "macos")]
     #[error(display = "Failed to set exclusion group")]
     GroupIdError(#[error(source)] io::Error),
@@ -294,7 +297,7 @@ pub enum DaemonCommand {
     /// Get the currently used API access method
     GetCurrentAccessMethod(ResponseTx<AccessMethodSetting, Error>),
     /// Test an API access method
-    TestApiAccessMethod(ResponseTx<(), Error>, mullvad_types::access_method::Id),
+    TestApiAccessMethod(ResponseTx<bool, 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
@@ -602,7 +605,7 @@ pub struct Daemon<L: EventListener> {
     account_history: account_history::AccountHistory,
     device_checker: device::TunnelStateChangeHandler,
     account_manager: device::AccountManagerHandle,
-    connection_modes: Arc<Mutex<api::ConnectionModesIterator>>,
+    connection_modes_handler: api::Ehandle,
     api_runtime: mullvad_api::Runtime,
     api_handle: mullvad_api::rest::MullvadRestHandle,
     version_updater_handle: version_check::VersionUpdaterHandle,
@@ -680,17 +683,18 @@ where
                 .set_config(new_selector_config(settings));
         });
 
-        let proxy_provider = match api::ApiConnectionModeProvider::new(
-            cache_dir.clone(),
-            relay_selector.clone(),
-            settings
+        let connection_modes: Vec<_> = settings
                 .api_access_methods
                 .access_method_settings
                 .iter()
                 // We only care about the access methods which are set to 'enabled' by the user.
                 .filter(|api_access_method| api_access_method.enabled())
                 .cloned()
-                .collect(),
+                .collect();
+        let proxy_provider = match api::ApiConnectionModeProvider::new(
+            cache_dir.clone(),
+            relay_selector.clone(),
+            connection_modes,
         ) {
             Ok(provider) => provider,
             Err(api::Error::NoAccessMethods) => {
@@ -708,10 +712,14 @@ where
             }
         };
 
-        let connection_modes = proxy_provider.handle();
+        let connection_modes_handler =
+            api::Ehandle::new(cache_dir.clone(), relay_selector.clone(), connection_modes);
 
         let api_handle = api_runtime
-            .mullvad_rest_handle(proxy_provider, endpoint_updater.callback())
+            .mullvad_rest_handle(
+                Box::pin(connection_modes_handler.as_stream()),
+                endpoint_updater.callback(),
+            )
             .await;
 
         let migration_complete = if let Some(migration_data) = migration_data {
@@ -861,7 +869,7 @@ where
             account_history,
             device_checker: device::TunnelStateChangeHandler::new(account_manager.clone()),
             account_manager,
-            connection_modes,
+            connection_modes_handler,
             api_runtime,
             api_handle,
             version_updater_handle,
@@ -2375,10 +2383,14 @@ where
     }
 
     fn on_get_current_api_access_method(&mut self, tx: ResponseTx<AccessMethodSetting, Error>) {
-        let result = self
-            .get_current_access_method()
-            .map_err(Error::AccessMethodError);
-        Self::oneshot_send(tx, result, "get_current_api_access_method response");
+        let handle = self.connection_modes_handler.clone();
+        tokio::spawn(async move {
+            let result = handle
+                .get_access_method()
+                .await
+                .map_err(Error::ApiConnectionModeError);
+            Self::oneshot_send(tx, result, "get_current_api_access_method response");
+        });
     }
 
     /// Try to reach the Mullvad API using a specific access method, returning
@@ -2390,48 +2402,114 @@ where
     /// access method is *always* reset.
     async fn on_test_api_access_method(
         &mut self,
-        tx: ResponseTx<(), Error>,
+        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 result = async {
+        let api_handle = self.api_handle.clone();
+        let handle = self.connection_modes_handler.clone();
+        let access_method = self.get_api_access_method(access_method);
+        // TODO(markus): Clean up this error handling
+        let new_access_method = if let Ok(access_method) = access_method {
+            access_method
+        } else {
+            Self::oneshot_send(
+                tx,
+                access_method
+                    .map(|_| false)
+                    .map_err(Error::AccessMethodError),
+                "on_test_api_access_method response",
+            );
+            return;
+        };
+
+        let fut = async move {
             // Setup test
-            let previous_access_method = self
-                .get_current_access_method()
-                .map_err(Error::AccessMethodError)?;
+            let previous_access_method = handle
+                .get_access_method()
+                .await
+                .map_err(Error::ApiConnectionModeError)
+                // TODO(markus): Do not unwrap!
+                .unwrap();
+
+            let x = new_access_method.clone();
+            handle.set_access_method(new_access_method)
+                .await
+                .map_err(Error::ApiConnectionModeError)
+                // TODO(markus): Do not unwrap!
+                .unwrap();
+
+            // We need to perform a rotation of API endpoint after a set action
+            let rotation_handle = api_handle.clone();
+            rotation_handle
+                .service()
+                .next_api_endpoint()
+                .await
+                .map_err(|err| {
+                    log::error!("Failed to rotate API endpoint: {err}");
+                    err
+                })
+                // TODO(markus): Error handling
+                .unwrap();
 
-            self.set_api_access_method(access_method)
+            // 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.
+            handle
+                .set_access_method(previous_access_method)
                 .await
-                .map_err(Error::AccessMethodError)?;
+                .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."
+                    );
+                    err
+                })
+                // TODO(markus): Do not unwrap!
+                .unwrap();
+
             // 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())
+            let result = mullvad_api::ApiProxy::new(api_handle)
                 .api_addrs_available()
                 .await
                 .map_err(Error::RestError);
 
-            // Reset test
-            self.set_api_access_method(previous_access_method.get_id())
+            // 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.as_ref().is_ok_and(|&succeeded| succeeded) {
+                rotation_handle
+                .service()
+                .next_api_endpoint()
                 .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)
-                })?;
+                    log::error!("Failed to rotate API endpoint: {err}");
+                    err
+                })
+                // TODO(markus): Error handling
+                .unwrap();
+            }
+
+            log::info!(
+                "The result of testing {method:?} is {result:?}",
+                method = x.access_method,
+                result = result
+            );
 
-            result
+            Self::oneshot_send(tx, result, "on_test_api_access_method response");
         };
 
-        Self::oneshot_send(tx, result.await, "on_test_api_access_method response");
+        tokio::spawn(fut);
     }
 
     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 633bcc09454b..c194825a3438 100644
--- a/mullvad-daemon/src/management_interface.rs
+++ b/mullvad-daemon/src/management_interface.rs
@@ -693,7 +693,7 @@ impl ManagementService for ManagementServiceImpl {
             .map_err(map_daemon_error)
     }
 
-    async fn test_api_access_method(&self, request: Request<types::Uuid>) -> ServiceResult<()> {
+    async fn test_api_access_method(&self, request: Request<types::Uuid>) -> ServiceResult<bool> {
         log::debug!("test_api_access_method");
         let (tx, rx) = oneshot::channel();
         let api_access_method = mullvad_types::access_method::Id::try_from(request.into_inner())?;
diff --git a/mullvad-management-interface/proto/management_interface.proto b/mullvad-management-interface/proto/management_interface.proto
index bb426afdea18..d66707b79f7f 100644
--- a/mullvad-management-interface/proto/management_interface.proto
+++ b/mullvad-management-interface/proto/management_interface.proto
@@ -81,7 +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) {}
+  rpc TestApiAccessMethod(UUID) returns (google.protobuf.BoolValue) {}
 
   // Split tunneling (Linux)
   rpc GetSplitTunnelProcesses(google.protobuf.Empty) returns (stream google.protobuf.Int32Value) {}
diff --git a/mullvad-management-interface/src/client.rs b/mullvad-management-interface/src/client.rs
index c24f30260e32..1c9d80b2e8f6 100644
--- a/mullvad-management-interface/src/client.rs
+++ b/mullvad-management-interface/src/client.rs
@@ -208,12 +208,13 @@ impl MullvadProxyClient {
             })
     }
 
-    pub async fn test_api_access_method(&mut self, id: access_method::Id) -> Result<()> {
-        self.0
+    pub async fn test_api_access_method(&mut self, id: access_method::Id) -> Result<bool> {
+        let result = self
+            .0
             .test_api_access_method(types::Uuid::from(id))
             .await
             .map_err(Error::Rpc)?;
-        Ok(())
+        Ok(result.into_inner())
     }
 
     pub async fn update_relay_locations(&mut self) -> Result<()> {

From 9358da4abff1855b6ffacee9b9bdbe7f89349198 Mon Sep 17 00:00:00 2001
From: Markus Pettersson <markus.pettersson@mullvad.net>
Date: Thu, 16 Nov 2023 15:05:25 +0100
Subject: [PATCH 06/14] Refactor `access_methods.rs`

- Naming of structs/enums/variables
- Add documentation
- Change existing doc comment to better reflect their context / any new naming
---
 mullvad-daemon/src/access_method.rs | 11 ++++-------
 mullvad-daemon/src/api.rs           | 18 ++++++++++--------
 mullvad-daemon/src/lib.rs           |  9 ++++++---
 3 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/mullvad-daemon/src/access_method.rs b/mullvad-daemon/src/access_method.rs
index 51a0f0ae4a05..b0dd0506c31f 100644
--- a/mullvad-daemon/src/access_method.rs
+++ b/mullvad-daemon/src/access_method.rs
@@ -19,16 +19,13 @@ pub enum Error {
     /// Can not find access method
     #[error(display = "Cannot find custom access method {}", _0)]
     NoSuchMethod(access_method::Id),
-    /// Can not find *any* access method. This should never happen. If it does,
-    /// the user should do a factory reset.
-    #[error(display = "No access methods are configured")]
-    NoMethodsExist,
     /// Access method could not be rotate
     #[error(display = "Access method could not be rotated")]
     RotationError,
-    /// Daemon API error
-    #[error(display = "Daemon API handling error")]
-    Api(#[error(source)] api::Error),
+    /// Some error occured in the daemon's state of handling
+    /// [`AccessMethodSetting`]s & [`ApiConnectionMode`]s.
+    #[error(display = "Error occured when handling connection settings & details")]
+    ConnectionMode(#[error(source)] api::Error),
     /// Access methods settings error
     #[error(display = "Settings error")]
     Settings(#[error(source)] settings::Error),
diff --git a/mullvad-daemon/src/api.rs b/mullvad-daemon/src/api.rs
index d309854473ac..47a9969a7693 100644
--- a/mullvad-daemon/src/api.rs
+++ b/mullvad-daemon/src/api.rs
@@ -1,3 +1,8 @@
+//! This module is responsible for enabling custom [`AccessMethodSetting`]s to
+//! be used when connecting to the Mullvad API. In practice this means
+//! converting [`AccessMethodSetting`]s to connection details as encoded by
+//! [`ApiConnectionMode`], which in turn is used by `mullvad-api` for
+//! establishing connections when performing API requests.
 #[cfg(target_os = "android")]
 use crate::{DaemonCommand, DaemonEventSender};
 use futures::{
@@ -21,9 +26,6 @@ use talpid_core::mpsc::Sender;
 use talpid_core::tunnel_state_machine::TunnelCommand;
 use talpid_types::net::{openvpn::ProxySettings, AllowedEndpoint, Endpoint};
 
-// TODO(markus): Remove text
-/// Here, a new agent was born.
-
 pub enum Message {
     Get(ResponseTx<AccessMethodSetting>),
     Set(ResponseTx<()>, AccessMethodSetting),
@@ -39,11 +41,11 @@ pub enum Error {
 }
 
 #[derive(Clone)]
-pub struct Ehandle {
+pub struct AccessModeSelectorHandle {
     cmd_tx: mpsc::UnboundedSender<Message>,
 }
 
-impl Ehandle {
+impl AccessModeSelectorHandle {
     pub fn new(
         cache_dir: PathBuf,
         relay_selector: RelaySelector,
@@ -51,7 +53,7 @@ impl Ehandle {
     ) -> Self {
         let (cmd_tx, cmd_rx) = mpsc::unbounded();
 
-        let mut actor = EActor {
+        let mut actor = AccessModeSelector {
             cmd_rx,
             state: ApiConnectionModeProvider::new(cache_dir, relay_selector, connection_modes),
         };
@@ -112,12 +114,12 @@ impl Ehandle {
     }
 }
 
-pub struct EActor {
+pub struct AccessModeSelector {
     cmd_rx: mpsc::UnboundedReceiver<Message>,
     state: ApiConnectionModeProvider,
 }
 
-impl EActor {
+impl AccessModeSelector {
     async fn run(&mut self) {
         while let Some(cmd) = self.cmd_rx.next().await {
             let _ = match cmd {
diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs
index 629959785844..ee3e75d7d675 100644
--- a/mullvad-daemon/src/lib.rs
+++ b/mullvad-daemon/src/lib.rs
@@ -605,7 +605,7 @@ pub struct Daemon<L: EventListener> {
     account_history: account_history::AccountHistory,
     device_checker: device::TunnelStateChangeHandler,
     account_manager: device::AccountManagerHandle,
-    connection_modes_handler: api::Ehandle,
+    connection_modes_handler: api::AccessModeSelectorHandle,
     api_runtime: mullvad_api::Runtime,
     api_handle: mullvad_api::rest::MullvadRestHandle,
     version_updater_handle: version_check::VersionUpdaterHandle,
@@ -712,8 +712,11 @@ where
             }
         };
 
-        let connection_modes_handler =
-            api::Ehandle::new(cache_dir.clone(), relay_selector.clone(), connection_modes);
+        let connection_modes_handler = api::AccessModeSelectorHandle::new(
+            cache_dir.clone(),
+            relay_selector.clone(),
+            connection_modes,
+        );
 
         let api_handle = api_runtime
             .mullvad_rest_handle(

From e31d9e01fa4e4635c4ddead14b18fa7e8381e3f6 Mon Sep 17 00:00:00 2001
From: Markus Pettersson <markus.pettersson@mullvad.net>
Date: Thu, 16 Nov 2023 15:17:11 +0100
Subject: [PATCH 07/14] Construct `AccessModeSelectorHandle` by calling
 `AccessModeSelector::spawn`

Construct an `AccessModeSelectorHandle` by calling
`AccessModeSelector::spawn`, which is consistent with how actors are
spawned in other parts of the `mullvad-daemon`.
---
 mullvad-daemon/src/access_method.rs | 10 +------
 mullvad-daemon/src/api.rs           | 46 +++++++++++++++++++----------
 mullvad-daemon/src/lib.rs           | 31 ++-----------------
 mullvad-types/src/access_method.rs  |  8 +++++
 4 files changed, 41 insertions(+), 54 deletions(-)

diff --git a/mullvad-daemon/src/access_method.rs b/mullvad-daemon/src/access_method.rs
index b0dd0506c31f..97dfa815f599 100644
--- a/mullvad-daemon/src/access_method.rs
+++ b/mullvad-daemon/src/access_method.rs
@@ -198,15 +198,7 @@ where
                 .notify_settings(self.settings.to_settings());
 
             let handle = self.connection_modes_handler.clone();
-            let new_access_methods = self
-                .settings
-                .api_access_methods
-                .access_method_settings
-                .iter()
-                .filter(|api_access_method| api_access_method.enabled())
-                .cloned()
-                .collect();
-
+            let new_access_methods = self.settings.api_access_methods.collect_enabled();
             tokio::spawn(async move {
                 match handle.update_access_methods(new_access_methods).await {
                     Ok(_) => (),
diff --git a/mullvad-daemon/src/api.rs b/mullvad-daemon/src/api.rs
index 47a9969a7693..ef6d2ff1067d 100644
--- a/mullvad-daemon/src/api.rs
+++ b/mullvad-daemon/src/api.rs
@@ -46,21 +46,6 @@ pub struct AccessModeSelectorHandle {
 }
 
 impl AccessModeSelectorHandle {
-    pub fn new(
-        cache_dir: PathBuf,
-        relay_selector: RelaySelector,
-        connection_modes: Vec<AccessMethodSetting>,
-    ) -> Self {
-        let (cmd_tx, cmd_rx) = mpsc::unbounded();
-
-        let mut actor = AccessModeSelector {
-            cmd_rx,
-            state: ApiConnectionModeProvider::new(cache_dir, relay_selector, connection_modes),
-        };
-        tokio::spawn(async move { actor.run().await });
-        Self { cmd_tx }
-    }
-
     async fn send_command<T>(&self, make_cmd: impl FnOnce(ResponseTx<T>) -> Message) -> Result<T> {
         let (tx, rx) = oneshot::channel();
         // TODO(markus): Error handling
@@ -120,7 +105,36 @@ pub struct AccessModeSelector {
 }
 
 impl AccessModeSelector {
-    async fn run(&mut self) {
+    pub fn spawn(
+        cache_dir: PathBuf,
+        relay_selector: RelaySelector,
+        connection_modes: Vec<AccessMethodSetting>,
+    ) -> AccessModeSelectorHandle {
+        let (cmd_tx, cmd_rx) = mpsc::unbounded();
+
+        let state =
+            match ApiConnectionModeProvider::new(cache_dir, relay_selector, connection_modes) {
+                Ok(provider) => provider,
+                Err(api::Error::NoAccessMethods) => {
+                    // No settings seem to have been found. Default to using the the
+                    // direct access method.
+                    let default = mullvad_types::access_method::Settings::direct();
+                    api::ApiConnectionModeProvider::new(
+                    cache_dir.clone(),
+                    relay_selector.clone(),
+                    vec![default],
+                )
+                .expect(
+                    "Failed to create the data structure responsible for managing access methods",
+                )
+                }
+            };
+        let selector = AccessModeSelector { cmd_rx, state };
+        tokio::spawn(selector.into_future());
+        AccessModeSelectorHandle { cmd_tx }
+    }
+
+    async fn into_future(mut self) {
         while let Some(cmd) = self.cmd_rx.next().await {
             let _ = match cmd {
                 Message::Get(tx) => self.on_get_access_method(tx),
diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs
index ee3e75d7d675..90dbd1f4f409 100644
--- a/mullvad-daemon/src/lib.rs
+++ b/mullvad-daemon/src/lib.rs
@@ -683,36 +683,9 @@ where
                 .set_config(new_selector_config(settings));
         });
 
-        let connection_modes: Vec<_> = settings
-                .api_access_methods
-                .access_method_settings
-                .iter()
-                // We only care about the access methods which are set to 'enabled' by the user.
-                .filter(|api_access_method| api_access_method.enabled())
-                .cloned()
-                .collect();
-        let proxy_provider = match api::ApiConnectionModeProvider::new(
-            cache_dir.clone(),
-            relay_selector.clone(),
-            connection_modes,
-        ) {
-            Ok(provider) => provider,
-            Err(api::Error::NoAccessMethods) => {
-                // No settings seem to have been found. Default to using the the
-                // direct access method.
-                let default = mullvad_types::access_method::Settings::direct();
-                api::ApiConnectionModeProvider::new(
-                    cache_dir.clone(),
-                    relay_selector.clone(),
-                    vec![default],
-                )
-                .expect(
-                    "Failed to create the data structure responsible for managing access methods",
-                )
-            }
-        };
+        let connection_modes = settings.api_access_methods.collect_enabled();
 
-        let connection_modes_handler = api::AccessModeSelectorHandle::new(
+        let connection_modes_handler = api::AccessModeSelector::spawn(
             cache_dir.clone(),
             relay_selector.clone(),
             connection_modes,
diff --git a/mullvad-types/src/access_method.rs b/mullvad-types/src/access_method.rs
index fe4b2507edd9..7afaf94dfc40 100644
--- a/mullvad-types/src/access_method.rs
+++ b/mullvad-types/src/access_method.rs
@@ -61,6 +61,14 @@ impl Settings {
         let method = BuiltInAccessMethod::Bridge;
         AccessMethodSetting::new(method.canonical_name(), true, AccessMethod::from(method))
     }
+
+    /// Retrieve all [`AccessMethodSetting`]s which are enabled.
+    pub fn collect_enabled(&self) -> Vec<AccessMethodSetting> {
+        self.cloned()
+            .into_iter()
+            .filter(|access_method| access_method.enabled)
+            .collect()
+    }
 }
 
 impl Default for Settings {

From 2f648dd809195335e7ba84d005b877b55e03fc2c Mon Sep 17 00:00:00 2001
From: Markus Pettersson <markus.pettersson@mullvad.net>
Date: Thu, 16 Nov 2023 16:23:36 +0100
Subject: [PATCH 08/14] Add proper error handling

---
 mullvad-daemon/src/api.rs | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/mullvad-daemon/src/api.rs b/mullvad-daemon/src/api.rs
index ef6d2ff1067d..6074604b89a0 100644
--- a/mullvad-daemon/src/api.rs
+++ b/mullvad-daemon/src/api.rs
@@ -38,6 +38,16 @@ pub enum Error {
     /// Oddly specific.
     #[error(display = "Very Generic error.")]
     Generic,
+    #[error(display = "{}", _0)]
+    Actor(#[error(source)] ActorError),
+}
+
+#[derive(err_derive::Error, Debug)]
+pub enum ActorError {
+    #[error(display = "AccessModeSelector is not receiving any messages.")]
+    SendFailed(#[error(source)] mpsc::TrySendError<Message>),
+    #[error(display = "AccessModeSelector is not responding.")]
+    NotRunning(#[error(source)] oneshot::Canceled),
 }
 
 #[derive(Clone)]
@@ -48,10 +58,10 @@ pub struct AccessModeSelectorHandle {
 impl AccessModeSelectorHandle {
     async fn send_command<T>(&self, make_cmd: impl FnOnce(ResponseTx<T>) -> Message) -> Result<T> {
         let (tx, rx) = oneshot::channel();
-        // TODO(markus): Error handling
-        self.cmd_tx.unbounded_send(make_cmd(tx)).unwrap();
-        // TODO(markus): Error handling
-        rx.await.unwrap()
+        self.cmd_tx
+            .unbounded_send(make_cmd(tx))
+            .map_err(ActorError::SendFailed)?;
+        rx.await.map_err(ActorError::NotRunning)?
     }
 
     pub async fn get_access_method(&self) -> Result<AccessMethodSetting> {
@@ -90,11 +100,12 @@ impl AccessModeSelectorHandle {
     pub fn as_stream(&self) -> impl Stream<Item = ApiConnectionMode> {
         let handle = self.clone();
         unfold(handle, |handle| async move {
-            let connection_mode = handle
-                .next()
-                .await
-                .expect("It should always be safe to `unwrap` a new API connection mode");
-            Some((connection_mode, handle))
+            match handle.next().await {
+                Ok(connection_mode) => Some((connection_mode, handle)),
+                // End this stream in case of failure in `next`. `next` should
+                // not fail if the actor is in a good state.
+                Err(_) => None,
+            }
         })
     }
 }

From aee1d38a61676a41cf3fe0d99ffd0fa0b469e94f Mon Sep 17 00:00:00 2001
From: Markus Pettersson <markus.pettersson@mullvad.net>
Date: Thu, 16 Nov 2023 16:38:45 +0100
Subject: [PATCH 09/14] Get rid of unnecessary `Arc<Mutex<_>>`

---
 mullvad-daemon/src/api.rs | 47 ++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 25 deletions(-)

diff --git a/mullvad-daemon/src/api.rs b/mullvad-daemon/src/api.rs
index 6074604b89a0..a7e35f0ad5dd 100644
--- a/mullvad-daemon/src/api.rs
+++ b/mullvad-daemon/src/api.rs
@@ -166,13 +166,12 @@ impl AccessModeSelector {
     }
 
     fn on_get_access_method(&mut self, tx: ResponseTx<AccessMethodSetting>) -> Result<()> {
-        let value = self.get_access_method()?;
+        let value = self.get_access_method();
         self.reply(tx, value)
     }
 
-    fn get_access_method(&mut self) -> Result<AccessMethodSetting> {
-        let connections_modes = self.state.connection_modes.lock().unwrap();
-        Ok(connections_modes.peek())
+    fn get_access_method(&mut self) -> AccessMethodSetting {
+        self.state.connection_modes.peek()
     }
 
     fn on_set_access_method(
@@ -180,14 +179,12 @@ impl AccessModeSelector {
         tx: ResponseTx<()>,
         value: AccessMethodSetting,
     ) -> Result<()> {
-        self.set_access_method(value)?;
+        self.set_access_method(value);
         self.reply(tx, ())
     }
 
-    fn set_access_method(&mut self, value: AccessMethodSetting) -> Result<()> {
-        let mut connections_modes = self.state.connection_modes.lock().unwrap();
-        connections_modes.set_access_method(value);
-        Ok(())
+    fn set_access_method(&mut self, value: AccessMethodSetting) {
+        self.state.connection_modes.set_access_method(value);
     }
 
     fn on_next_connection_mode(&mut self, tx: ResponseTx<ApiConnectionMode>) -> Result<()> {
@@ -209,16 +206,18 @@ impl AccessModeSelector {
     }
 
     fn next_connection_mode(&mut self) -> ApiConnectionMode {
-        let access_method = {
-            let mut connection_modes = self.state.connection_modes.lock().unwrap();
-            connection_modes
-                .next()
-                .map(|access_method_setting| access_method_setting.access_method)
-                .unwrap_or(AccessMethod::from(BuiltInAccessMethod::Direct))
-        };
+        let access_method = self
+            .state
+            .connection_modes
+            .next()
+            .map(|access_method_setting| access_method_setting.access_method)
+            .unwrap_or(AccessMethod::from(BuiltInAccessMethod::Direct));
 
         let connection_mode = self.state.from(access_method);
-        log::info!("New API connection mode selected: {}", connection_mode);
+        log::info!(
+            "New API connection mode selected: {connection_mode}",
+            connection_mode = connection_mode
+        );
         connection_mode
     }
 
@@ -227,14 +226,12 @@ impl AccessModeSelector {
         tx: ResponseTx<()>,
         values: Vec<AccessMethodSetting>,
     ) -> Result<()> {
-        self.update_access_methods(values)?;
+        self.update_access_methods(values);
         self.reply(tx, ())
     }
 
-    fn update_access_methods(&mut self, values: Vec<AccessMethodSetting>) -> Result<()> {
-        let mut connection_modes = self.state.connection_modes.lock().unwrap();
-        connection_modes.update_access_methods(values);
-        Ok(())
+    fn update_access_methods(&mut self, values: Vec<AccessMethodSetting>) {
+        self.state.connection_modes.update_access_methods(values);
     }
 }
 
@@ -254,7 +251,7 @@ pub struct ApiConnectionModeProvider {
     cache_dir: PathBuf,
     /// Used for selecting a Bridge when the `Mullvad Bridges` access method is used.
     relay_selector: RelaySelector,
-    connection_modes: Arc<Mutex<ConnectionModesIterator>>,
+    connection_modes: ConnectionModesIterator,
 }
 
 impl ApiConnectionModeProvider {
@@ -267,8 +264,8 @@ impl ApiConnectionModeProvider {
         Ok(Self {
             cache_dir,
             relay_selector,
-            connection_modes: Arc::new(Mutex::new(connection_modes_iterator)),
-        })
+            connection_modes: connection_modes_iterator,
+        }
     }
 
     /// Ad-hoc version of [`std::convert::From::from`], but since some

From 5a8b75effca1d8b563190021b40cc73f37302798 Mon Sep 17 00:00:00 2001
From: Markus Pettersson <markus.pettersson@mullvad.net>
Date: Thu, 16 Nov 2023 16:48:08 +0100
Subject: [PATCH 10/14] Add more comments to `AccessModeSelector`

---
 mullvad-daemon/src/api.rs | 35 +++++++++++++++++++++++------------
 mullvad-daemon/src/lib.rs |  2 +-
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/mullvad-daemon/src/api.rs b/mullvad-daemon/src/api.rs
index a7e35f0ad5dd..9f60bab8ed95 100644
--- a/mullvad-daemon/src/api.rs
+++ b/mullvad-daemon/src/api.rs
@@ -50,6 +50,8 @@ pub enum ActorError {
     NotRunning(#[error(source)] oneshot::Canceled),
 }
 
+/// A channel for sending [`Message`] commands to a running
+/// [`AccessModeSelector`].
 #[derive(Clone)]
 pub struct AccessModeSelectorHandle {
     cmd_tx: mpsc::UnboundedSender<Message>,
@@ -96,10 +98,13 @@ impl AccessModeSelectorHandle {
         })
     }
 
-    /// Stream the connection modes of this actor.
-    pub fn as_stream(&self) -> impl Stream<Item = ApiConnectionMode> {
-        let handle = self.clone();
-        unfold(handle, |handle| async move {
+    /// Convert this handle to a [`Stream`] of [`ApiConnectionMode`] from the
+    /// associated [`AccessModeSelector`].
+    ///
+    /// Practically converts the handle to a listener for when the
+    /// currently valid connection modes changes.
+    pub fn as_stream(self) -> impl Stream<Item = ApiConnectionMode> {
+        unfold(self, |handle| async move {
             match handle.next().await {
                 Ok(connection_mode) => Some((connection_mode, handle)),
                 // End this stream in case of failure in `next`. `next` should
@@ -110,9 +115,11 @@ impl AccessModeSelectorHandle {
     }
 }
 
+/// A small actor which takes care of handling the logic around rotating
+/// connection modes to be used for Mullvad API request.
 pub struct AccessModeSelector {
     cmd_rx: mpsc::UnboundedReceiver<Message>,
-    state: ApiConnectionModeProvider,
+    connection_mode_provider: ApiConnectionModeProvider,
 }
 
 impl AccessModeSelector {
@@ -140,7 +147,7 @@ impl AccessModeSelector {
                 )
                 }
             };
-        let selector = AccessModeSelector { cmd_rx, state };
+        let selector = AccessModeSelector { cmd_rx, connection_mode_provider: state };
         tokio::spawn(selector.into_future());
         AccessModeSelectorHandle { cmd_tx }
     }
@@ -171,7 +178,7 @@ impl AccessModeSelector {
     }
 
     fn get_access_method(&mut self) -> AccessMethodSetting {
-        self.state.connection_modes.peek()
+        self.connection_mode_provider.connection_modes.peek()
     }
 
     fn on_set_access_method(
@@ -184,14 +191,16 @@ impl AccessModeSelector {
     }
 
     fn set_access_method(&mut self, value: AccessMethodSetting) {
-        self.state.connection_modes.set_access_method(value);
+        self.connection_mode_provider
+            .connection_modes
+            .set_access_method(value);
     }
 
     fn on_next_connection_mode(&mut self, tx: ResponseTx<ApiConnectionMode>) -> Result<()> {
         let next = self.next_connection_mode();
         // Save the new connection mode to cache!
         {
-            let cache_dir = self.state.cache_dir.clone();
+            let cache_dir = self.connection_mode_provider.cache_dir.clone();
             let next = next.clone();
             tokio::spawn(async move {
                 if next.save(&cache_dir).await.is_err() {
@@ -207,13 +216,13 @@ impl AccessModeSelector {
 
     fn next_connection_mode(&mut self) -> ApiConnectionMode {
         let access_method = self
-            .state
+            .connection_mode_provider
             .connection_modes
             .next()
             .map(|access_method_setting| access_method_setting.access_method)
             .unwrap_or(AccessMethod::from(BuiltInAccessMethod::Direct));
 
-        let connection_mode = self.state.from(access_method);
+        let connection_mode = self.connection_mode_provider.from(access_method);
         log::info!(
             "New API connection mode selected: {connection_mode}",
             connection_mode = connection_mode
@@ -231,7 +240,9 @@ impl AccessModeSelector {
     }
 
     fn update_access_methods(&mut self, values: Vec<AccessMethodSetting>) {
-        self.state.connection_modes.update_access_methods(values);
+        self.connection_mode_provider
+            .connection_modes
+            .update_access_methods(values);
     }
 }
 
diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs
index 90dbd1f4f409..9a2121260b98 100644
--- a/mullvad-daemon/src/lib.rs
+++ b/mullvad-daemon/src/lib.rs
@@ -693,7 +693,7 @@ where
 
         let api_handle = api_runtime
             .mullvad_rest_handle(
-                Box::pin(connection_modes_handler.as_stream()),
+                Box::pin(connection_modes_handler.clone().as_stream()),
                 endpoint_updater.callback(),
             )
             .await;

From ad7beb86f0ea5e796de5a352309d4f798c5d7b63 Mon Sep 17 00:00:00 2001
From: Markus Pettersson <markus.pettersson@mullvad.net>
Date: Thu, 16 Nov 2023 17:03:57 +0100
Subject: [PATCH 11/14] Add more error handling

---
 mullvad-daemon/src/api.rs | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/mullvad-daemon/src/api.rs b/mullvad-daemon/src/api.rs
index 9f60bab8ed95..6a69beda6588 100644
--- a/mullvad-daemon/src/api.rs
+++ b/mullvad-daemon/src/api.rs
@@ -46,6 +46,8 @@ pub enum Error {
 pub enum ActorError {
     #[error(display = "AccessModeSelector is not receiving any messages.")]
     SendFailed(#[error(source)] mpsc::TrySendError<Message>),
+    #[error(display = "AccessModeSelector is not receiving any messages.")]
+    OneshotSendFailed,
     #[error(display = "AccessModeSelector is not responding.")]
     NotRunning(#[error(source)] oneshot::Canceled),
 }
@@ -154,22 +156,29 @@ impl AccessModeSelector {
 
     async fn into_future(mut self) {
         while let Some(cmd) = self.cmd_rx.next().await {
-            let _ = match cmd {
+            let execution = match cmd {
                 Message::Get(tx) => self.on_get_access_method(tx),
                 Message::Set(tx, value) => self.on_set_access_method(tx, value),
                 Message::Next(tx) => self.on_next_connection_mode(tx),
                 Message::Update(tx, values) => self.on_update_access_methods(tx, values),
+            };
+            match execution {
+                Ok(_) => (),
+                Err(err) => {
+                    log::trace!(
+                        "AccessModeSelector is going down due to {error}",
+                        error = err
+                    );
+                    break;
+                }
             }
-            .map_err(|err| {
-                log::error!("todo(markus): Some error occured {err}");
-                err
-            });
         }
     }
 
     fn reply<T>(&self, tx: ResponseTx<T>, value: T) -> Result<()> {
-        // TODO(markus): The error probably should come from the value/tx
-        tx.send(Ok(value)).map_err(|_| Error::Generic)
+        Ok(tx
+            .send(Ok(value))
+            .map_err(|_| ActorError::OneshotSendFailed)?)
     }
 
     fn on_get_access_method(&mut self, tx: ResponseTx<AccessMethodSetting>) -> Result<()> {

From 031aa3b2cc4d7111634917e3d46254e58c23e3b9 Mon Sep 17 00:00:00 2001
From: Markus Pettersson <markus.pettersson@mullvad.net>
Date: Mon, 27 Nov 2023 10:04:32 +0100
Subject: [PATCH 12/14] Inline `ApiConnectionModeProvider` into
 `AccessModeSelector`

---
 mullvad-daemon/src/api.rs | 97 +++++++++++++++------------------------
 1 file changed, 36 insertions(+), 61 deletions(-)

diff --git a/mullvad-daemon/src/api.rs b/mullvad-daemon/src/api.rs
index 6a69beda6588..2814a1982adb 100644
--- a/mullvad-daemon/src/api.rs
+++ b/mullvad-daemon/src/api.rs
@@ -119,9 +119,21 @@ impl AccessModeSelectorHandle {
 
 /// A small actor which takes care of handling the logic around rotating
 /// connection modes to be used for Mullvad API request.
+///
+/// When `mullvad-api` fails to contact the API, it will request a new
+/// connection mode. The API can be connected to either directly (i.e.,
+/// [`ApiConnectionMode::Direct`]) via a bridge ([`ApiConnectionMode::Proxied`])
+/// or via any supported custom proxy protocol
+/// ([`api_access_methods::ObfuscationProtocol`]).
+///
+/// The strategy for determining the next [`ApiConnectionMode`] is handled by
+/// [`ConnectionModesIterator`].
 pub struct AccessModeSelector {
     cmd_rx: mpsc::UnboundedReceiver<Message>,
-    connection_mode_provider: ApiConnectionModeProvider,
+    cache_dir: PathBuf,
+    /// Used for selecting a Bridge when the `Mullvad Bridges` access method is used.
+    relay_selector: RelaySelector,
+    connection_modes: ConnectionModesIterator,
 }
 
 impl AccessModeSelector {
@@ -132,24 +144,23 @@ impl AccessModeSelector {
     ) -> AccessModeSelectorHandle {
         let (cmd_tx, cmd_rx) = mpsc::unbounded();
 
-        let state =
-            match ApiConnectionModeProvider::new(cache_dir, relay_selector, connection_modes) {
-                Ok(provider) => provider,
-                Err(api::Error::NoAccessMethods) => {
-                    // No settings seem to have been found. Default to using the the
-                    // direct access method.
-                    let default = mullvad_types::access_method::Settings::direct();
-                    api::ApiConnectionModeProvider::new(
-                    cache_dir.clone(),
-                    relay_selector.clone(),
-                    vec![default],
-                )
-                .expect(
+        let connection_modes = match ConnectionModesIterator::new(connection_modes) {
+            Ok(provider) => provider,
+            Err(Error::NoAccessMethods) => {
+                // No settings seem to have been found. Default to using the the
+                // direct access method.
+                let default = mullvad_types::access_method::Settings::direct();
+                ConnectionModesIterator::new(vec![default]).expect(
                     "Failed to create the data structure responsible for managing access methods",
                 )
-                }
-            };
-        let selector = AccessModeSelector { cmd_rx, connection_mode_provider: state };
+            }
+        };
+        let selector = AccessModeSelector {
+            cmd_rx,
+            cache_dir,
+            relay_selector,
+            connection_modes,
+        };
         tokio::spawn(selector.into_future());
         AccessModeSelectorHandle { cmd_tx }
     }
@@ -187,7 +198,7 @@ impl AccessModeSelector {
     }
 
     fn get_access_method(&mut self) -> AccessMethodSetting {
-        self.connection_mode_provider.connection_modes.peek()
+        self.connection_modes.peek()
     }
 
     fn on_set_access_method(
@@ -200,16 +211,14 @@ impl AccessModeSelector {
     }
 
     fn set_access_method(&mut self, value: AccessMethodSetting) {
-        self.connection_mode_provider
-            .connection_modes
-            .set_access_method(value);
+        self.connection_modes.set_access_method(value);
     }
 
     fn on_next_connection_mode(&mut self, tx: ResponseTx<ApiConnectionMode>) -> Result<()> {
         let next = self.next_connection_mode();
         // Save the new connection mode to cache!
         {
-            let cache_dir = self.connection_mode_provider.cache_dir.clone();
+            let cache_dir = self.cache_dir.clone();
             let next = next.clone();
             tokio::spawn(async move {
                 if next.save(&cache_dir).await.is_err() {
@@ -225,13 +234,12 @@ impl AccessModeSelector {
 
     fn next_connection_mode(&mut self) -> ApiConnectionMode {
         let access_method = self
-            .connection_mode_provider
             .connection_modes
             .next()
             .map(|access_method_setting| access_method_setting.access_method)
             .unwrap_or(AccessMethod::from(BuiltInAccessMethod::Direct));
 
-        let connection_mode = self.connection_mode_provider.from(access_method);
+        let connection_mode = self.from(access_method);
         log::info!(
             "New API connection mode selected: {connection_mode}",
             connection_mode = connection_mode
@@ -249,43 +257,7 @@ impl AccessModeSelector {
     }
 
     fn update_access_methods(&mut self, values: Vec<AccessMethodSetting>) {
-        self.connection_mode_provider
-            .connection_modes
-            .update_access_methods(values);
-    }
-}
-
-type ResponseTx<T> = oneshot::Sender<Result<T>>;
-type Result<T> = std::result::Result<T, Error>;
-
-/// A stream that returns the next API connection mode to use for reaching the API.
-///
-/// When `mullvad-api` fails to contact the API, it requests a new connection
-/// mode. The API can be connected to either directly (i.e.,
-/// [`ApiConnectionMode::Direct`]) via a bridge ([`ApiConnectionMode::Proxied`])
-/// or via any supported custom proxy protocol ([`api_access_methods::ObfuscationProtocol`]).
-///
-/// The strategy for determining the next [`ApiConnectionMode`] is handled by
-/// [`ConnectionModesIterator`].
-pub struct ApiConnectionModeProvider {
-    cache_dir: PathBuf,
-    /// Used for selecting a Bridge when the `Mullvad Bridges` access method is used.
-    relay_selector: RelaySelector,
-    connection_modes: ConnectionModesIterator,
-}
-
-impl ApiConnectionModeProvider {
-    pub(crate) fn new(
-        cache_dir: PathBuf,
-        relay_selector: RelaySelector,
-        connection_modes: Vec<AccessMethodSetting>,
-    ) -> Result<Self, Error> {
-        let connection_modes_iterator = ConnectionModesIterator::new(connection_modes)?;
-        Ok(Self {
-            cache_dir,
-            relay_selector,
-            connection_modes: connection_modes_iterator,
-        }
+        self.connection_modes.update_access_methods(values);
     }
 
     /// Ad-hoc version of [`std::convert::From::from`], but since some
@@ -330,6 +302,9 @@ impl ApiConnectionModeProvider {
     }
 }
 
+type ResponseTx<T> = oneshot::Sender<Result<T>>;
+type Result<T> = std::result::Result<T, Error>;
+
 /// An iterator which will always produce an [`AccessMethod`].
 ///
 /// Safety: It is always safe to [`unwrap`] after calling [`next`] on a

From 03c5a35285bbdb4fe89b2924bbca1115d8088357 Mon Sep 17 00:00:00 2001
From: Markus Pettersson <markus.pettersson@mullvad.net>
Date: Mon, 27 Nov 2023 10:05:27 +0100
Subject: [PATCH 13/14] Clean up error handling

---
 mullvad-daemon/src/access_method.rs |  96 ++++++++++++++++++++-
 mullvad-daemon/src/api.rs           |  55 +++++-------
 mullvad-daemon/src/lib.rs           | 126 +++++-----------------------
 3 files changed, 133 insertions(+), 144 deletions(-)

diff --git a/mullvad-daemon/src/access_method.rs b/mullvad-daemon/src/access_method.rs
index 97dfa815f599..7d9d3dba95d0 100644
--- a/mullvad-daemon/src/access_method.rs
+++ b/mullvad-daemon/src/access_method.rs
@@ -1,8 +1,9 @@
 use crate::{
-    api,
+    api::{self, AccessModeSelectorHandle},
     settings::{self, MadeChanges},
     Daemon, EventListener,
 };
+use mullvad_api::rest::{self, MullvadRestHandle};
 use mullvad_types::{
     access_method::{self, AccessMethod, AccessMethodSetting},
     settings::Settings,
@@ -26,6 +27,8 @@ pub enum Error {
     /// [`AccessMethodSetting`]s & [`ApiConnectionMode`]s.
     #[error(display = "Error occured when handling connection settings & details")]
     ConnectionMode(#[error(source)] api::Error),
+    #[error(display = "API endpoint rotation failed")]
+    RestError(#[error(source)] rest::Error),
     /// Access methods settings error
     #[error(display = "Settings error")]
     Settings(#[error(source)] settings::Error),
@@ -202,7 +205,7 @@ where
             tokio::spawn(async move {
                 match handle.update_access_methods(new_access_methods).await {
                     Ok(_) => (),
-                    Err(crate::api::Error::NoAccessMethods) => {
+                    Err(api::Error::NoAccessMethods) | Err(_) => {
                         // `access_methods` was empty! This implies that the user
                         // disabled all access methods. If we ever get into this
                         // state, we should default to using the direct access
@@ -225,3 +228,92 @@ where
         }
     }
 }
+
+/// 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::RestError(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::RestError)?;
+
+    // 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::RestError(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/api.rs b/mullvad-daemon/src/api.rs
index 2814a1982adb..2da307ff5f7b 100644
--- a/mullvad-daemon/src/api.rs
+++ b/mullvad-daemon/src/api.rs
@@ -35,15 +35,8 @@ pub enum Message {
 
 #[derive(err_derive::Error, Debug)]
 pub enum Error {
-    /// Oddly specific.
-    #[error(display = "Very Generic error.")]
-    Generic,
-    #[error(display = "{}", _0)]
-    Actor(#[error(source)] ActorError),
-}
-
-#[derive(err_derive::Error, Debug)]
-pub enum ActorError {
+    #[error(display = "No access methods were provided.")]
+    NoAccessMethods,
     #[error(display = "AccessModeSelector is not receiving any messages.")]
     SendFailed(#[error(source)] mpsc::TrySendError<Message>),
     #[error(display = "AccessModeSelector is not receiving any messages.")]
@@ -52,6 +45,9 @@ pub enum ActorError {
     NotRunning(#[error(source)] oneshot::Canceled),
 }
 
+type ResponseTx<T> = oneshot::Sender<Result<T>>;
+type Result<T> = std::result::Result<T, Error>;
+
 /// A channel for sending [`Message`] commands to a running
 /// [`AccessModeSelector`].
 #[derive(Clone)]
@@ -64,8 +60,8 @@ impl AccessModeSelectorHandle {
         let (tx, rx) = oneshot::channel();
         self.cmd_tx
             .unbounded_send(make_cmd(tx))
-            .map_err(ActorError::SendFailed)?;
-        rx.await.map_err(ActorError::NotRunning)?
+            .map_err(Error::SendFailed)?;
+        rx.await.map_err(Error::NotRunning)?
     }
 
     pub async fn get_access_method(&self) -> Result<AccessMethodSetting> {
@@ -105,7 +101,7 @@ impl AccessModeSelectorHandle {
     ///
     /// Practically converts the handle to a listener for when the
     /// currently valid connection modes changes.
-    pub fn as_stream(self) -> impl Stream<Item = ApiConnectionMode> {
+    pub fn into_stream(self) -> impl Stream<Item = ApiConnectionMode> {
         unfold(self, |handle| async move {
             match handle.next().await {
                 Ok(connection_mode) => Some((connection_mode, handle)),
@@ -146,7 +142,7 @@ impl AccessModeSelector {
 
         let connection_modes = match ConnectionModesIterator::new(connection_modes) {
             Ok(provider) => provider,
-            Err(Error::NoAccessMethods) => {
+            Err(Error::NoAccessMethods) | Err(_) => {
                 // No settings seem to have been found. Default to using the the
                 // direct access method.
                 let default = mullvad_types::access_method::Settings::direct();
@@ -187,9 +183,8 @@ impl AccessModeSelector {
     }
 
     fn reply<T>(&self, tx: ResponseTx<T>, value: T) -> Result<()> {
-        Ok(tx
-            .send(Ok(value))
-            .map_err(|_| ActorError::OneshotSendFailed)?)
+        tx.send(Ok(value)).map_err(|_| Error::OneshotSendFailed)?;
+        Ok(())
     }
 
     fn on_get_access_method(&mut self, tx: ResponseTx<AccessMethodSetting>) -> Result<()> {
@@ -240,10 +235,7 @@ impl AccessModeSelector {
             .unwrap_or(AccessMethod::from(BuiltInAccessMethod::Direct));
 
         let connection_mode = self.from(access_method);
-        log::info!(
-            "New API connection mode selected: {connection_mode}",
-            connection_mode = connection_mode
-        );
+        log::info!("New API connection mode selected: {connection_mode}");
         connection_mode
     }
 
@@ -252,12 +244,12 @@ impl AccessModeSelector {
         tx: ResponseTx<()>,
         values: Vec<AccessMethodSetting>,
     ) -> Result<()> {
-        self.update_access_methods(values);
+        self.update_access_methods(values)?;
         self.reply(tx, ())
     }
 
-    fn update_access_methods(&mut self, values: Vec<AccessMethodSetting>) {
-        self.connection_modes.update_access_methods(values);
+    fn update_access_methods(&mut self, values: Vec<AccessMethodSetting>) -> Result<()> {
+        self.connection_modes.update_access_methods(values)
     }
 
     /// Ad-hoc version of [`std::convert::From::from`], but since some
@@ -302,9 +294,6 @@ impl AccessModeSelector {
     }
 }
 
-type ResponseTx<T> = oneshot::Sender<Result<T>>;
-type Result<T> = std::result::Result<T, Error>;
-
 /// An iterator which will always produce an [`AccessMethod`].
 ///
 /// Safety: It is always safe to [`unwrap`] after calling [`next`] on a
@@ -319,14 +308,10 @@ pub struct ConnectionModesIterator {
     current: AccessMethodSetting,
 }
 
-#[derive(err_derive::Error, Debug)]
-pub enum Error {
-    #[error(display = "No access methods were provided.")]
-    NoAccessMethods,
-}
-
 impl ConnectionModesIterator {
-    pub fn new(access_methods: Vec<AccessMethodSetting>) -> Result<ConnectionModesIterator, Error> {
+    pub fn new(
+        access_methods: Vec<AccessMethodSetting>,
+    ) -> std::result::Result<ConnectionModesIterator, Error> {
         let mut iterator = Self::new_iterator(access_methods)?;
         Ok(Self {
             next: None,
@@ -344,7 +329,7 @@ impl ConnectionModesIterator {
     pub fn update_access_methods(
         &mut self,
         access_methods: Vec<AccessMethodSetting>,
-    ) -> Result<(), Error> {
+    ) -> std::result::Result<(), Error> {
         self.available_modes = Self::new_iterator(access_methods)?;
         Ok(())
     }
@@ -355,7 +340,7 @@ impl ConnectionModesIterator {
     /// returned.
     fn new_iterator(
         access_methods: Vec<AccessMethodSetting>,
-    ) -> Result<Box<dyn Iterator<Item = AccessMethodSetting> + Send>, Error> {
+    ) -> std::result::Result<Box<dyn Iterator<Item = AccessMethodSetting> + Send>, Error> {
         if access_methods.is_empty() {
             Err(Error::NoAccessMethods)
         } else {
diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs
index 9a2121260b98..0288f9d8c48f 100644
--- a/mullvad-daemon/src/lib.rs
+++ b/mullvad-daemon/src/lib.rs
@@ -693,7 +693,7 @@ where
 
         let api_handle = api_runtime
             .mullvad_rest_handle(
-                Box::pin(connection_modes_handler.clone().as_stream()),
+                Box::pin(connection_modes_handler.clone().into_stream()),
                 endpoint_updater.callback(),
             )
             .await;
@@ -1135,7 +1135,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).await,
+            TestApiAccessMethod(tx, method) => self.on_test_api_access_method(tx, method),
             IsPerformingPostUpgrade(tx) => self.on_is_performing_post_upgrade(tx),
             GetCurrentVersion(tx) => self.on_get_current_version(tx),
             #[cfg(not(target_os = "android"))]
@@ -2369,14 +2369,7 @@ where
         });
     }
 
-    /// 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(
+    fn on_test_api_access_method(
         &mut self,
         tx: ResponseTx<bool, Error>,
         access_method: mullvad_types::access_method::Id,
@@ -2387,105 +2380,24 @@ where
         // access method.
         let api_handle = self.api_handle.clone();
         let handle = self.connection_modes_handler.clone();
-        let access_method = self.get_api_access_method(access_method);
-        // TODO(markus): Clean up this error handling
-        let new_access_method = if let Ok(access_method) = access_method {
-            access_method
-        } else {
-            Self::oneshot_send(
-                tx,
-                access_method
-                    .map(|_| false)
-                    .map_err(Error::AccessMethodError),
-                "on_test_api_access_method response",
-            );
-            return;
-        };
-
-        let fut = async move {
-            // Setup test
-            let previous_access_method = handle
-                .get_access_method()
-                .await
-                .map_err(Error::ApiConnectionModeError)
-                // TODO(markus): Do not unwrap!
-                .unwrap();
+        let access_method_lookup = self
+            .get_api_access_method(access_method)
+            .map_err(Error::AccessMethodError);
 
-            let x = new_access_method.clone();
-            handle.set_access_method(new_access_method)
-                .await
-                .map_err(Error::ApiConnectionModeError)
-                // TODO(markus): Do not unwrap!
-                .unwrap();
-
-            // We need to perform a rotation of API endpoint after a set action
-            let rotation_handle = api_handle.clone();
-            rotation_handle
-                .service()
-                .next_api_endpoint()
-                .await
-                .map_err(|err| {
-                    log::error!("Failed to rotate API endpoint: {err}");
-                    err
-                })
-                // TODO(markus): Error handling
-                .unwrap();
-
-            // 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.
-            handle
-                .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."
-                    );
-                    err
-                })
-                // TODO(markus): Do not unwrap!
-                .unwrap();
-
-            // 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(api_handle)
-                .api_addrs_available()
-                .await
-                .map_err(Error::RestError);
-
-            // 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.as_ref().is_ok_and(|&succeeded| succeeded) {
-                rotation_handle
-                .service()
-                .next_api_endpoint()
-                .await
-                .map_err(|err| {
-                    log::error!("Failed to rotate API endpoint: {err}");
-                    err
-                })
-                // TODO(markus): Error handling
-                .unwrap();
+        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");
+                });
             }
-
-            log::info!(
-                "The result of testing {method:?} is {result:?}",
-                method = x.access_method,
-                result = result
-            );
-
-            Self::oneshot_send(tx, result, "on_test_api_access_method response");
-        };
-
-        tokio::spawn(fut);
+            Err(err) => {
+                Self::oneshot_send(tx, Err(err), "on_test_api_access_method response");
+            }
+        }
     }
 
     fn on_get_settings(&self, tx: oneshot::Sender<Settings>) {

From 7963de8a86902c4eb02e2a967353124e77ce3616 Mon Sep 17 00:00:00 2001
From: Jon Petersson <jon.petersson@kvadrat.se>
Date: Wed, 29 Nov 2023 16:17:38 +0100
Subject: [PATCH 14/14] Refactor account deletion in TunnelManager

---
 ios/MullvadVPN.xcodeproj/project.pbxproj      |  6 --
 .../DeleteAccountOperation.swift              | 61 -------------------
 .../TunnelManager/SetAccountOperation.swift   | 54 ++++++++++++++++
 .../TunnelManager/TunnelInteractor.swift      |  1 +
 .../TunnelManager/TunnelManager.swift         | 56 +++++------------
 .../AccountDeletionInteractor.swift           |  4 +-
 .../AccountDeletionViewController.swift       |  4 +-
 7 files changed, 73 insertions(+), 113 deletions(-)
 delete mode 100644 ios/MullvadVPN/TunnelManager/DeleteAccountOperation.swift

diff --git a/ios/MullvadVPN.xcodeproj/project.pbxproj b/ios/MullvadVPN.xcodeproj/project.pbxproj
index 820982df23da..f0867032d355 100644
--- a/ios/MullvadVPN.xcodeproj/project.pbxproj
+++ b/ios/MullvadVPN.xcodeproj/project.pbxproj
@@ -576,7 +576,6 @@
 		A9A5FA0F2ACB05160083449F /* StoreSubscription.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5846227026E229F20035F7C2 /* StoreSubscription.swift */; };
 		A9A5FA102ACB05160083449F /* PacketTunnelTransport.swift in Sources */ = {isa = PBXBuildFile; fileRef = 063687B928EB234F00BE7161 /* PacketTunnelTransport.swift */; };
 		A9A5FA112ACB05160083449F /* TransportMonitor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0697D6E628F01513007A9E99 /* TransportMonitor.swift */; };
-		A9A5FA122ACB05160083449F /* DeleteAccountOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0C6FA802A66E23300F521F0 /* DeleteAccountOperation.swift */; };
 		A9A5FA132ACB05160083449F /* LoadTunnelConfigurationOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 588527B1276B3F0700BAA373 /* LoadTunnelConfigurationOperation.swift */; };
 		A9A5FA142ACB05160083449F /* MapConnectionStatusOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58F2E147276A307400A79513 /* MapConnectionStatusOperation.swift */; };
 		A9A5FA152ACB05160083449F /* RedeemVoucherOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = F07BF2612A26279100042943 /* RedeemVoucherOperation.swift */; };
@@ -672,7 +671,6 @@
 		F0B0E6972AFE6E7E001DC66B /* XCTest+Async.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0B0E6962AFE6E7E001DC66B /* XCTest+Async.swift */; };
 		F0C2AEFD2A0BB5CC00986207 /* NotificationProviderIdentifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0C2AEFC2A0BB5CC00986207 /* NotificationProviderIdentifier.swift */; };
 		F0C6A8432AB08E54000777A8 /* RedeemVoucherViewConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0C6A8422AB08E54000777A8 /* RedeemVoucherViewConfiguration.swift */; };
-		F0C6FA812A66E23300F521F0 /* DeleteAccountOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0C6FA802A66E23300F521F0 /* DeleteAccountOperation.swift */; };
 		F0C6FA852A6A733700F521F0 /* InAppPurchaseInteractor.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0C6FA842A6A733700F521F0 /* InAppPurchaseInteractor.swift */; };
 		F0D8825B2B04F53600D3EF9A /* OutgoingConnectionData.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0D8825A2B04F53600D3EF9A /* OutgoingConnectionData.swift */; };
 		F0D8825C2B04F70E00D3EF9A /* OutgoingConnectionData.swift in Sources */ = {isa = PBXBuildFile; fileRef = F0D8825A2B04F53600D3EF9A /* OutgoingConnectionData.swift */; };
@@ -1683,7 +1681,6 @@
 		F0B0E6962AFE6E7E001DC66B /* XCTest+Async.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "XCTest+Async.swift"; sourceTree = "<group>"; };
 		F0C2AEFC2A0BB5CC00986207 /* NotificationProviderIdentifier.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationProviderIdentifier.swift; sourceTree = "<group>"; };
 		F0C6A8422AB08E54000777A8 /* RedeemVoucherViewConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RedeemVoucherViewConfiguration.swift; sourceTree = "<group>"; };
-		F0C6FA802A66E23300F521F0 /* DeleteAccountOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DeleteAccountOperation.swift; sourceTree = "<group>"; };
 		F0C6FA842A6A733700F521F0 /* InAppPurchaseInteractor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InAppPurchaseInteractor.swift; sourceTree = "<group>"; };
 		F0D8825A2B04F53600D3EF9A /* OutgoingConnectionData.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OutgoingConnectionData.swift; sourceTree = "<group>"; };
 		F0DA87462A9CB9A2006044F1 /* AccountExpiryRow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AccountExpiryRow.swift; sourceTree = "<group>"; };
@@ -2014,7 +2011,6 @@
 		5823FA5726CE4A4100283BF8 /* TunnelManager */ = {
 			isa = PBXGroup;
 			children = (
-				F0C6FA802A66E23300F521F0 /* DeleteAccountOperation.swift */,
 				588527B1276B3F0700BAA373 /* LoadTunnelConfigurationOperation.swift */,
 				58F2E147276A307400A79513 /* MapConnectionStatusOperation.swift */,
 				F07BF2612A26279100042943 /* RedeemVoucherOperation.swift */,
@@ -4217,7 +4213,6 @@
 				A9A5FA0F2ACB05160083449F /* StoreSubscription.swift in Sources */,
 				A9A5FA102ACB05160083449F /* PacketTunnelTransport.swift in Sources */,
 				A9A5FA112ACB05160083449F /* TransportMonitor.swift in Sources */,
-				A9A5FA122ACB05160083449F /* DeleteAccountOperation.swift in Sources */,
 				A9B6AC1A2ADE8FBB00F7802A /* InMemorySettingsStore.swift in Sources */,
 				A9A5FA132ACB05160083449F /* LoadTunnelConfigurationOperation.swift in Sources */,
 				A9A5FA142ACB05160083449F /* MapConnectionStatusOperation.swift in Sources */,
@@ -4519,7 +4514,6 @@
 				7AF9BE8E2A331C7B00DBFEDB /* RelayFilterViewModel.swift in Sources */,
 				58F3C0A4249CB069003E76BE /* HeaderBarView.swift in Sources */,
 				5864AF0829C78849005B0CD9 /* CellFactoryProtocol.swift in Sources */,
-				F0C6FA812A66E23300F521F0 /* DeleteAccountOperation.swift in Sources */,
 				F07CFF2029F2720E008C0343 /* RegisteredDeviceInAppNotificationProvider.swift in Sources */,
 				587A01FC23F1F0BE00B68763 /* SimulatorTunnelProviderHost.swift in Sources */,
 				7A6F2FA72AFBB9AE006D0856 /* AccountExpiry.swift in Sources */,
diff --git a/ios/MullvadVPN/TunnelManager/DeleteAccountOperation.swift b/ios/MullvadVPN/TunnelManager/DeleteAccountOperation.swift
deleted file mode 100644
index 8731ca2a5cf1..000000000000
--- a/ios/MullvadVPN/TunnelManager/DeleteAccountOperation.swift
+++ /dev/null
@@ -1,61 +0,0 @@
-//
-//  DeleteAccountOperation.swift
-//  MullvadVPN
-//
-//  Created by Mojgan on 2023-07-18.
-//  Copyright © 2023 Mullvad VPN AB. All rights reserved.
-//
-
-import Foundation
-import MullvadLogging
-import MullvadREST
-import MullvadTypes
-import Operations
-
-class DeleteAccountOperation: ResultOperation<Void> {
-    private let logger = Logger(label: "\(DeleteAccountOperation.self)")
-
-    private let accountNumber: String
-    private let accountsProxy: RESTAccountHandling
-    private let accessTokenManager: RESTAccessTokenManagement
-    private var task: Cancellable?
-
-    init(
-        dispatchQueue: DispatchQueue,
-        accountsProxy: RESTAccountHandling,
-        accessTokenManager: RESTAccessTokenManagement,
-        accountNumber: String
-    ) {
-        self.accountNumber = accountNumber
-        self.accountsProxy = accountsProxy
-        self.accessTokenManager = accessTokenManager
-        super.init(dispatchQueue: dispatchQueue)
-    }
-
-    override func main() {
-        task = accountsProxy.deleteAccount(
-            accountNumber: accountNumber,
-            retryStrategy: .default,
-            completion: { [weak self] result in
-                self?.dispatchQueue.async {
-                    switch result {
-                    case .success:
-                        self?.accessTokenManager.invalidateAllTokens()
-                        self?.finish(result: .success(()))
-                    case let .failure(error):
-                        self?.logger.error(
-                            error: error,
-                            message: "Failed to delete account."
-                        )
-                        self?.finish(result: .failure(error))
-                    }
-                }
-            }
-        )
-    }
-
-    override func operationDidCancel() {
-        task?.cancel()
-        task = nil
-    }
-}
diff --git a/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift b/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift
index 1f8307d2b7e3..da9dab6fef3d 100644
--- a/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift
+++ b/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift
@@ -24,11 +24,15 @@ enum SetAccountAction {
     /// Unset account.
     case unset
 
+    /// Delete account.
+    case delete(String)
+
     var taskName: String {
         switch self {
         case .new: "Set new account"
         case .existing: "Set existing account"
         case .unset: "Unset account"
+        case .delete: "Delete account"
         }
     }
 }
@@ -78,6 +82,11 @@ class SetAccountOperation: ResultOperation<StoredAccountData?> {
 
             case .unset:
                 finish(result: .success(nil))
+
+            case let .delete(accountNumber):
+                startDeleteAccountFlow(accountNumber: accountNumber) { [self] result in
+                    finish(result: result.map { .none })
+                }
             }
         }
     }
@@ -141,6 +150,27 @@ class SetAccountOperation: ResultOperation<StoredAccountData?> {
         }
     }
 
+    /**
+     Begin delete flow of an existing account by performing the following steps:
+
+     1. Delete existing account with the API.
+     2. Reset tunnel settings to default and remove last used account.
+     */
+    private func startDeleteAccountFlow(
+        accountNumber: String,
+        completion: @escaping (Result<Void, Error>) -> Void
+    ) {
+        deleteAccount(accountNumber: accountNumber) { [self] result in
+            interactor.setSettings(LatestTunnelSettings(), persist: true)
+
+            if result.isSuccess {
+                interactor.removeLastUsedAccount()
+            }
+
+            completion(result)
+        }
+    }
+
     /**
      Continue login flow after receiving account data as a part of creating new or retrieving existing account from
      the API by performing the following steps:
@@ -264,6 +294,28 @@ class SetAccountOperation: ResultOperation<StoredAccountData?> {
         tasks.append(task)
     }
 
+    /// Delete account.
+    private func deleteAccount(accountNumber: String, completion: @escaping (Result<Void, Error>) -> Void) {
+        logger.debug("Delete account...")
+
+        let task = accountsProxy.deleteAccount(
+            accountNumber: accountNumber,
+            retryStrategy: .default
+        ) { [self] result in
+            dispatchQueue.async { [self] in
+                let result = result.inspectError { error in
+                    guard !error.isOperationCancellationError else { return }
+
+                    logger.error(error: error, message: "Failed to delete account.")
+                }
+
+                completion(result)
+            }
+        }
+
+        tasks.append(task)
+    }
+
     /// Delete device from API.
     private func deleteDevice(accountNumber: String, deviceIdentifier: String, completion: @escaping (Error?) -> Void) {
         logger.debug("Delete current device...")
@@ -398,3 +450,5 @@ class SetAccountOperation: ResultOperation<StoredAccountData?> {
         var device: Device
     }
 }
+
+// swiftlint:disable:this file_length
diff --git a/ios/MullvadVPN/TunnelManager/TunnelInteractor.swift b/ios/MullvadVPN/TunnelManager/TunnelInteractor.swift
index 15f118f0352e..3ec3a9791bf8 100644
--- a/ios/MullvadVPN/TunnelManager/TunnelInteractor.swift
+++ b/ios/MullvadVPN/TunnelManager/TunnelInteractor.swift
@@ -33,6 +33,7 @@ protocol TunnelInteractor {
     func setConfigurationLoaded()
     func setSettings(_ settings: LatestTunnelSettings, persist: Bool)
     func setDeviceState(_ deviceState: DeviceState, persist: Bool)
+    func removeLastUsedAccount()
     func handleRestError(_ error: Error)
 
     func startTunnel()
diff --git a/ios/MullvadVPN/TunnelManager/TunnelManager.swift b/ios/MullvadVPN/TunnelManager/TunnelManager.swift
index bd1583b11fbe..c6c4b7323452 100644
--- a/ios/MullvadVPN/TunnelManager/TunnelManager.swift
+++ b/ios/MullvadVPN/TunnelManager/TunnelManager.swift
@@ -364,10 +364,13 @@ final class TunnelManager: StorePaymentObserver {
             MutuallyExclusive(category: OperationCategory.settingsUpdate.category)
         )
 
-        // Unsetting the account (ie. logging out) should cancel all other currently ongoing
-        // activity.
-        if case .unset = action {
+        // Unsetting (ie. logging out) or deleting the account should cancel all other
+        // currently ongoing activity.
+        switch action {
+        case .unset, .delete:
             operationQueue.cancelAllOperations()
+        default:
+            break
         }
 
         operationQueue.addOperation(operation)
@@ -437,43 +440,10 @@ final class TunnelManager: StorePaymentObserver {
     func deleteAccount(
         accountNumber: String,
         completion: ((Error?) -> Void)? = nil
-    ) -> Cancellable {
-        let operation = DeleteAccountOperation(
-            dispatchQueue: internalQueue,
-            accountsProxy: accountsProxy,
-            accessTokenManager: accessTokenManager,
-            accountNumber: accountNumber
-        )
-
-        operation.completionQueue = .main
-        operation.completionHandler = { [weak self] result in
-            switch result {
-            case .success:
-                self?.unsetTunnelConfiguration {
-                    self?.operationQueue.cancelAllOperations()
-                    self?.wipeAllUserData()
-                    self?.setDeviceState(.loggedOut, persist: true)
-                    DispatchQueue.main.async {
-                        completion?(nil)
-                    }
-                }
-            case let .failure(error):
-                completion?(error)
-            }
+    ) {
+        setAccount(action: .delete(accountNumber)) { result in
+            completion?(result.error)
         }
-
-        operation.addObserver(
-            BackgroundObserver(
-                application: application,
-                name: "Delete account",
-                cancelUponExpiration: true
-            )
-        )
-
-        operation.addCondition(MutuallyExclusive(category: OperationCategory.deviceStateUpdate.category))
-
-        operationQueue.addOperation(operation)
-        return operation
     }
 
     func updateDeviceData(_ completionHandler: ((Error?) -> Void)? = nil) {
@@ -1092,7 +1062,7 @@ final class TunnelManager: StorePaymentObserver {
         isRunningPeriodicPrivateKeyRotation = false
     }
 
-    private func wipeAllUserData() {
+    fileprivate func removeLastUsedAccount() {
         do {
             try SettingsManager.setLastUsedAccount(nil)
         } catch {
@@ -1121,7 +1091,7 @@ final class TunnelManager: StorePaymentObserver {
             unsetTunnelConfiguration {
                 self.setDeviceState(.revoked, persist: true)
                 self.operationQueue.cancelAllOperations()
-                self.wipeAllUserData()
+                self.removeLastUsedAccount()
             }
         default:
             break
@@ -1307,6 +1277,10 @@ private struct TunnelInteractorProxy: TunnelInteractor {
         tunnelManager.setDeviceState(deviceState, persist: persist)
     }
 
+    func removeLastUsedAccount() {
+        tunnelManager.removeLastUsedAccount()
+    }
+
     func startTunnel() {
         tunnelManager.startTunnel()
     }
diff --git a/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionInteractor.swift b/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionInteractor.swift
index 50bf101f4fad..1562c013b835 100644
--- a/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionInteractor.swift	
+++ b/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionInteractor.swift	
@@ -48,7 +48,7 @@ class AccountDeletionInteractor {
         }
     }
 
-    func delete(accountNumber: String, completionHandler: @escaping (Error?) -> Void) -> Cancellable {
-        return tunnelManager.deleteAccount(accountNumber: accountNumber, completion: completionHandler)
+    func delete(accountNumber: String, completionHandler: @escaping (Error?) -> Void) {
+        tunnelManager.deleteAccount(accountNumber: accountNumber, completion: completionHandler)
     }
 }
diff --git a/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionViewController.swift b/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionViewController.swift
index 8fce5224aa53..cea4095b75c1 100644
--- a/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionViewController.swift	
+++ b/ios/MullvadVPN/View controllers/AccountDeletion/AccountDeletionViewController.swift	
@@ -15,7 +15,6 @@ protocol AccountDeletionViewControllerDelegate: AnyObject {
 }
 
 class AccountDeletionViewController: UIViewController {
-    private var task: Cancellable?
     private lazy var contentView: AccountDeletionContentView = {
         let view = AccountDeletionContentView()
         view.delegate = self
@@ -63,7 +62,7 @@ class AccountDeletionViewController: UIViewController {
 
     private func submit(accountNumber: String) {
         contentView.state = .loading
-        task = interactor.delete(accountNumber: accountNumber) { [weak self] error in
+        interactor.delete(accountNumber: accountNumber) { [weak self] error in
             guard let self else { return }
             guard let error else {
                 self.contentView.state = .initial
@@ -78,7 +77,6 @@ class AccountDeletionViewController: UIViewController {
 extension AccountDeletionViewController: AccountDeletionContentViewDelegate {
     func didTapCancelButton(contentView: AccountDeletionContentView, button: AppButton) {
         contentView.isEditing = false
-        task?.cancel()
         delegate?.deleteAccountDidCancel(controller: self)
     }