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
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java
index 89ef749..c4ea45d 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.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 @@
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 @@
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 @@
@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 @@
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 7394005..7b59632 100644
--- a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitorCallback.java
+++ b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitorCallback.java
@@ -327,4 +327,9 @@
* 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 a192803..be6d753 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 @@
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 @@
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 @@
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 a82646a..710588c 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 @@
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.
@@ -419,6 +426,16 @@
}
@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 901c3fb..4263091 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.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 @@
TelephonyManager.SIM_STATE_UNKNOWN);
}
+ @Test
+ public void onAuthEnrollmentChangesCallbacksAreNotified() {
+ KeyguardUpdateMonitorCallback callback = mock(KeyguardUpdateMonitorCallback.class);
+ ArgumentCaptor<AuthController.Callback> 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 3e90ed9..4aac841 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.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
@@ -135,6 +137,19 @@
}
@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 d787ada..5cabcd4 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.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.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 @@
@Mock
private KeyguardUpdateMonitorLogger mLogger;
+ @Captor
+ private ArgumentCaptor<KeyguardUpdateMonitorCallback> mUpdateCallbackCaptor;
+
@Before
public void setup() {
MockitoAnnotations.initMocks(this);
@@ -84,6 +89,23 @@
}
@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();
mKeyguardStateController.notifyKeyguardState(true /* showing */, false /* occluded */);
@@ -177,16 +199,14 @@
@Test
public void testOnEnabledTrustAgentsChangedCallback() {
final Random random = new Random();
- final ArgumentCaptor<KeyguardUpdateMonitorCallback> 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();
}
}