diff --git a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt index 1607fb4db..88bf6fb74 100644 --- a/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt +++ b/android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt @@ -111,8 +111,9 @@ class GutenbergView : FrameLayout { private var hasAutofocused = false private lateinit var assetLoader: WebViewAssetLoader private lateinit var assetAuthority: String + private lateinit var assetScheme: String - /** The editor document [loadEditor] loaded, so its load failures can be told apart. */ + /** The editor document [loadEditor] loaded, the only page given the editor globals. */ private var editorUri: Uri? = null private val configuration: EditorConfiguration private lateinit var dependencies: EditorDependencies @@ -474,7 +475,7 @@ class GutenbergView : FrameLayout { override fun onPageStarted(view: WebView?, url: String?, favicon: Bitmap?) { super.onPageStarted(view, url, favicon) - onEditorPageStarted() + onEditorPageStarted(url) } override fun shouldInterceptRequest( @@ -526,28 +527,17 @@ class GutenbergView : FrameLayout { // Allow asset URLs (restrict to the asset path prefix so that // arbitrary site pages don't load inside the WebView when the // asset authority matches the site authority) - if (url.authority == assetAuthority && url.path?.startsWith("/assets/") == true) { + if (isAssetUrl(url)) { return false } - // Allow WordPress.com REST API - if (url.host == "public-api.wordpress.com") { - return false - } - - // Allow WordPress REST API - if (url.authority == originAuthority(configuration.siteApiRoot)) { - if (url.path?.contains("/wp-json/") == true || url.query?.contains("rest_route=") == true) { - return false - } - } - // Allow local development server if configured if (isDevServerUrl(url, BuildConfig.GUTENBERG_EDITOR_URL)) { return false } - // For all other URLs, open in external browser + // For all other URLs, open in external browser. This includes the site's + // REST API: the editor reaches it by fetch, which never passes through here. val intent = Intent(Intent.ACTION_VIEW, url) intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) view?.context?.startActivity(intent) @@ -653,6 +643,15 @@ class GutenbergView : FrameLayout { } } + /** + * Whether [url] is an asset [assetLoader] serves over the scheme the editor loads + * with. On an https site, http on the same authority reaches the site instead. + */ + private fun isAssetUrl(url: Uri): Boolean = + url.scheme == assetScheme && + url.authority == assetAuthority && + url.path?.startsWith("/assets/") == true + /** * Loads the editor with the given dependencies. * @@ -683,6 +682,7 @@ class GutenbergView : FrameLayout { // avoid accidentally downgrading asset traffic for production sites. val siteUri = Uri.parse(configuration.siteURL) val isLocalHttpSite = siteUri.scheme == "http" && siteUri.host in LOCAL_HOSTS + assetScheme = if (isLocalHttpSite) "http" else "https" assetLoader = WebViewAssetLoader.Builder() .setDomain(assetAuthority) .setHttpAllowed(isLocalHttpSite) @@ -694,8 +694,7 @@ class GutenbergView : FrameLayout { initializeWebView() - val scheme = if (isLocalHttpSite) "http" else "https" - val assetUrl = "$scheme://$assetAuthority$ASSET_PATH_INDEX" + val assetUrl = "$assetScheme://$assetAuthority$ASSET_PATH_INDEX" val editorUrl = BuildConfig.GUTENBERG_EDITOR_URL.ifEmpty { assetUrl } @@ -721,20 +720,28 @@ class GutenbergView : FrameLayout { } /** - * Invoked when the editor page begins loading. Starts the upload server once — - * capturing the [mediaUploadDelegate] provided before load — then advertises - * the editor globals (including the server's port and token) to the page. + * Invoked when any page begins loading in the main frame. Resets readiness for + * every page; for the editor document alone, starts the upload server once — + * capturing the [mediaUploadDelegate] provided before load — then advertises the + * editor globals (including the server's port and token). * * Starting the server here, on the UI thread, rather than from the * [mediaUploadDelegate] setter keeps its whole lifecycle — start here, stop in * [onDetachedFromWindow] — on the UI thread, so it can't race a * background-thread delegate assignment. */ - private fun onEditorPageStarted() { + private fun onEditorPageStarted(url: String?) { // Readiness belongs to the page: a new page, including one a reload starts, // is not ready until it reports `onEditorLoaded`. isEditorLoaded = false didFireEditorLoaded = false + + // The globals carry the site credential and the upload server's token, so + // they go to the editor document alone. `shouldOverrideUrlLoading` admits + // other pages into this frame, and on Android the editor shares an origin + // with the site, so the destination is checked rather than assumed. + if (url == null || !isEditorDocument(Uri.parse(url))) return + if (!hasStartedLoading) { hasStartedLoading = true startUploadServer() @@ -1417,9 +1424,10 @@ class GutenbergView : FrameLayout { * * This deliberately does not use [Uri.authority], which returns whatever the * URL was written with. Chromium canonicalizes a URL before it reaches - * [WebResourceRequest.url], dropping a default port and any userinfo, so - * `https://example.com:443` arrives as `example.com`. Comparing that against - * a raw authority of `example.com:443` would never match — and since + * [WebResourceRequest.url], lowercasing the host and dropping a default port + * and any userinfo, so `https://Example.com:443` arrives as `example.com`. + * Comparing that against a raw authority of `Example.com:443` would never + * match — and since * `WebViewAssetLoader.PathMatcher` compares authorities exactly, the bundled * editor document would not be served at all. * @@ -1435,7 +1443,7 @@ class GutenbergView : FrameLayout { // supports reaching, and the authority is at least well-formed. if (authority.startsWith("[")) return authority - val host = uri.host ?: return null + val host = uri.host?.lowercase() ?: return null val defaultPort = if (uri.scheme == "http") 80 else 443 return if (uri.port != -1 && uri.port != defaultPort) "$host:${uri.port}" else host } diff --git a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt new file mode 100644 index 000000000..ceb759abc --- /dev/null +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewNavigationTest.kt @@ -0,0 +1,184 @@ +package org.wordpress.gutenberg + +import android.net.Uri +import android.os.Looper +import android.webkit.WebResourceRequest +import kotlinx.coroutines.test.TestScope +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mockito.mock +import org.mockito.Mockito.`when` +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import org.robolectric.Shadows.shadowOf +import org.wordpress.gutenberg.model.EditorConfiguration +import org.wordpress.gutenberg.model.EditorDependencies + +/** + * What the editor's main frame admits, and which document the editor globals reach. + * + * On Android the editor loads from the site's own origin, so the site's ordinary + * pages are one navigation away from the frame holding the site credential. + */ +@RunWith(RobolectricTestRunner::class) +class GutenbergViewNavigationTest { + + private val testScope = TestScope() + + /** A view whose configuration carries a recognizable credential. */ + private fun configuredSiteView() = GutenbergView( + EditorConfiguration.builder("https://example.com", "https://example.com/wp-json/") + .setAuthHeader("Bearer secret-credential") + .build(), + EditorDependencies.empty, + testScope, + RuntimeEnvironment.getApplication() + ) + + /** + * The editor document for [siteUrl], mirroring the fallback in `loadEditor` so + * this holds whether or not a local `GUTENBERG_EDITOR_URL` dev server is set. + */ + private fun editorUrlFor(siteUrl: String) = BuildConfig.GUTENBERG_EDITOR_URL + .ifEmpty { "$siteUrl/assets/index.html" } + + private fun opensExternally(view: GutenbergView, url: String): Boolean { + val request = mock(WebResourceRequest::class.java) + `when`(request.url).thenReturn(Uri.parse(url)) + return view.editorWebView.webViewClient.shouldOverrideUrlLoading(view.editorWebView, request) + } + + @Test + fun `shouldOverrideUrlLoading opens REST API URLs externally`() { + // The editor reaches the API by fetch, which never navigates the frame. + val siteView = configuredSiteView() + listOf( + "https://example.com/wp-json/wp/v2/posts", + "https://example.com/?rest_route=/wp/v2/posts", + "https://public-api.wordpress.com/wp/v2/sites/123/posts" + ).forEach { url -> + assertTrue("$url should open externally", opensExternally(siteView, url)) + } + } + + @Test + fun `shouldOverrideUrlLoading opens site pages that resemble the REST API externally`() { + // WordPress serves each of these with the site's theme and plugins. + val siteView = configuredSiteView() + listOf( + "https://example.com/blog/wp-json/a-post", + "https://example.com/a-page/?utm_campaign=rest_route=x", + "https://example.com/wp-json/?rest_route=", + "https://example.com/a-page/?rest_route=/wp/v2/posts&rest_route=", + "http://example.com/wp-json/wp/v2/posts" + ).forEach { url -> + assertTrue("$url should open externally", opensExternally(siteView, url)) + } + } + + @Test + fun `shouldOverrideUrlLoading blocks asset paths over a scheme the asset loader does not serve`() { + // An https site's assets are served over https alone, so the same path over + // http goes to the site over the network. + val result = opensExternally(configuredSiteView(), "http://example.com/assets/index.html") + + assertTrue("an asset path over the other scheme should open externally", result) + } + + @Test + fun `onPageStarted injects the configuration into the editor document`() { + val siteView = configuredSiteView() + val webView = siteView.editorWebView + + webView.webViewClient.onPageStarted(webView, editorUrlFor("https://example.com"), null) + + assertTrue( + "the editor document should receive the globals it boots from", + shadowOf(webView).lastEvaluatedJavascript.orEmpty().contains("window.GBKit") + ) + } + + @Test + fun `onPageStarted injects the configuration again when the editor reloads`() { + val siteView = configuredSiteView() + val webView = siteView.editorWebView + webView.webViewClient.onPageStarted(webView, editorUrlFor("https://example.com"), null) + webView.evaluateJavascript("editor.undo()", null) + + siteView.reloadEditor() + shadowOf(Looper.getMainLooper()).idle() + webView.webViewClient.onPageStarted(webView, editorUrlFor("https://example.com"), null) + + assertTrue( + "the reloaded editor document should receive the globals again", + shadowOf(webView).lastEvaluatedJavascript.orEmpty().contains("window.GBKit") + ) + } + + @Test + fun `onPageStarted injects the configuration into a local http site's editor document`() { + // A local site serves the editor over http, which the asset scheme must follow. + val siteView = GutenbergView( + EditorConfiguration.builder("http://10.0.2.2:8888", "http://10.0.2.2:8888/wp-json/") + .setAuthHeader("Bearer secret-credential") + .build(), + EditorDependencies.empty, + testScope, + RuntimeEnvironment.getApplication() + ) + val webView = siteView.editorWebView + + webView.webViewClient.onPageStarted(webView, editorUrlFor("http://10.0.2.2:8888"), null) + + assertTrue( + "a local http site's editor document should receive the globals", + shadowOf(webView).lastEvaluatedJavascript.orEmpty().contains("window.GBKit") + ) + } + + @Test + fun `onPageStarted withholds the configuration from a non-editor page`() { + // Some loads never pass `shouldOverrideUrlLoading` (POST forms, history, a + // host's `loadUrl`), so a site page can still reach this frame. It must not + // receive the credential. + val siteView = configuredSiteView() + val webView = siteView.editorWebView + + webView.webViewClient.onPageStarted(webView, "https://example.com/wp-json/wp/v2/posts", null) + + // Nothing evaluated at all, so an injection followed by another script still fails. + assertNull( + "a non-editor page must not receive the site credential", + shadowOf(webView).lastEvaluatedJavascript + ) + } + + @Test + fun `onPageStarted withholds the configuration from another bundled asset page`() { + // The asset loader serves every page the host app bundles, not only the editor. + val siteView = configuredSiteView() + val webView = siteView.editorWebView + + webView.webViewClient.onPageStarted(webView, "https://example.com/assets/support.html", null) + + assertNull( + "a bundled page other than the editor must not receive the site credential", + shadowOf(webView).lastEvaluatedJavascript + ) + } + + @Test + fun `onPageStarted withholds the configuration from an asset path the network served`() { + val siteView = configuredSiteView() + val webView = siteView.editorWebView + + webView.webViewClient.onPageStarted(webView, "http://example.com/assets/index.html", null) + + assertNull( + "a network-served page must not receive the site credential", + shadowOf(webView).lastEvaluatedJavascript + ) + } +} diff --git a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewTest.kt b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewTest.kt index 685cd9242..569a820df 100644 --- a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewTest.kt +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewTest.kt @@ -257,6 +257,12 @@ class GutenbergViewTest { assertEquals("example.com", GutenbergView.originAuthority("http://example.com:80")) } + @Test + fun `originAuthority lowercases the host`() { + // Chromium lowercases the host, e.g. a Mac's `Davids-MacBook-Pro.local`. + assertEquals("mymac.local:5173", GutenbergView.originAuthority("http://MyMac.local:5173")) + } + @Test fun `originAuthority omits a port that is absent`() { assertEquals("example.com", GutenbergView.originAuthority("https://example.com")) @@ -312,6 +318,13 @@ class GutenbergViewTest { ) } + @Test + fun `isDevServerUrl matches a dev server URL written with a capitalized host`() { + assertTrue( + GutenbergView.isDevServerUrl(Uri.parse("http://mymac.local:5173/"), "http://MyMac.local:5173/") + ) + } + @Test fun `isDevServerUrl matches a dev server URL written with its default port`() { // Chromium drops a default port before the URL reaches the WebViewClient. @@ -332,82 +345,6 @@ class GutenbergViewTest { assertFalse(GutenbergView.isDevServerUrl(Uri.parse("tel:5551234"), "10.0.2.2:5173")) } - // ===== REST API navigation ===== - - @Test - fun `shouldOverrideUrlLoading allows REST API URLs on the site's API root`() { - // Callers pass a full API root with a path, e.g. WordPress-Android's - // `site.wpApiRestUrl ?: "${site.url}/wp-json/"`. - val siteView = GutenbergView( - EditorConfiguration.builder("https://example.com", "https://example.com/wp-json/") - .build(), - EditorDependencies.empty, - testScope, - RuntimeEnvironment.getApplication() - ) - - val request = mock(WebResourceRequest::class.java) - `when`(request.url).thenReturn(Uri.parse("https://example.com/wp-json/wp/v2/posts")) - - val result = siteView.editorWebView.webViewClient.shouldOverrideUrlLoading(siteView.editorWebView, request) - assertFalse("REST API URLs on the site's API root should load in the WebView", result) - } - - @Test - fun `shouldOverrideUrlLoading allows REST API URLs for a rest_route API root`() { - val siteView = GutenbergView( - EditorConfiguration.builder( - "https://example.com", - "https://example.com/index.php?rest_route=/" - ).build(), - EditorDependencies.empty, - testScope, - RuntimeEnvironment.getApplication() - ) - - val request = mock(WebResourceRequest::class.java) - `when`(request.url).thenReturn( - Uri.parse("https://example.com/index.php?rest_route=/wp/v2/posts") - ) - - val result = siteView.editorWebView.webViewClient.shouldOverrideUrlLoading(siteView.editorWebView, request) - assertFalse("rest_route REST API URLs should load in the WebView", result) - } - - @Test - fun `shouldOverrideUrlLoading blocks REST API URLs on a different host`() { - val siteView = GutenbergView( - EditorConfiguration.builder("https://example.com", "https://example.com/wp-json/") - .build(), - EditorDependencies.empty, - testScope, - RuntimeEnvironment.getApplication() - ) - - val request = mock(WebResourceRequest::class.java) - `when`(request.url).thenReturn(Uri.parse("https://other.example.net/wp-json/wp/v2/posts")) - - val result = siteView.editorWebView.webViewClient.shouldOverrideUrlLoading(siteView.editorWebView, request) - assertTrue("REST API URLs on another host should open externally", result) - } - - @Test - fun `shouldOverrideUrlLoading allows REST API URLs when the API root has a port`() { - val siteView = GutenbergView( - EditorConfiguration.builder("http://10.0.2.2:8888", "http://10.0.2.2:8888/wp-json/") - .build(), - EditorDependencies.empty, - testScope, - RuntimeEnvironment.getApplication() - ) - - val request = mock(WebResourceRequest::class.java) - `when`(request.url).thenReturn(Uri.parse("http://10.0.2.2:8888/wp-json/wp/v2/posts")) - - val result = siteView.editorWebView.webViewClient.shouldOverrideUrlLoading(siteView.editorWebView, request) - assertFalse("REST API URLs on a port-bearing API root should load in the WebView", result) - } - @Test fun `shouldOverrideUrlLoading allows asset URLs when the site URL has an explicit default port`() { val siteView = GutenbergView( diff --git a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewUploadServerTest.kt b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewUploadServerTest.kt index 293c08778..85dd1930b 100644 --- a/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewUploadServerTest.kt +++ b/android/Gutenberg/src/test/java/org/wordpress/gutenberg/GutenbergViewUploadServerTest.kt @@ -20,6 +20,17 @@ import org.wordpress.gutenberg.model.EditorDependencies @Config(manifest = Config.NONE) class GutenbergViewUploadServerTest { + private companion object { + /** + * The editor document for [makeView]'s site, which the globals are scoped to. + * + * Mirrors the fallback in `loadEditor`, so this holds whether or not a local + * `GUTENBERG_EDITOR_URL` dev server is configured. + */ + val EDITOR_URL = BuildConfig.GUTENBERG_EDITOR_URL + .ifEmpty { "https://example.com/assets/index.html" } + } + private val testScope = TestScope() private fun makeView(): GutenbergView { @@ -46,10 +57,13 @@ class GutenbergViewUploadServerTest { * `onPageStarted`) to simulate the editor page beginning to load — the point at * which the delegate is captured and the upload server starts. */ - private fun startLoading(view: GutenbergView) { - val method = GutenbergView::class.java.getDeclaredMethod("onEditorPageStarted") + private fun startLoading(view: GutenbergView, url: String = EDITOR_URL) { + val method = GutenbergView::class.java.getDeclaredMethod( + "onEditorPageStarted", + String::class.java + ) method.isAccessible = true - method.invoke(view) + method.invoke(view, url) } /** Invokes the protected `onDetachedFromWindow` lifecycle callback. */ @@ -94,6 +108,22 @@ class GutenbergViewUploadServerTest { } } + @Test + fun `a non-editor page does not start the upload server`() { + val view = makeView() + try { + view.mediaUploadDelegate = mock(MediaUploadDelegate::class.java) + startLoading(view, "https://example.com/assets/support.html") + idle() + assertNull( + "only the editor document should bring up the upload server", + uploadServerOf(view) + ) + } finally { + detach(view) + } + } + @Test fun `setting the delegate after the page has started loading throws`() { val view = makeView()