Merge branch 'update-account-migration'

8 files changed, 623 insertions, 374 deletions

diff --git a/mullvad-daemon/src/account_history.rs b/mullvad-daemon/src/account_history.rs
index 4b83915ff7..da8d86570e 100644
--- a/mullvad-daemon/src/account_history.rs
+++ b/mullvad-daemon/src/account_history.rs
@@ -1,13 +1,11 @@
-use crate::settings::SettingsPersister;
-use mullvad_types::{account::AccountToken, wireguard::WireguardData};
+use mullvad_types::account::AccountToken;
 use regex::Regex;
-use std::{
+use std::path::Path;
+use talpid_types::ErrorExt;
+use tokio::{
     fs,
-    io::{self, Read, Seek, Write},
-    path::Path,
-    sync::{Arc, Mutex},
+    io::{self, AsyncReadExt, AsyncSeekExt, AsyncWriteExt},
 };
-use talpid_types::ErrorExt;
 
 pub type Result<T> = std::result::Result<T, Error>;
 
@@ -30,7 +28,7 @@ pub enum Error {
 static ACCOUNT_HISTORY_FILE: &str = "account-history.json";
 
 pub struct AccountHistory {
-    file: Arc<Mutex<io::BufWriter<fs::File>>>,
+    file: io::BufWriter<fs::File>,
     token: Option<AccountToken>,
 }
 
@@ -41,128 +39,55 @@ lazy_static::lazy_static! {
 
 impl AccountHistory {
     pub async fn new(
-        cache_dir: &Path,
         settings_dir: &Path,
-        settings: &mut SettingsPersister,
+        current_token: Option<AccountToken>,
     ) -> Result<AccountHistory> {
-        Self::migrate_from_old_file_location(cache_dir, settings_dir).await;
-
         let mut options = fs::OpenOptions::new();
         #[cfg(unix)]
         {
-            use std::os::unix::fs::OpenOptionsExt;
             options.mode(0o600);
         }
         #[cfg(windows)]
         {
-            use std::os::windows::fs::OpenOptionsExt;
             // a share mode of zero ensures exclusive access to the file to *this* process
             options.share_mode(0);
         }
+
         let path = settings_dir.join(ACCOUNT_HISTORY_FILE);
-        let (file, token) = if path.is_file() {
-            log::info!("Opening account history file in {}", path.display());
-            let mut reader = options
-                .write(true)
-                .read(true)
-                .open(path)
-                .map(io::BufReader::new)
-                .map_err(Error::Read)?;
+        log::info!("Opening account history file in {}", path.display());
+        let mut reader = options
+            .write(true)
+            .create(true)
+            .read(true)
+            .open(path)
+            .await
+            .map(io::BufReader::new)
+            .map_err(Error::Read)?;
 
-            let mut buffer = String::new();
-            let token: Option<AccountToken> = match reader.read_to_string(&mut buffer) {
-                Ok(0) => None,
-                Ok(_) if ACCOUNT_REGEX.is_match(&buffer) => Some(buffer),
+        let mut buffer = String::new();
+        let (token, should_save): (Option<AccountToken>, bool) =
+            match reader.read_to_string(&mut buffer).await {
+                Ok(_) if ACCOUNT_REGEX.is_match(&buffer) => (Some(buffer), false),
+                Ok(0) => (current_token, true),
                 Ok(_) | Err(_) => {
-                    log::warn!("Failed to parse account history. Trying old formats",);
-                    match Self::try_format_v2(&mut reader)? {
-                        Some((token, migrated_data)) => {
-                            if let Err(error) = settings.set_wireguard(migrated_data).await {
-                                log::error!(
-                                    "{}",
-                                    error.display_chain_with_msg(
-                                        "Failed to migrate WireGuard key from account history"
-                                    )
-                                );
-                            }
-                            Some(token)
-                        }
-                        None => Self::try_format_v1(&mut reader)?,
-                    }
+                    log::warn!("Failed to parse account history");
+                    (current_token, true)
                 }
             };
 
-            (reader.into_inner(), token)
-        } else {
-            log::info!("Creating account history file in {}", path.display());
-            (
-                options
-                    .write(true)
-                    .create(true)
-                    .open(path)
-                    .map_err(Error::Read)?,
-                settings.get_account_token(),
-            )
-        };
-        let file = io::BufWriter::new(file);
-        let mut history = AccountHistory {
-            file: Arc::new(Mutex::new(file)),
-            token,
-        };
-        if let Err(e) = history.save_to_disk().await {
-            log::error!("Failed to save account cache after opening it: {}", e);
+        let file = io::BufWriter::new(reader.into_inner());
+        let mut history = AccountHistory { file, token };
+        if should_save {
+            if let Err(error) = history.save_to_disk().await {
+                log::error!(
+                    "{}",
+                    error.display_chain_with_msg("Failed to save account history after opening it")
+                );
+            }
         }
         Ok(history)
     }
 
-    async fn migrate_from_old_file_location(old_dir: &Path, new_dir: &Path) {
-        use tokio::fs;
-
-        let old_path = old_dir.join(ACCOUNT_HISTORY_FILE);
-        let new_path = new_dir.join(ACCOUNT_HISTORY_FILE);
-        if !old_path.exists() || new_path.exists() || new_path == old_path {
-            return;
-        }
-
-        if let Err(error) = fs::copy(&old_path, &new_path).await {
-            log::error!(
-                "{}",
-                error.display_chain_with_msg("Failed to migrate account history file location")
-            );
-        } else {
-            let _ = fs::remove_file(old_path).await;
-        }
-    }
-
-    fn try_format_v1(reader: &mut io::BufReader<fs::File>) -> Result<Option<AccountToken>> {
-        #[derive(Deserialize)]
-        struct OldFormat {
-            accounts: Vec<AccountToken>,
-        }
-        reader.seek(io::SeekFrom::Start(0)).map_err(Error::Read)?;
-        Ok(serde_json::from_reader(reader)
-            .map(|old_format: OldFormat| old_format.accounts.first().cloned())
-            .unwrap_or_else(|_| None))
-    }
-
-    fn try_format_v2(
-        reader: &mut io::BufReader<fs::File>,
-    ) -> Result<Option<(AccountToken, Option<WireguardData>)>> {
-        #[derive(Serialize, Deserialize, Clone, Debug)]
-        pub struct AccountEntry {
-            pub account: AccountToken,
-            pub wireguard: Option<WireguardData>,
-        }
-        reader.seek(io::SeekFrom::Start(0)).map_err(Error::Read)?;
-        Ok(serde_json::from_reader(reader)
-            .map(|entries: Vec<AccountEntry>| {
-                entries
-                    .first()
-                    .map(|entry| (entry.account.clone(), entry.wireguard.clone()))
-            })
-            .unwrap_or_else(|_| None))
-    }
-
     /// Gets the account token in the history
     pub fn get(&self) -> Option<AccountToken> {
         self.token.clone()
@@ -181,20 +106,18 @@ impl AccountHistory {
     }
 
     async fn save_to_disk(&mut self) -> Result<()> {
-        let file = self.file.clone();
-        let token = self.token.clone();
-
-        tokio::task::spawn_blocking(move || {
-            let mut file = file.lock().unwrap();
-            file.get_mut().set_len(0).map_err(Error::Write)?;
-            file.seek(io::SeekFrom::Start(0)).map_err(Error::Write)?;
-            if let Some(token) = token {
-                write!(&mut file, "{}", token).map_err(Error::Write)?;
-            }
-            file.flush().map_err(Error::Write)?;
-            file.get_mut().sync_all().map_err(Error::Write)
-        })
-        .await
-        .map_err(Error::WriteCancelled)?
+        self.file.get_mut().set_len(0).await.map_err(Error::Write)?;
+        self.file
+            .seek(io::SeekFrom::Start(0))
+            .await
+            .map_err(Error::Write)?;
+        if let Some(ref token) = self.token {
+            self.file
+                .write_all(token.as_bytes())
+                .await
+                .map_err(Error::Write)?;
+        }
+        self.file.flush().await.map_err(Error::Write)?;
+        self.file.get_mut().sync_all().await.map_err(Error::Write)
     }
 }
diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs
index 556483b50e..7597f01020 100644
--- a/mullvad-daemon/src/lib.rs
+++ b/mullvad-daemon/src/lib.rs
@@ -603,10 +603,10 @@ where
         );
 
 
-        if let Err(error) = migrations::migrate_all(&settings_dir).await {
+        if let Err(error) = migrations::migrate_all(&cache_dir, &settings_dir).await {
             log::error!(
                 "{}",
-                error.display_chain_with_msg("Failed to migrate settings")
+                error.display_chain_with_msg("Failed to migrate settings or cache")
             );
         }
         let mut settings = SettingsPersister::load(&settings_dir).await;
@@ -626,7 +626,7 @@ where
         );
         tokio::spawn(version_updater.run());
         let account_history =
-            account_history::AccountHistory::new(&cache_dir, &settings_dir, &mut settings)
+            account_history::AccountHistory::new(&settings_dir, settings.get_account_token())
                 .await
                 .map_err(Error::LoadAccountHistory)?;
 
diff --git a/mullvad-daemon/src/migrations/account_history.rs b/mullvad-daemon/src/migrations/account_history.rs
new file mode 100644
index 0000000000..4dceceb9fd
--- /dev/null
+++ b/mullvad-daemon/src/migrations/account_history.rs
@@ -0,0 +1,345 @@
+use super::{Error, Result};
+use mullvad_types::{account::AccountToken, wireguard::WireguardData};
+use regex::Regex;
+use std::path::Path;
+use talpid_types::ErrorExt;
+use tokio::{
+    fs,
+    io::{self, AsyncReadExt, AsyncSeekExt, AsyncWriteExt},
+};
+
+
+const ACCOUNT_HISTORY_FILE: &str = "account-history.json";
+
+lazy_static::lazy_static! {
+    static ref ACCOUNT_REGEX: Regex = Regex::new(r"^[0-9]+$").unwrap();
+}
+
+
+pub async fn migrate_location(old_dir: &Path, new_dir: &Path) {
+    let old_path = old_dir.join(ACCOUNT_HISTORY_FILE);
+    let new_path = new_dir.join(ACCOUNT_HISTORY_FILE);
+    if !old_path.exists() || new_path.exists() || new_path == old_path {
+        return;
+    }
+
+    if let Err(error) = fs::copy(&old_path, &new_path).await {
+        log::error!(
+            "{}",
+            error.display_chain_with_msg("Failed to migrate account history file location")
+        );
+    } else {
+        let _ = fs::remove_file(old_path).await;
+    }
+}
+
+pub async fn migrate_formats(settings_dir: &Path, settings: &mut serde_json::Value) -> Result<()> {
+    let path = settings_dir.join(ACCOUNT_HISTORY_FILE);
+    if !path.is_file() {
+        return Ok(());
+    }
+
+    let mut options = fs::OpenOptions::new();
+    #[cfg(unix)]
+    {
+        options.mode(0o600);
+    }
+    let mut file = options
+        .write(true)
+        .read(true)
+        .open(path)
+        .await
+        .map_err(Error::ReadHistoryError)?;
+
+    let mut bytes = vec![];
+    file.read_to_end(&mut bytes)
+        .await
+        .map_err(Error::ReadHistoryError)?;
+
+    if is_format_v3(&bytes) {
+        return Ok(());
+    }
+
+    let token = migrate_formats_inner(&bytes, settings)?;
+
+    file.set_len(0).await.map_err(Error::WriteHistoryError)?;
+    file.seek(io::SeekFrom::Start(0))
+        .await
+        .map_err(Error::WriteHistoryError)?;
+    file.write_all(token.as_bytes())
+        .await
+        .map_err(Error::WriteHistoryError)?;
+    file.flush().await.map_err(Error::WriteHistoryError)?;
+    file.sync_all().await.map_err(Error::WriteHistoryError)?;
+
+    Ok(())
+}
+
+fn migrate_formats_inner(
+    account_bytes: &[u8],
+    settings: &mut serde_json::Value,
+) -> Result<AccountToken> {
+    if let Some((token, wg_data)) = try_format_v2(account_bytes) {
+        settings["wireguard"] = serde_json::json!(wg_data);
+        Ok(token)
+    } else if let Some(token) = try_format_v1(account_bytes) {
+        Ok(token)
+    } else {
+        Err(Error::ParseHistoryError)
+    }
+}
+
+fn is_format_v3(bytes: &[u8]) -> bool {
+    match std::str::from_utf8(bytes) {
+        Ok(token) => token.is_empty() || ACCOUNT_REGEX.is_match(token),
+        Err(_) => false,
+    }
+}
+
+fn try_format_v2(bytes: &[u8]) -> Option<(AccountToken, Option<WireguardData>)> {
+    #[derive(Serialize, Deserialize, Clone, Debug)]
+    pub struct AccountEntry {
+        pub account: AccountToken,
+        pub wireguard: Option<WireguardData>,
+    }
+    serde_json::from_slice(bytes)
+        .map(|entries: Vec<AccountEntry>| {
+            entries
+                .first()
+                .map(|entry| (entry.account.clone(), entry.wireguard.clone()))
+        })
+        .unwrap_or(None)
+}
+
+fn try_format_v1(bytes: &[u8]) -> Option<AccountToken> {
+    #[derive(Deserialize)]
+    struct OldFormat {
+        accounts: Vec<AccountToken>,
+    }
+    serde_json::from_slice(bytes)
+        .map(|old_format: OldFormat| old_format.accounts.first().cloned())
+        .unwrap_or(None)
+}
+
+#[cfg(test)]
+mod test {
+    use serde_json;
+
+    pub const ACCOUNT_HISTORY_V1: &str = r#"
+{
+  "accounts": ["1234", "4567"]
+}
+"#;
+    pub const ACCOUNT_HISTORY_V2: &str = r#"
+[
+  {
+  "account": "1234",
+    "wireguard": {
+      "private_key": "mAdSb4AfQOsAD5O/5+zG1oIhk3cUl0jUsyOeaOMFu3o=",
+      "addresses": {
+        "ipv4_address": "109.111.108.101/32",
+        "ipv6_address": "ffff::ffff/128"
+      },
+      "created": "1970-01-01T00:00:00Z"
+    }
+  },
+  {
+    "account": "4567",
+    "wireguard": {
+      "private_key": "mAdSb4AfQOsAD5O/5+zG1oIhk3cUl0jUsyOeaOMFu3o=",
+      "addresses": {
+        "ipv4_address": "109.111.108.101/32",
+        "ipv6_address": "ffff::ffff/128"
+      },
+      "created": "1970-01-01T00:00:00Z"
+    }
+  }
+]"#;
+    pub const ACCOUNT_HISTORY_V3: &str = r#"123456"#;
+
+    pub const OLD_SETTINGS: &str = r#"
+{
+  "account_token": "1234",
+  "relay_settings": {
+    "normal": {
+      "location": {
+        "only": {
+          "country": "se"
+        }
+      },
+      "tunnel_protocol": "any",
+      "wireguard_constraints": {
+        "port": {
+          "only": {
+            "protocol": "tcp",
+            "port": {
+              "only": 80
+            }
+          }
+        }
+      },
+      "openvpn_constraints": {
+        "port": {
+          "only": {
+            "protocol": "udp",
+            "port": {
+              "only": 1195
+            }
+          }
+        }
+      }
+    }
+  },
+  "bridge_settings": {
+    "normal": {
+      "location": "any"
+    }
+  },
+  "bridge_state": "auto",
+  "allow_lan": true,
+  "block_when_disconnected": false,
+  "auto_connect": false,
+  "tunnel_options": {
+    "openvpn": {
+      "mssfix": null
+    },
+    "wireguard": {
+      "mtu": null,
+      "rotation_interval": {
+          "secs": 86400,
+          "nanos": 0
+      }
+    },
+    "generic": {
+      "enable_ipv6": false
+    },
+    "dns_options": {
+      "state": "default",
+      "default_options": {
+        "block_ads": false,
+        "block_trackers": false
+      },
+      "custom_options": {
+        "addresses": [
+          "1.1.1.1",
+          "1.2.3.4"
+        ]
+      }
+    }
+  },
+  "settings_version": 5
+}
+"#;
+
+    pub const NEW_SETTINGS: &str = r#"
+{
+  "account_token": "1234",
+  "wireguard": {
+    "private_key": "mAdSb4AfQOsAD5O/5+zG1oIhk3cUl0jUsyOeaOMFu3o=",
+    "addresses": {
+      "ipv4_address": "109.111.108.101/32",
+      "ipv6_address": "ffff::ffff/128"
+    },
+    "created": "1970-01-01T00:00:00Z"
+  },
+  "relay_settings": {
+    "normal": {
+      "location": {
+        "only": {
+          "country": "se"
+        }
+      },
+      "tunnel_protocol": "any",
+      "wireguard_constraints": {
+        "port": {
+          "only": {
+            "protocol": "tcp",
+            "port": {
+              "only": 80
+            }
+          }
+        }
+      },
+      "openvpn_constraints": {
+        "port": {
+          "only": {
+            "protocol": "udp",
+            "port": {
+              "only": 1195
+            }
+          }
+        }
+      }
+    }
+  },
+  "bridge_settings": {
+    "normal": {
+      "location": "any"
+    }
+  },
+  "bridge_state": "auto",
+  "allow_lan": true,
+  "block_when_disconnected": false,
+  "auto_connect": false,
+  "tunnel_options": {
+    "openvpn": {
+      "mssfix": null
+    },
+    "wireguard": {
+      "mtu": null,
+      "rotation_interval": {
+          "secs": 86400,
+          "nanos": 0
+      }
+    },
+    "generic": {
+      "enable_ipv6": false
+    },
+    "dns_options": {
+      "state": "default",
+      "default_options": {
+        "block_ads": false,
+        "block_trackers": false
+      },
+      "custom_options": {
+        "addresses": [
+          "1.1.1.1",
+          "1.2.3.4"
+        ]
+      }
+    }
+  },
+  "settings_version": 5
+}
+"#;
+
+
+    // Test whether the current format is parsed correctly
+    #[test]
+    fn test_v3() {
+        assert!(!super::is_format_v3(ACCOUNT_HISTORY_V1.as_bytes()));
+        assert!(!super::is_format_v3(ACCOUNT_HISTORY_V2.as_bytes()));
+        assert!(super::is_format_v3(ACCOUNT_HISTORY_V3.as_bytes()));
+    }
+
+    #[test]
+    fn test_v2() {
+        assert!(super::try_format_v2(ACCOUNT_HISTORY_V1.as_bytes()).is_none());
+
+        let mut old_settings = serde_json::from_str(OLD_SETTINGS).unwrap();
+        let new_settings: serde_json::Value = serde_json::from_str(NEW_SETTINGS).unwrap();
+
+        // Test whether the wireguard data is moved to the settings correctly
+        let token =
+            super::migrate_formats_inner(ACCOUNT_HISTORY_V2.as_bytes(), &mut old_settings).unwrap();
+
+        assert_eq!(&old_settings, &new_settings);
+        assert_eq!(token, "1234");
+    }
+
+    #[test]
+    fn test_v1() {
+        let token = super::try_format_v1(ACCOUNT_HISTORY_V1.as_bytes());
+        assert_eq!(token, Some("1234".to_string()));
+    }
+}
diff --git a/mullvad-daemon/src/migrations/mod.rs b/mullvad-daemon/src/migrations/mod.rs
index 594ed21df8..28eaa1c83a 100644
--- a/mullvad-daemon/src/migrations/mod.rs
+++ b/mullvad-daemon/src/migrations/mod.rs
@@ -1,10 +1,10 @@
-use mullvad_types::settings::{Settings, CURRENT_SETTINGS_VERSION};
 use std::path::Path;
 use tokio::{
     fs,
     io::{self, AsyncWriteExt},
 };
 
+mod account_history;
 mod v1;
 mod v2;
 mod v3;
@@ -30,6 +30,15 @@ pub enum Error {
     #[error(display = "Unable to write new settings")]
     WriteError(#[error(source)] io::Error),
 
+    #[error(display = "Failed to read the account history")]
+    ReadHistoryError(#[error(source)] io::Error),
+
+    #[error(display = "Failed to write new account history")]
+    WriteHistoryError(#[error(source)] io::Error),
+
+    #[error(display = "Failed to parse account history")]
+    ParseHistoryError,
+
     #[cfg(windows)]
     #[error(display = "Failed to restore Windows update backup")]
     WinMigrationError(#[error(source)] windows::Error),
@@ -38,12 +47,7 @@ pub enum Error {
 pub type Result<T> = std::result::Result<T, Error>;
 
 
-trait SettingsMigration {
-    fn version_matches(&self, settings: &mut serde_json::Value) -> bool;
-    fn migrate(&self, settings: &mut serde_json::Value) -> Result<()>;
-}
-
-pub async fn migrate_all(settings_dir: &Path) -> Result<()> {
+pub async fn migrate_all(cache_dir: &Path, settings_dir: &Path) -> Result<()> {
     #[cfg(windows)]
     windows::migrate_after_windows_update(settings_dir)
         .await
@@ -64,27 +68,13 @@ pub async fn migrate_all(settings_dir: &Path) -> Result<()> {
         return Err(Error::NoMatchingVersion);
     }
 
-    {
-        let settings: Settings =
-            serde_json::from_slice(&settings_bytes[..]).map_err(Error::ParseError)?;
-        if settings.get_settings_version() == CURRENT_SETTINGS_VERSION {
-            return Ok(());
-        }
-    }
-
-    let migrations: Vec<Box<dyn SettingsMigration>> = vec![
-        Box::new(v1::Migration),
-        Box::new(v2::Migration),
-        Box::new(v3::Migration),
-        Box::new(v4::Migration),
-    ];
+    v1::migrate(&mut settings)?;
+    v2::migrate(&mut settings)?;
+    v3::migrate(&mut settings)?;
+    v4::migrate(&mut settings)?;
 
-    for migration in &migrations {
-        if !migration.version_matches(&mut settings) {
-            continue;
-        }
-        migration.migrate(&mut settings)?;
-    }
+    account_history::migrate_location(cache_dir, settings_dir).await;
+    account_history::migrate_formats(settings_dir, &mut settings).await?;
 
     let buffer = serde_json::to_string_pretty(&settings).map_err(Error::SerializeError)?;
 
@@ -103,7 +93,6 @@ pub async fn migrate_all(settings_dir: &Path) -> Result<()> {
     file.write_all(&buffer.into_bytes())
         .await
         .map_err(Error::WriteError)?;
-
     Ok(())
 }
 
diff --git a/mullvad-daemon/src/migrations/v1.rs b/mullvad-daemon/src/migrations/v1.rs
index a7c8e51738..d04812cd00 100644
--- a/mullvad-daemon/src/migrations/v1.rs
+++ b/mullvad-daemon/src/migrations/v1.rs
@@ -3,65 +3,65 @@ use mullvad_types::{relay_constraints::Constraint, settings::SettingsVersion};
 use talpid_types::net::TunnelType;
 
 
-pub(super) struct Migration;
-
-impl super::SettingsMigration for Migration {
-    fn version_matches(&self, settings: &mut serde_json::Value) -> bool {
-        settings.get("settings_version").is_none()
+pub fn migrate(settings: &mut serde_json::Value) -> Result<()> {
+    if !version_matches(settings) {
+        return Ok(());
     }
 
-    fn migrate(&self, settings: &mut serde_json::Value) -> Result<()> {
-        log::info!("Migrating settings format to V2");
+    log::info!("Migrating settings format to V2");
 
-        let openvpn_constraints = || -> Option<serde_json::Value> {
-            settings
-                .get("relay_settings")?
-                .get("normal")?
-                .get("tunnel")?
-                .get("only")?
-                .get("openvpn")
-                .cloned()
-        }();
-        let wireguard_constraints = || -> Option<serde_json::Value> {
-            settings
-                .get("relay_settings")?
-                .get("normal")?
-                .get("tunnel")?
-                .get("only")?
-                .get("wireguard")
-                .cloned()
-        }();
+    let openvpn_constraints = || -> Option<serde_json::Value> {
+        settings
+            .get("relay_settings")?
+            .get("normal")?
+            .get("tunnel")?
+            .get("only")?
+            .get("openvpn")
+            .cloned()
+    }();
+    let wireguard_constraints = || -> Option<serde_json::Value> {
+        settings
+            .get("relay_settings")?
+            .get("normal")?
+            .get("tunnel")?
+            .get("only")?
+            .get("wireguard")
+            .cloned()
+    }();
 
-        if let Some(relay_settings) = settings.get_mut("relay_settings") {
-            if let Some(normal_settings) = relay_settings.get_mut("normal") {
-                if let Some(openvpn_constraints) = openvpn_constraints {
-                    normal_settings["openvpn_constraints"] = openvpn_constraints;
-                    normal_settings["tunnel_protocol"] =
-                        serde_json::json!(Constraint::<TunnelType>::Any);
-                } else if let Some(wireguard_constraints) = wireguard_constraints {
-                    normal_settings["wireguard_constraints"] = wireguard_constraints;
-                    normal_settings["tunnel_protocol"] =
-                        serde_json::json!(Constraint::Only(TunnelType::Wireguard));
-                } else {
-                    normal_settings["tunnel_protocol"] =
-                        serde_json::json!(Constraint::<TunnelType>::Any);
-                }
-                if let Some(object) = normal_settings.as_object_mut() {
-                    object.remove("tunnel");
-                }
+    if let Some(relay_settings) = settings.get_mut("relay_settings") {
+        if let Some(normal_settings) = relay_settings.get_mut("normal") {
+            if let Some(openvpn_constraints) = openvpn_constraints {
+                normal_settings["openvpn_constraints"] = openvpn_constraints;
+                normal_settings["tunnel_protocol"] =
+                    serde_json::json!(Constraint::<TunnelType>::Any);
+            } else if let Some(wireguard_constraints) = wireguard_constraints {
+                normal_settings["wireguard_constraints"] = wireguard_constraints;
+                normal_settings["tunnel_protocol"] =
+                    serde_json::json!(Constraint::Only(TunnelType::Wireguard));
+            } else {
+                normal_settings["tunnel_protocol"] =
+                    serde_json::json!(Constraint::<TunnelType>::Any);
+            }
+            if let Some(object) = normal_settings.as_object_mut() {
+                object.remove("tunnel");
             }
         }
+    }
 
-        settings["show_beta_releases"] = serde_json::json!(false);
-        settings["settings_version"] = serde_json::json!(SettingsVersion::V2);
+    settings["show_beta_releases"] = serde_json::json!(false);
+    settings["settings_version"] = serde_json::json!(SettingsVersion::V2);
 
-        Ok(())
-    }
+    Ok(())
+}
+
+fn version_matches(settings: &mut serde_json::Value) -> bool {
+    settings.get("settings_version").is_none()
 }
 
 #[cfg(test)]
 mod test {
-    use super::{super::SettingsMigration, Migration};
+    use super::{migrate, version_matches};
     use serde_json;
 
     pub const V2_SETTINGS: &str = r#"
@@ -191,10 +191,9 @@ mod test {
     fn test_v1_migration() {
         let mut old_settings = serde_json::from_str(V1_SETTINGS).unwrap();
 
-        let migration = Migration;
-        assert!(migration.version_matches(&mut old_settings));
+        assert!(version_matches(&mut old_settings));
 
-        migration.migrate(&mut old_settings).unwrap();
+        migrate(&mut old_settings).unwrap();
         let new_settings: serde_json::Value = serde_json::from_str(V2_SETTINGS).unwrap();
 
         assert_eq!(&old_settings, &new_settings);
@@ -204,10 +203,9 @@ mod test {
     fn test_v1_2019v3_migration() {
         let mut old_settings = serde_json::from_str(V1_SETTINGS_2019V3).unwrap();
 
-        let migration = Migration;
-        assert!(migration.version_matches(&mut old_settings));
+        assert!(version_matches(&mut old_settings));
 
-        migration.migrate(&mut old_settings).unwrap();
+        migrate(&mut old_settings).unwrap();
         let new_settings: serde_json::Value = serde_json::from_str(V2_SETTINGS).unwrap();
 
         assert_eq!(&old_settings, &new_settings);
diff --git a/mullvad-daemon/src/migrations/v2.rs b/mullvad-daemon/src/migrations/v2.rs
index 1ccf247212..5b0900ca13 100644
--- a/mullvad-daemon/src/migrations/v2.rs
+++ b/mullvad-daemon/src/migrations/v2.rs
@@ -4,69 +4,68 @@ use mullvad_types::settings::SettingsVersion;
 use std::time::Duration;
 
 
-pub(super) struct Migration;
+pub fn migrate(settings: &mut serde_json::Value) -> Result<()> {
+    if !version_matches(settings) {
+        return Ok(());
+    }
+
+    log::info!("Migrating settings format to V3");
 
-impl super::SettingsMigration for Migration {
-    fn version_matches(&self, settings: &mut serde_json::Value) -> bool {
+    // `show_beta_releases` used to be nullable
+    if settings
+        .get_mut("show_beta_releases")
+        .map(|val| val.is_null())
+        .unwrap_or(false)
+    {
         settings
-            .get("settings_version")
-            .map(|version| version == SettingsVersion::V2 as u64)
-            .unwrap_or(false)
+            .as_object_mut()
+            .ok_or(Error::NoMatchingVersion)?
+            .remove("show_beta_releases");
     }
 
-    fn migrate(&self, settings: &mut serde_json::Value) -> Result<()> {
-        log::info!("Migrating settings format to V3");
-
-        // `show_beta_releases` used to be nullable
-        if settings
-            .get_mut("show_beta_releases")
-            .map(|val| val.is_null())
-            .unwrap_or(false)
-        {
-            settings
-                .as_object_mut()
-                .ok_or(Error::NoMatchingVersion)?
-                .remove("show_beta_releases");
-        }
+    let automatic_rotation = || -> Option<u64> {
+        settings
+            .get("tunnel_options")?
+            .get("wireguard")?
+            .get("automatic_rotation")
+            .map(|ivl| ivl.as_u64())?
+    }();
 
-        let automatic_rotation = || -> Option<u64> {
-            settings
-                .get("tunnel_options")?
-                .get("wireguard")?
-                .get("automatic_rotation")
-                .map(|ivl| ivl.as_u64())?
-        }();
+    if let Some(interval) = automatic_rotation {
+        let new_ivl = match Duration::from_secs(60 * 60 * interval) {
+            ivl if ivl < MIN_ROTATION_INTERVAL => {
+                log::warn!("Increasing key rotation interval since it is below minimum");
+                MIN_ROTATION_INTERVAL
+            }
+            ivl if ivl > MAX_ROTATION_INTERVAL => {
+                log::warn!("Decreasing key rotation interval since it is above maximum");
+                MAX_ROTATION_INTERVAL
+            }
+            ivl => ivl,
+        };
 
-        if let Some(interval) = automatic_rotation {
-            let new_ivl = match Duration::from_secs(60 * 60 * interval) {
-                ivl if ivl < MIN_ROTATION_INTERVAL => {
-                    log::warn!("Increasing key rotation interval since it is below minimum");
-                    MIN_ROTATION_INTERVAL
-                }
-                ivl if ivl > MAX_ROTATION_INTERVAL => {
-                    log::warn!("Decreasing key rotation interval since it is above maximum");
-                    MAX_ROTATION_INTERVAL
-                }
-                ivl => ivl,
-            };
+        settings["tunnel_options"]["wireguard"]["rotation_interval"] = serde_json::json!(new_ivl);
+        settings["tunnel_options"]["wireguard"]
+            .as_object_mut()
+            .ok_or(Error::NoMatchingVersion)?
+            .remove("automatic_rotation");
+    }
 
-            settings["tunnel_options"]["wireguard"]["rotation_interval"] =
-                serde_json::json!(new_ivl);
-            settings["tunnel_options"]["wireguard"]
-                .as_object_mut()
-                .ok_or(Error::NoMatchingVersion)?
-                .remove("automatic_rotation");
-        }
+    settings["settings_version"] = serde_json::json!(SettingsVersion::V3);
 
-        settings["settings_version"] = serde_json::json!(SettingsVersion::V3);
+    Ok(())
+}
 
-        Ok(())
-    }
+fn version_matches(settings: &mut serde_json::Value) -> bool {
+    settings
+        .get("settings_version")
+        .map(|version| version == SettingsVersion::V2 as u64)
+        .unwrap_or(false)
 }
 
 #[cfg(test)]
 mod test {
-    use super::{super::SettingsMigration, Migration};
+    use super::{migrate, version_matches};
     use serde_json;
 
     const V2_SETTINGS: &str = r#"
@@ -176,10 +175,9 @@ mod test {
     fn test_v2_migration() {
         let mut old_settings = serde_json::from_str(V2_SETTINGS).unwrap();
 
-        let migration = Migration;
-        assert!(migration.version_matches(&mut old_settings));
+        assert!(version_matches(&mut old_settings));
 
-        migration.migrate(&mut old_settings).unwrap();
+        migrate(&mut old_settings).unwrap();
         let new_settings: serde_json::Value = serde_json::from_str(V3_SETTINGS).unwrap();
 
         assert_eq!(&old_settings, &new_settings);
diff --git a/mullvad-daemon/src/migrations/v3.rs b/mullvad-daemon/src/migrations/v3.rs
index 042fd79dcd..76b3522a96 100644
--- a/mullvad-daemon/src/migrations/v3.rs
+++ b/mullvad-daemon/src/migrations/v3.rs
@@ -4,57 +4,56 @@ use mullvad_types::settings::{
 };
 
 
-pub(super) struct Migration;
-
-impl super::SettingsMigration for Migration {
-    fn version_matches(&self, settings: &mut serde_json::Value) -> bool {
-        settings
-            .get("settings_version")
-            .map(|version| version == SettingsVersion::V3 as u64)
-            .unwrap_or(false)
+pub fn migrate(settings: &mut serde_json::Value) -> Result<()> {
+    if !version_matches(settings) {
+        return Ok(());
     }
 
-    fn migrate(&self, settings: &mut serde_json::Value) -> Result<()> {
-        log::info!("Migrating settings format to V4");
+    log::info!("Migrating settings format to V4");
 
-        let dns_options = || -> Option<&serde_json::Value> {
-            settings.get("tunnel_options")?.get("dns_options")
-        }();
+    let dns_options =
+        || -> Option<&serde_json::Value> { settings.get("tunnel_options")?.get("dns_options") }();
 
-        if let Some(options) = dns_options {
-            if options.get("state").is_none() {
-                let new_state = if options
-                    .get("custom")
-                    .map(|custom| custom.as_bool().unwrap_or(false))
-                    .unwrap_or(false)
-                {
-                    DnsState::Custom
-                } else {
-                    DnsState::Default
-                };
-                let addresses = if let Some(addrs) = options.get("addresses") {
-                    serde_json::from_value(addrs.clone()).map_err(Error::ParseError)?
-                } else {
-                    vec![]
-                };
+    if let Some(options) = dns_options {
+        if options.get("state").is_none() {
+            let new_state = if options
+                .get("custom")
+                .map(|custom| custom.as_bool().unwrap_or(false))
+                .unwrap_or(false)
+            {
+                DnsState::Custom
+            } else {
+                DnsState::Default
+            };
+            let addresses = if let Some(addrs) = options.get("addresses") {
+                serde_json::from_value(addrs.clone()).map_err(Error::ParseError)?
+            } else {
+                vec![]
+            };
 
-                settings["tunnel_options"]["dns_options"] = serde_json::json!(DnsOptions {
-                    state: new_state,
-                    default_options: DefaultDnsOptions::default(),
-                    custom_options: CustomDnsOptions { addresses },
-                });
-            }
+            settings["tunnel_options"]["dns_options"] = serde_json::json!(DnsOptions {
+                state: new_state,
+                default_options: DefaultDnsOptions::default(),
+                custom_options: CustomDnsOptions { addresses },
+            });
         }
+    }
 
-        settings["settings_version"] = serde_json::json!(SettingsVersion::V4);
+    settings["settings_version"] = serde_json::json!(SettingsVersion::V4);
 
-        Ok(())
-    }
+    Ok(())
+}
+
+fn version_matches(settings: &mut serde_json::Value) -> bool {
+    settings
+        .get("settings_version")
+        .map(|version| version == SettingsVersion::V3 as u64)
+        .unwrap_or(false)
 }
 
 #[cfg(test)]
 mod test {
-    use super::{super::SettingsMigration, Migration};
+    use super::{migrate, version_matches};
     use serde_json;
 
     pub const V3_SETTINGS: &str = r#"
@@ -186,10 +185,9 @@ mod test {
     fn test_v3_migration() {
         let mut old_settings = serde_json::from_str(V3_SETTINGS).unwrap();
 
-        let migration = Migration;
-        assert!(migration.version_matches(&mut old_settings));
+        assert!(version_matches(&mut old_settings));
 
-        migration.migrate(&mut old_settings).unwrap();
+        migrate(&mut old_settings).unwrap();
         let new_settings: serde_json::Value = serde_json::from_str(V4_SETTINGS).unwrap();
 
         assert_eq!(&old_settings, &new_settings);
diff --git a/mullvad-daemon/src/migrations/v4.rs b/mullvad-daemon/src/migrations/v4.rs
index 7a459ce516..76b88ce987 100644
--- a/mullvad-daemon/src/migrations/v4.rs
+++ b/mullvad-daemon/src/migrations/v4.rs
@@ -10,30 +10,23 @@ const WIREGUARD_TCP_PORTS: [u16; 3] = [80, 443, 5001];
 const OPENVPN_TCP_PORTS: [u16; 2] = [80, 443];
 
 
-pub(super) struct Migration;
-
-impl super::SettingsMigration for Migration {
-    fn version_matches(&self, settings: &mut serde_json::Value) -> bool {
-        settings
-            .get("settings_version")
-            .map(|version| version == SettingsVersion::V4 as u64)
-            .unwrap_or(false)
+pub fn migrate(settings: &mut serde_json::Value) -> Result<()> {
+    if !version_matches(settings) {
+        return Ok(());
     }
 
-    fn migrate(&self, settings: &mut serde_json::Value) -> Result<()> {
-        log::info!("Migrating settings format to V5");
+    log::info!("Migrating settings format to V5");
 
-        let wireguard_constraints = || -> Option<&serde_json::Value> {
-            settings
-                .get("relay_settings")?
-                .get("normal")?
-                .get("wireguard_constraints")
-        }();
+    let wireguard_constraints = || -> Option<&serde_json::Value> {
+        settings
+            .get("relay_settings")?
+            .get("normal")?
+            .get("wireguard_constraints")
+    }();
 
-        if let Some(constraints) = wireguard_constraints {
-            let (port, protocol): (Constraint<u16>, TransportProtocol) = if let Some(port) =
-                constraints.get("port")
-            {
+    if let Some(constraints) = wireguard_constraints {
+        let (port, protocol): (Constraint<u16>, TransportProtocol) =
+            if let Some(port) = constraints.get("port") {
                 let port_constraint =
                     serde_json::from_value(port.clone()).map_err(Error::ParseError)?;
                 match port_constraint {
@@ -44,64 +37,70 @@ impl super::SettingsMigration for Migration {
                 (Constraint::Any, TransportProtocol::Udp)
             };
 
-            settings["relay_settings"]["normal"]["wireguard_constraints"]["port"] = match port {
-                Constraint::Any => {
-                    serde_json::json!(Constraint::<TransportPort>::Any)
-                }
-                Constraint::Only(_) => {
-                    serde_json::json!(Constraint::Only(TransportPort { protocol, port }))
-                }
-            };
+        settings["relay_settings"]["normal"]["wireguard_constraints"]["port"] = match port {
+            Constraint::Any => {
+                serde_json::json!(Constraint::<TransportPort>::Any)
+            }
+            Constraint::Only(_) => {
+                serde_json::json!(Constraint::Only(TransportPort { protocol, port }))
+            }
+        };
 
-            settings["relay_settings"]["normal"]["wireguard_constraints"]
-                .as_object_mut()
-                .ok_or(Error::NoMatchingVersion)?
-                .remove("protocol");
-        }
+        settings["relay_settings"]["normal"]["wireguard_constraints"]
+            .as_object_mut()
+            .ok_or(Error::NoMatchingVersion)?
+            .remove("protocol");
+    }
 
-        let openvpn_constraints = || -> Option<&serde_json::Value> {
-            settings
-                .get("relay_settings")?
-                .get("normal")?
-                .get("openvpn_constraints")
-        }();
+    let openvpn_constraints = || -> Option<&serde_json::Value> {
+        settings
+            .get("relay_settings")?
+            .get("normal")?
+            .get("openvpn_constraints")
+    }();
 
-        if let Some(constraints) = openvpn_constraints {
-            let port: Constraint<u16> = if let Some(port) = constraints.get("port") {
-                serde_json::from_value(port.clone()).map_err(Error::ParseError)?
+    if let Some(constraints) = openvpn_constraints {
+        let port: Constraint<u16> = if let Some(port) = constraints.get("port") {
+            serde_json::from_value(port.clone()).map_err(Error::ParseError)?
+        } else {
+            Constraint::Any
+        };
+        let transport_constraint: Constraint<TransportProtocol> =
+            if let Some(protocol) = constraints.get("protocol") {
+                serde_json::from_value(protocol.clone()).map_err(Error::ParseError)?
             } else {
                 Constraint::Any
             };
-            let transport_constraint: Constraint<TransportProtocol> =
-                if let Some(protocol) = constraints.get("protocol") {
-                    serde_json::from_value(protocol.clone()).map_err(Error::ParseError)?
-                } else {
-                    Constraint::Any
-                };
 
-            let port = match (port, transport_constraint) {
-                (Constraint::Only(port), Constraint::Any) => Constraint::Only(TransportPort {
-                    protocol: openvpn_protocol_from_port(port),
-                    port: Constraint::Only(port),
-                }),
-                (port, Constraint::Only(protocol)) => {
-                    Constraint::Only(TransportPort { protocol, port })
-                }
-                (Constraint::Any, Constraint::Any) => Constraint::Any,
-            };
+        let port = match (port, transport_constraint) {
+            (Constraint::Only(port), Constraint::Any) => Constraint::Only(TransportPort {
+                protocol: openvpn_protocol_from_port(port),
+                port: Constraint::Only(port),
+            }),
+            (port, Constraint::Only(protocol)) => {
+                Constraint::Only(TransportPort { protocol, port })
+            }
+            (Constraint::Any, Constraint::Any) => Constraint::Any,
+        };
 
-            settings["relay_settings"]["normal"]["openvpn_constraints"]["port"] =
-                serde_json::json!(port);
-            settings["relay_settings"]["normal"]["openvpn_constraints"]
-                .as_object_mut()
-                .ok_or(Error::NoMatchingVersion)?
-                .remove("protocol");
-        }
+        settings["relay_settings"]["normal"]["openvpn_constraints"]["port"] =
+            serde_json::json!(port);
+        settings["relay_settings"]["normal"]["openvpn_constraints"]
+            .as_object_mut()
+            .ok_or(Error::NoMatchingVersion)?
+            .remove("protocol");
+    }
 
-        settings["settings_version"] = serde_json::json!(SettingsVersion::V5);
+    settings["settings_version"] = serde_json::json!(SettingsVersion::V5);
 
-        Ok(())
-    }
+    Ok(())
+}
+
+fn version_matches(settings: &mut serde_json::Value) -> bool {
+    settings
+        .get("settings_version")
+        .map(|version| version == SettingsVersion::V4 as u64)
+        .unwrap_or(false)
 }
 
 fn openvpn_protocol_from_port(port: u16) -> TransportProtocol {
@@ -124,7 +123,7 @@ fn wg_protocol_from_port(port: u16) -> TransportProtocol {
 
 #[cfg(test)]
 mod test {
-    use super::{super::SettingsMigration, Migration};
+    use super::{migrate, version_matches};
     use serde_json;
 
     pub const V4_SETTINGS: &str = r#"
@@ -272,10 +271,9 @@ mod test {
     fn test_v4_migration() {
         let mut old_settings = serde_json::from_str(V4_SETTINGS).unwrap();
 
-        let migration = Migration;
-        assert!(migration.version_matches(&mut old_settings));
+        assert!(version_matches(&mut old_settings));
 
-        migration.migrate(&mut old_settings).unwrap();
+        migrate(&mut old_settings).unwrap();
         let new_settings: serde_json::Value = serde_json::from_str(V5_SETTINGS).unwrap();
 
         assert_eq!(&old_settings, &new_settings);