From 15d98dc30608fbde77b93dd86151802e5eb5e82f Mon Sep 17 00:00:00 2001 From: Chandru S Date: Fri, 23 Jun 2023 15:37:17 -0700 Subject: [PATCH] BypassEnabled change is not propagated when face enrollment state changes. - KeyguardStateController was not calling onFaceAuthEnabledChanged() when the enrollment state changes upstream in KeyguardUpdateMonitor - bypassEnabled getter combines three values: actual setting value && face auth enrolled && posture supports face auth - When the device first boots up, face enrollment state in KeyguardUpdateMonitor is always false - Once AuthController#onEnrollmentsChanged gets invoked, KeyguardUpdateMonitor gets the correct enrollment state - That change is not propagated down to KeyguardBypassController until the bypassEnabled property is read again. Why is this not an issue currently? - In the old system, we keep re-reading the property value which ensures we don't miss the update. - With the new face auth system, we rely on state changes to be published always - When bypassEnabled is initially false, we assume there will be an update when it changes and don't keep re-checking the property value. - However, that update never comes until we auth once, which triggers state changes that eventually make us re-read the property. Bug: 285523886 Test: atest KeyguardBypassControllerTest Test: atest KeyguardUpdateMonitorTest Test: atest KeyguardStateControllerTest Change-Id: I544c3426972dcc03662848096fc1cac1c9412a75 --- .../keyguard/KeyguardUpdateMonitor.java | 27 ++++++++++++++++++ .../KeyguardUpdateMonitorCallback.java | 5 ++++ .../logging/KeyguardUpdateMonitorLogger.kt | 28 ++++++++++++------- .../policy/KeyguardStateControllerImpl.java | 17 +++++++++++ .../keyguard/KeyguardUpdateMonitorTest.java | 19 +++++++++++++ .../phone/KeyguardBypassControllerTest.kt | 15 ++++++++++ .../policy/KeyguardStateControllerTest.java | 28 ++++++++++++++++--- 7 files changed, 125 insertions(+), 14 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java index 89ef749f05bfe..c4ea45dbbec76 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java @@ -24,6 +24,8 @@ import static android.app.WindowConfiguration.WINDOWING_MODE_UNDEFINED; import static android.content.Intent.ACTION_USER_REMOVED; import static android.content.Intent.ACTION_USER_STOPPED; import static android.content.Intent.ACTION_USER_UNLOCKED; +import static android.hardware.biometrics.BiometricAuthenticator.TYPE_FACE; +import static android.hardware.biometrics.BiometricAuthenticator.TYPE_FINGERPRINT; import static android.hardware.biometrics.BiometricConstants.BIOMETRIC_LOCKOUT_NONE; import static android.hardware.biometrics.BiometricConstants.BIOMETRIC_LOCKOUT_PERMANENT; import static android.hardware.biometrics.BiometricConstants.BIOMETRIC_LOCKOUT_TIMED; @@ -251,6 +253,7 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab private static final int MSG_REQUIRE_NFC_UNLOCK = 345; private static final int MSG_KEYGUARD_DISMISS_ANIMATION_FINISHED = 346; private static final int MSG_SERVICE_PROVIDERS_UPDATED = 347; + private static final int MSG_BIOMETRIC_ENROLLMENT_STATE_CHANGED = 348; /** Biometric authentication state: Not listening. */ private static final int BIOMETRIC_STATE_STOPPED = 0; @@ -2484,6 +2487,9 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab case MSG_KEYGUARD_DISMISS_ANIMATION_FINISHED: handleKeyguardDismissAnimationFinished(); break; + case MSG_BIOMETRIC_ENROLLMENT_STATE_CHANGED: + notifyAboutEnrollmentChange(msg.arg1); + break; default: super.handleMessage(msg); break; @@ -2584,6 +2590,8 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab @Override public void onEnrollmentsChanged(@BiometricAuthenticator.Modality int modality) { + mHandler.obtainMessage(MSG_BIOMETRIC_ENROLLMENT_STATE_CHANGED, modality, 0) + .sendToTarget(); mainExecutor.execute(() -> updateBiometricListeningState(BIOMETRIC_ACTION_UPDATE, FACE_AUTH_TRIGGERED_ENROLLMENTS_CHANGED)); } @@ -3433,6 +3441,25 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab return mIsFaceEnrolled; } + private void notifyAboutEnrollmentChange(@BiometricAuthenticator.Modality int modality) { + BiometricSourceType biometricSourceType; + if (modality == TYPE_FINGERPRINT) { + biometricSourceType = FINGERPRINT; + } else if (modality == TYPE_FACE) { + biometricSourceType = FACE; + } else { + return; + } + mLogger.notifyAboutEnrollmentsChanged(biometricSourceType); + Assert.isMainThread(); + for (int i = 0; i < mCallbacks.size(); i++) { + KeyguardUpdateMonitorCallback cb = mCallbacks.get(i).get(); + if (cb != null) { + cb.onBiometricEnrollmentStateChanged(biometricSourceType); + } + } + } + private void stopListeningForFingerprint() { mLogger.v("stopListeningForFingerprint()"); if (mFingerprintRunningState == BIOMETRIC_STATE_RUNNING) { diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitorCallback.java b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitorCallback.java index 73940055c89fa..7b596328ca138 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitorCallback.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitorCallback.java @@ -327,4 +327,9 @@ public class KeyguardUpdateMonitorCallback { * Called when the enabled trust agents associated with the specified user. */ public void onEnabledTrustAgentsChanged(int userId) { } + + /** + * On biometric enrollment state changed + */ + public void onBiometricEnrollmentStateChanged(BiometricSourceType biometricSourceType) { } } diff --git a/packages/SystemUI/src/com/android/keyguard/logging/KeyguardUpdateMonitorLogger.kt b/packages/SystemUI/src/com/android/keyguard/logging/KeyguardUpdateMonitorLogger.kt index a192803ea0e89..be6d753b72bdc 100644 --- a/packages/SystemUI/src/com/android/keyguard/logging/KeyguardUpdateMonitorLogger.kt +++ b/packages/SystemUI/src/com/android/keyguard/logging/KeyguardUpdateMonitorLogger.kt @@ -18,6 +18,7 @@ package com.android.keyguard.logging import android.content.Intent import android.hardware.biometrics.BiometricConstants.LockoutMode +import android.hardware.biometrics.BiometricSourceType import android.os.PowerManager import android.os.PowerManager.WakeReason import android.telephony.ServiceState @@ -370,16 +371,14 @@ constructor(@KeyguardUpdateMonitorLog private val logBuffer: LogBuffer) { fun logServiceProvidersUpdated(intent: Intent) { logBuffer.log( - TAG, - VERBOSE, - { - int1 = intent.getIntExtra(EXTRA_SUBSCRIPTION_INDEX, INVALID_SUBSCRIPTION_ID) - str1 = intent.getStringExtra(TelephonyManager.EXTRA_SPN) - str2 = intent.getStringExtra(TelephonyManager.EXTRA_PLMN) - }, - { - "action SERVICE_PROVIDERS_UPDATED subId=$int1 spn=$str1 plmn=$str2" - } + TAG, + VERBOSE, + { + int1 = intent.getIntExtra(EXTRA_SUBSCRIPTION_INDEX, INVALID_SUBSCRIPTION_ID) + str1 = intent.getStringExtra(TelephonyManager.EXTRA_SPN) + str2 = intent.getStringExtra(TelephonyManager.EXTRA_PLMN) + }, + { "action SERVICE_PROVIDERS_UPDATED subId=$int1 spn=$str1 plmn=$str2" } ) } @@ -719,4 +718,13 @@ constructor(@KeyguardUpdateMonitorLog private val logBuffer: LogBuffer) { fun scheduleWatchdog(@CompileTimeConstant watchdogType: String) { logBuffer.log(TAG, DEBUG, "Scheduling biometric watchdog for $watchdogType") } + + fun notifyAboutEnrollmentsChanged(biometricSourceType: BiometricSourceType) { + logBuffer.log( + TAG, + DEBUG, + { str1 = "$biometricSourceType" }, + { "notifying about enrollments changed: $str1" } + ) + } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/KeyguardStateControllerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/KeyguardStateControllerImpl.java index a82646aba825b..710588c82a4a5 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/KeyguardStateControllerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/KeyguardStateControllerImpl.java @@ -16,6 +16,8 @@ package com.android.systemui.statusbar.policy; +import static android.hardware.biometrics.BiometricSourceType.FACE; + import android.annotation.NonNull; import android.content.BroadcastReceiver; import android.content.Context; @@ -199,6 +201,11 @@ public class KeyguardStateControllerImpl implements KeyguardStateController, Dum Trace.endSection(); } + private void notifyKeyguardFaceAuthEnabledChanged() { + // Copy the list to allow removal during callback. + new ArrayList<>(mCallbacks).forEach(Callback::onFaceAuthEnabledChanged); + } + private void notifyUnlockedChanged() { Trace.beginSection("KeyguardStateController#notifyUnlockedChanged"); // Copy the list to allow removal during callback. @@ -418,6 +425,16 @@ public class KeyguardStateControllerImpl implements KeyguardStateController, Dum update(false /* updateAlways */); } + @Override + public void onBiometricEnrollmentStateChanged(BiometricSourceType biometricSourceType) { + if (biometricSourceType == FACE) { + // We only care about enrollment state here. Keyguard face auth enabled is just + // same as face auth enrolled + update(false); + notifyKeyguardFaceAuthEnabledChanged(); + } + } + @Override public void onStartedWakingUp() { update(false /* updateAlways */); diff --git a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java index 901c3fb05b2ff..4263091c5ee81 100644 --- a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java +++ b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java @@ -83,6 +83,7 @@ import android.content.pm.ServiceInfo; import android.content.pm.UserInfo; import android.database.ContentObserver; import android.hardware.SensorPrivacyManager; +import android.hardware.biometrics.BiometricAuthenticator; import android.hardware.biometrics.BiometricConstants; import android.hardware.biometrics.BiometricManager; import android.hardware.biometrics.BiometricSourceType; @@ -3000,6 +3001,24 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { TelephonyManager.SIM_STATE_UNKNOWN); } + @Test + public void onAuthEnrollmentChangesCallbacksAreNotified() { + KeyguardUpdateMonitorCallback callback = mock(KeyguardUpdateMonitorCallback.class); + ArgumentCaptor authCallback = ArgumentCaptor.forClass( + AuthController.Callback.class); + verify(mAuthController).addCallback(authCallback.capture()); + + mKeyguardUpdateMonitor.registerCallback(callback); + + authCallback.getValue().onEnrollmentsChanged(TYPE_FINGERPRINT); + mTestableLooper.processAllMessages(); + verify(callback).onBiometricEnrollmentStateChanged(BiometricSourceType.FINGERPRINT); + + authCallback.getValue().onEnrollmentsChanged(BiometricAuthenticator.TYPE_FACE); + mTestableLooper.processAllMessages(); + verify(callback).onBiometricEnrollmentStateChanged(BiometricSourceType.FACE); + } + private void verifyFingerprintAuthenticateNeverCalled() { verify(mFingerprintManager, never()).authenticate(any(), any(), any(), any(), any()); verify(mFingerprintManager, never()).authenticate(any(), any(), any(), any(), anyInt(), diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/KeyguardBypassControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/KeyguardBypassControllerTest.kt index 3e90ed9811d02..4aac8419af3be 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/KeyguardBypassControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/KeyguardBypassControllerTest.kt @@ -42,6 +42,8 @@ import org.junit.runner.RunWith import org.mockito.ArgumentCaptor import org.mockito.Captor import org.mockito.Mock +import org.mockito.Mockito.anyBoolean +import org.mockito.Mockito.mock import org.mockito.Mockito.never import org.mockito.Mockito.reset import org.mockito.Mockito.verify @@ -134,6 +136,19 @@ class KeyguardBypassControllerTest : SysuiTestCase() { ) } + @Test + fun onFaceAuthEnabledChanged_notifiesBypassEnabledListeners() { + initKeyguardBypassController() + val bypassListener = mock(KeyguardBypassController.OnBypassStateChangedListener::class.java) + val callback = ArgumentCaptor.forClass(KeyguardStateController.Callback::class.java) + + keyguardBypassController.registerOnBypassStateChangedListener(bypassListener) + verify(keyguardStateController).addCallback(callback.capture()) + + callback.value.onFaceAuthEnabledChanged() + verify(bypassListener).onBypassStateChanged(anyBoolean()) + } + @Test fun configDevicePostureClosed_matchState_isPostureAllowedForFaceAuth_returnTrue() { defaultConfigPostureClosed() diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/KeyguardStateControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/KeyguardStateControllerTest.java index d787ada90a73c..5cabcd4163b2b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/KeyguardStateControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/KeyguardStateControllerTest.java @@ -24,6 +24,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import android.hardware.biometrics.BiometricSourceType; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; @@ -43,6 +44,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; +import org.mockito.Captor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -66,6 +68,9 @@ public class KeyguardStateControllerTest extends SysuiTestCase { @Mock private KeyguardUpdateMonitorLogger mLogger; + @Captor + private ArgumentCaptor mUpdateCallbackCaptor; + @Before public void setup() { MockitoAnnotations.initMocks(this); @@ -83,6 +88,23 @@ public class KeyguardStateControllerTest extends SysuiTestCase { verify(mKeyguardUpdateMonitor).registerCallback(any()); } + @Test + public void testFaceAuthEnabledChanged_calledWhenFaceEnrollmentStateChanges() { + KeyguardStateController.Callback callback = mock(KeyguardStateController.Callback.class); + + when(mKeyguardUpdateMonitor.isFaceAuthEnabledForUser(anyInt())).thenReturn(false); + verify(mKeyguardUpdateMonitor).registerCallback(mUpdateCallbackCaptor.capture()); + mKeyguardStateController.addCallback(callback); + assertThat(mKeyguardStateController.isFaceAuthEnabled()).isFalse(); + + when(mKeyguardUpdateMonitor.isFaceAuthEnabledForUser(anyInt())).thenReturn(true); + mUpdateCallbackCaptor.getValue().onBiometricEnrollmentStateChanged( + BiometricSourceType.FACE); + + assertThat(mKeyguardStateController.isFaceAuthEnabled()).isTrue(); + verify(callback).onFaceAuthEnabledChanged(); + } + @Test public void testIsShowing() { assertThat(mKeyguardStateController.isShowing()).isFalse(); @@ -177,16 +199,14 @@ public class KeyguardStateControllerTest extends SysuiTestCase { @Test public void testOnEnabledTrustAgentsChangedCallback() { final Random random = new Random(); - final ArgumentCaptor updateCallbackCaptor = - ArgumentCaptor.forClass(KeyguardUpdateMonitorCallback.class); - verify(mKeyguardUpdateMonitor).registerCallback(updateCallbackCaptor.capture()); + verify(mKeyguardUpdateMonitor).registerCallback(mUpdateCallbackCaptor.capture()); final KeyguardStateController.Callback stateCallback = mock(KeyguardStateController.Callback.class); mKeyguardStateController.addCallback(stateCallback); when(mLockPatternUtils.isSecure(anyInt())).thenReturn(true); - updateCallbackCaptor.getValue().onEnabledTrustAgentsChanged(random.nextInt()); + mUpdateCallbackCaptor.getValue().onEnabledTrustAgentsChanged(random.nextInt()); verify(stateCallback).onUnlockedChanged(); } }