From b7c12884d382c17ed8a928c430dc9f648e082ba0 Mon Sep 17 00:00:00 2001 From: Chun-Wei Wang Date: Thu, 20 Jul 2023 07:58:43 +0000 Subject: [PATCH] Fix DND display error when caller is DO/PO DPM calls should check uid instead of the calling uid. It is 'pkg' that we want to check access for, not the caller which is especially true for Settings app (caller) which needs to display DND details for each app (package in query). Bug: 292163859 Bug: 291863956 Test: atest FrameworksUiServicesTests Test: atest FrameworksServicesTests:DevicePolicyManagerTest Test: atest CtsAdminTestCases:DevicePolicyManagerTest Test: atest NotificationManagerServiceTest Change-Id: I15b84dc48cf5225749998c0e25ab8f599235c283 --- .../NotificationManagerService.java | 14 +- .../NotificationManagerServiceTest.java | 129 ++++++++++++++++++ .../TestableNotificationManagerService.java | 12 ++ 3 files changed, 151 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 6f0b0bcf0112c..464778d7b8edc 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -3281,6 +3281,11 @@ public class NotificationManagerService extends SystemService { return new MultiRateLimiter.Builder(getContext()).addRateLimits(TOAST_RATE_LIMITS).build(); } + protected int checkComponentPermission(String permission, int uid, int owningUid, + boolean exported) { + return ActivityManager.checkComponentPermission(permission, uid, owningUid, exported); + } + @VisibleForTesting final IBinder mService = new INotificationManager.Stub() { // Toasts @@ -5238,10 +5243,11 @@ public class NotificationManagerService extends SystemService { } private boolean checkPolicyAccess(String pkg) { + final int uid; try { - int uid = getContext().getPackageManager().getPackageUidAsUser(pkg, + uid = getContext().getPackageManager().getPackageUidAsUser(pkg, UserHandle.getCallingUserId()); - if (PackageManager.PERMISSION_GRANTED == ActivityManager.checkComponentPermission( + if (PackageManager.PERMISSION_GRANTED == checkComponentPermission( android.Manifest.permission.MANAGE_NOTIFICATIONS, uid, -1, true)) { return true; @@ -5252,8 +5258,8 @@ public class NotificationManagerService extends SystemService { //TODO(b/169395065) Figure out if this flow makes sense in Device Owner mode. return checkPackagePolicyAccess(pkg) || mListeners.isComponentEnabledForPackage(pkg) - || (mDpm != null && (mDpm.isActiveProfileOwner(Binder.getCallingUid()) - || mDpm.isActiveDeviceOwner(Binder.getCallingUid()))); + || (mDpm != null && (mDpm.isActiveProfileOwner(uid) + || mDpm.isActiveDeviceOwner(uid))); } @Override 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 f1e26d2c70836..0aa5d56007198 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -697,6 +697,11 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mTestFlagResolver.setFlagOverride(FSI_FORCE_DEMOTE, false); mTestFlagResolver.setFlagOverride(SHOW_STICKY_HUN_FOR_DENIED_FSI, false); + + var checker = mock(TestableNotificationManagerService.ComponentPermissionChecker.class); + mService.permissionChecker = checker; + when(checker.check(anyString(), anyInt(), anyInt(), anyBoolean())) + .thenReturn(PackageManager.PERMISSION_DENIED); } @After @@ -12168,6 +12173,130 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { inOrder.verifyNoMoreInteractions(); } + @Test + public void isNotificationPolicyAccessGranted_invalidPackage() throws Exception { + final String notReal = "NOT REAL"; + final var checker = mService.permissionChecker; + + when(mPackageManagerClient.getPackageUidAsUser(eq(notReal), anyInt())).thenThrow( + PackageManager.NameNotFoundException.class); + + assertThat(mBinderService.isNotificationPolicyAccessGranted(notReal)).isFalse(); + verify(mPackageManagerClient).getPackageUidAsUser(eq(notReal), anyInt()); + verify(checker, never()).check(any(), anyInt(), anyInt(), anyBoolean()); + verify(mConditionProviders, never()).isPackageOrComponentAllowed(eq(notReal), anyInt()); + verify(mListeners, never()).isComponentEnabledForPackage(any()); + verify(mDevicePolicyManager, never()).isActiveDeviceOwner(anyInt()); + } + + @Test + public void isNotificationPolicyAccessGranted_hasPermission() throws Exception { + final String packageName = "target"; + final int uid = 123; + final var checker = mService.permissionChecker; + + when(mPackageManagerClient.getPackageUidAsUser(eq(packageName), anyInt())).thenReturn(uid); + when(checker.check(android.Manifest.permission.MANAGE_NOTIFICATIONS, uid, -1, true)) + .thenReturn(PackageManager.PERMISSION_GRANTED); + + assertThat(mBinderService.isNotificationPolicyAccessGranted(packageName)).isTrue(); + verify(mPackageManagerClient).getPackageUidAsUser(eq(packageName), anyInt()); + verify(checker).check(android.Manifest.permission.MANAGE_NOTIFICATIONS, uid, -1, true); + verify(mConditionProviders, never()).isPackageOrComponentAllowed(eq(packageName), anyInt()); + verify(mListeners, never()).isComponentEnabledForPackage(any()); + verify(mDevicePolicyManager, never()).isActiveDeviceOwner(anyInt()); + } + + @Test + public void isNotificationPolicyAccessGranted_isPackageAllowed() throws Exception { + final String packageName = "target"; + final int uid = 123; + final var checker = mService.permissionChecker; + + when(mPackageManagerClient.getPackageUidAsUser(eq(packageName), anyInt())).thenReturn(uid); + when(mConditionProviders.isPackageOrComponentAllowed(eq(packageName), anyInt())) + .thenReturn(true); + + assertThat(mBinderService.isNotificationPolicyAccessGranted(packageName)).isTrue(); + verify(mPackageManagerClient).getPackageUidAsUser(eq(packageName), anyInt()); + verify(checker).check(android.Manifest.permission.MANAGE_NOTIFICATIONS, uid, -1, true); + verify(mConditionProviders).isPackageOrComponentAllowed(eq(packageName), anyInt()); + verify(mListeners, never()).isComponentEnabledForPackage(any()); + verify(mDevicePolicyManager, never()).isActiveDeviceOwner(anyInt()); + } + + @Test + public void isNotificationPolicyAccessGranted_isComponentEnabled() throws Exception { + final String packageName = "target"; + final int uid = 123; + final var checker = mService.permissionChecker; + + when(mPackageManagerClient.getPackageUidAsUser(eq(packageName), anyInt())).thenReturn(uid); + when(mListeners.isComponentEnabledForPackage(packageName)).thenReturn(true); + + assertThat(mBinderService.isNotificationPolicyAccessGranted(packageName)).isTrue(); + verify(mPackageManagerClient).getPackageUidAsUser(eq(packageName), anyInt()); + verify(checker).check(android.Manifest.permission.MANAGE_NOTIFICATIONS, uid, -1, true); + verify(mConditionProviders).isPackageOrComponentAllowed(eq(packageName), anyInt()); + verify(mListeners).isComponentEnabledForPackage(packageName); + verify(mDevicePolicyManager, never()).isActiveDeviceOwner(anyInt()); + } + + @Test + public void isNotificationPolicyAccessGranted_isDeviceOwner() throws Exception { + final String packageName = "target"; + final int uid = 123; + final var checker = mService.permissionChecker; + + when(mPackageManagerClient.getPackageUidAsUser(eq(packageName), anyInt())).thenReturn(uid); + when(mDevicePolicyManager.isActiveDeviceOwner(uid)).thenReturn(true); + + assertThat(mBinderService.isNotificationPolicyAccessGranted(packageName)).isTrue(); + verify(mPackageManagerClient).getPackageUidAsUser(eq(packageName), anyInt()); + verify(checker).check(android.Manifest.permission.MANAGE_NOTIFICATIONS, uid, -1, true); + verify(mConditionProviders).isPackageOrComponentAllowed(eq(packageName), anyInt()); + verify(mListeners).isComponentEnabledForPackage(packageName); + verify(mDevicePolicyManager).isActiveDeviceOwner(uid); + } + + /** + * b/292163859 + */ + @Test + public void isNotificationPolicyAccessGranted_callerIsDeviceOwner() throws Exception { + final String packageName = "target"; + final int uid = 123; + final int callingUid = Binder.getCallingUid(); + final var checker = mService.permissionChecker; + + when(mPackageManagerClient.getPackageUidAsUser(eq(packageName), anyInt())).thenReturn(uid); + when(mDevicePolicyManager.isActiveDeviceOwner(callingUid)).thenReturn(true); + + assertThat(mBinderService.isNotificationPolicyAccessGranted(packageName)).isFalse(); + verify(mPackageManagerClient).getPackageUidAsUser(eq(packageName), anyInt()); + verify(checker).check(android.Manifest.permission.MANAGE_NOTIFICATIONS, uid, -1, true); + verify(mConditionProviders).isPackageOrComponentAllowed(eq(packageName), anyInt()); + verify(mListeners).isComponentEnabledForPackage(packageName); + verify(mDevicePolicyManager).isActiveDeviceOwner(uid); + verify(mDevicePolicyManager, never()).isActiveDeviceOwner(callingUid); + } + + @Test + public void isNotificationPolicyAccessGranted_notGranted() throws Exception { + final String packageName = "target"; + final int uid = 123; + final var checker = mService.permissionChecker; + + when(mPackageManagerClient.getPackageUidAsUser(eq(packageName), anyInt())).thenReturn(uid); + + assertThat(mBinderService.isNotificationPolicyAccessGranted(packageName)).isFalse(); + verify(mPackageManagerClient).getPackageUidAsUser(eq(packageName), anyInt()); + verify(checker).check(android.Manifest.permission.MANAGE_NOTIFICATIONS, uid, -1, true); + verify(mConditionProviders).isPackageOrComponentAllowed(eq(packageName), anyInt()); + verify(mListeners).isComponentEnabledForPackage(packageName); + verify(mDevicePolicyManager).isActiveDeviceOwner(uid); + } + private static T parcelAndUnparcel(T source, Parcelable.Creator creator) { Parcel parcel = Parcel.obtain(); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/TestableNotificationManagerService.java b/services/tests/uiservicestests/src/com/android/server/notification/TestableNotificationManagerService.java index 9f4eee7e332f1..27e8f3664a656 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/TestableNotificationManagerService.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/TestableNotificationManagerService.java @@ -43,6 +43,8 @@ public class TestableNotificationManagerService extends NotificationManagerServi @Nullable Boolean mIsVisibleToListenerReturnValue = null; + ComponentPermissionChecker permissionChecker; + TestableNotificationManagerService(Context context, NotificationRecordLogger logger, InstanceIdSequence notificationInstanceIdSequence) { super(context, logger, notificationInstanceIdSequence); @@ -150,6 +152,12 @@ public class TestableNotificationManagerService extends NotificationManagerServi return super.isVisibleToListener(sbn, notificationType, listener); } + @Override + protected int checkComponentPermission(String permission, int uid, int owningUid, + boolean exported) { + return permissionChecker.check(permission, uid, owningUid, exported); + } + public class StrongAuthTrackerFake extends NotificationManagerService.StrongAuthTracker { private int mGetStrongAuthForUserReturnValue = 0; StrongAuthTrackerFake(Context context) { @@ -165,4 +173,8 @@ public class TestableNotificationManagerService extends NotificationManagerServi return mGetStrongAuthForUserReturnValue; } } + + public interface ComponentPermissionChecker { + int check(String permission, int uid, int owningUid, boolean exported); + } }