From df14df341daca2c1e5ddfb9b115da39b24e7b7aa Mon Sep 17 00:00:00 2001 From: Beth Thibodeau Date: Mon, 15 Aug 2022 16:24:29 -0500 Subject: [PATCH] Improve app name handling Substitute app name requires a permission, which wasn't being checked in NotificationManagerService. Now it will be stripped out if the app isn't allowed to use it. In addition now MediaDataManager simply checks that extra when getting the app name, instead of recovering the builder to get it. Bug: 242044723 Test: atest NotificationManagerServiceTest MediaDataManagerTest Test: manual Change-Id: I2529f1a76a2bd9036899d5341f8dd47677690d1d --- .../systemui/media/MediaDataManager.kt | 45 ++++++++++++---- .../systemui/media/MediaDataManagerTest.kt | 27 +++++++++- .../NotificationManagerService.java | 14 +++++ .../NotificationManagerServiceTest.java | 53 +++++++++++++++++++ 4 files changed, 126 insertions(+), 13 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt b/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt index 30ba476abce23..c8826757355a1 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt @@ -17,6 +17,7 @@ package com.android.systemui.media import android.app.Notification +import android.app.Notification.EXTRA_SUBSTITUTE_APP_NAME import android.app.PendingIntent import android.app.smartspace.SmartspaceConfig import android.app.smartspace.SmartspaceManager @@ -27,6 +28,7 @@ import android.content.ContentResolver import android.content.Context import android.content.Intent import android.content.IntentFilter +import android.content.pm.ApplicationInfo import android.content.pm.PackageManager import android.graphics.Bitmap import android.graphics.ImageDecoder @@ -57,8 +59,8 @@ import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.dump.DumpManager import com.android.systemui.plugins.ActivityStarter import com.android.systemui.plugins.BcSmartspaceDataPlugin -import com.android.systemui.statusbar.NotificationMediaManager.isPlayingState import com.android.systemui.statusbar.NotificationMediaManager.isConnectingState +import com.android.systemui.statusbar.NotificationMediaManager.isPlayingState import com.android.systemui.statusbar.notification.row.HybridGroupManager import com.android.systemui.tuner.TunerService import com.android.systemui.util.Assert @@ -633,9 +635,14 @@ class MediaDataManager( } val mediaController = mediaControllerFactory.create(token) val metadata = mediaController.metadata + val notif: Notification = sbn.notification + + val appInfo = notif.extras.getParcelable( + Notification.EXTRA_BUILDER_APPLICATION_INFO, + ApplicationInfo::class.java + ) ?: getAppInfoFromPackage(sbn.packageName) // Album art - val notif: Notification = sbn.notification var artworkBitmap = metadata?.let { loadBitmapFromUri(it) } if (artworkBitmap == null) { artworkBitmap = metadata?.getBitmap(MediaMetadata.METADATA_KEY_ART) @@ -650,8 +657,7 @@ class MediaDataManager( } // App name - val builder = Notification.Builder.recoverBuilder(context, notif) - val app = builder.loadHeaderAppName() + val appName = getAppName(sbn, appInfo) // App Icon val smallIcon = sbn.notification.smallIcon @@ -712,12 +718,7 @@ class MediaDataManager( val currentEntry = mediaEntries.get(key) val instanceId = currentEntry?.instanceId ?: logger.getNewInstanceId() - val appUid = try { - context.packageManager.getApplicationInfo(sbn.packageName, 0)?.uid!! - } catch (e: PackageManager.NameNotFoundException) { - Log.w(TAG, "Could not get app UID for ${sbn.packageName}", e) - Process.INVALID_UID - } + val appUid = appInfo?.uid ?: Process.INVALID_UID if (logEvent) { logger.logActiveMediaAdded(appUid, sbn.packageName, instanceId, playbackLocation) @@ -730,7 +731,7 @@ class MediaDataManager( val resumeAction: Runnable? = mediaEntries[key]?.resumeAction val hasCheckedForResume = mediaEntries[key]?.hasCheckedForResume == true val active = mediaEntries[key]?.active ?: true - onMediaDataLoaded(key, oldKey, MediaData(sbn.normalizedUserId, true, app, + onMediaDataLoaded(key, oldKey, MediaData(sbn.normalizedUserId, true, appName, smallIcon, artist, song, artWorkIcon, actionIcons, actionsToShowCollapsed, semanticActions, sbn.packageName, token, notif.contentIntent, device, active, resumeAction = resumeAction, playbackLocation = playbackLocation, @@ -740,6 +741,28 @@ class MediaDataManager( } } + private fun getAppInfoFromPackage(packageName: String): ApplicationInfo? { + try { + return context.packageManager.getApplicationInfo(packageName, 0) + } catch (e: PackageManager.NameNotFoundException) { + Log.w(TAG, "Could not get app info for $packageName", e) + } + return null + } + + private fun getAppName(sbn: StatusBarNotification, appInfo: ApplicationInfo?): String { + val name = sbn.notification.extras.getString(EXTRA_SUBSTITUTE_APP_NAME) + if (name != null) { + return name + } + + return if (appInfo != null) { + context.packageManager.getApplicationLabel(appInfo).toString() + } else { + sbn.packageName + } + } + /** * Generate action buttons based on notification actions */ diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt index 1cce7cfb5b8af..d1ed8e983cdd2 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt @@ -50,11 +50,10 @@ import org.mockito.Mock import org.mockito.Mockito import org.mockito.Mockito.never import org.mockito.Mockito.reset -import org.mockito.Mockito.times import org.mockito.Mockito.verify import org.mockito.Mockito.verifyNoMoreInteractions -import org.mockito.junit.MockitoJUnit import org.mockito.Mockito.`when` as whenever +import org.mockito.junit.MockitoJUnit private const val KEY = "KEY" private const val KEY_2 = "KEY_2" @@ -286,6 +285,30 @@ class MediaDataManagerTest : SysuiTestCase() { eq(mediaDataCaptor.value.instanceId), eq(MediaData.PLAYBACK_CAST_REMOTE)) } + @Test + fun testOnNotificationAdded_hasSubstituteName_isUsed() { + val subName = "Substitute Name" + val notif = SbnBuilder().run { + modifyNotification(context).also { + it.extras = Bundle().apply { + putString(Notification.EXTRA_SUBSTITUTE_APP_NAME, subName) + } + it.setStyle(MediaStyle().apply { + setMediaSession(session.sessionToken) + }) + } + build() + } + + mediaDataManager.onNotificationAdded(KEY, notif) + assertThat(backgroundExecutor.runAllReady()).isEqualTo(1) + assertThat(foregroundExecutor.runAllReady()).isEqualTo(1) + verify(listener).onMediaDataLoaded(eq(KEY), eq(null), capture(mediaDataCaptor), eq(true), + eq(0), eq(false)) + + assertThat(mediaDataCaptor.value!!.app).isEqualTo(subName) + } + @Test fun testLoadMediaDataInBg_invalidTokenNoCrash() { val bundle = Bundle() diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 057afe84c03da..7c0f65f1c7c10 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -6685,6 +6685,20 @@ public class NotificationManagerService extends SystemService { } } + // Ensure only allowed packages have a substitute app name + if (notification.extras.containsKey(Notification.EXTRA_SUBSTITUTE_APP_NAME)) { + int hasSubstituteAppNamePermission = mPackageManager.checkPermission( + permission.SUBSTITUTE_NOTIFICATION_APP_NAME, pkg, userId); + if (hasSubstituteAppNamePermission != PERMISSION_GRANTED) { + notification.extras.remove(Notification.EXTRA_SUBSTITUTE_APP_NAME); + if (DBG) { + Slog.w(TAG, "warning: pkg " + pkg + " attempting to substitute app name" + + " without holding perm " + + Manifest.permission.SUBSTITUTE_NOTIFICATION_APP_NAME); + } + } + } + // Remote views? Are they too big? checkRemoteViews(pkg, tag, id, notification); } 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 8171540c3accb..d78ca1bd7b676 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -4238,6 +4238,59 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { .containsKey(Notification.EXTRA_MEDIA_REMOTE_DEVICE)); } + @Test + public void testSubstituteAppName_hasPermission() throws RemoteException { + String subName = "Substitute Name"; + when(mPackageManager.checkPermission( + eq(android.Manifest.permission.SUBSTITUTE_NOTIFICATION_APP_NAME), any(), anyInt())) + .thenReturn(PERMISSION_GRANTED); + Bundle extras = new Bundle(); + extras.putString(Notification.EXTRA_SUBSTITUTE_APP_NAME, subName); + Notification.Builder nb = new Notification.Builder(mContext, + mTestNotificationChannel.getId()) + .addExtras(extras); + StatusBarNotification sbn = new StatusBarNotification(PKG, PKG, 1, + "testSubstituteAppNamePermission", mUid, 0, + nb.build(), UserHandle.getUserHandleForUid(mUid), null, 0); + NotificationRecord nr = new NotificationRecord(mContext, sbn, mTestNotificationChannel); + + mBinderService.enqueueNotificationWithTag(PKG, PKG, sbn.getTag(), + nr.getSbn().getId(), nr.getSbn().getNotification(), nr.getSbn().getUserId()); + waitForIdle(); + NotificationRecord posted = mService.findNotificationLocked( + PKG, nr.getSbn().getTag(), nr.getSbn().getId(), nr.getSbn().getUserId()); + + assertTrue(posted.getNotification().extras + .containsKey(Notification.EXTRA_SUBSTITUTE_APP_NAME)); + assertEquals(posted.getNotification().extras + .getString(Notification.EXTRA_SUBSTITUTE_APP_NAME), subName); + } + + @Test + public void testSubstituteAppName_noPermission() throws RemoteException { + when(mPackageManager.checkPermission( + eq(android.Manifest.permission.SUBSTITUTE_NOTIFICATION_APP_NAME), any(), anyInt())) + .thenReturn(PERMISSION_DENIED); + Bundle extras = new Bundle(); + extras.putString(Notification.EXTRA_SUBSTITUTE_APP_NAME, "Substitute Name"); + Notification.Builder nb = new Notification.Builder(mContext, + mTestNotificationChannel.getId()) + .addExtras(extras); + StatusBarNotification sbn = new StatusBarNotification(PKG, PKG, 1, + "testSubstituteAppNamePermission", mUid, 0, + nb.build(), UserHandle.getUserHandleForUid(mUid), null, 0); + NotificationRecord nr = new NotificationRecord(mContext, sbn, mTestNotificationChannel); + + mBinderService.enqueueNotificationWithTag(PKG, PKG, sbn.getTag(), + nr.getSbn().getId(), nr.getSbn().getNotification(), nr.getSbn().getUserId()); + waitForIdle(); + NotificationRecord posted = mService.findNotificationLocked( + PKG, nr.getSbn().getTag(), nr.getSbn().getId(), nr.getSbn().getUserId()); + + assertFalse(posted.getNotification().extras + .containsKey(Notification.EXTRA_SUBSTITUTE_APP_NAME)); + } + @Test public void testGetNotificationCountLocked() { String sampleTagToExclude = null;