From 7e173b43837c419a7cb77f5758191a557fdc76fa Mon Sep 17 00:00:00 2001 From: Miranda Kephart Date: Fri, 28 Apr 2023 10:58:46 -0400 Subject: [PATCH] [DO NOT MERGE] Update quickshare intent rather than recreating Currently, we extract the quickshare intent and re-wrap it as a new PendingIntent once we get the screenshot URI. This is insecure as it leads to executing the original with SysUI's permissions, which the app may not have. This change switches to using Intent.fillin to add the URI, keeping the original PendingIntent and original permission set. Bug: 278720336 Test: manual (to test successful quickshare), atest SaveImageInBackgroundTaskTest (to verify original pending intent unchanged) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:02938e8ccae910d96578475a19dff0a5e746b03d) Merged-In: Icad3d5f939fcfb894e2038948954bc2735dbe326 Change-Id: Icad3d5f939fcfb894e2038948954bc2735dbe326 --- .../screenshot/SaveImageInBackgroundTask.java | 113 ++++--- .../screenshot/ScreenshotController.java | 1 + .../screenshot/SmartActionsReceiver.java | 7 +- .../SaveImageInBackgroundTaskTest.kt | 282 ++++++++++++++++++ 4 files changed, 353 insertions(+), 50 deletions(-) create mode 100644 packages/SystemUI/tests/src/com/android/systemui/screenshot/SaveImageInBackgroundTaskTest.kt diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/SaveImageInBackgroundTask.java b/packages/SystemUI/src/com/android/systemui/screenshot/SaveImageInBackgroundTask.java index bf5fbd223186..49989273d012 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/SaveImageInBackgroundTask.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/SaveImageInBackgroundTask.java @@ -21,6 +21,7 @@ import static com.android.systemui.screenshot.LogConfig.DEBUG_STORAGE; import static com.android.systemui.screenshot.LogConfig.logTag; import static com.android.systemui.screenshot.ScreenshotNotificationSmartActionsProvider.ScreenshotSmartActionType; +import static com.android.systemui.screenshot.ScreenshotNotificationSmartActionsProvider.ScreenshotSmartActionType.QUICK_SHARE_ACTION; import android.app.ActivityTaskManager; import android.app.Notification; @@ -141,7 +142,12 @@ protected Void doInBackground(Void... paramsUnused) { // Since Quick Share target recommendation does not rely on image URL, it is // queried and surfaced before image compress/export. Action intent would not be // used, because it does not contain image URL. - queryQuickShareAction(image, user); + Notification.Action quickShare = + queryQuickShareAction(mScreenshotId, image, user, null); + if (quickShare != null) { + mQuickShareData.quickShareAction = quickShare; + mParams.mQuickShareActionsReadyListener.onActionsReady(mQuickShareData); + } } // Call synchronously here since already on a background thread. @@ -180,8 +186,8 @@ protected Void doInBackground(Void... paramsUnused) { smartActionsEnabled); mImageData.deleteAction = createDeleteAction(mContext, mContext.getResources(), uri, smartActionsEnabled); - mImageData.quickShareAction = createQuickShareAction(mContext, - mQuickShareData.quickShareAction, uri); + mImageData.quickShareAction = createQuickShareAction( + mQuickShareData.quickShareAction, mScreenshotId, uri, mImageTime, image, user); mImageData.subject = getSubjectString(); mParams.mActionsReadyListener.onActionsReady(mImageData); @@ -423,75 +429,86 @@ private static void addIntentExtras(String screenshotId, Intent intent, String a } /** - * Populate image uri into intent of Quick Share action. + * Wrap the quickshare intent and populate the fillin intent with the URI */ @VisibleForTesting - private Notification.Action createQuickShareAction(Context context, Notification.Action action, - Uri uri) { - if (action == null) { + Notification.Action createQuickShareAction( + Notification.Action quickShare, String screenshotId, Uri uri, long imageTime, + Bitmap image, UserHandle user) { + if (quickShare == null) { return null; + } else if (quickShare.actionIntent.isImmutable()) { + Notification.Action quickShareWithUri = + queryQuickShareAction(screenshotId, image, user, uri); + if (quickShareWithUri == null + || !quickShareWithUri.title.toString().contentEquals(quickShare.title)) { + return null; + } + quickShare = quickShareWithUri; } - // Populate image URI into Quick Share chip intent - Intent sharingIntent = action.actionIntent.getIntent(); - sharingIntent.setType("image/png"); - sharingIntent.putExtra(Intent.EXTRA_STREAM, uri); - String subjectDate = DateFormat.getDateTimeInstance().format(new Date(mImageTime)); + + Intent wrappedIntent = new Intent(mContext, SmartActionsReceiver.class) + .putExtra(ScreenshotController.EXTRA_ACTION_INTENT, quickShare.actionIntent) + .putExtra(ScreenshotController.EXTRA_ACTION_INTENT_FILLIN, + createFillInIntent(uri, imageTime)) + .addFlags(Intent.FLAG_RECEIVER_FOREGROUND); + Bundle extras = quickShare.getExtras(); + String actionType = extras.getString( + ScreenshotNotificationSmartActionsProvider.ACTION_TYPE, + ScreenshotNotificationSmartActionsProvider.DEFAULT_ACTION_TYPE); + addIntentExtras(screenshotId, wrappedIntent, actionType, true); + PendingIntent broadcastIntent = + PendingIntent.getBroadcast(mContext, mRandom.nextInt(), wrappedIntent, + PendingIntent.FLAG_CANCEL_CURRENT | PendingIntent.FLAG_IMMUTABLE); + return new Notification.Action.Builder(quickShare.getIcon(), quickShare.title, + broadcastIntent) + .setContextual(true) + .addExtras(extras) + .build(); + } + + private Intent createFillInIntent(Uri uri, long imageTime) { + Intent fillIn = new Intent(); + fillIn.setType("image/png"); + fillIn.putExtra(Intent.EXTRA_STREAM, uri); + String subjectDate = DateFormat.getDateTimeInstance().format(new Date(imageTime)); String subject = String.format(SCREENSHOT_SHARE_SUBJECT_TEMPLATE, subjectDate); - sharingIntent.putExtra(Intent.EXTRA_SUBJECT, subject); + fillIn.putExtra(Intent.EXTRA_SUBJECT, subject); // Include URI in ClipData also, so that grantPermission picks it up. // We don't use setData here because some apps interpret this as "to:". - ClipData clipdata = new ClipData(new ClipDescription("content", - new String[]{"image/png"}), + ClipData clipData = new ClipData( + new ClipDescription("content", new String[]{"image/png"}), new ClipData.Item(uri)); - sharingIntent.setClipData(clipdata); - sharingIntent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION); - PendingIntent updatedPendingIntent = PendingIntent.getActivity( - context, 0, sharingIntent, - PendingIntent.FLAG_CANCEL_CURRENT | PendingIntent.FLAG_IMMUTABLE); - - // Proxy smart actions through {@link SmartActionsReceiver} for logging smart actions. - Bundle extras = action.getExtras(); - String actionType = extras.getString( - ScreenshotNotificationSmartActionsProvider.ACTION_TYPE, - ScreenshotNotificationSmartActionsProvider.DEFAULT_ACTION_TYPE); - Intent intent = new Intent(context, SmartActionsReceiver.class) - .putExtra(ScreenshotController.EXTRA_ACTION_INTENT, updatedPendingIntent) - .addFlags(Intent.FLAG_RECEIVER_FOREGROUND); - // We only query for quick share actions when smart actions are enabled, so we can assert - // that it's true here. - addIntentExtras(mScreenshotId, intent, actionType, true /* smartActionsEnabled */); - PendingIntent broadcastIntent = PendingIntent.getBroadcast(context, - mRandom.nextInt(), - intent, - PendingIntent.FLAG_CANCEL_CURRENT | PendingIntent.FLAG_IMMUTABLE); - return new Notification.Action.Builder(action.getIcon(), action.title, - broadcastIntent).setContextual(true).addExtras(extras).build(); + fillIn.setClipData(clipData); + fillIn.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION); + return fillIn; } /** * Query and surface Quick Share chip if it is available. Action intent would not be used, * because it does not contain image URL which would be populated in {@link - * #createQuickShareAction(Context, Notification.Action, Uri)} + * #createQuickShareAction(Notification.Action, String, Uri, long, Bitmap, UserHandle)} */ - private void queryQuickShareAction(Bitmap image, UserHandle user) { + + @VisibleForTesting + Notification.Action queryQuickShareAction( + String screenshotId, Bitmap image, UserHandle user, Uri uri) { CompletableFuture> quickShareActionsFuture = mScreenshotSmartActions.getSmartActionsFuture( - mScreenshotId, null, image, mSmartActionsProvider, - ScreenshotSmartActionType.QUICK_SHARE_ACTION, - true /* smartActionsEnabled */, user); + screenshotId, uri, image, mSmartActionsProvider, QUICK_SHARE_ACTION, + true, user); int timeoutMs = DeviceConfig.getInt( DeviceConfig.NAMESPACE_SYSTEMUI, SystemUiDeviceConfigFlags.SCREENSHOT_NOTIFICATION_QUICK_SHARE_ACTIONS_TIMEOUT_MS, 500); List quickShareActions = mScreenshotSmartActions.getSmartActions( - mScreenshotId, quickShareActionsFuture, timeoutMs, - mSmartActionsProvider, - ScreenshotSmartActionType.QUICK_SHARE_ACTION); + screenshotId, quickShareActionsFuture, timeoutMs, + mSmartActionsProvider, QUICK_SHARE_ACTION); if (!quickShareActions.isEmpty()) { - mQuickShareData.quickShareAction = quickShareActions.get(0); - mParams.mQuickShareActionsReadyListener.onActionsReady(mQuickShareData); + return quickShareActions.get(0); } + return null; } private String getSubjectString() { diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java index 2e51cefb2c4b..8b0e7ff51aa8 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java @@ -246,6 +246,7 @@ interface TransitionDestination { static final String EXTRA_SMART_ACTIONS_ENABLED = "android:smart_actions_enabled"; static final String EXTRA_OVERRIDE_TRANSITION = "android:screenshot_override_transition"; static final String EXTRA_ACTION_INTENT = "android:screenshot_action_intent"; + static final String EXTRA_ACTION_INTENT_FILLIN = "android:screenshot_action_intent_fillin"; static final String SCREENSHOT_URI_ID = "android:screenshot_uri_id"; static final String EXTRA_CANCEL_NOTIFICATION = "android:screenshot_cancel_notification"; diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/SmartActionsReceiver.java b/packages/SystemUI/src/com/android/systemui/screenshot/SmartActionsReceiver.java index 45af1874e9db..9761f5931193 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/SmartActionsReceiver.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/SmartActionsReceiver.java @@ -18,6 +18,7 @@ import static com.android.systemui.screenshot.LogConfig.DEBUG_ACTIONS; import static com.android.systemui.screenshot.ScreenshotController.EXTRA_ACTION_INTENT; +import static com.android.systemui.screenshot.ScreenshotController.EXTRA_ACTION_INTENT_FILLIN; import static com.android.systemui.screenshot.ScreenshotController.EXTRA_ACTION_TYPE; import static com.android.systemui.screenshot.ScreenshotController.EXTRA_ID; @@ -46,7 +47,9 @@ public class SmartActionsReceiver extends BroadcastReceiver { @Override public void onReceive(Context context, Intent intent) { - PendingIntent pendingIntent = intent.getParcelableExtra(EXTRA_ACTION_INTENT); + PendingIntent pendingIntent = + intent.getParcelableExtra(EXTRA_ACTION_INTENT, PendingIntent.class); + Intent fillIn = intent.getParcelableExtra(EXTRA_ACTION_INTENT_FILLIN, Intent.class); String actionType = intent.getStringExtra(EXTRA_ACTION_TYPE); if (DEBUG_ACTIONS) { Log.d(TAG, "Executing smart action [" + actionType + "]:" + pendingIntent.getIntent()); @@ -54,7 +57,7 @@ public void onReceive(Context context, Intent intent) { ActivityOptions opts = ActivityOptions.makeBasic(); try { - pendingIntent.send(context, 0, null, null, null, null, opts.toBundle()); + pendingIntent.send(context, 0, fillIn, null, null, null, opts.toBundle()); } catch (PendingIntent.CanceledException e) { Log.e(TAG, "Pending intent canceled", e); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/screenshot/SaveImageInBackgroundTaskTest.kt b/packages/SystemUI/tests/src/com/android/systemui/screenshot/SaveImageInBackgroundTaskTest.kt new file mode 100644 index 000000000000..03f8c9394218 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/screenshot/SaveImageInBackgroundTaskTest.kt @@ -0,0 +1,282 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.screenshot + +import android.app.Notification +import android.app.PendingIntent +import android.content.ComponentName +import android.content.Intent +import android.graphics.Bitmap +import android.graphics.drawable.Icon +import android.net.Uri +import android.os.UserHandle +import android.testing.AndroidTestingRunner +import androidx.test.filters.SmallTest + +import com.android.systemui.SysuiTestCase +import com.android.systemui.screenshot.ScreenshotController.SaveImageInBackgroundData +import com.android.systemui.screenshot.ScreenshotNotificationSmartActionsProvider.ScreenshotSmartActionType +import com.android.systemui.util.mockito.any +import com.android.systemui.util.mockito.eq +import com.android.systemui.util.mockito.mock +import java.util.concurrent.CompletableFuture +import java.util.function.Supplier +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Before +import org.junit.runner.RunWith +import org.junit.Test +import org.mockito.Mockito + +@SmallTest +@RunWith(AndroidTestingRunner::class) +class SaveImageInBackgroundTaskTest : SysuiTestCase() { + private val imageExporter = mock() + private val smartActions = mock() + private val saveImageData = SaveImageInBackgroundData() + private val sharedTransitionSupplier = + mock>() + private val testScreenshotId: String = "testScreenshotId" + private val testBitmap = mock() + private val testUser = UserHandle.getUserHandleForUid(0) + private val testIcon = mock() + private val testImageTime = 1234.toLong() + + private val smartActionsUriFuture = mock>>() + private val smartActionsFuture = mock>>() + + private val testUri: Uri = Uri.parse("testUri") + private val intent = + Intent(Intent.ACTION_SEND) + .setComponent( + ComponentName.unflattenFromString( + "com.google.android.test/com.google.android.test.TestActivity" + ) + ) + private val immutablePendingIntent = + PendingIntent.getBroadcast( + mContext, + 0, + intent, + PendingIntent.FLAG_CANCEL_CURRENT or PendingIntent.FLAG_IMMUTABLE + ) + private val mutablePendingIntent = + PendingIntent.getBroadcast( + mContext, + 0, + intent, + PendingIntent.FLAG_CANCEL_CURRENT or PendingIntent.FLAG_MUTABLE + ) + + private val saveImageTask = + SaveImageInBackgroundTask( + mContext, + imageExporter, + smartActions, + saveImageData, + sharedTransitionSupplier, + ) + + @Before + fun setup() { + Mockito.`when`( + smartActions.getSmartActionsFuture( + eq(testScreenshotId), + any(Uri::class.java), + eq(testBitmap), + any(ScreenshotNotificationSmartActionsProvider::class.java), + any(ScreenshotSmartActionType::class.java), + any(Boolean::class.java), + eq(testUser) + ) + ) + .thenReturn(smartActionsUriFuture) + Mockito.`when`( + smartActions.getSmartActionsFuture( + eq(testScreenshotId), + eq(null), + eq(testBitmap), + any(ScreenshotNotificationSmartActionsProvider::class.java), + any(ScreenshotSmartActionType::class.java), + any(Boolean::class.java), + eq(testUser) + ) + ) + .thenReturn(smartActionsFuture) + } + + @Test + fun testQueryQuickShare_noAction() { + Mockito.`when`( + smartActions.getSmartActions( + eq(testScreenshotId), + eq(smartActionsFuture), + any(Int::class.java), + any(ScreenshotNotificationSmartActionsProvider::class.java), + eq(ScreenshotSmartActionType.QUICK_SHARE_ACTION) + ) + ) + .thenReturn(ArrayList()) + + val quickShareAction = + saveImageTask.queryQuickShareAction(testScreenshotId, testBitmap, testUser, testUri) + + assertNull(quickShareAction) + } + + @Test + fun testQueryQuickShare_withActions() { + val actions = ArrayList() + actions.add(constructAction("Action One", mutablePendingIntent)) + actions.add(constructAction("Action Two", mutablePendingIntent)) + Mockito.`when`( + smartActions.getSmartActions( + eq(testScreenshotId), + eq(smartActionsUriFuture), + any(Int::class.java), + any(ScreenshotNotificationSmartActionsProvider::class.java), + eq(ScreenshotSmartActionType.QUICK_SHARE_ACTION) + ) + ) + .thenReturn(actions) + + val quickShareAction = + saveImageTask.queryQuickShareAction(testScreenshotId, testBitmap, testUser, testUri)!! + + assertEquals("Action One", quickShareAction.title) + assertEquals(mutablePendingIntent, quickShareAction.actionIntent) + } + + @Test + fun testCreateQuickShareAction_originalWasNull_returnsNull() { + val quickShareAction = + saveImageTask.createQuickShareAction( + null, + testScreenshotId, + testUri, + testImageTime, + testBitmap, + testUser + ) + + assertNull(quickShareAction) + } + + @Test + fun testCreateQuickShareAction_immutableIntentDifferentAction_returnsNull() { + val actions = ArrayList() + actions.add(constructAction("New Test Action", immutablePendingIntent)) + Mockito.`when`( + smartActions.getSmartActions( + eq(testScreenshotId), + eq(smartActionsUriFuture), + any(Int::class.java), + any(ScreenshotNotificationSmartActionsProvider::class.java), + eq(ScreenshotSmartActionType.QUICK_SHARE_ACTION) + ) + ) + .thenReturn(actions) + val origAction = constructAction("Old Test Action", immutablePendingIntent) + + val quickShareAction = + saveImageTask.createQuickShareAction( + origAction, + testScreenshotId, + testUri, + testImageTime, + testBitmap, + testUser, + ) + + assertNull(quickShareAction) + } + + @Test + fun testCreateQuickShareAction_mutableIntent_returnsSafeIntent() { + val actions = ArrayList() + val action = constructAction("Action One", mutablePendingIntent) + actions.add(action) + Mockito.`when`( + smartActions.getSmartActions( + eq(testScreenshotId), + eq(smartActionsUriFuture), + any(Int::class.java), + any(ScreenshotNotificationSmartActionsProvider::class.java), + eq(ScreenshotSmartActionType.QUICK_SHARE_ACTION) + ) + ) + .thenReturn(actions) + + val quickShareAction = + saveImageTask.createQuickShareAction( + constructAction("Test Action", mutablePendingIntent), + testScreenshotId, + testUri, + testImageTime, + testBitmap, + testUser + ) + val quickSharePendingIntent = + quickShareAction.actionIntent.intent.extras!!.getParcelable( + ScreenshotController.EXTRA_ACTION_INTENT, + PendingIntent::class.java + ) + + assertEquals("Test Action", quickShareAction.title) + assertEquals(mutablePendingIntent, quickSharePendingIntent) + } + + @Test + fun testCreateQuickShareAction_immutableIntent_returnsSafeIntent() { + val actions = ArrayList() + val action = constructAction("Test Action", immutablePendingIntent) + actions.add(action) + Mockito.`when`( + smartActions.getSmartActions( + eq(testScreenshotId), + eq(smartActionsUriFuture), + any(Int::class.java), + any(ScreenshotNotificationSmartActionsProvider::class.java), + eq(ScreenshotSmartActionType.QUICK_SHARE_ACTION) + ) + ) + .thenReturn(actions) + + val quickShareAction = + saveImageTask.createQuickShareAction( + constructAction("Test Action", immutablePendingIntent), + testScreenshotId, + testUri, + testImageTime, + testBitmap, + testUser, + )!! + + assertEquals("Test Action", quickShareAction.title) + assertEquals( + immutablePendingIntent, + quickShareAction.actionIntent.intent.extras!!.getParcelable( + ScreenshotController.EXTRA_ACTION_INTENT, + PendingIntent::class.java + ) + ) + } + + private fun constructAction(title: String, intent: PendingIntent): Notification.Action { + return Notification.Action.Builder(testIcon, title, intent).build() + } +}