From deeb41a8521374e4390e32e1f7942c08c8886886 Mon Sep 17 00:00:00 2001 From: Nate Jiang Date: Mon, 27 Jul 2020 18:49:27 +0000 Subject: Revert "Update IMSI protection notification flow" Revert "Make broadcast protected" Revert submission 12228948-cherrypick-imsiFlow-cq2jn82jd4 Reason for revert: protect broadcast by NETWORK_SETTINGS permission Reverted Changes: I8be7c1b9e:Update IMSI protection notification flow I0cd36cb60:Make broadcast protected Bug: 161932419 Merged-In: I8be7c1b9e66162fdd244946b47367290116a7747 Change-Id: I625df183899e59e9acfc7258518810aff050b35c --- libs/WifiTrackerLib/res/values/strings.xml | 2 +- .../server/wifi/WifiCarrierInfoManager.java | 18 ++---- service/res/values/strings.xml | 6 +- .../server/wifi/WifiCarrierInfoManagerTest.java | 75 ++++++++-------------- 4 files changed, 33 insertions(+), 68 deletions(-) diff --git a/libs/WifiTrackerLib/res/values/strings.xml b/libs/WifiTrackerLib/res/values/strings.xml index 7e81b5877..c238f663c 100644 --- a/libs/WifiTrackerLib/res/values/strings.xml +++ b/libs/WifiTrackerLib/res/values/strings.xml @@ -170,7 +170,7 @@ Sign-up complete. Connecting\u2026 - This network receives a SIM ID that can be used to track device location. Learn more + This network receives a unique ID that can be used to track device location. Learn more diff --git a/service/java/com/android/server/wifi/WifiCarrierInfoManager.java b/service/java/com/android/server/wifi/WifiCarrierInfoManager.java index 79441dae5..d483047d2 100644 --- a/service/java/com/android/server/wifi/WifiCarrierInfoManager.java +++ b/service/java/com/android/server/wifi/WifiCarrierInfoManager.java @@ -96,10 +96,6 @@ public class WifiCarrierInfoManager { @VisibleForTesting public static final String NOTIFICATION_USER_DISMISSED_INTENT_ACTION = "com.android.server.wifi.action.CarrierNetwork.USER_DISMISSED"; - /** Intent when user clicked on the notification. */ - @VisibleForTesting - public static final String NOTIFICATION_USER_CLICKED_INTENT_ACTION = - "com.android.server.wifi.action.CarrierNetwork.USER_CLICKED"; @VisibleForTesting public static final String EXTRA_CARRIER_NAME = "com.android.server.wifi.extra.CarrierNetwork.CARRIER_NAME"; @@ -227,16 +223,14 @@ public class WifiCarrierInfoManager { switch (intent.getAction()) { case NOTIFICATION_USER_ALLOWED_CARRIER_INTENT_ACTION: - handleUserAllowCarrierExemptionAction(carrierName, carrierId); - break; - case NOTIFICATION_USER_DISALLOWED_CARRIER_INTENT_ACTION: - handleUserDisallowCarrierExemptionAction(carrierName, carrierId); - break; - case NOTIFICATION_USER_CLICKED_INTENT_ACTION: + Log.i(TAG, "User clicked to allow carrier"); sendImsiPrivacyConfirmationDialog(carrierName, carrierId); // Collapse the notification bar mContext.sendBroadcast(new Intent(Intent.ACTION_CLOSE_SYSTEM_DIALOGS)); break; + case NOTIFICATION_USER_DISALLOWED_CARRIER_INTENT_ACTION: + handleUserDisallowCarrierExemptionAction(carrierName, carrierId); + break; case NOTIFICATION_USER_DISMISSED_INTENT_ACTION: handleUserDismissAction(); return; // no need to cancel a dismissed notification, return. @@ -303,7 +297,6 @@ public class WifiCarrierInfoManager { mIntentFilter.addAction(NOTIFICATION_USER_DISMISSED_INTENT_ACTION); mIntentFilter.addAction(NOTIFICATION_USER_ALLOWED_CARRIER_INTENT_ACTION); mIntentFilter.addAction(NOTIFICATION_USER_DISALLOWED_CARRIER_INTENT_ACTION); - mIntentFilter.addAction(NOTIFICATION_USER_CLICKED_INTENT_ACTION); mContext.registerReceiver(mBroadcastReceiver, mIntentFilter, null, handler); configStore.registerStoreData(wifiInjector.makeImsiProtectionExemptionStoreData( @@ -1455,9 +1448,6 @@ public class WifiCarrierInfoManager { .setStyle(new Notification.BigTextStyle() .bigText(mResources.getString( R.string.wifi_suggestion_imsi_privacy_content))) - .setContentIntent(getPrivateBroadcast(NOTIFICATION_USER_CLICKED_INTENT_ACTION, - Pair.create(EXTRA_CARRIER_NAME, carrierName), - Pair.create(EXTRA_CARRIER_ID, carrierId))) .setDeleteIntent(getPrivateBroadcast(NOTIFICATION_USER_DISMISSED_INTENT_ACTION, Pair.create(EXTRA_CARRIER_NAME, carrierName), Pair.create(EXTRA_CARRIER_ID, carrierId))) diff --git a/service/res/values/strings.xml b/service/res/values/strings.xml index ae3f1977b..441ecca15 100644 --- a/service/res/values/strings.xml +++ b/service/res/values/strings.xml @@ -49,11 +49,11 @@ No thanks - Connect to %s Wi\u2011Fi? + %s wants to auto\u2011connect - These networks receive a SIM ID that can be used to track device location + These networks receive a unique ID that can be used to track device location - Connect + Connect anyway Don\u0027t connect diff --git a/tests/wifitests/src/com/android/server/wifi/WifiCarrierInfoManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiCarrierInfoManagerTest.java index 750e8d46b..2c1c2ec91 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiCarrierInfoManagerTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiCarrierInfoManagerTest.java @@ -19,7 +19,6 @@ package com.android.server.wifi; import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn; import static com.android.dx.mockito.inline.extended.ExtendedMockito.when; import static com.android.server.wifi.WifiCarrierInfoManager.NOTIFICATION_USER_ALLOWED_CARRIER_INTENT_ACTION; -import static com.android.server.wifi.WifiCarrierInfoManager.NOTIFICATION_USER_CLICKED_INTENT_ACTION; import static com.android.server.wifi.WifiCarrierInfoManager.NOTIFICATION_USER_DISALLOWED_CARRIER_INTENT_ACTION; import static com.android.server.wifi.WifiCarrierInfoManager.NOTIFICATION_USER_DISMISSED_INTENT_ACTION; @@ -167,7 +166,6 @@ public class WifiCarrierInfoManagerTest extends WifiBaseTest { when(mNotificationBuilder.setTicker(any())).thenReturn(mNotificationBuilder); when(mNotificationBuilder.setContentTitle(any())).thenReturn(mNotificationBuilder); when(mNotificationBuilder.setStyle(any())).thenReturn(mNotificationBuilder); - when(mNotificationBuilder.setContentIntent(any())).thenReturn(mNotificationBuilder); when(mNotificationBuilder.setDeleteIntent(any())).thenReturn(mNotificationBuilder); when(mNotificationBuilder.setShowWhen(anyBoolean())).thenReturn(mNotificationBuilder); when(mNotificationBuilder.setLocalOnly(anyBoolean())).thenReturn(mNotificationBuilder); @@ -1540,13 +1538,26 @@ public class WifiCarrierInfoManagerTest extends WifiBaseTest { verify(mNotificationManger).cancel(SystemMessage.NOTE_NETWORK_SUGGESTION_AVAILABLE); verify(mWifiMetrics).addUserApprovalCarrierUiReaction( WifiCarrierInfoManager.ACTION_USER_ALLOWED_CARRIER, false); + validateUserApprovalDialog(CARRIER_NAME); + + // Simulate user clicking on allow in the dialog. + ArgumentCaptor clickListenerCaptor = + ArgumentCaptor.forClass(DialogInterface.OnClickListener.class); + verify(mAlertDialogBuilder, atLeastOnce()).setPositiveButton( + any(), clickListenerCaptor.capture()); + assertNotNull(clickListenerCaptor.getValue()); + clickListenerCaptor.getValue().onClick(mAlertDialog, 0); + mLooper.dispatchAll(); + ArgumentCaptor intentCaptor = ArgumentCaptor.forClass(Intent.class); + verify(mContext).sendBroadcast(intentCaptor.capture()); + assertEquals(Intent.ACTION_CLOSE_SYSTEM_DIALOGS, intentCaptor.getValue().getAction()); verify(mWifiConfigManager).saveToStore(true); assertTrue(mImsiDataSource.hasNewDataToSerialize()); assertTrue(mWifiCarrierInfoManager .hasUserApprovedImsiPrivacyExemptionForCarrier(DATA_CARRIER_ID)); verify(mListener).onUserAllowed(DATA_CARRIER_ID); verify(mWifiMetrics).addUserApprovalCarrierUiReaction( - WifiCarrierInfoManager.ACTION_USER_ALLOWED_CARRIER, false); + WifiCarrierInfoManager.ACTION_USER_ALLOWED_CARRIER, true); } @Test @@ -1596,7 +1607,7 @@ public class WifiCarrierInfoManagerTest extends WifiBaseTest { validateImsiProtectionNotification(CARRIER_NAME); //Simulate user dismissal the notification sendBroadcastForUserActionOnImsi(NOTIFICATION_USER_DISMISSED_INTENT_ACTION, - CARRIER_NAME, DATA_CARRIER_ID); + CARRIER_NAME, DATA_SUBID); verify(mWifiMetrics).addUserApprovalCarrierUiReaction( WifiCarrierInfoManager.ACTION_USER_DISMISS, false); reset(mNotificationManger); @@ -1607,7 +1618,7 @@ public class WifiCarrierInfoManagerTest extends WifiBaseTest { // As there is notification is active, should not send notification again. sendBroadcastForUserActionOnImsi(NOTIFICATION_USER_DISMISSED_INTENT_ACTION, - CARRIER_NAME, DATA_CARRIER_ID); + CARRIER_NAME, DATA_SUBID); verifyNoMoreInteractions(mNotificationManger); verify(mWifiConfigManager, never()).saveToStore(true); assertFalse(mImsiDataSource.hasNewDataToSerialize()); @@ -1631,10 +1642,12 @@ public class WifiCarrierInfoManagerTest extends WifiBaseTest { mWifiCarrierInfoManager.sendImsiProtectionExemptionNotificationIfRequired(DATA_CARRIER_ID); validateImsiProtectionNotification(CARRIER_NAME); - // Simulate user clicking on the notification. - sendBroadcastForUserActionOnImsi(NOTIFICATION_USER_CLICKED_INTENT_ACTION, - CARRIER_NAME, DATA_CARRIER_ID); + // Simulate user clicking on allow in the notification. + sendBroadcastForUserActionOnImsi(NOTIFICATION_USER_ALLOWED_CARRIER_INTENT_ACTION, + CARRIER_NAME, DATA_SUBID); verify(mNotificationManger).cancel(SystemMessage.NOTE_NETWORK_SUGGESTION_AVAILABLE); + verify(mWifiMetrics).addUserApprovalCarrierUiReaction( + WifiCarrierInfoManager.ACTION_USER_ALLOWED_CARRIER, false); validateUserApprovalDialog(CARRIER_NAME); // Simulate user clicking on disallow in the dialog. @@ -1672,10 +1685,11 @@ public class WifiCarrierInfoManagerTest extends WifiBaseTest { mWifiCarrierInfoManager.sendImsiProtectionExemptionNotificationIfRequired(DATA_CARRIER_ID); validateImsiProtectionNotification(CARRIER_NAME); - // Simulate user clicking on the notification. - sendBroadcastForUserActionOnImsi(NOTIFICATION_USER_CLICKED_INTENT_ACTION, - CARRIER_NAME, DATA_CARRIER_ID); + sendBroadcastForUserActionOnImsi(NOTIFICATION_USER_ALLOWED_CARRIER_INTENT_ACTION, + CARRIER_NAME, DATA_SUBID); verify(mNotificationManger).cancel(SystemMessage.NOTE_NETWORK_SUGGESTION_AVAILABLE); + verify(mWifiMetrics).addUserApprovalCarrierUiReaction( + WifiCarrierInfoManager.ACTION_USER_ALLOWED_CARRIER, false); validateUserApprovalDialog(CARRIER_NAME); // Simulate user clicking on dismissal in the dialog. @@ -1703,45 +1717,6 @@ public class WifiCarrierInfoManagerTest extends WifiBaseTest { WifiCarrierInfoManager.ACTION_USER_DISMISS, true); } - @Test - public void testSendImsiProtectionExemptionDialogWithUserAllowed() { - // Setup carrier without IMSI privacy protection - when(mCarrierConfigManager.getConfigForSubId(DATA_SUBID)) - .thenReturn(generateTestCarrierConfig(false)); - ArgumentCaptor receiver = - ArgumentCaptor.forClass(BroadcastReceiver.class); - verify(mContext).registerReceiver(receiver.capture(), any(IntentFilter.class)); - - receiver.getValue().onReceive(mContext, - new Intent(CarrierConfigManager.ACTION_CARRIER_CONFIG_CHANGED)); - assertFalse(mWifiCarrierInfoManager.requiresImsiEncryption(DATA_SUBID)); - - mWifiCarrierInfoManager.sendImsiProtectionExemptionNotificationIfRequired(DATA_CARRIER_ID); - validateImsiProtectionNotification(CARRIER_NAME); - // Simulate user clicking on the notification. - sendBroadcastForUserActionOnImsi(NOTIFICATION_USER_CLICKED_INTENT_ACTION, - CARRIER_NAME, DATA_CARRIER_ID); - verify(mNotificationManger).cancel(SystemMessage.NOTE_NETWORK_SUGGESTION_AVAILABLE); - validateUserApprovalDialog(CARRIER_NAME); - - // Simulate user clicking on allow in the dialog. - ArgumentCaptor clickListenerCaptor = - ArgumentCaptor.forClass(DialogInterface.OnClickListener.class); - verify(mAlertDialogBuilder, atLeastOnce()).setPositiveButton( - any(), clickListenerCaptor.capture()); - assertNotNull(clickListenerCaptor.getValue()); - clickListenerCaptor.getValue().onClick(mAlertDialog, 0); - mLooper.dispatchAll(); - ArgumentCaptor intentCaptor = ArgumentCaptor.forClass(Intent.class); - verify(mContext).sendBroadcast(intentCaptor.capture()); - assertEquals(Intent.ACTION_CLOSE_SYSTEM_DIALOGS, intentCaptor.getValue().getAction()); - verify(mWifiConfigManager).saveToStore(true); - assertTrue(mImsiDataSource.hasNewDataToSerialize()); - verify(mListener).onUserAllowed(DATA_CARRIER_ID); - verify(mWifiMetrics).addUserApprovalCarrierUiReaction( - WifiCarrierInfoManager.ACTION_USER_ALLOWED_CARRIER, true); - } - @Test public void testUserDataStoreIsNotLoadedNotificationWillNotBeSent() { // reset data source to unloaded state. -- cgit v1.2.3