Merge branch '#8853_error_handling_adding_providers' into '0.9.8'

#8853 error handling adding providers

See merge request leap/bitmask_android!46

8 files changed, 49 insertions, 65 deletions

diff --git a/app/src/main/java/de/blinkt/openvpn/core/OpenVPNService.java b/app/src/main/java/de/blinkt/openvpn/core/OpenVPNService.java
index 6c312c87..f701b7aa 100644
--- a/app/src/main/java/de/blinkt/openvpn/core/OpenVPNService.java
+++ b/app/src/main/java/de/blinkt/openvpn/core/OpenVPNService.java
@@ -913,7 +913,7 @@ public class OpenVPNService extends VpnService implements StateListener, Callbac
             // CONNECTED
             // Does not work :(
             notificationManager.buildOpenVpnNotification(
-                    mProfile.mName,
+                    mProfile != null ? mProfile.mName : "",
                     VpnStatus.getLastCleanLogMessage(this),
                     VpnStatus.getLastCleanLogMessage(this),
                     level,
@@ -944,7 +944,7 @@ public class OpenVPNService extends VpnService implements StateListener, Callbac
                     humanReadableByteCount(out, false, getResources()),
                     humanReadableByteCount(diffOut / OpenVPNManagement.mBytecountInterval, true, getResources()));
             notificationManager.buildOpenVpnNotification(
-                    mProfile.mName,
+                    mProfile != null ? mProfile.mName : "",
                     netstat,
                     null,
                     LEVEL_CONNECTED,
@@ -987,7 +987,7 @@ public class OpenVPNService extends VpnService implements StateListener, Callbac
     public void requestInputFromUser(int resid, String needed) {
         VpnStatus.updateStateString("NEED", "need " + needed, resid, LEVEL_WAITING_FOR_USER_INPUT);
         notificationManager.buildOpenVpnNotification(
-                mProfile.mName,
+                mProfile != null ? mProfile.mName : "",
                 getString(resid),
                 getString(resid),
                 LEVEL_WAITING_FOR_USER_INPUT,
diff --git a/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java b/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java
index a52df460..a84432e9 100644
--- a/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java
+++ b/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java
@@ -74,7 +74,7 @@ public class ConfigHelper {
 
     public static boolean checkErroneousDownload(String downloadedString) {
         try {
-            if (downloadedString == null || downloadedString.isEmpty() || new JSONObject(downloadedString).has(ProviderAPI.ERRORS)) {
+            if (downloadedString == null || downloadedString.isEmpty() || new JSONObject(downloadedString).has(ProviderAPI.ERRORS) || new JSONObject(downloadedString).has(ProviderAPI.BACKEND_ERROR_KEY)) {
                 return true;
             } else {
                 return false;
diff --git a/app/src/main/java/se/leap/bitmaskclient/EipFragment.java b/app/src/main/java/se/leap/bitmaskclient/EipFragment.java
index fb57aea8..8d1fa03a 100644
--- a/app/src/main/java/se/leap/bitmaskclient/EipFragment.java
+++ b/app/src/main/java/se/leap/bitmaskclient/EipFragment.java
@@ -60,7 +60,7 @@ import static se.leap.bitmaskclient.Constants.REQUEST_CODE_LOG_IN;
 import static se.leap.bitmaskclient.Constants.REQUEST_CODE_SWITCH_PROVIDER;
 import static se.leap.bitmaskclient.Constants.SHARED_PREFERENCES;
 import static se.leap.bitmaskclient.ProviderAPI.DOWNLOAD_VPN_CERTIFICATE;
-import static se.leap.bitmaskclient.ProviderCredentialsBaseActivity.USER_MESSAGE;
+import static se.leap.bitmaskclient.ProviderAPI.USER_MESSAGE;
 import static se.leap.bitmaskclient.R.string.vpn_certificate_user_message;
 
 public class EipFragment extends Fragment implements Observer {
diff --git a/app/src/main/java/se/leap/bitmaskclient/MainActivity.java b/app/src/main/java/se/leap/bitmaskclient/MainActivity.java
index 6e778309..b4be55a4 100644
--- a/app/src/main/java/se/leap/bitmaskclient/MainActivity.java
+++ b/app/src/main/java/se/leap/bitmaskclient/MainActivity.java
@@ -20,7 +20,6 @@ import android.support.v7.app.AppCompatActivity;
 import android.support.v7.widget.Toolbar;
 import android.util.Log;
 
-import org.jetbrains.annotations.NotNull;
 import org.json.JSONException;
 import org.json.JSONObject;
 
@@ -58,7 +57,7 @@ import static se.leap.bitmaskclient.ProviderAPI.CORRECTLY_DOWNLOADED_VPN_CERTIFI
 import static se.leap.bitmaskclient.ProviderAPI.ERRORS;
 import static se.leap.bitmaskclient.ProviderAPI.INCORRECTLY_DOWNLOADED_EIP_SERVICE;
 import static se.leap.bitmaskclient.ProviderAPI.INCORRECTLY_DOWNLOADED_VPN_CERTIFICATE;
-import static se.leap.bitmaskclient.ProviderCredentialsBaseActivity.USER_MESSAGE;
+import static se.leap.bitmaskclient.ProviderAPI.USER_MESSAGE;
 import static se.leap.bitmaskclient.R.string.downloading_vpn_certificate_failed;
 import static se.leap.bitmaskclient.R.string.vpn_certificate_user_message;
 
@@ -67,12 +66,7 @@ public class MainActivity extends AppCompatActivity implements Observer {
 
     public final static String TAG = MainActivity.class.getSimpleName();
 
-    private static final String KEY_ACTIVITY_STATE = "key state of activity";
-    private static final String DEFAULT_UI_STATE = "default state";
-    private static final String SHOW_DIALOG_STATE = "show dialog";
-    private static final String REASON_TO_FAIL = "reason to fail";
-
-    private static Provider provider = new Provider();
+    private Provider provider = new Provider();
     private SharedPreferences preferences;
     private EipStatus eipStatus;
     private NavigationDrawerFragment navigationDrawerFragment;
@@ -198,7 +192,7 @@ public class MainActivity extends AppCompatActivity implements Observer {
                     break;
             }
         }
-
+        //TODO: Why do we want this --v? legacy and redundant?
         Fragment fragment = new EipFragment();
         Bundle arguments = new Bundle();
         arguments.putParcelable(PROVIDER_KEY, provider);
diff --git a/app/src/main/java/se/leap/bitmaskclient/Provider.java b/app/src/main/java/se/leap/bitmaskclient/Provider.java
index 7104143c..98662783 100644
--- a/app/src/main/java/se/leap/bitmaskclient/Provider.java
+++ b/app/src/main/java/se/leap/bitmaskclient/Provider.java
@@ -90,9 +90,8 @@ public final class Provider implements Parcelable {
         }
         if (definition != null) {
             try {
-                this.definition = new JSONObject(definition);
-                parseDefinition(this.definition);
-            } catch (JSONException | NullPointerException e) {
+                define(new JSONObject(definition));
+            } catch (JSONException e) {
                 e.printStackTrace();
             }
         }
@@ -133,26 +132,8 @@ public final class Provider implements Parcelable {
     }
 
     public boolean define(JSONObject providerJson) {
-       /*
-        * fix against "api_uri": "https://calyx.net.malicious.url.net:4430",
-        * This method aims to prevent attacks where the provider.json file got manipulated by a third party.
-        * The main url should not change.
-        */
-
-        try {
-            String providerApiUrl = providerJson.getString(Provider.API_URL);
-            String providerDomain = providerJson.getString(Provider.DOMAIN);
-            if (getMainUrlString().contains(providerDomain) && providerApiUrl.contains(providerDomain  + ":")) {
-                definition = providerJson;
-                parseDefinition(definition);
-                return true;
-            } else {
-                return false;
-            }
-        } catch (JSONException e) {
-            e.printStackTrace();
-            return false;
-        }
+        definition = providerJson;
+        return parseDefinition(definition);
     }
 
     public JSONObject getDefinition() {
@@ -345,7 +326,7 @@ public final class Provider implements Parcelable {
         }
     }
 
-    private void parseDefinition(JSONObject definition) {
+    private boolean parseDefinition(JSONObject definition) {
         try {
             String pin =  definition.getString(CA_CERT_FINGERPRINT);
             this.certificatePin = pin.split(":")[1].trim();
@@ -354,8 +335,9 @@ public final class Provider implements Parcelable {
             this.allowAnonymous = definition.getJSONObject(Provider.SERVICE).getBoolean(PROVIDER_ALLOW_ANONYMOUS);
             this.allowRegistered = definition.getJSONObject(Provider.SERVICE).getBoolean(PROVIDER_ALLOWED_REGISTERED);
             this.apiVersion = getDefinition().getString(Provider.API_VERSION);
+            return true;
         } catch (JSONException | ArrayIndexOutOfBoundsException | MalformedURLException e) {
-            e.printStackTrace();
+            return false;
         }
     }
 
diff --git a/app/src/main/java/se/leap/bitmaskclient/ProviderAPI.java b/app/src/main/java/se/leap/bitmaskclient/ProviderAPI.java
index f5efde05..f1f474d7 100644
--- a/app/src/main/java/se/leap/bitmaskclient/ProviderAPI.java
+++ b/app/src/main/java/se/leap/bitmaskclient/ProviderAPI.java
@@ -51,6 +51,7 @@ public class ProviderAPI extends IntentService implements ProviderApiManagerBase
             ERRORID = "errorId",
             BACKEND_ERROR_KEY = "error",
             BACKEND_ERROR_MESSAGE = "message",
+            USER_MESSAGE = "userMessage",
             DOWNLOAD_SERVICE_JSON = "ProviderAPI.DOWNLOAD_SERVICE_JSON",
             PROVIDER_SET_UP = "ProviderAPI.PROVIDER_SET_UP";
 
diff --git a/app/src/main/java/se/leap/bitmaskclient/ProviderApiManagerBase.java b/app/src/main/java/se/leap/bitmaskclient/ProviderApiManagerBase.java
index b93abaeb..2cde431e 100644
--- a/app/src/main/java/se/leap/bitmaskclient/ProviderApiManagerBase.java
+++ b/app/src/main/java/se/leap/bitmaskclient/ProviderApiManagerBase.java
@@ -46,6 +46,7 @@ import java.security.cert.X509Certificate;
 import java.security.interfaces.RSAPrivateKey;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.NoSuchElementException;
 
 import javax.net.ssl.SSLHandshakeException;
 
@@ -63,19 +64,16 @@ import static se.leap.bitmaskclient.Constants.PROVIDER_PRIVATE_KEY;
 import static se.leap.bitmaskclient.Constants.PROVIDER_VPN_CERTIFICATE;
 import static se.leap.bitmaskclient.ProviderAPI.BACKEND_ERROR_KEY;
 import static se.leap.bitmaskclient.ProviderAPI.BACKEND_ERROR_MESSAGE;
-import static se.leap.bitmaskclient.ProviderSetupFailedDialog.DOWNLOAD_ERRORS.ERROR_CERTIFICATE_PINNING;
-import static se.leap.bitmaskclient.ProviderSetupFailedDialog.DOWNLOAD_ERRORS.ERROR_CORRUPTED_PROVIDER_JSON;
-import static se.leap.bitmaskclient.ProviderSetupFailedDialog.DOWNLOAD_ERRORS.ERROR_INVALID_CERTIFICATE;
-import static se.leap.bitmaskclient.ProviderAPI.CORRECTLY_DOWNLOADED_VPN_CERTIFICATE;
 import static se.leap.bitmaskclient.ProviderAPI.CORRECTLY_DOWNLOADED_EIP_SERVICE;
-import static se.leap.bitmaskclient.ProviderAPI.DOWNLOAD_VPN_CERTIFICATE;
+import static se.leap.bitmaskclient.ProviderAPI.CORRECTLY_DOWNLOADED_VPN_CERTIFICATE;
 import static se.leap.bitmaskclient.ProviderAPI.DOWNLOAD_SERVICE_JSON;
+import static se.leap.bitmaskclient.ProviderAPI.DOWNLOAD_VPN_CERTIFICATE;
 import static se.leap.bitmaskclient.ProviderAPI.ERRORID;
 import static se.leap.bitmaskclient.ProviderAPI.ERRORS;
 import static se.leap.bitmaskclient.ProviderAPI.FAILED_LOGIN;
 import static se.leap.bitmaskclient.ProviderAPI.FAILED_SIGNUP;
-import static se.leap.bitmaskclient.ProviderAPI.INCORRECTLY_DOWNLOADED_VPN_CERTIFICATE;
 import static se.leap.bitmaskclient.ProviderAPI.INCORRECTLY_DOWNLOADED_EIP_SERVICE;
+import static se.leap.bitmaskclient.ProviderAPI.INCORRECTLY_DOWNLOADED_VPN_CERTIFICATE;
 import static se.leap.bitmaskclient.ProviderAPI.LOGOUT_FAILED;
 import static se.leap.bitmaskclient.ProviderAPI.LOG_IN;
 import static se.leap.bitmaskclient.ProviderAPI.LOG_OUT;
@@ -90,15 +88,18 @@ import static se.leap.bitmaskclient.ProviderAPI.SUCCESSFUL_LOGIN;
 import static se.leap.bitmaskclient.ProviderAPI.SUCCESSFUL_LOGOUT;
 import static se.leap.bitmaskclient.ProviderAPI.SUCCESSFUL_SIGNUP;
 import static se.leap.bitmaskclient.ProviderAPI.UPDATE_PROVIDER_DETAILS;
+import static se.leap.bitmaskclient.ProviderAPI.USER_MESSAGE;
+import static se.leap.bitmaskclient.ProviderSetupFailedDialog.DOWNLOAD_ERRORS.ERROR_CERTIFICATE_PINNING;
+import static se.leap.bitmaskclient.ProviderSetupFailedDialog.DOWNLOAD_ERRORS.ERROR_CORRUPTED_PROVIDER_JSON;
+import static se.leap.bitmaskclient.ProviderSetupFailedDialog.DOWNLOAD_ERRORS.ERROR_INVALID_CERTIFICATE;
 import static se.leap.bitmaskclient.R.string.certificate_error;
-import static se.leap.bitmaskclient.R.string.switch_provider_menu_option;
-import static se.leap.bitmaskclient.R.string.vpn_certificate_is_invalid;
 import static se.leap.bitmaskclient.R.string.error_io_exception_user_message;
 import static se.leap.bitmaskclient.R.string.error_json_exception_user_message;
 import static se.leap.bitmaskclient.R.string.error_no_such_algorithm_exception_user_message;
 import static se.leap.bitmaskclient.R.string.malformed_url;
 import static se.leap.bitmaskclient.R.string.server_unreachable_message;
 import static se.leap.bitmaskclient.R.string.service_is_down_error;
+import static se.leap.bitmaskclient.R.string.vpn_certificate_is_invalid;
 import static se.leap.bitmaskclient.R.string.warning_corrupted_provider_cert;
 import static se.leap.bitmaskclient.R.string.warning_corrupted_provider_details;
 import static se.leap.bitmaskclient.R.string.warning_expired_provider_cert;
@@ -290,7 +291,7 @@ public abstract class ProviderApiManagerBase {
         JSONObject stepResult = null;
         OkHttpClient okHttpClient = clientGenerator.initSelfSignedCAHttpClient(provider.getCaCert(), stepResult);
         if (okHttpClient == null) {
-            return authFailedNotification(stepResult, username);
+            return backendErrorNotification(stepResult, username);
         }
 
         LeapSRPSession client = new LeapSRPSession(username, password);
@@ -302,7 +303,7 @@ public abstract class ProviderApiManagerBase {
 
         Bundle result = new Bundle();
         if (api_result.has(ERRORS) || api_result.has(BACKEND_ERROR_KEY))
-            result = authFailedNotification(api_result, username);
+            result = backendErrorNotification(api_result, username);
         else {
             result.putString(CREDENTIALS_USERNAME, username);
             result.putString(CREDENTIALS_PASSWORD, password);
@@ -349,7 +350,7 @@ public abstract class ProviderApiManagerBase {
 
         OkHttpClient okHttpClient = clientGenerator.initSelfSignedCAHttpClient(provider.getCaCert(), stepResult);
         if (okHttpClient == null) {
-            return authFailedNotification(stepResult, username);
+            return backendErrorNotification(stepResult, username);
         }
 
         LeapSRPSession client = new LeapSRPSession(username, password);
@@ -367,15 +368,15 @@ public abstract class ProviderApiManagerBase {
                 if (client.verify(M2)) {
                     result.putBoolean(BROADCAST_RESULT_KEY, true);
                 } else {
-                    authFailedNotification(step_result, username);
+                    backendErrorNotification(step_result, username);
                 }
             } else {
                 result.putBoolean(BROADCAST_RESULT_KEY, false);
                 result.putString(CREDENTIALS_USERNAME, username);
-                result.putString(resources.getString(R.string.user_message), resources.getString(R.string.error_srp_math_error_user_message));
+                result.putString(USER_MESSAGE, resources.getString(R.string.error_srp_math_error_user_message));
             }
         } catch (JSONException e) {
-            result = authFailedNotification(step_result, username);
+            result = backendErrorNotification(step_result, username);
             e.printStackTrace();
         }
 
@@ -391,7 +392,7 @@ public abstract class ProviderApiManagerBase {
         return true;
     }
 
-    private Bundle authFailedNotification(JSONObject result, String username) {
+    private Bundle backendErrorNotification(JSONObject result, String username) {
         Bundle userNotificationBundle = new Bundle();
         if (result.has(ERRORS)) {
             Object baseErrorMessage = result.opt(ERRORS);
@@ -400,14 +401,14 @@ public abstract class ProviderApiManagerBase {
                     JSONObject errorMessage = result.getJSONObject(ERRORS);
                     String errorType = errorMessage.keys().next().toString();
                     String message = errorMessage.get(errorType).toString();
-                    userNotificationBundle.putString(resources.getString(R.string.user_message), message);
-                } catch (JSONException e) {
+                    userNotificationBundle.putString(USER_MESSAGE, message);
+                } catch (JSONException | NoSuchElementException | NullPointerException e) {
                     e.printStackTrace();
                 }
             } else if (baseErrorMessage instanceof String) {
                 try {
                     String errorMessage = result.getString(ERRORS);
-                    userNotificationBundle.putString(resources.getString(R.string.user_message), errorMessage);
+                    userNotificationBundle.putString(USER_MESSAGE, errorMessage);
                 } catch (JSONException e) {
                     e.printStackTrace();
                 }
@@ -418,7 +419,7 @@ public abstract class ProviderApiManagerBase {
                 if (result.has(BACKEND_ERROR_MESSAGE)) {
                     backendErrorMessage = resources.getString(R.string.error) + result.getString(BACKEND_ERROR_MESSAGE);
                 }
-                userNotificationBundle.putString(resources.getString(R.string.user_message), backendErrorMessage);
+                userNotificationBundle.putString(USER_MESSAGE, backendErrorMessage);
             } catch (JSONException e) {
                 e.printStackTrace();
             }
@@ -431,7 +432,7 @@ public abstract class ProviderApiManagerBase {
         return userNotificationBundle;
     }
 
-    void sendToReceiverOrBroadcast(ResultReceiver receiver, int resultCode, Bundle resultData, Provider provider) {
+    private void sendToReceiverOrBroadcast(ResultReceiver receiver, int resultCode, Bundle resultData, Provider provider) {
         if (resultData == null || resultData == Bundle.EMPTY) {
             resultData = new Bundle();
         }
diff --git a/app/src/main/java/se/leap/bitmaskclient/ProviderCredentialsBaseActivity.java b/app/src/main/java/se/leap/bitmaskclient/ProviderCredentialsBaseActivity.java
index d41be512..e6877756 100644
--- a/app/src/main/java/se/leap/bitmaskclient/ProviderCredentialsBaseActivity.java
+++ b/app/src/main/java/se/leap/bitmaskclient/ProviderCredentialsBaseActivity.java
@@ -30,6 +30,7 @@ import org.json.JSONException;
 import butterknife.InjectView;
 import butterknife.OnClick;
 import se.leap.bitmaskclient.Constants.CREDENTIAL_ERRORS;
+import se.leap.bitmaskclient.eip.EipCommand;
 import se.leap.bitmaskclient.userstatus.User;
 
 import static android.view.View.GONE;
@@ -41,9 +42,12 @@ import static se.leap.bitmaskclient.Constants.BROADCAST_RESULT_KEY;
 import static se.leap.bitmaskclient.Constants.CREDENTIALS_PASSWORD;
 import static se.leap.bitmaskclient.Constants.CREDENTIALS_USERNAME;
 import static se.leap.bitmaskclient.Constants.PROVIDER_KEY;
+import static se.leap.bitmaskclient.ProviderAPI.BACKEND_ERROR_KEY;
 import static se.leap.bitmaskclient.ProviderAPI.DOWNLOAD_VPN_CERTIFICATE;
+import static se.leap.bitmaskclient.ProviderAPI.ERRORS;
 import static se.leap.bitmaskclient.ProviderAPI.LOG_IN;
 import static se.leap.bitmaskclient.ProviderAPI.SIGN_UP;
+import static se.leap.bitmaskclient.ProviderAPI.USER_MESSAGE;
 
 /**
  * Base Activity for activities concerning a provider interaction
@@ -59,7 +63,6 @@ public abstract class ProviderCredentialsBaseActivity extends ConfigWizardBaseAc
 
     final private static String SHOWING_FORM = "SHOWING_FORM";
     final private static String PERFORMING_ACTION = "PERFORMING_ACTION";
-    final public static String USER_MESSAGE = "USER_MESSAGE";
     final private static String USERNAME_ERROR = "USERNAME_ERROR";
     final private static String PASSWORD_ERROR = "PASSWORD_ERROR";
     final private static String PASSWORD_VERIFICATION_ERROR = "PASSWORD_VERIFICATION_ERROR";
@@ -344,8 +347,8 @@ public abstract class ProviderCredentialsBaseActivity extends ConfigWizardBaseAc
         if (arguments.containsKey(CREDENTIAL_ERRORS.USERNAME_MISSING.toString())) {
             usernameError.setError(getString(R.string.username_ask));
         }
-        if (arguments.containsKey(getString(R.string.user_message))) {
-            String userMessageString = arguments.getString(getString(R.string.user_message));
+        if (arguments.containsKey(USER_MESSAGE)) {
+            String userMessageString = arguments.getString(USER_MESSAGE);
             try {
                  userMessageString = new JSONArray(userMessageString).getString(0);
             } catch (JSONException e) {
@@ -395,6 +398,10 @@ public abstract class ProviderCredentialsBaseActivity extends ConfigWizardBaseAc
 
             switch (resultCode) {
                 case ProviderAPI.SUCCESSFUL_SIGNUP:
+                    String password = resultData.getString(CREDENTIALS_PASSWORD);
+                    String username = resultData.getString(CREDENTIALS_USERNAME);
+                    login(username, password);
+                    break;
                 case ProviderAPI.SUCCESSFUL_LOGIN:
                     downloadVpnCertificate(handledProvider);
                     break;
@@ -403,12 +410,11 @@ public abstract class ProviderCredentialsBaseActivity extends ConfigWizardBaseAc
                     handleReceivedErrors((Bundle) intent.getParcelableExtra(BROADCAST_RESULT_KEY));
                     break;
 
+                case ProviderAPI.INCORRECTLY_DOWNLOADED_VPN_CERTIFICATE:
+                    // error handling takes place in MainActivity
                 case ProviderAPI.CORRECTLY_DOWNLOADED_VPN_CERTIFICATE:
                     successfullyFinished(handledProvider);
                     break;
-                case ProviderAPI.INCORRECTLY_DOWNLOADED_VPN_CERTIFICATE:
-                    // TODO activity.setResult(RESULT_CANCELED);
-                    break;
             }
         }
     }