From 876d03fc2f564b70a4d52445454236935936065a Mon Sep 17 00:00:00 2001 From: Daniel Chapin Date: Fri, 29 Oct 2021 20:44:51 +0000 Subject: [PATCH] Revert "Enforce that callers to matchesCallFilter have either li..." Revert "Test permissions for matchesCallFilter in NotificationMa..." Revert submission 16077951-yl-mcfperm Reason for revert: Droidfood Blocking Bug: b/204523343 Reverted Changes: I8e7c8b699:Enforce that callers to matchesCallFilter have eit... I8a05ffa89:Test permissions for matchesCallFilter in Notifica... Change-Id: Ib5c6814de37cae4260fb35b50a1bfc95143e1b2d --- .../java/android/app/NotificationManager.java | 3 -- .../NotificationManagerService.java | 38 -------------- .../NotificationListenersTest.java | 21 ++------ .../NotificationManagerServiceTest.java | 49 ------------------- 4 files changed, 3 insertions(+), 108 deletions(-) diff --git a/core/java/android/app/NotificationManager.java b/core/java/android/app/NotificationManager.java index 7dbd8148657aa..9be4adcbec753 100644 --- a/core/java/android/app/NotificationManager.java +++ b/core/java/android/app/NotificationManager.java @@ -2579,9 +2579,6 @@ public class NotificationManager { * for more information. *

*

- * Callers of this method must have notification listener access or permission to read contacts. - *

- *

* NOTE: This method calls into Contacts, which may take some time, and should not be called * on the main thread. *

diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index e5df3e7d83e2d..f701c2abe0c64 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -5083,27 +5083,6 @@ public class NotificationManagerService extends SystemService { @Override public boolean matchesCallFilter(Bundle extras) { - // Because matchesCallFilter may use contact data to filter calls, the callers of this - // method need to either have notification listener access or permission to read - // contacts. - boolean listenerAccess = false; - try { - String[] pkgNames = mPackageManager.getPackagesForUid(Binder.getCallingUid()); - for (int i = 0; i < pkgNames.length; i++) { - // in most cases there should only be one package here - listenerAccess |= mListeners.hasAllowedListener(pkgNames[i], - Binder.getCallingUserHandle().getIdentifier()); - } - } catch (RemoteException e) { - } finally { - if (!listenerAccess) { - getContext().enforceCallingPermission( - Manifest.permission.READ_CONTACTS, - "matchesCallFilter requires listener permissions or " - + "contacts read access"); - } - } - return mZenModeHelper.matchesCallFilter( Binder.getCallingUserHandle(), extras, @@ -10975,23 +10954,6 @@ public class NotificationManagerService extends SystemService { } return false; } - - // Returns whether there is a component with listener access granted that is associated - // with the given package name / user ID. - boolean hasAllowedListener(String packageName, int userId) { - if (packageName == null) { - return false; - } - - // Loop through allowed components to compare package names - List allowedComponents = getAllowedComponents(userId); - for (int i = 0; i < allowedComponents.size(); i++) { - if (allowedComponents.get(i).getPackageName().equals(packageName)) { - return true; - } - } - return false; - } } // TODO (b/194833441): remove when we've fully migrated to a permission diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationListenersTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationListenersTest.java index d4420bd86fc14..50ebffc31035f 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationListenersTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationListenersTest.java @@ -24,9 +24,8 @@ import static com.android.server.notification.NotificationManagerService.Notific import static com.google.common.truth.Truth.assertThat; -import static junit.framework.Assert.assertFalse; -import static junit.framework.Assert.assertTrue; - +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; @@ -42,6 +41,7 @@ import android.service.notification.NotificationListenerFilter; import android.service.notification.NotificationListenerService; import android.util.ArraySet; import android.util.Pair; +import android.util.Slog; import android.util.TypedXmlPullParser; import android.util.TypedXmlSerializer; import android.util.Xml; @@ -355,19 +355,4 @@ public class NotificationListenersTest extends UiServiceTestCase { .getDisallowedPackages()).isEmpty(); } - @Test - public void testHasAllowedListener() { - final int uid1 = 1, uid2 = 2; - // enable mCn1 but not mCn2 for uid1 - mListeners.addApprovedList(mCn1.flattenToString(), uid1, true); - - // verify that: - // the package for mCn1 has an allowed listener for uid1 and not uid2 - assertTrue(mListeners.hasAllowedListener(mCn1.getPackageName(), uid1)); - assertFalse(mListeners.hasAllowedListener(mCn1.getPackageName(), uid2)); - - // and that mCn2 has no allowed listeners for either user id - assertFalse(mListeners.hasAllowedListener(mCn2.getPackageName(), uid1)); - assertFalse(mListeners.hasAllowedListener(mCn2.getPackageName(), uid2)); - } } diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index e4f889bfdd2cb..5694e599edc38 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -8356,53 +8356,4 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { verify(mPermissionHelper, never()).hasPermission(anyInt()); verify(mPreferencesHelper, never()).getNotificationChannelsBypassingDnd(PKG, mUid); } - - @Test - public void testMatchesCallFilter_noPermissionShouldThrow() throws Exception { - // make sure a caller without listener access or read_contacts permission can't call - // matchesCallFilter. - when(mListeners.hasAllowedListener(anyString(), anyInt())).thenReturn(false); - doThrow(new SecurityException()).when(mContext).enforceCallingPermission( - eq("android.permission.READ_CONTACTS"), anyString()); - - try { - // shouldn't matter what we're passing in, if we get past this line fail immediately - ((INotificationManager) mService.mService).matchesCallFilter(null); - fail("call to matchesCallFilter with no permissions should fail"); - } catch (SecurityException e) { - // pass - } - } - - @Test - public void testMatchesCallFilter_hasListenerPermission() throws Exception { - // make sure a caller with only listener access and not read_contacts permission can call - // matchesCallFilter. - when(mListeners.hasAllowedListener(anyString(), anyInt())).thenReturn(true); - doThrow(new SecurityException()).when(mContext).enforceCallingPermission( - eq("android.permission.READ_CONTACTS"), anyString()); - - try { - ((INotificationManager) mService.mService).matchesCallFilter(null); - // pass, this is not a functionality test - } catch (SecurityException e) { - fail("call to matchesCallFilter with listener permissions should work"); - } - } - - @Test - public void testMatchesCallFilter_hasContactsPermission() throws Exception { - // make sure a caller with only read_contacts permission and not listener access can call - // matchesCallFilter. - when(mListeners.hasAllowedListener(anyString(), anyInt())).thenReturn(false); - doNothing().when(mContext).enforceCallingPermission( - eq("android.permission.READ_CONTACTS"), anyString()); - - try { - ((INotificationManager) mService.mService).matchesCallFilter(null); - // pass, this is not a functionality test - } catch (SecurityException e) { - fail("call to matchesCallFilter with listener permissions should work"); - } - } }