Ensure authentication lifecycle ends for callers
Bug: 194970282
Bug: 193089985
In coex, we introduced the capability to temporarily store face
auth results if fingerprint auth was still in progress. However,
upon successful fingerprint auth, we forgot to notify the caller
that fingerprint lifecycle is ended.
Since the purpose of coex is to distill auth callbacks for keyguard,
we should send ERROR_CANCELED for face, since fingerprint sends
auth success.
Test: atest CoexCoordinatorTest
Test: manual, see new "Removing from queue, canceling, and finishing"
log. Keyguard face auth is no longer wedged
Change-Id: If809a9338f7a96bb1f039626bff8efacbcec509e
diff --git a/services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java b/services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java
index f4327e8..705b3aa 100644
--- a/services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java
+++ b/services/core/java/com/android/server/biometrics/sensors/AuthenticationClient.java
@@ -180,7 +180,8 @@
+ ", isBP: " + isBiometricPrompt()
+ ", listener: " + listener
+ ", requireConfirmation: " + mRequireConfirmation
- + ", user: " + getTargetUserId());
+ + ", user: " + getTargetUserId()
+ + ", clientMonitor: " + toString());
final PerformanceTracker pm = PerformanceTracker.getInstanceForSensorId(getSensorId());
if (isCryptoOperation()) {
@@ -304,6 +305,11 @@
public void handleLifecycleAfterAuth() {
AuthenticationClient.this.handleLifecycleAfterAuth(true /* authenticated */);
}
+
+ @Override
+ public void sendAuthenticationCanceled() {
+ sendCancelOnly(listener);
+ }
});
} else {
// Allow system-defined limit of number of attempts before giving up
@@ -338,10 +344,30 @@
public void handleLifecycleAfterAuth() {
AuthenticationClient.this.handleLifecycleAfterAuth(false /* authenticated */);
}
+
+ @Override
+ public void sendAuthenticationCanceled() {
+ sendCancelOnly(listener);
+ }
});
}
}
+ private void sendCancelOnly(@Nullable ClientMonitorCallbackConverter listener) {
+ if (listener == null) {
+ Slog.e(TAG, "Unable to sendAuthenticationCanceled, listener null");
+ return;
+ }
+ try {
+ listener.onError(getSensorId(),
+ getCookie(),
+ BiometricConstants.BIOMETRIC_ERROR_CANCELED,
+ 0 /* vendorCode */);
+ } catch (RemoteException e) {
+ Slog.e(TAG, "Remote exception", e);
+ }
+ }
+
@Override
public void onAcquired(int acquiredInfo, int vendorCode) {
super.onAcquired(acquiredInfo, vendorCode);
diff --git a/services/core/java/com/android/server/biometrics/sensors/CoexCoordinator.java b/services/core/java/com/android/server/biometrics/sensors/CoexCoordinator.java
index a15ecad..6d32fde 100644
--- a/services/core/java/com/android/server/biometrics/sensors/CoexCoordinator.java
+++ b/services/core/java/com/android/server/biometrics/sensors/CoexCoordinator.java
@@ -73,6 +73,11 @@
* from scheduler if auth was successful).
*/
void handleLifecycleAfterAuth();
+
+ /**
+ * Requests the owner to notify the caller that authentication was canceled.
+ */
+ void sendAuthenticationCanceled();
}
private static CoexCoordinator sInstance;
@@ -356,7 +361,11 @@
private SuccessfulAuth popSuccessfulFaceAuthIfExists(long currentTimeMillis) {
for (SuccessfulAuth auth : mSuccessfulAuths) {
if (currentTimeMillis - auth.mAuthTimestamp >= SUCCESSFUL_AUTH_VALID_DURATION_MS) {
- Slog.d(TAG, "Removing stale auth: " + auth);
+ // TODO(b/193089985): This removes the auth but does not notify the client with
+ // an appropriate lifecycle event (such as ERROR_CANCELED), and violates the
+ // API contract. However, this might be OK for now since the validity duration
+ // is way longer than the time it takes to auth with fingerprint.
+ Slog.e(TAG, "Removing stale auth: " + auth);
mSuccessfulAuths.remove(auth);
} else if (auth.mSensorType == SENSOR_TYPE_FACE) {
mSuccessfulAuths.remove(auth);
@@ -367,9 +376,13 @@
}
private void removeAndFinishAllFaceFromQueue() {
+ // Note that these auth are all successful, but have never notified the client (e.g.
+ // keyguard). To comply with the authentication lifecycle, we must notify the client that
+ // auth is "done". The safest thing to do is to send ERROR_CANCELED.
for (SuccessfulAuth auth : mSuccessfulAuths) {
if (auth.mSensorType == SENSOR_TYPE_FACE) {
- Slog.d(TAG, "Removing from queue and finishing: " + auth);
+ Slog.d(TAG, "Removing from queue, canceling, and finishing: " + auth);
+ auth.mCallback.sendAuthenticationCanceled();
auth.mCallback.handleLifecycleAfterAuth();
mSuccessfulAuths.remove(auth);
}
diff --git a/services/tests/servicestests/src/com/android/server/biometrics/sensors/CoexCoordinatorTest.java b/services/tests/servicestests/src/com/android/server/biometrics/sensors/CoexCoordinatorTest.java
index 7f5f3c2..1263f7b 100644
--- a/services/tests/servicestests/src/com/android/server/biometrics/sensors/CoexCoordinatorTest.java
+++ b/services/tests/servicestests/src/com/android/server/biometrics/sensors/CoexCoordinatorTest.java
@@ -255,13 +255,16 @@
mCoexCoordinator.addAuthenticationClient(SENSOR_TYPE_FACE, faceClient);
mCoexCoordinator.addAuthenticationClient(SENSOR_TYPE_UDFPS, udfpsClient);
+ // For easier reading
+ final CoexCoordinator.Callback faceCallback = mCallback;
+
mCoexCoordinator.onAuthenticationSucceeded(0 /* currentTimeMillis */, faceClient,
- mCallback);
- verify(mCallback, never()).sendHapticFeedback();
- verify(mCallback, never()).sendAuthenticationResult(anyBoolean());
+ faceCallback);
+ verify(faceCallback, never()).sendHapticFeedback();
+ verify(faceCallback, never()).sendAuthenticationResult(anyBoolean());
// CoexCoordinator requests the system to hold onto this AuthenticationClient until
// UDFPS result is known
- verify(mCallback, never()).handleLifecycleAfterAuth();
+ verify(faceCallback, never()).handleLifecycleAfterAuth();
// Reset the mock
CoexCoordinator.Callback udfpsCallback = mock(CoexCoordinator.Callback.class);
@@ -274,6 +277,8 @@
verify(udfpsCallback).sendAuthenticationResult(true /* addAuthTokenIfStrong */);
verify(udfpsCallback).handleLifecycleAfterAuth();
+ verify(faceCallback).sendAuthenticationCanceled();
+
assertTrue(mCoexCoordinator.mSuccessfulAuths.isEmpty());
} else {
mCoexCoordinator.onAuthenticationRejected(udfpsRejectedAfterMs, udfpsClient,
@@ -281,16 +286,16 @@
if (udfpsRejectedAfterMs <= CoexCoordinator.SUCCESSFUL_AUTH_VALID_DURATION_MS) {
verify(udfpsCallback, never()).sendHapticFeedback();
- verify(mCallback).sendHapticFeedback();
- verify(mCallback).sendAuthenticationResult(eq(true) /* addAuthTokenIfStrong */);
- verify(mCallback).handleLifecycleAfterAuth();
+ verify(faceCallback).sendHapticFeedback();
+ verify(faceCallback).sendAuthenticationResult(eq(true) /* addAuthTokenIfStrong */);
+ verify(faceCallback).handleLifecycleAfterAuth();
assertTrue(mCoexCoordinator.mSuccessfulAuths.isEmpty());
} else {
assertTrue(mCoexCoordinator.mSuccessfulAuths.isEmpty());
- verify(mCallback, never()).sendHapticFeedback();
- verify(mCallback, never()).sendAuthenticationResult(anyBoolean());
+ verify(faceCallback, never()).sendHapticFeedback();
+ verify(faceCallback, never()).sendAuthenticationResult(anyBoolean());
verify(udfpsCallback).sendHapticFeedback();
verify(udfpsCallback)