From fd2bfd34bfbbe8403e1e77353d7851cbcbb9e34c Mon Sep 17 00:00:00 2001 From: Aran Ink Date: Fri, 4 Oct 2019 16:30:01 -0400 Subject: [PATCH] Allow insertion of images from IMEs into notification quick replies. Test: Unit tests pass. Creating a Notification with the Notify app allows access to rich media insertion via gboard, and inserted images show up in the Notify app upon sending. Bug: 137398133 Change-Id: I65218dfaa083f7c24512430e647d8ca79058dff9 --- .../internal/statusbar/IStatusBarService.aidl | 2 + .../statusbar/policy/RemoteInputView.java | 88 +++++++++++-- .../notification/NotificationDelegate.java | 7 ++ .../NotificationManagerService.java | 51 +++++++- .../statusbar/StatusBarManagerService.java | 13 ++ .../NotificationManagerServiceTest.java | 118 ++++++++++++++++++ 6 files changed, 270 insertions(+), 9 deletions(-) diff --git a/core/java/com/android/internal/statusbar/IStatusBarService.aidl b/core/java/com/android/internal/statusbar/IStatusBarService.aidl index 4c3a177a013b7..a7306189c790c 100644 --- a/core/java/com/android/internal/statusbar/IStatusBarService.aidl +++ b/core/java/com/android/internal/statusbar/IStatusBarService.aidl @@ -17,6 +17,7 @@ package com.android.internal.statusbar; import android.app.Notification; +import android.net.Uri; import android.content.ComponentName; import android.graphics.Rect; import android.os.Bundle; @@ -77,6 +78,7 @@ interface IStatusBarService void onNotificationSettingsViewed(String key); void setSystemUiVisibility(int displayId, int vis, int mask, String cause); void onNotificationBubbleChanged(String key, boolean isBubble); + void grantInlineReplyUriPermission(String key, in Uri uri); void onGlobalActionsShown(); void onGlobalActionsHidden(); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/RemoteInputView.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/RemoteInputView.java index 43795dc08c913..cca9479d20d69 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/RemoteInputView.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/RemoteInputView.java @@ -23,12 +23,16 @@ import android.app.ActivityManager; import android.app.Notification; import android.app.PendingIntent; import android.app.RemoteInput; +import android.content.ClipDescription; import android.content.Context; import android.content.Intent; +import android.content.pm.PackageManager; import android.content.pm.ShortcutManager; import android.graphics.Rect; import android.graphics.drawable.Drawable; +import android.net.Uri; import android.os.Bundle; +import android.os.ServiceManager; import android.os.SystemClock; import android.os.UserHandle; import android.text.Editable; @@ -53,8 +57,13 @@ import android.widget.LinearLayout; import android.widget.ProgressBar; import android.widget.TextView; +import androidx.core.view.inputmethod.EditorInfoCompat; +import androidx.core.view.inputmethod.InputConnectionCompat; +import androidx.core.view.inputmethod.InputContentInfoCompat; + import com.android.internal.logging.MetricsLogger; import com.android.internal.logging.nano.MetricsProto; +import com.android.internal.statusbar.IStatusBarService; import com.android.systemui.Dependency; import com.android.systemui.Interpolators; import com.android.systemui.R; @@ -65,6 +74,7 @@ import com.android.systemui.statusbar.notification.row.wrapper.NotificationViewW import com.android.systemui.statusbar.notification.stack.StackStateAnimator; import com.android.systemui.statusbar.phone.LightBarController; +import java.util.HashMap; import java.util.function.Consumer; /** @@ -88,6 +98,8 @@ public class RemoteInputView extends LinearLayout implements View.OnClickListene private RemoteInputController mController; private RemoteInputQuickSettingsDisabler mRemoteInputQuickSettingsDisabler; + private IStatusBarService mStatusBarManagerService; + private NotificationEntry mEntry; private boolean mRemoved; @@ -103,6 +115,8 @@ public class RemoteInputView extends LinearLayout implements View.OnClickListene public RemoteInputView(Context context, AttributeSet attrs) { super(context, attrs); mRemoteInputQuickSettingsDisabler = Dependency.get(RemoteInputQuickSettingsDisabler.class); + mStatusBarManagerService = IStatusBarService.Stub.asInterface( + ServiceManager.getService(Context.STATUS_BAR_SERVICE)); } @Override @@ -128,7 +142,7 @@ public class RemoteInputView extends LinearLayout implements View.OnClickListene if (isSoftImeEvent || isKeyboardEnterKey) { if (mEditText.length() > 0) { - sendRemoteInput(); + sendRemoteInput(prepareRemoteInputFromText()); } // Consume action to prevent IME from closing. return true; @@ -141,7 +155,7 @@ public class RemoteInputView extends LinearLayout implements View.OnClickListene mEditText.mRemoteInputView = this; } - private void sendRemoteInput() { + protected Intent prepareRemoteInputFromText() { Bundle results = new Bundle(); results.putString(mRemoteInput.getResultKey(), mEditText.getText().toString()); Intent fillInIntent = new Intent().addFlags(Intent.FLAG_RECEIVER_FOREGROUND); @@ -153,6 +167,25 @@ public class RemoteInputView extends LinearLayout implements View.OnClickListene RemoteInput.setResultsSource(fillInIntent, RemoteInput.SOURCE_CHOICE); } + return fillInIntent; + } + + protected Intent prepareRemoteInputFromData(String contentType, Uri data) { + HashMap results = new HashMap<>(); + results.put(contentType, data); + try { + mStatusBarManagerService.grantInlineReplyUriPermission( + mEntry.notification.getKey(), data); + } catch (Exception e) { + Log.e(TAG, "Failed to grant URI permissions:" + e.getMessage(), e); + } + Intent fillInIntent = new Intent().addFlags(Intent.FLAG_RECEIVER_FOREGROUND); + RemoteInput.addDataResultToIntent(mRemoteInput, fillInIntent, results); + + return fillInIntent; + } + + private void sendRemoteInput(Intent intent) { mEditText.setEnabled(false); mSendButton.setVisibility(INVISIBLE); mProgressBar.setVisibility(VISIBLE); @@ -176,7 +209,7 @@ public class RemoteInputView extends LinearLayout implements View.OnClickListene MetricsLogger.action(mContext, MetricsProto.MetricsEvent.ACTION_REMOTE_INPUT_SEND, mEntry.notification.getPackageName()); try { - mPendingIntent.send(mContext, 0, fillInIntent); + mPendingIntent.send(mContext, 0, intent); } catch (PendingIntent.CanceledException e) { Log.i(TAG, "Unable to send remote input result", e); MetricsLogger.action(mContext, MetricsProto.MetricsEvent.ACTION_REMOTE_INPUT_FAIL, @@ -195,7 +228,9 @@ public class RemoteInputView extends LinearLayout implements View.OnClickListene LayoutInflater.from(context).inflate(R.layout.remote_input, root, false); v.mController = controller; v.mEntry = entry; - v.mEditText.setTextOperationUser(computeTextOperationUser(entry.notification.getUser())); + UserHandle user = computeTextOperationUser(entry.notification.getUser()); + v.mEditText.mUser = user; + v.mEditText.setTextOperationUser(user); v.setTag(VIEW_TAG); return v; @@ -204,7 +239,7 @@ public class RemoteInputView extends LinearLayout implements View.OnClickListene @Override public void onClick(View v) { if (v == mSendButton) { - sendRemoteInput(); + sendRemoteInput(prepareRemoteInputFromText()); } } @@ -518,6 +553,7 @@ public class RemoteInputView extends LinearLayout implements View.OnClickListene private RemoteInputView mRemoteInputView; boolean mShowImeOnInputConnection; private LightBarController mLightBarController; + UserHandle mUser; public RemoteEditText(Context context, AttributeSet attrs) { super(context, attrs); @@ -617,11 +653,47 @@ public class RemoteInputView extends LinearLayout implements View.OnClickListene @Override public InputConnection onCreateInputConnection(EditorInfo outAttrs) { + String[] allowedDataTypes = mRemoteInputView.mRemoteInput.getAllowedDataTypes() + .toArray(new String[0]); + EditorInfoCompat.setContentMimeTypes(outAttrs, allowedDataTypes); final InputConnection inputConnection = super.onCreateInputConnection(outAttrs); - if (mShowImeOnInputConnection && inputConnection != null) { + final InputConnectionCompat.OnCommitContentListener callback = + new InputConnectionCompat.OnCommitContentListener() { + @Override + public boolean onCommitContent( + InputContentInfoCompat inputContentInfoCompat, int i, + Bundle bundle) { + Uri contentUri = inputContentInfoCompat.getContentUri(); + ClipDescription description = inputContentInfoCompat.getDescription(); + String mimeType = null; + if (description != null && description.getMimeTypeCount() > 0) { + mimeType = description.getMimeType(0); + } + if (mimeType != null) { + Intent dataIntent = mRemoteInputView.prepareRemoteInputFromData( + mimeType, contentUri); + mRemoteInputView.sendRemoteInput(dataIntent); + } + return true; + } + }; + + InputConnection ic = InputConnectionCompat.createWrapper( + inputConnection, outAttrs, callback); + + Context userContext = null; + try { + userContext = mContext.createPackageContextAsUser( + mContext.getPackageName(), 0, mUser); + } catch (PackageManager.NameNotFoundException e) { + Log.e(TAG, "Unable to create user context:" + e.getMessage(), e); + } + + if (mShowImeOnInputConnection && ic != null) { + Context targetContext = userContext != null ? userContext : getContext(); final InputMethodManager imm = - getContext().getSystemService(InputMethodManager.class); + targetContext.getSystemService(InputMethodManager.class); if (imm != null) { // onCreateInputConnection is called by InputMethodManager in the middle of // setting up the connection to the IME; wait with requesting the IME until that @@ -636,7 +708,7 @@ public class RemoteInputView extends LinearLayout implements View.OnClickListene } } - return inputConnection; + return ic; } @Override diff --git a/services/core/java/com/android/server/notification/NotificationDelegate.java b/services/core/java/com/android/server/notification/NotificationDelegate.java index 61be1f5e559b9..6f0ad33244e4c 100644 --- a/services/core/java/com/android/server/notification/NotificationDelegate.java +++ b/services/core/java/com/android/server/notification/NotificationDelegate.java @@ -17,6 +17,7 @@ package com.android.server.notification; import android.app.Notification; +import android.net.Uri; import android.service.notification.NotificationStats; import com.android.internal.statusbar.NotificationVisibility; @@ -48,6 +49,12 @@ public interface NotificationDelegate { void onNotificationSettingsViewed(String key); void onNotificationBubbleChanged(String key, boolean isBubble); + /** + * Grant permission to read the specified URI to the package associated with the + * NotificationRecord associated with the given key. + */ + void grantInlineReplyUriPermission(String key, Uri uri, int callingUid); + /** * Notifies that smart replies and actions have been added to the UI. */ diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 30b2245ec24e2..ff7a7df5a27b3 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -1078,6 +1078,56 @@ public class NotificationManagerService extends SystemService { } } } + + @Override + /** + * Grant permission to read the specified URI to the package specified in the + * NotificationRecord associated with the given key. The callingUid represents the UID of + * SystemUI from which this method is being called. + * + * For this to work, SystemUI must have permission to read the URI when running under the + * user associated with the NotificationRecord, and this grant will fail when trying + * to grant URI permissions across users. + */ + public void grantInlineReplyUriPermission(String key, Uri uri, int callingUid) { + synchronized (mNotificationLock) { + NotificationRecord r = mNotificationsByKey.get(key); + if (r != null) { + IBinder owner = r.permissionOwner; + if (owner == null) { + r.permissionOwner = mUgmInternal.newUriPermissionOwner("NOTIF:" + key); + owner = r.permissionOwner; + } + int uid = callingUid; + int userId = r.sbn.getUserId(); + if (userId == UserHandle.USER_ALL) { + userId = USER_SYSTEM; + } + if (UserHandle.getUserId(uid) != userId) { + try { + final String[] pkgs = mPackageManager.getPackagesForUid(callingUid); + if (pkgs == null) { + Log.e(TAG, "Cannot grant uri permission to unknown UID: " + + callingUid); + } + final String pkg = pkgs[0]; // Get the SystemUI package + // Find the UID for SystemUI for the correct user + uid = mPackageManager.getPackageUid(pkg, 0, userId); + } catch (RemoteException re) { + Log.e(TAG, "Cannot talk to package manager", re); + } + } + grantUriPermission(owner, uri, uid, r.sbn.getPackageName(), userId); + } else { + Log.w(TAG, "No record found for notification key:" + key); + + // TODO: figure out cancel story. I think it's: sysui needs to tell us + // whenever noitifications held by a lifetimextender go away + // IBinder owner = mUgmInternal.newUriPermissionOwner("InlineReply:" + key); + // pass in userId and package as well as key (key for logging purposes) + } + } + } }; @VisibleForTesting @@ -6785,7 +6835,6 @@ public class NotificationManagerService extends SystemService { private void grantUriPermission(IBinder owner, Uri uri, int sourceUid, String targetPkg, int targetUserId) { if (uri == null || !ContentResolver.SCHEME_CONTENT.equals(uri.getScheme())) return; - final long ident = Binder.clearCallingIdentity(); try { mUgm.grantUriPermissionFromOwner(owner, sourceUid, targetPkg, diff --git a/services/core/java/com/android/server/statusbar/StatusBarManagerService.java b/services/core/java/com/android/server/statusbar/StatusBarManagerService.java index 8897eca85d7ad..3ef0858614819 100644 --- a/services/core/java/com/android/server/statusbar/StatusBarManagerService.java +++ b/services/core/java/com/android/server/statusbar/StatusBarManagerService.java @@ -29,6 +29,7 @@ import android.graphics.Rect; import android.hardware.biometrics.IBiometricServiceReceiverInternal; import android.hardware.display.DisplayManager; import android.hardware.display.DisplayManager.DisplayListener; +import android.net.Uri; import android.os.Binder; import android.os.Bundle; import android.os.Handler; @@ -1333,6 +1334,18 @@ public class StatusBarManagerService extends IStatusBarService.Stub implements D } } + @Override + public void grantInlineReplyUriPermission(String key, Uri uri) { + enforceStatusBarService(); + int callingUid = Binder.getCallingUid(); + long identity = Binder.clearCallingIdentity(); + try { + mNotificationDelegate.grantInlineReplyUriPermission(key, uri, callingUid); + } finally { + Binder.restoreCallingIdentity(identity); + } + } + @Override public void onShellCommand(FileDescriptor in, FileDescriptor out, FileDescriptor err, String[] args, ShellCallback callback, ResultReceiver resultReceiver) { 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 3ac7a79a16301..e0dc92431b5f9 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -506,6 +506,18 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { return new NotificationRecord(mContext, sbn, channel); } + private NotificationRecord generateNotificationRecord(NotificationChannel channel, int userId) { + if (channel == null) { + channel = mTestNotificationChannel; + } + Notification.Builder nb = new Notification.Builder(mContext, channel.getId()) + .setContentTitle("foo") + .setSmallIcon(android.R.drawable.sym_def_app_icon); + StatusBarNotification sbn = new StatusBarNotification(PKG, PKG, 1, "tag", mUid, 0, + nb.build(), new UserHandle(userId), null, 0); + return new NotificationRecord(mContext, sbn, channel); + } + private Map getSignalExtractorSideEffects() { Map answers = new ArrayMap<>(); @@ -5227,6 +5239,112 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { assertEquals((notifsAfter[0].getNotification().flags & FLAG_BUBBLE), 0); } + @Test + public void testGrantInlineReplyUriPermission_recordExists() throws Exception { + NotificationRecord nr = generateNotificationRecord(mTestNotificationChannel, 0); + mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", + nr.sbn.getId(), nr.sbn.getNotification(), nr.sbn.getUserId()); + waitForIdle(); + + // A notification exists for the given record + StatusBarNotification[] notifsBefore = mBinderService.getActiveNotifications(PKG); + assertEquals(1, notifsBefore.length); + + reset(mPackageManager); + + Uri uri = ContentUris.withAppendedId(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, 1); + + mService.mNotificationDelegate.grantInlineReplyUriPermission( + nr.getKey(), uri, nr.sbn.getUid()); + + // Grant permission called for the UID of SystemUI under the target user ID + verify(mUgm, times(1)).grantUriPermissionFromOwner(any(), + eq(nr.sbn.getUid()), eq(nr.sbn.getPackageName()), eq(uri), anyInt(), anyInt(), + eq(nr.sbn.getUserId())); + } + + @Test + public void testGrantInlineReplyUriPermission_userAll() throws Exception { + // generate a NotificationRecord for USER_ALL to make sure it's converted into USER_SYSTEM + NotificationRecord nr = + generateNotificationRecord(mTestNotificationChannel, UserHandle.USER_ALL); + mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", + nr.sbn.getId(), nr.sbn.getNotification(), nr.sbn.getUserId()); + waitForIdle(); + + // A notification exists for the given record + StatusBarNotification[] notifsBefore = mBinderService.getActiveNotifications(PKG); + assertEquals(1, notifsBefore.length); + + reset(mPackageManager); + + Uri uri = ContentUris.withAppendedId(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, 1); + + mService.mNotificationDelegate.grantInlineReplyUriPermission( + nr.getKey(), uri, nr.sbn.getUid()); + + // Target user for the grant is USER_ALL instead of USER_SYSTEM + verify(mUgm, times(1)).grantUriPermissionFromOwner(any(), + eq(nr.sbn.getUid()), eq(nr.sbn.getPackageName()), eq(uri), anyInt(), anyInt(), + eq(UserHandle.USER_SYSTEM)); + } + + @Test + public void testGrantInlineReplyUriPermission_acrossUsers() throws Exception { + // generate a NotificationRecord for USER_ALL to make sure it's converted into USER_SYSTEM + int otherUserId = 11; + NotificationRecord nr = + generateNotificationRecord(mTestNotificationChannel, otherUserId); + mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", + nr.sbn.getId(), nr.sbn.getNotification(), nr.sbn.getUserId()); + waitForIdle(); + + // A notification exists for the given record + StatusBarNotification[] notifsBefore = mBinderService.getActiveNotifications(PKG); + assertEquals(1, notifsBefore.length); + + reset(mPackageManager); + + Uri uri = ContentUris.withAppendedId(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, 1); + + int uid = 0; // sysui on primary user + int otherUserUid = (otherUserId * 100000) + 1; // SystemUI as a different user + String sysuiPackage = "sysui"; + final String[] sysuiPackages = new String[] { sysuiPackage }; + when(mPackageManager.getPackagesForUid(uid)).thenReturn(sysuiPackages); + + // Make sure to mock call for USER_SYSTEM and not USER_ALL, since it's been replaced by the + // time this is called + when(mPackageManager.getPackageUid(sysuiPackage, 0, otherUserId)) + .thenReturn(otherUserUid); + + mService.mNotificationDelegate.grantInlineReplyUriPermission(nr.getKey(), uri, uid); + + // Target user for the grant is USER_ALL instead of USER_SYSTEM + verify(mUgm, times(1)).grantUriPermissionFromOwner(any(), + eq(otherUserUid), eq(nr.sbn.getPackageName()), eq(uri), anyInt(), anyInt(), + eq(otherUserId)); + } + + @Test + public void testGrantInlineReplyUriPermission_noRecordExists() throws Exception { + NotificationRecord nr = generateNotificationRecord(mTestNotificationChannel); + waitForIdle(); + + // No notifications exist for the given record + StatusBarNotification[] notifsBefore = mBinderService.getActiveNotifications(PKG); + assertEquals(0, notifsBefore.length); + + Uri uri = ContentUris.withAppendedId(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, 1); + int uid = 0; // sysui on primary user + + mService.mNotificationDelegate.grantInlineReplyUriPermission(nr.getKey(), uri, uid); + + // Grant permission not called if no record exists for the given key + verify(mUgm, times(0)).grantUriPermissionFromOwner(any(), anyInt(), any(), + eq(uri), anyInt(), anyInt(), anyInt()); + } + @Test public void testNotificationBubbles_disabled_lowRamDevice() throws Exception { // Bubbles are allowed!