From 12dcd5034ac798dd6cc0f6e8dd5a38bab684da61 Mon Sep 17 00:00:00 2001 From: Weng Su Date: Fri, 1 Oct 2021 08:27:32 +0000 Subject: [PATCH 1/2] [RESTRICT AUTOMERGE] Revert "[DO NOT MERGE] Add SafetyNet logging" Revert submission 15207347-add_permission_wifidialog Reason for revert: rollback CLs to avoid compatibility risks Reverted Changes: If7ca069c8:[DO NOT MERGE] Add permission checking to WifiDial... I1535f6f2d:[DO NOT MERGE] Add SafetyNet logging Bug: 185126813 Change-Id: I41721dbb34612144d00dba7885944d834bf93c7d --- src/com/android/settings/wifi/WifiDialogActivity.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/com/android/settings/wifi/WifiDialogActivity.java b/src/com/android/settings/wifi/WifiDialogActivity.java index 53c941e9c21..a2121908894 100644 --- a/src/com/android/settings/wifi/WifiDialogActivity.java +++ b/src/com/android/settings/wifi/WifiDialogActivity.java @@ -28,7 +28,6 @@ import android.net.wifi.WifiConfiguration; import android.net.wifi.WifiManager; import android.net.wifi.WifiManager.ActionListener; import android.os.Bundle; -import android.util.EventLog; import android.util.Log; import androidx.annotation.VisibleForTesting; @@ -217,7 +216,6 @@ public class WifiDialogActivity extends Activity implements WifiDialog.WifiDialo final String callingPackage = getCallingPackage(); if (callingPackage == null) { Log.d(TAG, "Failed to get the calling package, don't return the result."); - EventLog.writeEvent(0x534e4554, "185126813", -1 /* UID */, "no calling package"); return false; } @@ -234,14 +232,6 @@ public class WifiDialogActivity extends Activity implements WifiDialog.WifiDialo } Log.d(TAG, "The calling package does not have the necessary permissions for result."); - try { - EventLog.writeEvent(0x534e4554, "185126813", - getPackageManager().getPackageUid(callingPackage, 0 /* flags */), - "no permission"); - } catch (PackageManager.NameNotFoundException e) { - EventLog.writeEvent(0x534e4554, "185126813", -1 /* UID */, "no permission"); - Log.w(TAG, "Cannot find the UID, calling package: " + callingPackage, e); - } return false; } } From 18043562c1389e79e4c45a37a859401a7820bbd1 Mon Sep 17 00:00:00 2001 From: Weng Su Date: Fri, 1 Oct 2021 08:27:32 +0000 Subject: [PATCH 2/2] [RESTRICT AUTOMERGE] Revert "[DO NOT MERGE] Add permission checking to WifiDialogActivity" Revert submission 15207347-add_permission_wifidialog Reason for revert: rollback CLs to avoid compatibility risks Reverted Changes: If7ca069c8:[DO NOT MERGE] Add permission checking to WifiDial... I1535f6f2d:[DO NOT MERGE] Add SafetyNet logging Bug: 185126813 Change-Id: Ifc3b53dc91fc8d5aa31c1a10d4eb033bbe54c47d --- .../settings/wifi/WifiDialogActivity.java | 55 ++----- .../settings/wifi/WifiDialogActivityTest.java | 147 ------------------ 2 files changed, 9 insertions(+), 193 deletions(-) diff --git a/src/com/android/settings/wifi/WifiDialogActivity.java b/src/com/android/settings/wifi/WifiDialogActivity.java index a2121908894..77827867630 100644 --- a/src/com/android/settings/wifi/WifiDialogActivity.java +++ b/src/com/android/settings/wifi/WifiDialogActivity.java @@ -16,13 +16,9 @@ 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; @@ -57,12 +53,10 @@ public class WifiDialogActivity extends Activity implements WifiDialog.WifiDialo public static final String KEY_WIFI_CONFIGURATION = "wifi_configuration"; - @VisibleForTesting - static final int RESULT_CONNECTED = RESULT_FIRST_USER; + private static final int RESULT_CONNECTED = RESULT_FIRST_USER; private static final int RESULT_FORGET = RESULT_FIRST_USER + 1; - @VisibleForTesting - static final int REQUEST_CODE_WIFI_DPP_ENROLLEE_QR_CODE_SCANNER = 0; + private static final int REQUEST_CODE_WIFI_DPP_ENROLLEE_QR_CODE_SCANNER = 0; private WifiDialog mDialog; @@ -162,22 +156,17 @@ public class WifiDialogActivity extends Activity implements WifiDialog.WifiDialo } } - Intent resultData = hasPermissionForResult() ? createResultData(config, accessPoint) : null; - setResult(RESULT_CONNECTED, resultData); - finish(); - } - - protected Intent createResultData(WifiConfiguration config, AccessPoint accessPoint) { - Intent result = new Intent(); + Intent resultData = new Intent(); if (accessPoint != null) { Bundle accessPointState = new Bundle(); accessPoint.saveWifiState(accessPointState); - result.putExtra(KEY_ACCESS_POINT_STATE, accessPointState); + resultData.putExtra(KEY_ACCESS_POINT_STATE, accessPointState); } if (config != null) { - result.putExtra(KEY_WIFI_CONFIGURATION, config); + resultData.putExtra(KEY_WIFI_CONFIGURATION, config); } - return result; + setResult(RESULT_CONNECTED, resultData); + finish(); } @Override @@ -203,35 +192,9 @@ public class WifiDialogActivity extends Activity implements WifiDialog.WifiDialo if (resultCode != RESULT_OK) { return; } - if (hasPermissionForResult()) { - setResult(RESULT_CONNECTED, data); - } else { - setResult(RESULT_CONNECTED); - } + + setResult(RESULT_CONNECTED, data); 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 1a6de550e5f..41d1bbe4f38 100644 --- a/tests/robotests/src/com/android/settings/wifi/WifiDialogActivityTest.java +++ b/tests/robotests/src/com/android/settings/wifi/WifiDialogActivityTest.java @@ -16,31 +16,17 @@ 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.settingslib.wifi.AccessPoint; import com.google.android.setupcompat.util.WizardManagerHelper; @@ -62,30 +48,13 @@ 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; @@ -105,29 +74,6 @@ 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 = @@ -165,97 +111,4 @@ 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(); - } }