Merge "WifiPermissionsUtil: Resolve refactoring bug"

2 files changed, 68 insertions, 42 deletions

diff --git a/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java b/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java
index 31f17033f..110e9575e 100644
--- a/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java
+++ b/service/java/com/android/server/wifi/util/WifiPermissionsUtil.java
@@ -30,7 +30,7 @@ import java.util.List;
 
 /**
  * A wifi permissions utility assessing permissions
- * for getting scan results by a package
+ * for getting scan results by a package.
  */
 public class WifiPermissionsUtil {
     private static final String TAG = "WifiPermissionsUtil";
@@ -60,29 +60,36 @@ public class WifiPermissionsUtil {
     public boolean canAccessScanResults(String pkgName, int uid,
                 int minVersion) throws SecurityException {
         mAppOps.checkPackage(uid, pkgName);
-        // Location Permission is granted if Location Mode is enabled or if the
-        // caller has Location Permissions
-        boolean mLocationPermission = isLocationModeEnabled(pkgName, minVersion)
-                && checkCallersLocationPermission(pkgName, uid, minVersion);
-        if (!checkCallerHasPeersMacAddressPermission(uid)
-                && !isCallerActiveNwScorer(uid)
-                && !mLocationPermission) {
+        // Check if the calling Uid has CAN_READ_PEER_MAC_ADDRESS
+        // permission or is an Active Nw scorer.
+        boolean canCallingUidAccessLocation = checkCallerHasPeersMacAddressPermission(uid)
+                || isCallerActiveNwScorer(uid);
+        // LocationAccess by App: For AppVersion older than minVersion,
+        // it is sufficient to check if the App is foreground.
+        // Otherwise, Location Mode must be enabled and caller must have
+        // Coarse Location permission to have access to location information.
+        boolean canAppPackageUseLocation = isLegacyForeground(pkgName, minVersion)
+                || (isLocationModeEnabled(pkgName)
+                        && checkCallersLocationPermission(pkgName, uid));
+        // If neither caller or app has location access, there is no need to check
+        // any other permissions. Deny access to scan results.
+        if (!canCallingUidAccessLocation && !canAppPackageUseLocation) {
             return false;
         }
+        // Check if Wifi Scan request is an operation allowed for this App.
         if (!isScanAllowedbyApps(pkgName, uid)) {
             return false;
         }
-        if (!isCurrentProfile(uid)) {
-            return false;
-        }
-        if (!checkInteractAcrossUsersFull(uid)) {
+        // If the User or profile is current, permission is granted
+        // Otherwise, uid must have INTERACT_ACROSS_USERS_FULL permission.
+        if (!isCurrentProfile(uid) && !checkInteractAcrossUsersFull(uid)) {
             return false;
         }
         return true;
     }
 
     /**
-     * Returns true if the caller holds PEERS_MAC_ADDRESS permission
+     * Returns true if the caller holds PEERS_MAC_ADDRESS permission.
      */
     private boolean checkCallerHasPeersMacAddressPermission(int uid) {
         return mWifiPermissionsWrapper.getUidPermission(
@@ -91,14 +98,15 @@ public class WifiPermissionsUtil {
     }
 
     /**
-     * Returns true if the caller is an Active Network Scorer
+     * Returns true if the caller is an Active Network Scorer.
      */
     private boolean isCallerActiveNwScorer(int uid) {
         return mWifiPermissionsWrapper.isCallerActiveNwScorer(uid);
     }
 
     /**
-     * Returns true if Wifi scan is allowed in App
+     * Returns true if Wifi scan operation is allowed for this caller
+     * and package.
      */
     private boolean isScanAllowedbyApps(String pkgName, int uid) {
         return checkAppOpAllowed(AppOpsManager.OP_WIFI_SCAN, pkgName, uid);
@@ -133,6 +141,9 @@ public class WifiPermissionsUtil {
         return false;
     }
 
+    /**
+     * Returns true if the App version is older than minVersion.
+     */
     private boolean isLegacyVersion(String pkgName, int minVersion) {
         try {
             if (mContext.getPackageManager().getApplicationInfo(pkgName, 0)
@@ -141,6 +152,8 @@ public class WifiPermissionsUtil {
             }
         } catch (PackageManager.NameNotFoundException e) {
             // In case of exception, assume known app (more strict checking)
+            // Note: This case will never happen since checkPackage is
+            // called to verify valididity before checking App's version.
         }
         return false;
     }
@@ -159,9 +172,9 @@ public class WifiPermissionsUtil {
 
     /**
      * Checks that calling process has android.Manifest.permission.ACCESS_COARSE_LOCATION
-     * and a corresponding app op is allowed for this package and uid
+     * and a corresponding app op is allowed for this package and uid.
      */
-    private boolean checkCallersLocationPermission(String pkgName, int uid, int version) {
+    private boolean checkCallersLocationPermission(String pkgName, int uid) {
         // Coarse Permission implies Fine permission
         if ((mWifiPermissionsWrapper.getUidPermission(
                 Manifest.permission.ACCESS_COARSE_LOCATION, uid)
@@ -169,17 +182,11 @@ public class WifiPermissionsUtil {
                 && checkAppOpAllowed(AppOpsManager.OP_COARSE_LOCATION, pkgName, uid)) {
             return true;
         }
-        // Location permission is granted for apps older than version if foreground
-        if (isLegacyForeground(pkgName, version)) {
-            return true;
-        }
         return false;
     }
-    private boolean isLocationModeEnabled(String pkgName, int version) {
-        // Location mode check on applications that are later than version, for older
-        // versions, foreground apps can skip this check and always return true
-        return isLegacyForeground(pkgName, version)
-                 || (mSettingsStore.getLocationModeSetting(mContext)
+    private boolean isLocationModeEnabled(String pkgName) {
+        // Location mode check on applications that are later than version.
+        return (mSettingsStore.getLocationModeSetting(mContext)
                  != Settings.Secure.LOCATION_MODE_OFF);
     }
 }
diff --git a/tests/wifitests/src/com/android/server/wifi/util/WifiPermissionsUtilTest.java b/tests/wifitests/src/com/android/server/wifi/util/WifiPermissionsUtilTest.java
index 1e05f121f..61820d780 100644
--- a/tests/wifitests/src/com/android/server/wifi/util/WifiPermissionsUtilTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/util/WifiPermissionsUtilTest.java
@@ -112,7 +112,6 @@ public class WifiPermissionsUtilTest {
      *                    Caller can read peers mac address
      *                    This App has permission to request WIFI_SCAN
      *                    User is current
-     *                    User has full permission to interact
      * Validate result is true
      * - User has all the permissions
      */
@@ -124,7 +123,6 @@ public class WifiPermissionsUtilTest {
         mPermissionsList.put(mMacAddressPermission, mUid);
         mWifiScanAllowApps = AppOpsManager.MODE_ALLOWED;
         mCurrentUser = UserHandle.USER_CURRENT_OR_SELF;
-        mPermissionsList.put(mInteractAcrossUsersFullPermission, mUid);
         setupTestCase();
         WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper,
                 mMockContext, mMockWifiSettingsStore, mMockUserManager);
@@ -141,7 +139,6 @@ public class WifiPermissionsUtilTest {
      *                    Caller can read peers mac address
      *                    This App has permission to request WIFI_SCAN
      *                    User profile is current
-     *                    User has full permission to interact
      * Validate result is true
      * - User has all the permissions
      */
@@ -153,7 +150,6 @@ public class WifiPermissionsUtilTest {
         mPermissionsList.put(mMacAddressPermission, mUid);
         mWifiScanAllowApps = AppOpsManager.MODE_ALLOWED;
         mMockUserInfo.id = mCallingUser;
-        mPermissionsList.put(mInteractAcrossUsersFullPermission, mUid);
         setupTestCase();
         WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper,
                 mMockContext, mMockWifiSettingsStore, mMockUserManager);
@@ -191,11 +187,40 @@ public class WifiPermissionsUtilTest {
      * Test case setting: Package is valid
      *                    Caller can read peers mac address
      *                    This App has permission to request WIFI_SCAN
+     *                    User or profile is not current but the uid has
+     *                    permission to INTERACT_ACROSS_USERS_FULL
+     * Validate result is true
+     * - User has all the permissions
+     */
+    @Test
+    public void testCanAccessScanResults_UserOrProfileNotCurrent() throws Exception {
+        boolean output = false;
+        mThrowSecurityException = false;
+        mUid = MANAGED_PROFILE_UID;
+        mPermissionsList.put(mMacAddressPermission, mUid);
+        mWifiScanAllowApps = AppOpsManager.MODE_ALLOWED;
+        mPermissionsList.put(mInteractAcrossUsersFullPermission, mUid);
+        setupTestCase();
+        WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper,
+                mMockContext, mMockWifiSettingsStore, mMockUserManager);
+        try {
+            output = codeUnderTest.canAccessScanResults(TEST_PACKAGE_NAME, mUid, mTargetVersion);
+        } catch (SecurityException e) {
+            throw e;
+        }
+        assertEquals(output, true);
+    }
+
+    /**
+     * Test case setting: Package is valid
+     *                    Caller can read peers mac address
+     *                    This App has permission to request WIFI_SCAN
+     *                    User or profile is not Current
      * Validate result is false
-     * - User or profile is not current
+     * - Calling uid doesn't have INTERACT_ACROSS_USERS_FULL permission
      */
     @Test
-    public void testCannotAccessScanResults_UserOrProfileNotCurrent() throws Exception {
+    public void testCannotAccessScanResults_NoInteractAcrossUsersFullPermission() throws Exception {
         boolean output = true;
         mThrowSecurityException = false;
         mUid = MANAGED_PROFILE_UID;
@@ -217,16 +242,14 @@ public class WifiPermissionsUtilTest {
      *                    Caller is active network scorer
      *                    This App has permission to request WIFI_SCAN
      *                    User is current
-     * Validate result is false
-     * - User doesn't have Interact Across Users Full Permission
+     * Validate result is true
      */
     @Test
-    public void testCannotAccessScanResults_NoInteractAcrossUsersFullPermission() throws Exception {
-        boolean output = true;
+    public void testCanAccessScanResults_CallerIsActiveNwScorer() throws Exception {
+        boolean output = false;
         mThrowSecurityException = false;
         mActiveNwScorer = true;
         mWifiScanAllowApps = AppOpsManager.MODE_ALLOWED;
-        mUid = MANAGED_PROFILE_UID;
         mCurrentUser = UserHandle.USER_CURRENT_OR_SELF;
         setupTestCase();
         WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper,
@@ -236,7 +259,7 @@ public class WifiPermissionsUtilTest {
         } catch (SecurityException e) {
             throw e;
         }
-        assertEquals(output, false);
+        assertEquals(output, true);
     }
 
     /**
@@ -245,7 +268,6 @@ public class WifiPermissionsUtilTest {
      *                    Foreground
      *                    This App has permission to request WIFI_SCAN
      *                    User is current
-     *                    User has full permission to interact
      *  Validate result is true - has all permissions
      */
     @Test
@@ -257,7 +279,6 @@ public class WifiPermissionsUtilTest {
         mWifiScanAllowApps = AppOpsManager.MODE_ALLOWED;
         mUid = MANAGED_PROFILE_UID;
         mCurrentUser = UserHandle.USER_CURRENT_OR_SELF;
-        mPermissionsList.put(mInteractAcrossUsersFullPermission, mUid);
         setupTestCase();
         WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper,
                 mMockContext, mMockWifiSettingsStore, mMockUserManager);
@@ -276,7 +297,6 @@ public class WifiPermissionsUtilTest {
      *                    Coarse Location Access
      *                    This App has permission to request WIFI_SCAN
      *                    User profile is current
-     *                    User has full permission to interact
      *  Validate result is true - has all permissions
      */
     @Test
@@ -290,7 +310,6 @@ public class WifiPermissionsUtilTest {
         mWifiScanAllowApps = AppOpsManager.MODE_ALLOWED;
         mUid = MANAGED_PROFILE_UID;
         mMockUserInfo.id = mCallingUser;
-        mPermissionsList.put(mInteractAcrossUsersFullPermission, mUid);
         setupTestCase();
         WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper,
                 mMockContext, mMockWifiSettingsStore, mMockUserManager);