diff --git a/core/src/main/assets/js/bridge_components.js b/core/src/main/assets/js/bridge_components.js index 6b9470af..0b75f771 100644 --- a/core/src/main/assets/js/bridge_components.js +++ b/core/src/main/assets/js/bridge_components.js @@ -1,4 +1,10 @@ (() => { + // BridgeComponentsChannel is injected by the native side via WebViewCompat.addWebMessageListener(). + const BridgeComponentsNative = new Proxy({}, { + get: (_, name) => (...args) => + window.BridgeComponentsChannel.postMessage(JSON.stringify({ name, args })) + }) + // This represents the adapter that is installed on the webBridge // All adapters implement the same interface so the web doesn't need to // know anything specific about the client platform diff --git a/core/src/main/assets/js/turbo.js b/core/src/main/assets/js/turbo.js index 98c05fa9..552774d6 100644 --- a/core/src/main/assets/js/turbo.js +++ b/core/src/main/assets/js/turbo.js @@ -1,6 +1,12 @@ (() => { const TURBO_LOAD_TIMEOUT = 4000 + // TurboSessionChannel is injected by the native side via WebViewCompat.addWebMessageListener(). + const TurboSession = new Proxy({}, { + get: (_, name) => (...args) => + window.TurboSessionChannel.postMessage(JSON.stringify({ name, args })) + }) + // Bridge between Turbo JS and native code. Built for Turbo 7 // with backwards compatibility for Turbolinks 5 class TurboNative { diff --git a/core/src/main/kotlin/dev/hotwire/core/bridge/Bridge.kt b/core/src/main/kotlin/dev/hotwire/core/bridge/Bridge.kt index 9a0fd2df..37f692ca 100644 --- a/core/src/main/kotlin/dev/hotwire/core/bridge/Bridge.kt +++ b/core/src/main/kotlin/dev/hotwire/core/bridge/Bridge.kt @@ -1,16 +1,19 @@ package dev.hotwire.core.bridge -import android.webkit.JavascriptInterface import android.webkit.WebView import androidx.annotation.VisibleForTesting import dev.hotwire.core.logging.logDebug import dev.hotwire.core.logging.logVerbose +import dev.hotwire.core.logging.logWarning +import dev.hotwire.core.security.JavascriptChannel +import dev.hotwire.core.security.JavascriptMessage +import dev.hotwire.core.security.stringAt import kotlinx.serialization.json.JsonElement import java.lang.ref.WeakReference // These need to match whatever is set in bridge_components.js private const val bridgeGlobal = "window.nativeBridge" -private const val bridgeJavascriptInterface = "BridgeComponentsNative" +private const val bridgeChannelName = "BridgeComponentsChannel" @Suppress("unused") class Bridge internal constructor(webView: WebView) { @@ -20,14 +23,14 @@ class Bridge internal constructor(webView: WebView) { internal val webView: WebView? get() = webViewRef.get() internal var repository = Repository() internal var delegate: BridgeDelegate<*>? = null + internal val channel = JavascriptChannel(bridgeChannelName, ::dispatchBridgeMessage) init { // Use a weak reference in case the WebView is no longer being // used by the app, such as when the render process is gone. webViewRef = WeakReference(webView) - // The JavascriptInterface must be added before the page is loaded - webView.addJavascriptInterface(this, bridgeJavascriptInterface) + channel.install(webView) } internal fun register(component: String) { @@ -69,26 +72,31 @@ class Bridge internal constructor(webView: WebView) { return componentsAreRegistered } - @JavascriptInterface - fun bridgeDidInitialize() { - logDebug("bridgeDidInitialize", "success") - runOnUiThread { - delegate?.bridgeDidInitialize() + private fun dispatchBridgeMessage(message: JavascriptMessage) { + when (message.name) { + "bridgeDidInitialize" -> bridgeDidInitialize() + "bridgeDidUpdateSupportedComponents" -> bridgeDidUpdateSupportedComponents() + "bridgeDidReceiveMessage" -> bridgeDidReceiveMessage(message.args.stringAt(0)) + else -> logWarning( + "javascriptMessageUnknown", + listOf("channel" to bridgeChannelName, "name" to message.name) + ) } } - @JavascriptInterface - fun bridgeDidUpdateSupportedComponents() { + private fun bridgeDidInitialize() { + logDebug("bridgeDidInitialize", "success") + delegate?.bridgeDidInitialize() + } + + private fun bridgeDidUpdateSupportedComponents() { logDebug("bridgeDidUpdateSupportedComponents", "success") componentsAreRegistered = true } - @JavascriptInterface - fun bridgeDidReceiveMessage(message: String?) { - runOnUiThread { - InternalMessage.fromJson(message)?.let { - delegate?.bridgeDidReceiveMessage(it.toMessage()) - } + private fun bridgeDidReceiveMessage(message: String) { + InternalMessage.fromJson(message)?.let { + delegate?.bridgeDidReceiveMessage(it.toMessage()) } } diff --git a/core/src/main/kotlin/dev/hotwire/core/bridge/BridgeDelegate.kt b/core/src/main/kotlin/dev/hotwire/core/bridge/BridgeDelegate.kt index f0b7aefa..9996750a 100644 --- a/core/src/main/kotlin/dev/hotwire/core/bridge/BridgeDelegate.kt +++ b/core/src/main/kotlin/dev/hotwire/core/bridge/BridgeDelegate.kt @@ -5,6 +5,7 @@ import androidx.lifecycle.DefaultLifecycleObserver import androidx.lifecycle.LifecycleOwner import dev.hotwire.core.logging.logDebug import dev.hotwire.core.logging.logWarning +import dev.hotwire.core.security.isTrustedForNativeAccess @Suppress("unused") class BridgeDelegate( @@ -14,15 +15,19 @@ class BridgeDelegate( ) : DefaultLifecycleObserver { internal var bridge: Bridge? = null private var destinationIsActive: Boolean = false + // Trust checks use this, not resolvedLocation: the destination's intended + // location says nothing about what is loaded. + private val currentLocation: String? + get() = bridge?.webView?.url private val resolvedLocation: String - get() = bridge?.webView?.url ?: location + get() = currentLocation ?: location val initializedComponents = hashMapOf>() val activeComponents: List> get() = initializedComponents.map { it.value }.takeIf { destinationIsActive }.orEmpty() fun onColdBootPageCompleted() { - bridge?.load() + loadBridge() } fun onColdBootPageStarted() { @@ -36,7 +41,7 @@ class BridgeDelegate( if (bridge != null) { if (shouldReloadBridge()) { - bridge?.load() + loadBridge() } } else { logWarning("bridgeNotInitializedForWebView", resolvedLocation) @@ -49,6 +54,11 @@ class BridgeDelegate( } fun replyWith(message: Message): Boolean { + if (!isTrustedForNativeAccess(currentLocation)) { + logBlockedForUntrustedOrigin("bridgeReplyBlockedForUntrustedOrigin") + return false + } + bridge?.replyWith(message) ?: run { logWarning("bridgeMessageFailedToReply", "bridge is not available") return false @@ -72,10 +82,23 @@ class BridgeDelegate( } } + private fun loadBridge() { + if (!isTrustedForNativeAccess(currentLocation)) { + logBlockedForUntrustedOrigin("bridgeLoadBlockedForUntrustedOrigin") + return + } + + bridge?.load() + } + private fun shouldReloadBridge(): Boolean { return destination.bridgeWebViewIsReady() && bridge?.isReady() == false } + private fun logBlockedForUntrustedOrigin(event: String) { + logWarning(event, listOf("location" to currentLocation.orEmpty())) + } + // Lifecycle events override fun onStart(owner: LifecycleOwner) { diff --git a/core/src/main/kotlin/dev/hotwire/core/bridge/Helpers.kt b/core/src/main/kotlin/dev/hotwire/core/bridge/Helpers.kt deleted file mode 100644 index f78a03de..00000000 --- a/core/src/main/kotlin/dev/hotwire/core/bridge/Helpers.kt +++ /dev/null @@ -1,16 +0,0 @@ -package dev.hotwire.core.bridge - -import android.os.Handler -import android.os.Looper - -/** - * Guarantees main thread execution, posting a Runnable on - * the main Looper if necessary. This allows compatibility - * with unit tests that are already on the main thread. - */ -internal fun runOnUiThread(func: () -> Unit) { - when (val mainLooper = Looper.getMainLooper()) { - Looper.myLooper() -> func() - else -> Handler(mainLooper).post { func() } - } -} diff --git a/core/src/main/kotlin/dev/hotwire/core/config/HotwireConfig.kt b/core/src/main/kotlin/dev/hotwire/core/config/HotwireConfig.kt index a37aac92..39263375 100644 --- a/core/src/main/kotlin/dev/hotwire/core/config/HotwireConfig.kt +++ b/core/src/main/kotlin/dev/hotwire/core/config/HotwireConfig.kt @@ -2,11 +2,16 @@ package dev.hotwire.core.config import android.content.Context import android.webkit.WebView +import androidx.annotation.RestrictTo import dev.hotwire.core.bridge.BridgeComponent import dev.hotwire.core.bridge.BridgeComponentFactory import dev.hotwire.core.bridge.BridgeComponentJsonConverter import dev.hotwire.core.logging.DefaultHotwireLogger import dev.hotwire.core.logging.HotwireLogger +import dev.hotwire.core.security.DefaultOriginTrustPolicy +import dev.hotwire.core.security.Origin +import dev.hotwire.core.security.OriginTrustPolicy +import dev.hotwire.core.security.StartLocationRegistry import dev.hotwire.core.turbo.config.PathConfiguration import dev.hotwire.core.turbo.offline.OfflineRequestHandler import dev.hotwire.core.turbo.webview.HotwireWebView @@ -40,6 +45,33 @@ class HotwireConfig internal constructor() { */ var logger: HotwireLogger = DefaultHotwireLogger + internal val startLocationRegistry = StartLocationRegistry() + + /** + * A live view of the origins of the start locations that `NavigatorHost` + * registers. + */ + val registeredOrigins: Set + get() = startLocationRegistry.origins + + // Not public: under the default policy, adding an origin grants it + // native access. + @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) + fun registerStartLocation(startLocation: String) { + startLocationRegistry.register(startLocation) + } + + @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) + fun unregisterStartLocation(startLocation: String) { + startLocationRegistry.unregister(startLocation) + } + + /** + * The default, [DefaultOriginTrustPolicy], trusts only [registeredOrigins]. + * Set your own if your app trusts more or does not use `NavigatorHost`. + */ + var originTrustPolicy: OriginTrustPolicy = DefaultOriginTrustPolicy + /** * Enables/disables debugging of web contents loaded into WebViews. * Disabled by default. diff --git a/core/src/main/kotlin/dev/hotwire/core/files/delegates/FileChooserDelegate.kt b/core/src/main/kotlin/dev/hotwire/core/files/delegates/FileChooserDelegate.kt index 59dfd512..79820598 100644 --- a/core/src/main/kotlin/dev/hotwire/core/files/delegates/FileChooserDelegate.kt +++ b/core/src/main/kotlin/dev/hotwire/core/files/delegates/FileChooserDelegate.kt @@ -11,6 +11,8 @@ import dev.hotwire.core.R import dev.hotwire.core.files.util.HOTWIRE_REQUEST_CODE_FILES import dev.hotwire.core.files.util.HotwireFileProvider import dev.hotwire.core.logging.logError +import dev.hotwire.core.logging.logWarning +import dev.hotwire.core.security.isTrustedForNativeAccess import dev.hotwire.core.turbo.session.Session import dev.hotwire.core.turbo.util.dispatcherProvider import kotlinx.coroutines.CoroutineScope @@ -31,6 +33,16 @@ class FileChooserDelegate(val session: Session) : CoroutineScope { filePathCallback: ValueCallback>, params: FileChooserParams ): Boolean { + // FileChooserParams has no origin, so gate on the page's URL. + val pageLocation = session.webView.url + if (!isTrustedForNativeAccess(pageLocation)) { + logWarning("fileChooserBlockedForUntrustedOrigin", listOf("location" to pageLocation.orEmpty())) + filePathCallback.onReceiveValue(null) + return true + } + + // Answer a request still held from before; the WebView needs a verdict. + handleCancellation() uploadCallback = filePathCallback return openChooser(params).also { success -> @@ -84,8 +96,16 @@ class FileChooserDelegate(val session: Session) : CoroutineScope { } } - private fun sendResult(results: Array?) { - uploadCallback?.onReceiveValue(results) + internal fun sendResult(results: Array?) { + // The WebView may have navigated while the picker was open. + val pageLocation = session.webView.url + + if (results != null && !isTrustedForNativeAccess(pageLocation)) { + logWarning("fileChooserResultBlockedForUntrustedOrigin", listOf("location" to pageLocation.orEmpty())) + uploadCallback?.onReceiveValue(null) + } else { + uploadCallback?.onReceiveValue(results) + } uploadCallback = null } diff --git a/core/src/main/kotlin/dev/hotwire/core/files/delegates/GeolocationPermissionDelegate.kt b/core/src/main/kotlin/dev/hotwire/core/files/delegates/GeolocationPermissionDelegate.kt index e6bbe1bf..8e0d9f49 100644 --- a/core/src/main/kotlin/dev/hotwire/core/files/delegates/GeolocationPermissionDelegate.kt +++ b/core/src/main/kotlin/dev/hotwire/core/files/delegates/GeolocationPermissionDelegate.kt @@ -10,6 +10,8 @@ import androidx.core.content.ContextCompat import androidx.core.content.PermissionChecker import dev.hotwire.core.files.util.HOTWIRE_REQUEST_CODE_GEOLOCATION_PERMISSION import dev.hotwire.core.logging.logError +import dev.hotwire.core.logging.logWarning +import dev.hotwire.core.security.isTrustedForNativeAccess import dev.hotwire.core.turbo.session.Session class GeolocationPermissionDelegate(private val session: Session) { @@ -23,10 +25,16 @@ class GeolocationPermissionDelegate(private val session: Session) { origin: String?, callback: GeolocationPermissions.Callback? ) { + // Answer a request still held from before; the WebView needs a verdict. + permissionDenied() + requestOrigin = origin requestCallback = callback - if (requestOrigin == null || requestCallback == null || permissionToRequest == null) { + if (!isTrustedForNativeAccess(origin)) { + logWarning("geolocationPermissionBlockedForUntrustedOrigin", listOf("origin" to origin.orEmpty())) + permissionDenied() + } else if (callback == null || permissionToRequest == null) { permissionDenied() } else if (hasLocationPermission(context)) { permissionGranted() @@ -43,6 +51,16 @@ class GeolocationPermissionDelegate(private val session: Session) { } } + /** + * Called from [android.webkit.WebChromeClient.onGeolocationPermissionsHidePrompt]. + * The WebView no longer wants an answer, so the held request is dropped + * unanswered. + */ + fun onHidePrompt() { + requestOrigin = null + requestCallback = null + } + private fun startPermissionRequest() { val destination = session.currentVisit?.callback?.visitDestination() ?: return val resultLauncher = destination.activityPermissionResultLauncher( @@ -64,7 +82,9 @@ class GeolocationPermissionDelegate(private val session: Session) { } private fun permissionGranted() { - requestCallback?.invoke(requestOrigin, true, true) + // The policy's answer may have changed while the dialog was up. + val allow = isTrustedForNativeAccess(requestOrigin) + requestCallback?.invoke(requestOrigin, allow, allow) requestOrigin = null requestCallback = null } diff --git a/core/src/main/kotlin/dev/hotwire/core/files/delegates/WebViewPermissionDelegate.kt b/core/src/main/kotlin/dev/hotwire/core/files/delegates/WebViewPermissionDelegate.kt index 4eeb47ac..71a3b5b6 100644 --- a/core/src/main/kotlin/dev/hotwire/core/files/delegates/WebViewPermissionDelegate.kt +++ b/core/src/main/kotlin/dev/hotwire/core/files/delegates/WebViewPermissionDelegate.kt @@ -11,6 +11,7 @@ import androidx.core.content.PermissionChecker import dev.hotwire.core.files.util.HOTWIRE_REQUEST_CODE_WEBVIEW_PERMISSION import dev.hotwire.core.logging.logError import dev.hotwire.core.logging.logWarning +import dev.hotwire.core.security.isTrustedForNativeAccess import dev.hotwire.core.turbo.session.Session /** @@ -39,6 +40,13 @@ class WebViewPermissionDelegate(private val session: Session) { private var pendingRequest: PermissionRequest? = null fun onRequest(request: PermissionRequest) { + val origin = request.origin?.toString() + if (!isTrustedForNativeAccess(origin)) { + logWarning("webViewPermissionBlockedForUntrustedOrigin", listOf("origin" to origin.orEmpty())) + request.deny() + return + } + val requestedResources = request.resources?.toList().orEmpty() val supportedResources = requestedResources.filter { it in SUPPORTED_RESOURCES } @@ -100,7 +108,8 @@ class WebViewPermissionDelegate(private val session: Session) { grantResults[permission] == true || isGranted(permission) } - if (allGranted) { + // The policy's answer may have changed while the dialog was up. + if (allGranted && isTrustedForNativeAccess(request.origin?.toString())) { request.grant(resources.toTypedArray()) } else { request.deny() diff --git a/core/src/main/kotlin/dev/hotwire/core/logging/CoreLog.kt b/core/src/main/kotlin/dev/hotwire/core/logging/CoreLog.kt index 4e6657e1..ae011e9e 100644 --- a/core/src/main/kotlin/dev/hotwire/core/logging/CoreLog.kt +++ b/core/src/main/kotlin/dev/hotwire/core/logging/CoreLog.kt @@ -40,6 +40,22 @@ internal fun logWarning(event: String, details: String) { } } +internal fun logWarning(event: String, attributes: List>) { + Hotwire.config.logger.w(DEFAULT_TAG) { + val description = attributes.joinToString(prefix = "[", postfix = "]", separator = ", ") { + "${it.first}: ${it.second}" + } + + "${"$event ".padEnd(PAD_END_LENGTH, '.')} $description" + } +} + +internal fun logError(event: String, details: String) { + Hotwire.config.logger.e(DEFAULT_TAG, null) { + "${"$event ".padEnd(PAD_END_LENGTH, '.')} [$details]" + } +} + internal fun logError(event: String, throwable: Throwable) { Hotwire.config.logger.e(DEFAULT_TAG, throwable) { "$event: ${throwable.stackTraceToString()}" diff --git a/core/src/main/kotlin/dev/hotwire/core/security/DefaultOriginTrustPolicy.kt b/core/src/main/kotlin/dev/hotwire/core/security/DefaultOriginTrustPolicy.kt new file mode 100644 index 00000000..aa9bf927 --- /dev/null +++ b/core/src/main/kotlin/dev/hotwire/core/security/DefaultOriginTrustPolicy.kt @@ -0,0 +1,18 @@ +package dev.hotwire.core.security + +import dev.hotwire.core.config.Hotwire + +/** + * Trusts only [dev.hotwire.core.config.HotwireConfig.registeredOrigins]. An + * app that does not use `NavigatorHost` registers none, so it must set its + * own policy. + */ +object DefaultOriginTrustPolicy : OriginTrustPolicy() { + override fun isTrustedForNavigation(origin: Origin): Boolean { + return origin in Hotwire.config.registeredOrigins + } + + override fun isTrustedForNativeAccess(origin: Origin): Boolean { + return origin in Hotwire.config.registeredOrigins + } +} diff --git a/core/src/main/kotlin/dev/hotwire/core/security/JavascriptChannel.kt b/core/src/main/kotlin/dev/hotwire/core/security/JavascriptChannel.kt new file mode 100644 index 00000000..e8692ad9 --- /dev/null +++ b/core/src/main/kotlin/dev/hotwire/core/security/JavascriptChannel.kt @@ -0,0 +1,66 @@ +package dev.hotwire.core.security + +import android.webkit.WebView +import androidx.annotation.VisibleForTesting +import androidx.webkit.WebViewCompat +import androidx.webkit.WebViewFeature.WEB_MESSAGE_LISTENER +import androidx.webkit.WebViewFeature.isFeatureSupported +import dev.hotwire.core.logging.logError +import dev.hotwire.core.logging.logWarning + +/** + * The only way the library's bundled JavaScript reaches native code. Every + * message is gated on its source frame's origin before it is decoded. + */ +internal class JavascriptChannel( + private val name: String, + private val onMessage: (JavascriptMessage) -> Unit +) { + // Robolectric lacks WebMessageListener, so tests set this directly. + var isInstalled = false + @VisibleForTesting set + + /** + * Must run before the page loads. + */ + fun install(webView: WebView) { + if (isFeatureSupported(WEB_MESSAGE_LISTENER)) { + // The policy can change after install, so every frame gets the + // channel and receive() gates each message. + WebViewCompat.addWebMessageListener(webView, name, setOf("*")) { + _, message, sourceOrigin, isMainFrame, _ -> + receive(message.data.orEmpty(), sourceOrigin.toString(), isMainFrame) + } + isInstalled = true + } else { + logError( + "webMessageListenerNotSupported", + "The WebView version on this device is not supported" + ) + } + } + + /** + * Runs on the main thread, where the message listener delivers. + */ + fun receive(data: String, sourceOrigin: String, isMainFrame: Boolean) { + if (!isMainFrame || !isTrustedForNativeAccess(sourceOrigin)) { + logWarning( + "javascriptMessageBlockedForUntrustedOrigin", + listOf("channel" to name, "origin" to sourceOrigin, "isMainFrame" to isMainFrame) + ) + return + } + + val message = data.toJavascriptMessageOrNull() ?: run { + logWarning("javascriptMessageMalformed", listOf("channel" to name)) + return + } + + try { + onMessage(message) + } catch (e: RuntimeException) { + logError("javascriptMessageFailed", e) + } + } +} diff --git a/core/src/main/kotlin/dev/hotwire/core/security/JavascriptMessage.kt b/core/src/main/kotlin/dev/hotwire/core/security/JavascriptMessage.kt new file mode 100644 index 00000000..f42b8af7 --- /dev/null +++ b/core/src/main/kotlin/dev/hotwire/core/security/JavascriptMessage.kt @@ -0,0 +1,20 @@ +package dev.hotwire.core.security + +import dev.hotwire.core.bridge.decode +import kotlinx.serialization.Serializable +import kotlinx.serialization.json.JsonArray +import kotlinx.serialization.json.boolean +import kotlinx.serialization.json.int +import kotlinx.serialization.json.jsonPrimitive + +@Serializable +internal data class JavascriptMessage( + val name: String, + val args: JsonArray = JsonArray(emptyList()) +) + +internal fun String.toJavascriptMessageOrNull(): JavascriptMessage? = decode() + +internal fun JsonArray.stringAt(index: Int): String = this[index].jsonPrimitive.content +internal fun JsonArray.booleanAt(index: Int): Boolean = this[index].jsonPrimitive.boolean +internal fun JsonArray.intAt(index: Int): Int = this[index].jsonPrimitive.int diff --git a/core/src/main/kotlin/dev/hotwire/core/security/Origin.kt b/core/src/main/kotlin/dev/hotwire/core/security/Origin.kt new file mode 100644 index 00000000..737a33ef --- /dev/null +++ b/core/src/main/kotlin/dev/hotwire/core/security/Origin.kt @@ -0,0 +1,31 @@ +package dev.hotwire.core.security + +import okhttp3.HttpUrl +import okhttp3.HttpUrl.Companion.toHttpUrlOrNull + +/** + * Scheme, host, and effective port of an http(s) URL, so `https://a.com` + * and `https://a.com:443/path` are equal. Only parsing creates an origin, + * so its parts are always normalized. + */ +@ConsistentCopyVisibility +data class Origin private constructor(val scheme: String, val host: String, val port: Int) { + override fun toString(): String { + val authority = if (':' in host) "[$host]" else host + return if (port == HttpUrl.defaultPort(scheme)) "$scheme://$authority" else "$scheme://$authority:$port" + } + + companion object { + /** + * @throws IllegalArgumentException if [location] is not an http(s) URL. + */ + fun parse(location: String): Origin { + return requireNotNull(parseOrNull(location)) { "Not an http(s) URL: $location" } + } + + fun parseOrNull(location: String): Origin? { + val url = location.toHttpUrlOrNull() ?: return null + return Origin(url.scheme, url.host, url.port) + } + } +} diff --git a/core/src/main/kotlin/dev/hotwire/core/security/OriginTrustPolicy.kt b/core/src/main/kotlin/dev/hotwire/core/security/OriginTrustPolicy.kt new file mode 100644 index 00000000..56b30bd2 --- /dev/null +++ b/core/src/main/kotlin/dev/hotwire/core/security/OriginTrustPolicy.kt @@ -0,0 +1,40 @@ +package dev.hotwire.core.security + +/** + * Decides which origins the library trusts. Set via + * [dev.hotwire.core.config.HotwireConfig.originTrustPolicy]. + * + * To add to the default, call [DefaultOriginTrustPolicy]. Compare full + * origins, not hosts, so a partner's `http://` pages or other ports stay + * untrusted: + * + * ``` + * private val partner = Origin.parse("https://partner.example.com") + * + * Hotwire.config.originTrustPolicy = object : OriginTrustPolicy() { + * override fun isTrustedForNavigation(origin: Origin) = + * origin == partner || DefaultOriginTrustPolicy.isTrustedForNavigation(origin) + * + * override fun isTrustedForNativeAccess(origin: Origin) = + * DefaultOriginTrustPolicy.isTrustedForNativeAccess(origin) + * } + * ``` + * + * The library only passes authoritative origins here: the WebView's current + * URL, a WebViewClient callback, or the browser-reported origin of the frame + * that posted a message. + */ +abstract class OriginTrustPolicy { + // Add future gates as open methods that default to isTrustedForNativeAccess, + // so existing policies keep compiling and answer with their strictest trust. + + abstract fun isTrustedForNavigation(origin: Origin): Boolean + + /** + * Gates JavaScript injection, bridge and Turbo message dispatch, the file + * chooser, and native permission grants (geolocation, media capture) for + * pages at [origin]. The library also requires [isTrustedForNavigation], + * so trusting an origin here alone has no effect. + */ + abstract fun isTrustedForNativeAccess(origin: Origin): Boolean +} diff --git a/core/src/main/kotlin/dev/hotwire/core/security/Origins.kt b/core/src/main/kotlin/dev/hotwire/core/security/Origins.kt new file mode 100644 index 00000000..cfd1ddb3 --- /dev/null +++ b/core/src/main/kotlin/dev/hotwire/core/security/Origins.kt @@ -0,0 +1,23 @@ +package dev.hotwire.core.security + +import androidx.annotation.RestrictTo +import dev.hotwire.core.config.Hotwire + +@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) +fun String.hasSameOriginAs(other: String): Boolean { + val origin = Origin.parseOrNull(this) ?: return false + return origin == Origin.parseOrNull(other) +} + +@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) +fun isTrustedForNavigation(location: String?): Boolean { + val origin = location?.let { Origin.parseOrNull(it) } ?: return false + return Hotwire.config.originTrustPolicy.isTrustedForNavigation(origin) +} + +@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) +fun isTrustedForNativeAccess(location: String?): Boolean { + val origin = location?.let { Origin.parseOrNull(it) } ?: return false + val policy = Hotwire.config.originTrustPolicy + return policy.isTrustedForNavigation(origin) && policy.isTrustedForNativeAccess(origin) +} diff --git a/core/src/main/kotlin/dev/hotwire/core/security/StartLocationRegistry.kt b/core/src/main/kotlin/dev/hotwire/core/security/StartLocationRegistry.kt new file mode 100644 index 00000000..bfb6e253 --- /dev/null +++ b/core/src/main/kotlin/dev/hotwire/core/security/StartLocationRegistry.kt @@ -0,0 +1,34 @@ +package dev.hotwire.core.security + +import androidx.annotation.VisibleForTesting +import dev.hotwire.core.logging.logError +import java.util.Collections +import java.util.concurrent.ConcurrentHashMap + +/** + * Registrations are counted so one navigator host's teardown cannot drop an + * origin another host still uses. + */ +internal class StartLocationRegistry { + private val registrationCounts = ConcurrentHashMap() + + val origins: Set = Collections.unmodifiableSet(registrationCounts.keys) + + fun register(startLocation: String) { + val origin = Origin.parseOrNull(startLocation) ?: run { + logError("startLocationNotHttp", startLocation) + return + } + registrationCounts.merge(origin, 1, Int::plus) + } + + fun unregister(startLocation: String) { + val origin = Origin.parseOrNull(startLocation) ?: return + registrationCounts.computeIfPresent(origin) { _, count -> (count - 1).takeIf { it > 0 } } + } + + @VisibleForTesting + fun clear() { + registrationCounts.clear() + } +} diff --git a/core/src/main/kotlin/dev/hotwire/core/turbo/errors/HttpError.kt b/core/src/main/kotlin/dev/hotwire/core/turbo/errors/HttpError.kt index 7ccc1739..56f46bdd 100644 --- a/core/src/main/kotlin/dev/hotwire/core/turbo/errors/HttpError.kt +++ b/core/src/main/kotlin/dev/hotwire/core/turbo/errors/HttpError.kt @@ -13,6 +13,8 @@ sealed interface HttpError : VisitError { * Errors representing HTTP client errors in the 400..499 range. */ sealed interface ClientError : HttpError { + override val description: String? get() = reasonPhrase + data object BadRequest : ClientError { override val statusCode = 400 override val reasonPhrase = "Bad Request" @@ -93,6 +95,8 @@ sealed interface HttpError : VisitError { * Errors representing HTTP server errors in the 500..599 range. */ sealed interface ServerError : HttpError { + override val description: String? get() = reasonPhrase + data object InternalServerError : ServerError { override val statusCode = 500 override val reasonPhrase = "Internal Server Error" @@ -132,7 +136,9 @@ sealed interface HttpError : VisitError { data class UnknownError( override val statusCode: Int, override val reasonPhrase: String? - ) : HttpError + ) : HttpError { + override val description: String? get() = reasonPhrase + } companion object { fun from(errorResponse: WebResourceResponse): HttpError { diff --git a/core/src/main/kotlin/dev/hotwire/core/turbo/errors/LoadError.kt b/core/src/main/kotlin/dev/hotwire/core/turbo/errors/LoadError.kt index 4e88167b..3f9149b8 100644 --- a/core/src/main/kotlin/dev/hotwire/core/turbo/errors/LoadError.kt +++ b/core/src/main/kotlin/dev/hotwire/core/turbo/errors/LoadError.kt @@ -5,7 +5,7 @@ package dev.hotwire.core.turbo.errors * to load on a page. */ sealed interface LoadError : VisitError { - val description: String + override val description: String data object NotPresent : LoadError { override val description = "Turbo Not Present" @@ -14,4 +14,12 @@ sealed interface LoadError : VisitError { data object NotReady : LoadError { override val description = "Turbo Not Ready" } + + data class UntrustedOrigin(val location: String) : LoadError { + override val description = "Untrusted Origin" + } + + data object WebViewNotSupported : LoadError { + override val description = "WebView Not Supported" + } } diff --git a/core/src/main/kotlin/dev/hotwire/core/turbo/errors/VisitError.kt b/core/src/main/kotlin/dev/hotwire/core/turbo/errors/VisitError.kt index a0b8a19f..48a925e7 100644 --- a/core/src/main/kotlin/dev/hotwire/core/turbo/errors/VisitError.kt +++ b/core/src/main/kotlin/dev/hotwire/core/turbo/errors/VisitError.kt @@ -4,10 +4,10 @@ package dev.hotwire.core.turbo.errors * Represents all possible errors received when attempting to load a page. */ sealed interface VisitError { - fun description() = when (this) { - is HttpError -> reasonPhrase - is LoadError -> description - is WebError -> description - is WebSslError -> description - } + val description: String? } + +// An extension, not a member: a default method on VisitError fails R8's +// bytecode verification for HttpError's nested cases. +@Deprecated("Use the description property.", ReplaceWith("description")) +fun VisitError.description(): String? = description diff --git a/core/src/main/kotlin/dev/hotwire/core/turbo/errors/WebError.kt b/core/src/main/kotlin/dev/hotwire/core/turbo/errors/WebError.kt index 98f5ddf6..073c2987 100644 --- a/core/src/main/kotlin/dev/hotwire/core/turbo/errors/WebError.kt +++ b/core/src/main/kotlin/dev/hotwire/core/turbo/errors/WebError.kt @@ -12,7 +12,7 @@ import androidx.webkit.WebViewFeature.isFeatureSupported */ sealed interface WebError : VisitError { val errorCode: Int - val description: String? + override val description: String? data object Unknown : WebError { override val errorCode = WebViewClientCompat.ERROR_UNKNOWN diff --git a/core/src/main/kotlin/dev/hotwire/core/turbo/errors/WebSslError.kt b/core/src/main/kotlin/dev/hotwire/core/turbo/errors/WebSslError.kt index e34ef6bf..e6ea312e 100644 --- a/core/src/main/kotlin/dev/hotwire/core/turbo/errors/WebSslError.kt +++ b/core/src/main/kotlin/dev/hotwire/core/turbo/errors/WebSslError.kt @@ -9,7 +9,7 @@ import android.net.http.SslError */ sealed interface WebSslError : VisitError { val errorCode: Int - val description: String? + override val description: String? data object NotYetValid : WebSslError { override val errorCode = SslError.SSL_NOTYETVALID diff --git a/core/src/main/kotlin/dev/hotwire/core/turbo/session/Session.kt b/core/src/main/kotlin/dev/hotwire/core/turbo/session/Session.kt index 23c81f5b..72c8b26d 100644 --- a/core/src/main/kotlin/dev/hotwire/core/turbo/session/Session.kt +++ b/core/src/main/kotlin/dev/hotwire/core/turbo/session/Session.kt @@ -6,7 +6,6 @@ import android.graphics.Bitmap import android.net.http.SslError import android.util.SparseArray import android.webkit.HttpAuthHandler -import android.webkit.JavascriptInterface import android.webkit.RenderProcessGoneDetail import android.webkit.SslErrorHandler import android.webkit.WebChromeClient @@ -26,6 +25,12 @@ import dev.hotwire.core.files.delegates.GeolocationPermissionDelegate import dev.hotwire.core.files.delegates.WebViewPermissionDelegate import dev.hotwire.core.logging.logDebug import dev.hotwire.core.logging.logWarning +import dev.hotwire.core.security.JavascriptChannel +import dev.hotwire.core.security.JavascriptMessage +import dev.hotwire.core.security.booleanAt +import dev.hotwire.core.security.intAt +import dev.hotwire.core.security.isTrustedForNativeAccess +import dev.hotwire.core.security.stringAt import dev.hotwire.core.turbo.errors.HttpError import dev.hotwire.core.turbo.errors.LoadError import dev.hotwire.core.turbo.errors.WebError @@ -38,7 +43,6 @@ import dev.hotwire.core.turbo.offline.OfflineRequestHandler import dev.hotwire.core.turbo.offline.OfflineWebViewRequestInterceptor import dev.hotwire.core.turbo.util.isHttpGetRequest import dev.hotwire.core.turbo.util.runOnUiThread -import dev.hotwire.core.turbo.util.toJson import dev.hotwire.core.turbo.visit.Visit import dev.hotwire.core.turbo.visit.VisitAction import dev.hotwire.core.turbo.visit.VisitOptions @@ -46,8 +50,12 @@ import dev.hotwire.core.turbo.webview.HotwireWebView import dev.hotwire.core.turbo.webview.WebViewInfo import dev.hotwire.core.turbo.webview.WebViewVersionCompatibility import kotlinx.coroutines.launch +import kotlinx.serialization.json.JsonArray import java.util.Date +// This needs to match whatever is set in turbo.js +private const val turboSessionChannelName = "TurboSessionChannel" + /** * This class is primarily responsible for managing an instance of an Android WebView that will * be shared between destinations. @@ -71,6 +79,7 @@ class Session( internal val httpRepository = HttpRepository() internal val offlineHttpRepository = OfflineHttpRepository(activity.lifecycleScope) internal val offlineRequestInterceptor = OfflineWebViewRequestInterceptor(this) + internal val turboSessionChannel = JavascriptChannel(turboSessionChannelName, ::dispatchTurboSessionMessage) // User accessible @@ -222,16 +231,10 @@ class Session( /** * Called by Turbo bridge when a new visit is proposed. * - * Warning: This method is public so it can be used as a Javascript Interface. - * You should never call this directly as it could lead to unintended behavior. - * * @param location The location to visit. - * @param optionsJson A JSON block to be serialized into [VisitOptions]. + * @param options The options of the proposed visit. */ - @JavascriptInterface - fun visitProposedToLocation(location: String, optionsJson: String) { - val options = VisitOptions.fromJSON(optionsJson) ?: return - + internal fun visitProposedToLocation(location: String, options: VisitOptions) { logEvent("visitProposedToLocation", "location" to location, "options" to options) callback { it.visitProposedToLocation(location, options) } } @@ -256,15 +259,10 @@ class Session( * Called by Turbo bridge when a new visit proposal will refresh the * current page. * - * Warning: This method is public so it can be used as a Javascript Interface. - * You should never call this directly as it could lead to unintended behavior. - * * @param location The location to visit. - * @param optionsJson A JSON block to be serialized into [VisitOptions]. + * @param options The options of the proposed visit. */ - @JavascriptInterface - fun visitProposalRefreshingPage(location: String, optionsJson: String) { - val options = VisitOptions.fromJSON(optionsJson) ?: return + internal fun visitProposalRefreshingPage(location: String, options: VisitOptions) { logEvent("visitProposalRefreshingPage", "location" to location, "options" to options) } @@ -272,30 +270,21 @@ class Session( * Called by Turbo bridge when a new visit proposal will scroll to an anchor * on the same page. * - * Warning: This method is public so it can be used as a Javascript Interface. - * You should never call this directly as it could lead to unintended behavior. - * * @param location The location to visit. - * @param optionsJson A JSON block to be serialized into [VisitOptions]. + * @param options The options of the proposed visit. */ - @JavascriptInterface - fun visitProposalScrollingToAnchor(location: String, optionsJson: String) { - val options = VisitOptions.fromJSON(optionsJson) ?: return + internal fun visitProposalScrollingToAnchor(location: String, options: VisitOptions) { logEvent("visitProposalScrollingToAnchor", "location" to location, "options" to options) } /** * Called by Turbo bridge when a new visit has just started. * - * Warning: This method is public so it can be used as a Javascript Interface. - * You should never call this directly as it could lead to unintended behavior. - * * @param visitIdentifier A unique identifier for the visit. * @param visitHasCachedSnapshot Whether the visit has a cached snapshot available. * @param location The location being visited. */ - @JavascriptInterface - fun visitStarted(visitIdentifier: String, visitHasCachedSnapshot: Boolean, + internal fun visitStarted(visitIdentifier: String, visitHasCachedSnapshot: Boolean, visitIsPageRefresh: Boolean, location: String ) { logEvent( @@ -313,8 +302,7 @@ class Session( * * @param visitIdentifier A unique identifier for the visit. */ - @JavascriptInterface - fun visitRequestStarted(visitIdentifier: String) { + internal fun visitRequestStarted(visitIdentifier: String) { logEvent("visitRequestStarted", "visitIdentifier" to visitIdentifier) } @@ -323,24 +311,19 @@ class Session( * * @param visitIdentifier A unique identifier for the visit. */ - @JavascriptInterface - fun visitRequestCompleted(visitIdentifier: String) { + internal fun visitRequestCompleted(visitIdentifier: String) { logEvent("visitRequestCompleted", "visitIdentifier" to visitIdentifier) } /** * Called by Turbo bridge when the HTTP request has failed. * - * Warning: This method is public so it can be used as a Javascript Interface. - * You should never call this directly as it could lead to unintended behavior. - * * @param location The location of the failed visit. * @param visitIdentifier A unique identifier for the visit. * @param visitHasCachedSnapshot Whether the visit has a cached snapshot available. * @param statusCode The HTTP status code that caused the failure. */ - @JavascriptInterface - fun visitRequestFailedWithStatusCode( + internal fun visitRequestFailedWithStatusCode( location: String, visitIdentifier: String, visitHasCachedSnapshot: Boolean, @@ -368,15 +351,11 @@ class Session( * the native side. Propose a cross-origin redirect visit if a redirect is found, otherwise * fail the visit. * - * Warning: This method is public so it can be used as a Javascript Interface. - * You should never call this directly as it could lead to unintended behavior. - * * @param location The original visit location requested. * @param visitIdentifier A unique identifier for the visit. * @param visitHasCachedSnapshot Whether the visit has a cached snapshot available. */ - @JavascriptInterface - fun visitRequestFailedWithNonHttpStatusCode( + internal fun visitRequestFailedWithNonHttpStatusCode( location: String, visitIdentifier: String, visitHasCachedSnapshot: Boolean @@ -411,13 +390,9 @@ class Session( /** * Called by Turbo bridge when the HTTP request has been completed. * - * Warning: This method is public so it can be used as a Javascript Interface. - * You should never call this directly as it could lead to unintended behavior. - * * @param visitIdentifier A unique identifier for the visit. */ - @JavascriptInterface - fun visitRequestFinished(visitIdentifier: String) { + internal fun visitRequestFinished(visitIdentifier: String) { logEvent("visitRequestFinished", "visitIdentifier" to visitIdentifier) currentVisit?.let { visit -> @@ -430,14 +405,10 @@ class Session( /** * Called by Turbo bridge once the page has been fully loaded by the WebView. * - * Warning: This method is public so it can be used as a Javascript Interface. - * You should never call this directly as it could lead to unintended behavior. - * * @param restorationIdentifier A unique identifier for restoring the page and scroll position * from cache. */ - @JavascriptInterface - fun pageLoaded(restorationIdentifier: String) { + internal fun pageLoaded(restorationIdentifier: String) { logEvent("pageLoaded", "restorationIdentifier" to restorationIdentifier) currentVisit?.let { visit -> @@ -448,13 +419,9 @@ class Session( /** * Called by Turbo bridge once the page has been fully rendered in the webView. * - * Warning: This method is public so it can be used as a Javascript Interface. - * You should never call this directly as it could lead to unintended behavior. - * * @param visitIdentifier A unique identifier for the visit. */ - @JavascriptInterface - fun visitRendered(visitIdentifier: String) { + internal fun visitRendered(visitIdentifier: String) { logEvent("visitRendered", "visitIdentifier" to visitIdentifier) currentVisit?.let { visit -> @@ -472,15 +439,11 @@ class Session( * Called by Turbo bridge when the visit is fully completed (request successful and * page rendered). * - * Warning: This method is public so it can be used as a Javascript Interface. - * You should never call this directly as it could lead to unintended behavior. - * * @param visitIdentifier A unique identifier for the visit. * @param restorationIdentifier A unique identifier for restoring the page and scroll position * from cache. */ - @JavascriptInterface - fun visitCompleted(visitIdentifier: String, restorationIdentifier: String) { + internal fun visitCompleted(visitIdentifier: String, restorationIdentifier: String) { logEvent( "visitCompleted", "visitIdentifier" to visitIdentifier, @@ -498,13 +461,9 @@ class Session( /** * Called by Turbo bridge when a form submission has started. * - * Warning: This method is public so it can be used as a Javascript Interface. - * You should never call this directly as it could lead to unintended behavior. - * * @param location The location of the form submission. */ - @JavascriptInterface - fun formSubmissionStarted(location: String) { + internal fun formSubmissionStarted(location: String) { logEvent( "formSubmissionStarted", "location" to location @@ -518,13 +477,9 @@ class Session( /** * Called by Turbo bridge when a form submission has finished. * - * Warning: This method is public so it can be used as a Javascript Interface. - * You should never call this directly as it could lead to unintended behavior. - * * @param location The location of the form submission. */ - @JavascriptInterface - fun formSubmissionFinished(location: String) { + internal fun formSubmissionFinished(location: String) { logEvent( "formSubmissionFinished", "location" to location @@ -538,13 +493,8 @@ class Session( /** * Called when Turbo bridge detects that the page being visited has been invalidated, * typically by new resources in the the page HEAD. - * - * Warning: This method is public so it can be used as a Javascript Interface. - * You should never call this directly as it could lead to unintended behavior. - * */ - @JavascriptInterface - fun pageInvalidated() { + internal fun pageInvalidated() { logEvent("pageInvalidated") currentVisit?.let { visit -> @@ -558,13 +508,9 @@ class Session( /** * Sets internal flags that indicate whether Turbo in the WebView is ready for use. * - * Warning: This method is public so it can be used as a Javascript Interface. - * You should never call this directly as it could lead to unintended behavior. - * * @param isReady */ - @JavascriptInterface - fun turboIsReady(isReady: Boolean) { + internal fun turboIsReady(isReady: Boolean) { logEvent("turboIsReady", "isReady" to isReady) currentVisit?.let { visit -> @@ -592,12 +538,8 @@ class Session( /** * Sets internal flags indicating that Turbo did not properly initialize. - * - * Warning: This method is public so it can be used as a Javascript Interface. - * You should never call this directly as it could lead to unintended behavior. */ - @JavascriptInterface - fun turboFailedToLoad() { + internal fun turboFailedToLoad() { val visitError = LoadError.NotPresent logEvent("turboFailedToLoad", "error" to visitError) @@ -607,23 +549,15 @@ class Session( /** * Called when a touched element event has started. - * - * Warning: This method is public so it can be used as a Javascript Interface. - * You should never call this directly as it could lead to unintended behavior. */ - @JavascriptInterface - fun elementTouchStarted(preventsPullsToRefresh: Boolean) { + internal fun elementTouchStarted(preventsPullsToRefresh: Boolean) { webView.elementTouchPreventsPullsToRefresh = preventsPullsToRefresh } /** * Called when a touched element event has ended. - * - * Warning: This method is public so it can be used as a Javascript Interface. - * You should never call this directly as it could lead to unintended behavior. */ - @JavascriptInterface - fun elementTouchEnded() { + internal fun elementTouchEnded() { webView.elementTouchPreventsPullsToRefresh = false } @@ -728,7 +662,7 @@ class Session( } webView.apply { - addJavascriptInterface(this@Session, "TurboSession") + turboSessionChannel.install(this) webChromeClient = WebChromeClient() webViewClient = TurboWebViewClient() initDownloadListener() @@ -738,11 +672,27 @@ class Session( private fun WebView.initDownloadListener() { setDownloadListener { url, _, _, _, _ -> logEvent("downloadListener", "location" to url) - visitProposedToLocation(url, VisitOptions().toJson()) + visitProposedToLocation(url, VisitOptions()) } } private fun installBridge(location: String) { + // Without the channel Turbo can never report back, so fail instead of + // hanging. + if (!turboSessionChannel.isInstalled) { + logWarningEvent("bridgeInstallationBlockedForUnsupportedWebView") + reset() + callback { it.onReceivedError(LoadError.WebViewNotSupported) } + return + } + + if (!isTrustedForNativeAccess(location)) { + logWarningEvent("bridgeInstallationBlockedForUntrustedOrigin", "location" to location) + reset() + callback { it.onReceivedError(LoadError.UntrustedOrigin(location)) } + return + } + logEvent("installBridge", "location" to location) webView.installBridge { @@ -764,11 +714,48 @@ class Session( } } - private fun logEvent(event: String, vararg params: Pair) { - val attributes = params.toMutableList().apply { add(0, "session" to sessionName) } - logDebug(event, attributes) + private fun dispatchTurboSessionMessage(message: JavascriptMessage) { + with(message.args) { + when (message.name) { + "visitProposedToLocation" -> visitOptionsAt(1)?.let { visitProposedToLocation(stringAt(0), it) } + "visitProposalRefreshingPage" -> visitOptionsAt(1)?.let { visitProposalRefreshingPage(stringAt(0), it) } + "visitProposalScrollingToAnchor" -> visitOptionsAt(1)?.let { visitProposalScrollingToAnchor(stringAt(0), it) } + "visitStarted" -> visitStarted(stringAt(0), booleanAt(1), booleanAt(2), stringAt(3)) + "visitRequestStarted" -> visitRequestStarted(stringAt(0)) + "visitRequestCompleted" -> visitRequestCompleted(stringAt(0)) + "visitRequestFailedWithStatusCode" -> visitRequestFailedWithStatusCode(stringAt(0), stringAt(1), booleanAt(2), intAt(3)) + "visitRequestFailedWithNonHttpStatusCode" -> visitRequestFailedWithNonHttpStatusCode(stringAt(0), stringAt(1), booleanAt(2)) + "visitRequestFinished" -> visitRequestFinished(stringAt(0)) + "pageLoaded" -> pageLoaded(stringAt(0)) + "visitRendered" -> visitRendered(stringAt(0)) + "visitCompleted" -> visitCompleted(stringAt(0), stringAt(1)) + "formSubmissionStarted" -> formSubmissionStarted(stringAt(0)) + "formSubmissionFinished" -> formSubmissionFinished(stringAt(0)) + "pageInvalidated" -> pageInvalidated() + "turboIsReady" -> turboIsReady(booleanAt(0)) + "turboFailedToLoad" -> turboFailedToLoad() + "elementTouchStarted" -> elementTouchStarted(booleanAt(0)) + "elementTouchEnded" -> elementTouchEnded() + else -> logWarningEvent( + "javascriptMessageUnknown", + "channel" to turboSessionChannelName, + "name" to message.name + ) + } + } } + private fun JsonArray.visitOptionsAt(index: Int) = VisitOptions.fromJSON(stringAt(index)) + + private fun sessionAttributes(params: Array>) = + params.toMutableList().apply { add(0, "session" to sessionName) } + + private fun logEvent(event: String, vararg params: Pair) = + logDebug(event, sessionAttributes(params)) + + private fun logWarningEvent(event: String, vararg params: Pair) = + logWarning(event, sessionAttributes(params)) + // Classes and objects @@ -862,7 +849,7 @@ class Session( true -> VisitOptions(action = VisitAction.REPLACE) else -> VisitOptions(action = VisitAction.ADVANCE) } - visitProposedToLocation(location, options.toJson()) + visitProposedToLocation(location, options) } logEvent( diff --git a/core/src/main/kotlin/dev/hotwire/core/turbo/webview/HotwireWebChromeClient.kt b/core/src/main/kotlin/dev/hotwire/core/turbo/webview/HotwireWebChromeClient.kt index 3a83c1d8..a673610c 100644 --- a/core/src/main/kotlin/dev/hotwire/core/turbo/webview/HotwireWebChromeClient.kt +++ b/core/src/main/kotlin/dev/hotwire/core/turbo/webview/HotwireWebChromeClient.kt @@ -11,7 +11,6 @@ import android.webkit.WebView import com.google.android.material.dialog.MaterialAlertDialogBuilder import dev.hotwire.core.R import dev.hotwire.core.turbo.session.Session -import dev.hotwire.core.turbo.util.toJson import dev.hotwire.core.turbo.visit.VisitOptions open class HotwireWebChromeClient(val session: Session) : WebChromeClient() { @@ -84,7 +83,7 @@ open class HotwireWebChromeClient(val session: Session) : WebChromeClient() { message.data.getString("url")?.let { session.visitProposedToLocation( location = it, - optionsJson = VisitOptions().toJson() + options = VisitOptions() ) } @@ -98,6 +97,10 @@ open class HotwireWebChromeClient(val session: Session) : WebChromeClient() { session.geolocationPermissionDelegate.onRequestPermission(origin, callback) } + override fun onGeolocationPermissionsHidePrompt() { + session.geolocationPermissionDelegate.onHidePrompt() + } + override fun onPermissionRequest(request: PermissionRequest) { if (request.requestsMediaCapture()) { session.webViewPermissionDelegate.onRequest(request) diff --git a/core/src/test/kotlin/dev/hotwire/core/bridge/BridgeDelegateTest.kt b/core/src/test/kotlin/dev/hotwire/core/bridge/BridgeDelegateTest.kt index 418a5e66..fa033300 100644 --- a/core/src/test/kotlin/dev/hotwire/core/bridge/BridgeDelegateTest.kt +++ b/core/src/test/kotlin/dev/hotwire/core/bridge/BridgeDelegateTest.kt @@ -9,6 +9,8 @@ import com.nhaarman.mockito_kotlin.mock import com.nhaarman.mockito_kotlin.never import com.nhaarman.mockito_kotlin.times import com.nhaarman.mockito_kotlin.whenever +import dev.hotwire.core.config.Hotwire +import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull @@ -37,8 +39,12 @@ class BridgeDelegateTest { fun setup() { whenever(destination.bridgeWebViewIsReady()).thenReturn(true) whenever(bridge.webView).thenReturn(webView) + whenever(webView.url).thenReturn("https://37signals.com") Bridge.initialize(bridge) + Hotwire.config.startLocationRegistry.clear() + Hotwire.config.registerStartLocation("https://37signals.com") + delegate = BridgeDelegate( location = "https://37signals.com", destination = destination, @@ -50,6 +56,11 @@ class BridgeDelegateTest { lifecycleOwner.lifecycle.addObserver(delegate) } + @After + fun teardown() { + Hotwire.config.startLocationRegistry.clear() + } + @Test fun onColdBootPageCompleted() { delegate.onColdBootPageCompleted() @@ -132,6 +143,47 @@ class BridgeDelegateTest { assertNotNull(delegate.component()) } + @Test + fun onColdBootPageCompletedBlockedWithNoPageLoaded() { + whenever(webView.url).thenReturn(null) + + delegate.onColdBootPageCompleted() + verify(bridge, never()).load() + } + + @Test + fun replyWithBlockedWithNoPageLoaded() { + whenever(webView.url).thenReturn(null) + + val message = Message( + id = "1", + component = "page", + event = "connect", + metadata = Metadata("https://37signals.com"), + jsonData = """{"title":"Page-title","subtitle":"Page-subtitle"}""" + ) + + assertEquals(false, delegate.replyWith(message)) + verify(bridge, never()).replyWith(any()) + } + + @Test + fun onColdBootPageCompletedBlockedForUntrustedOrigin() { + whenever(webView.url).thenReturn("https://evil.attacker.com/page") + + delegate.onColdBootPageCompleted() + verify(bridge, never()).load() + } + + @Test + fun onWebViewAttachedBlockedForUntrustedOrigin() { + whenever(webView.url).thenReturn("https://evil.attacker.com/page") + whenever(bridge.isReady()).thenReturn(false) + + delegate.onWebViewAttached(webView) + verify(bridge, never()).load() + } + @Test fun bridgeDidReceiveMessageIgnored() { val message = Message( @@ -158,6 +210,22 @@ class BridgeDelegateTest { assertEquals(true, delegate.replyWith(message)) } + @Test + fun replyWithBlockedForUntrustedOrigin() { + whenever(webView.url).thenReturn("https://evil.attacker.com/page") + + val message = Message( + id = "1", + component = "page", + event = "connect", + metadata = Metadata("https://37signals.com"), + jsonData = """{"title":"Page-title","subtitle":"Page-subtitle"}""" + ) + + assertEquals(false, delegate.replyWith(message)) + verify(bridge, never()).replyWith(any()) + } + @Test fun replyWithFailsWithoutBridge() { val message = Message( diff --git a/core/src/test/kotlin/dev/hotwire/core/bridge/BridgeTest.kt b/core/src/test/kotlin/dev/hotwire/core/bridge/BridgeTest.kt index fe93c9be..629165bc 100644 --- a/core/src/test/kotlin/dev/hotwire/core/bridge/BridgeTest.kt +++ b/core/src/test/kotlin/dev/hotwire/core/bridge/BridgeTest.kt @@ -1,16 +1,29 @@ package dev.hotwire.core.bridge import android.content.Context +import android.os.Build import android.webkit.WebView import com.nhaarman.mockito_kotlin.any import com.nhaarman.mockito_kotlin.eq import com.nhaarman.mockito_kotlin.mock +import com.nhaarman.mockito_kotlin.never import com.nhaarman.mockito_kotlin.whenever +import dev.hotwire.core.config.Hotwire +import kotlinx.serialization.json.add +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import kotlinx.serialization.json.putJsonArray +import org.junit.After import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test +import org.junit.runner.RunWith import org.mockito.Mockito.verify +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [Build.VERSION_CODES.R]) class BridgeTest { private lateinit var bridge: Bridge private val webView: WebView = mock() @@ -18,13 +31,28 @@ class BridgeTest { private val repository: Repository = mock() private val delegate: BridgeDelegate = mock() + private val bridgeDidReceiveMessageEnvelope = buildJsonObject { + put("name", "bridgeDidReceiveMessage") + putJsonArray("args") { + add("""{"id":"1","component":"page","event":"connect","data":{"metadata":{"url":"https://37signals.com"},"title":"Page title","subtitle":"Page subtitle"}}""") + } + }.toString() + @Before fun setup() { + Hotwire.config.startLocationRegistry.clear() + Hotwire.config.registerStartLocation("https://37signals.com") + bridge = Bridge(webView) bridge.delegate = delegate bridge.repository = repository } + @After + fun teardown() { + Hotwire.config.startLocationRegistry.clear() + } + @Test fun registerComponent() { val javascript = """window.nativeBridge.register("page")""" @@ -69,18 +97,22 @@ class BridgeTest { whenever(repository.getUserScript(context)).thenReturn("") bridge.load() - verify(webView).addJavascriptInterface(eq(bridge), any()) + verify(webView).evaluateJavascript(eq(""), any()) } @Test fun bridgeDidInitialize() { - bridge.bridgeDidInitialize() + bridge.channel.receive( + data = """{"name":"bridgeDidInitialize","args":[]}""", + sourceOrigin = "https://37signals.com", + isMainFrame = true + ) + verify(delegate).bridgeDidInitialize() } @Test fun bridgeDidReceiveMessage() { - val json = """{"id":"1","component":"page","event":"connect","data":{"metadata":{"url":"https://37signals.com"},"title":"Page title","subtitle":"Page subtitle"}}""" val data = """{"metadata":{"url":"https://37signals.com"},"title":"Page title","subtitle":"Page subtitle"}""" val message = Message( id = "1", @@ -90,10 +122,57 @@ class BridgeTest { jsonData = data ) - bridge.bridgeDidReceiveMessage(json) + bridge.channel.receive( + data = bridgeDidReceiveMessageEnvelope, + sourceOrigin = "https://37signals.com", + isMainFrame = true + ) + verify(delegate).bridgeDidReceiveMessage(message) } + @Test + fun bridgeMessagesFromAnUntrustedOriginAreDropped() { + bridge.channel.receive( + data = bridgeDidReceiveMessageEnvelope, + sourceOrigin = "https://evil.attacker.com", + isMainFrame = true + ) + bridge.channel.receive( + data = """{"name":"bridgeDidInitialize","args":[]}""", + sourceOrigin = "https://evil.attacker.com", + isMainFrame = true + ) + + verify(delegate, never()).bridgeDidReceiveMessage(any()) + verify(delegate, never()).bridgeDidInitialize() + } + + @Test + fun bridgeMessagesFromASubFrameAreDropped() { + bridge.channel.receive( + data = bridgeDidReceiveMessageEnvelope, + sourceOrigin = "https://37signals.com", + isMainFrame = false + ) + + verify(delegate, never()).bridgeDidReceiveMessage(any()) + } + + @Test + fun malformedBridgeMessagesAreDropped() { + listOf( + "not json", + """{"args":[]}""", + """{"name":"noSuchMethod","args":[]}""", + """{"name":"bridgeDidReceiveMessage","args":[]}""" + ).forEach { + bridge.channel.receive(it, sourceOrigin = "https://37signals.com", isMainFrame = true) + } + + verify(delegate, never()).bridgeDidReceiveMessage(any()) + } + @Test fun userScript() { whenever(webView.context).thenReturn(context) diff --git a/core/src/test/kotlin/dev/hotwire/core/files/delegates/FileChooserDelegateTest.kt b/core/src/test/kotlin/dev/hotwire/core/files/delegates/FileChooserDelegateTest.kt index 8fd7b9d2..7e84ede6 100644 --- a/core/src/test/kotlin/dev/hotwire/core/files/delegates/FileChooserDelegateTest.kt +++ b/core/src/test/kotlin/dev/hotwire/core/files/delegates/FileChooserDelegateTest.kt @@ -1,16 +1,32 @@ package dev.hotwire.core.files.delegates import android.content.Context +import android.content.Intent +import android.net.Uri import android.os.Build +import android.webkit.ValueCallback +import android.webkit.WebChromeClient.FileChooserParams +import androidx.activity.result.ActivityResultLauncher import androidx.appcompat.app.AppCompatActivity import androidx.test.core.app.ApplicationProvider +import com.nhaarman.mockito_kotlin.anyOrNull +import com.nhaarman.mockito_kotlin.mock +import com.nhaarman.mockito_kotlin.never +import com.nhaarman.mockito_kotlin.verify +import com.nhaarman.mockito_kotlin.whenever +import dev.hotwire.core.config.Hotwire import dev.hotwire.core.files.util.HotwireFileProvider import dev.hotwire.core.turbo.BaseRepositoryTest import dev.hotwire.core.turbo.session.Session +import dev.hotwire.core.turbo.session.SessionCallback +import dev.hotwire.core.turbo.visit.Visit +import dev.hotwire.core.turbo.visit.VisitDestination +import dev.hotwire.core.turbo.visit.VisitOptions import dev.hotwire.core.turbo.webview.HotwireWebView import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.runBlocking import org.assertj.core.api.Assertions.assertThat +import org.junit.After import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -39,6 +55,113 @@ class FileChooserDelegateTest : BaseRepositoryTest() { activity = buildActivity(TurboTestActivity::class.java).get() context = ApplicationProvider.getApplicationContext() session = Session("test", activity, webView) + + Hotwire.config.startLocationRegistry.clear() + Hotwire.config.registerStartLocation("https://37signals.com") + } + + @After + fun teardownRegisteredOrigins() { + Hotwire.config.startLocationRegistry.clear() + } + + @Test + fun `file chooser is blocked on an untrusted page`() { + whenever(webView.url).thenReturn("https://evil.attacker.com/page") + val callback = mock>>() + + val handled = session.fileChooserDelegate.onShowFileChooser(callback, mock()) + + assertThat(handled).isTrue() + verify(callback).onReceiveValue(null) + } + + @Test + fun `file chooser is blocked with no page loaded`() { + whenever(webView.url).thenReturn(null) + val callback = mock>>() + + val handled = session.fileChooserDelegate.onShowFileChooser(callback, mock()) + + assertThat(handled).isTrue() + verify(callback).onReceiveValue(null) + } + + @Test + fun `file chooser passes the gate on a trusted page`() { + whenever(webView.url).thenReturn("https://37signals.com/page") + val callback = mock>>() + + // The gate passes; the chooser then fails to open because no visit + // destination exists, which reports "not handled". + val handled = session.fileChooserDelegate.onShowFileChooser(callback, params()) + + assertThat(handled).isFalse() + } + + @Test + fun `a second request answers the first before replacing it`() { + whenever(webView.url).thenReturn("https://37signals.com/page") + wireDestinationWithLauncher() + val first = mock>>() + val second = mock>>() + + session.fileChooserDelegate.onShowFileChooser(first, params()) + session.fileChooserDelegate.onShowFileChooser(second, params()) + + verify(first).onReceiveValue(null) + verify(second, never()).onReceiveValue(anyOrNull()) + } + + @Test + fun `picker results are dropped when the page navigated to an untrusted origin`() { + whenever(webView.url).thenReturn("https://37signals.com/page") + wireDestinationWithLauncher() + val callback = mock>>() + session.fileChooserDelegate.onShowFileChooser(callback, params()) + + whenever(webView.url).thenReturn("https://evil.attacker.com/page") + session.fileChooserDelegate.sendResult(arrayOf(Uri.parse("content://files/1"))) + + verify(callback).onReceiveValue(null) + } + + @Test + fun `picker results are delivered when the page stays trusted`() { + whenever(webView.url).thenReturn("https://37signals.com/page") + wireDestinationWithLauncher() + val callback = mock>>() + session.fileChooserDelegate.onShowFileChooser(callback, params()) + + val results = arrayOf(Uri.parse("content://files/1")) + session.fileChooserDelegate.sendResult(results) + + verify(callback).onReceiveValue(results) + } + + private fun params(): FileChooserParams = mock { + whenever(it.acceptTypes).thenReturn(arrayOf("*/*")) + whenever(it.isCaptureEnabled).thenReturn(false) + } + + private fun wireDestinationWithLauncher() { + val launcher = mock>() + val visitDestination = object : VisitDestination { + override fun isActive() = true + override fun activityResultLauncher(requestCode: Int) = launcher + override fun activityPermissionResultLauncher(requestCode: Int) = null + } + val callback = mock() + whenever(callback.visitDestination()).thenReturn(visitDestination) + session.currentVisit = Visit( + location = "https://37signals.com", + destinationIdentifier = 1, + restoreWithCachedSnapshot = false, + reload = false, + callback = callback, + identifier = "", + options = VisitOptions(), + ) } @Test diff --git a/core/src/test/kotlin/dev/hotwire/core/files/delegates/GeolocationPermissionDelegateTest.kt b/core/src/test/kotlin/dev/hotwire/core/files/delegates/GeolocationPermissionDelegateTest.kt new file mode 100644 index 00000000..4ab190aa --- /dev/null +++ b/core/src/test/kotlin/dev/hotwire/core/files/delegates/GeolocationPermissionDelegateTest.kt @@ -0,0 +1,132 @@ +package dev.hotwire.core.files.delegates + +import android.Manifest.permission.ACCESS_FINE_LOCATION +import android.content.Context +import android.os.Build +import android.webkit.GeolocationPermissions +import androidx.appcompat.app.AppCompatActivity +import androidx.test.core.app.ApplicationProvider +import com.nhaarman.mockito_kotlin.any +import com.nhaarman.mockito_kotlin.mock +import com.nhaarman.mockito_kotlin.never +import com.nhaarman.mockito_kotlin.verify +import dev.hotwire.core.config.Hotwire +import dev.hotwire.core.turbo.BaseRepositoryTest +import dev.hotwire.core.turbo.session.Session +import dev.hotwire.core.turbo.webview.HotwireWebView +import kotlinx.coroutines.ExperimentalCoroutinesApi +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.MockitoAnnotations +import org.robolectric.Robolectric.buildActivity +import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config + +@ExperimentalCoroutinesApi +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [Build.VERSION_CODES.R]) +class GeolocationPermissionDelegateTest : BaseRepositoryTest() { + @Mock + private lateinit var webView: HotwireWebView + private lateinit var activity: AppCompatActivity + private lateinit var session: Session + + @Before + override fun setup() { + super.setup() + MockitoAnnotations.openMocks(this) + + activity = buildActivity(TurboTestActivity::class.java).get() + + // The delegate picks its runtime permission from the manifest at + // construction, so declare it before the session builds the delegate. + declareInManifest(ACCESS_FINE_LOCATION) + session = Session("test", activity, webView) + + Hotwire.config.startLocationRegistry.clear() + Hotwire.config.registerStartLocation("https://37signals.com") + } + + @After + fun teardownRegisteredOrigins() { + Hotwire.config.startLocationRegistry.clear() + } + + @Test + fun `denies a request from an untrusted origin`() { + val callback = mock() + val origin = "https://evil.attacker.com" + + session.geolocationPermissionDelegate.onRequestPermission(origin, callback) + + verify(callback).invoke(origin, false, false) + } + + @Test + fun `denies a request with no origin`() { + val callback = mock() + + session.geolocationPermissionDelegate.onRequestPermission(null, callback) + + verify(callback).invoke(null, false, false) + } + + @Test + fun `grants a pending trusted request when the permission dialog resolves`() { + val callback = mock() + val origin = "https://37signals.com" + + session.geolocationPermissionDelegate.onRequestPermission(origin, callback) + session.geolocationPermissionDelegate.onActivityResult(isGranted = true) + + verify(callback).invoke(origin, true, true) + } + + @Test + fun `re-verifies the origin when the permission dialog resolves`() { + val callback = mock() + val origin = "https://37signals.com" + + session.geolocationPermissionDelegate.onRequestPermission(origin, callback) + Hotwire.config.startLocationRegistry.clear() + session.geolocationPermissionDelegate.onActivityResult(isGranted = true) + + verify(callback).invoke(origin, false, false) + } + + @Test + fun `a second request answers the first before replacing it`() { + val first = mock() + val second = mock() + val origin = "https://37signals.com" + + session.geolocationPermissionDelegate.onRequestPermission(origin, first) + session.geolocationPermissionDelegate.onRequestPermission(origin, second) + + verify(first).invoke(origin, false, false) + } + + @Test + fun `a hidden prompt drops the held request`() { + val callback = mock() + val origin = "https://37signals.com" + + session.geolocationPermissionDelegate.onRequestPermission(origin, callback) + session.geolocationPermissionDelegate.onHidePrompt() + session.geolocationPermissionDelegate.onActivityResult(isGranted = true) + + verify(callback, never()).invoke(any(), any(), any()) + } + + private fun declareInManifest(vararg permissions: String) { + val context: Context = ApplicationProvider.getApplicationContext() + val packageInfo = shadowOf(context.packageManager) + .getInternalMutablePackageInfo(context.packageName) + val existing = packageInfo.requestedPermissions ?: emptyArray() + packageInfo.requestedPermissions = (existing + permissions).distinct().toTypedArray() + } +} diff --git a/core/src/test/kotlin/dev/hotwire/core/files/delegates/WebViewPermissionDelegateTest.kt b/core/src/test/kotlin/dev/hotwire/core/files/delegates/WebViewPermissionDelegateTest.kt index 2d89106f..ef774d9a 100644 --- a/core/src/test/kotlin/dev/hotwire/core/files/delegates/WebViewPermissionDelegateTest.kt +++ b/core/src/test/kotlin/dev/hotwire/core/files/delegates/WebViewPermissionDelegateTest.kt @@ -5,12 +5,14 @@ import android.Manifest.permission.MODIFY_AUDIO_SETTINGS import android.Manifest.permission.RECORD_AUDIO import android.app.Application import android.content.Context +import android.net.Uri import android.os.Build import android.webkit.PermissionRequest import androidx.activity.result.ActivityResultLauncher import androidx.appcompat.app.AppCompatActivity import androidx.test.core.app.ApplicationProvider import com.nhaarman.mockito_kotlin.whenever +import dev.hotwire.core.config.Hotwire import dev.hotwire.core.turbo.BaseRepositoryTest import dev.hotwire.core.turbo.session.Session import dev.hotwire.core.turbo.session.SessionCallback @@ -19,11 +21,13 @@ import dev.hotwire.core.turbo.visit.VisitDestination import dev.hotwire.core.turbo.visit.VisitOptions import dev.hotwire.core.turbo.webview.HotwireWebView import kotlinx.coroutines.ExperimentalCoroutinesApi +import org.junit.After import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mock import org.mockito.Mockito.mock +import org.mockito.Mockito.never import org.mockito.Mockito.verify import org.mockito.MockitoAnnotations import org.robolectric.Robolectric.buildActivity @@ -49,6 +53,36 @@ class WebViewPermissionDelegateTest : BaseRepositoryTest() { activity = buildActivity(TurboTestActivity::class.java).get() context = ApplicationProvider.getApplicationContext() session = Session("test", activity, webView) + + Hotwire.config.startLocationRegistry.clear() + Hotwire.config.registerStartLocation("https://37signals.com") + } + + @After + fun teardownRegisteredOrigins() { + Hotwire.config.startLocationRegistry.clear() + } + + @Test + fun `denies request from an untrusted origin before any permission checks`() { + declareInManifest(RECORD_AUDIO, MODIFY_AUDIO_SETTINGS) + val request = mockRequest(PermissionRequest.RESOURCE_AUDIO_CAPTURE) + whenever(request.origin).thenReturn(Uri.parse("https://evil.attacker.com")) + + session.webViewPermissionDelegate.onRequest(request) + + verify(request).deny() + } + + @Test + fun `denies request with no origin`() { + declareInManifest(RECORD_AUDIO, MODIFY_AUDIO_SETTINGS) + val request = mockRequest(PermissionRequest.RESOURCE_AUDIO_CAPTURE) + whenever(request.origin).thenReturn(null) + + session.webViewPermissionDelegate.onRequest(request) + + verify(request).deny() } @Test @@ -179,8 +213,23 @@ class WebViewPermissionDelegateTest : BaseRepositoryTest() { // request. session.webViewPermissionDelegate.onActivityResult(mapOf(RECORD_AUDIO to true)) - verify(request, org.mockito.Mockito.never()).grant(arrayOf(PermissionRequest.RESOURCE_AUDIO_CAPTURE)) - verify(request, org.mockito.Mockito.never()).deny() + verify(request, never()).grant(arrayOf(PermissionRequest.RESOURCE_AUDIO_CAPTURE)) + verify(request, never()).deny() + } + + @Test + fun `re-verifies the origin when the permission dialog resolves`() { + declareInManifest(RECORD_AUDIO, MODIFY_AUDIO_SETTINGS) + grantRuntimePermissions(MODIFY_AUDIO_SETTINGS) + wireDestinationWithLauncher() + val request = mockRequest(PermissionRequest.RESOURCE_AUDIO_CAPTURE) + + session.webViewPermissionDelegate.onRequest(request) + Hotwire.config.startLocationRegistry.clear() + session.webViewPermissionDelegate.onActivityResult(mapOf(RECORD_AUDIO to true)) + + verify(request, never()).grant(arrayOf(PermissionRequest.RESOURCE_AUDIO_CAPTURE)) + verify(request).deny() } @Test @@ -242,6 +291,7 @@ class WebViewPermissionDelegateTest : BaseRepositoryTest() { private fun mockRequest(vararg resources: String): PermissionRequest { val request = mock(PermissionRequest::class.java) + whenever(request.origin).thenReturn(Uri.parse("https://37signals.com")) whenever(request.resources).thenReturn(resources) return request } diff --git a/core/src/test/kotlin/dev/hotwire/core/security/DefaultOriginTrustPolicyTest.kt b/core/src/test/kotlin/dev/hotwire/core/security/DefaultOriginTrustPolicyTest.kt new file mode 100644 index 00000000..3e09118d --- /dev/null +++ b/core/src/test/kotlin/dev/hotwire/core/security/DefaultOriginTrustPolicyTest.kt @@ -0,0 +1,136 @@ +package dev.hotwire.core.security + +import dev.hotwire.core.config.Hotwire +import org.junit.After +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class DefaultOriginTrustPolicyTest { + private val policy = DefaultOriginTrustPolicy + + private fun isTrustedForNavigation(location: String): Boolean { + return Origin.parseOrNull(location)?.let { policy.isTrustedForNavigation(it) } ?: false + } + + private fun isTrustedForNativeAccess(location: String): Boolean { + return Origin.parseOrNull(location)?.let { policy.isTrustedForNativeAccess(it) } ?: false + } + + @Before + fun setup() { + Hotwire.config.startLocationRegistry.clear() + Hotwire.config.registerStartLocation("https://my.app.com/start") + } + + @After + fun teardown() { + Hotwire.config.startLocationRegistry.clear() + } + + @Test + fun `same origin as a registered start location grants native access`() { + assertTrue(isTrustedForNavigation("https://my.app.com/another/page?q=1")) + assertTrue(isTrustedForNativeAccess("https://my.app.com/another/page?q=1")) + } + + @Test + fun `explicit default port is the same origin`() { + assertTrue(isTrustedForNavigation("https://my.app.com:443/page")) + assertTrue(isTrustedForNativeAccess("https://my.app.com:443/page")) + } + + @Test + fun `any registered origin grants native access, not just the first`() { + Hotwire.config.registerStartLocation("https://other.app.com/home") + + assertTrue(isTrustedForNavigation("https://other.app.com/page")) + assertTrue(isTrustedForNativeAccess("https://other.app.com/page")) + assertTrue(isTrustedForNavigation("https://my.app.com/page")) + assertTrue(isTrustedForNativeAccess("https://my.app.com/page")) + } + + @Test + fun `nothing is trusted when no start location is registered`() { + Hotwire.config.startLocationRegistry.clear() + + assertFalse(isTrustedForNavigation("https://my.app.com/page")) + assertFalse(isTrustedForNativeAccess("https://my.app.com/page")) + } + + @Test + fun `different host is not trusted`() { + assertFalse(isTrustedForNavigation("https://evil.com/page")) + assertFalse(isTrustedForNativeAccess("https://evil.com/page")) + } + + @Test + fun `subdomain of a registered host is not trusted`() { + assertFalse(isTrustedForNavigation("https://sub.my.app.com/page")) + assertFalse(isTrustedForNativeAccess("https://sub.my.app.com/page")) + } + + @Test + fun `parent domain of a registered host is not trusted`() { + assertFalse(isTrustedForNavigation("https://app.com/page")) + assertFalse(isTrustedForNativeAccess("https://app.com/page")) + } + + @Test + fun `scheme downgrade is not trusted`() { + assertFalse(isTrustedForNavigation("http://my.app.com/page")) + assertFalse(isTrustedForNativeAccess("http://my.app.com/page")) + } + + @Test + fun `different port is not trusted`() { + assertFalse(isTrustedForNavigation("https://my.app.com:8443/page")) + assertFalse(isTrustedForNativeAccess("https://my.app.com:8443/page")) + } + + @Test + fun `userinfo masquerade is not trusted`() { + assertFalse(isTrustedForNavigation("https://my.app.com@evil.com/page")) + assertFalse(isTrustedForNativeAccess("https://my.app.com@evil.com/page")) + } + + @Test + fun `non-http schemes are not trusted`() { + assertFalse(isTrustedForNavigation("javascript:alert(1)")) + assertFalse(isTrustedForNativeAccess("javascript:alert(1)")) + assertFalse(isTrustedForNavigation("about:blank")) + assertFalse(isTrustedForNativeAccess("about:blank")) + assertFalse(isTrustedForNavigation("file:///etc/hosts")) + assertFalse(isTrustedForNativeAccess("file:///etc/hosts")) + } + + @Test + fun `a policy that adds a partner for navigation keeps the defaults for the rest`() { + val partner = Origin.parse("https://partner.example.com") + val policy = object : OriginTrustPolicy() { + override fun isTrustedForNavigation(origin: Origin) = + origin == partner || DefaultOriginTrustPolicy.isTrustedForNavigation(origin) + + override fun isTrustedForNativeAccess(origin: Origin) = + DefaultOriginTrustPolicy.isTrustedForNativeAccess(origin) + } + + assertTrue(policy.isTrustedForNavigation(partner)) + assertFalse(policy.isTrustedForNativeAccess(partner)) + assertFalse(policy.isTrustedForNavigation(Origin.parse("http://partner.example.com"))) + assertTrue(policy.isTrustedForNavigation(Origin.parse("https://my.app.com"))) + assertTrue(policy.isTrustedForNativeAccess(Origin.parse("https://my.app.com"))) + } + + @Test + fun `unparseable locations are not trusted`() { + assertFalse(isTrustedForNavigation("not a url")) + assertFalse(isTrustedForNativeAccess("not a url")) + assertFalse(isTrustedForNavigation("")) + assertFalse(isTrustedForNativeAccess("")) + } +} diff --git a/core/src/test/kotlin/dev/hotwire/core/security/JavascriptChannelTest.kt b/core/src/test/kotlin/dev/hotwire/core/security/JavascriptChannelTest.kt new file mode 100644 index 00000000..01d029ca --- /dev/null +++ b/core/src/test/kotlin/dev/hotwire/core/security/JavascriptChannelTest.kt @@ -0,0 +1,60 @@ +package dev.hotwire.core.security + +import dev.hotwire.core.config.Hotwire +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class JavascriptChannelTest { + private val received = mutableListOf() + private val channel = JavascriptChannel("TestChannel") { received.add(it) } + + @Before + fun setup() { + Hotwire.config.startLocationRegistry.clear() + Hotwire.config.registerStartLocation("https://my.app.com/start") + } + + @After + fun teardown() { + Hotwire.config.startLocationRegistry.clear() + } + + @Test + fun `a message from a trusted main frame is decoded and delivered`() { + channel.receive("""{"name":"ping","args":["a",1,true]}""", "https://my.app.com", isMainFrame = true) + + assertEquals(listOf("ping"), received.map { it.name }) + assertEquals("a", received.single().args.stringAt(0)) + assertEquals(1, received.single().args.intAt(1)) + assertEquals(true, received.single().args.booleanAt(2)) + } + + @Test + fun `messages from untrusted, opaque, or sub-frame sources are not delivered`() { + channel.receive("""{"name":"ping"}""", "https://evil.attacker.com", isMainFrame = true) + channel.receive("""{"name":"ping"}""", "null", isMainFrame = true) + channel.receive("""{"name":"ping"}""", "https://my.app.com", isMainFrame = false) + + assertEquals(emptyList(), received) + } + + @Test + fun `malformed messages are not delivered`() { + channel.receive("not json", "https://my.app.com", isMainFrame = true) + channel.receive("""{"args":[]}""", "https://my.app.com", isMainFrame = true) + + assertEquals(emptyList(), received) + } + + @Test + fun `a handler failure does not escape the channel`() { + val failing = JavascriptChannel("TestChannel") { it.args.stringAt(0) } + + failing.receive("""{"name":"ping","args":[]}""", "https://my.app.com", isMainFrame = true) + } +} diff --git a/core/src/test/kotlin/dev/hotwire/core/security/OriginTest.kt b/core/src/test/kotlin/dev/hotwire/core/security/OriginTest.kt new file mode 100644 index 00000000..fa6861be --- /dev/null +++ b/core/src/test/kotlin/dev/hotwire/core/security/OriginTest.kt @@ -0,0 +1,81 @@ +package dev.hotwire.core.security + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +class OriginTest { + @Test + fun `https without an explicit port defaults to 443`() { + assertEquals(443, Origin.parse("https://a.com/path").port) + } + + @Test + fun `an explicit port is kept`() { + assertEquals(8443, Origin.parse("https://a.com:8443/path").port) + } + + @Test + fun `http without an explicit port defaults to 80`() { + assertEquals(80, Origin.parse("http://a.com/path").port) + } + + @Test + fun `scheme and host are lowercased`() { + val origin = Origin.parse("HTTPS://My.App.COM/Path") + + assertEquals("https", origin.scheme) + assertEquals("my.app.com", origin.host) + assertEquals(Origin.parse("https://my.app.com"), origin) + } + + @Test + fun `non-http schemes do not parse`() { + assertNull(Origin.parseOrNull("about:blank")) + assertNull(Origin.parseOrNull("javascript:alert(1)")) + assertNull(Origin.parseOrNull("file:///x")) + } + + @Test + fun `unparseable locations do not parse`() { + assertNull(Origin.parseOrNull("not a url")) + } + + @Test(expected = IllegalArgumentException::class) + fun `parse throws for a non-http location`() { + Origin.parse("javascript:alert(1)") + } + + @Test + fun `path, query, and fragment are ignored`() { + val origin = Origin.parse("https://a.com") + + assertEquals(origin, Origin.parseOrNull("https://a.com/path")) + assertEquals(origin, Origin.parseOrNull("https://a.com/path?q=1")) + assertEquals(origin, Origin.parseOrNull("https://a.com/path#fragment")) + } + + @Test + fun `equality holds across an implicit and an explicit default port`() { + assertEquals(Origin.parse("https://a.com"), Origin.parse("https://a.com:443/path")) + } + + @Test + fun `toString omits a default port`() { + assertEquals("https://a.com", Origin.parse("https://a.com:443/path").toString()) + } + + @Test + fun `toString keeps a non-default port`() { + assertEquals("https://a.com:8443", Origin.parse("https://a.com:8443/path").toString()) + } + + @Test + fun `toString brackets an IPv6 host`() { + val origin = Origin.parse("https://[2001:db8::1]:8443/path") + + assertEquals("2001:db8::1", origin.host) + assertEquals("https://[2001:db8::1]:8443", origin.toString()) + assertEquals(origin, Origin.parseOrNull(origin.toString())) + } +} diff --git a/core/src/test/kotlin/dev/hotwire/core/security/OriginsTest.kt b/core/src/test/kotlin/dev/hotwire/core/security/OriginsTest.kt new file mode 100644 index 00000000..d44e9de6 --- /dev/null +++ b/core/src/test/kotlin/dev/hotwire/core/security/OriginsTest.kt @@ -0,0 +1,73 @@ +package dev.hotwire.core.security + +import dev.hotwire.core.config.Hotwire +import org.junit.After +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class OriginsTest { + @After + fun teardown() { + Hotwire.config.originTrustPolicy = DefaultOriginTrustPolicy + } + + @Test + fun `hasSameOriginAs is true for locations sharing scheme, host, and port`() { + assertTrue("https://a.com/one".hasSameOriginAs("https://a.com:443/two")) + } + + @Test + fun `hasSameOriginAs is false for a different scheme, host, or port`() { + assertFalse("https://a.com/one".hasSameOriginAs("http://a.com/one")) + assertFalse("https://a.com/one".hasSameOriginAs("https://b.com/one")) + assertFalse("https://a.com/one".hasSameOriginAs("https://a.com:8443/one")) + } + + @Test + fun `hasSameOriginAs is false when either side does not parse`() { + assertFalse("https://a.com".hasSameOriginAs("not a url")) + assertFalse("not a url".hasSameOriginAs("https://a.com")) + } + + @Test + fun `isTrustedForNavigation reflects the policy's navigation answer, not its native access answer`() { + Hotwire.config.originTrustPolicy = recordingPolicy(navigation = true, nativeAccess = false) + + assertTrue(isTrustedForNavigation("https://a.com")) + assertFalse(isTrustedForNativeAccess("https://a.com")) + } + + @Test + fun `isTrustedForNativeAccess needs both of the policy's answers`() { + Hotwire.config.originTrustPolicy = recordingPolicy(navigation = true, nativeAccess = true) + assertTrue(isTrustedForNativeAccess("https://a.com")) + + Hotwire.config.originTrustPolicy = recordingPolicy(navigation = false, nativeAccess = true) + assertFalse(isTrustedForNavigation("https://a.com")) + assertFalse(isTrustedForNativeAccess("https://a.com")) + } + + @Test + fun `a non-http location fails closed without consulting the policy`() { + Hotwire.config.originTrustPolicy = throwingPolicy() + + assertFalse(isTrustedForNavigation("not a url")) + assertFalse(isTrustedForNavigation("javascript:alert(1)")) + assertFalse(isTrustedForNativeAccess("not a url")) + assertFalse(isTrustedForNativeAccess("javascript:alert(1)")) + } + + private fun recordingPolicy(navigation: Boolean, nativeAccess: Boolean) = object : OriginTrustPolicy() { + override fun isTrustedForNavigation(origin: Origin) = navigation + override fun isTrustedForNativeAccess(origin: Origin) = nativeAccess + } + + private fun throwingPolicy() = object : OriginTrustPolicy() { + override fun isTrustedForNavigation(origin: Origin): Boolean = error("policy should not be consulted") + override fun isTrustedForNativeAccess(origin: Origin): Boolean = error("policy should not be consulted") + } +} diff --git a/core/src/test/kotlin/dev/hotwire/core/security/StartLocationRegistryTest.kt b/core/src/test/kotlin/dev/hotwire/core/security/StartLocationRegistryTest.kt new file mode 100644 index 00000000..d778b733 --- /dev/null +++ b/core/src/test/kotlin/dev/hotwire/core/security/StartLocationRegistryTest.kt @@ -0,0 +1,99 @@ +package dev.hotwire.core.security + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class StartLocationRegistryTest { + private val registry = StartLocationRegistry() + private val myAppOrigin = Origin.parse("https://my.app.com") + + @Test + fun `a registered start location's origin is present`() { + registry.register("https://my.app.com/start") + + assertTrue(myAppOrigin in registry.origins) + } + + @Test + fun `unregistering a host's location removes its origin`() { + registry.register("https://my.app.com/start") + registry.unregister("https://my.app.com/start") + + assertFalse(myAppOrigin in registry.origins) + } + + @Test + fun `a shared origin survives until its last registration is withdrawn`() { + registry.register("https://my.app.com/start") + registry.register("https://my.app.com/other-start") + + registry.unregister("https://my.app.com/start") + assertTrue(myAppOrigin in registry.origins) + + registry.unregister("https://my.app.com/other-start") + assertFalse(myAppOrigin in registry.origins) + } + + @Test + fun `host recreation re-registers the same origin without losing it`() { + registry.register("https://my.app.com/start") + + registry.unregister("https://my.app.com/start") + registry.register("https://my.app.com/start") + + assertTrue(myAppOrigin in registry.origins) + } + + @Test + fun `a non-http location never registers`() { + registry.register("file:///sdcard/start") + registry.register("not a url") + + assertEquals(emptySet(), registry.origins) + } + + @Test + fun `unregistering an unknown location is a no-op`() { + registry.register("https://my.app.com/start") + registry.unregister("https://other.app.com/start") + registry.unregister("not a url") + + assertTrue(myAppOrigin in registry.origins) + } + + @Test + fun `clear withdraws every registration`() { + registry.register("https://my.app.com/start") + registry.register("https://other.app.com/start") + + registry.clear() + + assertEquals(emptySet(), registry.origins) + } + + @Test + fun `origins is a live view`() { + val origins = registry.origins + + registry.register("https://my.app.com/start") + + assertEquals(setOf(myAppOrigin), origins) + } + + @Test + fun `origins reports bare origins`() { + registry.register("https://my.app.com/start?utm=1") + + assertEquals(setOf(myAppOrigin), registry.origins) + } + + @Test + fun `an unregistered origin is absent`() { + assertFalse(myAppOrigin in registry.origins) + } +} diff --git a/core/src/test/kotlin/dev/hotwire/core/turbo/errors/VisitErrorTest.kt b/core/src/test/kotlin/dev/hotwire/core/turbo/errors/VisitErrorTest.kt new file mode 100644 index 00000000..d2f21ac9 --- /dev/null +++ b/core/src/test/kotlin/dev/hotwire/core/turbo/errors/VisitErrorTest.kt @@ -0,0 +1,20 @@ +package dev.hotwire.core.turbo.errors + +import org.junit.Assert.assertEquals +import org.junit.Test + +class VisitErrorTest { + @Test + @Suppress("DEPRECATION") + fun `the deprecated description function still resolves to the property`() { + val errors: List = listOf( + LoadError.NotPresent, + HttpError.ClientError.NotFound, + HttpError.UnknownError(599, null), + WebError.Unknown, + WebSslError.NotYetValid + ) + + errors.forEach { assertEquals(it.description, it.description()) } + } +} diff --git a/core/src/test/kotlin/dev/hotwire/core/turbo/session/SessionTest.kt b/core/src/test/kotlin/dev/hotwire/core/turbo/session/SessionTest.kt index c4f8c9b5..a2b5e81f 100644 --- a/core/src/test/kotlin/dev/hotwire/core/turbo/session/SessionTest.kt +++ b/core/src/test/kotlin/dev/hotwire/core/turbo/session/SessionTest.kt @@ -1,10 +1,17 @@ package dev.hotwire.core.turbo.session import android.os.Build +import android.webkit.HttpAuthHandler +import android.webkit.WebViewClient import androidx.appcompat.app.AppCompatActivity +import com.nhaarman.mockito_kotlin.any +import com.nhaarman.mockito_kotlin.argumentCaptor +import com.nhaarman.mockito_kotlin.mock import com.nhaarman.mockito_kotlin.never import com.nhaarman.mockito_kotlin.times import com.nhaarman.mockito_kotlin.whenever +import dev.hotwire.core.config.Hotwire +import dev.hotwire.core.security.DefaultOriginTrustPolicy import dev.hotwire.core.turbo.BaseRepositoryTest import dev.hotwire.core.turbo.errors.HttpError import dev.hotwire.core.turbo.errors.HttpError.ServerError @@ -17,7 +24,12 @@ import dev.hotwire.core.turbo.visit.VisitDestination import dev.hotwire.core.turbo.visit.VisitOptions import dev.hotwire.core.turbo.webview.HotwireWebView import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.serialization.json.add +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import kotlinx.serialization.json.putJsonArray import org.assertj.core.api.Assertions.assertThat +import org.junit.After import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -50,7 +62,11 @@ class SessionTest : BaseRepositoryTest() { MockitoAnnotations.openMocks(this) activity = buildActivity(TurboTestActivity::class.java).get() + Hotwire.config.startLocationRegistry.clear() + Hotwire.config.registerStartLocation(baseUrl()) session = Session("test", activity, webView) + session.turboSessionChannel.isInstalled = true + whenever(webView.url).thenReturn(baseUrl()) visit = Visit( location = baseUrl(), destinationIdentifier = 1, @@ -70,6 +86,12 @@ class SessionTest : BaseRepositoryTest() { whenever(callback.visitDestination()).thenReturn(visitDestination) } + @After + fun teardownRegisteredOrigins() { + Hotwire.config.startLocationRegistry.clear() + Hotwire.config.originTrustPolicy = DefaultOriginTrustPolicy + } + @Test fun `session is always new instance`() { val session = Session("test", activity, webView) @@ -84,11 +106,138 @@ class SessionTest : BaseRepositoryTest() { val newLocation = "${visit.location}/page" session.currentVisit = visit - session.visitProposedToLocation(newLocation, options.toJson()) + session.visitProposedToLocation(newLocation, options) verify(callback).visitProposedToLocation(newLocation, options) } + @Test + fun `turbo session messages from an untrusted origin are dropped`() { + session.currentVisit = visit + + listOf( + envelope("visitProposedToLocation", "${visit.location}/page", VisitOptions().toJson()), + envelope("turboIsReady", true), + envelope("visitStarted", "12345", true, false, visit.location) + ).forEach { + session.turboSessionChannel.receive(it, sourceOrigin = "https://evil.attacker.com", isMainFrame = true) + } + + verify(callback, never()).visitProposedToLocation(any(), any()) + assertThat(session.isReady).isFalse() + assertThat(session.currentVisit?.identifier).isEmpty() + } + + @Test + fun `turbo session messages from a sub frame are dropped`() { + session.currentVisit = visit + + session.turboSessionChannel.receive( + envelope("visitProposedToLocation", "${visit.location}/page", VisitOptions().toJson()), + sourceOrigin = baseUrl(), + isMainFrame = false + ) + + verify(callback, never()).visitProposedToLocation(any(), any()) + } + + @Test + fun `turbo session messages from a trusted main frame are dispatched`() { + val options = VisitOptions() + val newLocation = "${visit.location}/page" + session.currentVisit = visit + + session.turboSessionChannel.receive( + envelope("visitProposedToLocation", newLocation, options.toJson()), + sourceOrigin = baseUrl(), + isMainFrame = true + ) + + verify(callback).visitProposedToLocation(newLocation, options) + } + + @Test + fun `malformed turbo session messages are dropped`() { + session.currentVisit = visit + + listOf( + "not json", + """{"args":[]}""", + envelope("noSuchMethod"), + envelope("visitProposedToLocation") + ).forEach { + session.turboSessionChannel.receive(it, sourceOrigin = baseUrl(), isMainFrame = true) + } + + verify(callback, never()).visitProposedToLocation(any(), any()) + } + + private fun envelope(name: String, vararg args: Any): String { + return buildJsonObject { + put("name", name) + putJsonArray("args") { + args.forEach { + when (it) { + is Boolean -> add(it) + is Number -> add(it) + else -> add(it.toString()) + } + } + } + }.toString() + } + + @Test + fun `cold boot page finished on an untrusted origin surfaces an error and resets`() { + session.currentVisit = visit + session.isColdBooting = true + + webViewClient().onPageFinished(webView, "https://evil.attacker.com/page") + + verify(callback).onReceivedError(LoadError.UntrustedOrigin("https://evil.attacker.com/page")) + assertThat(session.isColdBooting).isFalse() + assertThat(session.coldBootVisitIdentifier).isEmpty() + } + + @Test + fun `cold boot without the message channel surfaces an unsupported error`() { + session.turboSessionChannel.isInstalled = false + session.currentVisit = visit + session.isColdBooting = true + + webViewClient().onPageFinished(webView, "${visit.location}/page") + + verify(callback).onReceivedError(LoadError.WebViewNotSupported) + assertThat(session.isColdBooting).isFalse() + } + + @Test + fun `cold boot page finished on a trusted origin does not surface an error`() { + session.currentVisit = visit + session.isColdBooting = true + + webViewClient().onPageFinished(webView, "${visit.location}/page") + + verify(callback, never()).onReceivedError(any()) + } + + private fun webViewClient(): WebViewClient { + val captor = argumentCaptor() + verify(webView).webViewClient = captor.capture() + return captor.lastValue + } + + @Test + fun `http auth challenges are forwarded to the callback`() { + val handler: HttpAuthHandler = mock() + session.currentVisit = visit + + webViewClient().onReceivedHttpAuthRequest(webView, handler, "37signals.com", "realm") + + verify(callback).onReceivedHttpAuthRequest(handler, "37signals.com", "realm") + verify(handler, never()).cancel() + } + @Test fun `visit started saves current visit identifier`() { val visitIdentifier = "12345" diff --git a/navigation-fragments/src/main/java/dev/hotwire/navigation/fragments/HotwireWebBottomSheetFragment.kt b/navigation-fragments/src/main/java/dev/hotwire/navigation/fragments/HotwireWebBottomSheetFragment.kt index 18110e99..78dc8d38 100644 --- a/navigation-fragments/src/main/java/dev/hotwire/navigation/fragments/HotwireWebBottomSheetFragment.kt +++ b/navigation-fragments/src/main/java/dev/hotwire/navigation/fragments/HotwireWebBottomSheetFragment.kt @@ -157,7 +157,7 @@ open class HotwireWebBottomSheetFragment : HotwireBottomSheetFragment(), Hotwire @SuppressLint("InflateParams") override fun createErrorView(error: VisitError): View { return layoutInflater.inflate(R.layout.hotwire_error, null).apply { - findViewById(R.id.hotwire_error_description).text = error.description() + findViewById(R.id.hotwire_error_description).text = error.description } } diff --git a/navigation-fragments/src/main/java/dev/hotwire/navigation/fragments/HotwireWebFragment.kt b/navigation-fragments/src/main/java/dev/hotwire/navigation/fragments/HotwireWebFragment.kt index 791bf430..f8da4ace 100644 --- a/navigation-fragments/src/main/java/dev/hotwire/navigation/fragments/HotwireWebFragment.kt +++ b/navigation-fragments/src/main/java/dev/hotwire/navigation/fragments/HotwireWebFragment.kt @@ -175,7 +175,7 @@ open class HotwireWebFragment : HotwireFragment(), HotwireWebFragmentCallback { @SuppressLint("InflateParams") override fun createErrorView(error: VisitError): View { return layoutInflater.inflate(R.layout.hotwire_error, null).apply { - findViewById(R.id.hotwire_error_description).text = error.description() + findViewById(R.id.hotwire_error_description).text = error.description } } diff --git a/navigation-fragments/src/main/java/dev/hotwire/navigation/fragments/HotwireWebFragmentCallback.kt b/navigation-fragments/src/main/java/dev/hotwire/navigation/fragments/HotwireWebFragmentCallback.kt index 7e84d67b..e486e1bb 100644 --- a/navigation-fragments/src/main/java/dev/hotwire/navigation/fragments/HotwireWebFragmentCallback.kt +++ b/navigation-fragments/src/main/java/dev/hotwire/navigation/fragments/HotwireWebFragmentCallback.kt @@ -97,6 +97,8 @@ interface HotwireWebFragmentCallback { /** * Called when the WebView has received an HTTP authentication request. + * [host] can be any server the page loads resources from, and it receives + * the credentials you pass to [HttpAuthHandler.proceed], so check it first. */ fun onReceivedHttpAuthRequest(handler: HttpAuthHandler, host: String, realm: String) { handler.cancel() diff --git a/navigation-fragments/src/main/java/dev/hotwire/navigation/navigator/NavigatorHost.kt b/navigation-fragments/src/main/java/dev/hotwire/navigation/navigator/NavigatorHost.kt index 3570c538..692366f7 100644 --- a/navigation-fragments/src/main/java/dev/hotwire/navigation/navigator/NavigatorHost.kt +++ b/navigation-fragments/src/main/java/dev/hotwire/navigation/navigator/NavigatorHost.kt @@ -4,13 +4,13 @@ import android.os.Bundle import android.view.View import androidx.annotation.VisibleForTesting import androidx.annotation.VisibleForTesting.Companion.PROTECTED -import androidx.core.net.toUri import androidx.fragment.app.Fragment import androidx.fragment.app.FragmentManager import androidx.fragment.app.FragmentOnAttachListener import androidx.navigation.fragment.NavHostFragment import androidx.navigation.fragment.findNavController import dev.hotwire.core.config.Hotwire +import dev.hotwire.core.security.hasSameOriginAs import dev.hotwire.navigation.activities.HotwireActivity import dev.hotwire.navigation.config.HotwireNavigation @@ -30,10 +30,16 @@ open class NavigatorHost : NavHostFragment(), FragmentOnAttachListener { internal var isGraphInitialized = false private set + // The configuration's start location can change before onDestroy. + private var registeredStartLocation: String? = null + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) activity = requireActivity() as HotwireActivity + registeredStartLocation = configuration.startLocation.also { + Hotwire.config.registerStartLocation(it) + } navigator = Navigator(this, configuration, activity) childFragmentManager.addFragmentOnAttachListener(this) } @@ -49,6 +55,8 @@ open class NavigatorHost : NavHostFragment(), FragmentOnAttachListener { } override fun onDestroy() { + registeredStartLocation?.let { Hotwire.config.unregisterStartLocation(it) } + registeredStartLocation = null activity.delegate.unregisterNavigatorHost(this) super.onDestroy() } @@ -100,7 +108,7 @@ open class NavigatorHost : NavHostFragment(), FragmentOnAttachListener { * Google's Navigation library automatically navigates to deep links provided in the launching * Intent, which lets a malicious Intent open an arbitrary page in the WebView. Sanitize the * Intent's attacker-controllable deep-link arguments so the start location stays within the - * app's domain. + * app's origin. */ @VisibleForTesting(otherwise = PROTECTED) fun ensureDeeplinkStartLocationValid() { @@ -115,10 +123,7 @@ open class NavigatorHost : NavHostFragment(), FragmentOnAttachListener { val extrasBundle = intent.extras?.getBundle(DEEPLINK_EXTRAS_KEY) ?: return val startLocation = extrasBundle.getString(LOCATION_KEY) ?: return - val deepLinkStartUri = startLocation.toUri() - val configStartUri = configuration.startLocation.toUri() - - if (deepLinkStartUri.host != configStartUri.host) { + if (!startLocation.hasSameOriginAs(configuration.startLocation)) { extrasBundle.putString(LOCATION_KEY, configuration.startLocation) intent.putExtra(DEEPLINK_EXTRAS_KEY, extrasBundle) } diff --git a/navigation-fragments/src/main/java/dev/hotwire/navigation/routing/AppNavigationRouteDecisionHandler.kt b/navigation-fragments/src/main/java/dev/hotwire/navigation/routing/AppNavigationRouteDecisionHandler.kt index 5981438c..6d11fcc3 100644 --- a/navigation-fragments/src/main/java/dev/hotwire/navigation/routing/AppNavigationRouteDecisionHandler.kt +++ b/navigation-fragments/src/main/java/dev/hotwire/navigation/routing/AppNavigationRouteDecisionHandler.kt @@ -1,12 +1,15 @@ package dev.hotwire.navigation.routing -import androidx.core.net.toUri +import dev.hotwire.core.security.isTrustedForNavigation import dev.hotwire.core.turbo.visit.VisitProposal import dev.hotwire.navigation.activities.HotwireActivity import dev.hotwire.navigation.navigator.NavigatorConfiguration /** - * Navigates internal urls through in-app routing. + * Navigates urls in-app when + * [dev.hotwire.core.security.OriginTrustPolicy.isTrustedForNavigation] trusts + * their origin. The default trusts every registered start location, so a + * navigator also opens other navigators' origins in-app. */ class AppNavigationRouteDecisionHandler : Router.RouteDecisionHandler { override val name = "app-navigation" @@ -15,7 +18,7 @@ class AppNavigationRouteDecisionHandler : Router.RouteDecisionHandler { proposal: VisitProposal, configuration: NavigatorConfiguration ): Boolean { - return configuration.startLocation.toUri().host == proposal.location.toUri().host + return isTrustedForNavigation(proposal.location) } override fun handle( diff --git a/navigation-fragments/src/test/kotlin/dev/hotwire/navigation/navigator/NavigatorHostTest.kt b/navigation-fragments/src/test/kotlin/dev/hotwire/navigation/navigator/NavigatorHostTest.kt index 6165ae6e..97807722 100644 --- a/navigation-fragments/src/test/kotlin/dev/hotwire/navigation/navigator/NavigatorHostTest.kt +++ b/navigation-fragments/src/test/kotlin/dev/hotwire/navigation/navigator/NavigatorHostTest.kt @@ -50,6 +50,48 @@ class NavigatorHostTest { .isEqualTo("https://example.com/path") } + @Test + fun `reverts to config start location when deep link downgrades the scheme`() { + val intent = Intent().apply { + putExtra(DEEPLINK_EXTRAS_KEY, bundleOf(LOCATION_KEY to "http://example.com/path")) + } + activity = Robolectric.buildActivity(TestActivity::class.java, intent).get() + + host.activity = activity + host.ensureDeeplinkStartLocationValid() + + assertThat(activity.intent.getBundleExtra(DEEPLINK_EXTRAS_KEY)?.getString(LOCATION_KEY)) + .isEqualTo("https://example.com/start") + } + + @Test + fun `reverts to config start location when deep link uses another port`() { + val intent = Intent().apply { + putExtra(DEEPLINK_EXTRAS_KEY, bundleOf(LOCATION_KEY to "https://example.com:8443/path")) + } + activity = Robolectric.buildActivity(TestActivity::class.java, intent).get() + + host.activity = activity + host.ensureDeeplinkStartLocationValid() + + assertThat(activity.intent.getBundleExtra(DEEPLINK_EXTRAS_KEY)?.getString(LOCATION_KEY)) + .isEqualTo("https://example.com/start") + } + + @Test + fun `reverts to config start location when deep link does not parse as a web URL`() { + val intent = Intent().apply { + putExtra(DEEPLINK_EXTRAS_KEY, bundleOf(LOCATION_KEY to "javascript:alert(1)")) + } + activity = Robolectric.buildActivity(TestActivity::class.java, intent).get() + + host.activity = activity + host.ensureDeeplinkStartLocationValid() + + assertThat(activity.intent.getBundleExtra(DEEPLINK_EXTRAS_KEY)?.getString(LOCATION_KEY)) + .isEqualTo("https://example.com/start") + } + // NavController merges deepLinkArgs over deepLinkExtras (last write wins); the intent's args // must not survive to override the validated start location. @Test diff --git a/navigation-fragments/src/test/kotlin/dev/hotwire/navigation/routing/AppNavigationRouteDecisionHandlerTest.kt b/navigation-fragments/src/test/kotlin/dev/hotwire/navigation/routing/AppNavigationRouteDecisionHandlerTest.kt index 2eec1611..250d4ee7 100644 --- a/navigation-fragments/src/test/kotlin/dev/hotwire/navigation/routing/AppNavigationRouteDecisionHandlerTest.kt +++ b/navigation-fragments/src/test/kotlin/dev/hotwire/navigation/routing/AppNavigationRouteDecisionHandlerTest.kt @@ -1,5 +1,9 @@ package dev.hotwire.navigation.routing +import dev.hotwire.core.config.Hotwire +import dev.hotwire.core.security.DefaultOriginTrustPolicy +import dev.hotwire.core.security.Origin +import dev.hotwire.core.security.OriginTrustPolicy import dev.hotwire.core.turbo.config.PathConfigurationProperties import dev.hotwire.core.turbo.visit.VisitOptions import dev.hotwire.core.turbo.visit.VisitProposal @@ -8,6 +12,7 @@ import dev.hotwire.navigation.navigator.NavigatorConfiguration import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue +import org.junit.After import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -28,6 +33,13 @@ class AppNavigationRouteDecisionHandlerTest { @Before fun setup() { activity = buildActivity(TestActivity::class.java).get() + Hotwire.config.registerStartLocation(config.startLocation) + } + + @After + fun teardown() { + Hotwire.config.unregisterStartLocation(config.startLocation) + Hotwire.config.originTrustPolicy = DefaultOriginTrustPolicy } @Test @@ -54,6 +66,41 @@ class AppNavigationRouteDecisionHandlerTest { assertFalse(route.matches(proposal(url), config)) } + @Test + fun `http url on the app domain does not match`() { + val url = "http://my.app.com/page" + assertFalse(route.matches(proposal(url), config)) + } + + @Test + fun `url on another port does not match`() { + val url = "https://my.app.com:8443/page" + assertFalse(route.matches(proposal(url), config)) + } + + @Test + fun `url on another navigator's start origin matches`() { + val otherStartLocation = "https://other.app.com/start" + Hotwire.config.registerStartLocation(otherStartLocation) + + try { + assertTrue(route.matches(proposal("https://other.app.com/page"), config)) + } finally { + Hotwire.config.unregisterStartLocation(otherStartLocation) + } + } + + @Test + fun `a custom origin trust policy decides the match`() { + Hotwire.config.originTrustPolicy = object : OriginTrustPolicy() { + override fun isTrustedForNavigation(origin: Origin) = origin.host == "asset.cdn.com" + override fun isTrustedForNativeAccess(origin: Origin) = false + } + + assertTrue(route.matches(proposal("https://asset.cdn.com/image.png"), config)) + assertFalse(route.matches(proposal(config.startLocation), config)) + } + private fun proposal(location: String) = VisitProposal( location = location, options = VisitOptions(),