ignore gateways with invalid transports while still adding gateways with valid configurations

4 files changed, 37 insertions, 21 deletions

diff --git a/app/src/main/java/se/leap/bitmaskclient/eip/Gateway.java b/app/src/main/java/se/leap/bitmaskclient/eip/Gateway.java
index 15ee13c2..aae622b8 100644
--- a/app/src/main/java/se/leap/bitmaskclient/eip/Gateway.java
+++ b/app/src/main/java/se/leap/bitmaskclient/eip/Gateway.java
@@ -17,7 +17,6 @@
 package se.leap.bitmaskclient.eip;
 
 import android.content.Context;
-
 import android.support.annotation.NonNull;
 
 import com.google.gson.Gson;
@@ -26,14 +25,14 @@ import org.json.JSONException;
 import org.json.JSONObject;
 
 import java.io.IOException;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Set;
-import java.util.HashMap;
 
 import de.blinkt.openvpn.VpnProfile;
 import de.blinkt.openvpn.core.ConfigParser;
-import se.leap.bitmaskclient.utils.PreferenceHelper;
 import de.blinkt.openvpn.core.connection.Connection;
+import se.leap.bitmaskclient.utils.PreferenceHelper;
 
 import static se.leap.bitmaskclient.Constants.IP_ADDRESS;
 import static se.leap.bitmaskclient.Constants.LOCATION;
@@ -70,7 +69,8 @@ public class Gateway {
      * Build a gateway object from a JSON OpenVPN gateway definition in eip-service.json
      * and create a VpnProfile belonging to it.
      */
-    public Gateway(JSONObject eipDefinition, JSONObject secrets, JSONObject gateway, Context context) {
+    public Gateway(JSONObject eipDefinition, JSONObject secrets, JSONObject gateway, Context context)
+            throws ConfigParser.ConfigParseError, JSONException, IOException {
 
         this.gateway = gateway;
         this.secrets = secrets;
@@ -132,16 +132,11 @@ public class Gateway {
     /**
      * Create and attach the VpnProfile to our gateway object
      */
-    private @NonNull HashMap<Connection.TransportType, VpnProfile> createVPNProfiles(Context context) {
-        HashMap<Connection.TransportType, VpnProfile> profiles = new HashMap<>();
-        try {
-            VpnConfigGenerator vpnConfigurationGenerator = new VpnConfigGenerator(generalConfiguration, secrets, gateway, apiVersion);
-            profiles = vpnConfigurationGenerator.generateVpnProfiles();
-            addProfileInfos(context, profiles);
-        } catch (ConfigParser.ConfigParseError | IOException | JSONException e) {
-            // FIXME We didn't get a VpnProfile!  Error handling! and log level
-            e.printStackTrace();
-        }
+    private @NonNull HashMap<Connection.TransportType, VpnProfile> createVPNProfiles(Context context)
+            throws ConfigParser.ConfigParseError, IOException, JSONException {
+        VpnConfigGenerator vpnConfigurationGenerator = new VpnConfigGenerator(generalConfiguration, secrets, gateway, apiVersion);
+        HashMap<Connection.TransportType, VpnProfile> profiles = vpnConfigurationGenerator.generateVpnProfiles();
+        addProfileInfos(context, profiles);
         return profiles;
     }
 
diff --git a/app/src/main/java/se/leap/bitmaskclient/eip/GatewaysManager.java b/app/src/main/java/se/leap/bitmaskclient/eip/GatewaysManager.java
index 0847a07e..c7d7f86f 100644
--- a/app/src/main/java/se/leap/bitmaskclient/eip/GatewaysManager.java
+++ b/app/src/main/java/se/leap/bitmaskclient/eip/GatewaysManager.java
@@ -26,10 +26,13 @@ import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
 
+import java.io.IOException;
 import java.lang.reflect.Type;
 import java.util.ArrayList;
 import java.util.LinkedHashMap;
 
+import de.blinkt.openvpn.core.ConfigParser;
+import de.blinkt.openvpn.core.VpnStatus;
 import se.leap.bitmaskclient.Provider;
 import se.leap.bitmaskclient.utils.PreferenceHelper;
 
@@ -87,19 +90,25 @@ public class GatewaysManager {
      * @param eipDefinition eipServiceJson
      */
     void fromEipServiceJson(JSONObject eipDefinition) {
+        JSONArray gatewaysDefined = new JSONArray();
         try {
-            JSONArray gatewaysDefined = eipDefinition.getJSONArray(GATEWAYS);
-            for (int i = 0; i < gatewaysDefined.length(); i++) {
+            gatewaysDefined = eipDefinition.getJSONArray(GATEWAYS);
+        } catch (Exception e) {
+            e.printStackTrace();
+        }
+
+        for (int i = 0; i < gatewaysDefined.length(); i++) {
+            try {
                 JSONObject gw = gatewaysDefined.getJSONObject(i);
                 JSONObject secrets = secretsConfiguration();
                 Gateway aux = new Gateway(eipDefinition, secrets, gw, this.context);
                 if (gateways.get(aux.getRemoteIP()) == null) {
                     addGateway(aux);
                 }
+            } catch (JSONException | ConfigParser.ConfigParseError | IOException e) {
+                e.printStackTrace();
+                VpnStatus.logError("Unable to parse gateway config!");
             }
-        } catch (Exception e) {
-            // TODO Auto-generated catch block
-            e.printStackTrace();
         }
     }
 
diff --git a/app/src/test/java/se/leap/bitmaskclient/eip/GatewaySelectorTest.java b/app/src/test/java/se/leap/bitmaskclient/eip/GatewaySelectorTest.java
index 2f5ae9b6..5089de54 100644
--- a/app/src/test/java/se/leap/bitmaskclient/eip/GatewaySelectorTest.java
+++ b/app/src/test/java/se/leap/bitmaskclient/eip/GatewaySelectorTest.java
@@ -49,6 +49,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
 
+import de.blinkt.openvpn.core.ConfigParser;
 import se.leap.bitmaskclient.Provider;
 import se.leap.bitmaskclient.utils.ConfigHelper;
 
@@ -99,7 +100,7 @@ public class GatewaySelectorTest {
     ArrayList<Gateway> gatewayList = new ArrayList<>();
 
     @Before
-    public void setup() throws IOException, JSONException {
+    public void setup() throws IOException, JSONException, ConfigParser.ConfigParseError {
         mockStatic(ConfigHelper.class);
         eipDefinition = new JSONObject(getInputAsString(getClass().getClassLoader().getResourceAsStream("eip-service-four-gateways.json")));
         JSONArray gateways = eipDefinition.getJSONArray("gateways");
diff --git a/app/src/test/java/se/leap/bitmaskclient/eip/GatewaysManagerTest.java b/app/src/test/java/se/leap/bitmaskclient/eip/GatewaysManagerTest.java
index e4c5f390..46e166d4 100644
--- a/app/src/test/java/se/leap/bitmaskclient/eip/GatewaysManagerTest.java
+++ b/app/src/test/java/se/leap/bitmaskclient/eip/GatewaysManagerTest.java
@@ -14,12 +14,14 @@ import org.mockito.junit.MockitoJUnitRunner;
 
 import java.io.IOException;
 
+import de.blinkt.openvpn.core.connection.Connection;
 import se.leap.bitmaskclient.Constants;
 import se.leap.bitmaskclient.Provider;
 import se.leap.bitmaskclient.testutils.TestSetupHelper;
 
 import static junit.framework.Assert.assertEquals;
-import static org.mockito.ArgumentMatchers.anyInt;
+import static junit.framework.Assert.assertNotNull;
+import static junit.framework.Assert.assertNull;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.when;
@@ -68,6 +70,15 @@ public class GatewaysManagerTest {
     }
 
     @Test
+    public void testFromEipServiceJson_ignoreGatewaysWithMisconfiguredTransportsWhileAddingValidOnes() throws Exception {
+        String eipServiceJson = TestSetupHelper.getInputAsString(getClass().getClassLoader().getResourceAsStream("ptdemo_misconfigured_mixed_gateways.json"));
+        gatewaysManager.fromEipServiceJson(new JSONObject(eipServiceJson));
+        assertEquals(1, gatewaysManager.size());
+        assertNull(gatewaysManager.select(0).getProfile(Connection.TransportType.OBFS4));
+        assertNotNull(gatewaysManager.select(0).getProfile(Connection.TransportType.OPENVPN));
+    }
+
+    @Test
     public void testFromEipServiceJson_ignoreDuplicateGateways() throws Exception {
         String eipServiceJson = TestSetupHelper.getInputAsString(getClass().getClassLoader().getResourceAsStream("eip-service-two-gateways.json"));
         gatewaysManager.fromEipServiceJson(new JSONObject(eipServiceJson));