Merge branch 'add-multiple-providers'

9 files changed, 138 insertions, 75 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index fe027457ee..8d1feca7a5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -33,6 +33,7 @@ Line wrap the file at 100 chars.                                              Th
 ### Changed
 - Update Electron from 11.0.2 to 11.2.1 which includes a newer Chromium version and
   security patches.
+- Allow provider constraint to specify multiple hosting providers.
 
 #### Android
 - WireGuard key is now rotated sooner: every four days instead of seven.
diff --git a/mullvad-cli/src/cmds/bridge.rs b/mullvad-cli/src/cmds/bridge.rs
index 9a674069a8..b6692d0198 100644
--- a/mullvad-cli/src/cmds/bridge.rs
+++ b/mullvad-cli/src/cmds/bridge.rs
@@ -1,5 +1,5 @@
 use crate::{location, new_rpc_client, Command, Error, Result};
-use clap::value_t;
+use clap::{value_t, values_t};
 
 use mullvad_management_interface::types::{
     bridge_settings::{Type as BridgeSettingsType, *},
@@ -48,12 +48,13 @@ fn create_bridge_set_subcommand() -> clap::App<'static, 'static> {
         .subcommand(
             clap::SubCommand::with_name("provider")
                 .about(
-                    "Set a hosting provider to select bridge relays from. The 'list' \
+                    "Set hosting provider(s) to select bridge relays from. The 'list' \
                         command shows the available relays and their providers.",
                 )
                 .arg(
                     clap::Arg::with_name("provider")
-                        .help("The hosting provider to use, or 'any' for no preference.")
+                        .help("The hosting provider(s) to use, or 'any' for no preference.")
+                        .multiple(true)
                         .required(true),
                 ),
         )
@@ -192,7 +193,7 @@ impl Bridge {
                 println!(
                     "Bridge constraints - {}, {}",
                     location::format_location(constraints.location.as_ref()),
-                    location::format_provider(constraints.provider.as_ref())
+                    location::format_providers(&constraints.providers)
                 );
             }
         };
@@ -204,20 +205,20 @@ impl Bridge {
     }
 
     async fn handle_set_bridge_provider(matches: &clap::ArgMatches<'_>) -> Result<()> {
-        let new_provider =
-            value_t!(matches.value_of("provider"), String).unwrap_or_else(|e| e.exit());
-        let new_provider = if new_provider == "any" {
-            "".to_string()
+        let providers =
+            values_t!(matches.values_of("provider"), String).unwrap_or_else(|e| e.exit());
+        let providers = if providers.iter().next().map(String::as_str) == Some("any") {
+            vec![]
         } else {
-            new_provider
+            providers
         };
 
-        Self::update_bridge_settings(None, Some(new_provider)).await
+        Self::update_bridge_settings(None, Some(providers)).await
     }
 
     async fn update_bridge_settings(
         location: Option<RelayLocation>,
-        provider: Option<String>,
+        providers: Option<Vec<String>>,
     ) -> Result<()> {
         let mut rpc = new_rpc_client().await?;
         let settings = rpc.get_settings(()).await?.into_inner();
@@ -228,18 +229,18 @@ impl Bridge {
                 if let Some(new_location) = location {
                     constraints.location = Some(new_location);
                 }
-                if let Some(new_provider) = provider {
-                    constraints.provider = new_provider;
+                if let Some(new_providers) = providers {
+                    constraints.providers = new_providers;
                 }
                 constraints
             }
             _ => {
                 let location = location.unwrap_or_default();
-                let provider = provider.unwrap_or_default();
+                let providers = providers.unwrap_or_default();
 
                 BridgeConstraints {
                     location: Some(location),
-                    provider,
+                    providers,
                 }
             }
         };
diff --git a/mullvad-cli/src/cmds/relay.rs b/mullvad-cli/src/cmds/relay.rs
index e8783e04f3..4ad950fe7a 100644
--- a/mullvad-cli/src/cmds/relay.rs
+++ b/mullvad-cli/src/cmds/relay.rs
@@ -128,11 +128,12 @@ impl Command for Relay {
                     )
                     .subcommand(
                         clap::SubCommand::with_name("provider")
-                            .about("Set a hosting provider to select relays from. The 'list' \
+                            .about("Set hosting provider(s) to select relays from. The 'list' \
                                    command shows the available relays and their providers.")
                             .arg(
                                 clap::Arg::with_name("provider")
-                                .help("The hosting provider to use, or 'any' for no preference.")
+                                .help("The hosting provider(s) to use, or 'any' for no preference.")
+                                .multiple(true)
                                 .required(true)
                             )
                     )
@@ -207,8 +208,8 @@ impl Relay {
             self.set_location(location_matches).await
         } else if let Some(relay_matches) = matches.subcommand_matches("hostname") {
             self.set_hostname(relay_matches).await
-        } else if let Some(provider_matches) = matches.subcommand_matches("provider") {
-            self.set_provider(provider_matches).await
+        } else if let Some(providers_matches) = matches.subcommand_matches("provider") {
+            self.set_providers(providers_matches).await
         } else if let Some(tunnel_matches) = matches.subcommand_matches("tunnel") {
             self.set_tunnel(tunnel_matches).await
         } else if let Some(tunnel_matches) = matches.subcommand_matches("tunnel-protocol") {
@@ -440,19 +441,20 @@ impl Relay {
         .await
     }
 
-    async fn set_provider(&self, matches: &clap::ArgMatches<'_>) -> Result<()> {
-        let provider = value_t!(matches.value_of("provider"), String).unwrap_or_else(|e| e.exit());
+    async fn set_providers(&self, matches: &clap::ArgMatches<'_>) -> Result<()> {
+        let providers =
+            values_t!(matches.values_of("provider"), String).unwrap_or_else(|e| e.exit());
+
+        let providers = if providers.iter().next().map(String::as_str) == Some("any") {
+            vec![]
+        } else {
+            providers
+        };
 
         self.update_constraints(RelaySettingsUpdate {
             r#type: Some(relay_settings_update::Type::Normal(
                 NormalRelaySettingsUpdate {
-                    provider: Some(ProviderUpdate {
-                        provider: if provider == "any" {
-                            "".to_string()
-                        } else {
-                            provider
-                        },
-                    }),
+                    providers: Some(ProviderUpdate { providers }),
                     ..Default::default()
                 },
             )),
@@ -545,7 +547,7 @@ impl Relay {
                         Self::format_openvpn_constraints(settings.openvpn_constraints.as_ref()),
                         Self::format_wireguard_constraints(settings.wireguard_constraints.as_ref()),
                         location::format_location(settings.location.as_ref()),
-                        location::format_provider(settings.provider.as_ref())
+                        location::format_providers(&settings.providers)
                     );
                 }
                 Some(constraint) => match TunnelType::from_i32(constraint.tunnel_type).unwrap() {
@@ -556,7 +558,7 @@ impl Relay {
                                 settings.wireguard_constraints.as_ref()
                             ),
                             location::format_location(settings.location.as_ref()),
-                            location::format_provider(settings.provider.as_ref())
+                            location::format_providers(&settings.providers)
                         );
                     }
                     TunnelType::Openvpn => {
@@ -564,7 +566,7 @@ impl Relay {
                             "OpenVPN over {} in {} using {}",
                             Self::format_openvpn_constraints(settings.openvpn_constraints.as_ref()),
                             location::format_location(settings.location.as_ref()),
-                            location::format_provider(settings.provider.as_ref())
+                            location::format_providers(&settings.providers)
                         );
                     }
                 },
diff --git a/mullvad-cli/src/location.rs b/mullvad-cli/src/location.rs
index bc4c9bc630..09f39720ba 100644
--- a/mullvad-cli/src/location.rs
+++ b/mullvad-cli/src/location.rs
@@ -73,9 +73,9 @@ pub fn format_location(location: Option<&RelayLocation>) -> String {
     "any location".to_string()
 }
 
-pub fn format_provider(provider: &str) -> String {
-    if !provider.is_empty() {
-        format!("provider {}", provider)
+pub fn format_providers(providers: &Vec<String>) -> String {
+    if !providers.is_empty() {
+        format!("provider(s) {}", providers.join(", "))
     } else {
         "any provider".to_string()
     }
diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs
index 41aad6ed23..073773a418 100644
--- a/mullvad-daemon/src/lib.rs
+++ b/mullvad-daemon/src/lib.rs
@@ -973,7 +973,7 @@ where
                     BridgeSettings::Normal(settings) => {
                         let bridge_constraints = InternalBridgeConstraints {
                             location: settings.location.clone(),
-                            provider: settings.provider.clone(),
+                            providers: settings.providers.clone(),
                             // FIXME: This is temporary while talpid-core only supports TCP proxies
                             transport_protocol: Constraint::Only(TransportProtocol::Tcp),
                         };
diff --git a/mullvad-daemon/src/management_interface.rs b/mullvad-daemon/src/management_interface.rs
index 7faa0eee14..c87b846387 100644
--- a/mullvad-daemon/src/management_interface.rs
+++ b/mullvad-daemon/src/management_interface.rs
@@ -13,7 +13,7 @@ use mullvad_types::{
     location::GeoIpLocation,
     relay_constraints::{
         BridgeConstraints, BridgeSettings, BridgeState, Constraint, LocationConstraint,
-        OpenVpnConstraints, Provider, RelayConstraintsUpdate, RelaySettings, RelaySettingsUpdate,
+        OpenVpnConstraints, Providers, RelayConstraintsUpdate, RelaySettings, RelaySettingsUpdate,
         WireguardConstraints,
     },
     relay_list::{Relay, RelayList, RelayListCountry},
@@ -241,12 +241,20 @@ impl ManagementService for ManagementServiceImpl {
                     None => Constraint::Any,
                     Some(location) => convert_proto_location(location),
                 };
-                let provider = match constraints.provider.as_ref() {
-                    "" => Constraint::Any,
-                    provider => Constraint::Only(String::from(provider)),
+                let providers = if constraints.providers.is_empty() {
+                    Constraint::Any
+                } else {
+                    Constraint::Only(
+                        Providers::new(constraints.providers.clone().into_iter()).map_err(
+                            |_| Status::invalid_argument("must specify at least one provider"),
+                        )?,
+                    )
                 };
 
-                BridgeSettings::Normal(BridgeConstraints { location, provider })
+                BridgeSettings::Normal(BridgeConstraints {
+                    location,
+                    providers,
+                })
             }
             BridgeSettingType::Local(proxy_settings) => {
                 let peer = proxy_settings
@@ -928,15 +936,23 @@ fn convert_relay_settings_update(
                 None
             };
 
+            let providers = if let Some(ref provider_update) = settings.providers {
+                if !provider_update.providers.is_empty() {
+                    Some(Constraint::Only(
+                        Providers::new(provider_update.providers.clone().into_iter()).map_err(
+                            |_| Status::invalid_argument("must specify at least one provider"),
+                        )?,
+                    ))
+                } else {
+                    Some(Constraint::Any)
+                }
+            } else {
+                None
+            };
+
             Ok(RelaySettingsUpdate::Normal(RelayConstraintsUpdate {
                 location,
-                provider: settings.provider.map(|provider_update| {
-                    if !provider_update.provider.is_empty() {
-                        Constraint::Only(provider_update.provider.clone())
-                    } else {
-                        Constraint::Any
-                    }
-                }),
+                providers,
                 tunnel_protocol,
                 wireguard_constraints: settings.wireguard_constraints.map(|constraints| {
                     WireguardConstraints {
@@ -975,7 +991,7 @@ fn convert_relay_settings(settings: &RelaySettings) -> types::RelaySettings {
         RelaySettings::Normal(constraints) => {
             relay_settings::Endpoint::Normal(types::NormalRelaySettings {
                 location: convert_location_constraint(&constraints.location),
-                provider: convert_provider_constraint(&constraints.provider),
+                providers: convert_providers_constraint(&constraints.providers),
                 tunnel_type: match constraints.tunnel_protocol {
                     Constraint::Any => None,
                     Constraint::Only(TunnelType::Wireguard) => Some(types::TunnelType::Wireguard),
@@ -1070,7 +1086,7 @@ fn convert_bridge_settings(settings: &BridgeSettings) -> types::BridgeSettings {
         BridgeSettings::Normal(constraints) => {
             BridgeSettingType::Normal(types::bridge_settings::BridgeConstraints {
                 location: convert_location_constraint(&constraints.location),
-                provider: convert_provider_constraint(&constraints.provider),
+                providers: convert_providers_constraint(&constraints.providers),
             })
         }
         BridgeSettings::Custom(proxy_settings) => match proxy_settings {
@@ -1157,10 +1173,10 @@ fn convert_location_constraint(
     })
 }
 
-fn convert_provider_constraint(provider: &Constraint<Provider>) -> String {
-    match provider.as_ref() {
-        Constraint::Any => "".to_string(),
-        Constraint::Only(ref provider) => provider.to_string(),
+fn convert_providers_constraint(providers: &Constraint<Providers>) -> Vec<String> {
+    match providers.as_ref() {
+        Constraint::Any => vec![],
+        Constraint::Only(providers) => Vec::from(providers.clone()),
     }
 }
 
diff --git a/mullvad-daemon/src/relays.rs b/mullvad-daemon/src/relays.rs
index ce880a3d19..c3c2e7e365 100644
--- a/mullvad-daemon/src/relays.rs
+++ b/mullvad-daemon/src/relays.rs
@@ -14,7 +14,7 @@ use mullvad_types::{
     location::Location,
     relay_constraints::{
         BridgeState, Constraint, InternalBridgeConstraints, LocationConstraint, Match,
-        OpenVpnConstraints, Provider, RelayConstraints, WireguardConstraints,
+        OpenVpnConstraints, Providers, RelayConstraints, WireguardConstraints,
     },
     relay_list::{OpenVpnEndpointData, Relay, RelayList, RelayTunnels, WireguardEndpointData},
 };
@@ -253,7 +253,7 @@ impl RelaySelector {
                 self.preferred_tunnel_constraints(
                     retry_attempt,
                     &original_constraints.location,
-                    &original_constraints.provider,
+                    &original_constraints.providers,
                     wg_key_exists,
                 )
             } else {
@@ -378,7 +378,7 @@ impl RelaySelector {
         &self,
         retry_attempt: u32,
         location_constraint: &Constraint<LocationConstraint>,
-        provider_constraint: &Constraint<Provider>,
+        providers_constraint: &Constraint<Providers>,
         wg_key_exists: bool,
     ) -> (Constraint<u16>, TransportProtocol, TunnelType) {
         #[cfg(not(target_os = "windows"))]
@@ -388,7 +388,7 @@ impl RelaySelector {
                     relay.active
                         && !relay.tunnels.wireguard.is_empty()
                         && location_constraint.matches(relay)
-                        && provider_constraint.matches_eq(&relay.provider)
+                        && providers_constraint.matches(relay)
                 });
             // If location does not support WireGuard, defer to preferred OpenVPN tunnel
             // constraints
@@ -475,7 +475,7 @@ impl RelaySelector {
         if !constraints.location.matches(relay) {
             return None;
         }
-        if !constraints.provider.matches_eq(&relay.provider) {
+        if !constraints.providers.matches(&relay) {
             return None;
         }
 
@@ -543,7 +543,7 @@ impl RelaySelector {
         if !constraints.location.matches(relay) {
             return None;
         }
-        if !constraints.provider.matches_eq(&relay.provider) {
+        if !constraints.providers.matches(relay) {
             return None;
         }
 
diff --git a/mullvad-management-interface/proto/management_interface.proto b/mullvad-management-interface/proto/management_interface.proto
index 755703c3ee..08b8a708d9 100644
--- a/mullvad-management-interface/proto/management_interface.proto
+++ b/mullvad-management-interface/proto/management_interface.proto
@@ -201,7 +201,7 @@ message AccountHistory {
 message BridgeSettings {
 	message BridgeConstraints {
 		RelayLocation location = 1;
-		string provider = 2;
+		repeated string providers = 2;
 	}
 
 	message LocalProxySettings {
@@ -271,7 +271,7 @@ message TunnelTypeConstraint {
 
 message NormalRelaySettings {
 	RelayLocation location = 1;
-	string provider = 2;
+	repeated string providers = 2;
 	TunnelTypeConstraint tunnel_type = 3;
 	WireguardConstraints wireguard_constraints = 4;
 	OpenvpnConstraints openvpn_constraints = 5;
@@ -280,14 +280,14 @@ message NormalRelaySettings {
 // Constraints are only updated for fields that are provided
 message NormalRelaySettingsUpdate {
 	RelayLocation location = 1;
-	ProviderUpdate provider = 2;
+	ProviderUpdate providers = 2;
 	TunnelTypeUpdate tunnel_type = 3;
 	WireguardConstraints wireguard_constraints = 4;
 	OpenvpnConstraints openvpn_constraints = 5;
 }
 
 message ProviderUpdate {
-	string provider = 1;
+	repeated string providers = 1;
 }
 
 message TunnelTypeUpdate {
diff --git a/mullvad-types/src/relay_constraints.rs b/mullvad-types/src/relay_constraints.rs
index a5a60f5921..c9042c8f49 100644
--- a/mullvad-types/src/relay_constraints.rs
+++ b/mullvad-types/src/relay_constraints.rs
@@ -9,7 +9,7 @@ use crate::{
 #[cfg(target_os = "android")]
 use jnix::{FromJava, IntoJava};
 use serde::{Deserialize, Serialize};
-use std::fmt;
+use std::{collections::HashSet, fmt};
 use talpid_types::net::{openvpn::ProxySettings, TransportProtocol, TunnelType};
 
 
@@ -186,7 +186,7 @@ impl RelaySettings {
 pub struct RelayConstraints {
     pub location: Constraint<LocationConstraint>,
     #[cfg_attr(target_os = "android", jnix(skip))]
-    pub provider: Constraint<Provider>,
+    pub providers: Constraint<Providers>,
     #[cfg_attr(target_os = "android", jnix(skip))]
     pub tunnel_protocol: Constraint<TunnelType>,
     #[cfg_attr(target_os = "android", jnix(skip))]
@@ -201,7 +201,7 @@ impl Default for RelayConstraints {
         RelayConstraints {
             tunnel_protocol: Constraint::Only(TunnelType::Wireguard),
             location: Constraint::default(),
-            provider: Constraint::default(),
+            providers: Constraint::default(),
             wireguard_constraints: WireguardConstraints::default(),
             openvpn_constraints: OpenVpnConstraints::default(),
         }
@@ -212,7 +212,7 @@ impl RelayConstraints {
     pub fn merge(&self, update: RelayConstraintsUpdate) -> Self {
         RelayConstraints {
             location: update.location.unwrap_or_else(|| self.location.clone()),
-            provider: update.provider.unwrap_or_else(|| self.provider.clone()),
+            providers: update.providers.unwrap_or_else(|| self.providers.clone()),
             tunnel_protocol: update
                 .tunnel_protocol
                 .unwrap_or_else(|| self.tunnel_protocol.clone()),
@@ -252,12 +252,9 @@ impl fmt::Display for RelayConstraints {
             Constraint::Only(ref location_constraint) => location_constraint.fmt(f)?,
         }
         write!(f, " using ")?;
-        match self.provider {
+        match self.providers {
             Constraint::Any => write!(f, "any provider"),
-            Constraint::Only(ref constraint) => {
-                write!(f, "provider ")?;
-                constraint.fmt(f)
-            }
+            Constraint::Only(ref constraint) => constraint.fmt(f),
         }
     }
 }
@@ -308,6 +305,52 @@ impl Match<Relay> for LocationConstraint {
 /// provider.
 pub type Provider = String;
 
+#[derive(Debug, Clone, Eq, PartialEq, Deserialize, Serialize)]
+pub struct Providers {
+    providers: HashSet<Provider>,
+}
+
+/// Returned if the iterator contained no providers.
+pub struct NoProviders(());
+
+impl Providers {
+    pub fn new(providers: impl Iterator<Item = Provider>) -> Result<Providers, NoProviders> {
+        let providers = Providers {
+            providers: providers.collect(),
+        };
+        if providers.providers.is_empty() {
+            return Err(NoProviders(()));
+        }
+        Ok(providers)
+    }
+}
+
+impl Match<Relay> for Providers {
+    fn matches(&self, relay: &Relay) -> bool {
+        self.providers.contains(&relay.provider)
+    }
+}
+
+impl From<Providers> for Vec<Provider> {
+    fn from(providers: Providers) -> Vec<Provider> {
+        providers.providers.into_iter().collect()
+    }
+}
+
+impl fmt::Display for Providers {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
+        write!(f, "provider(s) ")?;
+        for (i, provider) in self.providers.iter().enumerate() {
+            if i == 0 {
+                write!(f, "{}", provider)?;
+            } else {
+                write!(f, ", {}", provider)?;
+            }
+        }
+        Ok(())
+    }
+}
+
 impl fmt::Display for LocationConstraint {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
         match self {
@@ -434,7 +477,7 @@ pub enum BridgeSettings {
 #[serde(rename_all = "snake_case")]
 pub struct BridgeConstraints {
     pub location: Constraint<LocationConstraint>,
-    pub provider: Constraint<Provider>,
+    pub providers: Constraint<Providers>,
 }
 
 impl fmt::Display for BridgeConstraints {
@@ -472,7 +515,7 @@ impl fmt::Display for BridgeState {
 #[derive(Debug, Clone, Eq, PartialEq, Deserialize, Serialize)]
 pub struct InternalBridgeConstraints {
     pub location: Constraint<LocationConstraint>,
-    pub provider: Constraint<Provider>,
+    pub providers: Constraint<Providers>,
     pub transport_protocol: Constraint<TransportProtocol>,
 }
 
@@ -520,7 +563,7 @@ impl RelaySettingsUpdate {
 pub struct RelayConstraintsUpdate {
     pub location: Option<Constraint<LocationConstraint>>,
     #[cfg_attr(target_os = "android", jnix(default))]
-    pub provider: Option<Constraint<Provider>>,
+    pub providers: Option<Constraint<Providers>>,
     #[cfg_attr(target_os = "android", jnix(default))]
     pub tunnel_protocol: Option<Constraint<TunnelType>>,
     #[cfg_attr(target_os = "android", jnix(default))]