From 270b8af39fc952eda960782bf4a83c0f22773062 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 8 Mar 2019 17:14:03 -0300 Subject: [PATCH] [android] add review comments for panic induced account deletion --- .../account/AndroidAccountManager.java | 8 ++++++-- .../lifecycle/LifecycleManagerImpl.java | 1 - .../briar/android/activity/BriarActivity.java | 6 ++++-- .../android/controller/BriarController.java | 5 ++++- .../controller/BriarControllerImpl.java | 10 +++++++++- .../android/navdrawer/NavDrawerActivity.java | 4 ++-- .../android/panic/PanicResponderActivity.java | 20 +++---------------- 7 files changed, 28 insertions(+), 26 deletions(-) diff --git a/bramble-android/src/main/java/org/briarproject/bramble/account/AndroidAccountManager.java b/bramble-android/src/main/java/org/briarproject/bramble/account/AndroidAccountManager.java index 924fe4a41..b2745aa8c 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/account/AndroidAccountManager.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/account/AndroidAccountManager.java @@ -93,10 +93,14 @@ class AndroidAccountManager extends AccountManagerImpl LOG.warning("Could not clear shared preferences"); } // Delete files, except lib and shared_prefs directories + HashSet files = new HashSet<>(); File dataDir = new File(appContext.getApplicationInfo().dataDir); - HashSet files = new HashSet<>(asList(dataDir.listFiles())); - if (files.isEmpty()) { + @Nullable + File[] fileArray = dataDir.listFiles(); + if (fileArray == null) { LOG.warning("Could not list files in app data dir"); + } else { + files.addAll(asList(fileArray)); } files.add(appContext.getFilesDir()); files.add(appContext.getCacheDir()); diff --git a/bramble-core/src/main/java/org/briarproject/bramble/lifecycle/LifecycleManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/lifecycle/LifecycleManagerImpl.java index 67791f5e3..da55a780c 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/lifecycle/LifecycleManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/lifecycle/LifecycleManagerImpl.java @@ -168,7 +168,6 @@ class LifecycleManagerImpl implements LifecycleManager, MigrationListener { @Override public void stopServices() { - if (state == STOPPING) return; try { startStopSemaphore.acquire(); } catch (InterruptedException e) { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/activity/BriarActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/activity/BriarActivity.java index 7cbd63944..917dce9b6 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/activity/BriarActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/activity/BriarActivity.java @@ -185,13 +185,15 @@ public abstract class BriarActivity extends BaseActivity { b.show(); } - protected void signOut(boolean removeFromRecentApps) { + protected void signOut(boolean removeFromRecentApps, + boolean deleteAccount) { if (briarController.accountSignedIn()) { // Don't use UiResultHandler because we want the result even if // this activity has been destroyed briarController.signOut(result -> runOnUiThread( - () -> exit(removeFromRecentApps))); + () -> exit(removeFromRecentApps)), deleteAccount); } else { + if (deleteAccount) briarController.deleteAccount(); exit(removeFromRecentApps); } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/controller/BriarController.java b/briar-android/src/main/java/org/briarproject/briar/android/controller/BriarController.java index e4e60c692..c86e6f905 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/controller/BriarController.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/controller/BriarController.java @@ -16,5 +16,8 @@ public interface BriarController extends ActivityLifecycleController { void doNotAskAgainForDozeWhiteListing(); - void signOut(ResultHandler eventHandler); + void signOut(ResultHandler eventHandler, boolean deleteAccount); + + void deleteAccount(); + } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/controller/BriarControllerImpl.java b/briar-android/src/main/java/org/briarproject/briar/android/controller/BriarControllerImpl.java index 1271f4d21..d492f0472 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/controller/BriarControllerImpl.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/controller/BriarControllerImpl.java @@ -120,7 +120,8 @@ public class BriarControllerImpl implements BriarController { } @Override - public void signOut(ResultHandler eventHandler) { + public void signOut(ResultHandler eventHandler, + boolean deleteAccount) { new Thread(() -> { try { // Wait for the service to finish starting up @@ -134,11 +135,18 @@ public class BriarControllerImpl implements BriarController { service.waitForShutdown(); } catch (InterruptedException e) { LOG.warning("Interrupted while waiting for service"); + } finally { + if (deleteAccount) accountManager.deleteAccount(); } eventHandler.onResult(null); }).start(); } + @Override + public void deleteAccount() { + accountManager.deleteAccount(); + } + private void unbindService() { if (bound) activity.unbindService(serviceConnection); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/navdrawer/NavDrawerActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/navdrawer/NavDrawerActivity.java index 67d6d8195..49a49d5bc 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/navdrawer/NavDrawerActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/navdrawer/NavDrawerActivity.java @@ -109,7 +109,7 @@ public class NavDrawerActivity extends BriarActivity implements } else if (intent.getBooleanExtra(INTENT_BLOGS, false)) { startFragment(FeedFragment.newInstance(), R.id.nav_btn_blogs); } else if (intent.getBooleanExtra(INTENT_SIGN_OUT, false)) { - signOut(false); + signOut(false, false); } setIntent(null); } @@ -279,7 +279,7 @@ public class NavDrawerActivity extends BriarActivity implements private void signOut() { drawerLayout.setDrawerLockMode(LOCK_MODE_LOCKED_CLOSED); - signOut(false); + signOut(false, false); finish(); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/panic/PanicResponderActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/panic/PanicResponderActivity.java index 7cec90c43..dfa9fa079 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/panic/PanicResponderActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/panic/PanicResponderActivity.java @@ -5,7 +5,6 @@ import android.content.SharedPreferences; import android.os.Bundle; import android.support.v7.preference.PreferenceManager; -import org.briarproject.bramble.api.account.AccountManager; import org.briarproject.bramble.api.lifecycle.LifecycleManager; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; @@ -24,8 +23,6 @@ import info.guardianproject.panic.PanicResponder; import info.guardianproject.trustedintents.TrustedIntents; import static android.os.Build.VERSION.SDK_INT; -import static java.util.logging.Level.WARNING; -import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.briar.android.panic.PanicPreferencesFragment.KEY_LOCK; import static org.briarproject.briar.android.panic.PanicPreferencesFragment.KEY_PURGE; @@ -39,8 +36,6 @@ public class PanicResponderActivity extends BriarActivity { @Inject protected LifecycleManager lifecycleManager; @Inject - protected AccountManager accountManager; - @Inject protected AndroidExecutor androidExecutor; @Override @@ -74,7 +69,7 @@ public class PanicResponderActivity extends BriarActivity { // non-destructive actions are allowed by non-connected trusted apps if (sharedPref.getBoolean(KEY_LOCK, true)) { LOG.info("Signing out..."); - signOut(true); + signOut(true, false); } } } @@ -93,17 +88,8 @@ public class PanicResponderActivity extends BriarActivity { private void deleteAllData() { androidExecutor.runOnBackgroundThread(() -> { - lifecycleManager.stopServices(); - try { - lifecycleManager.waitForShutdown(); - } catch (InterruptedException e) { - logException(LOG, WARNING, e); - } finally { - accountManager.deleteAccount(); - // nothing left to do after everything is deleted, so sign out - LOG.info("Signing out..."); - signOut(true); - } + LOG.info("Signing out..."); + signOut(true, true); }); } }