From 71e728e934bec9c5d121b285e5c0089f658723d2 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 Change-Id: If7ca069c842ed2bd1aed23f9d4041473c68a4dad --- .../settings/wifi/WifiDialogActivity.java | 60 ++++-- .../settings/wifi/WifiDialogActivityTest.java | 175 ++++++++++++++++++ 2 files changed, 222 insertions(+), 13 deletions(-) diff --git a/src/com/android/settings/wifi/WifiDialogActivity.java b/src/com/android/settings/wifi/WifiDialogActivity.java index 877933edafe..abe1205c544 100644 --- a/src/com/android/settings/wifi/WifiDialogActivity.java +++ b/src/com/android/settings/wifi/WifiDialogActivity.java @@ -16,8 +16,12 @@ package com.android.settings.wifi; +import static android.Manifest.permission.ACCESS_COARSE_LOCATION; +import static android.Manifest.permission.ACCESS_FINE_LOCATION; + 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; @@ -78,10 +82,12 @@ public class WifiDialogActivity extends ObservableActivity implements WifiDialog 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; // Max age of tracked WifiEntries. private static final long MAX_SCAN_AGE_MILLIS = 15_000; @@ -258,10 +264,7 @@ public class WifiDialogActivity extends ObservableActivity implements WifiDialog } } - final Intent resultData = new Intent(); - if (config != null) { - resultData.putExtra(KEY_WIFI_CONFIGURATION, config); - } + Intent resultData = hasPermissionForResult() ? createResultData(config, null) : null; setResult(RESULT_CONNECTED, resultData); finish(); } @@ -289,17 +292,22 @@ public class WifiDialogActivity extends ObservableActivity implements WifiDialog } } - 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 @@ -335,9 +343,35 @@ public class WifiDialogActivity extends ObservableActivity implements WifiDialog 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 f601e36229c..685819e7839 100644 --- a/tests/robotests/src/com/android/settings/wifi/WifiDialogActivityTest.java +++ b/tests/robotests/src/com/android/settings/wifi/WifiDialogActivityTest.java @@ -16,12 +16,25 @@ 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 androidx.lifecycle.Lifecycle.State; import androidx.test.core.app.ActivityScenario; @@ -31,6 +44,7 @@ import com.android.settings.testutils.shadow.ShadowAlertDialogCompat; import com.android.settings.testutils.shadow.ShadowConnectivityManager; import com.android.settings.testutils.shadow.ShadowNetworkDetailsTracker; import com.android.settings.testutils.shadow.ShadowWifiManager; +import com.android.settingslib.wifi.AccessPoint; import com.google.android.setupcompat.util.WizardManagerHelper; @@ -55,7 +69,25 @@ 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 + WifiDialog2 mWifiDialog2; + @Mock + WifiConfigController2 mWifiConfiguration2; + @Mock + Intent mResultData; @Mock private WifiConfigController mController; @Mock @@ -66,6 +98,10 @@ public class WifiDialogActivityTest { @Before public void setUp() { MockitoAnnotations.initMocks(this); + when(mWifiDialog.getController()).thenReturn(mController); + when(mController.getConfig()).thenReturn(mWifiConfiguration); + when(mController.getAccessPoint()).thenReturn(mAccessPoint); + when(mWifiDialog2.getController()).thenReturn(mWifiConfiguration2); WifiConfiguration wifiConfig = new WifiConfiguration(); wifiConfig.SSID = AP1_SSID; @@ -97,6 +133,52 @@ 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 onSubmit2_noPermissionForResult_setResultWithoutData() { + WifiDialogActivity activity = spy(Robolectric.setupActivity(WifiDialogActivity.class)); + when(activity.hasPermissionForResult()).thenReturn(false); + when(activity.getSystemService(WifiManager.class)).thenReturn(mWifiManager); + + activity.onSubmit(mWifiDialog2); + + verify(activity).setResult(RESULT_CONNECTED, null); + } + + @Test + public void onSubmit2_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(mWifiDialog2); + + verify(activity).setResult(RESULT_CONNECTED, mResultData); + } + @Test @Ignore public void onSubmit2_whenConnectForCallerIsTrue_shouldConnectToNetwork() { @@ -178,4 +260,97 @@ public class WifiDialogActivityTest { assertThat(dialog.getContext().getThemeResId()) .isEqualTo(R.style.SuwAlertDialogThemeCompat_Light); } + + @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(); + } }