Add FLAG_SECURE to Dialog Window If Hosted in Activity with FLAG_SECURE Set [143778149]

Fixed

Bug

Status Update

No update yet.

Description

mm...@commonsware.com

created issue #1

Nov 3, 2019 02:51PM

Android Studio Build:

Android Studio 4.0 Canary 1
Build #AI-192.6817.14.36.5959023, built on October 22, 2019
Runtime version: 1.8.0_212-release-1586-b4-5784211 amd64
VM: OpenJDK 64-Bit Server VM by JetBrains s.r.o
Linux 5.0.0-32-generic
GC: ParNew, ConcurrentMarkSweep
Memory: 1981M
Cores: 8
Registry: ide.new.welcome.screen.force=true, debugger.watches.in.variables=false
Non-Bundled Plugins: CheckStyle-IDEA

Version of Gradle Plugin: 4.0.0-alpha01
Version of Gradle: 5.6.1
Version of Kotlin: 1.3.60-eap-25
OS: Ubuntu 19.04

-----------

Steps to Reproduce:
1. UnZIP the attached project and import it into Android Studio
2. Run the app
3. Start recording a screencast (e.g., from Android Studio)
4. Tap the "Show Dialog" button to show an AlertDialog
5. Wait a moment
6. Tap the "OK" button again to dismiss that dialog
7. Stop recording the screencast
8. Look at the screencast

Expected Results: The screencast to be completely empty, as we are recording the functionality of an activity that has FLAG_SECURE set

Actual Results: The AlertDialog appears in the screencast

The attached MP4 is a screencast taken of this app, using a Pixel 4 running Android 10.

----

If you are going to create a Window with WindowManager, you need to see whether we are in a secure window now and propagate that secure status. Otherwise, we wind up in the same situation that we have with the View-based UI framework, where FLAG_SECURE misses lots of content (menus, Spinner, dialogs, toasts, etc.), because we do not control those windows and cannot set FLAG_SECURE on them. See

https://github.com/commonsguy/cwac-security/blob/master/docs/FLAGSECURE.md and

https://issuetracker.google.com/issues/37100029 for historical context.

I think that Dialog should handle FLAG_SECURE itself, either implicitly or via a WindowSecurityPolicy value (see

https://issuetracker.google.com/issues/143778148 for more on that). While it is theoretically possible for use to maintain a SecureDialog composable that adds FLAG_SECURE, we would also need to maintain a SecureAlertDialog composable, along with Secure... variants of everything else in Compose that uses Dialog.

Given that you know whether a DialogWrapper needs to be secure, making it secure is a matter of updating the dialog's window's flags:

if (markAsSecure) window?.setFlags(WindowManager.LayoutParams.FLAG_SECURE, WindowManager.LayoutParams.FLAG_SECURE)

(assumed to be in the init block of DialogWrapper)

Thanks for considering this!

ComposeInsecure-20191103.zip

138 KB

Download

ComposeInsecureDemo-20191103.mp4

249 KB

Download

Comments

pa...@google.com <pa...@google.com> Nov 4, 2019 12:16PM

Assigned to pa...@google.com.

ch...@gmail.com <ch...@gmail.com> #2Nov 14, 2019 05:38PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

ap...@google.com <ap...@google.com> #3Oct 5, 2020 04:14PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit a9b67eca31e73a3f975b53e22a9e5983e2d76864
Author: Filip Pavlis <pavlis@google.com>
Date: Thu Oct 01 15:32:27 2020

Add FLAG_SECURE on Dialog and Popup.

Added a new way on how to pass platform specific parameters for popups
and dialogs.

Also renamed children to content. And made expect / actual to be
internal for dialog as it is for popup.

Bug: 143778148
Bug: 143778149
Test: Added tests for both dialog and popup.
Relnote: "Popups and dialogs now inherit FLAG_SECURE from parent Window.
Also added option to configure this explicitly".

Change-Id: I649663482e91757df751315b03fee9867b580e96

M compose/material/material/api/current.txt
M compose/material/material/api/public_plus_experimental_current.txt
M compose/material/material/api/restricted_current.txt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/AlertDialog.kt
M compose/ui/ui/api/current.txt
M compose/ui/ui/api/public_plus_experimental_current.txt
M compose/ui/ui/api/restricted_current.txt
M compose/ui/ui/src/androidAndroidTest/AndroidManifest.xml
A compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/window/DialogSecureFlagTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/window/PopupDismissTest.kt
A compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/window/PopupSecureFlagTest.kt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/window/PopupTestUtils.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/window/AndroidDialog.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/window/AndroidPopup.kt
A compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/window/SecureFlagPolicy.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/selection/SelectionLayout.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/window/Dialog.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/window/Popup.kt
M compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/window/DesktopDialog.kt
M compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/window/DesktopPopup.kt

https://android-review.googlesource.com/1444435

pa...@google.com <pa...@google.com> #4Oct 7, 2020 03:21PM

Marked as fixed.

Thanks for your feedback! We have just landed a CL that enables to configure a FLAG_SECURE on a dialog. By default it will inherit it from window launching it, so all compose popups from now on should inherit the flag. The flag can be also configured for AlertDialog. This will be available in the next release.

mm...@commonsware.com <mm...@commonsware.com> #5Oct 11, 2020 02:08PM

I am tremendously grateful that you added fixes for this and https://issuetracker.google.com/issues/143778148!

For testing purposes, by "the next release", do you mean alpha05? I don't know when your internal cutoff is for what makes it into a particular release.

Thanks again!

pa...@google.com <pa...@google.com> #6Oct 13, 2020 10:08AM

Sorry, yes you are right, alpha05 is the release that will have this change included. Thanks!

Issue 143778149