diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/AuthBiometricFaceToFingerprintView.java b/packages/SystemUI/src/com/android/systemui/biometrics/AuthBiometricFaceToFingerprintView.java index ca59393577c00..76fb49a730a35 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/AuthBiometricFaceToFingerprintView.java +++ b/packages/SystemUI/src/com/android/systemui/biometrics/AuthBiometricFaceToFingerprintView.java @@ -130,19 +130,29 @@ public class AuthBiometricFaceToFingerprintView extends AuthBiometricFaceView { @Override public void onAuthenticationFailed( @Modality int modality, @Nullable String failureReason) { - if (modality == TYPE_FACE && mActiveSensorType == TYPE_FACE) { - // switching from face -> fingerprint mode, suppress soft error messages - failureReason = mContext.getString(R.string.fingerprint_dialog_use_fingerprint_instead); + super.onAuthenticationFailed(modality, checkErrorForFallback(failureReason)); + } + + @Override + public void onError(int modality, String error) { + super.onError(modality, checkErrorForFallback(error)); + } + + private String checkErrorForFallback(String message) { + if (mActiveSensorType == TYPE_FACE) { + Log.d(TAG, "Falling back to fingerprint: " + message); + + // switching from face -> fingerprint mode, suppress root error messages + mCallback.onAction(Callback.ACTION_START_DELAYED_FINGERPRINT_SENSOR); + return mContext.getString(R.string.fingerprint_dialog_use_fingerprint_instead); } - super.onAuthenticationFailed(modality, failureReason); + return message; } @Override @BiometricState protected int getStateForAfterError() { if (mActiveSensorType == TYPE_FACE) { - mHandler.post(() -> mCallback.onAction( - Callback.ACTION_START_DELAYED_FINGERPRINT_SENSOR)); return STATE_AUTHENTICATING; } diff --git a/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthBiometricFaceToFingerprintViewTest.java b/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthBiometricFaceToFingerprintViewTest.java index aed0da656e86b..82bf041e42729 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthBiometricFaceToFingerprintViewTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/biometrics/AuthBiometricFaceToFingerprintViewTest.java @@ -22,6 +22,8 @@ import static org.junit.Assert.assertEquals; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; import android.content.Context; @@ -124,16 +126,50 @@ public class AuthBiometricFaceToFingerprintViewTest extends SysuiTestCase { } @Test - public void testModeUpdated_whenSwitchToFingerprint() { + public void testModeUpdated_onSoftError_whenSwitchToFingerprint() { mFaceToFpView.onDialogAnimatedIn(); mFaceToFpView.onAuthenticationFailed(TYPE_FACE, "no face"); waitForIdleSync(); verify(mIndicatorView).setText( eq(mContext.getString(R.string.fingerprint_dialog_use_fingerprint_instead))); + verify(mCallback).onAction( + eq(AuthBiometricView.Callback.ACTION_START_DELAYED_FINGERPRINT_SENSOR)); assertEquals(AuthBiometricFaceToFingerprintView.STATE_AUTHENTICATING, mFaceToFpView.mState); } + @Test + public void testModeUpdated_onHardError_whenSwitchToFingerprint() { + mFaceToFpView.onDialogAnimatedIn(); + mFaceToFpView.onError(TYPE_FACE, "oh no!"); + waitForIdleSync(); + + verify(mIndicatorView).setText( + eq(mContext.getString(R.string.fingerprint_dialog_use_fingerprint_instead))); + verify(mCallback).onAction( + eq(AuthBiometricView.Callback.ACTION_START_DELAYED_FINGERPRINT_SENSOR)); + assertEquals(AuthBiometricFaceToFingerprintView.STATE_AUTHENTICATING, mFaceToFpView.mState); + } + + @Test + public void testFingerprintOnlyStartsOnFirstError() { + mFaceToFpView.onDialogAnimatedIn(); + verify(mFaceToFpView.mIconController) + .updateState(anyInt(), eq(AuthBiometricFaceToFingerprintView.STATE_AUTHENTICATING)); + + mFaceToFpView.onDialogAnimatedIn(); + mFaceToFpView.updateState(AuthBiometricFaceToFingerprintView.STATE_ERROR); + mFaceToFpView.updateState(AuthBiometricFaceToFingerprintView.STATE_AUTHENTICATING); + + reset(mCallback); + + mFaceToFpView.onError(TYPE_FACE, "oh no!"); + mFaceToFpView.onAuthenticationFailed(TYPE_FACE, "no face"); + + verify(mCallback, never()).onAction( + eq(AuthBiometricView.Callback.ACTION_START_DELAYED_FINGERPRINT_SENSOR)); + } + public class TestableView extends AuthBiometricFaceToFingerprintView { public TestableView(Context context) { super(context, null, new MockInjector()); diff --git a/services/core/java/com/android/server/biometrics/AuthSession.java b/services/core/java/com/android/server/biometrics/AuthSession.java index 36dc5cd21288e..ef02a472969b3 100644 --- a/services/core/java/com/android/server/biometrics/AuthSession.java +++ b/services/core/java/com/android/server/biometrics/AuthSession.java @@ -355,10 +355,6 @@ public final class AuthSession implements IBinder.DeathRecipient { } } - private void cancelAllFingerprintSensors() { - cancelAllSensors(sensor -> sensor.modality == TYPE_FINGERPRINT); - } - private void cancelAllSensors() { cancelAllSensors(sensor -> true); } @@ -387,6 +383,9 @@ public final class AuthSession implements IBinder.DeathRecipient { */ boolean onErrorReceived(int sensorId, int cookie, @BiometricConstants.Errors int error, int vendorCode) throws RemoteException { + if (DEBUG) { + Slog.v(TAG, "onErrorReceived sensor: " + sensorId + " error: " + error); + } if (!containsCookie(cookie)) { Slog.e(TAG, "Unknown/expired cookie: " + cookie); @@ -454,12 +453,14 @@ public final class AuthSession implements IBinder.DeathRecipient { // a round trip to SystemUI. mClientReceiver.onError(modality, error, vendorCode); return true; - } else if (shouldErrorTriggerMultiSensorTransition()) { - // wait for the UI to signal when modality should switch - mMultiSensorState = MULTI_SENSOR_STATE_SWITCHING; - Slog.d(TAG, "onErrorReceived: waiting for modality switch callback"); } else { mState = STATE_ERROR_PENDING_SYSUI; + if (mMultiSensorMode == BIOMETRIC_MULTI_SENSOR_FACE_THEN_FINGERPRINT + && mMultiSensorState == MULTI_SENSOR_STATE_FACE_SCANNING) { + // wait for the UI to signal when modality should switch + Slog.d(TAG, "onErrorReceived: waiting for modality switch callback"); + mMultiSensorState = MULTI_SENSOR_STATE_SWITCHING; + } mStatusBarService.onBiometricError(modality, error, vendorCode); } break; @@ -627,7 +628,7 @@ public final class AuthSession implements IBinder.DeathRecipient { void onDeviceCredentialPressed() { // Cancel authentication. Skip the token/package check since we are cancelling // from system server. The interface is permission protected so this is fine. - cancelBiometricOnly(); + cancelAllSensors(); mState = STATE_SHOWING_DEVICE_CREDENTIAL; } @@ -733,12 +734,10 @@ public final class AuthSession implements IBinder.DeathRecipient { } mClientReceiver.onAuthenticationSucceeded( Utils.getAuthenticationTypeForResult(reason)); - cancelBiometricOnly(); break; case BiometricPrompt.DISMISSED_REASON_NEGATIVE: mClientReceiver.onDialogDismissed(reason); - cancelBiometricOnly(); break; case BiometricPrompt.DISMISSED_REASON_USER_CANCEL: @@ -747,7 +746,6 @@ public final class AuthSession implements IBinder.DeathRecipient { BiometricConstants.BIOMETRIC_ERROR_USER_CANCELED, 0 /* vendorCode */ ); - cancelBiometricOnly(); break; case BiometricPrompt.DISMISSED_REASON_SERVER_REQUESTED: @@ -765,6 +763,9 @@ public final class AuthSession implements IBinder.DeathRecipient { } } catch (RemoteException e) { Slog.e(TAG, "Remote exception", e); + } finally { + // ensure everything is cleaned up when dismissed + cancelAllSensors(); } } @@ -780,8 +781,8 @@ public final class AuthSession implements IBinder.DeathRecipient { || mState == STATE_AUTH_STARTED || mState == STATE_AUTH_STARTED_UI_SHOWING; + cancelAllSensors(); if (authStarted && !force) { - cancelAllSensors(); // Wait for ERROR_CANCELED to be returned from the sensors return false; } else { @@ -804,22 +805,6 @@ public final class AuthSession implements IBinder.DeathRecipient { return false; } - /** - * Cancels biometric authentication only. AuthSession may either be going into - * {@link #STATE_SHOWING_DEVICE_CREDENTIAL} or dismissed. - */ - private void cancelBiometricOnly() { - if (mState == STATE_AUTH_STARTED || mState == STATE_AUTH_STARTED_UI_SHOWING) { - cancelAllSensors(); - } else if (mMultiSensorMode == BIOMETRIC_MULTI_SENSOR_FACE_THEN_FINGERPRINT) { - cancelAllFingerprintSensors(); - } else { - if (DEBUG) { - Slog.v(TAG, "nothing to cancel - wrong state: " + mState); - } - } - } - boolean isCrypto() { return mOperationId != 0; } @@ -896,13 +881,6 @@ public final class AuthSession implements IBinder.DeathRecipient { } } - private boolean shouldErrorTriggerMultiSensorTransition() { - if (mMultiSensorMode == BIOMETRIC_MULTI_SENSOR_FACE_THEN_FINGERPRINT) { - return mMultiSensorState == MULTI_SENSOR_STATE_FACE_SCANNING; - } - return false; - } - @BiometricMultiSensorMode private static int getMultiSensorModeForNewSession(Collection sensors) { boolean hasFace = false; diff --git a/services/tests/servicestests/src/com/android/server/biometrics/AuthSessionTest.java b/services/tests/servicestests/src/com/android/server/biometrics/AuthSessionTest.java index 4afb7dd2ba82e..2892bf58a7d2a 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/AuthSessionTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/AuthSessionTest.java @@ -18,6 +18,7 @@ package com.android.server.biometrics; import static android.hardware.biometrics.BiometricAuthenticator.TYPE_FACE; import static android.hardware.biometrics.BiometricAuthenticator.TYPE_FINGERPRINT; +import static android.hardware.biometrics.BiometricPrompt.DISMISSED_REASON_NEGATIVE; import static com.android.server.biometrics.BiometricServiceStateProto.*; @@ -38,7 +39,6 @@ import android.annotation.NonNull; import android.app.admin.DevicePolicyManager; import android.app.trust.ITrustManager; import android.content.Context; -import android.hardware.biometrics.BiometricConstants; import android.hardware.biometrics.BiometricManager.Authenticators; import android.hardware.biometrics.ComponentInfoInternal; import android.hardware.biometrics.IBiometricAuthenticator; @@ -67,6 +67,7 @@ import org.mockito.MockitoAnnotations; import java.util.ArrayList; import java.util.List; import java.util.Random; +import java.util.function.Consumer; @Presubmit @SmallTest @@ -245,9 +246,6 @@ public class AuthSessionTest { assertEquals(STATE_AUTH_STARTED_UI_SHOWING, session.getState()); assertEquals(BiometricSensor.STATE_COOKIE_RETURNED, session.mPreAuthInfo.eligibleSensors.get(fingerprintSensorId).getSensorState()); - session.onErrorReceived(fingerprintSensorId, - session.mPreAuthInfo.eligibleSensors.get(fingerprintSensorId).getCookie(), - BiometricConstants.BIOMETRIC_ERROR_VENDOR, 0 /* vendorCode */); session.onStartFingerprint(); } assertEquals(STATE_AUTH_STARTED_UI_SHOWING, session.getState()); @@ -258,6 +256,21 @@ public class AuthSessionTest { @Test public void testCancelAuthentication_whenStateAuthCalled_invokesCancel() throws RemoteException { + testInvokesCancel(session -> session.onCancelAuthSession(false /* force */)); + } + + @Test + public void testCancelAuthentication_whenStateAuthForcedCalled_invokesCancel() + throws RemoteException { + testInvokesCancel(session -> session.onCancelAuthSession(true /* force */)); + } + + @Test + public void testCancelAuthentication_whenDialogDismissed() throws RemoteException { + testInvokesCancel(session -> session.onDialogDismissed(DISMISSED_REASON_NEGATIVE, null)); + } + + private void testInvokesCancel(Consumer sessionConsumer) throws RemoteException { final IBiometricAuthenticator faceAuthenticator = mock(IBiometricAuthenticator.class); setupFace(0 /* id */, false /* confirmationAlwaysRequired */, faceAuthenticator); @@ -269,7 +282,8 @@ public class AuthSessionTest { session.goToInitialState(); assertEquals(STATE_AUTH_CALLED, session.getState()); - session.onCancelAuthSession(false /* force */); + + sessionConsumer.accept(session); verify(faceAuthenticator).cancelAuthenticationFromService(eq(mToken), eq(TEST_PACKAGE)); } diff --git a/services/tests/servicestests/src/com/android/server/biometrics/BiometricServiceTest.java b/services/tests/servicestests/src/com/android/server/biometrics/BiometricServiceTest.java index ae836cead3b8d..7c7afb7dde36d 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/BiometricServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/BiometricServiceTest.java @@ -1036,7 +1036,7 @@ public class BiometricServiceTest { } @Test - public void testDismissedReasonNegative_whilePaused_doesntInvokeHalCancel() throws Exception { + public void testDismissedReasonNegative_whilePaused_invokeHalCancel() throws Exception { setupAuthForOnly(BiometricAuthenticator.TYPE_FACE, Authenticators.BIOMETRIC_STRONG); invokeAuthenticateAndStart(mBiometricService.mImpl, mReceiver1, false /* requireConfirmation */, null /* authenticators */); @@ -1050,14 +1050,12 @@ public class BiometricServiceTest { BiometricPrompt.DISMISSED_REASON_NEGATIVE, null /* credentialAttestation */); waitForIdle(); - verify(mBiometricService.mSensors.get(0).impl, - never()).cancelAuthenticationFromService( - any(), - any()); + verify(mBiometricService.mSensors.get(0).impl) + .cancelAuthenticationFromService(any(), any()); } @Test - public void testDismissedReasonUserCancel_whilePaused_doesntInvokeHalCancel() throws + public void testDismissedReasonUserCancel_whilePaused_invokesHalCancel() throws Exception { setupAuthForOnly(BiometricAuthenticator.TYPE_FACE, Authenticators.BIOMETRIC_STRONG); invokeAuthenticateAndStart(mBiometricService.mImpl, mReceiver1, @@ -1072,10 +1070,8 @@ public class BiometricServiceTest { BiometricPrompt.DISMISSED_REASON_USER_CANCEL, null /* credentialAttestation */); waitForIdle(); - verify(mBiometricService.mSensors.get(0).impl, - never()).cancelAuthenticationFromService( - any(), - any()); + verify(mBiometricService.mSensors.get(0).impl) + .cancelAuthenticationFromService(any(), any()); } @Test @@ -1091,11 +1087,8 @@ public class BiometricServiceTest { BiometricPrompt.DISMISSED_REASON_USER_CANCEL, null /* credentialAttestation */); waitForIdle(); - // doesn't send cancel to HAL - verify(mBiometricService.mSensors.get(0).impl, - never()).cancelAuthenticationFromService( - any(), - any()); + verify(mBiometricService.mSensors.get(0).impl) + .cancelAuthenticationFromService(any(), any()); verify(mReceiver1).onError( eq(BiometricAuthenticator.TYPE_FACE), eq(BiometricConstants.BIOMETRIC_ERROR_USER_CANCELED),