From 2170ab7d207a2ee2dd684b2f1626a04f7818cb95 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Wed, 3 Apr 2019 20:28:28 -0700 Subject: [PATCH 1/4] Settings should be per-profile Fixes: 129905061 Test: Builds Change-Id: If1b3e92da61c051c5f38a425da17f98cdfc3336d --- .../server/biometrics/BiometricService.java | 76 ++++++++++--------- 1 file changed, 40 insertions(+), 36 deletions(-) diff --git a/services/core/java/com/android/server/biometrics/BiometricService.java b/services/core/java/com/android/server/biometrics/BiometricService.java index 516844d85484b..e2f06309f8603 100644 --- a/services/core/java/com/android/server/biometrics/BiometricService.java +++ b/services/core/java/com/android/server/biometrics/BiometricService.java @@ -351,6 +351,11 @@ public class BiometricService extends SystemService { } private final class SettingObserver extends ContentObserver { + + private static final boolean DEFAULT_KEYGUARD_ENABLED = true; + private static final boolean DEFAULT_APP_ENABLED = true; + private static final boolean DEFAULT_ALWAYS_REQUIRE_CONFIRMATION = false; + private final Uri FACE_UNLOCK_KEYGUARD_ENABLED = Settings.Secure.getUriFor(Settings.Secure.FACE_UNLOCK_KEYGUARD_ENABLED); private final Uri FACE_UNLOCK_APP_ENABLED = @@ -359,9 +364,10 @@ public class BiometricService extends SystemService { Settings.Secure.getUriFor(Settings.Secure.FACE_UNLOCK_ALWAYS_REQUIRE_CONFIRMATION); private final ContentResolver mContentResolver; - private boolean mFaceEnabledOnKeyguard; - private boolean mFaceEnabledForApps; - private boolean mFaceAlwaysRequireConfirmation; + + private Map mFaceEnabledOnKeyguard = new HashMap<>(); + private Map mFaceEnabledForApps = new HashMap<>(); + private Map mFaceAlwaysRequireConfirmation = new HashMap<>(); /** * Creates a content observer. @@ -379,63 +385,61 @@ public class BiometricService extends SystemService { mContentResolver.registerContentObserver(FACE_UNLOCK_KEYGUARD_ENABLED, false /* notifyForDescendents */, this /* observer */, - UserHandle.USER_CURRENT); + UserHandle.USER_ALL); mContentResolver.registerContentObserver(FACE_UNLOCK_APP_ENABLED, false /* notifyForDescendents */, this /* observer */, - UserHandle.USER_CURRENT); + UserHandle.USER_ALL); mContentResolver.registerContentObserver(FACE_UNLOCK_ALWAYS_REQUIRE_CONFIRMATION, false /* notifyForDescendents */, this /* observer */, - UserHandle.USER_CURRENT); - - // Update the value immediately - onChange(true /* selfChange */, FACE_UNLOCK_KEYGUARD_ENABLED); - onChange(true /* selfChange */, FACE_UNLOCK_APP_ENABLED); - onChange(true /* selfChange */, FACE_UNLOCK_ALWAYS_REQUIRE_CONFIRMATION); + UserHandle.USER_ALL); } @Override - public void onChange(boolean selfChange, Uri uri) { + public void onChange(boolean selfChange, Uri uri, int userId) { if (FACE_UNLOCK_KEYGUARD_ENABLED.equals(uri)) { - mFaceEnabledOnKeyguard = - Settings.Secure.getIntForUser( + mFaceEnabledOnKeyguard.put(userId, Settings.Secure.getIntForUser( mContentResolver, Settings.Secure.FACE_UNLOCK_KEYGUARD_ENABLED, - 1 /* default */, - UserHandle.USER_CURRENT) != 0; + DEFAULT_KEYGUARD_ENABLED ? 1 : 0 /* default */, + userId) != 0); - List callbacks = mEnabledOnKeyguardCallbacks; - for (int i = 0; i < callbacks.size(); i++) { - callbacks.get(i).notify(BiometricSourceType.FACE, mFaceEnabledOnKeyguard); + if (userId == ActivityManager.getCurrentUser()) { + List callbacks = mEnabledOnKeyguardCallbacks; + for (int i = 0; i < callbacks.size(); i++) { + callbacks.get(i).notify(BiometricSourceType.FACE, + mFaceEnabledOnKeyguard.getOrDefault(userId, + DEFAULT_KEYGUARD_ENABLED)); + } } } else if (FACE_UNLOCK_APP_ENABLED.equals(uri)) { - mFaceEnabledForApps = - Settings.Secure.getIntForUser( + mFaceEnabledForApps.put(userId, Settings.Secure.getIntForUser( mContentResolver, Settings.Secure.FACE_UNLOCK_APP_ENABLED, - 1 /* default */, - UserHandle.USER_CURRENT) != 0; + DEFAULT_APP_ENABLED ? 1 : 0 /* default */, + userId) != 0); } else if (FACE_UNLOCK_ALWAYS_REQUIRE_CONFIRMATION.equals(uri)) { - mFaceAlwaysRequireConfirmation = - Settings.Secure.getIntForUser( + mFaceAlwaysRequireConfirmation.put(userId, Settings.Secure.getIntForUser( mContentResolver, Settings.Secure.FACE_UNLOCK_ALWAYS_REQUIRE_CONFIRMATION, - 0 /* default */, - UserHandle.USER_CURRENT) != 0; + DEFAULT_ALWAYS_REQUIRE_CONFIRMATION ? 1 : 0 /* default */, + userId) != 0); } } boolean getFaceEnabledOnKeyguard() { - return mFaceEnabledOnKeyguard; + return mFaceEnabledOnKeyguard.getOrDefault( + ActivityManager.getCurrentUser(), DEFAULT_KEYGUARD_ENABLED); } - boolean getFaceEnabledForApps() { - return mFaceEnabledForApps; + boolean getFaceEnabledForApps(int userId) { + return mFaceEnabledForApps.getOrDefault(userId, DEFAULT_APP_ENABLED); } - boolean getFaceAlwaysRequireConfirmation() { - return mFaceAlwaysRequireConfirmation; + boolean getFaceAlwaysRequireConfirmation(int userId) { + return mFaceAlwaysRequireConfirmation + .getOrDefault(userId, DEFAULT_ALWAYS_REQUIRE_CONFIRMATION); } } @@ -884,7 +888,7 @@ public class BiometricService extends SystemService { } if (authenticator.hasEnrolledTemplates(userId)) { hasTemplatesEnrolled = true; - if (isEnabledForApp(modality)) { + if (isEnabledForApp(modality, userId)) { // TODO(b/110907543): When face settings (and other settings) have both a // user toggle as well as a work profile settings page, this needs to be // updated to reflect the correct setting. @@ -909,14 +913,14 @@ public class BiometricService extends SystemService { return new Pair<>(modality, BiometricConstants.BIOMETRIC_SUCCESS); } - private boolean isEnabledForApp(int modality) { + private boolean isEnabledForApp(int modality, int userId) { switch(modality) { case TYPE_FINGERPRINT: return true; case TYPE_IRIS: return true; case TYPE_FACE: - return mSettingObserver.getFaceEnabledForApps(); + return mSettingObserver.getFaceEnabledForApps(userId); default: Slog.w(TAG, "Unsupported modality: " + modality); return false; @@ -1363,7 +1367,7 @@ public class BiometricService extends SystemService { if ((modality & TYPE_FACE) != 0) { // Check if the user has forced confirmation to be required in Settings. requireConfirmation = requireConfirmation - || mSettingObserver.getFaceAlwaysRequireConfirmation(); + || mSettingObserver.getFaceAlwaysRequireConfirmation(userId); } // Generate random cookies to pass to the services that should prepare to start // authenticating. Store the cookie here and wait for all services to "ack" From b13733024f463e69167c9dd249bb12fe36c3d312 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Wed, 3 Apr 2019 19:02:27 -0700 Subject: [PATCH 2/4] Launch ConfirmDeviceCredential with actual userId Fixes: 129905318 Bug: 110907543 Test: Set up work profile, use BiometricPromptDemo to allow device credentials. Work profile demo app now requests work profile password. Change-Id: Iec53966c6ad87021fdaa1808a1814a4abfbf89bb --- .../java/com/android/server/biometrics/BiometricService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/biometrics/BiometricService.java b/services/core/java/com/android/server/biometrics/BiometricService.java index e2f06309f8603..2fee68ee2dcd5 100644 --- a/services/core/java/com/android/server/biometrics/BiometricService.java +++ b/services/core/java/com/android/server/biometrics/BiometricService.java @@ -629,7 +629,7 @@ public class BiometricService extends SystemService { mConfirmDeviceCredentialReceiver = receiver; // Use this so we don't need to duplicate logic.. final Intent intent = kgm.createConfirmDeviceCredentialIntent(null /* title */, - null /* description */); + null /* description */, userId); // Then give it the bundle to do magic behavior.. intent.putExtra(KeyguardManager.EXTRA_BIOMETRIC_PROMPT_BUNDLE, bundle); // Create a new task with this activity located at the root. From 7776536966335c0723d67d623bb998f8b328e8ae Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Wed, 3 Apr 2019 18:12:48 -0700 Subject: [PATCH 3/4] Do not updateActiveGroup on a handler This method must not return to the caller until complete, otherwise weird race conditions may occur. Bug: 110907543 Test: Builds Change-Id: Ia82671b34fac3662928012d8c709143efdf92266 --- .../com/android/server/biometrics/BiometricServiceBase.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/biometrics/BiometricServiceBase.java b/services/core/java/com/android/server/biometrics/BiometricServiceBase.java index d8e7b7db7b750..b631a93d7739c 100644 --- a/services/core/java/com/android/server/biometrics/BiometricServiceBase.java +++ b/services/core/java/com/android/server/biometrics/BiometricServiceBase.java @@ -903,9 +903,8 @@ public abstract class BiometricServiceBase extends SystemService } protected void setActiveUserInternal(int userId) { - mHandler.post(() -> { - updateActiveGroup(userId, null /* clientPackage */); - }); + // Do not put on handler, since it should finish before returning to caller. + updateActiveGroup(userId, null /* clientPackage */); } protected void removeInternal(RemovalClient client) { From 1a878c1fba73802989e5ed8bf5ff3b9f755fb2ee Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Thu, 4 Apr 2019 15:50:11 -0700 Subject: [PATCH 4/4] Enroll for the correct user Bug: 110907543 Test: Builds Change-Id: I637809cd18d84402918c95d1fc010dacc14f8785 --- .../java/com/android/server/biometrics/face/FaceService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/biometrics/face/FaceService.java b/services/core/java/com/android/server/biometrics/face/FaceService.java index c573bbb2e1f0e..d67853ab0d1b1 100644 --- a/services/core/java/com/android/server/biometrics/face/FaceService.java +++ b/services/core/java/com/android/server/biometrics/face/FaceService.java @@ -165,7 +165,7 @@ public class FaceService extends BiometricServiceBase { } }; - enrollInternal(client, UserHandle.getCallingUserId()); + enrollInternal(client, mCurrentUserId); } @Override // Binder call @@ -813,7 +813,7 @@ public class FaceService extends BiometricServiceBase { com.android.internal.R.integer.config_faceMaxTemplatesPerUser); final int enrolled = FaceService.this.getEnrolledTemplates(userId).size(); if (enrolled >= limit) { - Slog.w(TAG, "Too many faces registered"); + Slog.w(TAG, "Too many faces registered, user: " + userId); return true; } return false;