From aaba13594c7b375148a5c1e819a7f303f324d8f6 Mon Sep 17 00:00:00 2001 From: xshu Date: Tue, 27 Mar 2018 15:14:57 -0700 Subject: Refactor canAccessScanResult WifiPermissionUtil.canAccessScanResults is now void. Where it would return false before it now throws a SecurityException. Also fixed an existing test that is passing for the wrong reason. The test should be checking for a failure, but the CAN_READ_PEER_MAC_ADDRESS permission made it pass. Bug: 73160471 Test: compile, run ./frameworks/opt/net/wifi/tests/wifitests/runtests.sh Change-Id: If32a546ee646e6c8e786a23c017c86f33be7e304 --- .../server/wifi/ScoredNetworkEvaluatorTest.java | 17 ++-- .../android/server/wifi/WifiServiceImplTest.java | 13 +-- .../server/wifi/util/WifiPermissionsUtilTest.java | 111 +++++++-------------- 3 files changed, 49 insertions(+), 92 deletions(-) (limited to 'tests') diff --git a/tests/wifitests/src/com/android/server/wifi/ScoredNetworkEvaluatorTest.java b/tests/wifitests/src/com/android/server/wifi/ScoredNetworkEvaluatorTest.java index 65454c421..1afbf04e5 100644 --- a/tests/wifitests/src/com/android/server/wifi/ScoredNetworkEvaluatorTest.java +++ b/tests/wifitests/src/com/android/server/wifi/ScoredNetworkEvaluatorTest.java @@ -88,9 +88,6 @@ public class ScoredNetworkEvaluatorTest { Settings.Global.NETWORK_RECOMMENDATIONS_ENABLED, 0)) .thenReturn(1); - when(mWifiPermissionsUtil.canAccessScanResults(eq(TEST_PACKAGE_NAME), anyInt())) - .thenReturn(true); - ArgumentCaptor observerCaptor = ArgumentCaptor.forClass(ContentObserver.class); mScoreCache = new WifiNetworkScoreCache(mContext); @@ -234,8 +231,8 @@ public class ScoredNetworkEvaluatorTest { int[] securities = {SECURITY_PSK, SECURITY_NONE}; int[] levels = {mThresholdQualifiedRssi2G + 8, mThresholdQualifiedRssi2G + 10}; - when(mWifiPermissionsUtil.canAccessScanResults(any(), anyInt())) - .thenReturn(false); + doThrow(new SecurityException()).when(mWifiPermissionsUtil).enforceCanAccessScanResults( + any(), anyInt()); ScanDetailsAndWifiConfigs scanDetailsAndConfigs = WifiNetworkSelectorTestUtil .setupScanDetailsAndConfigStore( @@ -244,7 +241,8 @@ public class ScoredNetworkEvaluatorTest { mScoredNetworkEvaluator.update(scanDetailsAndConfigs.getScanDetails()); verify(mNetworkScoreManager, never()).requestScores(any()); - verify(mWifiPermissionsUtil).canAccessScanResults(eq(TEST_PACKAGE_NAME), eq(TEST_UID)); + verify(mWifiPermissionsUtil).enforceCanAccessScanResults( + eq(TEST_PACKAGE_NAME), eq(TEST_UID)); } @Test @@ -256,8 +254,8 @@ public class ScoredNetworkEvaluatorTest { int[] securities = {SECURITY_PSK, SECURITY_NONE}; int[] levels = {mThresholdQualifiedRssi2G + 8, mThresholdQualifiedRssi2G + 10}; - when(mWifiPermissionsUtil.canAccessScanResults(any(), anyInt())) - .thenThrow(new SecurityException()); + doThrow(new SecurityException()).when(mWifiPermissionsUtil).enforceCanAccessScanResults( + any(), anyInt()); ScanDetailsAndWifiConfigs scanDetailsAndConfigs = WifiNetworkSelectorTestUtil .setupScanDetailsAndConfigStore( @@ -266,7 +264,8 @@ public class ScoredNetworkEvaluatorTest { mScoredNetworkEvaluator.update(scanDetailsAndConfigs.getScanDetails()); verify(mNetworkScoreManager, never()).requestScores(any()); - verify(mWifiPermissionsUtil).canAccessScanResults(eq(TEST_PACKAGE_NAME), eq(TEST_UID)); + verify(mWifiPermissionsUtil).enforceCanAccessScanResults( + eq(TEST_PACKAGE_NAME), eq(TEST_UID)); } /** diff --git a/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java b/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java index 72e463100..fecbcea23 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java @@ -1049,7 +1049,8 @@ public class WifiServiceImplTest { public void testConnectedIdsAreHiddenFromAppWithoutPermission() throws Exception { setupForGetConnectionInfo(); - when(mWifiPermissionsUtil.canAccessScanResults(anyString(), anyInt())).thenReturn(false); + doThrow(new SecurityException()).when(mWifiPermissionsUtil).enforceCanAccessScanResults( + anyString(), anyInt()); WifiInfo connectionInfo = mWifiServiceImpl.getConnectionInfo(TEST_PACKAGE); @@ -1059,14 +1060,14 @@ public class WifiServiceImplTest { /** * Test that connected SSID and BSSID are not exposed to an app that does not have the - * appropriate permissions, when canAccessScanResults raises a SecurityException. + * appropriate permissions, when enforceCanAccessScanResults raises a SecurityException. */ @Test public void testConnectedIdsAreHiddenOnSecurityException() throws Exception { setupForGetConnectionInfo(); - when(mWifiPermissionsUtil.canAccessScanResults(anyString(), anyInt())) - .thenThrow(new SecurityException()); + doThrow(new SecurityException()).when(mWifiPermissionsUtil).enforceCanAccessScanResults( + anyString(), anyInt()); WifiInfo connectionInfo = mWifiServiceImpl.getConnectionInfo(TEST_PACKAGE); @@ -1082,8 +1083,6 @@ public class WifiServiceImplTest { public void testConnectedIdsAreVisibleFromPermittedApp() throws Exception { setupForGetConnectionInfo(); - when(mWifiPermissionsUtil.canAccessScanResults(anyString(), anyInt())).thenReturn(true); - WifiInfo connectionInfo = mWifiServiceImpl.getConnectionInfo(TEST_PACKAGE); assertEquals(TEST_SSID_WITH_QUOTES, connectionInfo.getSSID()); @@ -1105,7 +1104,6 @@ public class WifiServiceImplTest { when(mScanRequestProxy.getScanResults()).thenReturn(scanResultList); String packageName = "test.com"; - when(mWifiPermissionsUtil.canAccessScanResults(eq(packageName), anyInt())).thenReturn(true); List retrievedScanResultList = mWifiServiceImpl.getScanResults(packageName); verify(mScanRequestProxy).getScanResults(); @@ -1131,7 +1129,6 @@ public class WifiServiceImplTest { when(mScanRequestProxy.getScanResults()).thenReturn(scanResultList); String packageName = "test.com"; - when(mWifiPermissionsUtil.canAccessScanResults(eq(packageName), anyInt())).thenReturn(true); List retrievedScanResultList = mWifiServiceImpl.getScanResults(packageName); verify(mScanRequestProxy, never()).getScanResults(); 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 6e582db6d..bfe660c9d 100644 --- a/tests/wifitests/src/com/android/server/wifi/util/WifiPermissionsUtilTest.java +++ b/tests/wifitests/src/com/android/server/wifi/util/WifiPermissionsUtilTest.java @@ -16,9 +16,9 @@ package com.android.server.wifi.util; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.doAnswer; @@ -161,12 +161,11 @@ public class WifiPermissionsUtilTest { * Caller can read peers mac address * This App has permission to request WIFI_SCAN * User is current - * Validate result is true + * Validate no Exceptions are thrown * - User has all the permissions */ @Test public void testCanReadPeersMacAddressCurrentUserAndAllPermissions() throws Exception { - boolean output = false; mThrowSecurityException = false; mUid = MANAGED_PROFILE_UID; mPermissionsList.put(mMacAddressPermission, mUid); @@ -175,12 +174,7 @@ public class WifiPermissionsUtilTest { setupTestCase(); WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper, mMockContext, mMockWifiSettingsStore, mMockUserManager, mWifiInjector); - try { - output = codeUnderTest.canAccessScanResults(TEST_PACKAGE_NAME, mUid); - } catch (SecurityException e) { - throw e; - } - assertEquals(output, true); + codeUnderTest.enforceCanAccessScanResults(TEST_PACKAGE_NAME, mUid); } /** @@ -188,12 +182,11 @@ public class WifiPermissionsUtilTest { * Caller can read peers mac address * This App has permission to request WIFI_SCAN * User profile is current - * Validate result is true + * Validate no Exceptions are thrown * - User has all the permissions */ @Test public void testCanReadPeersMacAddressCurrentProfileAndAllPermissions() throws Exception { - boolean output = false; mThrowSecurityException = false; mUid = MANAGED_PROFILE_UID; mPermissionsList.put(mMacAddressPermission, mUid); @@ -202,34 +195,27 @@ public class WifiPermissionsUtilTest { setupTestCase(); WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper, mMockContext, mMockWifiSettingsStore, mMockUserManager, mWifiInjector); - try { - output = codeUnderTest.canAccessScanResults(TEST_PACKAGE_NAME, mUid); - } catch (SecurityException e) { - throw e; - } - assertEquals(output, true); + codeUnderTest.enforceCanAccessScanResults(TEST_PACKAGE_NAME, mUid); } /** * Test case setting: Package is valid * Caller can read peers mac address - * Validate result is false + * Validate that a SecurityException is thrown * - This App doesn't have permission to request Wifi Scan */ @Test public void testCannotAccessScanResult_AppNotAllowed() throws Exception { - boolean output = true; mThrowSecurityException = false; mPermissionsList.put(mMacAddressPermission, mUid); setupTestCase(); WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper, mMockContext, mMockWifiSettingsStore, mMockUserManager, mWifiInjector); try { - output = codeUnderTest.canAccessScanResults(TEST_PACKAGE_NAME, mUid); + codeUnderTest.enforceCanAccessScanResults(TEST_PACKAGE_NAME, mUid); + fail("Expected SecurityException is not thrown"); } catch (SecurityException e) { - throw e; } - assertEquals(output, false); } /** @@ -238,12 +224,11 @@ public class WifiPermissionsUtilTest { * 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 + * Validate no Exceptions are thrown * - User has all the permissions */ @Test - public void testCanAccessScanResults_UserOrProfileNotCurrent() throws Exception { - boolean output = false; + public void testenforceCanAccessScanResults_UserOrProfileNotCurrent() throws Exception { mThrowSecurityException = false; mUid = MANAGED_PROFILE_UID; mPermissionsList.put(mMacAddressPermission, mUid); @@ -252,12 +237,7 @@ public class WifiPermissionsUtilTest { setupTestCase(); WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper, mMockContext, mMockWifiSettingsStore, mMockUserManager, mWifiInjector); - try { - output = codeUnderTest.canAccessScanResults(TEST_PACKAGE_NAME, mUid); - } catch (SecurityException e) { - throw e; - } - assertEquals(output, true); + codeUnderTest.enforceCanAccessScanResults(TEST_PACKAGE_NAME, mUid); } /** @@ -265,12 +245,11 @@ public class WifiPermissionsUtilTest { * Caller can read peers mac address * This App has permission to request WIFI_SCAN * User or profile is not Current - * Validate result is false + * Validate that a SecurityException is thrown * - Calling uid doesn't have INTERACT_ACROSS_USERS_FULL permission */ @Test public void testCannotAccessScanResults_NoInteractAcrossUsersFullPermission() throws Exception { - boolean output = true; mThrowSecurityException = false; mUid = MANAGED_PROFILE_UID; mPermissionsList.put(mMacAddressPermission, mUid); @@ -279,11 +258,10 @@ public class WifiPermissionsUtilTest { WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper, mMockContext, mMockWifiSettingsStore, mMockUserManager, mWifiInjector); try { - output = codeUnderTest.canAccessScanResults(TEST_PACKAGE_NAME, mUid); + codeUnderTest.enforceCanAccessScanResults(TEST_PACKAGE_NAME, mUid); + fail("Expected SecurityException is not thrown"); } catch (SecurityException e) { - throw e; } - assertEquals(output, false); } /** @@ -291,11 +269,10 @@ public class WifiPermissionsUtilTest { * Foreground * This App has permission to request WIFI_SCAN * User is current - * Validate result is false - app does not have location permission + * Validate that a SecurityException is thrown - app does not have location permission */ @Test public void testLegacyForegroundAppWithOtherPermissionsDenied() throws Exception { - boolean output = false; mThrowSecurityException = false; mMockApplInfo.targetSdkVersion = Build.VERSION_CODES.GINGERBREAD; mPkgNameOfTopActivity = TEST_PACKAGE_NAME; @@ -306,11 +283,10 @@ public class WifiPermissionsUtilTest { WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper, mMockContext, mMockWifiSettingsStore, mMockUserManager, mWifiInjector); try { - output = codeUnderTest.canAccessScanResults(TEST_PACKAGE_NAME, mUid); + codeUnderTest.enforceCanAccessScanResults(TEST_PACKAGE_NAME, mUid); + fail("Expected SecurityException is not thrown"); } catch (SecurityException e) { - throw e; } - assertEquals(output, false); } /** @@ -319,11 +295,10 @@ public class WifiPermissionsUtilTest { * Coarse Location Access * This App has permission to request WIFI_SCAN * User profile is current - * Validate result is true - has all permissions + * Validate no Exceptions are thrown - has all permissions */ @Test public void testLegacyAppHasLocationAndAllPermissions() throws Exception { - boolean output = false; mThrowSecurityException = false; mMockApplInfo.targetSdkVersion = Build.VERSION_CODES.GINGERBREAD; mLocationModeSetting = Settings.Secure.LOCATION_MODE_HIGH_ACCURACY; @@ -335,18 +310,13 @@ public class WifiPermissionsUtilTest { setupTestCase(); WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper, mMockContext, mMockWifiSettingsStore, mMockUserManager, mWifiInjector); - try { - output = codeUnderTest.canAccessScanResults(TEST_PACKAGE_NAME, mUid); - } catch (SecurityException e) { - throw e; - } - assertEquals(output, true); + codeUnderTest.enforceCanAccessScanResults(TEST_PACKAGE_NAME, mUid); } /** * Test case setting: Package is valid * Location Mode Enabled - * Validate result is false + * Validate that a SecurityException is thrown * - Doesn't have Peer Mac Address read permission * - Uid is not an active network scorer * - Location Mode is enabled but the uid @@ -355,36 +325,31 @@ public class WifiPermissionsUtilTest { */ @Test public void testCannotAccessScanResults_NoCoarseLocationPermission() throws Exception { - boolean output = true; mThrowSecurityException = false; mLocationModeSetting = Settings.Secure.LOCATION_MODE_HIGH_ACCURACY; setupTestCase(); WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper, mMockContext, mMockWifiSettingsStore, mMockUserManager, mWifiInjector); try { - output = codeUnderTest.canAccessScanResults(TEST_PACKAGE_NAME, mUid); + codeUnderTest.enforceCanAccessScanResults(TEST_PACKAGE_NAME, mUid); + fail("Expected SecurityException is not thrown"); } catch (SecurityException e) { - throw e; } - assertEquals(output, false); } /** * Test case setting: Package is valid * Location Mode Disabled * Caller has location permission - * has Peer Mac Address read permission - * Validate result is false + * Validate an Exception is thrown * - Uid is not an active network scorer * - Uid doesn't have Coarse Location Access * - which implies No Location Permission */ @Test public void testCannotAccessScanResults_LocationModeDisabled() throws Exception { - boolean output = true; mThrowSecurityException = false; mUid = MANAGED_PROFILE_UID; - mPermissionsList.put(mMacAddressPermission, mUid); mWifiScanAllowApps = AppOpsManager.MODE_ALLOWED; mPermissionsList.put(mInteractAcrossUsersFullPermission, mUid); mLocationModeSetting = Settings.Secure.LOCATION_MODE_OFF; @@ -396,11 +361,10 @@ public class WifiPermissionsUtilTest { WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper, mMockContext, mMockWifiSettingsStore, mMockUserManager, mWifiInjector); try { - output = codeUnderTest.canAccessScanResults(TEST_PACKAGE_NAME, mUid); + codeUnderTest.enforceCanAccessScanResults(TEST_PACKAGE_NAME, mUid); + fail("Expected SecurityException is not thrown"); } catch (SecurityException e) { - throw e; } - assertEquals(true, output); } /** @@ -408,7 +372,7 @@ public class WifiPermissionsUtilTest { * Location Mode Disabled * Caller has location permisson * Caller has CHANGE_WIFI_STATE - * Validate result is false + * Validate no Exceptions are thrown * - Doesn't have Peer Mac Address read permission * - Uid is not an active network scorer * - Location Mode is enabled but the uid @@ -416,8 +380,8 @@ public class WifiPermissionsUtilTest { * - which implies scan result access */ @Test - public void testCanAccessScanResults_LocationModeDisabledHasChangeWifiState() throws Exception { - boolean output = false; + public void testenforceCanAccessScanResults_LocationModeDisabledHasChangeWifiState() + throws Exception { mThrowSecurityException = false; mUid = MANAGED_PROFILE_UID; mPermissionsList.put(mMacAddressPermission, mUid); @@ -432,11 +396,10 @@ public class WifiPermissionsUtilTest { WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper, mMockContext, mMockWifiSettingsStore, mMockUserManager, mWifiInjector); try { - output = codeUnderTest.canAccessScanResults(TEST_PACKAGE_NAME, mUid); + codeUnderTest.enforceCanAccessScanResults(TEST_PACKAGE_NAME, mUid); } catch (SecurityException e) { throw e; } - assertEquals(true, output); } /** @@ -444,7 +407,7 @@ public class WifiPermissionsUtilTest { * Location Mode Disabled * Caller has location permisson * Caller has ACCESS_WIFI_STATE - * Validate result is false + * Validate no Exceptions are thrown * - Doesn't have Peer Mac Address read permission * - Uid is not an active network scorer * - Location Mode is enabled but the uid @@ -452,8 +415,8 @@ public class WifiPermissionsUtilTest { * - which implies scan result access */ @Test - public void testCanAccessScanResults_LocationModeDisabledHasAccessWifiState() throws Exception { - boolean output = false; + public void testenforceCanAccessScanResults_LocationModeDisabledHasAccessWifiState() + throws Exception { mThrowSecurityException = false; mUid = MANAGED_PROFILE_UID; mPermissionsList.put(mMacAddressPermission, mUid); @@ -468,27 +431,25 @@ public class WifiPermissionsUtilTest { WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper, mMockContext, mMockWifiSettingsStore, mMockUserManager, mWifiInjector); try { - output = codeUnderTest.canAccessScanResults(TEST_PACKAGE_NAME, mUid); + codeUnderTest.enforceCanAccessScanResults(TEST_PACKAGE_NAME, mUid); } catch (SecurityException e) { throw e; } - assertEquals(true, output); } /** * Test case setting: Invalid Package * Expect a securityException */ - @Test (expected = SecurityException.class) + @Test public void testInvalidPackage() throws Exception { - boolean output = false; setupTestCase(); WifiPermissionsUtil codeUnderTest = new WifiPermissionsUtil(mMockPermissionsWrapper, mMockContext, mMockWifiSettingsStore, mMockUserManager, mWifiInjector); try { - output = codeUnderTest.canAccessScanResults(TEST_PACKAGE_NAME, mUid); + codeUnderTest.enforceCanAccessScanResults(TEST_PACKAGE_NAME, mUid); + fail("Expected SecurityException is not thrown"); } catch (SecurityException e) { - throw e; } } -- cgit v1.2.3