Merge "Update location permission check for ConnectivityUtil"

2 files changed, 67 insertions, 86 deletions

diff --git a/core/java/com/android/internal/util/ConnectivityUtil.java b/core/java/com/android/internal/util/LocationPermissionChecker.java
index b1d4fa0d3fd3..dc318b45a7e2 100644
--- a/core/java/com/android/internal/util/ConnectivityUtil.java
+++ b/core/java/com/android/internal/util/LocationPermissionChecker.java
@@ -33,28 +33,59 @@ import com.android.internal.annotations.VisibleForTesting;
 
 
 /**
- * Utility methods for common functionality using by different networks.
+ * The methods used for location permission and location mode checking.
  *
  * @hide
  */
-public class ConnectivityUtil {
+public class LocationPermissionChecker {
 
-    private static final String TAG = "ConnectivityUtil";
+    private static final String TAG = "LocationPermissionChecker";
 
     private final Context mContext;
-    private final AppOpsManager mAppOps;
+    private final AppOpsManager mAppOpsManager;
     private final UserManager mUserManager;
+    private final LocationManager mLocationManager;
 
-    public ConnectivityUtil(Context context) {
+    public LocationPermissionChecker(Context context) {
         mContext = context;
-        mAppOps = (AppOpsManager) mContext.getSystemService(Context.APP_OPS_SERVICE);
+        mAppOpsManager = (AppOpsManager) mContext.getSystemService(Context.APP_OPS_SERVICE);
         mUserManager = (UserManager) mContext.getSystemService(Context.USER_SERVICE);
+        mLocationManager =
+            (LocationManager) context.getSystemService(Context.LOCATION_SERVICE);
     }
 
     /**
-     * API to determine if the caller has fine/coarse location permission (depending on
-     * config/targetSDK level) and the location mode is enabled for the user. SecurityException is
-     * thrown if the caller has no permission or the location mode is disabled.
+     * Check location permission granted by the caller.
+     *
+     * This API check if the location mode enabled for the caller and the caller has
+     * ACCESS_COARSE_LOCATION permission is targetSDK<29, otherwise, has ACCESS_FINE_LOCATION.
+     *
+     * @param pkgName package name of the application requesting access
+     * @param featureId The feature in the package
+     * @param uid The uid of the package
+     * @param message A message describing why the permission was checked. Only needed if this is
+     *                not inside of a two-way binder call from the data receiver
+     *
+     * @return {@code true} returns if the caller has location permission and the location mode is
+     *         enabled.
+     */
+    public boolean checkLocationPermission(String pkgName, @Nullable String featureId,
+            int uid, @Nullable String message) {
+        try {
+            enforceLocationPermission(pkgName, featureId, uid, message);
+            return true;
+        } catch (SecurityException e) {
+            return false;
+        }
+    }
+
+    /**
+     * Enforce the caller has location permission.
+     *
+     * This API determines if the location mode enabled for the caller and the caller has
+     * ACCESS_COARSE_LOCATION permission is targetSDK<29, otherwise, has ACCESS_FINE_LOCATION.
+     * SecurityException is thrown if the caller has no permission or the location mode is disabled.
+     *
      * @param pkgName package name of the application requesting access
      * @param featureId The feature in the package
      * @param uid The uid of the package
@@ -62,31 +93,21 @@ public class ConnectivityUtil {
      *                not inside of a two-way binder call from the data receiver
      */
     public void enforceLocationPermission(String pkgName, @Nullable String featureId, int uid,
-            @Nullable String message)
-            throws SecurityException {
+            @Nullable String message) throws SecurityException {
+
         checkPackage(uid, pkgName);
 
         // Location mode must be enabled
         if (!isLocationModeEnabled()) {
-            // Location mode is disabled, scan results cannot be returned
             throw new SecurityException("Location mode is disabled for the device");
         }
 
         // LocationAccess by App: caller must have Coarse/Fine Location permission to have access to
         // location information.
-        boolean canAppPackageUseLocation = checkCallersLocationPermission(pkgName, featureId,
-                uid, /* coarseForTargetSdkLessThanQ */ true, message);
-
-        // If neither caller or app has location access, there is no need to check
-        // any other permissions. Deny access to scan results.
-        if (!canAppPackageUseLocation) {
+        if (!checkCallersLocationPermission(pkgName, featureId,
+                uid, /* coarseForTargetSdkLessThanQ */ true, message)) {
             throw new SecurityException("UID " + uid + " has no location permission");
         }
-        // If the User or profile is current, permission is granted
-        // Otherwise, uid must have INTERACT_ACROSS_USERS_FULL permission.
-        if (!isCurrentProfile(uid) && !checkInteractAcrossUsersFull(uid)) {
-            throw new SecurityException("UID " + uid + " profile not permitted");
-        }
     }
 
     /**
@@ -104,6 +125,7 @@ public class ConnectivityUtil {
      */
     public boolean checkCallersLocationPermission(String pkgName, @Nullable String featureId,
             int uid, boolean coarseForTargetSdkLessThanQ, @Nullable String message) {
+
         boolean isTargetSdkLessThanQ = isTargetSdkLessThan(pkgName, Build.VERSION_CODES.Q, uid);
 
         String permissionType = Manifest.permission.ACCESS_FINE_LOCATION;
@@ -111,8 +133,7 @@ public class ConnectivityUtil {
             // Having FINE permission implies having COARSE permission (but not the reverse)
             permissionType = Manifest.permission.ACCESS_COARSE_LOCATION;
         }
-        if (getUidPermission(permissionType, uid)
-                == PackageManager.PERMISSION_DENIED) {
+        if (getUidPermission(permissionType, uid) == PackageManager.PERMISSION_DENIED) {
             return false;
         }
 
@@ -134,10 +155,8 @@ public class ConnectivityUtil {
      * Retrieves a handle to LocationManager (if not already done) and check if location is enabled.
      */
     public boolean isLocationModeEnabled() {
-        LocationManager locationManager =
-                (LocationManager) mContext.getSystemService(Context.LOCATION_SERVICE);
         try {
-            return locationManager.isLocationEnabledForUser(UserHandle.of(
+            return mLocationManager.isLocationEnabledForUser(UserHandle.of(
                     getCurrentUser()));
         } catch (Exception e) {
             Log.e(TAG, "Failure to get location mode via API, falling back to settings", e);
@@ -166,28 +185,15 @@ public class ConnectivityUtil {
 
     private boolean noteAppOpAllowed(String op, String pkgName, @Nullable String featureId,
             int uid, @Nullable String message) {
-        return mAppOps.noteOp(op, uid, pkgName) == AppOpsManager.MODE_ALLOWED;
+        return mAppOpsManager.noteOp(op, uid, pkgName) == AppOpsManager.MODE_ALLOWED;
     }
 
-    private void checkPackage(int uid, String pkgName) throws SecurityException {
+    private void checkPackage(int uid, String pkgName)
+            throws SecurityException {
         if (pkgName == null) {
             throw new SecurityException("Checking UID " + uid + " but Package Name is Null");
         }
-        mAppOps.checkPackage(uid, pkgName);
-    }
-
-    private boolean isCurrentProfile(int uid) {
-        UserHandle currentUser = UserHandle.of(getCurrentUser());
-        UserHandle callingUser = UserHandle.getUserHandleForUid(uid);
-        return currentUser.equals(callingUser)
-                || mUserManager.isSameProfileGroup(
-                        currentUser.getIdentifier(), callingUser.getIdentifier());
-    }
-
-    private boolean checkInteractAcrossUsersFull(int uid) {
-        return getUidPermission(
-                android.Manifest.permission.INTERACT_ACROSS_USERS_FULL, uid)
-                == PackageManager.PERMISSION_GRANTED;
+        mAppOpsManager.checkPackage(uid, pkgName);
     }
 
     @VisibleForTesting
diff --git a/core/tests/utiltests/src/com/android/internal/util/ConnectivityUtilTest.java b/core/tests/utiltests/src/com/android/internal/util/LocationPermissionCheckerTest.java
index 556471260141..6a0a3dcf3b68 100644
--- a/core/tests/utiltests/src/com/android/internal/util/ConnectivityUtilTest.java
+++ b/core/tests/utiltests/src/com/android/internal/util/LocationPermissionCheckerTest.java
@@ -27,6 +27,7 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import android.Manifest;
+import android.app.ActivityManager;
 import android.app.AppOpsManager;
 import android.content.Context;
 import android.content.pm.ApplicationInfo;
@@ -47,8 +48,8 @@ import org.mockito.stubbing.Answer;
 
 import java.util.HashMap;
 
-/** Unit tests for {@link ConnectivityUtil}. */
-public class ConnectivityUtilTest {
+/** Unit tests for {@link LocationPermissionChecker}. */
+public class LocationPermissionCheckerTest {
 
     public static final String TAG = "ConnectivityUtilTest";
 
@@ -84,18 +85,7 @@ public class ConnectivityUtilTest {
     private boolean mThrowSecurityException;
     private Answer<Integer> mReturnPermission;
     private HashMap<String, Integer> mPermissionsList = new HashMap<String, Integer>();
-
-    private class TestConnectivityUtil extends ConnectivityUtil {
-
-        TestConnectivityUtil(Context context) {
-            super(context);
-        }
-
-        @Override
-        protected int getCurrentUser() {
-            return mCurrentUser;
-        }
-    }
+    private LocationPermissionChecker mChecker;
 
     @Before
     public void setUp() {
@@ -140,11 +130,12 @@ public class ConnectivityUtilTest {
         mThrowSecurityException = true;
         mMockApplInfo.targetSdkVersion = Build.VERSION_CODES.M;
         mIsLocationEnabled = false;
-        mCurrentUser = UserHandle.USER_SYSTEM;
+        mCurrentUser = ActivityManager.getCurrentUser();
         mCoarseLocationPermission = PackageManager.PERMISSION_DENIED;
         mFineLocationPermission = PackageManager.PERMISSION_DENIED;
         mAllowCoarseLocationApps = AppOpsManager.MODE_ERRORED;
         mAllowFineLocationApps = AppOpsManager.MODE_ERRORED;
+        mChecker = new LocationPermissionChecker(mMockContext);
     }
 
     private void setupMockInterface() {
@@ -188,8 +179,7 @@ public class ConnectivityUtilTest {
         mWifiScanAllowApps = AppOpsManager.MODE_ALLOWED;
         mUid = mCurrentUser;
         setupTestCase();
-        new TestConnectivityUtil(mMockContext)
-                .enforceLocationPermission(TEST_PKG_NAME, TEST_FEATURE_ID, mUid, null);
+        mChecker.enforceLocationPermission(TEST_PKG_NAME, TEST_FEATURE_ID, mUid, null);
     }
 
     @Test
@@ -202,8 +192,7 @@ public class ConnectivityUtilTest {
         mAllowFineLocationApps = AppOpsManager.MODE_ALLOWED;
         mWifiScanAllowApps = AppOpsManager.MODE_ALLOWED;
         setupTestCase();
-        new TestConnectivityUtil(mMockContext)
-                .enforceLocationPermission(TEST_PKG_NAME, TEST_FEATURE_ID, mUid, null);
+        mChecker.enforceLocationPermission(TEST_PKG_NAME, TEST_FEATURE_ID, mUid, null);
     }
 
     @Test
@@ -216,22 +205,8 @@ public class ConnectivityUtilTest {
         setupTestCase();
 
         assertThrows(SecurityException.class,
-                () -> new TestConnectivityUtil(mMockContext)
-                        .enforceLocationPermission(TEST_PKG_NAME, TEST_FEATURE_ID, mUid, null));
-    }
-
-    @Test
-    public void testenforceCanAccessScanResults_UserOrProfileNotCurrent() throws Exception {
-        mIsLocationEnabled = true;
-        mThrowSecurityException = false;
-        mCoarseLocationPermission = PackageManager.PERMISSION_GRANTED;
-        mAllowCoarseLocationApps = AppOpsManager.MODE_ALLOWED;
-        mWifiScanAllowApps = AppOpsManager.MODE_ALLOWED;
-        setupTestCase();
-
-        assertThrows(SecurityException.class,
-                () -> new TestConnectivityUtil(mMockContext)
-                        .enforceLocationPermission(TEST_PKG_NAME, TEST_FEATURE_ID, mUid, null));
+                () -> mChecker.enforceLocationPermission(
+                        TEST_PKG_NAME, TEST_FEATURE_ID, mUid, null));
     }
 
     @Test
@@ -240,8 +215,8 @@ public class ConnectivityUtilTest {
         mIsLocationEnabled = true;
         setupTestCase();
         assertThrows(SecurityException.class,
-                () -> new TestConnectivityUtil(mMockContext)
-                        .enforceLocationPermission(TEST_PKG_NAME, TEST_FEATURE_ID, mUid, null));
+                () -> mChecker.enforceLocationPermission(
+                        TEST_PKG_NAME, TEST_FEATURE_ID, mUid, null));
     }
 
     @Test
@@ -255,8 +230,8 @@ public class ConnectivityUtilTest {
         setupTestCase();
 
         assertThrows(SecurityException.class,
-                () -> new TestConnectivityUtil(mMockContext)
-                        .enforceLocationPermission(TEST_PKG_NAME, TEST_FEATURE_ID, mUid, null));
+                () -> mChecker.enforceLocationPermission(
+                        TEST_PKG_NAME, TEST_FEATURE_ID, mUid, null));
         verify(mMockAppOps, never()).noteOp(anyInt(), anyInt(), anyString());
     }
 
@@ -271,8 +246,8 @@ public class ConnectivityUtilTest {
         setupTestCase();
 
         assertThrows(SecurityException.class,
-                () -> new TestConnectivityUtil(mMockContext)
-                        .enforceLocationPermission(TEST_PKG_NAME, TEST_FEATURE_ID, mUid, null));
+                () -> mChecker.enforceLocationPermission(
+                        TEST_PKG_NAME, TEST_FEATURE_ID, mUid, null));
     }
 
     private static void assertThrows(Class<? extends Exception> exceptionClass, Runnable r) {