beemdevelopment / Aegis

A free, secure and open source app for Android to manage your 2-step verification tokens.
https://getaegis.app
GNU General Public License v3.0
9.15k stars 387 forks source link

Fingerprint unlock prompt gets stuck sometimes drawing over all apps until reboot on OnePlus 6 #342

Closed xPaw closed 4 years ago

xPaw commented 4 years ago
Info
Steps to reproduce

Really unsure about this one, happens randomly. When opening the app the fingerprint unlock shows up, you unlock it, and the unlock overlay remains stuck over all apps, even the lock screen. The only way to fix it I found is to reboot the device. Pressing cancel does nothing, touching the fingerprint sensor also does not remove it.

The only possible reproduction that may be possible is changing from portrait to landscape during unlock, but I'm unsure about that.

alexbakker commented 4 years ago

From a quick Google search, it appears this issue is not unheard of on OnePlus devices:

We basically only tell the system to "show prompt" and "close prompt". Other than that, we have no control over the prompt. I think your best bet would be to report this to OnePlus to get this fixed.

xPaw commented 4 years ago

The linked threads only show the under-the-screen unlocks, which is not the same on OP6, but I would not be surprised if it's the same issue.

I use fingerprint unlocking (in other apps too), and this issue only happens in Aegis.

alexbakker commented 4 years ago

OK, we can keep this open and see if other users report the same issue, but I'm afraid there isn't much we can do about this on our side.

xPaw commented 4 years ago

It's understandable.

I noticed that if you close the unlock prompt and switch orientation it popups the unlock prompt again. Could it be a race condition?

alexbakker commented 4 years ago

When you switch orientation, the activity is "recreated". When this happens, onPause and then onResume is called. In onPause, we close the prompt and in onResume we show the prompt. That's why you see the prompt pop up again.

xPaw commented 4 years ago

So I managed to reproduce it somewhat, lock the screen while the aegis is on the unlock screen. When I unlocked the phone with fingerprint I saw that two unlock prompts appeared on the screen one after another. I can cancel one of them, but the second remains.

This makes me think that whatever the render/update code triggers the biometric unlock popup can happen more than once, and it only keeps track of the latest prompt it creates.

alexbakker commented 4 years ago

Are you still able to reproduce this on the latest release (v1.2)?

xPaw commented 4 years ago

Yep, and I think I figured out a consistent repro for this.

  1. Rotate the phone while aegis fingerprint unlock prompt is on the screen, you will observe that the prompt pops up again during rotation.
  2. Lock the screen
  3. Unlock with fingerprint
  4. There is now a stuck prompt on screen
michaelschattgen commented 4 years ago

Thanks for your reproduction steps! However, I still can't reproduce this... What about you @alexbakker?

xPaw commented 4 years ago

I still think it's related to the popup triggering more than it should (e.g. when screen is rotated).

I tried in keepass, and it does not have this bug, and I don't see the unlock prompt popping up again, it just stays there and rotates.

EDIT: I just realized locking the screen isn't even required. Simply rotating the screen triggers the second unlock prompt, while the previous one remains under. Tapping cancel removes one, but the second one is unresponsive.

alexbakker commented 4 years ago

@michaelschattgen No, I think it's safe to say that this is a OnePlus-specific bug.

@xPaw Can you try to reproduce the issue with this debug APK? I applied the following patch to it:

diff --git a/app/src/main/java/com/beemdevelopment/aegis/ui/AuthActivity.java b/app/src/main/java/com/beemdevelopment/aegis/ui/AuthActivity.java
index 00bef1f..02c5468 100644
--- a/app/src/main/java/com/beemdevelopment/aegis/ui/AuthActivity.java
+++ b/app/src/main/java/com/beemdevelopment/aegis/ui/AuthActivity.java
@@ -199,7 +199,7 @@ public class AuthActivity extends AegisActivity {
         if (_bioKey != null) {
             if (_prefs.isPasswordReminderNeeded()) {
                 focusPasswordField();
-            } else {
+            } else if (_bioPrompt == null) {
                 showBiometricPrompt();
             }
         } else {
@@ -252,15 +252,6 @@ public class AuthActivity extends AegisActivity {
         _bioPrompt.authenticate(info, cryptoObj);
     }

-    @Override
-    public void onPause() {
-        super.onPause();
-
-        if (_bioPrompt != null) {
-            _bioPrompt.cancelAuthentication();
-        }
-    }
-
     private void finish(MasterKey key, boolean isSlotRepaired) {
         VaultFileCredentials creds = new VaultFileCredentials(key, _slots);

Apparently BiometricPrompt is able to handle Android lifecycle events on its own, so perhaps manually closing/showing the prompt instead causes this issue on your device. Shot in the dark.

xPaw commented 4 years ago

This dev version has certainly reduced the amount of prompts that re-appear, thus reduced the chance to cause it get stuck. However I was still able to get it stuck while messing with it (rotating and locking screen multiple times).

Is there a reason why a new prompt is triggered on screen rotation? KeePass does not do this for example.

alexbakker commented 4 years ago

Thanks for reporting back.

Is there a reason why a new prompt is triggered on screen rotation? KeePass does not do this for example.

That's what I was trying to eliminate, but I overlooked something. Want to give it one more try with a new APK? https://alexbakker.me/u/5dhg60wnlt.apk

diff --git a/app/src/main/java/com/beemdevelopment/aegis/ui/AegisActivity.java b/app/src/main/java/com/beemdevelopment/aegis/ui/AegisActivity.java
index 2127126..8cf3a45 100644
--- a/app/src/main/java/com/beemdevelopment/aegis/ui/AegisActivity.java
+++ b/app/src/main/java/com/beemdevelopment/aegis/ui/AegisActivity.java
@@ -144,7 +144,7 @@ public abstract class AegisActivity extends AppCompatActivity implements AegisAp
      * the vault was locked by an external trigger while the Activity was still open.
      */
     private boolean isOrphan() {
-        return !(this instanceof MainActivity) && _app.isVaultLocked();
+        return !(this instanceof MainActivity) && !(this instanceof AuthActivity) && _app.isVaultLocked();
     }

     protected Theme getCurrentTheme() {
diff --git a/app/src/main/java/com/beemdevelopment/aegis/ui/AuthActivity.java b/app/src/main/java/com/beemdevelopment/aegis/ui/AuthActivity.java
index 00bef1f..6be27a6 100644
--- a/app/src/main/java/com/beemdevelopment/aegis/ui/AuthActivity.java
+++ b/app/src/main/java/com/beemdevelopment/aegis/ui/AuthActivity.java
@@ -145,6 +145,10 @@ public class AuthActivity extends AegisActivity {
         biometricsButton.setOnClickListener(v -> {
             showBiometricPrompt();
         });
+
+        if (_bioKey != null) {
+            showBiometricPrompt();
+        }
     }

     @Override
@@ -196,13 +200,7 @@ public class AuthActivity extends AegisActivity {
     public void onResume() {
         super.onResume();

-        if (_bioKey != null) {
-            if (_prefs.isPasswordReminderNeeded()) {
-                focusPasswordField();
-            } else {
-                showBiometricPrompt();
-            }
-        } else {
+        if (_bioKey == null || _prefs.isPasswordReminderNeeded()) {
             focusPasswordField();
         }
     }
@@ -252,15 +250,6 @@ public class AuthActivity extends AegisActivity {
         _bioPrompt.authenticate(info, cryptoObj);
     }

-    @Override
-    public void onPause() {
-        super.onPause();
-
-        if (_bioPrompt != null) {
-            _bioPrompt.cancelAuthentication();
-        }
-    }
-
     private void finish(MasterKey key, boolean isSlotRepaired) {
         VaultFileCredentials creds = new VaultFileCredentials(key, _slots);
xPaw commented 4 years ago

This version is indeed much better, and I haven't been able to get it stuck while messing around.

alexbakker commented 4 years ago

Great! Thanks for helping us out by testing our attempts at fixing this. I've submitted a PR for that patch. The bug on OnePlus' side is still there of course, but at least we'll be less likely to trigger it (and it forced us to improve things on our end as well).

xPaw commented 4 years ago

I have no idea what OnePlus is breaking there to cause this, haven't had this happen in any other app though.

FonzTech commented 1 year ago

This also happens on my Xiaomi 9T. This happens because Android lags so much that if the fingerprint dialog appears when the app who requested it is in background, then you are stuck due to an internal exception. Completing the unlock procedure does nothing. Even the "Use Password" button does nothing. You can't exit this dialog, until you restart the phone.