Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b0551399 authored by Kevin Chyn's avatar Kevin Chyn
Browse files

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
parent 4a468bd4
Loading
Loading
Loading
Loading
+27 −1
Original line number Diff line number Diff line
@@ -180,7 +180,8 @@ public abstract class AuthenticationClient<T> extends AcquisitionClient<T>
                + ", 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 abstract class AuthenticationClient<T> extends AcquisitionClient<T>
                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 abstract class AuthenticationClient<T> extends AcquisitionClient<T>
                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);
+15 −2
Original line number Diff line number Diff line
@@ -73,6 +73,11 @@ public class CoexCoordinator {
         * 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 @@ public class CoexCoordinator {
    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 @@ public class CoexCoordinator {
    }

    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);
            }
+14 −9
Original line number Diff line number Diff line
@@ -255,13 +255,16 @@ public class CoexCoordinatorTest {
        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 @@ public class CoexCoordinatorTest {
            verify(udfpsCallback).sendAuthenticationResult(true /* addAuthTokenIfStrong */);
            verify(udfpsCallback).handleLifecycleAfterAuth();

            verify(faceCallback).sendAuthenticationCanceled();

            assertTrue(mCoexCoordinator.mSuccessfulAuths.isEmpty());
        } else {
            mCoexCoordinator.onAuthenticationRejected(udfpsRejectedAfterMs, udfpsClient,
@@ -281,16 +286,16 @@ public class CoexCoordinatorTest {
            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)