Merge changes I171604ee,I81d648e5 into tm-dev

* changes:
  Do nothing in duplicate onDialogAnimatedIn calls
  Don't crash on illegal biometric states
This commit is contained in:
Ilya Matyukhin
2022-06-29 22:29:42 +00:00
committed by Android (Google) Code Review
4 changed files with 207 additions and 26 deletions

View File

@@ -538,13 +538,12 @@ public final class AuthSession implements IBinder.DeathRecipient {
void onDialogAnimatedIn() {
if (mState != STATE_AUTH_STARTED) {
Slog.w(TAG, "onDialogAnimatedIn, unexpected state: " + mState);
Slog.e(TAG, "onDialogAnimatedIn, unexpected state: " + mState);
return;
}
mState = STATE_AUTH_STARTED_UI_SHOWING;
startAllPreparedFingerprintSensors();
mState = STATE_AUTH_STARTED_UI_SHOWING;
}
void onTryAgainPressed() {

View File

@@ -20,14 +20,18 @@ import android.annotation.IntDef;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.hardware.biometrics.BiometricConstants;
import android.os.Build;
import android.os.Handler;
import android.os.IBinder;
import android.util.Slog;
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.util.ArrayUtils;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.util.Arrays;
import java.util.function.BooleanSupplier;
/**
* Contains all the necessary information for a HAL operation.
@@ -84,6 +88,8 @@ public class BiometricSchedulerOperation {
private final BaseClientMonitor mClientMonitor;
@Nullable
private final ClientMonitorCallback mClientCallback;
@NonNull
private final BooleanSupplier mIsDebuggable;
@Nullable
private ClientMonitorCallback mOnStartCallback;
@OperationState
@@ -99,14 +105,33 @@ public class BiometricSchedulerOperation {
this(clientMonitor, callback, STATE_WAITING_IN_QUEUE);
}
@VisibleForTesting
BiometricSchedulerOperation(
@NonNull BaseClientMonitor clientMonitor,
@Nullable ClientMonitorCallback callback,
@NonNull BooleanSupplier isDebuggable
) {
this(clientMonitor, callback, STATE_WAITING_IN_QUEUE, isDebuggable);
}
protected BiometricSchedulerOperation(
@NonNull BaseClientMonitor clientMonitor,
@Nullable ClientMonitorCallback callback,
@OperationState int state
) {
this(clientMonitor, callback, state, Build::isDebuggable);
}
private BiometricSchedulerOperation(
@NonNull BaseClientMonitor clientMonitor,
@Nullable ClientMonitorCallback callback,
@OperationState int state,
@NonNull BooleanSupplier isDebuggable
) {
mClientMonitor = clientMonitor;
mClientCallback = callback;
mState = state;
mIsDebuggable = isDebuggable;
mCancelWatchdog = () -> {
if (!isFinished()) {
Slog.e(TAG, "[Watchdog Triggered]: " + this);
@@ -144,13 +169,19 @@ public class BiometricSchedulerOperation {
* @return if this operation started
*/
public boolean start(@NonNull ClientMonitorCallback callback) {
checkInState("start",
if (errorWhenNoneOf("start",
STATE_WAITING_IN_QUEUE,
STATE_WAITING_FOR_COOKIE,
STATE_WAITING_IN_QUEUE_CANCELING);
STATE_WAITING_IN_QUEUE_CANCELING)) {
return false;
}
if (mClientMonitor.getCookie() != 0) {
throw new IllegalStateException("operation requires cookie");
String err = "operation requires cookie";
if (mIsDebuggable.getAsBoolean()) {
throw new IllegalStateException(err);
}
Slog.e(TAG, err);
}
return doStart(callback);
@@ -164,16 +195,18 @@ public class BiometricSchedulerOperation {
* @return if this operation started
*/
public boolean startWithCookie(@NonNull ClientMonitorCallback callback, int cookie) {
checkInState("start",
STATE_WAITING_IN_QUEUE,
STATE_WAITING_FOR_COOKIE,
STATE_WAITING_IN_QUEUE_CANCELING);
if (mClientMonitor.getCookie() != cookie) {
Slog.e(TAG, "Mismatched cookie for operation: " + this + ", received: " + cookie);
return false;
}
if (errorWhenNoneOf("start",
STATE_WAITING_IN_QUEUE,
STATE_WAITING_FOR_COOKIE,
STATE_WAITING_IN_QUEUE_CANCELING)) {
return false;
}
return doStart(callback);
}
@@ -217,10 +250,12 @@ public class BiometricSchedulerOperation {
* immediately abort the operation and notify the client that it has finished unsuccessfully.
*/
public void abort() {
checkInState("cannot abort a non-pending operation",
if (errorWhenNoneOf("abort",
STATE_WAITING_IN_QUEUE,
STATE_WAITING_FOR_COOKIE,
STATE_WAITING_IN_QUEUE_CANCELING);
STATE_WAITING_IN_QUEUE_CANCELING)) {
return;
}
if (isHalOperation()) {
((HalClientMonitor<?>) mClientMonitor).unableToStart();
@@ -247,7 +282,9 @@ public class BiometricSchedulerOperation {
* the callback used from {@link #start(ClientMonitorCallback)} is used)
*/
public void cancel(@NonNull Handler handler, @NonNull ClientMonitorCallback callback) {
checkNotInState("cancel", STATE_FINISHED);
if (errorWhenOneOf("cancel", STATE_FINISHED)) {
return;
}
final int currentState = mState;
if (!isInterruptable()) {
@@ -402,21 +439,28 @@ public class BiometricSchedulerOperation {
return mClientMonitor;
}
private void checkNotInState(String message, @OperationState int... states) {
for (int state : states) {
if (mState == state) {
throw new IllegalStateException(message + ": illegal state= " + state);
private boolean errorWhenOneOf(String op, @OperationState int... states) {
final boolean isError = ArrayUtils.contains(states, mState);
if (isError) {
String err = op + ": mState must not be " + mState;
if (mIsDebuggable.getAsBoolean()) {
throw new IllegalStateException(err);
}
Slog.e(TAG, err);
}
return isError;
}
private void checkInState(String message, @OperationState int... states) {
for (int state : states) {
if (mState == state) {
return;
private boolean errorWhenNoneOf(String op, @OperationState int... states) {
final boolean isError = !ArrayUtils.contains(states, mState);
if (isError) {
String err = op + ": mState=" + mState + " must be one of " + Arrays.toString(states);
if (mIsDebuggable.getAsBoolean()) {
throw new IllegalStateException(err);
}
Slog.e(TAG, err);
}
throw new IllegalStateException(message + ": illegal state= " + mState);
return isError;
}
@Override

View File

@@ -20,7 +20,9 @@ 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.*;
import static com.android.server.biometrics.BiometricServiceStateProto.STATE_AUTH_CALLED;
import static com.android.server.biometrics.BiometricServiceStateProto.STATE_AUTH_STARTED;
import static com.android.server.biometrics.BiometricServiceStateProto.STATE_AUTH_STARTED_UI_SHOWING;
import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertFalse;
@@ -32,6 +34,8 @@ import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -279,6 +283,43 @@ public class AuthSessionTest {
session.mPreAuthInfo.eligibleSensors.get(fingerprintSensorId).getSensorState());
}
@Test
public void testOnDialogAnimatedInDoesNothingDuringInvalidState() throws Exception {
setupFingerprint(0 /* id */, FingerprintSensorProperties.TYPE_UDFPS_OPTICAL);
final long operationId = 123;
final int userId = 10;
final AuthSession session = createAuthSession(mSensors,
false /* checkDevicePolicyManager */,
Authenticators.BIOMETRIC_STRONG,
TEST_REQUEST_ID,
operationId,
userId);
final IBiometricAuthenticator impl = session.mPreAuthInfo.eligibleSensors.get(0).impl;
session.goToInitialState();
for (BiometricSensor sensor : session.mPreAuthInfo.eligibleSensors) {
assertEquals(BiometricSensor.STATE_WAITING_FOR_COOKIE, sensor.getSensorState());
session.onCookieReceived(
session.mPreAuthInfo.eligibleSensors.get(sensor.id).getCookie());
}
assertTrue(session.allCookiesReceived());
assertEquals(STATE_AUTH_STARTED, session.getState());
verify(impl, never()).startPreparedClient(anyInt());
// First invocation should start the client monitor.
session.onDialogAnimatedIn();
assertEquals(STATE_AUTH_STARTED_UI_SHOWING, session.getState());
verify(impl).startPreparedClient(anyInt());
// Subsequent invocations should not start the client monitor again.
session.onDialogAnimatedIn();
session.onDialogAnimatedIn();
session.onDialogAnimatedIn();
assertEquals(STATE_AUTH_STARTED_UI_SHOWING, session.getState());
verify(impl, times(1)).startPreparedClient(anyInt());
}
@Test
public void testCancelAuthentication_whenStateAuthCalled_invokesCancel()
throws RemoteException {

View File

@@ -80,11 +80,14 @@ public class BiometricSchedulerOperationTest {
private Handler mHandler;
private BiometricSchedulerOperation mOperation;
private boolean mIsDebuggable;
@Before
public void setUp() {
mHandler = new Handler(TestableLooper.get(this).getLooper());
mOperation = new BiometricSchedulerOperation(mClientMonitor, mClientCallback);
mIsDebuggable = false;
mOperation = new BiometricSchedulerOperation(mClientMonitor, mClientCallback,
() -> mIsDebuggable);
}
@Test
@@ -125,6 +128,34 @@ public class BiometricSchedulerOperationTest {
verify(mClientMonitor, never()).start(any());
}
@Test
public void testSecondStartWithCookieCrashesWhenDebuggable() {
final int cookie = 5;
mIsDebuggable = true;
when(mClientMonitor.getCookie()).thenReturn(cookie);
when(mClientMonitor.getFreshDaemon()).thenReturn(mHal);
final boolean started = mOperation.startWithCookie(mOnStartCallback, cookie);
assertThat(started).isTrue();
assertThrows(IllegalStateException.class,
() -> mOperation.startWithCookie(mOnStartCallback, cookie));
}
@Test
public void testSecondStartWithCookieFailsNicelyWhenNotDebuggable() {
final int cookie = 5;
mIsDebuggable = false;
when(mClientMonitor.getCookie()).thenReturn(cookie);
when(mClientMonitor.getFreshDaemon()).thenReturn(mHal);
final boolean started = mOperation.startWithCookie(mOnStartCallback, cookie);
assertThat(started).isTrue();
final boolean startedAgain = mOperation.startWithCookie(mOnStartCallback, cookie);
assertThat(startedAgain).isFalse();
}
@Test
public void startsWhenReadyAndHalAvailable() {
when(mClientMonitor.getCookie()).thenReturn(0);
@@ -169,8 +200,35 @@ public class BiometricSchedulerOperationTest {
verify(mOnStartCallback).onClientFinished(eq(mClientMonitor), eq(false));
}
@Test
public void secondStartCrashesWhenDebuggable() {
mIsDebuggable = true;
when(mClientMonitor.getCookie()).thenReturn(0);
when(mClientMonitor.getFreshDaemon()).thenReturn(mHal);
final boolean started = mOperation.start(mOnStartCallback);
assertThat(started).isTrue();
assertThrows(IllegalStateException.class, () -> mOperation.start(mOnStartCallback));
}
@Test
public void secondStartFailsNicelyWhenNotDebuggable() {
mIsDebuggable = false;
when(mClientMonitor.getCookie()).thenReturn(0);
when(mClientMonitor.getFreshDaemon()).thenReturn(mHal);
final boolean started = mOperation.start(mOnStartCallback);
assertThat(started).isTrue();
final boolean startedAgain = mOperation.start(mOnStartCallback);
assertThat(startedAgain).isFalse();
}
@Test
public void doesNotStartWithCookie() {
// This class only throws exceptions when debuggable.
mIsDebuggable = true;
when(mClientMonitor.getCookie()).thenReturn(9);
assertThrows(IllegalStateException.class,
() -> mOperation.start(mock(ClientMonitorCallback.class)));
@@ -178,6 +236,8 @@ public class BiometricSchedulerOperationTest {
@Test
public void cannotRestart() {
// This class only throws exceptions when debuggable.
mIsDebuggable = true;
when(mClientMonitor.getFreshDaemon()).thenReturn(mHal);
mOperation.start(mOnStartCallback);
@@ -188,6 +248,8 @@ public class BiometricSchedulerOperationTest {
@Test
public void abortsNotRunning() {
// This class only throws exceptions when debuggable.
mIsDebuggable = true;
when(mClientMonitor.getFreshDaemon()).thenReturn(mHal);
mOperation.abort();
@@ -200,7 +262,8 @@ public class BiometricSchedulerOperationTest {
}
@Test
public void cannotAbortRunning() {
public void abortCrashesWhenDebuggableIfOperationIsRunning() {
mIsDebuggable = true;
when(mClientMonitor.getFreshDaemon()).thenReturn(mHal);
mOperation.start(mOnStartCallback);
@@ -208,6 +271,16 @@ public class BiometricSchedulerOperationTest {
assertThrows(IllegalStateException.class, () -> mOperation.abort());
}
@Test
public void abortFailsNicelyWhenNotDebuggableIfOperationIsRunning() {
mIsDebuggable = false;
when(mClientMonitor.getFreshDaemon()).thenReturn(mHal);
mOperation.start(mOnStartCallback);
mOperation.abort();
}
@Test
public void cancel() {
when(mClientMonitor.getFreshDaemon()).thenReturn(mHal);
@@ -253,6 +326,30 @@ public class BiometricSchedulerOperationTest {
verify(mClientMonitor).destroy();
}
@Test
public void cancelCrashesWhenDebuggableIfOperationIsFinished() {
mIsDebuggable = true;
when(mClientMonitor.getFreshDaemon()).thenReturn(mHal);
mOperation.abort();
assertThat(mOperation.isFinished()).isTrue();
final ClientMonitorCallback cancelCb = mock(ClientMonitorCallback.class);
assertThrows(IllegalStateException.class, () -> mOperation.cancel(mHandler, cancelCb));
}
@Test
public void cancelFailsNicelyWhenNotDebuggableIfOperationIsFinished() {
mIsDebuggable = false;
when(mClientMonitor.getFreshDaemon()).thenReturn(mHal);
mOperation.abort();
assertThat(mOperation.isFinished()).isTrue();
final ClientMonitorCallback cancelCb = mock(ClientMonitorCallback.class);
mOperation.cancel(mHandler, cancelCb);
}
@Test
public void markCanceling() {
when(mClientMonitor.getFreshDaemon()).thenReturn(mHal);