From 0d108d8f4c71c80b825206ac50f2033f58597b5b Mon Sep 17 00:00:00 2001
From: Soonil Nagarkar <sooniln@google.com>
Date: Thu, 19 Nov 2020 10:57:16 -0800
Subject: [PATCH] Normalize user active logic across Location APIs

Adds additional logic for location requests coming from the system
server, such that those requests may allowed so long as location is on
for the current user, even if the system user has location disabled.
Even though the system server runs under the system user, it may need
to service requests from or on behalf of other users.

Test: presubmits + manual
Change-Id: Ic9d7762e75fc930b7d26f1d1bf20b272d562939c
---
 .../util/identity/CallerIdentity.java         |  4 +-
 .../location/geofence/GeofenceManager.java    | 28 +++++++---
 .../gnss/GnssListenerMultiplexer.java         | 29 ++++++++--
 .../injector/SystemUserInfoHelper.java        | 24 +++++----
 .../location/injector/UserInfoHelper.java     |  6 +++
 .../provider/LocationProviderManager.java     | 53 +++++++++++--------
 .../location/injector/FakeUserInfoHelper.java |  5 ++
 7 files changed, 106 insertions(+), 43 deletions(-)

diff --git a/location/java/android/location/util/identity/CallerIdentity.java b/location/java/android/location/util/identity/CallerIdentity.java
index e023aa1dcd224..9dbdedeb8a030 100644
--- a/location/java/android/location/util/identity/CallerIdentity.java
+++ b/location/java/android/location/util/identity/CallerIdentity.java
@@ -150,8 +150,8 @@ public final class CallerIdentity {
         return mListenerId;
     }
 
-    /** Returns true if this represents a system identity. */
-    public boolean isSystem() {
+    /** Returns true if this represents a system server identity. */
+    public boolean isSystemServer() {
         return mUid == Process.SYSTEM_UID;
     }
 
diff --git a/services/core/java/com/android/server/location/geofence/GeofenceManager.java b/services/core/java/com/android/server/location/geofence/GeofenceManager.java
index 4a8534292eaf0..5a90fa7a271cd 100644
--- a/services/core/java/com/android/server/location/geofence/GeofenceManager.java
+++ b/services/core/java/com/android/server/location/geofence/GeofenceManager.java
@@ -322,12 +322,28 @@ public class GeofenceManager extends
 
     @Override
     protected boolean isActive(GeofenceRegistration registration) {
-        CallerIdentity identity = registration.getIdentity();
-        return registration.isPermitted()
-                && (identity.isSystem() || mUserInfoHelper.isCurrentUserId(identity.getUserId()))
-                && mSettingsHelper.isLocationEnabled(identity.getUserId())
-                && !mSettingsHelper.isLocationPackageBlacklisted(identity.getUserId(),
-                identity.getPackageName());
+        return registration.isPermitted() && isActive(registration.getIdentity());
+    }
+
+    private boolean isActive(CallerIdentity identity) {
+        if (identity.isSystemServer()) {
+            if (!mSettingsHelper.isLocationEnabled(mUserInfoHelper.getCurrentUserId())) {
+                return false;
+            }
+        } else {
+            if (!mSettingsHelper.isLocationEnabled(identity.getUserId())) {
+                return false;
+            }
+            if (!mUserInfoHelper.isCurrentUserId(identity.getUserId())) {
+                return false;
+            }
+            if (mSettingsHelper.isLocationPackageBlacklisted(identity.getUserId(),
+                    identity.getPackageName())) {
+                return false;
+            }
+        }
+
+        return true;
     }
 
     @Override
diff --git a/services/core/java/com/android/server/location/gnss/GnssListenerMultiplexer.java b/services/core/java/com/android/server/location/gnss/GnssListenerMultiplexer.java
index e68f595c3c43e..7e848e03c3a0f 100644
--- a/services/core/java/com/android/server/location/gnss/GnssListenerMultiplexer.java
+++ b/services/core/java/com/android/server/location/gnss/GnssListenerMultiplexer.java
@@ -266,11 +266,30 @@ public abstract class GnssListenerMultiplexer<TRequest, TListener extends IInter
         CallerIdentity identity = registration.getIdentity();
         return registration.isPermitted()
                 && (registration.isForeground() || isBackgroundRestrictionExempt(identity))
-                && (identity.isSystem() || mUserInfoHelper.isCurrentUserId(identity.getUserId()))
-                && mLocationManagerInternal.isProviderEnabledForUser(GPS_PROVIDER,
-                identity.getUserId())
-                && !mSettingsHelper.isLocationPackageBlacklisted(identity.getUserId(),
-                identity.getPackageName());
+                && isActive(identity);
+    }
+
+    private boolean isActive(CallerIdentity identity) {
+        if (identity.isSystemServer()) {
+            if (!mLocationManagerInternal.isProviderEnabledForUser(GPS_PROVIDER,
+                    mUserInfoHelper.getCurrentUserId())) {
+                return false;
+            }
+        } else {
+            if (!mLocationManagerInternal.isProviderEnabledForUser(GPS_PROVIDER,
+                    identity.getUserId())) {
+                return false;
+            }
+            if (!mUserInfoHelper.isCurrentUserId(identity.getUserId())) {
+                return false;
+            }
+            if (mSettingsHelper.isLocationPackageBlacklisted(identity.getUserId(),
+                    identity.getPackageName())) {
+                return false;
+            }
+        }
+
+        return true;
     }
 
     private boolean isBackgroundRestrictionExempt(CallerIdentity identity) {
diff --git a/services/core/java/com/android/server/location/injector/SystemUserInfoHelper.java b/services/core/java/com/android/server/location/injector/SystemUserInfoHelper.java
index 56dd92ab1829a..d4a8fbd0ceb0f 100644
--- a/services/core/java/com/android/server/location/injector/SystemUserInfoHelper.java
+++ b/services/core/java/com/android/server/location/injector/SystemUserInfoHelper.java
@@ -88,11 +88,6 @@ public class SystemUserInfoHelper extends UserInfoHelper {
         return mUserManager;
     }
 
-    /**
-     * Returns an array of running user ids. This will include all running users, and will also
-     * include any profiles of the running users. The caller must never mutate the returned
-     * array.
-     */
     @Override
     public int[] getRunningUserIds() {
         IActivityManager activityManager = getActivityManager();
@@ -110,10 +105,6 @@ public class SystemUserInfoHelper extends UserInfoHelper {
         }
     }
 
-    /**
-     * Returns true if the given user id is either the current user or a profile of the current
-     * user.
-     */
     @Override
     public boolean isCurrentUserId(@UserIdInt int userId) {
         ActivityManagerInternal activityManagerInternal = getActivityManagerInternal();
@@ -129,6 +120,21 @@ public class SystemUserInfoHelper extends UserInfoHelper {
         }
     }
 
+    @Override
+    public @UserIdInt int getCurrentUserId() {
+        ActivityManagerInternal activityManagerInternal = getActivityManagerInternal();
+        if (activityManagerInternal != null) {
+            final long identity = Binder.clearCallingIdentity();
+            try {
+                return activityManagerInternal.getCurrentUserId();
+            } finally {
+                Binder.restoreCallingIdentity(identity);
+            }
+        } else {
+            return UserHandle.USER_NULL;
+        }
+    }
+
     @Override
     protected int[] getProfileIds(@UserIdInt int userId) {
         UserManager userManager = getUserManager();
diff --git a/services/core/java/com/android/server/location/injector/UserInfoHelper.java b/services/core/java/com/android/server/location/injector/UserInfoHelper.java
index 3b7e3b40324e9..0fcc1ecc4c1a9 100644
--- a/services/core/java/com/android/server/location/injector/UserInfoHelper.java
+++ b/services/core/java/com/android/server/location/injector/UserInfoHelper.java
@@ -132,6 +132,12 @@ public abstract class UserInfoHelper {
      */
     public abstract boolean isCurrentUserId(@UserIdInt int userId);
 
+    /**
+     * Returns the current user id. Where possible, prefer to use {@link #isCurrentUserId(int)}
+     * instead, as that method has more flexibility.
+     */
+    public abstract @UserIdInt int getCurrentUserId();
+
     protected abstract int[] getProfileIds(@UserIdInt int userId);
 
     /**
diff --git a/services/core/java/com/android/server/location/provider/LocationProviderManager.java b/services/core/java/com/android/server/location/provider/LocationProviderManager.java
index eca6070304447..c5d7f2c033dd8 100644
--- a/services/core/java/com/android/server/location/provider/LocationProviderManager.java
+++ b/services/core/java/com/android/server/location/provider/LocationProviderManager.java
@@ -1469,18 +1469,9 @@ public class LocationProviderManager extends
 
     public @Nullable Location getLastLocation(CallerIdentity identity,
             @PermissionLevel int permissionLevel, boolean ignoreLocationSettings) {
-        if (mSettingsHelper.isLocationPackageBlacklisted(identity.getUserId(),
-                identity.getPackageName())) {
+        if (!isActive(ignoreLocationSettings, identity)) {
             return null;
         }
-        if (!ignoreLocationSettings) {
-            if (!isEnabled(identity.getUserId())) {
-                return null;
-            }
-            if (!identity.isSystem() && !mUserHelper.isCurrentUserId(identity.getUserId())) {
-                return null;
-            }
-        }
 
         // lastly - note app ops
         if (!mAppOpsHelper.noteOpNoThrow(LocationPermissions.asAppOp(permissionLevel),
@@ -1905,20 +1896,16 @@ public class LocationProviderManager extends
             Preconditions.checkState(Thread.holdsLock(mLock));
         }
 
-        CallerIdentity identity = registration.getIdentity();
-
         if (!registration.isPermitted()) {
             return false;
         }
 
-        if (!registration.getRequest().isLocationSettingsIgnored()) {
-            if (!isEnabled(identity.getUserId())) {
-                return false;
-            }
-            if (!identity.isSystem() && !mUserHelper.isCurrentUserId(identity.getUserId())) {
-                return false;
-            }
+        boolean locationSettingsIgnored = registration.getRequest().isLocationSettingsIgnored();
+        if (!isActive(locationSettingsIgnored, registration.getIdentity())) {
+            return false;
+        }
 
+        if (!locationSettingsIgnored) {
             switch (mLocationPowerSaveModeHelper.getLocationPowerSaveMode()) {
                 case LOCATION_MODE_FOREGROUND_ONLY:
                     if (!registration.isForeground()) {
@@ -1944,8 +1931,32 @@ public class LocationProviderManager extends
             }
         }
 
-        return !mSettingsHelper.isLocationPackageBlacklisted(identity.getUserId(),
-                identity.getPackageName());
+        return true;
+    }
+
+    private boolean isActive(boolean locationSettingsIgnored, CallerIdentity identity) {
+        if (identity.isSystemServer()) {
+            if (!locationSettingsIgnored) {
+                if (!isEnabled(mUserHelper.getCurrentUserId())) {
+                    return false;
+                }
+            }
+        } else {
+            if (!locationSettingsIgnored) {
+                if (!isEnabled(identity.getUserId())) {
+                    return false;
+                }
+                if (!mUserHelper.isCurrentUserId(identity.getUserId())) {
+                    return false;
+                }
+            }
+            if (mSettingsHelper.isLocationPackageBlacklisted(identity.getUserId(),
+                    identity.getPackageName())) {
+                return false;
+            }
+        }
+
+        return true;
     }
 
     @GuardedBy("mLock")
diff --git a/services/tests/mockingservicestests/src/com/android/server/location/injector/FakeUserInfoHelper.java b/services/tests/mockingservicestests/src/com/android/server/location/injector/FakeUserInfoHelper.java
index be6bc99581404..ac23d4e427c49 100644
--- a/services/tests/mockingservicestests/src/com/android/server/location/injector/FakeUserInfoHelper.java
+++ b/services/tests/mockingservicestests/src/com/android/server/location/injector/FakeUserInfoHelper.java
@@ -99,6 +99,11 @@ public class FakeUserInfoHelper extends UserInfoHelper {
         return ArrayUtils.contains(getProfileIds(mCurrentUserId), userId);
     }
 
+    @Override
+    public int getCurrentUserId() {
+        return mCurrentUserId;
+    }
+
     @Override
     protected int[] getProfileIds(int userId) {
         IntArray profiles = mProfiles.get(userId);