From 9e6fe0e215e32343b38cdf20080de209a31287dd Mon Sep 17 00:00:00 2001 From: Fup Duck Date: Fri, 9 Feb 2018 12:46:06 +0100 Subject: 8827 - merge request discussions * add NullPointer checks to EipFragment * add Provider to DownloadFailedDialog * remove unused code * store certificates for pinning in SharedPreferences --- .../java/se/leap/bitmaskclient/ConfigHelper.java | 10 ++ .../leap/bitmaskclient/DownloadFailedDialog.java | 36 ++-- .../java/se/leap/bitmaskclient/EipFragment.java | 186 +++++++++++++-------- .../leap/bitmaskclient/OkHttpClientGenerator.java | 5 - .../leap/bitmaskclient/ProviderApiManagerBase.java | 4 +- .../bitmaskclient/ProviderListBaseActivity.java | 7 +- 6 files changed, 150 insertions(+), 98 deletions(-) (limited to 'app/src/main') diff --git a/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java b/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java index 5a97624d..2e9e1897 100644 --- a/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java +++ b/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java @@ -16,6 +16,7 @@ */ package se.leap.bitmaskclient; +import android.annotation.SuppressLint; import android.content.SharedPreferences; import android.support.annotation.NonNull; import android.support.annotation.Nullable; @@ -348,6 +349,7 @@ public class ConfigHelper { } } + // TODO: replace commit with apply after refactoring EIP public static void storeProviderInPreferences(SharedPreferences preferences, Provider provider) { preferences.edit().putBoolean(PROVIDER_CONFIGURED, true). putString(Provider.MAIN_URL, provider.getMainUrlString()). @@ -355,6 +357,14 @@ public class ConfigHelper { putString(Provider.CA_CERT, provider.getCaCert()). putString(PROVIDER_KEY, provider.getEipServiceJsonString()). commit(); + + String providerDomain = provider.getDomain(); + preferences.edit().putBoolean(PROVIDER_CONFIGURED, true). + putString(Provider.MAIN_URL + "." + providerDomain, provider.getMainUrlString()). + putString(Provider.KEY + "." + providerDomain, provider.getDefinitionString()). + putString(Provider.CA_CERT + "." + providerDomain, provider.getCaCert()). + putString(PROVIDER_KEY + "." + providerDomain, provider.getEipServiceJsonString()). + apply(); } diff --git a/app/src/main/java/se/leap/bitmaskclient/DownloadFailedDialog.java b/app/src/main/java/se/leap/bitmaskclient/DownloadFailedDialog.java index 1a0c85ad..8d56cdf8 100644 --- a/app/src/main/java/se/leap/bitmaskclient/DownloadFailedDialog.java +++ b/app/src/main/java/se/leap/bitmaskclient/DownloadFailedDialog.java @@ -21,6 +21,7 @@ import android.app.Dialog; import android.content.Context; import android.content.DialogInterface; import android.os.Bundle; +import android.support.annotation.NonNull; import android.support.v4.app.DialogFragment; import org.json.JSONObject; @@ -38,8 +39,11 @@ import static se.leap.bitmaskclient.ProviderAPI.ERRORS; public class DownloadFailedDialog extends DialogFragment { public static String TAG = "downloaded_failed_dialog"; - private String reason_to_fail; + private String reasonToFail; private DOWNLOAD_ERRORS downloadError = DEFAULT; + + private Provider provider; + public enum DOWNLOAD_ERRORS { DEFAULT, ERROR_CORRUPTED_PROVIDER_JSON, @@ -50,39 +54,41 @@ public class DownloadFailedDialog extends DialogFragment { /** * @return a new instance of this DialogFragment. */ - public static DialogFragment newInstance(String reason_to_fail) { - DownloadFailedDialog dialog_fragment = new DownloadFailedDialog(); - dialog_fragment.reason_to_fail = reason_to_fail; - return dialog_fragment; + public static DialogFragment newInstance(Provider provider, String reasonToFail) { + DownloadFailedDialog dialogFragment = new DownloadFailedDialog(); + dialogFragment.reasonToFail = reasonToFail; + dialogFragment.provider = provider; + return dialogFragment; } /** * @return a new instance of this DialogFragment. */ - public static DialogFragment newInstance(JSONObject errorJson) { - DownloadFailedDialog dialog_fragment = new DownloadFailedDialog(); + public static DialogFragment newInstance(Provider provider, JSONObject errorJson) { + DownloadFailedDialog dialogFragment = new DownloadFailedDialog(); + dialogFragment.provider = provider; try { if (errorJson.has(ERRORS)) { - dialog_fragment.reason_to_fail = errorJson.getString(ERRORS); + dialogFragment.reasonToFail = errorJson.getString(ERRORS); } else { //default error msg - dialog_fragment.reason_to_fail = dialog_fragment.getString(R.string.error_io_exception_user_message); + dialogFragment.reasonToFail = dialogFragment.getString(R.string.error_io_exception_user_message); } if (errorJson.has(ERRORID)) { - dialog_fragment.downloadError = valueOf(errorJson.getString(ERRORID)); + dialogFragment.downloadError = valueOf(errorJson.getString(ERRORID)); } } catch (Exception e) { e.printStackTrace(); - dialog_fragment.reason_to_fail = dialog_fragment.getString(R.string.error_io_exception_user_message); + dialogFragment.reasonToFail = dialogFragment.getString(R.string.error_io_exception_user_message); } - return dialog_fragment; + return dialogFragment; } @Override public Dialog onCreateDialog(Bundle savedInstanceState) { AlertDialog.Builder builder = new AlertDialog.Builder(getActivity()); - builder.setMessage(reason_to_fail) + builder.setMessage(reasonToFail) .setNegativeButton(R.string.cancel, new DialogInterface.OnClickListener() { public void onClick(DialogInterface dialog, int id) { interface_with_ConfigurationWizard.cancelSettingUpProvider(); @@ -113,7 +119,7 @@ switch (downloadError) { builder.setPositiveButton(R.string.retry, new DialogInterface.OnClickListener() { public void onClick(DialogInterface dialog, int id) { dismiss(); - interface_with_ConfigurationWizard.retrySetUpProvider(null); + interface_with_ConfigurationWizard.retrySetUpProvider(provider); } }); break; @@ -124,7 +130,7 @@ switch (downloadError) { } public interface DownloadFailedDialogInterface { - void retrySetUpProvider(Provider provider); + void retrySetUpProvider(@NonNull Provider provider); void cancelSettingUpProvider(); diff --git a/app/src/main/java/se/leap/bitmaskclient/EipFragment.java b/app/src/main/java/se/leap/bitmaskclient/EipFragment.java index 2f7977dd..ceae6706 100644 --- a/app/src/main/java/se/leap/bitmaskclient/EipFragment.java +++ b/app/src/main/java/se/leap/bitmaskclient/EipFragment.java @@ -33,6 +33,7 @@ import android.os.Handler; import android.os.IBinder; import android.os.RemoteException; import android.os.ResultReceiver; +import android.support.annotation.NonNull; import android.support.v4.app.Fragment; import android.support.v7.widget.AppCompatImageView; import android.util.Log; @@ -140,16 +141,19 @@ public class EipFragment extends Fragment implements Observer { public void onAttach(Context context) { super.onAttach(context); Bundle arguments = getArguments(); - if (arguments != null) { - provider = getArguments().getParcelable(PROVIDER_KEY); - if (provider == null) { - getActivity().startActivityForResult(new Intent(getActivity(), ProviderListActivity.class), REQUEST_CODE_SWITCH_PROVIDER); + Activity activity = getActivity(); + if (activity != null) { + if (arguments != null) { + provider = arguments.getParcelable(PROVIDER_KEY); + if (provider == null) { + activity.startActivityForResult(new Intent(activity, ProviderListActivity.class), REQUEST_CODE_SWITCH_PROVIDER); + } else { + Log.d(TAG, provider.getName() + " configured as provider"); + } } else { - Log.d(TAG, provider.getName() + " configured as provider"); + Log.e(TAG, "no provider given - starting ProviderListActivity"); + activity.startActivityForResult(new Intent(activity, ProviderListActivity.class), REQUEST_CODE_SWITCH_PROVIDER); } - } else { - Log.e(TAG, "no provider given - starting ProviderListActivity"); - getActivity().startActivityForResult(new Intent(getActivity(), ProviderListActivity.class), REQUEST_CODE_SWITCH_PROVIDER); } } @@ -160,11 +164,16 @@ public class EipFragment extends Fragment implements Observer { eipReceiver = new EIPReceiver(new Handler()); eipBroadcastReceiver = new EIPBroadcastReceiver(); providerAPIResultReceiver = new ProviderAPIResultReceiver(new Handler(), new EipFragmentReceiver()); - preferences = getActivity().getSharedPreferences(SHARED_PREFERENCES, Context.MODE_PRIVATE); + Activity activity = getActivity(); + if (activity != null) { + preferences = getActivity().getSharedPreferences(SHARED_PREFERENCES, Context.MODE_PRIVATE); + } else { + Log.e(TAG, "activity is null in onCreate - no preferences set!"); + } } @Override - public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) { + public View onCreateView(@NonNull LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) { eipStatus.addObserver(this); View view = inflater.inflate(R.layout.eip_service_fragment, container, false); ButterKnife.inject(this, view); @@ -189,8 +198,12 @@ public class EipFragment extends Fragment implements Observer { @Override public void onPause() { super.onPause(); - getActivity().unbindService(openVpnConnection); - getActivity().unregisterReceiver(eipBroadcastReceiver); + + Activity activity = getActivity(); + if (activity != null) { + getActivity().unbindService(openVpnConnection); + getActivity().unregisterReceiver(eipBroadcastReceiver); + } Log.d(TAG, "broadcast unregistered"); } @@ -201,7 +214,7 @@ public class EipFragment extends Fragment implements Observer { } @Override - public void onSaveInstanceState(Bundle outState) { + public void onSaveInstanceState(@NonNull Bundle outState) { outState.putBoolean(IS_CONNECTED, eipStatus.isConnected()); super.onSaveInstanceState(outState); } @@ -238,7 +251,10 @@ public class EipFragment extends Fragment implements Observer { else if (canLogInToStartEIP()) { wantsToConnect = true; Intent intent = new Intent(getContext(), LoginActivity.class); - getActivity().startActivityForResult(intent, REQUEST_CODE_LOG_IN); + Activity activity = getActivity(); + if (activity != null) { + activity.startActivityForResult(intent, REQUEST_CODE_LOG_IN); + } } else { Log.d(TAG, "WHAT IS GOING ON HERE?!"); // TODO: implement a fallback: check if vpncertificate was not downloaded properly or give @@ -268,21 +284,25 @@ public class EipFragment extends Fragment implements Observer { private void askPendingStartCancellation() { Activity activity = getActivity(); - AlertDialog.Builder alertBuilder = new AlertDialog.Builder(getActivity()); - alertBuilder.setTitle(activity.getString(R.string.eip_cancel_connect_title)) - .setMessage(activity.getString(R.string.eip_cancel_connect_text)) - .setPositiveButton((android.R.string.yes), new DialogInterface.OnClickListener() { - @Override - public void onClick(DialogInterface dialog, int which) { - stopEipIfPossible(); - } - }) - .setNegativeButton(activity.getString(android.R.string.no), new DialogInterface.OnClickListener() { - @Override - public void onClick(DialogInterface dialog, int which) { - } - }) - .show(); + if (activity != null) { + AlertDialog.Builder alertBuilder = new AlertDialog.Builder(getActivity()); + alertBuilder.setTitle(activity.getString(R.string.eip_cancel_connect_title)) + .setMessage(activity.getString(R.string.eip_cancel_connect_text)) + .setPositiveButton((android.R.string.yes), new DialogInterface.OnClickListener() { + @Override + public void onClick(DialogInterface dialog, int which) { + stopEipIfPossible(); + } + }) + .setNegativeButton(activity.getString(android.R.string.no), new DialogInterface.OnClickListener() { + @Override + public void onClick(DialogInterface dialog, int which) { + } + }) + .show(); + } else { + Log.e(TAG, "activity is null when asking to cancel"); + } } public void startEipFromScratch() { @@ -302,9 +322,14 @@ public class EipFragment extends Fragment implements Observer { private void stopBlockingVpn() { Log.d(TAG, "stop VoidVpn!"); Activity activity = getActivity(); - Intent stopVoidVpnIntent = new Intent(activity, VoidVpnService.class); - stopVoidVpnIntent.setAction(EIP_ACTION_STOP_BLOCKING_VPN); - activity.startService(stopVoidVpnIntent); + if (activity != null) { + Intent stopVoidVpnIntent = new Intent(activity, VoidVpnService.class); + stopVoidVpnIntent.setAction(EIP_ACTION_STOP_BLOCKING_VPN); + activity.startService(stopVoidVpnIntent); + } else { + Log.e(TAG, "activity is null when trying to stop blocking vpn"); + // TODO what to do if not stopping void vpn? + } } private void disconnect() { @@ -325,21 +350,25 @@ public class EipFragment extends Fragment implements Observer { protected void askToStopEIP() { Activity activity = getActivity(); - AlertDialog.Builder alertBuilder = new AlertDialog.Builder(activity); - alertBuilder.setTitle(activity.getString(R.string.eip_cancel_connect_title)) - .setMessage(activity.getString(R.string.eip_warning_browser_inconsistency)) - .setPositiveButton((android.R.string.yes), new DialogInterface.OnClickListener() { - @Override - public void onClick(DialogInterface dialog, int which) { - stopEipIfPossible(); - } - }) - .setNegativeButton(activity.getString(android.R.string.no), new DialogInterface.OnClickListener() { - @Override - public void onClick(DialogInterface dialog, int which) { - } - }) - .show(); + if (activity != null) { + AlertDialog.Builder alertBuilder = new AlertDialog.Builder(activity); + alertBuilder.setTitle(activity.getString(R.string.eip_cancel_connect_title)) + .setMessage(activity.getString(R.string.eip_warning_browser_inconsistency)) + .setPositiveButton((android.R.string.yes), new DialogInterface.OnClickListener() { + @Override + public void onClick(DialogInterface dialog, int which) { + stopEipIfPossible(); + } + }) + .setNegativeButton(activity.getString(android.R.string.no), new DialogInterface.OnClickListener() { + @Override + public void onClick(DialogInterface dialog, int which) { + } + }) + .show(); + } else { + Log.e(TAG, "activity is null when asking to stop EIP"); + } } @Override @@ -362,25 +391,29 @@ public class EipFragment extends Fragment implements Observer { private void handleNewState() { Activity activity = getActivity(); - if (eipStatus.isConnecting()) { - mainButton.setText(activity.getString(android.R.string.cancel)); - key.setImageResource(R.drawable.vpn_connecting); - routedText.setVisibility(GONE); - vpnRoute.setVisibility(GONE); - colorBackgroundALittle(); - } else if (eipStatus.isConnected() || isOpenVpnRunningWithoutNetwork()) { - mainButton.setText(activity.getString(R.string.vpn_button_turn_off)); - key.setImageResource(R.drawable.vpn_connected); - routedText.setVisibility(VISIBLE); - vpnRoute.setVisibility(VISIBLE); - vpnRoute.setText(ConfigHelper.getProviderName(preferences)); - colorBackground(); + if (activity != null) { + if (eipStatus.isConnecting()) { + mainButton.setText(activity.getString(android.R.string.cancel)); + key.setImageResource(R.drawable.vpn_connecting); + routedText.setVisibility(GONE); + vpnRoute.setVisibility(GONE); + colorBackgroundALittle(); + } else if (eipStatus.isConnected() || isOpenVpnRunningWithoutNetwork()) { + mainButton.setText(activity.getString(R.string.vpn_button_turn_off)); + key.setImageResource(R.drawable.vpn_connected); + routedText.setVisibility(VISIBLE); + vpnRoute.setVisibility(VISIBLE); + vpnRoute.setText(ConfigHelper.getProviderName(preferences)); + colorBackground(); + } else { + mainButton.setText(activity.getString(R.string.vpn_button_turn_on)); + key.setImageResource(R.drawable.vpn_disconnected); + routedText.setVisibility(GONE); + vpnRoute.setVisibility(GONE); + greyscaleBackground(); + } } else { - mainButton.setText(activity.getString(R.string.vpn_button_turn_on)); - key.setImageResource(R.drawable.vpn_disconnected); - routedText.setVisibility(GONE); - vpnRoute.setVisibility(GONE); - greyscaleBackground(); + Log.e(TAG, "activity is null while trying to handle new state"); } } @@ -399,9 +432,13 @@ public class EipFragment extends Fragment implements Observer { private void bindOpenVpnService() { Activity activity = getActivity(); - Intent intent = new Intent(activity, OpenVPNService.class); - intent.setAction(OpenVPNService.START_SERVICE); - activity.bindService(intent, openVpnConnection, Context.BIND_AUTO_CREATE); + if (activity != null) { + Intent intent = new Intent(activity, OpenVPNService.class); + intent.setAction(OpenVPNService.START_SERVICE); + activity.bindService(intent, openVpnConnection, Context.BIND_AUTO_CREATE); + } else { + Log.e(TAG, "activity is null when binding OpenVpn"); + } } protected class EIPReceiver extends ResultReceiver { @@ -533,10 +570,15 @@ public class EipFragment extends Fragment implements Observer { } private void setUpBroadcastReceiver() { - IntentFilter updateIntentFilter = new IntentFilter(BROADCAST_EIP_EVENT); - updateIntentFilter.addCategory(Intent.CATEGORY_DEFAULT); - getActivity().registerReceiver(eipBroadcastReceiver, updateIntentFilter); - Log.d(TAG, "broadcast registered"); + Activity activity = getActivity(); + if (activity != null) { + IntentFilter updateIntentFilter = new IntentFilter(BROADCAST_EIP_EVENT); + updateIntentFilter.addCategory(Intent.CATEGORY_DEFAULT); + activity.registerReceiver(eipBroadcastReceiver, updateIntentFilter); + Log.d(TAG, "broadcast registered"); + } else { + Log.e(TAG, "activity null when setting up boradcat receiver"); + } } } diff --git a/app/src/main/java/se/leap/bitmaskclient/OkHttpClientGenerator.java b/app/src/main/java/se/leap/bitmaskclient/OkHttpClientGenerator.java index 6d554b0e..40b2ea7f 100644 --- a/app/src/main/java/se/leap/bitmaskclient/OkHttpClientGenerator.java +++ b/app/src/main/java/se/leap/bitmaskclient/OkHttpClientGenerator.java @@ -73,11 +73,6 @@ public class OkHttpClientGenerator { return initHttpClient(initError, caCert); } - public OkHttpClient initSelfSignedCAHttpClient(JSONObject initError, String certificate) { - return initHttpClient(initError, certificate); - } - - private OkHttpClient initHttpClient(JSONObject initError, String certificate) { try { TLSCompatSocketFactory sslCompatFactory; diff --git a/app/src/main/java/se/leap/bitmaskclient/ProviderApiManagerBase.java b/app/src/main/java/se/leap/bitmaskclient/ProviderApiManagerBase.java index 000dd164..6b6aa89d 100644 --- a/app/src/main/java/se/leap/bitmaskclient/ProviderApiManagerBase.java +++ b/app/src/main/java/se/leap/bitmaskclient/ProviderApiManagerBase.java @@ -66,7 +66,6 @@ import static se.leap.bitmaskclient.DownloadFailedDialog.DOWNLOAD_ERRORS.ERROR_C import static se.leap.bitmaskclient.DownloadFailedDialog.DOWNLOAD_ERRORS.ERROR_INVALID_CERTIFICATE; import static se.leap.bitmaskclient.ProviderAPI.CORRECTLY_DOWNLOADED_CERTIFICATE; import static se.leap.bitmaskclient.ProviderAPI.CORRECTLY_DOWNLOADED_EIP_SERVICE; -import static se.leap.bitmaskclient.ProviderAPI.CURRENT_PROGRESS; import static se.leap.bitmaskclient.ProviderAPI.DOWNLOAD_CERTIFICATE; import static se.leap.bitmaskclient.ProviderAPI.DOWNLOAD_EIP_SERVICE; import static se.leap.bitmaskclient.ProviderAPI.ERRORID; @@ -572,7 +571,7 @@ public abstract class ProviderApiManagerBase { JSONObject errorJson = new JSONObject(); String baseUrl = getApiUrl(providerDefinition); - OkHttpClient okHttpClient = clientGenerator.initSelfSignedCAHttpClient(errorJson, caCert); + OkHttpClient okHttpClient = clientGenerator.initSelfSignedCAHttpClient(caCert, errorJson); if (okHttpClient == null) { result.putString(ERRORS, errorJson.toString()); return false; @@ -706,7 +705,6 @@ public abstract class ProviderApiManagerBase { return setErrorResult(result, warning_corrupted_provider_cert, ERROR_CERTIFICATE_PINNING.toString()); } - if (!hasApiUrlExpectedDomain(providerDefinition, mainUrl)){ return setErrorResult(result, warning_corrupted_provider_details, ERROR_CORRUPTED_PROVIDER_JSON.toString()); } diff --git a/app/src/main/java/se/leap/bitmaskclient/ProviderListBaseActivity.java b/app/src/main/java/se/leap/bitmaskclient/ProviderListBaseActivity.java index b2daff82..6a76dd55 100644 --- a/app/src/main/java/se/leap/bitmaskclient/ProviderListBaseActivity.java +++ b/app/src/main/java/se/leap/bitmaskclient/ProviderListBaseActivity.java @@ -23,6 +23,7 @@ import android.content.Intent; import android.content.IntentFilter; import android.os.Bundle; import android.os.Handler; +import android.support.annotation.NonNull; import android.support.v4.app.DialogFragment; import android.support.v4.app.FragmentTransaction; import android.util.Log; @@ -102,7 +103,7 @@ public abstract class ProviderListBaseActivity extends ConfigWizardBaseActivity private boolean isActivityShowing; private String reasonToFail; - public abstract void retrySetUpProvider(Provider provider); + public abstract void retrySetUpProvider(@NonNull Provider provider); protected abstract void onItemSelectedLogic(); @@ -360,10 +361,10 @@ public abstract class ProviderListBaseActivity extends ConfigWizardBaseActivity DialogFragment newFragment; try { JSONObject errorJson = new JSONObject(reasonToFail); - newFragment = DownloadFailedDialog.newInstance(errorJson); + newFragment = DownloadFailedDialog.newInstance(provider, errorJson); } catch (JSONException e) { e.printStackTrace(); - newFragment = DownloadFailedDialog.newInstance(reasonToFail); + newFragment = DownloadFailedDialog.newInstance(provider, reasonToFail); } newFragment.show(fragmentTransaction, DownloadFailedDialog.TAG); } catch (IllegalStateException e) { -- cgit v1.2.3