Fix: Don't apply single-origin mode to native apps (#667)

An unwarranted use of the Elivs operator in Form.kt makes it such that
the restrictions of single-origin mode also apply to native apps.

This commit fixes the bug and also reduces the number of intermediate
values that can mask mistakes like this one.

It also renames saveFlag to saveFlags in BrowserAutofillSupportInfo
since this variable is not limited to contain only a single flag.
This commit is contained in:
Fabian Henneke
2020-03-26 16:29:19 +01:00
committed by GitHub
parent e05b544894
commit a736dcc255
2 changed files with 10 additions and 16 deletions

View File

@@ -141,7 +141,7 @@ private fun getBrowserSaveFlag(appPackage: String): Int? = BROWSER_SAVE_FLAG[app
data class BrowserAutofillSupportInfo( data class BrowserAutofillSupportInfo(
val multiOriginMethod: BrowserMultiOriginMethod, val multiOriginMethod: BrowserMultiOriginMethod,
val saveFlag: Int? val saveFlags: Int?
) )
@RequiresApi(Build.VERSION_CODES.O) @RequiresApi(Build.VERSION_CODES.O)
@@ -152,7 +152,7 @@ fun getBrowserAutofillSupportInfoIfTrusted(
if (!isTrustedBrowser(context, appPackage)) return null if (!isTrustedBrowser(context, appPackage)) return null
return BrowserAutofillSupportInfo( return BrowserAutofillSupportInfo(
multiOriginMethod = getBrowserMultiOriginMethod(appPackage), multiOriginMethod = getBrowserMultiOriginMethod(appPackage),
saveFlag = getBrowserSaveFlag(appPackage) saveFlags = getBrowserSaveFlag(appPackage)
) )
} }
@@ -175,7 +175,7 @@ private fun getBrowserAutofillSupportLevel(
val browserInfo = getBrowserAutofillSupportInfoIfTrusted(context, appPackage) val browserInfo = getBrowserAutofillSupportInfoIfTrusted(context, appPackage)
return when { return when {
browserInfo == null -> BrowserAutofillSupportLevel.None browserInfo == null -> BrowserAutofillSupportLevel.None
browserInfo.saveFlag != null -> BrowserAutofillSupportLevel.FillAndSave browserInfo.saveFlags != null -> BrowserAutofillSupportLevel.FillAndSave
appPackage in FLAKY_BROWSERS -> BrowserAutofillSupportLevel.FlakyFill appPackage in FLAKY_BROWSERS -> BrowserAutofillSupportLevel.FlakyFill
else -> BrowserAutofillSupportLevel.Fill else -> BrowserAutofillSupportLevel.Fill
} }

View File

@@ -81,15 +81,9 @@ private class Form(context: Context, structure: AssistStructure, isManualRequest
private var appPackage = structure.activityComponent.packageName private var appPackage = structure.activityComponent.packageName
private val browserAutofillSupportInfo = private val trustedBrowserInfo =
getBrowserAutofillSupportInfoIfTrusted(context, appPackage) getBrowserAutofillSupportInfoIfTrusted(context, appPackage)
private val isTrustedBrowser = browserAutofillSupportInfo != null val saveFlags = trustedBrowserInfo?.saveFlags
private val browserMultiOriginMethod =
browserAutofillSupportInfo?.multiOriginMethod ?: BrowserMultiOriginMethod.None
private val singleOriginMode = browserMultiOriginMethod == BrowserMultiOriginMethod.None
val saveFlags = browserAutofillSupportInfo?.saveFlag
private val webOrigins = mutableSetOf<String>() private val webOrigins = mutableSetOf<String>()
@@ -114,7 +108,7 @@ private class Form(context: Context, structure: AssistStructure, isManualRequest
private fun visitFormNode(node: AssistStructure.ViewNode, inheritedWebOrigin: String? = null) { private fun visitFormNode(node: AssistStructure.ViewNode, inheritedWebOrigin: String? = null) {
trackOrigin(node) trackOrigin(node)
val field = val field =
if (browserMultiOriginMethod == BrowserMultiOriginMethod.WebView) { if (trustedBrowserInfo?.multiOriginMethod == BrowserMultiOriginMethod.WebView) {
FormField(node, fieldIndex, true, inheritedWebOrigin) FormField(node, fieldIndex, true, inheritedWebOrigin)
} else { } else {
check(inheritedWebOrigin == null) check(inheritedWebOrigin == null)
@@ -135,12 +129,12 @@ private class Form(context: Context, structure: AssistStructure, isManualRequest
private fun detectFieldsToFill(isManualRequest: Boolean) = autofillStrategy.match( private fun detectFieldsToFill(isManualRequest: Boolean) = autofillStrategy.match(
relevantFields, relevantFields,
singleOriginMode = singleOriginMode, singleOriginMode = trustedBrowserInfo?.multiOriginMethod == BrowserMultiOriginMethod.None,
isManualRequest = isManualRequest isManualRequest = isManualRequest
) )
private fun trackOrigin(node: AssistStructure.ViewNode) { private fun trackOrigin(node: AssistStructure.ViewNode) {
if (!isTrustedBrowser) return if (trustedBrowserInfo == null) return
node.webOrigin?.let { node.webOrigin?.let {
if (it !in webOrigins) { if (it !in webOrigins) {
d { "Origin encountered: $it" } d { "Origin encountered: $it" }
@@ -159,14 +153,14 @@ private class Form(context: Context, structure: AssistStructure, isManualRequest
private fun determineFormOrigin(context: Context): FormOrigin? { private fun determineFormOrigin(context: Context): FormOrigin? {
if (scenario == null) return null if (scenario == null) return null
if (!isTrustedBrowser || webOrigins.isEmpty()) { if (trustedBrowserInfo == null || webOrigins.isEmpty()) {
// Security assumption: If a trusted browser includes no web origin in the provided // Security assumption: If a trusted browser includes no web origin in the provided
// AssistStructure, then the form is a native browser form (e.g. for a sync password). // AssistStructure, then the form is a native browser form (e.g. for a sync password).
// TODO: Support WebViews in apps via Digital Asset Links // TODO: Support WebViews in apps via Digital Asset Links
// See: https://developer.android.com/reference/android/service/autofill/AutofillService#web-security // See: https://developer.android.com/reference/android/service/autofill/AutofillService#web-security
return FormOrigin.App(appPackage) return FormOrigin.App(appPackage)
} }
return when (browserMultiOriginMethod) { return when (trustedBrowserInfo.multiOriginMethod) {
BrowserMultiOriginMethod.None -> { BrowserMultiOriginMethod.None -> {
// Security assumption: If a browser is trusted but does not support tracking // Security assumption: If a browser is trusted but does not support tracking
// multiple origins, it is expected to annotate a single field, in most cases its // multiple origins, it is expected to annotate a single field, in most cases its