From a7a5367e4c3dddd1c0e0c71506cca575bb123454 Mon Sep 17 00:00:00 2001 From: Edward Savage-Jones Date: Mon, 31 Oct 2022 16:28:19 +0100 Subject: Notifications channels are not locked at first boot After first boot when doing S->T upgrade or when doing factory data reset on Android T, important system app notification channels are not greyed out/disabled as expected. DefaultPermissionGrantPolicy set fixed POST_NOTIFICATIONS are ignored. If a user then modifies these channels and reboots their device, they can lock these fixed notification channels into an incorrect state, where the only recovery is factory reset. Bug: 254576888 Test: See issue Change-Id: If73e294b9a732ef47ad685fdd19f51afd8a04eef --- .../notification/NotificationManagerService.java | 4 +- .../server/notification/PreferencesHelper.java | 47 +++++++++++++--------- .../server/notification/PreferencesHelperTest.java | 4 -- 3 files changed, 30 insertions(+), 25 deletions(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 4d44c886fa22..a7b0bc5ae0cd 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -2317,7 +2317,6 @@ public class NotificationManagerService extends SystemService { mAppOps, new SysUiStatsEvent.BuilderFactory(), mShowReviewPermissionsNotification); - mPreferencesHelper.updateFixedImportance(mUm.getUsers()); mRankingHelper = new RankingHelper(getContext(), mRankingHandler, mPreferencesHelper, @@ -2760,6 +2759,9 @@ public class NotificationManagerService extends SystemService { maybeShowInitialReviewPermissionsNotification(); } else if (phase == SystemService.PHASE_ACTIVITY_MANAGER_READY) { mSnoozeHelper.scheduleRepostsForPersistedNotifications(System.currentTimeMillis()); + } else if (phase == SystemService.PHASE_DEVICE_SPECIFIC_SERVICES_READY) { + mPreferencesHelper.updateFixedImportance(mUm.getUsers()); + mPreferencesHelper.migrateNotificationPermissions(mUm.getUsers()); } } diff --git a/services/core/java/com/android/server/notification/PreferencesHelper.java b/services/core/java/com/android/server/notification/PreferencesHelper.java index 5507158f34da..d7e8c832cdf9 100644 --- a/services/core/java/com/android/server/notification/PreferencesHelper.java +++ b/services/core/java/com/android/server/notification/PreferencesHelper.java @@ -237,7 +237,6 @@ public class PreferencesHelper implements RankingConfig { Settings.Global.REVIEW_PERMISSIONS_NOTIFICATION_STATE, NotificationManagerService.REVIEW_NOTIF_STATE_SHOULD_SHOW); } - ArrayList pkgPerms = new ArrayList<>(); synchronized (mPackagePreferences) { while ((type = parser.next()) != XmlPullParser.END_DOCUMENT) { tag = parser.getName(); @@ -255,27 +254,18 @@ public class PreferencesHelper implements RankingConfig { String name = parser.getAttributeValue(null, ATT_NAME); if (!TextUtils.isEmpty(name)) { restorePackage(parser, forRestore, userId, name, upgradeForBubbles, - migrateToPermission, pkgPerms); + migrateToPermission); } } } } } - if (migrateToPermission) { - for (PackagePermission p : pkgPerms) { - try { - mPermissionHelper.setNotificationPermission(p); - } catch (Exception e) { - Slog.e(TAG, "could not migrate setting for " + p.packageName, e); - } - } - } } @GuardedBy("mPackagePreferences") private void restorePackage(TypedXmlPullParser parser, boolean forRestore, @UserIdInt int userId, String name, boolean upgradeForBubbles, - boolean migrateToPermission, ArrayList pkgPerms) { + boolean migrateToPermission) { try { int uid = parser.getAttributeInt(null, ATT_UID, UNKNOWN_UID); if (forRestore) { @@ -382,14 +372,6 @@ public class PreferencesHelper implements RankingConfig { if (migrateToPermission) { r.importance = appImportance; r.migrateToPm = true; - if (r.uid != UNKNOWN_UID) { - // Don't call into permission system until we have a valid uid - PackagePermission pkgPerm = new PackagePermission( - r.pkg, UserHandle.getUserId(r.uid), - r.importance != IMPORTANCE_NONE, - hasUserConfiguredSettings(r)); - pkgPerms.add(pkgPerm); - } } } catch (Exception e) { Slog.w(TAG, "Failed to restore pkg", e); @@ -2679,6 +2661,31 @@ public class PreferencesHelper implements RankingConfig { } } + public void migrateNotificationPermissions(List users) { + for (UserInfo user : users) { + List packages = mPm.getInstalledPackagesAsUser( + PackageManager.PackageInfoFlags.of(PackageManager.MATCH_ALL), + user.getUserHandle().getIdentifier()); + for (PackageInfo pi : packages) { + synchronized (mPackagePreferences) { + PackagePreferences p = getOrCreatePackagePreferencesLocked( + pi.packageName, pi.applicationInfo.uid); + if (p.migrateToPm && p.uid != UNKNOWN_UID) { + try { + PackagePermission pkgPerm = new PackagePermission( + p.pkg, UserHandle.getUserId(p.uid), + p.importance != IMPORTANCE_NONE, + hasUserConfiguredSettings(p)); + mPermissionHelper.setNotificationPermission(pkgPerm); + } catch (Exception e) { + Slog.e(TAG, "could not migrate setting for " + p.pkg, e); + } + } + } + } + } + } + private void updateConfig() { mRankingHandler.requestSort(); } 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 598a22bbde39..c9b264a1875b 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java @@ -676,10 +676,6 @@ public class PreferencesHelperTest extends UiServiceTestCase { compareChannels(ido, mHelper.getNotificationChannel(PKG_O, UID_O, ido.getId(), false)); compareChannels(idp, mHelper.getNotificationChannel(PKG_P, UID_P, idp.getId(), false)); - verify(mPermissionHelper).setNotificationPermission(nMr1Expected); - verify(mPermissionHelper).setNotificationPermission(oExpected); - verify(mPermissionHelper).setNotificationPermission(pExpected); - // verify that we also write a state for review_permissions_notification to eventually // show a notification assertEquals(NotificationManagerService.REVIEW_NOTIF_STATE_SHOULD_SHOW, -- cgit v1.2.3