fixup! Enforce that built-in access methods always exist

mullvad-daemon/src/migrations/v7.rs

@@ -145,18 +145,26 @@ fn migrate_api_access_settings(settings: &mut serde_json::Value) -> Result<()> {
         }
     }
 
-    // Step 1. Collect all of the built-in methods from { "api_access_methods": { "access_method_settings": [ .. ] } }.
-    // Step 2. Remove all of the built-in methods from { "api_access_methods": { "access_method_settings": [ .. ] } }.
-    // Step 3. Add the collected built-in methods from step 2 to { "api_access_methods": { .. } } under some appropriate key.
+    // Step 1. Rename { "api_access_methods": { "access_method_settings": .. } } to { "api_access_methods": { "user_defined": .. } }.
+    // Step 2. Collect all of the built-in methods from { "api_access_methods": { "user_defined": [ .. ] } }.
+    // Step 3. Remove all of the built-in methods from { "api_access_methods": { "user_defined": [ .. ] } }.
+    // Step 4. Add the collected built-in methods from step 2 to { "api_access_methods": { .. } } under some appropriate key.
     if let Some(access_method_settings) = settings
         .get_mut("api_access_methods")
         .and_then(serde_json::value::Value::as_object_mut)
     {
+        // Step 1.
+        rename_map_field(
+            access_method_settings,
+            "access_method_settings",
+            "user_defined",
+        )?;
+
         if let Some(access_method_settings_list) = access_method_settings
-            .get_mut("access_method_settings")
+            .get_mut("user_defined")
             .and_then(serde_json::value::Value::as_array_mut)
         {
-            // Step 1.
+            // Step 2.
             let built_ins: Vec<_> = access_method_settings_list
                 .iter()
                 .filter(|value| {
@@ -168,13 +176,13 @@ fn migrate_api_access_settings(settings: &mut serde_json::Value) -> Result<()> {
                 .cloned()
                 .collect();
 
-            // Step 2.
+            // Step 3.
             for built_in in built_ins.iter() {
                 access_method_settings_list
                     .retain(|access_method| access_method.get("id") != built_in.get("id"));
             }
 
-            // Step 3.
+            // Step 4.
             // Note that the only supported built-in access methods at this time
             // are "Direct" and "Mullvad Bridges", so we may discard anything
             // else.
@@ -284,15 +292,24 @@ fn extract_str(opt: Option<&serde_json::Value>) -> Result<&str> {
         .ok_or(Error::InvalidSettingsContent)
 }
 
-fn rename_field(object: &mut serde_json::Value, old_name: &str, new_name: &str) -> Result<()> {
-    object[new_name] = object
+fn rename_field(value: &mut serde_json::Value, old_name: &str, new_name: &str) -> Result<()> {
+    value
+        .as_object_mut()
+        .ok_or(Error::InvalidSettingsContent)
+        .and_then(|object| rename_map_field(object, old_name, new_name))
+}
+
+fn rename_map_field(
+    object: &mut serde_json::Map<String, serde_json::Value>,
+    old_name: &str,
+    new_name: &str,
+) -> Result<()> {
+    let old_value = object
         .get(old_name)
         .ok_or(Error::InvalidSettingsContent)?
         .clone();
-    object
-        .as_object_mut()
-        .ok_or(Error::InvalidSettingsContent)?
-        .remove(old_name);
+    let _ = object.insert(new_name.to_string(), old_value);
+    object.remove(old_name);
     Ok(())
 }
 
@@ -547,7 +564,7 @@ mod test {
         "built_in": "bridge"
       }
     },
-    "access_method_settings": [
+    "user_defined": [
       {
         "id": "1aaff7ab-e09f-4c03-af02-765e41943a7b",
         "name": "localsox",
@@ -907,7 +924,7 @@ mod test {
             r#"
 {
   "api_access_methods": {
-    "access_method_settings": [
+    "user_defined": [
       {
         "id": "5eb9b2ee-f764-47c8-8111-ee95910d0099",
         "name": "mysocks",
@@ -965,7 +982,7 @@ mod test {
             r#"
 {
   "api_access_methods": {
-    "access_method_settings": [
+    "user_defined": [
       {
         "id": "8e377232-8a53-4414-8b8f-f487227aaedb",
         "name": "remotesox",
@@ -1020,7 +1037,7 @@ mod test {
             r#"
 {
   "api_access_methods": {
-    "access_method_settings": [
+    "user_defined": [
       {
         "id": "74e5c659-acdd-4cad-a632-a25bf63c20e2",
         "name": "remotess",
@@ -1079,7 +1096,7 @@ mod test {
         "built_in": "direct"
       }
     },
-    "access_method_settings": []
+    "user_defined": []
   }
 }
 "#,
@@ -1124,7 +1141,7 @@ mod test {
         "built_in": "bridge"
       }
     },
-    "access_method_settings": []
+    "user_defined": []
   }
 }
 "#,
@@ -1171,7 +1188,7 @@ mod test {
             r#"
 {
   "api_access_methods": {
-    "access_method_settings": [
+    "user_defined": [
       {
         "id": "1aaff7ab-e09f-4c03-af02-765e41943a7b",
         "name": "localsox",
@@ -1225,7 +1242,7 @@ mod test {
             r#"
 {
   "api_access_methods": {
-    "access_method_settings": []
+    "user_defined": []
   }
 }
 "#,

mullvad-management-interface/src/types/conversions/access_method.rs

@@ -11,7 +11,7 @@ mod settings {
                 direct: Some(settings.direct.into()),
                 mullvad_bridges: Some(settings.mullvad_bridges.into()),
                 access_method_settings: settings
-                    .access_method_settings
+                    .user_defined
                     .into_iter()
                     .map(|method| method.into())
                     .collect(),
@@ -38,7 +38,7 @@ mod settings {
                     ))
                     .and_then(access_method::AccessMethodSetting::try_from)?,
 
-                access_method_settings: settings
+                user_defined: settings
                     .access_method_settings
                     .iter()
                     .map(access_method::AccessMethodSetting::try_from)

mullvad-types/src/access_method.rs

@@ -6,13 +6,14 @@ use talpid_types::net::proxy::{CustomProxy, Shadowsocks, Socks5Local, Socks5Remo
 pub struct Settings {
     pub direct: AccessMethodSetting,
     pub mullvad_bridges: AccessMethodSetting,
-    pub access_method_settings: Vec<AccessMethodSetting>,
+    /// User-defined API access methods.
+    pub user_defined: Vec<AccessMethodSetting>,
 }
 
 impl Settings {
     /// Append an [`AccessMethod`] to the end of `api_access_methods`.
     pub fn append(&mut self, api_access_method: AccessMethodSetting) {
-        self.access_method_settings.push(api_access_method)
+        self.user_defined.push(api_access_method)
     }
 
     /// Remove an [`ApiAccessMethod`] from `api_access_methods`.
@@ -21,7 +22,7 @@ impl Settings {
     /// be removed.
     pub fn remove(&mut self, api_access_method: &Id) -> Result<(), Error> {
         let maybe_setting = self
-            .access_method_settings
+            .user_defined
             .iter()
             .find(|setting| setting.get_id() == *api_access_method);
 
@@ -31,7 +32,7 @@ impl Settings {
                     attempted: built_in.clone(),
                 }),
                 AccessMethod::Custom(_) => {
-                    self.access_method_settings
+                    self.user_defined
                         .retain(|method| method.get_id() != *api_access_method);
                     Ok(())
                 }
@@ -53,15 +54,15 @@ impl Settings {
         use std::iter::once;
         once(&self.direct)
             .chain(once(&self.mullvad_bridges))
-            .chain(&self.access_method_settings)
+            .chain(&self.user_defined)
     }
 
     /// Iterate over mutable references of built-in & custom access methods.
     pub fn iter_mut(&mut self) -> impl Iterator<Item = &mut AccessMethodSetting> {
         use std::iter::once;
         once(&mut self.direct)
             .chain(once(&mut self.mullvad_bridges))
-            .chain(&mut self.access_method_settings)
+            .chain(&mut self.user_defined)
     }
 
     pub fn direct() -> AccessMethodSetting {
@@ -80,7 +81,7 @@ impl Default for Settings {
         Self {
             direct: Settings::direct(),
             mullvad_bridges: Settings::mullvad_bridges(),
-            access_method_settings: vec![],
+            user_defined: vec![],
         }
     }
 }