Do not dismiss keyguard after SIM PUK unlock

After PUK unlock, multiple calls to
KeyguardSecurityContainerController#dismiss() were being called from
the KeyguardSimPukViewController, which begins the transition to the
next security screen, if any. At the same time, other parts of the
system, also listening to SIM events, recognize the PUK unlock and
call KeyguardSecurityContainer#showSecurityScreen, which updates which
security method comes next. After boot, this should be one of PIN,
Password, Pattern, assuming they have a security method. If one of the
first dismiss() calls comes AFTER the security method changes, this is
incorrectly recognized by the code as a successful
PIN/pattern/password unlock. This causes the keyguard to be marked as
done, causing screen flickers and incorrect system state.

The solution: every call to dismiss() should include a new parameter
for the security method used. If there is a difference between this
parameter and the current value in KeyguardSecurityContainerCallback,
ignore the request, as the system state has changed.

Fixes: 238804980
Bug: 218500036
Test: atest KeyguardSecurityContainerTest
AdminSecondaryLockScreenControllerTest KeyguardHostViewControllerTest
KeyguardSecurityContainerControllerTest

Change-Id: I7c8714a177bc85fbce92f6e8fe911f74ca2ac243
Merged-In: I7c8714a177bc85fbce92f6e8fe911f74ca2ac243
(cherry picked from commit 37aeb26b0ae28a48c1ed40f008d5808d8a84be23)
(cherry picked from commit 3d89cc5df6729cab8d98c967d73e03e23048d52b)
Merged-In: I7c8714a177bc85fbce92f6e8fe911f74ca2ac243

12 files changed, 102 insertions, 26 deletions

diff --git a/packages/SystemUI/src/com/android/keyguard/AdminSecondaryLockScreenController.java b/packages/SystemUI/src/com/android/keyguard/AdminSecondaryLockScreenController.java
index 23195af8bdea..00f1c0108d0b 100644
--- a/packages/SystemUI/src/com/android/keyguard/AdminSecondaryLockScreenController.java
+++ b/packages/SystemUI/src/com/android/keyguard/AdminSecondaryLockScreenController.java
@@ -33,6 +33,7 @@ import android.view.SurfaceView;
 import android.view.ViewGroup;
 
 import com.android.internal.annotations.VisibleForTesting;
+import com.android.keyguard.KeyguardSecurityModel.SecurityMode;
 import com.android.keyguard.dagger.KeyguardBouncerScope;
 import com.android.systemui.dagger.qualifiers.Main;
 
@@ -208,7 +209,7 @@ public class AdminSecondaryLockScreenController {
             hide();
             if (mKeyguardCallback != null) {
                 mKeyguardCallback.dismiss(/* securityVerified= */ true, userId,
-                        /* bypassSecondaryLockScreen= */true);
+                        /* bypassSecondaryLockScreen= */true, SecurityMode.Invalid);
             }
         }
     }
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputViewController.java
index eb418ff3b142..b8fcb103402d 100644
--- a/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputViewController.java
+++ b/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputViewController.java
@@ -179,7 +179,7 @@ public abstract class KeyguardAbsKeyInputViewController<T extends KeyguardAbsKey
             if (dismissKeyguard) {
                 mDismissing = true;
                 mLatencyTracker.onActionStart(LatencyTracker.ACTION_LOCKSCREEN_UNLOCK);
-                getKeyguardSecurityCallback().dismiss(true, userId);
+                getKeyguardSecurityCallback().dismiss(true, userId, getSecurityMode());
             }
         } else {
             if (isValidPassword) {
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardHostViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardHostViewController.java
index 12fa401d7fea..befd59be061d 100644
--- a/packages/SystemUI/src/com/android/keyguard/KeyguardHostViewController.java
+++ b/packages/SystemUI/src/com/android/keyguard/KeyguardHostViewController.java
@@ -90,7 +90,7 @@ public class KeyguardHostViewController extends ViewController<KeyguardHostView>
                                 Log.i(TAG, "TrustAgent dismissed Keyguard.");
                             }
                             mSecurityCallback.dismiss(false /* authenticated */, userId,
-                                    /* bypassSecondaryLockScreen */ false);
+                                    /* bypassSecondaryLockScreen */ false, SecurityMode.Invalid);
                         } else {
                             mViewMediatorCallback.playTrustedSound();
                         }
@@ -102,9 +102,9 @@ public class KeyguardHostViewController extends ViewController<KeyguardHostView>
 
         @Override
         public boolean dismiss(boolean authenticated, int targetUserId,
-                boolean bypassSecondaryLockScreen) {
+                boolean bypassSecondaryLockScreen, SecurityMode expectedSecurityMode) {
             return mKeyguardSecurityContainerController.showNextSecurityScreenOrFinish(
-                    authenticated, targetUserId, bypassSecondaryLockScreen);
+                    authenticated, targetUserId, bypassSecondaryLockScreen, expectedSecurityMode);
         }
 
         @Override
@@ -212,7 +212,8 @@ public class KeyguardHostViewController extends ViewController<KeyguardHostView>
      * @return True if the keyguard is done.
      */
     public boolean dismiss(int targetUserId) {
-        return mSecurityCallback.dismiss(false, targetUserId, false);
+        return mSecurityCallback.dismiss(false, targetUserId, false,
+                getCurrentSecurityMode());
     }
 
     /**
@@ -355,10 +356,10 @@ public class KeyguardHostViewController extends ViewController<KeyguardHostView>
     }
 
     public boolean handleBackKey() {
-        if (mKeyguardSecurityContainerController.getCurrentSecurityMode()
-                != SecurityMode.None) {
+        SecurityMode securityMode = mKeyguardSecurityContainerController.getCurrentSecurityMode();
+        if (securityMode != SecurityMode.None) {
             mKeyguardSecurityContainerController.dismiss(
-                    false, KeyguardUpdateMonitor.getCurrentUser());
+                    false, KeyguardUpdateMonitor.getCurrentUser(), securityMode);
             return true;
         }
         return false;
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardInputViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardInputViewController.java
index 98ac640bf703..87300c3f0504 100644
--- a/packages/SystemUI/src/com/android/keyguard/KeyguardInputViewController.java
+++ b/packages/SystemUI/src/com/android/keyguard/KeyguardInputViewController.java
@@ -59,10 +59,11 @@ public abstract class KeyguardInputViewController<T extends KeyguardInputView>
             return false;
         }
         @Override
-        public void dismiss(boolean securityVerified, int targetUserId) { }
+        public void dismiss(boolean securityVerified, int targetUserId,
+                SecurityMode expectedSecurityMode) { }
         @Override
         public void dismiss(boolean authenticated, int targetId,
-                boolean bypassSecondaryLockScreen) { }
+                boolean bypassSecondaryLockScreen, SecurityMode expectedSecurityMode) { }
         @Override
         public void onUserInput() { }
         @Override
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardPatternViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardPatternViewController.java
index 39c394981193..1a59b820c1bd 100644
--- a/packages/SystemUI/src/com/android/keyguard/KeyguardPatternViewController.java
+++ b/packages/SystemUI/src/com/android/keyguard/KeyguardPatternViewController.java
@@ -171,7 +171,7 @@ public class KeyguardPatternViewController
                 if (dismissKeyguard) {
                     mLockPatternView.setDisplayMode(LockPatternView.DisplayMode.Correct);
                     mLatencyTracker.onActionStart(LatencyTracker.ACTION_LOCKSCREEN_UNLOCK);
-                    getKeyguardSecurityCallback().dismiss(true, userId);
+                    getKeyguardSecurityCallback().dismiss(true, userId, SecurityMode.Pattern);
                 }
             } else {
                 mLockPatternView.setDisplayMode(LockPatternView.DisplayMode.Wrong);
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityCallback.java b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityCallback.java
index e38472745234..bc72f7979a74 100644
--- a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityCallback.java
+++ b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityCallback.java
@@ -15,14 +15,17 @@
  */
 package com.android.keyguard;
 
+import com.android.keyguard.KeyguardSecurityModel.SecurityMode;
+
 public interface KeyguardSecurityCallback {
 
     /**
      * Dismiss the given security screen.
      * @param securityVerified true if the user correctly entered credentials for the given screen.
      * @param targetUserId a user that needs to be the foreground user at the dismissal completion.
+     * @param expectedSecurityMode The security mode that is invoking this dismiss.
      */
-    void dismiss(boolean securityVerified, int targetUserId);
+    void dismiss(boolean securityVerified, int targetUserId, SecurityMode expectedSecurityMode);
 
     /**
      * Dismiss the given security screen.
@@ -30,8 +33,10 @@ public interface KeyguardSecurityCallback {
      * @param targetUserId a user that needs to be the foreground user at the dismissal completion.
      * @param bypassSecondaryLockScreen true if the user can bypass the secondary lock screen,
      *                                  if any, during this dismissal.
+     * @param expectedSecurityMode The security mode that is invoking this dismiss.
      */
-    void dismiss(boolean securityVerified, int targetUserId, boolean bypassSecondaryLockScreen);
+    void dismiss(boolean securityVerified, int targetUserId, boolean bypassSecondaryLockScreen,
+            SecurityMode expectedSecurityMode);
 
     /**
      * Manually report user activity to keep the device awake.
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java
index cce516d981a5..12bb47b81d7f 100644
--- a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java
+++ b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java
@@ -233,7 +233,12 @@ public class KeyguardSecurityContainer extends FrameLayout {
 
     // Used to notify the container when something interesting happens.
     public interface SecurityCallback {
-        boolean dismiss(boolean authenticated, int targetUserId, boolean bypassSecondaryLockScreen);
+        /**
+         * Potentially dismiss the current security screen, after validating that all device
+         * security has been unlocked. Otherwise show the next screen.
+         */
+        boolean dismiss(boolean authenticated, int targetUserId, boolean bypassSecondaryLockScreen,
+                SecurityMode expectedSecurityMode);
 
         void userActivity();
 
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java
index 19a2d9ed5b7b..2b9553d3eda2 100644
--- a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java
+++ b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java
@@ -153,14 +153,17 @@ public class KeyguardSecurityContainerController extends ViewController<Keyguard
         }
 
         @Override
-        public void dismiss(boolean authenticated, int targetId) {
-            dismiss(authenticated, targetId, /* bypassSecondaryLockScreen */ false);
+        public void dismiss(boolean authenticated, int targetId,
+                SecurityMode expectedSecurityMode) {
+            dismiss(authenticated, targetId, /* bypassSecondaryLockScreen */ false,
+                    expectedSecurityMode);
         }
 
         @Override
         public void dismiss(boolean authenticated, int targetId,
-                boolean bypassSecondaryLockScreen) {
-            mSecurityCallback.dismiss(authenticated, targetId, bypassSecondaryLockScreen);
+                boolean bypassSecondaryLockScreen, SecurityMode expectedSecurityMode) {
+            mSecurityCallback.dismiss(authenticated, targetId, bypassSecondaryLockScreen,
+                    expectedSecurityMode);
         }
 
         public boolean isVerifyUnlockOnly() {
@@ -350,8 +353,13 @@ public class KeyguardSecurityContainerController extends ViewController<Keyguard
         return mCurrentSecurityMode;
     }
 
-    public void dismiss(boolean authenticated, int targetUserId) {
-        mKeyguardSecurityCallback.dismiss(authenticated, targetUserId);
+    /**
+     * Potentially dismiss the current security screen, after validating that all device
+     * security has been unlocked. Otherwise show the next screen.
+     */
+    public void dismiss(boolean authenticated, int targetUserId,
+            SecurityMode expectedSecurityMode) {
+        mKeyguardSecurityCallback.dismiss(authenticated, targetUserId, expectedSecurityMode);
     }
 
     public void reset() {
@@ -410,12 +418,21 @@ public class KeyguardSecurityContainerController extends ViewController<Keyguard
      *     completion.
      * @param bypassSecondaryLockScreen true if the user is allowed to bypass the secondary
      *     secondary lock screen requirement, if any.
+     * @param expectedSecurityMode SecurityMode that is invoking this request. SecurityMode.Invalid
+     *      indicates that no check should be done
      * @return true if keyguard is done
      */
     public boolean showNextSecurityScreenOrFinish(boolean authenticated, int targetUserId,
-            boolean bypassSecondaryLockScreen) {
+            boolean bypassSecondaryLockScreen, SecurityMode expectedSecurityMode) {
 
         if (DEBUG) Log.d(TAG, "showNextSecurityScreenOrFinish(" + authenticated + ")");
+        if (expectedSecurityMode != SecurityMode.Invalid
+                && expectedSecurityMode != getCurrentSecurityMode()) {
+            Log.w(TAG, "Attempted to invoke showNextSecurityScreenOrFinish with securityMode "
+                    + expectedSecurityMode + ", but current mode is " + getCurrentSecurityMode());
+            return false;
+        }
+
         boolean finish = false;
         boolean strongAuth = false;
         int eventSubtype = -1;
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardSimPinViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardSimPinViewController.java
index 47df70b522f7..b3f25c289b46 100644
--- a/packages/SystemUI/src/com/android/keyguard/KeyguardSimPinViewController.java
+++ b/packages/SystemUI/src/com/android/keyguard/KeyguardSimPinViewController.java
@@ -168,7 +168,8 @@ public class KeyguardSimPinViewController
                             mRemainingAttempts = -1;
                             mShowDefaultMessage = true;
                             getKeyguardSecurityCallback().dismiss(
-                                    true, KeyguardUpdateMonitor.getCurrentUser());
+                                    true, KeyguardUpdateMonitor.getCurrentUser(),
+                                    SecurityMode.SimPin);
                         } else {
                             mShowDefaultMessage = false;
                             if (result.getResult() == PinResult.PIN_RESULT_TYPE_INCORRECT) {
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardSimPukViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardSimPukViewController.java
index 47aa43b86599..203f9b660536 100644
--- a/packages/SystemUI/src/com/android/keyguard/KeyguardSimPukViewController.java
+++ b/packages/SystemUI/src/com/android/keyguard/KeyguardSimPukViewController.java
@@ -69,7 +69,8 @@ public class KeyguardSimPukViewController
             if (simState == TelephonyManager.SIM_STATE_READY) {
                 mRemainingAttempts = -1;
                 mShowDefaultMessage = true;
-                getKeyguardSecurityCallback().dismiss(true, KeyguardUpdateMonitor.getCurrentUser());
+                getKeyguardSecurityCallback().dismiss(true, KeyguardUpdateMonitor.getCurrentUser(),
+                        SecurityMode.SimPuk);
             } else {
                 resetState();
             }
@@ -278,7 +279,8 @@ public class KeyguardSimPukViewController
                             mShowDefaultMessage = true;
 
                             getKeyguardSecurityCallback().dismiss(
-                                    true, KeyguardUpdateMonitor.getCurrentUser());
+                                    true, KeyguardUpdateMonitor.getCurrentUser(),
+                                    SecurityMode.SimPuk);
                         } else {
                             mShowDefaultMessage = false;
                             if (result.getResult() == PinResult.PIN_RESULT_TYPE_INCORRECT) {
diff --git a/packages/SystemUI/tests/src/com/android/keyguard/AdminSecondaryLockScreenControllerTest.java b/packages/SystemUI/tests/src/com/android/keyguard/AdminSecondaryLockScreenControllerTest.java
index dffad6ccbea5..80385e69cfa4 100644
--- a/packages/SystemUI/tests/src/com/android/keyguard/AdminSecondaryLockScreenControllerTest.java
+++ b/packages/SystemUI/tests/src/com/android/keyguard/AdminSecondaryLockScreenControllerTest.java
@@ -44,6 +44,7 @@ import android.view.SurfaceView;
 
 import androidx.test.filters.SmallTest;
 
+import com.android.keyguard.KeyguardSecurityModel.SecurityMode;
 import com.android.systemui.SysuiTestCase;
 
 import org.junit.After;
@@ -190,7 +191,7 @@ public class AdminSecondaryLockScreenControllerTest extends SysuiTestCase {
 
     private void verifyViewDismissed(SurfaceView v) throws Exception {
         verify(mKeyguardSecurityContainer).removeView(v);
-        verify(mKeyguardCallback).dismiss(true, TARGET_USER_ID, true);
+        verify(mKeyguardCallback).dismiss(true, TARGET_USER_ID, true, SecurityMode.Invalid);
         assertThat(mContext.isBound(mComponentName)).isFalse();
     }
 }
diff --git a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardSecurityContainerControllerTest.java b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardSecurityContainerControllerTest.java
index 4d3343059718..efc9921fe8b9 100644
--- a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardSecurityContainerControllerTest.java
+++ b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardSecurityContainerControllerTest.java
@@ -21,7 +21,10 @@ import static android.view.WindowInsets.Type.ime;
 import static com.android.keyguard.KeyguardSecurityContainer.MODE_DEFAULT;
 import static com.android.keyguard.KeyguardSecurityContainer.MODE_ONE_HANDED;
 
+import static com.google.common.truth.Truth.assertThat;
+
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.eq;
@@ -69,6 +72,7 @@ import org.mockito.junit.MockitoRule;
 @TestableLooper.RunWithLooper()
 public class KeyguardSecurityContainerControllerTest extends SysuiTestCase {
     private static final int VIEW_WIDTH = 1600;
+    private static final int TARGET_USER_ID = 100;
 
     @Rule
     public MockitoRule mRule = MockitoJUnit.rule();
@@ -299,4 +303,42 @@ public class KeyguardSecurityContainerControllerTest extends SysuiTestCase {
         verify(mUserSwitcherController)
                 .removeUserSwitchCallback(any(UserSwitcherController.UserSwitchCallback.class));
     }
+
+    @Test
+    public void showNextSecurityScreenOrFinish_setsSecurityScreenToPinAfterSimPinUnlock() {
+        // GIVEN the current security method is SimPin
+        when(mKeyguardUpdateMonitor.getUserHasTrust(anyInt())).thenReturn(false);
+        when(mKeyguardUpdateMonitor.getUserUnlockedWithBiometric(TARGET_USER_ID)).thenReturn(false);
+        mKeyguardSecurityContainerController.showSecurityScreen(SecurityMode.SimPin);
+
+        // WHEN a request is made from the SimPin screens to show the next security method
+        when(mKeyguardSecurityModel.getSecurityMode(TARGET_USER_ID)).thenReturn(SecurityMode.PIN);
+        mKeyguardSecurityContainerController.showNextSecurityScreenOrFinish(
+                /* authenticated= */true,
+                TARGET_USER_ID,
+                /* bypassSecondaryLockScreen= */true,
+                SecurityMode.SimPin);
+
+        // THEN the next security method of PIN is set, and the keyguard is not marked as done
+        verify(mSecurityCallback, never()).finish(anyBoolean(), anyInt());
+        assertThat(mKeyguardSecurityContainerController.getCurrentSecurityMode())
+                .isEqualTo(SecurityMode.PIN);
+    }
+
+    @Test
+    public void showNextSecurityScreenOrFinish_ignoresCallWhenSecurityMethodHasChanged() {
+        //GIVEN current security mode has been set to PIN
+        mKeyguardSecurityContainerController.showSecurityScreen(SecurityMode.PIN);
+
+        //WHEN a request comes from SimPin to dismiss the security screens
+        boolean keyguardDone = mKeyguardSecurityContainerController.showNextSecurityScreenOrFinish(
+                /* authenticated= */true,
+                TARGET_USER_ID,
+                /* bypassSecondaryLockScreen= */true,
+                SecurityMode.SimPin);
+
+        //THEN no action has happened, which will not dismiss the security screens
+        assertThat(keyguardDone).isEqualTo(false);
+        verify(mKeyguardUpdateMonitor, never()).getUserHasTrust(anyInt());
+    }
 }