Merge branch 'settings-migration-fails-on-main-des-1809'

6 files changed, 230 insertions, 98 deletions

diff --git a/mullvad-daemon/src/migrations/mod.rs b/mullvad-daemon/src/migrations/mod.rs
index eaf3350e0c..14fb4fb4fe 100644
--- a/mullvad-daemon/src/migrations/mod.rs
+++ b/mullvad-daemon/src/migrations/mod.rs
@@ -46,6 +46,7 @@ use tokio::{
 mod account_history;
 mod device;
 mod v1;
+mod v10;
 mod v2;
 mod v3;
 mod v4;
@@ -207,6 +208,8 @@ async fn migrate_settings(
         }),
     )?;
 
+    v10::migrate(settings)?;
+
     Ok(migration_data)
 }
 
@@ -439,7 +442,7 @@ mod windows {
 
 #[cfg(test)]
 mod test {
-    use mullvad_types::settings::Settings;
+    use mullvad_types::settings::{Settings, CURRENT_SETTINGS_VERSION};
 
     use crate::migrations::migrate_settings;
 
@@ -456,4 +459,14 @@ mod test {
 
         assert_eq!(default_settings, migrated_settings);
     }
+
+    /// Ensure that the settings version is correct after running all migration code
+    #[tokio::test]
+    async fn test_all_migrations() {
+        const V1_SETTINGS: &str = include_str!("v1_settings.json");
+        let mut settings = serde_json::from_str(V1_SETTINGS).unwrap();
+        migrate_settings(None, &mut settings).await.unwrap();
+        let deserialized: Settings = serde_json::from_value(settings).unwrap();
+        assert_eq!(deserialized.settings_version, CURRENT_SETTINGS_VERSION);
+    }
 }
diff --git a/mullvad-daemon/src/migrations/v1.rs b/mullvad-daemon/src/migrations/v1.rs
index b20f96488b..44de9d1293 100644
--- a/mullvad-daemon/src/migrations/v1.rs
+++ b/mullvad-daemon/src/migrations/v1.rs
@@ -118,46 +118,7 @@ mod test {
 }
 "#;
 
-    const V1_SETTINGS: &str = r#"
-{
-  "account_token": "1234",
-  "relay_settings": {
-    "normal": {
-      "location": {
-        "only": {
-          "country": "se"
-        }
-      },
-      "tunnel": {
-        "only": {
-          "openvpn": {
-            "port": {
-              "only": 53
-            },
-            "protocol": {
-              "only": "udp"
-            }
-          }
-        }
-      }
-    }
-  },
-  "allow_lan": true,
-  "block_when_disconnected": false,
-  "auto_connect": false,
-  "tunnel_options": {
-    "openvpn": {
-      "mssfix": null
-    },
-    "wireguard": {
-      "mtu": null
-    },
-    "generic": {
-      "enable_ipv6": false
-    }
-  }
-}
-"#;
+    const V1_SETTINGS: &str = include_str!("v1_settings.json");
 
     const V1_SETTINGS_2019V3: &str = r#"
 {
diff --git a/mullvad-daemon/src/migrations/v10.rs b/mullvad-daemon/src/migrations/v10.rs
new file mode 100644
index 0000000000..2b713b97ab
--- /dev/null
+++ b/mullvad-daemon/src/migrations/v10.rs
@@ -0,0 +1,171 @@
+use super::{Error, Result};
+use mullvad_types::settings::SettingsVersion;
+use talpid_types::net::TunnelType;
+
+/// Automatic tunnel protocol has been removed. If the tunnel protocol is set to `any`, it will be
+/// migrated to `wireguard`, unless the location is an openvpn relay, in which case it will be
+/// migrated to `openvpn`.
+pub fn migrate(settings: &mut serde_json::Value) -> Result<()> {
+    if !(version(settings) == Some(SettingsVersion::V10)) {
+        return Ok(());
+    }
+
+    log::info!("Migrating settings format to v11");
+
+    migrate_tunnel_type(settings)?;
+
+    settings["settings_version"] = serde_json::json!(SettingsVersion::V11);
+
+    Ok(())
+}
+
+fn version(settings: &serde_json::Value) -> Option<SettingsVersion> {
+    settings
+        .get("settings_version")
+        .and_then(|version| serde_json::from_value(version.clone()).ok())
+}
+
+fn relay_settings(settings: &mut serde_json::Value) -> Option<&mut serde_json::Value> {
+    settings.get_mut("relay_settings")?.get_mut("normal")
+}
+
+fn migrate_tunnel_type(settings: &mut serde_json::Value) -> Result<()> {
+    if let Some(ref mut normal) = relay_settings(settings) {
+        migrate_tunnel_type_inner(normal)?;
+    }
+    Ok(())
+}
+
+fn migrate_tunnel_type_inner(normal: &mut serde_json::Value) -> Result<()> {
+    match normal.get_mut("tunnel_protocol") {
+        // Migrate tunnel protocol "any"
+        Some(serde_json::Value::String(s)) if s == "any" => {
+            // If openvpn is selected, migrate to openvpn tunnel type
+            // Otherwise, select wireguard
+            let hostname = normal
+                .get_mut("location")
+                .and_then(|location| location.get_mut("only"))
+                .and_then(|only| only.get_mut("location"))
+                .and_then(|only| only.get_mut("hostname").cloned());
+
+            let protocol = if let Some(serde_json::Value::String(s)) = hostname {
+                if s.split('-').any(|token| token == "ovpn") {
+                    TunnelType::OpenVpn
+                } else {
+                    TunnelType::Wireguard
+                }
+            } else {
+                TunnelType::Wireguard
+            };
+
+            normal["tunnel_protocol"] = serde_json::json!(protocol);
+        }
+        // Migrate '"only": { "tunnel_protocol": $tunnel_protocol }'
+        // to '"tunnel_protocol": $tunnel_protocol'
+        Some(serde_json::Value::Object(ref mut constraint)) => {
+            if let Some(tunnel_type) = constraint.get("only") {
+                let tunnel_type: TunnelType = serde_json::from_value(tunnel_type.clone())
+                    .map_err(|_| Error::InvalidSettingsContent)?;
+                normal["tunnel_protocol"] = serde_json::json!(tunnel_type);
+            } else {
+                return Err(Error::InvalidSettingsContent);
+            }
+        }
+        Some(_) => {
+            return Err(Error::InvalidSettingsContent);
+        }
+        // Unexpected result. Do nothing.
+        None => (),
+    };
+    Ok(())
+}
+#[cfg(test)]
+mod test {
+    use serde_json::json;
+
+    use crate::migrations::v10::migrate_tunnel_type_inner;
+
+    /// Tunnel protocol "any" is migrated to wireguard
+    #[test]
+    fn test_v10_to_v11_migration() {
+        let mut old_settings = json!({
+            "tunnel_protocol": "any",
+            "location": {
+                "only": {
+                    "location": {
+                        "country": "se"
+                    }
+                }
+            }
+        });
+        migrate_tunnel_type_inner(&mut old_settings).unwrap();
+        let new_settings: serde_json::Value = json!({
+            "tunnel_protocol": "wireguard",
+            "location": {
+                "only": {
+                    "location": {
+                        "country": "se"
+                    }
+                }
+            }
+        });
+        assert_eq!(&old_settings, &new_settings);
+    }
+
+    /// Tunnel protocol "any" is migrated to openvpn, since the location is an openvpn relay
+    #[test]
+    fn test_v10_to_v11_migration_openvpn_location() {
+        let mut old_settings = json!({
+            "tunnel_protocol": "any",
+            "location": {
+                "only": {
+                    "location": {
+                        "hostname": "at-vie-ovpn-001"
+                    }
+                }
+            }
+        });
+        migrate_tunnel_type_inner(&mut old_settings).unwrap();
+        let new_settings: serde_json::Value = json!({
+            "tunnel_protocol": "openvpn",
+            "location": {
+                "only": {
+                    "location": {
+                        "hostname": "at-vie-ovpn-001"
+                    }
+                }
+            }
+        });
+        assert_eq!(&old_settings, &new_settings);
+    }
+
+    /// Tunnel protocol '"only" = { "tunnel_protocol" = '$tunnel_protocol' } is migrated to
+    /// '"tunnel_protocol" = '$tunnel_protocol'
+    #[test]
+    fn test_v10_to_v11_migration_only_protocol() {
+        let mut old_settings = json!({
+            "tunnel_protocol": {
+                "only": "wireguard"
+            },
+            "location": {
+                "only": {
+                    "location": {
+                        "country": "se"
+                    }
+                }
+            }
+        });
+        migrate_tunnel_type_inner(&mut old_settings).unwrap();
+        let new_settings: serde_json::Value = json!({
+            "tunnel_protocol": "wireguard",
+            "location": {
+                "only": {
+                    "location": {
+                        "country": "se"
+                    }
+                }
+            }
+        });
+        assert_eq!(&old_settings, &new_settings);
+    }
+}
diff --git a/mullvad-daemon/src/migrations/v1_settings.json b/mullvad-daemon/src/migrations/v1_settings.json
new file mode 100644
index 0000000000..f1c689372a
--- /dev/null
+++ b/mullvad-daemon/src/migrations/v1_settings.json
@@ -0,0 +1,38 @@
+{
+    "account_token": "1234",
+    "relay_settings": {
+      "normal": {
+        "location": {
+          "only": {
+            "country": "se"
+          }
+        },
+        "tunnel": {
+          "only": {
+            "openvpn": {
+              "port": {
+                "only": 53
+              },
+              "protocol": {
+                "only": "udp"
+              }
+            }
+          }
+        }
+      }
+    },
+    "allow_lan": true,
+    "block_when_disconnected": false,
+    "auto_connect": false,
+    "tunnel_options": {
+      "openvpn": {
+        "mssfix": null
+      },
+      "wireguard": {
+        "mtu": null
+      },
+      "generic": {
+        "enable_ipv6": false
+      }
+    }
+}
\ No newline at end of file
diff --git a/mullvad-daemon/src/migrations/v9.rs b/mullvad-daemon/src/migrations/v9.rs
index 36ac89ed6d..91aac7e53b 100644
--- a/mullvad-daemon/src/migrations/v9.rs
+++ b/mullvad-daemon/src/migrations/v9.rs
@@ -84,64 +84,11 @@ pub fn migrate(
         }
     }
 
-    migrate_tunnel_type(settings)?;
+    json_blob["settings_version"] = serde_json::json!(SettingsVersion::V10);
 
-    // TODO: Uncomment this when closing the migration:
-    // json_blob["settings_version"] = serde_json::json!(SettingsVersion::V10);
-
-    Ok(())
-}
-
-fn migrate_tunnel_type(settings: &mut serde_json::Value) -> Result<()> {
-    let Some(ref mut normal) = relay_settings(settings) else {
-        return Ok(());
-    };
-    match normal.get_mut("tunnel_protocol") {
-        // Already migrated
-        Some(serde_json::Value::String(s)) if s == "any" => {
-            // If openvpn is selected, migrate to openvpn tunnel type
-            // Otherwise, select wireguard
-            let hostname = normal
-                .get_mut("location")
-                .and_then(|location| location.get_mut("only"))
-                .and_then(|only| only.get_mut("location"))
-                .and_then(|only| only.get_mut("hostname").cloned());
-
-            let protocol = if let Some(serde_json::Value::String(s)) = hostname {
-                if s.split('-').any(|token| token == "ovpn") {
-                    TunnelType::OpenVpn
-                } else {
-                    TunnelType::Wireguard
-                }
-            } else {
-                TunnelType::Wireguard
-            };
-
-            normal["tunnel_protocol"] = serde_json::json!(protocol);
-        }
-        // Migrate
-        Some(serde_json::Value::Object(ref mut constraint)) => {
-            if let Some(tunnel_type) = constraint.get("only") {
-                let tunnel_type: TunnelType = serde_json::from_value(tunnel_type.clone())
-                    .map_err(|_| Error::InvalidSettingsContent)?;
-                normal["tunnel_protocol"] = serde_json::json!(tunnel_type);
-            } else {
-                return Err(Error::InvalidSettingsContent);
-            }
-        }
-        Some(_) => {
-            return Err(Error::InvalidSettingsContent);
-        }
-        // Unexpected result. Do nothing.
-        None => (),
-    }
     Ok(())
 }
 
-fn relay_settings(settings: &mut serde_json::Value) -> Option<&mut serde_json::Value> {
-    settings.get_mut("relay_settings")?.get_mut("normal")
-}
-
 fn version_matches(settings: &serde_json::Value) -> bool {
     settings
         .get("settings_version")
@@ -644,7 +591,7 @@ mod test {
       },
       "providers": "any",
       "ownership": "any",
-      "tunnel_protocol": "openvpn",
+      "tunnel_protocol": "any",
       "wireguard_constraints": {
         "port": "any",
         "ip_version": "any",
@@ -732,7 +679,7 @@ mod test {
   },
   "relay_overrides": [],
   "show_beta_releases": true,
-  "settings_version": 9
+  "settings_version": 10
 }
 "#;
 }
diff --git a/mullvad-types/src/settings/mod.rs b/mullvad-types/src/settings/mod.rs
index e65c21c148..432e95391b 100644
--- a/mullvad-types/src/settings/mod.rs
+++ b/mullvad-types/src/settings/mod.rs
@@ -20,7 +20,7 @@ mod dns;
 /// latest version that exists in `SettingsVersion`.
 /// This should be bumped when a new version is introduced along with a migration
 /// being added to `mullvad-daemon`.
-pub const CURRENT_SETTINGS_VERSION: SettingsVersion = SettingsVersion::V10;
+pub const CURRENT_SETTINGS_VERSION: SettingsVersion = SettingsVersion::V11;
 
 #[derive(Debug, PartialEq, Eq, PartialOrd, Clone, Copy)]
 #[repr(u32)]
@@ -34,6 +34,7 @@ pub enum SettingsVersion {
     V8 = 8,
     V9 = 9,
     V10 = 10,
+    V11 = 11,
 }
 
 impl<'de> Deserialize<'de> for SettingsVersion {
@@ -51,6 +52,7 @@ impl<'de> Deserialize<'de> for SettingsVersion {
             v if v == SettingsVersion::V8 as u32 => Ok(SettingsVersion::V8),
             v if v == SettingsVersion::V9 as u32 => Ok(SettingsVersion::V9),
             v if v == SettingsVersion::V10 as u32 => Ok(SettingsVersion::V10),
+            v if v == SettingsVersion::V11 as u32 => Ok(SettingsVersion::V11),
             v => Err(serde::de::Error::custom(format!(
                 "{v} is not a valid SettingsVersion"
             ))),