From 593e62640de791af39ce57338b75d775186b6e94 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Fri, 28 Jun 2019 13:24:44 -0700 Subject: [PATCH] Always send userId to FaceService resetLockout may take longer on some devices, causing FaceSettings setActiveUser to be overwritten. To be safe, invoke updateActiveGroup whenever user-specific functionality is invoked from the upper layers. Ideally setActiveUser + operation should be atomic. In the future perhaps we could consider changing HIDL so each method is user-aware, but for now this is the only thing we can do. Fixes: 136264301 Test: Set up work profile on device, repeatedly do the following 1) Go to settings, enroll face 2) Back out, go back in, delete face 3) Enroll face 4) Delete face, go back out Change-Id: Ic32587cd4613f2bfd71171df6b69fe6028812ca8 --- core/java/android/hardware/face/FaceManager.java | 16 +++++++++------- .../java/android/hardware/face/IFaceService.aidl | 11 ++++++----- .../server/biometrics/face/FaceService.java | 15 ++++++++++----- 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/core/java/android/hardware/face/FaceManager.java b/core/java/android/hardware/face/FaceManager.java index c8f0e094b631a..12b285a0f0ab3 100644 --- a/core/java/android/hardware/face/FaceManager.java +++ b/core/java/android/hardware/face/FaceManager.java @@ -262,7 +262,7 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan * @hide */ @RequiresPermission(MANAGE_BIOMETRIC) - public void enroll(byte[] token, CancellationSignal cancel, + public void enroll(int userId, byte[] token, CancellationSignal cancel, EnrollmentCallback callback, int[] disabledFeatures) { if (callback == null) { throw new IllegalArgumentException("Must supply an enrollment callback"); @@ -281,7 +281,7 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan try { mEnrollmentCallback = callback; Trace.beginSection("FaceManager#enroll"); - mService.enroll(mToken, token, mServiceReceiver, + mService.enroll(userId, mToken, token, mServiceReceiver, mContext.getOpPackageName(), disabledFeatures); } catch (RemoteException e) { Log.w(TAG, "Remote exception in enroll: ", e); @@ -339,12 +339,13 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan * @hide */ @RequiresPermission(MANAGE_BIOMETRIC) - public void setFeature(int feature, boolean enabled, byte[] token, + public void setFeature(int userId, int feature, boolean enabled, byte[] token, SetFeatureCallback callback) { if (mService != null) { try { mSetFeatureCallback = callback; - mService.setFeature(feature, enabled, token, mServiceReceiver); + mService.setFeature(userId, feature, enabled, token, mServiceReceiver, + mContext.getOpPackageName()); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } @@ -355,11 +356,11 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan * @hide */ @RequiresPermission(MANAGE_BIOMETRIC) - public void getFeature(int feature, GetFeatureCallback callback) { + public void getFeature(int userId, int feature, GetFeatureCallback callback) { if (mService != null) { try { mGetFeatureCallback = callback; - mService.getFeature(feature, mServiceReceiver); + mService.getFeature(userId, feature, mServiceReceiver, mContext.getOpPackageName()); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } @@ -414,7 +415,8 @@ public class FaceManager implements BiometricAuthenticator, BiometricFaceConstan try { mRemovalCallback = callback; mRemovalFace = face; - mService.remove(mToken, face.getBiometricId(), userId, mServiceReceiver); + mService.remove(mToken, face.getBiometricId(), userId, mServiceReceiver, + mContext.getOpPackageName()); } catch (RemoteException e) { Log.w(TAG, "Remote exception in remove: ", e); if (callback != null) { diff --git a/core/java/android/hardware/face/IFaceService.aidl b/core/java/android/hardware/face/IFaceService.aidl index 601be7595581f..b6a0afbf716cf 100644 --- a/core/java/android/hardware/face/IFaceService.aidl +++ b/core/java/android/hardware/face/IFaceService.aidl @@ -50,14 +50,15 @@ interface IFaceService { int callingUid, int callingPid, int callingUserId, boolean fromClient); // Start face enrollment - void enroll(IBinder token, in byte [] cryptoToken, IFaceServiceReceiver receiver, + void enroll(int userId, IBinder token, in byte [] cryptoToken, IFaceServiceReceiver receiver, String opPackageName, in int [] disabledFeatures); // Cancel enrollment in progress void cancelEnrollment(IBinder token); // Any errors resulting from this call will be returned to the listener - void remove(IBinder token, int faceId, int userId, IFaceServiceReceiver receiver); + void remove(IBinder token, int faceId, int userId, IFaceServiceReceiver receiver, + String opPackageName); // Rename the face specified by faceId to the given name void rename(int faceId, String name); @@ -98,10 +99,10 @@ interface IFaceService { // Enumerate all faces void enumerate(IBinder token, int userId, IFaceServiceReceiver receiver); - void setFeature(int feature, boolean enabled, in byte [] token, - IFaceServiceReceiver receiver); + void setFeature(int userId, int feature, boolean enabled, in byte [] token, + IFaceServiceReceiver receiver, String opPackageName); - void getFeature(int feature, IFaceServiceReceiver receiver); + void getFeature(int userId, int feature, IFaceServiceReceiver receiver, String opPackageName); void userActivity(); } 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 def7f75ac3b64..72bac7494804c 100644 --- a/services/core/java/com/android/server/biometrics/face/FaceService.java +++ b/services/core/java/com/android/server/biometrics/face/FaceService.java @@ -346,10 +346,11 @@ public class FaceService extends BiometricServiceBase { } @Override // Binder call - public void enroll(final IBinder token, final byte[] cryptoToken, + public void enroll(int userId, final IBinder token, final byte[] cryptoToken, final IFaceServiceReceiver receiver, final String opPackageName, final int[] disabledFeatures) { checkPermission(MANAGE_BIOMETRIC); + updateActiveGroup(userId, opPackageName); mNotificationManager.cancelAsUser(NOTIFICATION_TAG, NOTIFICATION_ID, UserHandle.CURRENT); @@ -448,8 +449,9 @@ public class FaceService extends BiometricServiceBase { @Override // Binder call public void remove(final IBinder token, final int faceId, final int userId, - final IFaceServiceReceiver receiver) { + final IFaceServiceReceiver receiver, final String opPackageName) { checkPermission(MANAGE_BIOMETRIC); + updateActiveGroup(userId, opPackageName); if (token == null) { Slog.w(TAG, "remove(): token is null"); @@ -612,9 +614,10 @@ public class FaceService extends BiometricServiceBase { } @Override - public void setFeature(int feature, boolean enabled, final byte[] token, - IFaceServiceReceiver receiver) { + public void setFeature(int userId, int feature, boolean enabled, final byte[] token, + IFaceServiceReceiver receiver, final String opPackageName) { checkPermission(MANAGE_BIOMETRIC); + updateActiveGroup(userId, opPackageName); mHandler.post(() -> { if (!FaceService.this.hasEnrolledBiometrics(mCurrentUserId)) { @@ -644,8 +647,10 @@ public class FaceService extends BiometricServiceBase { } @Override - public void getFeature(int feature, IFaceServiceReceiver receiver) { + public void getFeature(int userId, int feature, IFaceServiceReceiver receiver, + final String opPackageName) { checkPermission(MANAGE_BIOMETRIC); + updateActiveGroup(userId, opPackageName); mHandler.post(() -> { // This should ideally return tri-state, but the user isn't shown settings unless