From f13406f3a0e8e8260778e091ab1f6075d8af84b2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mat=C3=ADas=20Hern=C3=A1ndez?= <matiashe@google.com>
Date: Wed, 15 Feb 2023 15:07:03 +0100
Subject: [PATCH] Disallow setting invalid importance when updating a
 NotificationChannel

The importance value was previously checked on the create path only.

Test: atest PreferencesHelperTest
Fixes: 260079555
Change-Id: If903eee3446fed4f3aeaf53de16e0b31434ffc51
---
 .../notification/PreferencesHelper.java       |  9 ++--
 .../notification/PreferencesHelperTest.java   | 42 +++++++++++++++++++
 2 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/services/core/java/com/android/server/notification/PreferencesHelper.java b/services/core/java/com/android/server/notification/PreferencesHelper.java
index 69ea55969b03f..c63cdddc85608 100644
--- a/services/core/java/com/android/server/notification/PreferencesHelper.java
+++ b/services/core/java/com/android/server/notification/PreferencesHelper.java
@@ -22,6 +22,7 @@ import static android.app.NotificationChannel.USER_LOCKED_IMPORTANCE;
 import static android.app.NotificationManager.BUBBLE_PREFERENCE_ALL;
 import static android.app.NotificationManager.BUBBLE_PREFERENCE_NONE;
 import static android.app.NotificationManager.IMPORTANCE_DEFAULT;
+import static android.app.NotificationManager.IMPORTANCE_MAX;
 import static android.app.NotificationManager.IMPORTANCE_NONE;
 import static android.app.NotificationManager.IMPORTANCE_UNSPECIFIED;
 import static android.util.StatsLog.ANNOTATION_ID_IS_UID;
@@ -905,6 +906,9 @@ public class PreferencesHelper implements RankingConfig {
         Objects.requireNonNull(channel);
         Objects.requireNonNull(channel.getId());
         Preconditions.checkArgument(!TextUtils.isEmpty(channel.getName()));
+        Preconditions.checkArgument(channel.getImportance() >= IMPORTANCE_NONE
+                && channel.getImportance() <= IMPORTANCE_MAX, "Invalid importance level");
+
         boolean needsPolicyFileChange = false, wasUndeleted = false, needsDndChange = false;
         synchronized (mPackagePreferences) {
             PackagePreferences r = getOrCreatePackagePreferencesLocked(pkg, uid);
@@ -993,11 +997,6 @@ public class PreferencesHelper implements RankingConfig {
 
                 needsPolicyFileChange = true;
 
-                if (channel.getImportance() < IMPORTANCE_NONE
-                        || channel.getImportance() > NotificationManager.IMPORTANCE_MAX) {
-                    throw new IllegalArgumentException("Invalid importance level");
-                }
-
                 // Reset fields that apps aren't allowed to set.
                 if (fromTargetApp && !hasDndAccess) {
                     channel.setBypassDnd(r.priority == Notification.PRIORITY_MAX);
diff --git a/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java
index f0f9204c4e4e0..1ecd4a1ffd7e0 100644
--- a/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java
@@ -57,6 +57,7 @@ import static junit.framework.Assert.fail;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
@@ -1576,6 +1577,47 @@ public class PreferencesHelperTest extends UiServiceTestCase {
                 new NotificationChannel("bananas", "bananas", IMPORTANCE_MAX), true, false));
     }
 
+    @Test
+    public void testUpdateChannel_downgradeImportance() {
+        mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1,
+                new NotificationChannel("bananas", "bananas", IMPORTANCE_DEFAULT),
+                true, false);
+
+        assertTrue(mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1,
+                new NotificationChannel("bananas", "bananas", IMPORTANCE_LOW), true, false));
+    }
+
+    @Test
+    public void testUpdateChannel_upgradeImportance_ignored() {
+        mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1,
+                new NotificationChannel("bananas", "bananas", IMPORTANCE_DEFAULT),
+                true, false);
+
+        assertFalse(mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1,
+                new NotificationChannel("bananas", "bananas", IMPORTANCE_MAX), true, false));
+    }
+
+    @Test
+    public void testUpdateChannel_badImportance() {
+        mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1,
+                new NotificationChannel("bananas", "bananas", IMPORTANCE_DEFAULT),
+                true, false);
+
+        assertThrows(IllegalArgumentException.class,
+                () -> mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1,
+                        new NotificationChannel("bananas", "bananas", IMPORTANCE_NONE - 1), true,
+                        false));
+
+        assertThrows(IllegalArgumentException.class,
+                () -> mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1,
+                        new NotificationChannel("bananas", "bananas", IMPORTANCE_UNSPECIFIED), true,
+                        false));
+
+        assertThrows(IllegalArgumentException.class,
+                () -> mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1,
+                        new NotificationChannel("bananas", "bananas", IMPORTANCE_MAX + 1), true,
+                        false));
+    }
 
     @Test
     public void testUpdate() throws Exception {