fix(android): scope the editor globals and keep site pages out of the editor frame

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
         }

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
+        )
+    }
+}