From 156db33a56b2c295d3cf711c33df36e7f21312de Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Thu, 12 Aug 2021 16:03:51 -0700 Subject: [PATCH] Disable combined controller only if all modalities require consent Additionally, ensure that consent is only requested for modalities that have not been previously consented to. We retrieve the consent requirement (which come from DPM) in onActivityResult instead of onCreate, since the signal may not be ready immediately. Fixes: 196060286 Fixes: 204592495 Test: make -j56 RunSettingsRoboTests ROBOTEST_FILTER=CombinedBiometricStatusPreferenceControllerTest Change-Id: I984e61f28ffbf957c16cac4ea84f40b6ad7d8ae9 --- .../biometrics/BiometricEnrollActivity.java | 41 ++++++++++++++++--- .../biometrics/ParentalConsentHelper.java | 18 ++++---- ...edBiometricStatusPreferenceController.java | 28 +++++++++++-- ...ometricStatusPreferenceControllerTest.java | 12 ++++-- .../biometrics/ParentalConsentHelperTest.java | 4 +- 5 files changed, 81 insertions(+), 22 deletions(-) diff --git a/src/com/android/settings/biometrics/BiometricEnrollActivity.java b/src/com/android/settings/biometrics/BiometricEnrollActivity.java index 88622e931c4..690ef13be7d 100644 --- a/src/com/android/settings/biometrics/BiometricEnrollActivity.java +++ b/src/com/android/settings/biometrics/BiometricEnrollActivity.java @@ -242,12 +242,11 @@ public class BiometricEnrollActivity extends InstrumentedActivity { } } - // start enrollment process if we haven't bailed out yet if (mParentalOptionsRequired && mParentalOptions == null) { - mParentalConsentHelper = new ParentalConsentHelper( - mIsFaceEnrollable, mIsFingerprintEnrollable, mGkPwHandle); + mParentalConsentHelper = new ParentalConsentHelper(mGkPwHandle); setOrConfirmCredentialsNow(); } else { + // Start enrollment process if we haven't bailed out yet startEnroll(); } } @@ -263,7 +262,10 @@ public class BiometricEnrollActivity extends InstrumentedActivity { private void startEnrollWith(@Authenticators.Types int authenticators, boolean setupWizard) { // If the caller is not setup wizard, and the user has something enrolled, finish. - if (!setupWizard) { + // Allow parental consent flow to skip this check, since one modality could be consented + // and another non-consented. This can also happen if the restriction is applied when + // enrollments already exists. + if (!setupWizard && !mParentalOptionsRequired) { final BiometricManager bm = getSystemService(BiometricManager.class); final @BiometricError int result = bm.canAuthenticate(authenticators); if (result != BiometricManager.BIOMETRIC_ERROR_NONE_ENROLLED) { @@ -330,6 +332,25 @@ public class BiometricEnrollActivity extends InstrumentedActivity { // single enrollment is handled entirely by the launched activity // this handles multi enroll or if parental consent is required if (mParentalConsentHelper != null) { + // Lazily retrieve the values from ParentalControlUtils, since the value may not be + // ready in onCreate. + final boolean faceConsentRequired = ParentalControlsUtils + .parentConsentRequired(this, BiometricAuthenticator.TYPE_FACE) != null; + final boolean fpConsentRequired = ParentalControlsUtils + .parentConsentRequired(this, BiometricAuthenticator.TYPE_FINGERPRINT) != null; + + final boolean requestFaceConsent = faceConsentRequired && mHasFeatureFace; + final boolean requestFpConsent = fpConsentRequired && mHasFeatureFingerprint; + + Log.d(TAG, "faceConsentRequired: " + faceConsentRequired + + ", fpConsentRequired: " + fpConsentRequired + + ", hasFeatureFace: " + mHasFeatureFace + + ", hasFeatureFingerprint: " + mHasFeatureFingerprint + + ", faceEnrollable: " + mIsFaceEnrollable + + ", fpEnrollable: " + mIsFingerprintEnrollable); + + mParentalConsentHelper.setConsentRequirement(requestFaceConsent, requestFpConsent); + handleOnActivityResultWhileConsenting(requestCode, resultCode, data); } else { handleOnActivityResultWhileEnrolling(requestCode, resultCode, data); @@ -362,10 +383,18 @@ public class BiometricEnrollActivity extends InstrumentedActivity { final boolean isStillPrompting = mParentalConsentHelper.launchNext( this, REQUEST_CHOOSE_OPTIONS, resultCode, data); if (!isStillPrompting) { - Log.d(TAG, "Enrollment consent options set, starting enrollment"); mParentalOptions = mParentalConsentHelper.getConsentResult(); mParentalConsentHelper = null; - startEnroll(); + Log.d(TAG, "Enrollment consent options set, starting enrollment: " + + mParentalOptions); + // Note that we start enrollment with CONVENIENCE instead of the default + // of WEAK in startEnroll(), since we want to allow enrollment for any + // sensor as long as it has been consented for. We should eventually + // clean up this logic and do something like pass in the parental consent + // result, so that we can request enrollment for specific sensors, but + // that's quite a large and risky change to the startEnrollWith() logic. + startEnrollWith(Authenticators.BIOMETRIC_CONVENIENCE, + WizardManagerHelper.isAnySetupWizard(getIntent())); } } else { Log.d(TAG, "Unknown or cancelled parental consent"); diff --git a/src/com/android/settings/biometrics/ParentalConsentHelper.java b/src/com/android/settings/biometrics/ParentalConsentHelper.java index e0e082b8fef..40ff8ea0581 100644 --- a/src/com/android/settings/biometrics/ParentalConsentHelper.java +++ b/src/com/android/settings/biometrics/ParentalConsentHelper.java @@ -52,8 +52,8 @@ public class ParentalConsentHelper { private static final String KEY_FINGERPRINT_CONSENT_STRINGS = "fingerprint_strings"; private static final String KEY_IRIS_CONSENT_STRINGS = "iris_strings"; - private final boolean mRequireFace; - private final boolean mRequireFingerprint; + private boolean mRequireFace; + private boolean mRequireFingerprint; private long mGkPwHandle; @Nullable @@ -64,15 +64,19 @@ public class ParentalConsentHelper { /** * Helper for aggregating user consent. * - * @param requireFace if face consent should be shown - * @param requireFingerprint if fingerprint consent should be shown * @param gkPwHandle for launched intents */ - public ParentalConsentHelper(boolean requireFace, boolean requireFingerprint, - @Nullable Long gkPwHandle) { + public ParentalConsentHelper(@Nullable Long gkPwHandle) { + mGkPwHandle = gkPwHandle != null ? gkPwHandle : 0L; + } + + /** + * @param requireFace if face consent should be shown + * @param requireFingerprint if fingerprint consent should be shown + */ + public void setConsentRequirement(boolean requireFace, boolean requireFingerprint) { mRequireFace = requireFace; mRequireFingerprint = requireFingerprint; - mGkPwHandle = gkPwHandle != null ? gkPwHandle : 0L; } /** diff --git a/src/com/android/settings/biometrics/combination/CombinedBiometricStatusPreferenceController.java b/src/com/android/settings/biometrics/combination/CombinedBiometricStatusPreferenceController.java index 6f24215e17a..32fb3a03c4a 100644 --- a/src/com/android/settings/biometrics/combination/CombinedBiometricStatusPreferenceController.java +++ b/src/com/android/settings/biometrics/combination/CombinedBiometricStatusPreferenceController.java @@ -104,12 +104,34 @@ public class CombinedBiometricStatusPreferenceController extends private void updateStateInternal() { // This controller currently is shown if fingerprint&face exist on the device. If this // changes in the future, the modalities passed into the below will need to be updated. - updateStateInternal(ParentalControlsUtils.parentConsentRequired(mContext, - BiometricAuthenticator.TYPE_FACE | BiometricAuthenticator.TYPE_FINGERPRINT)); + + final RestrictedLockUtils.EnforcedAdmin faceAdmin = ParentalControlsUtils + .parentConsentRequired(mContext, BiometricAuthenticator.TYPE_FACE); + final RestrictedLockUtils.EnforcedAdmin fpAdmin = ParentalControlsUtils + .parentConsentRequired(mContext, BiometricAuthenticator.TYPE_FINGERPRINT); + + // If the admins are non-null, they are actually always the same. Just the helper class + // we create above always return the admin, instead of a boolean. + final boolean faceConsentRequired = faceAdmin != null; + final boolean fpConsentRequired = fpAdmin != null; + final RestrictedLockUtils.EnforcedAdmin admin = faceAdmin != null ? faceAdmin : fpAdmin; + + updateStateInternal(admin, faceConsentRequired, fpConsentRequired); } @VisibleForTesting - void updateStateInternal(@Nullable RestrictedLockUtils.EnforcedAdmin enforcedAdmin) { + void updateStateInternal(@Nullable RestrictedLockUtils.EnforcedAdmin enforcedAdmin, + boolean faceConsentRequired, boolean fpConsentRequired) { + // Disable the preference (and show the consent flow) only if consent is required for all + // modalities. Otherwise, users will not be able to enter and modify settings for modalities + // which have already been consented. In any case, the controllers for the modalities which + // have not yet been consented will be disabled in the combined page anyway - users can + // go through the consent+enrollment flow from there. + final boolean disablePreference = faceConsentRequired && fpConsentRequired; + if (!disablePreference) { + enforcedAdmin = null; + } + if (mPreference != null) { mPreference.setDisabledByAdmin(enforcedAdmin); } diff --git a/tests/robotests/src/com/android/settings/biometrics/combination/CombinedBiometricStatusPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/biometrics/combination/CombinedBiometricStatusPreferenceControllerTest.java index 3a973879bda..02bca3e1e83 100644 --- a/tests/robotests/src/com/android/settings/biometrics/combination/CombinedBiometricStatusPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/biometrics/combination/CombinedBiometricStatusPreferenceControllerTest.java @@ -104,12 +104,16 @@ public class CombinedBiometricStatusPreferenceControllerTest { RestrictedLockUtils.EnforcedAdmin admin = mock(RestrictedLockUtils.EnforcedAdmin.class); mController.mPreference = restrictedPreference; - mController.updateStateInternal(admin); + mController.updateStateInternal(admin, true, true); verify(restrictedPreference).setDisabledByAdmin(eq(admin)); - reset(admin); + mController.updateStateInternal(admin, true, false); + verify(restrictedPreference).setDisabledByAdmin(eq(null)); - mController.updateStateInternal(null /* enforcedAdmin */); - verify(restrictedPreference, never()).setDisabledByAdmin(any()); + mController.updateStateInternal(admin, false, true); + verify(restrictedPreference).setDisabledByAdmin(eq(null)); + + mController.updateStateInternal(admin, false, false); + verify(restrictedPreference).setDisabledByAdmin(eq(null)); } } diff --git a/tests/unit/src/com/android/settings/biometrics/ParentalConsentHelperTest.java b/tests/unit/src/com/android/settings/biometrics/ParentalConsentHelperTest.java index 78856da7692..abb5355b590 100644 --- a/tests/unit/src/com/android/settings/biometrics/ParentalConsentHelperTest.java +++ b/tests/unit/src/com/android/settings/biometrics/ParentalConsentHelperTest.java @@ -167,8 +167,8 @@ public class ParentalConsentHelperTest { } // initial consent status - final ParentalConsentHelper helper = - new ParentalConsentHelper(requireFace, requireFingerprint, gkpw); + final ParentalConsentHelper helper = new ParentalConsentHelper(gkpw); + helper.setConsentRequirement(requireFace, requireFingerprint); assertThat(ParentalConsentHelper.hasFaceConsent(helper.getConsentResult())) .isFalse(); assertThat(ParentalConsentHelper.hasFingerprintConsent(helper.getConsentResult()))