From 3138213f3913a623163514d63d8c8f71a673e989 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 27 Apr 2022 08:38:43 -0300 Subject: [PATCH 1/5] Let MailboxManager#unPair() return a boolean for whether it could wipe the mailbox --- .../briarproject/bramble/api/mailbox/MailboxManager.java | 6 +++++- .../briarproject/bramble/mailbox/MailboxManagerImpl.java | 8 +++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxManager.java index 784fff21d..d2c88aebb 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxManager.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/mailbox/MailboxManager.java @@ -45,7 +45,11 @@ public interface MailboxManager { /** * Unpairs the owner's mailbox and tries to wipe it. * As this makes a network call, it should be run on the {@link IoExecutor}. + * + * @return true if we could wipe the mailbox, false if we couldn't. + * It is advised to inform the user to wipe the mailbox themselves, + * if we failed to wipe it. */ @IoExecutor - void unPair() throws DbException; + boolean unPair() throws DbException; } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java index d17401559..1671c0519 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java @@ -132,17 +132,19 @@ class MailboxManagerImpl implements MailboxManager { } @Override - public void unPair() throws DbException { + public boolean unPair() throws DbException { MailboxProperties properties = db.transactionWithNullableResult(true, mailboxSettingsManager::getOwnMailboxProperties); + boolean wasWiped; try { api.wipeMailbox(properties); + wasWiped = true; } catch (IOException | MailboxApi.ApiException e) { - // We wipe on a best-effort basis. - // If we can't do it, we still unpair. logException(LOG, WARNING, e); + wasWiped = false; } db.transaction(false, mailboxSettingsManager::removeOwnMailboxProperties); + return wasWiped; } } From 2411c82d9c41b987d1d9efb39463d017cfd9b0e7 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 27 Apr 2022 08:39:44 -0300 Subject: [PATCH 2/5] Remind user to wipe mailbox if it's unreachable when unpairing If we fail to tell the mailbox to wipe itself when unpairing, remind the user that they should wipe the mailbox next time they have access to it. --- .../briar/android/mailbox/MailboxActivity.java | 17 ++++++++++++++--- .../briar/android/mailbox/MailboxState.java | 9 +++++++++ .../briar/android/mailbox/MailboxViewModel.java | 4 ++-- briar-android/src/main/res/values/strings.xml | 2 ++ 4 files changed, 27 insertions(+), 5 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxActivity.java index 8c5f16b32..4f2bd00ea 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxActivity.java @@ -15,6 +15,7 @@ import org.briarproject.briar.android.fragment.FinalFragment; import javax.inject.Inject; import androidx.annotation.Nullable; +import androidx.appcompat.app.AlertDialog; import androidx.fragment.app.Fragment; import androidx.fragment.app.FragmentManager; import androidx.lifecycle.ViewModelProvider; @@ -53,7 +54,8 @@ public class MailboxActivity extends BriarActivity { viewModel.getPairingState().observeEvent(this, state -> { if (state instanceof MailboxState.NotSetup) { - onNotSetup(); + MailboxState.NotSetup s = (MailboxState.NotSetup) state; + onNotSetup(s.tellUserToWipeMailbox); } else if (state instanceof MailboxState.ShowDownload) { onShowDownload(); } else if (state instanceof MailboxState.ScanningQrCode) { @@ -95,8 +97,17 @@ public class MailboxActivity extends BriarActivity { } } - private void onNotSetup() { + private void onNotSetup(boolean tellUserToWipeMailbox) { progressBar.setVisibility(INVISIBLE); + if (tellUserToWipeMailbox) { + AlertDialog.Builder builder = + new AlertDialog.Builder(this, R.style.BriarDialogTheme); + builder.setTitle(R.string.mailbox_status_unlink_no_wipe_title); + builder.setMessage(R.string.mailbox_status_unlink_no_wipe_message); + builder.setNeutralButton(R.string.got_it, + (dialog, which) -> dialog.cancel()); + builder.show(); + } getSupportFragmentManager().beginTransaction() .replace(R.id.fragmentContainer, new SetupIntroFragment(), SetupIntroFragment.TAG) @@ -126,7 +137,7 @@ public class MailboxActivity extends BriarActivity { if (fm.getBackStackEntryCount() == 0) { // We re-launched into an existing state, // need to re-populate the back stack. - onNotSetup(); + onNotSetup(false); onShowDownload(); } Fragment f; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxState.java b/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxState.java index 4fcfdaf6a..532772103 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxState.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxState.java @@ -5,6 +5,15 @@ import org.briarproject.bramble.api.mailbox.MailboxPairingState; class MailboxState { static class NotSetup extends MailboxState { + final boolean tellUserToWipeMailbox; + + NotSetup() { + this(false); + } + + NotSetup(boolean tellUserToWipeMailbox) { + this.tellUserToWipeMailbox = tellUserToWipeMailbox; + } } static class ShowDownload extends MailboxState { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxViewModel.java index eeacb180e..e74e8fc6c 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxViewModel.java @@ -225,8 +225,8 @@ class MailboxViewModel extends DbViewModel void unlink() { ioExecutor.execute(() -> { try { - mailboxManager.unPair(); - pairingState.postEvent(new MailboxState.NotSetup()); + boolean wasWiped = mailboxManager.unPair(); + pairingState.postEvent(new MailboxState.NotSetup(!wasWiped)); } catch (DbException e) { handleException(e); } diff --git a/briar-android/src/main/res/values/strings.xml b/briar-android/src/main/res/values/strings.xml index ad98a1a7d..b7c3e3f1b 100644 --- a/briar-android/src/main/res/values/strings.xml +++ b/briar-android/src/main/res/values/strings.xml @@ -654,6 +654,8 @@ Unlink mailbox? Are you sure you want to unlink your Mailbox? If you unlink your Mailbox, you won\'t be able to receive messages while Briar is offline. + Your Mailbox has been unlinked + Next time you have access to your Mailbox device, please open the Mailbox app and tap the \"Unlink\" button to complete the process.\n\nIf you no longer have access to your Mailbox device, don\'t worry. Everything stored in your Mailbox is encrypted, so your data will remain secure even if you don\'t complete the process. Disappearing messages From 39c74f1363af29f1c9e777f8fcfb99a796b3708e Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 27 Apr 2022 11:57:39 -0300 Subject: [PATCH 3/5] Annotate MailboxApi with @NotNullByDefault --- .../java/org/briarproject/bramble/mailbox/MailboxApi.java | 2 ++ .../briarproject/bramble/mailbox/MailboxManagerImpl.java | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApi.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApi.java index 63c494f03..0ecaffbea 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApi.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxApi.java @@ -7,6 +7,7 @@ import org.briarproject.bramble.api.mailbox.MailboxAuthToken; import org.briarproject.bramble.api.mailbox.MailboxFileId; import org.briarproject.bramble.api.mailbox.MailboxFolderId; import org.briarproject.bramble.api.mailbox.MailboxProperties; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import java.io.File; import java.io.IOException; @@ -16,6 +17,7 @@ import java.util.List; import javax.annotation.Nonnull; import javax.annotation.concurrent.Immutable; +@NotNullByDefault interface MailboxApi { /** diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java index 1671c0519..da8d8d47d 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java @@ -102,6 +102,7 @@ class MailboxManagerImpl implements MailboxManager { try { MailboxProperties props = db.transactionWithNullableResult(true, mailboxSettingsManager::getOwnMailboxProperties); + if (props == null) throw new DbException(); success = api.checkStatus(props); } catch (DbException e) { logException(LOG, WARNING, e); @@ -135,6 +136,11 @@ class MailboxManagerImpl implements MailboxManager { public boolean unPair() throws DbException { MailboxProperties properties = db.transactionWithNullableResult(true, mailboxSettingsManager::getOwnMailboxProperties); + if (properties == null) { + // no more mailbox, that's strange but possible if called in quick + // succession, so let's return true this time + return true; + } boolean wasWiped; try { api.wipeMailbox(properties); From f05cbac20a8ae11458c4604a9cb80e12b4d557d0 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 27 Apr 2022 12:14:26 -0300 Subject: [PATCH 4/5] Use a new WasUnpaired state that shows a dialog over a blank fragment --- .../android/mailbox/MailboxActivity.java | 38 ++++++++++++------- .../briar/android/mailbox/MailboxState.java | 17 ++++----- .../android/mailbox/MailboxViewModel.java | 2 +- .../briar/android/view/BlankFragment.java | 12 ++++++ 4 files changed, 46 insertions(+), 23 deletions(-) create mode 100644 briar-android/src/main/java/org/briarproject/briar/android/view/BlankFragment.java diff --git a/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxActivity.java index 4f2bd00ea..528de5886 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxActivity.java @@ -11,6 +11,7 @@ import org.briarproject.briar.R; import org.briarproject.briar.android.activity.ActivityComponent; import org.briarproject.briar.android.activity.BriarActivity; import org.briarproject.briar.android.fragment.FinalFragment; +import org.briarproject.briar.android.view.BlankFragment; import javax.inject.Inject; @@ -54,8 +55,7 @@ public class MailboxActivity extends BriarActivity { viewModel.getPairingState().observeEvent(this, state -> { if (state instanceof MailboxState.NotSetup) { - MailboxState.NotSetup s = (MailboxState.NotSetup) state; - onNotSetup(s.tellUserToWipeMailbox); + onNotSetup(); } else if (state instanceof MailboxState.ShowDownload) { onShowDownload(); } else if (state instanceof MailboxState.ScanningQrCode) { @@ -70,6 +70,9 @@ public class MailboxActivity extends BriarActivity { onCameraError(); } else if (state instanceof MailboxState.IsPaired) { onIsPaired(((MailboxState.IsPaired) state).isOnline); + } else if (state instanceof MailboxState.WasUnpaired) { + MailboxState.WasUnpaired s = (MailboxState.WasUnpaired) state; + onUnPaired(s.tellUserToWipeMailbox); } else { throw new AssertionError("Unknown state: " + state); } @@ -97,17 +100,8 @@ public class MailboxActivity extends BriarActivity { } } - private void onNotSetup(boolean tellUserToWipeMailbox) { + private void onNotSetup() { progressBar.setVisibility(INVISIBLE); - if (tellUserToWipeMailbox) { - AlertDialog.Builder builder = - new AlertDialog.Builder(this, R.style.BriarDialogTheme); - builder.setTitle(R.string.mailbox_status_unlink_no_wipe_title); - builder.setMessage(R.string.mailbox_status_unlink_no_wipe_message); - builder.setNeutralButton(R.string.got_it, - (dialog, which) -> dialog.cancel()); - builder.show(); - } getSupportFragmentManager().beginTransaction() .replace(R.id.fragmentContainer, new SetupIntroFragment(), SetupIntroFragment.TAG) @@ -137,7 +131,7 @@ public class MailboxActivity extends BriarActivity { if (fm.getBackStackEntryCount() == 0) { // We re-launched into an existing state, // need to re-populate the back stack. - onNotSetup(false); + onNotSetup(); onShowDownload(); } Fragment f; @@ -201,4 +195,22 @@ public class MailboxActivity extends BriarActivity { showFragment(getSupportFragmentManager(), f, tag, false); } + private void onUnPaired(boolean tellUserToWipeMailbox) { + if (tellUserToWipeMailbox) { + showFragment(getSupportFragmentManager(), new BlankFragment(), + BlankFragment.TAG); + AlertDialog.Builder builder = + new AlertDialog.Builder(this, R.style.BriarDialogTheme); + builder.setTitle(R.string.mailbox_status_unlink_no_wipe_title); + builder.setMessage(R.string.mailbox_status_unlink_no_wipe_message); + builder.setNeutralButton(R.string.got_it, + (dialog, which) -> dialog.cancel()); + builder.setOnCancelListener( + dialog -> supportFinishAfterTransition()); + builder.show(); + } else { + supportFinishAfterTransition(); + } + } + } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxState.java b/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxState.java index 532772103..774f2c795 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxState.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxState.java @@ -5,15 +5,6 @@ import org.briarproject.bramble.api.mailbox.MailboxPairingState; class MailboxState { static class NotSetup extends MailboxState { - final boolean tellUserToWipeMailbox; - - NotSetup() { - this(false); - } - - NotSetup(boolean tellUserToWipeMailbox) { - this.tellUserToWipeMailbox = tellUserToWipeMailbox; - } } static class ShowDownload extends MailboxState { @@ -44,4 +35,12 @@ class MailboxState { } } + static class WasUnpaired extends MailboxState { + final boolean tellUserToWipeMailbox; + + WasUnpaired(boolean tellUserToWipeMailbox) { + this.tellUserToWipeMailbox = tellUserToWipeMailbox; + } + } + } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxViewModel.java index e74e8fc6c..0d61bfab5 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxViewModel.java @@ -226,7 +226,7 @@ class MailboxViewModel extends DbViewModel ioExecutor.execute(() -> { try { boolean wasWiped = mailboxManager.unPair(); - pairingState.postEvent(new MailboxState.NotSetup(!wasWiped)); + pairingState.postEvent(new MailboxState.WasUnpaired(!wasWiped)); } catch (DbException e) { handleException(e); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/BlankFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/view/BlankFragment.java new file mode 100644 index 000000000..6014e8994 --- /dev/null +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/BlankFragment.java @@ -0,0 +1,12 @@ +package org.briarproject.briar.android.view; + +import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; +import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; + +import androidx.fragment.app.Fragment; + +@MethodsNotNullByDefault +@ParametersNotNullByDefault +public class BlankFragment extends Fragment { + public static final String TAG = BlankFragment.class.getName(); +} From aa997a9c64d515a3353cd87d3f75a96d63e9ce6d Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 27 Apr 2022 13:41:58 -0300 Subject: [PATCH 5/5] Tweak mailbox wipe reminder string --- briar-android/src/main/res/values/strings.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/briar-android/src/main/res/values/strings.xml b/briar-android/src/main/res/values/strings.xml index b7c3e3f1b..2dfca0d34 100644 --- a/briar-android/src/main/res/values/strings.xml +++ b/briar-android/src/main/res/values/strings.xml @@ -655,7 +655,7 @@ Are you sure you want to unlink your Mailbox? If you unlink your Mailbox, you won\'t be able to receive messages while Briar is offline. Your Mailbox has been unlinked - Next time you have access to your Mailbox device, please open the Mailbox app and tap the \"Unlink\" button to complete the process.\n\nIf you no longer have access to your Mailbox device, don\'t worry. Everything stored in your Mailbox is encrypted, so your data will remain secure even if you don\'t complete the process. + Next time you have access to your Mailbox device, please open the Mailbox app and tap the \"Unlink\" button to complete the process.\n\nIf you no longer have access to your Mailbox device, don\'t worry. Your data is encrypted so it will remain secure even if you don\'t complete the process. Disappearing messages