Fix bug where fingerprint for wrong userId was attempted to be removed.

- cleaned up private API to ensure userId is distinct from groupId.
- fixed bug where we were sending the wrong userId when attempting to
- fix warning about wrong fingerId when receiving final id of 0.

Fixes bug 28268635

Change-Id: I9507723c1a763152775f2feff76c16762f23cf2d

diff --git a/core/java/android/hardware/fingerprint/FingerprintManager.java b/core/java/android/hardware/fingerprint/FingerprintManager.java
index 271ec79..1ff2e8a 100644 (file)
--- a/core/java/android/hardware/fingerprint/FingerprintManager.java
+++ b/core/java/android/hardware/fingerprint/FingerprintManager.java
@@ -590,7 +590,7 @@ public class FingerprintManager {
         if (mService != null) try {
             mRemovalCallback = callback;
             mRemovalFingerprint = fp;
-            mService.remove(mToken, fp.getFingerId(), userId, mServiceReceiver);
+            mService.remove(mToken, fp.getFingerId(), fp.getGroupId(), userId, mServiceReceiver);
         } catch (RemoteException e) {
             Log.w(TAG, "Remote exception in remove: ", e);
             if (callback != null) {
@@ -810,11 +810,13 @@ public class FingerprintManager {
             if (mRemovalCallback != null) {
                 int reqFingerId = mRemovalFingerprint.getFingerId();
                 int reqGroupId = mRemovalFingerprint.getGroupId();
-                if (reqFingerId != 0  &&  fingerId != reqFingerId) {
+                if (reqFingerId != 0 && fingerId != 0  &&  fingerId != reqFingerId) {
                     Log.w(TAG, "Finger id didn't match: " + fingerId + " != " + reqFingerId);
+                    return;
                 }
                 if (groupId != reqGroupId) {
                     Log.w(TAG, "Group id didn't match: " + groupId + " != " + reqGroupId);
+                    return;
                 }
                 mRemovalCallback.onRemovalSucceeded(new Fingerprint(null, groupId, fingerId,
                         deviceId));

diff --git a/core/java/android/hardware/fingerprint/IFingerprintService.aidl b/core/java/android/hardware/fingerprint/IFingerprintService.aidl
index d7915e3..a83397a 100644 (file)
--- a/core/java/android/hardware/fingerprint/IFingerprintService.aidl
+++ b/core/java/android/hardware/fingerprint/IFingerprintService.aidl
@@ -41,7 +41,8 @@ interface IFingerprintService {
     void cancelEnrollment(IBinder token);
 
     // Any errors resulting from this call will be returned to the listener
-    void remove(IBinder token, int fingerId, int groupId, IFingerprintServiceReceiver receiver);
+    void remove(IBinder token, int fingerId, int groupId, int userId,
+            IFingerprintServiceReceiver receiver);
 
     // Rename the fingerprint specified by fingerId and groupId to the given name
     void rename(int fingerId, int groupId, String name);

diff --git a/services/core/java/com/android/server/fingerprint/ClientMonitor.java b/services/core/java/com/android/server/fingerprint/ClientMonitor.java
index 45b6d3e..8163b79 100644 (file)
--- a/services/core/java/com/android/server/fingerprint/ClientMonitor.java
+++ b/services/core/java/com/android/server/fingerprint/ClientMonitor.java
@@ -38,7 +38,7 @@ public abstract class ClientMonitor implements IBinder.DeathRecipient {
     protected static final boolean DEBUG = FingerprintService.DEBUG;
     private IBinder mToken;
     private IFingerprintServiceReceiver mReceiver;
-    private int mCallingUserId;
+    private int mTargetUserId;
     private int mGroupId;
     private boolean mIsRestricted; // True if client does not have MANAGE_FINGERPRINT permission
     private String mOwner;
@@ -50,20 +50,20 @@ public abstract class ClientMonitor implements IBinder.DeathRecipient {
      * @param halDeviceId the HAL device ID of the associated fingerprint hardware
      * @param token a unique token for the client
      * @param receiver recipient of related events (e.g. authentication)
-     * @param callingUserId user id of calling user
+     * @param userId target user id for operation
      * @param groupId groupId for the fingerprint set
      * @param restricted whether or not client has the {@link Manifest#MANAGE_FINGERPRINT}
      * permission
      * @param owner name of the client that owns this
      */
     public ClientMonitor(Context context, long halDeviceId, IBinder token,
-            IFingerprintServiceReceiver receiver, int callingUserId, int groupId,boolean restricted,
+            IFingerprintServiceReceiver receiver, int userId, int groupId,boolean restricted,
             String owner) {
         mContext = context;
         mHalDeviceId = halDeviceId;
         mToken = token;
         mReceiver = receiver;
-        mCallingUserId = callingUserId;
+        mTargetUserId = userId;
         mGroupId = groupId;
         mIsRestricted = restricted;
         mOwner = owner;
@@ -197,8 +197,8 @@ public abstract class ClientMonitor implements IBinder.DeathRecipient {
         return mIsRestricted;
     }
 
-    public final int getCallingUserId() {
-        return mCallingUserId;
+    public final int getTargetUserId() {
+        return mTargetUserId;
     }
 
     public final int getGroupId() {

diff --git a/services/core/java/com/android/server/fingerprint/EnumerateClient.java b/services/core/java/com/android/server/fingerprint/EnumerateClient.java
index e826fee..52dbd5d 100644 (file)
--- a/services/core/java/com/android/server/fingerprint/EnumerateClient.java
+++ b/services/core/java/com/android/server/fingerprint/EnumerateClient.java
@@ -41,7 +41,7 @@ public abstract class EnumerateClient extends ClientMonitor {
         try {
             final int result = daemon.enumerate();
             if (result != 0) {
-                Slog.w(TAG, "start enumerate for user " + getCallingUserId()
+                Slog.w(TAG, "start enumerate for user " + getTargetUserId()
                     + " failed, result=" + result);
                 onError(FingerprintManager.FINGERPRINT_ERROR_HW_UNAVAILABLE);
                 return result;

diff --git a/services/core/java/com/android/server/fingerprint/FingerprintService.java b/services/core/java/com/android/server/fingerprint/FingerprintService.java
index fcf7bf5..9a2db8e 100644 (file)
--- a/services/core/java/com/android/server/fingerprint/FingerprintService.java
+++ b/services/core/java/com/android/server/fingerprint/FingerprintService.java
@@ -356,7 +356,7 @@ public class FingerprintService extends SystemService implements IBinder.DeathRe
         }
     }
 
-    void startRemove(IBinder token, int fingerId, int callingUserId, int groupId,
+    void startRemove(IBinder token, int fingerId, int groupId, int userId,
             IFingerprintServiceReceiver receiver, boolean restricted) {
         IFingerprintDaemon daemon = getFingerprintDaemon();
         if (daemon == null) {
@@ -364,7 +364,7 @@ public class FingerprintService extends SystemService implements IBinder.DeathRe
             return;
         }
         RemovalClient client = new RemovalClient(getContext(), mHalDeviceId, token,
-                receiver, callingUserId, groupId, fingerId, restricted, token.toString()) {
+                receiver, fingerId, groupId, userId, restricted, token.toString()) {
             @Override
             public void notifyUserActivity() {
                 FingerprintService.this.userActivity();
@@ -794,14 +794,13 @@ public class FingerprintService extends SystemService implements IBinder.DeathRe
 
         @Override // Binder call
         public void remove(final IBinder token, final int fingerId, final int groupId,
-                final IFingerprintServiceReceiver receiver) {
+                final int userId, final IFingerprintServiceReceiver receiver) {
             checkPermission(MANAGE_FINGERPRINT); // TODO: Maybe have another permission
             final boolean restricted = isRestricted();
-            final int callingUserId = UserHandle.getCallingUserId();
             mHandler.post(new Runnable() {
                 @Override
                 public void run() {
-                    startRemove(token, fingerId, callingUserId, groupId, receiver, restricted);
+                    startRemove(token, fingerId, groupId, userId, receiver, restricted);
                 }
             });
 

diff --git a/services/core/java/com/android/server/fingerprint/RemovalClient.java b/services/core/java/com/android/server/fingerprint/RemovalClient.java
index ffa3c3f..bcf2264 100644 (file)
--- a/services/core/java/com/android/server/fingerprint/RemovalClient.java
+++ b/services/core/java/com/android/server/fingerprint/RemovalClient.java
@@ -30,14 +30,12 @@ import android.util.Slog;
  */
 public abstract class RemovalClient extends ClientMonitor {
     private int mFingerId;
-    private int mUserIdForRemove;
 
     public RemovalClient(Context context, long halDeviceId, IBinder token,
-            IFingerprintServiceReceiver receiver, int userId, int groupId, int fingerId,
+            IFingerprintServiceReceiver receiver, int fingerId, int groupId, int userId,
             boolean restricted, String owner) {
         super(context, halDeviceId, token, receiver, userId, groupId, restricted, owner);
         mFingerId = fingerId;
-        mUserIdForRemove = userId;
     }
 
     @Override
@@ -72,25 +70,21 @@ public abstract class RemovalClient extends ClientMonitor {
      */
     private boolean sendRemoved(int fingerId, int groupId) {
         IFingerprintServiceReceiver receiver = getReceiver();
-        if (receiver == null)
-            return true; // client not listening
         try {
-            receiver.onRemoved(getHalDeviceId(), fingerId, groupId);
-            return fingerId == 0;
+            if (receiver != null) {
+                receiver.onRemoved(getHalDeviceId(), fingerId, groupId);
+            }
         } catch (RemoteException e) {
             Slog.w(TAG, "Failed to notify Removed:", e);
         }
-        return false;
+        return fingerId == 0;
     }
 
     @Override
     public boolean onRemoved(int fingerId, int groupId) {
         if (fingerId != 0) {
-            if (fingerId != mFingerId)
             FingerprintUtils.getInstance().removeFingerprintIdForUser(getContext(), fingerId,
-                    mUserIdForRemove);
-        } else {
-            mUserIdForRemove = UserHandle.USER_NULL;
+                    getTargetUserId());
         }
         return sendRemoved(fingerId, getGroupId());
     }