From 8800cdf527358838bbc653f5046f1e0b19fabece Mon Sep 17 00:00:00 2001 From: Weng Su Date: Fri, 2 Jul 2021 21:27:21 +0800 Subject: [PATCH] Add permission checking to WifiDialogActivity - Use getCallingPackage() to get calling package. - Check if the calling package has ACCESS_COARSE_LOCATION or ACCESS_COARSE_LOCATION permission. - Only set result data to permission granted callers Bug: 185126813 Test: manual test make RunSettingsRoboTests ROBOTEST_FILTER=WifiDialogActivityTest Merged-In: If7ca069c842ed2bd1aed23f9d4041473c68a4dad Change-Id: If7ca069c842ed2bd1aed23f9d4041473c68a4dad (cherry picked from commit 71e728e934bec9c5d121b285e5c0089f658723d2) --- .../settings/wifi/WifiDialogActivity.java | 55 +++++-- .../settings/wifi/WifiDialogActivityTest.java | 147 ++++++++++++++++++ 2 files changed, 193 insertions(+), 9 deletions(-) diff --git a/src/com/android/settings/wifi/WifiDialogActivity.java b/src/com/android/settings/wifi/WifiDialogActivity.java index 77827867630..a2121908894 100644 --- a/src/com/android/settings/wifi/WifiDialogActivity.java +++ b/src/com/android/settings/wifi/WifiDialogActivity.java @@ -16,9 +16,13 @@ package com.android.settings.wifi; +import static android.Manifest.permission.ACCESS_COARSE_LOCATION; +import static android.Manifest.permission.ACCESS_FINE_LOCATION; + import android.app.Activity; import android.content.DialogInterface; import android.content.Intent; +import android.content.pm.PackageManager; import android.net.NetworkInfo; import android.net.wifi.WifiConfiguration; import android.net.wifi.WifiManager; @@ -53,10 +57,12 @@ public class WifiDialogActivity extends Activity implements WifiDialog.WifiDialo public static final String KEY_WIFI_CONFIGURATION = "wifi_configuration"; - private static final int RESULT_CONNECTED = RESULT_FIRST_USER; + @VisibleForTesting + static final int RESULT_CONNECTED = RESULT_FIRST_USER; private static final int RESULT_FORGET = RESULT_FIRST_USER + 1; - private static final int REQUEST_CODE_WIFI_DPP_ENROLLEE_QR_CODE_SCANNER = 0; + @VisibleForTesting + static final int REQUEST_CODE_WIFI_DPP_ENROLLEE_QR_CODE_SCANNER = 0; private WifiDialog mDialog; @@ -156,17 +162,22 @@ public class WifiDialogActivity extends Activity implements WifiDialog.WifiDialo } } - Intent resultData = new Intent(); + Intent resultData = hasPermissionForResult() ? createResultData(config, accessPoint) : null; + setResult(RESULT_CONNECTED, resultData); + finish(); + } + + protected Intent createResultData(WifiConfiguration config, AccessPoint accessPoint) { + Intent result = new Intent(); if (accessPoint != null) { Bundle accessPointState = new Bundle(); accessPoint.saveWifiState(accessPointState); - resultData.putExtra(KEY_ACCESS_POINT_STATE, accessPointState); + result.putExtra(KEY_ACCESS_POINT_STATE, accessPointState); } if (config != null) { - resultData.putExtra(KEY_WIFI_CONFIGURATION, config); + result.putExtra(KEY_WIFI_CONFIGURATION, config); } - setResult(RESULT_CONNECTED, resultData); - finish(); + return result; } @Override @@ -192,9 +203,35 @@ public class WifiDialogActivity extends Activity implements WifiDialog.WifiDialo if (resultCode != RESULT_OK) { return; } - - setResult(RESULT_CONNECTED, data); + if (hasPermissionForResult()) { + setResult(RESULT_CONNECTED, data); + } else { + setResult(RESULT_CONNECTED); + } finish(); } } + + protected boolean hasPermissionForResult() { + final String callingPackage = getCallingPackage(); + if (callingPackage == null) { + Log.d(TAG, "Failed to get the calling package, don't return the result."); + return false; + } + + if (getPackageManager().checkPermission(ACCESS_COARSE_LOCATION, callingPackage) + == PackageManager.PERMISSION_GRANTED) { + Log.d(TAG, "The calling package has ACCESS_COARSE_LOCATION permission for result."); + return true; + } + + if (getPackageManager().checkPermission(ACCESS_FINE_LOCATION, callingPackage) + == PackageManager.PERMISSION_GRANTED) { + Log.d(TAG, "The calling package has ACCESS_FINE_LOCATION permission for result."); + return true; + } + + Log.d(TAG, "The calling package does not have the necessary permissions for result."); + return false; + } } diff --git a/tests/robotests/src/com/android/settings/wifi/WifiDialogActivityTest.java b/tests/robotests/src/com/android/settings/wifi/WifiDialogActivityTest.java index 15a07802be2..f86f6e7befc 100644 --- a/tests/robotests/src/com/android/settings/wifi/WifiDialogActivityTest.java +++ b/tests/robotests/src/com/android/settings/wifi/WifiDialogActivityTest.java @@ -16,18 +16,32 @@ package com.android.settings.wifi; +import static android.Manifest.permission.ACCESS_COARSE_LOCATION; +import static android.Manifest.permission.ACCESS_FINE_LOCATION; + +import static com.android.settings.wifi.WifiDialogActivity.REQUEST_CODE_WIFI_DPP_ENROLLEE_QR_CODE_SCANNER; +import static com.android.settings.wifi.WifiDialogActivity.RESULT_CONNECTED; +import static com.android.settings.wifi.WifiDialogActivity.RESULT_OK; + import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import android.content.Intent; +import android.content.pm.PackageManager; import android.net.wifi.WifiConfiguration; +import android.net.wifi.WifiManager; import com.android.settings.R; import com.android.settings.testutils.shadow.ShadowAlertDialogCompat; import com.android.settings.testutils.shadow.ShadowConnectivityManager; import com.android.settings.testutils.shadow.ShadowWifiManager; import com.android.settings.wifi.dpp.WifiDppEnrolleeActivity; +import com.android.settingslib.wifi.AccessPoint; import com.google.android.setupcompat.util.WizardManagerHelper; @@ -50,13 +64,30 @@ import org.robolectric.util.ReflectionHelpers; }) public class WifiDialogActivityTest { + private static final String CALLING_PACKAGE = "calling_package"; private static final String AP1_SSID = "\"ap1\""; + + @Mock + PackageManager mPackageManager; + @Mock + WifiManager mWifiManager; + @Mock + WifiDialog mWifiDialog; + @Mock + WifiConfiguration mWifiConfiguration; + @Mock + AccessPoint mAccessPoint; + @Mock + Intent mResultData; @Mock private WifiConfigController mController; @Before public void setUp() { MockitoAnnotations.initMocks(this); + when(mWifiDialog.getController()).thenReturn(mController); + when(mController.getConfig()).thenReturn(mWifiConfiguration); + when(mController.getAccessPoint()).thenReturn(mAccessPoint); WifiConfiguration wifiConfig = new WifiConfiguration(); wifiConfig.SSID = AP1_SSID; @@ -76,6 +107,29 @@ public class WifiDialogActivityTest { assertThat(ShadowWifiManager.get().savedWifiConfig.SSID).isEqualTo(AP1_SSID); } + @Test + public void onSubmit_noPermissionForResult_setResultWithoutData() { + WifiDialogActivity activity = spy(Robolectric.setupActivity(WifiDialogActivity.class)); + when(activity.hasPermissionForResult()).thenReturn(false); + when(activity.getSystemService(WifiManager.class)).thenReturn(mWifiManager); + + activity.onSubmit(mWifiDialog); + + verify(activity).setResult(RESULT_CONNECTED, null); + } + + @Test + public void onSubmit_hasPermissionForResult_setResultWithData() { + WifiDialogActivity activity = spy(Robolectric.setupActivity(WifiDialogActivity.class)); + when(activity.hasPermissionForResult()).thenReturn(true); + when(activity.createResultData(any(), any())).thenReturn(mResultData); + when(activity.getSystemService(WifiManager.class)).thenReturn(mWifiManager); + + activity.onSubmit(mWifiDialog); + + verify(activity).setResult(RESULT_CONNECTED, mResultData); + } + @Test public void onSubmit_whenConnectForCallerIsFalse_shouldNotConnectToNetwork() { WifiDialogActivity activity = @@ -132,4 +186,97 @@ public class WifiDialogActivityTest { assertThat(controller.get().getThemeResId()). isEqualTo(R.style.LightTheme_SettingsBase_SetupWizard); } + + @Test + public void onActivityResult_noPermissionForResult_setResultWithoutData() { + WifiDialogActivity activity = spy(Robolectric.setupActivity(WifiDialogActivity.class)); + when(activity.hasPermissionForResult()).thenReturn(false); + final Intent data = new Intent(); + + activity.onActivityResult(REQUEST_CODE_WIFI_DPP_ENROLLEE_QR_CODE_SCANNER, RESULT_OK, + data); + + verify(activity).setResult(RESULT_CONNECTED); + } + + @Test + public void onActivityResult_hasPermissionForResult_setResultWithData() { + WifiDialogActivity activity = spy(Robolectric.setupActivity(WifiDialogActivity.class)); + when(activity.hasPermissionForResult()).thenReturn(true); + final Intent data = new Intent(); + + activity.onActivityResult(REQUEST_CODE_WIFI_DPP_ENROLLEE_QR_CODE_SCANNER, RESULT_OK, + data); + + verify(activity).setResult(RESULT_CONNECTED, data); + } + + @Test + public void hasPermissionForResult_noCallingPackage_returnFalse() { + WifiDialogActivity activity = spy(Robolectric.setupActivity(WifiDialogActivity.class)); + when(activity.getCallingPackage()).thenReturn(null); + + final boolean result = activity.hasPermissionForResult(); + + assertThat(result).isFalse(); + } + + @Test + public void hasPermissionForResult_noPermission_returnFalse() { + WifiDialogActivity activity = spy(Robolectric.setupActivity(WifiDialogActivity.class)); + when(activity.getCallingPackage()).thenReturn(null); + when(mPackageManager.checkPermission(ACCESS_COARSE_LOCATION, CALLING_PACKAGE)) + .thenReturn(PackageManager.PERMISSION_DENIED); + when(mPackageManager.checkPermission(ACCESS_FINE_LOCATION, CALLING_PACKAGE)) + .thenReturn(PackageManager.PERMISSION_DENIED); + + final boolean result = activity.hasPermissionForResult(); + + assertThat(result).isFalse(); + } + + @Test + public void hasPermissionForResult_hasCoarseLocationPermission_returnTrue() { + WifiDialogActivity activity = spy(Robolectric.setupActivity(WifiDialogActivity.class)); + when(activity.getCallingPackage()).thenReturn(CALLING_PACKAGE); + when(activity.getPackageManager()).thenReturn(mPackageManager); + when(mPackageManager.checkPermission(ACCESS_COARSE_LOCATION, CALLING_PACKAGE)) + .thenReturn(PackageManager.PERMISSION_GRANTED); + when(mPackageManager.checkPermission(ACCESS_FINE_LOCATION, CALLING_PACKAGE)) + .thenReturn(PackageManager.PERMISSION_DENIED); + + final boolean result = activity.hasPermissionForResult(); + + assertThat(result).isTrue(); + } + + @Test + public void hasPermissionForResult_hasFineLocationPermission_returnTrue() { + WifiDialogActivity activity = spy(Robolectric.setupActivity(WifiDialogActivity.class)); + when(activity.getCallingPackage()).thenReturn(CALLING_PACKAGE); + when(activity.getPackageManager()).thenReturn(mPackageManager); + when(mPackageManager.checkPermission(ACCESS_COARSE_LOCATION, CALLING_PACKAGE)) + .thenReturn(PackageManager.PERMISSION_DENIED); + when(mPackageManager.checkPermission(ACCESS_FINE_LOCATION, CALLING_PACKAGE)) + .thenReturn(PackageManager.PERMISSION_GRANTED); + + final boolean result = activity.hasPermissionForResult(); + + assertThat(result).isTrue(); + } + + @Test + public void hasPermissionForResult_haveBothLocationPermissions_returnTrue() { + WifiDialogActivity activity = spy(Robolectric.setupActivity(WifiDialogActivity.class)); + when(activity.getCallingPackage()).thenReturn(CALLING_PACKAGE); + when(activity.getPackageManager()).thenReturn(mPackageManager); + when(mPackageManager.checkPermission(ACCESS_COARSE_LOCATION, CALLING_PACKAGE)) + .thenReturn(PackageManager.PERMISSION_GRANTED); + when(mPackageManager.checkPermission(ACCESS_FINE_LOCATION, CALLING_PACKAGE)) + .thenReturn(PackageManager.PERMISSION_GRANTED); + + final boolean result = activity.hasPermissionForResult(); + + assertThat(result).isTrue(); + } }