From 92a9f5fc1d07d5ab8b89b02d09b73f355595449c Mon Sep 17 00:00:00 2001 From: Francois Gaffie Date: Wed, 24 Feb 2021 17:26:29 +0100 Subject: [PATCH 1/2] [Notification] Fix Notification channel Dnd bypass for multiusers -Permission granted for a given package using shell cmd (user system) are not sufficient due to a check of notification uid vs packagepreference uid. -If the packagepreference is deleted then restored, the channel preference are not taken into account. Bug: 178032672 Test: CTS/AudioManagerTest Signed-off-by: Francois Gaffie Change-Id: Ia26467f27b31bc048bdb141beac7d764fcb27f51 Merged-in: Ia26467f27b31bc048bdb141beac7d764fcb27f51 --- .../notification/PreferencesHelper.java | 44 +++++++++++-------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/services/core/java/com/android/server/notification/PreferencesHelper.java b/services/core/java/com/android/server/notification/PreferencesHelper.java index 7d1827d1b6e2d..0c1ba962323fc 100644 --- a/services/core/java/com/android/server/notification/PreferencesHelper.java +++ b/services/core/java/com/android/server/notification/PreferencesHelper.java @@ -32,6 +32,7 @@ import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.UserIdInt; +import android.app.ActivityManager; import android.app.AppOpsManager; import android.app.Notification; import android.app.NotificationChannel; @@ -177,6 +178,8 @@ public class PreferencesHelper implements RankingConfig { private Map> mOemLockedApps = new HashMap(); + private int mCurrentUserId = UserHandle.USER_NULL; + public PreferencesHelper(Context context, PackageManager pm, RankingHandler rankingHandler, ZenModeHelper zenHelper, NotificationChannelLogger notificationChannelLogger, AppOpsManager appOpsManager, @@ -191,7 +194,8 @@ public class PreferencesHelper implements RankingConfig { updateBadgingEnabled(); updateBubblesEnabled(); - syncChannelsBypassingDnd(mContext.getUserId()); + mCurrentUserId = ActivityManager.getCurrentUser(); + syncChannelsBypassingDnd(); } public void readXml(XmlPullParser parser, boolean forRestore, int userId) @@ -790,7 +794,7 @@ public class PreferencesHelper implements RankingConfig { // but the system can if (group.isBlocked() != oldGroup.isBlocked()) { group.lockFields(NotificationChannelGroup.USER_LOCKED_BLOCKED_STATE); - updateChannelsBypassingDnd(mContext.getUserId()); + updateChannelsBypassingDnd(); } } } @@ -871,13 +875,13 @@ public class PreferencesHelper implements RankingConfig { // fields on the channel yet if (existing.getUserLockedFields() == 0 && hasDndAccess) { boolean bypassDnd = channel.canBypassDnd(); - if (bypassDnd != existing.canBypassDnd()) { + if (bypassDnd != existing.canBypassDnd() || wasUndeleted) { existing.setBypassDnd(bypassDnd); needsPolicyFileChange = true; if (bypassDnd != mAreChannelsBypassingDnd || previousExistingImportance != existing.getImportance()) { - updateChannelsBypassingDnd(mContext.getUserId()); + updateChannelsBypassingDnd(); } } } @@ -941,7 +945,7 @@ public class PreferencesHelper implements RankingConfig { r.channels.put(channel.getId(), channel); if (channel.canBypassDnd() != mAreChannelsBypassingDnd) { - updateChannelsBypassingDnd(mContext.getUserId()); + updateChannelsBypassingDnd(); } MetricsLogger.action(getChannelLog(channel, pkg).setType( com.android.internal.logging.nano.MetricsProto.MetricsEvent.TYPE_OPEN)); @@ -1013,7 +1017,7 @@ public class PreferencesHelper implements RankingConfig { if (updatedChannel.canBypassDnd() != mAreChannelsBypassingDnd || channel.getImportance() != updatedChannel.getImportance()) { - updateChannelsBypassingDnd(mContext.getUserId()); + updateChannelsBypassingDnd(); } } updateConfig(); @@ -1110,7 +1114,7 @@ public class PreferencesHelper implements RankingConfig { mNotificationChannelLogger.logNotificationChannelDeleted(channel, uid, pkg); if (mAreChannelsBypassingDnd && channel.canBypassDnd()) { - updateChannelsBypassingDnd(mContext.getUserId()); + updateChannelsBypassingDnd(); } } } @@ -1454,7 +1458,7 @@ public class PreferencesHelper implements RankingConfig { } } if (!deletedChannelIds.isEmpty() && mAreChannelsBypassingDnd) { - updateChannelsBypassingDnd(mContext.getUserId()); + updateChannelsBypassingDnd(); } return deletedChannelIds; } @@ -1600,29 +1604,29 @@ public class PreferencesHelper implements RankingConfig { } /** - * Syncs {@link #mAreChannelsBypassingDnd} with the user's notification policy before + * Syncs {@link #mAreChannelsBypassingDnd} with the current user's notification policy before * updating - * @param userId */ - private void syncChannelsBypassingDnd(int userId) { + private void syncChannelsBypassingDnd() { mAreChannelsBypassingDnd = (mZenModeHelper.getNotificationPolicy().state & NotificationManager.Policy.STATE_CHANNELS_BYPASSING_DND) == 1; - updateChannelsBypassingDnd(userId); + updateChannelsBypassingDnd(); } /** - * Updates the user's NotificationPolicy based on whether the given userId + * Updates the user's NotificationPolicy based on whether the current userId * has channels bypassing DND * @param userId */ - private void updateChannelsBypassingDnd(int userId) { + private void updateChannelsBypassingDnd() { synchronized (mPackagePreferences) { final int numPackagePreferences = mPackagePreferences.size(); for (int i = 0; i < numPackagePreferences; i++) { final PackagePreferences r = mPackagePreferences.valueAt(i); - // Package isn't associated with this userId or notifications from this package are - // blocked - if (userId != UserHandle.getUserId(r.uid) || r.importance == IMPORTANCE_NONE) { + // Package isn't associated with the current userId or notifications from this + // package are blocked + if (mCurrentUserId != UserHandle.getUserId(r.uid) + || r.importance == IMPORTANCE_NONE) { continue; } @@ -2168,14 +2172,16 @@ public class PreferencesHelper implements RankingConfig { * Called when user switches */ public void onUserSwitched(int userId) { - syncChannelsBypassingDnd(userId); + mCurrentUserId = userId; + syncChannelsBypassingDnd(); } /** * Called when user is unlocked */ public void onUserUnlocked(int userId) { - syncChannelsBypassingDnd(userId); + mCurrentUserId = userId; + syncChannelsBypassingDnd(); } public void onUserRemoved(int userId) { From 978bb8f8101135ace0579b7f4ae2e3a39e48f89f Mon Sep 17 00:00:00 2001 From: Francois Gaffie Date: Tue, 9 Mar 2021 10:20:44 +0100 Subject: [PATCH 2/2] [Notification] Fix NotificationManagerServiceTest NotificationManagerServiceTest fails (10 failures) when launched with different user as 0 (e.g. Automotive platform). Test could be launched with "--user 0" to fix these test or test can be adapted not to hardcode user id launching these test. This CL makes the test adapative to the user id launching unit test. Bug: 178032672 Test: adb shell am instrument -w -e class com.android.server.notification.NotificationManagerServiceTest com.android.frameworks.tests.uiservices Signed-off-by: Francois Gaffie Change-Id: I1f124deafd50d257683667ef986b88df1e4b8a75 Merged-in: I1f124deafd50d257683667ef986b88df1e4b8a75 --- .../NotificationManagerServiceTest.java | 82 +++++++++---------- 1 file changed, 41 insertions(+), 41 deletions(-) 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 4259831c15f89..ee89e1c840d62 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -833,10 +833,10 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { @Test public void testDefaultAssistant_overrideDefault() { - final int userId = 0; + final int userId = mContext.getUserId(); final String testComponent = "package/class"; final List userInfos = new ArrayList<>(); - userInfos.add(new UserInfo(0, "", 0)); + userInfos.add(new UserInfo(userId, "", 0)); final ArraySet validAssistants = new ArraySet<>(); validAssistants.add(ComponentName.unflattenFromString(testComponent)); when(mActivityManager.isLowRamDevice()).thenReturn(false); @@ -2393,7 +2393,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { .thenReturn(mTestNotificationChannel); reset(mListeners); - mBinderService.updateNotificationChannelForPackage(PKG, 0, mTestNotificationChannel); + mBinderService.updateNotificationChannelForPackage(PKG, mUid, mTestNotificationChannel); verify(mListeners, times(1)).notifyNotificationChannelChanged(eq(PKG), eq(Process.myUserHandle()), eq(mTestNotificationChannel), eq(NotificationListenerService.NOTIFICATION_CHANNEL_OR_GROUP_UPDATED)); @@ -2929,7 +2929,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { @Test public void testSetListenerAccessForUser() throws Exception { - UserHandle user = UserHandle.of(10); + UserHandle user = UserHandle.of(mContext.getUserId() + 10); ComponentName c = ComponentName.unflattenFromString("package/Component"); mBinderService.setNotificationListenerAccessGrantedForUser(c, user.getIdentifier(), true); @@ -2945,20 +2945,20 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { @Test public void testSetAssistantAccessForUser() throws Exception { - UserHandle user = UserHandle.of(10); - List uis = new ArrayList<>(); UserInfo ui = new UserInfo(); - ui.id = 10; + ui.id = mContext.getUserId() + 10; + UserHandle user = UserHandle.of(ui.id); + List uis = new ArrayList<>(); uis.add(ui); ComponentName c = ComponentName.unflattenFromString("package/Component"); - when(mUm.getEnabledProfiles(10)).thenReturn(uis); + when(mUm.getEnabledProfiles(ui.id)).thenReturn(uis); mBinderService.setNotificationAssistantAccessGrantedForUser(c, user.getIdentifier(), true); verify(mContext, times(1)).sendBroadcastAsUser(any(), eq(user), any()); verify(mAssistants, times(1)).setPackageOrComponentEnabled( c.flattenToString(), user.getIdentifier(), true, true); - verify(mAssistants).setUserSet(10, true); + verify(mAssistants).setUserSet(ui.id, true); verify(mConditionProviders, times(1)).setPackageOrComponentEnabled( c.flattenToString(), user.getIdentifier(), false, true); verify(mListeners, never()).setPackageOrComponentEnabled( @@ -2967,7 +2967,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { @Test public void testGetAssistantAllowedForUser() throws Exception { - UserHandle user = UserHandle.of(10); + UserHandle user = UserHandle.of(mContext.getUserId() + 10); try { mBinderService.getAllowedNotificationAssistantForUser(user.getIdentifier()); } catch (IllegalStateException e) { @@ -2987,12 +2987,12 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { throw e; } } - verify(mAssistants, times(1)).getAllowedComponents(0); + verify(mAssistants, times(1)).getAllowedComponents(mContext.getUserId()); } @Test public void testSetDndAccessForUser() throws Exception { - UserHandle user = UserHandle.of(10); + UserHandle user = UserHandle.of(mContext.getUserId() + 10); ComponentName c = ComponentName.unflattenFromString("package/Component"); mBinderService.setNotificationPolicyAccessGrantedForUser( c.getPackageName(), user.getIdentifier(), true); @@ -3012,9 +3012,9 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mBinderService.setNotificationListenerAccessGranted(c, true); verify(mListeners, times(1)).setPackageOrComponentEnabled( - c.flattenToString(), 0, true, true); + c.flattenToString(), mContext.getUserId(), true, true); verify(mConditionProviders, times(1)).setPackageOrComponentEnabled( - c.flattenToString(), 0, false, true); + c.flattenToString(), mContext.getUserId(), false, true); verify(mAssistants, never()).setPackageOrComponentEnabled( any(), anyInt(), anyBoolean(), anyBoolean()); } @@ -3023,7 +3023,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { public void testSetAssistantAccess() throws Exception { List uis = new ArrayList<>(); UserInfo ui = new UserInfo(); - ui.id = 0; + ui.id = mContext.getUserId(); uis.add(ui); when(mUm.getEnabledProfiles(ui.id)).thenReturn(uis); ComponentName c = ComponentName.unflattenFromString("package/Component"); @@ -3031,9 +3031,9 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mBinderService.setNotificationAssistantAccessGranted(c, true); verify(mAssistants, times(1)).setPackageOrComponentEnabled( - c.flattenToString(), 0, true, true); + c.flattenToString(), ui.id, true, true); verify(mConditionProviders, times(1)).setPackageOrComponentEnabled( - c.flattenToString(), 0, false, true); + c.flattenToString(), ui.id, false, true); verify(mListeners, never()).setPackageOrComponentEnabled( any(), anyInt(), anyBoolean(), anyBoolean()); } @@ -3042,10 +3042,10 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { public void testSetAssistantAccess_multiProfile() throws Exception { List uis = new ArrayList<>(); UserInfo ui = new UserInfo(); - ui.id = 0; + ui.id = mContext.getUserId(); uis.add(ui); UserInfo ui10 = new UserInfo(); - ui10.id = 10; + ui10.id = mContext.getUserId() + 10; uis.add(ui10); when(mUm.getEnabledProfiles(ui.id)).thenReturn(uis); ComponentName c = ComponentName.unflattenFromString("package/Component"); @@ -3053,13 +3053,13 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mBinderService.setNotificationAssistantAccessGranted(c, true); verify(mAssistants, times(1)).setPackageOrComponentEnabled( - c.flattenToString(), 0, true, true); + c.flattenToString(), ui.id, true, true); verify(mAssistants, times(1)).setPackageOrComponentEnabled( - c.flattenToString(), 10, true, true); + c.flattenToString(), ui10.id, true, true); verify(mConditionProviders, times(1)).setPackageOrComponentEnabled( - c.flattenToString(), 0, false, true); + c.flattenToString(), ui.id, false, true); verify(mConditionProviders, times(1)).setPackageOrComponentEnabled( - c.flattenToString(), 10, false, true); + c.flattenToString(), ui10.id, false, true); verify(mListeners, never()).setPackageOrComponentEnabled( any(), anyInt(), anyBoolean(), anyBoolean()); } @@ -3072,16 +3072,16 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { when(mAssistants.getAllowedComponents(anyInt())).thenReturn(componentList); List uis = new ArrayList<>(); UserInfo ui = new UserInfo(); - ui.id = 0; + ui.id = mContext.getUserId(); uis.add(ui); when(mUm.getEnabledProfiles(ui.id)).thenReturn(uis); mBinderService.setNotificationAssistantAccessGranted(null, true); verify(mAssistants, times(1)).setPackageOrComponentEnabled( - c.flattenToString(), 0, true, false); + c.flattenToString(), ui.id, true, false); verify(mConditionProviders, times(1)).setPackageOrComponentEnabled( - c.flattenToString(), 0, false, false); + c.flattenToString(), ui.id, false, false); verify(mListeners, never()).setPackageOrComponentEnabled( any(), anyInt(), anyBoolean(), anyBoolean()); } @@ -3090,21 +3090,21 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { public void testSetAssistantAccessForUser_nullWithAllowedAssistant() throws Exception { List uis = new ArrayList<>(); UserInfo ui = new UserInfo(); - ui.id = 10; + ui.id = mContext.getUserId() + 10; uis.add(ui); UserHandle user = ui.getUserHandle(); ArrayList componentList = new ArrayList<>(); ComponentName c = ComponentName.unflattenFromString("package/Component"); componentList.add(c); when(mAssistants.getAllowedComponents(anyInt())).thenReturn(componentList); - when(mUm.getEnabledProfiles(10)).thenReturn(uis); + when(mUm.getEnabledProfiles(ui.id)).thenReturn(uis); mBinderService.setNotificationAssistantAccessGrantedForUser( null, user.getIdentifier(), true); verify(mAssistants, times(1)).setPackageOrComponentEnabled( c.flattenToString(), user.getIdentifier(), true, false); - verify(mAssistants).setUserSet(10, true); + verify(mAssistants).setUserSet(ui.id, true); verify(mConditionProviders, times(1)).setPackageOrComponentEnabled( c.flattenToString(), user.getIdentifier(), false, false); verify(mListeners, never()).setPackageOrComponentEnabled( @@ -3116,10 +3116,10 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { throws Exception { List uis = new ArrayList<>(); UserInfo ui = new UserInfo(); - ui.id = 0; + ui.id = mContext.getUserId(); uis.add(ui); UserInfo ui10 = new UserInfo(); - ui10.id = 10; + ui10.id = mContext.getUserId() + 10; uis.add(ui10); UserHandle user = ui.getUserHandle(); ArrayList componentList = new ArrayList<>(); @@ -3135,8 +3135,8 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { c.flattenToString(), user.getIdentifier(), true, false); verify(mAssistants, times(1)).setPackageOrComponentEnabled( c.flattenToString(), ui10.id, true, false); - verify(mAssistants).setUserSet(0, true); - verify(mAssistants).setUserSet(10, true); + verify(mAssistants).setUserSet(ui.id, true); + verify(mAssistants).setUserSet(ui10.id, true); verify(mConditionProviders, times(1)).setPackageOrComponentEnabled( c.flattenToString(), user.getIdentifier(), false, false); verify(mConditionProviders, times(1)).setPackageOrComponentEnabled( @@ -3152,7 +3152,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mBinderService.setNotificationPolicyAccessGranted(c.getPackageName(), true); verify(mConditionProviders, times(1)).setPackageOrComponentEnabled( - c.getPackageName(), 0, true, true); + c.getPackageName(), mContext.getUserId(), true, true); verify(mAssistants, never()).setPackageOrComponentEnabled( any(), anyInt(), anyBoolean(), anyBoolean()); verify(mListeners, never()).setPackageOrComponentEnabled( @@ -3179,7 +3179,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { ComponentName c = ComponentName.unflattenFromString("package/Component"); List uis = new ArrayList<>(); UserInfo ui = new UserInfo(); - ui.id = 0; + ui.id = mContext.getUserId(); uis.add(ui); when(mUm.getEnabledProfiles(ui.id)).thenReturn(uis); @@ -3214,9 +3214,9 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mBinderService.setNotificationListenerAccessGranted(c, true); verify(mListeners, times(1)).setPackageOrComponentEnabled( - c.flattenToString(), 0, true, true); + c.flattenToString(), mContext.getUserId(), true, true); verify(mConditionProviders, times(1)).setPackageOrComponentEnabled( - c.flattenToString(), 0, false, true); + c.flattenToString(), mContext.getUserId(), false, true); verify(mAssistants, never()).setPackageOrComponentEnabled( any(), anyInt(), anyBoolean(), anyBoolean()); } @@ -3228,7 +3228,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { ComponentName c = ComponentName.unflattenFromString("package/Component"); List uis = new ArrayList<>(); UserInfo ui = new UserInfo(); - ui.id = 0; + ui.id = mContext.getUserId(); uis.add(ui); when(mUm.getEnabledProfiles(ui.id)).thenReturn(uis); @@ -3237,9 +3237,9 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { verify(mListeners, never()).setPackageOrComponentEnabled( anyString(), anyInt(), anyBoolean(), anyBoolean()); verify(mConditionProviders, times(1)).setPackageOrComponentEnabled( - c.flattenToString(), 0, false, true); + c.flattenToString(), ui.id, false, true); verify(mAssistants, times(1)).setPackageOrComponentEnabled( - c.flattenToString(), 0, true, true); + c.flattenToString(), ui.id, true, true); } @Test @@ -3253,7 +3253,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { verify(mListeners, never()).setPackageOrComponentEnabled( anyString(), anyInt(), anyBoolean(), anyBoolean()); verify(mConditionProviders, times(1)).setPackageOrComponentEnabled( - c.getPackageName(), 0, true, true); + c.getPackageName(), mContext.getUserId(), true, true); verify(mAssistants, never()).setPackageOrComponentEnabled( any(), anyInt(), anyBoolean(), anyBoolean()); }