Allow conversation channels to inherit canBypassDnd from their parent.

This change passes through a more correct value of fromTargetApp to PreferencesHelper.createNotificationChannel. The reason this fixes the inheritance of canBypassDnd for conversation channels is that that field cannot be modified by apps; but when the system creates a conversation channel, it should be able to inherit all its properties from its parent.

Bug: 253848209
Test: atest NotificationManagerServiceTest, atest NotificationManagerTest (CTS)
Change-Id: I407a239376afac18afd6c2cf2471a0e5399253a3

3 files changed, 64 insertions, 15 deletions

diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index c2df904d07ce..8a7fd82e6123 100755
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -3796,13 +3796,13 @@ public class NotificationManagerService extends SystemService {
         }
 
         private void createNotificationChannelsImpl(String pkg, int uid,
-                ParceledListSlice channelsList) {
-            createNotificationChannelsImpl(pkg, uid, channelsList,
+                ParceledListSlice channelsList, boolean fromTargetApp) {
+            createNotificationChannelsImpl(pkg, uid, channelsList, fromTargetApp,
                     ActivityTaskManager.INVALID_TASK_ID);
         }
 
         private void createNotificationChannelsImpl(String pkg, int uid,
-                ParceledListSlice channelsList, int startingTaskId) {
+                ParceledListSlice channelsList, boolean fromTargetApp, int startingTaskId) {
             List<NotificationChannel> channels = channelsList.getList();
             final int channelsSize = channels.size();
             ParceledListSlice<NotificationChannel> oldChannels =
@@ -3814,7 +3814,7 @@ public class NotificationManagerService extends SystemService {
                 final NotificationChannel channel = channels.get(i);
                 Objects.requireNonNull(channel, "channel in list is null");
                 needsPolicyFileChange = mPreferencesHelper.createNotificationChannel(pkg, uid,
-                        channel, true /* fromTargetApp */,
+                        channel, fromTargetApp,
                         mConditionProviders.isPackageOrComponentAllowed(
                                 pkg, UserHandle.getUserId(uid)));
                 if (needsPolicyFileChange) {
@@ -3850,6 +3850,7 @@ public class NotificationManagerService extends SystemService {
         @Override
         public void createNotificationChannels(String pkg, ParceledListSlice channelsList) {
             checkCallerIsSystemOrSameApp(pkg);
+            boolean fromTargetApp = !isCallerSystemOrPhone();  // if not system, it's from the app
             int taskId = ActivityTaskManager.INVALID_TASK_ID;
             try {
                 int uid = mPackageManager.getPackageUid(pkg, 0,
@@ -3858,14 +3859,15 @@ public class NotificationManagerService extends SystemService {
             } catch (RemoteException e) {
                 // Do nothing
             }
-            createNotificationChannelsImpl(pkg, Binder.getCallingUid(), channelsList, taskId);
+            createNotificationChannelsImpl(pkg, Binder.getCallingUid(), channelsList, fromTargetApp,
+                    taskId);
         }
 
         @Override
         public void createNotificationChannelsForPackage(String pkg, int uid,
                 ParceledListSlice channelsList) {
             enforceSystemOrSystemUI("only system can call this");
-            createNotificationChannelsImpl(pkg, uid, channelsList);
+            createNotificationChannelsImpl(pkg, uid, channelsList, false /* fromTargetApp */);
         }
 
         @Override
@@ -3880,7 +3882,8 @@ public class NotificationManagerService extends SystemService {
                     CONVERSATION_CHANNEL_ID_FORMAT, parentId, conversationId));
             conversationChannel.setConversationId(parentId, conversationId);
             createNotificationChannelsImpl(
-                    pkg, uid, new ParceledListSlice(Arrays.asList(conversationChannel)));
+                    pkg, uid, new ParceledListSlice(Arrays.asList(conversationChannel)),
+                    false /* fromTargetApp */);
             mRankingHandler.requestSort();
             handleSavePolicyFile();
         }
diff --git a/services/core/java/com/android/server/notification/PreferencesHelper.java b/services/core/java/com/android/server/notification/PreferencesHelper.java
index d8aa469bcd81..97911581c59c 100644
--- a/services/core/java/com/android/server/notification/PreferencesHelper.java
+++ b/services/core/java/com/android/server/notification/PreferencesHelper.java
@@ -916,7 +916,7 @@ public class PreferencesHelper implements RankingConfig {
                 throw new IllegalArgumentException("Reserved id");
             }
             NotificationChannel existing = r.channels.get(channel.getId());
-            if (existing != null && fromTargetApp) {
+            if (existing != null) {
                 // Actually modifying an existing channel - keep most of the existing settings
                 if (existing.isDeleted()) {
                     // The existing channel was deleted - undelete it.
@@ -1002,9 +1002,7 @@ public class PreferencesHelper implements RankingConfig {
                 }
                 if (fromTargetApp) {
                     channel.setLockscreenVisibility(r.visibility);
-                    channel.setAllowBubbles(existing != null
-                            ? existing.getAllowBubbles()
-                            : NotificationChannel.DEFAULT_ALLOW_BUBBLE);
+                    channel.setAllowBubbles(NotificationChannel.DEFAULT_ALLOW_BUBBLE);
                 }
                 clearLockedFieldsLocked(channel);
 
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 7df4b57b6cf8..6edd87c0c6cf 100755
--- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
@@ -1101,6 +1101,10 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
                 new NotificationChannel("id", "name", IMPORTANCE_HIGH);
         mBinderService.updateNotificationChannelForPackage(PKG, mUid, updatedChannel);
 
+        // pretend only this following part is called by the app (system permissions are required to
+        // update the notification channel on behalf of the user above)
+        mService.isSystemUid = false;
+
         // Recreating with a lower importance leaves channel unchanged.
         final NotificationChannel dupeChannel =
                 new NotificationChannel("id", "name", NotificationManager.IMPORTANCE_LOW);
@@ -1126,6 +1130,46 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
     }
 
     @Test
+    public void testCreateNotificationChannels_fromAppCannotSetFields() throws Exception {
+        // Confirm that when createNotificationChannels is called from the relevant app and not
+        // system, then it cannot set fields that can't be set by apps
+        mService.isSystemUid = false;
+
+        final NotificationChannel channel =
+                new NotificationChannel("id", "name", IMPORTANCE_DEFAULT);
+        channel.setBypassDnd(true);
+        channel.setAllowBubbles(true);
+
+        mBinderService.createNotificationChannels(PKG,
+                new ParceledListSlice(Arrays.asList(channel)));
+
+        final NotificationChannel createdChannel =
+                mBinderService.getNotificationChannel(PKG, mContext.getUserId(), PKG, "id");
+        assertFalse(createdChannel.canBypassDnd());
+        assertFalse(createdChannel.canBubble());
+    }
+
+    @Test
+    public void testCreateNotificationChannels_fromSystemCanSetFields() throws Exception {
+        // Confirm that when createNotificationChannels is called from system,
+        // then it can set fields that can't be set by apps
+        mService.isSystemUid = true;
+
+        final NotificationChannel channel =
+                new NotificationChannel("id", "name", IMPORTANCE_DEFAULT);
+        channel.setBypassDnd(true);
+        channel.setAllowBubbles(true);
+
+        mBinderService.createNotificationChannels(PKG,
+                new ParceledListSlice(Arrays.asList(channel)));
+
+        final NotificationChannel createdChannel =
+                mBinderService.getNotificationChannel(PKG, mContext.getUserId(), PKG, "id");
+        assertTrue(createdChannel.canBypassDnd());
+        assertTrue(createdChannel.canBubble());
+    }
+
+    @Test
     public void testBlockedNotifications_suspended() throws Exception {
         when(mPackageManager.isPackageSuspendedForUser(anyString(), anyInt())).thenReturn(true);
 
@@ -3088,6 +3132,8 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
 
     @Test
     public void testDeleteChannelGroupChecksForFgses() throws Exception {
+        // the setup for this test requires it to seem like it's coming from the app
+        mService.isSystemUid = false;
         when(mCompanionMgr.getAssociations(PKG, UserHandle.getUserId(mUid)))
                 .thenReturn(singletonList(mock(AssociationInfo.class)));
         CountDownLatch latch = new CountDownLatch(2);
@@ -3100,7 +3146,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
             ParceledListSlice<NotificationChannel> pls =
                     new ParceledListSlice(ImmutableList.of(notificationChannel));
             try {
-                mBinderService.createNotificationChannelsForPackage(PKG, mUid, pls);
+                mBinderService.createNotificationChannels(PKG, pls);
             } catch (RemoteException e) {
                 throw new RuntimeException(e);
             }
@@ -3119,8 +3165,10 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
                 ParceledListSlice<NotificationChannel> pls =
                         new ParceledListSlice(ImmutableList.of(notificationChannel));
                 try {
-                mBinderService.createNotificationChannelsForPackage(PKG, mUid, pls);
-                mBinderService.deleteNotificationChannelGroup(PKG, "group");
+                    // Because existing channels won't have their groups overwritten when the call
+                    // is from the app, this call won't take the channel out of the group
+                    mBinderService.createNotificationChannels(PKG, pls);
+                    mBinderService.deleteNotificationChannelGroup(PKG, "group");
                 } catch (RemoteException e) {
                     throw new RuntimeException(e);
                 }
@@ -8566,7 +8614,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
         assertEquals("friend", friendChannel.getConversationId());
         assertEquals(null, original.getConversationId());
         assertEquals(original.canShowBadge(), friendChannel.canShowBadge());
-        assertFalse(friendChannel.canBubble()); // can't be modified by app
+        assertEquals(original.canBubble(), friendChannel.canBubble()); // called by system
         assertFalse(original.getId().equals(friendChannel.getId()));
         assertNotNull(friendChannel.getId());
     }