fix!: send the Authorization header only to the site and the hosts the app names

android/Gutenberg/src/main/java/org/wordpress/gutenberg/EditorAssetsLibrary.kt

@@ -8,6 +8,7 @@ import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.cancel
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
+import org.wordpress.gutenberg.model.EditorAuthorizationScope
 import org.wordpress.gutenberg.model.EditorConfiguration
 import java.io.File
 import java.net.HttpURLConnection
@@ -48,8 +49,9 @@ class EditorAssetsLibrary(
                 val defaultUserAgent = System.getProperty("http.agent") ?: ""
                 connection.setRequestProperty("User-Agent", "$defaultUserAgent GutenbergKit/${GutenbergKitVersion.VERSION}")
 
-                // Set headers from configuration
-                if (configuration.authHeader.isNotEmpty()) {
+                // Set headers from configuration. The site's credentials go only where they
+                // may: a custom endpoint can be on another party's host.
+                if (configuration.authHeader.isNotEmpty() && EditorAuthorizationScope(configuration).allows(endpoint)) {
                     connection.setRequestProperty("Authorization", configuration.authHeader)
                 }
 

android/Gutenberg/src/main/java/org/wordpress/gutenberg/EditorHTTPClient.kt

@@ -10,6 +10,7 @@ import okhttp3.OkHttpClient
 import okhttp3.Request
 import okhttp3.RequestBody.Companion.toRequestBody
 import okhttp3.Response
+import org.wordpress.gutenberg.model.EditorAuthorizationScope
 import org.wordpress.gutenberg.model.http.EditorHTTPHeaders
 import org.wordpress.gutenberg.model.http.EditorHttpMethod
 import java.io.File
@@ -120,10 +121,15 @@ sealed class EditorHTTPClientError : Exception() {
  * An HTTP client for making authenticated requests to the WordPress REST API.
  *
  * This class handles request signing, error parsing, and response validation.
- * All requests are automatically authenticated using the provided authorization header.
+ * A request within the client's [EditorAuthorizationScope] is authenticated using the provided
+ * authorization header. A request anywhere else — an asset on another party's host, say — goes
+ * out without it.
+ *
+ * @param authorizationScope The requests that carry [authHeader].
  */
 class EditorHTTPClient(
     private val authHeader: String,
+    private val authorizationScope: EditorAuthorizationScope,
     private val delegate: EditorHTTPClientDelegate? = null,
     private val requestTimeoutSeconds: Long = 60,
     okHttpClient: OkHttpClient? = null
@@ -139,11 +145,19 @@ class EditorHTTPClient(
             .writeTimeout(requestTimeoutSeconds, TimeUnit.SECONDS)
             .build()
 
+    /**
+     * Adds the site's credentials to a request for [url], if they may go there. The site's
+     * credentials are for the site, and a request can be for anywhere: an editor downloads assets
+     * from whichever hosts the site names.
+     */
+    private fun Request.Builder.authorize(url: String): Request.Builder =
+        if (authorizationScope.allows(url)) addHeader("Authorization", authHeader) else this
+
     override suspend fun download(url: String, destination: File): EditorHTTPClientDownloadResponse =
         withContext(Dispatchers.IO) {
             val request = Request.Builder()
                 .url(url)
-                .addHeader("Authorization", authHeader)
+                .authorize(url)
                 .get()
                 .build()
 
@@ -186,7 +200,7 @@ class EditorHTTPClient(
 
             val request = Request.Builder()
                 .url(url)
-                .addHeader("Authorization", authHeader)
+                .authorize(url)
                 .method(method.toString(), requestBody)
                 .build()
 

android/Gutenberg/src/main/java/org/wordpress/gutenberg/model/EditorAuthorizationScope.kt

@@ -0,0 +1,102 @@
+package org.wordpress.gutenberg.model
+
+import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
+
+/**
+ * The requests that may carry a site's credentials.
+ *
+ * Credentials go to the site and to its REST API, and nowhere else. An editor also downloads
+ * plugin and theme assets from whichever hosts the site names, and one of those can be a third
+ * party's — a script on a vendor's CDN, say — which has no business receiving them.
+ *
+ * An app that knows of other places its credentials belong names them: a site reached through
+ * WordPress.com has assets on `wp.com` and files on `files.wordpress.com`, run by the same party
+ * as its API. See [EditorConfiguration.Builder.setAuthHeaderDomains] for how they're named.
+ *
+ * @param siteURL The site's address.
+ * @param siteApiRoot The root of the site's REST API.
+ * @param domains Other places that may receive the site's credentials, over HTTPS only. A name is
+ *   one host, exactly: `s0.wp.com`. A name that starts with `*.` is the domain that follows and
+ *   every subdomain of it: `*.wp.com`. A name that isn't one of the two is ignored.
+ */
+class EditorAuthorizationScope(
+    siteURL: String,
+    siteApiRoot: String,
+    domains: Collection<String> = emptySet()
+) {
+
+    /** Creates the scope for the site in [configuration]. */
+    constructor(configuration: EditorConfiguration) : this(
+        siteURL = configuration.siteURL,
+        siteApiRoot = configuration.siteApiRoot,
+        domains = configuration.authHeaderDomains
+    )
+
+    /** The origins that may receive credentials: the site's, and its REST API's. */
+    private val origins: Set<Origin> = setOfNotNull(Origin.of(siteURL), Origin.of(siteApiRoot))
+
+    private val names: List<Name> = domains.mapNotNull(Name::of)
+
+    /** The hosts that may receive credentials over HTTPS, each named in full. */
+    private val hosts: Set<String> = names.filterIsInstance<Name.Host>().map { it.host }.toSet()
+
+    /**
+     * The domains that may receive credentials over HTTPS along with their every subdomain:
+     * `wp.com` for `*.wp.com`.
+     */
+    private val wildcardDomains: Set<String> =
+        names.filterIsInstance<Name.DomainAndSubdomains>().map { it.domain }.toSet()
+
+    /** Whether a request to [url] may carry the site's credentials. */
+    fun allows(url: String): Boolean {
+        val origin = Origin.of(url) ?: return false
+
+        if (origin in origins) {
+            return true
+        }
+
+        return origin.scheme == "https" &&
+            (origin.host in hosts || wildcardDomains.any { origin.host == it || origin.host.endsWith(".$it") })
+    }
+
+    /** What an app names as a place for the site's credentials. */
+    private sealed interface Name {
+        /** One host, exactly. */
+        data class Host(val host: String) : Name
+
+        /** A domain and every subdomain of it. */
+        data class DomainAndSubdomains(val domain: String) : Name
+
+        companion object {
+            private const val WILDCARD = "*."
+
+            /**
+             * `null` for a name that is neither: an empty one, or one with a wildcard anywhere
+             * but in front. What a wildcard is over is the app's business: `*.com` is every
+             * `.com` site.
+             */
+            fun of(name: String): Name? {
+                // Without the dot that ends a fully qualified name
+                val trimmed = name.trim().lowercase().removeSuffix(".")
+                val isWildcard = trimmed.startsWith(WILDCARD)
+                val domain = trimmed.removePrefix(WILDCARD)
+                val labels = domain.split(".")
+
+                return when {
+                    labels.any { it.isEmpty() || it.contains("*") } -> null
+                    isWildcard -> DomainAndSubdomains(domain)
+                    else -> Host(domain)
+                }
+            }
+        }
+    }
+
+    /** A URL's scheme, host and port: what has to match for two URLs to be the same place. */
+    private data class Origin(val scheme: String, val host: String, val port: Int) {
+        companion object {
+            /** `null` for anything but an absolute `http` or `https` URL. */
+            fun of(url: String): Origin? =
+                url.toHttpUrlOrNull()?.let { Origin(scheme = it.scheme, host = it.host, port = it.port) }
+        }
+    }
+}

android/Gutenberg/src/main/java/org/wordpress/gutenberg/model/EditorConfiguration.kt

@@ -27,6 +27,13 @@ data class EditorConfiguration(
     val cookies: Map<String, String>,
     val enableAssetCaching: Boolean = false,
     val cachedAssetHosts: Set<String> = emptySet(),
+    /**
+     * Places that [authHeader] may be sent to over HTTPS, besides the site and its API, which
+     * always receive it. A name is one host, exactly: `s0.wp.com`. A name that starts with `*.`
+     * is the domain that follows and every subdomain of it: `*.wp.com`. See
+     * [Builder.setAuthHeaderDomains].
+     */
+    val authHeaderDomains: Set<String> = emptySet(),
     val editorAssetsEndpoint: String? = null,
     val enableNetworkLogging: Boolean = false,
     var enableOfflineMode: Boolean = false,
@@ -85,6 +92,7 @@ data class EditorConfiguration(
         private var cookies: Map<String, String> = mapOf()
         private var enableAssetCaching: Boolean = false
         private var cachedAssetHosts: Set<String> = emptySet()
+        private var authHeaderDomains: Set<String> = emptySet()
         private var editorAssetsEndpoint: String? = null
         private var enableNetworkLogging: Boolean = false
         private var enableOfflineMode: Boolean = false
@@ -104,6 +112,24 @@ data class EditorConfiguration(
         fun setSiteApiNamespace(siteApiNamespace: Array<String>) = apply { this.siteApiNamespace = siteApiNamespace }
         fun setNamespaceExcludedPaths(namespaceExcludedPaths: Array<String>) = apply { this.namespaceExcludedPaths = namespaceExcludedPaths }
         fun setAuthHeader(authHeader: String) = apply { this.authHeader = authHeader }
+
+        /**
+         * Sets the places the auth header may be sent to, besides the site and its API, which
+         * always receive it. Only requests over HTTPS qualify.
+         *
+         * Each name is taken exactly as written:
+         *
+         * - `s0.wp.com` is that one host. It isn't its subdomains, and it isn't `s1.wp.com`.
+         * - `*.wp.com` is `wp.com` and every subdomain of it, however deep.
+         *
+         * A wildcard is only ever the whole first label, and is taken at its word: `*.com` is
+         * every `.com` site. Name the narrowest domain that will do.
+         *
+         * A site reached through WordPress.com is served from more than its own address — its
+         * assets from `wp.com` and its files from `files.wordpress.com`, say — and can name those
+         * here: `setOf("*.wp.com", "*.files.wordpress.com")`.
+         */
+        fun setAuthHeaderDomains(authHeaderDomains: Set<String>) = apply { this.authHeaderDomains = authHeaderDomains }
         fun setEditorSettings(editorSettings: String?) = apply { this.editorSettings = editorSettings }
         /**
          * Stores [locale] verbatim without running the resolver. Reserved for
@@ -168,6 +194,7 @@ data class EditorConfiguration(
             cookies = cookies,
             enableAssetCaching = enableAssetCaching,
             cachedAssetHosts = cachedAssetHosts,
+            authHeaderDomains = authHeaderDomains,
             editorAssetsEndpoint = editorAssetsEndpoint,
             enableNetworkLogging = enableNetworkLogging,
             enableOfflineMode = enableOfflineMode,
@@ -197,6 +224,7 @@ data class EditorConfiguration(
         .setCookies(cookies)
         .setEnableAssetCaching(enableAssetCaching)
         .setCachedAssetHosts(cachedAssetHosts)
+        .setAuthHeaderDomains(authHeaderDomains)
         .setEditorAssetsEndpoint(editorAssetsEndpoint)
         .setEnableNetworkLogging(enableNetworkLogging)
         .setEnableOfflineMode(enableOfflineMode)
@@ -227,6 +255,7 @@ data class EditorConfiguration(
         if (cookies != other.cookies) return false
         if (enableAssetCaching != other.enableAssetCaching) return false
         if (cachedAssetHosts != other.cachedAssetHosts) return false
+        if (authHeaderDomains != other.authHeaderDomains) return false
         if (editorAssetsEndpoint != other.editorAssetsEndpoint) return false
         if (enableNetworkLogging != other.enableNetworkLogging) return false
         if (enableOfflineMode != other.enableOfflineMode) return false
@@ -256,6 +285,7 @@ data class EditorConfiguration(
         result = 31 * result + cookies.hashCode()
         result = 31 * result + enableAssetCaching.hashCode()
         result = 31 * result + cachedAssetHosts.hashCode()
+        result = 31 * result + authHeaderDomains.hashCode()
         result = 31 * result + (editorAssetsEndpoint?.hashCode() ?: 0)
         result = 31 * result + enableNetworkLogging.hashCode()
         result = 31 * result + enableOfflineMode.hashCode()

android/Gutenberg/src/main/java/org/wordpress/gutenberg/model/GBKitGlobal.kt

@@ -42,6 +42,8 @@ data class GBKitGlobal(
     val namespaceExcludedPaths: List<String>,
     /** The authorization header value for authenticated API requests. */
     val authHeader: String,
+    /** Domains that [authHeader] may be sent to, besides the site and its API. */
+    val authHeaderDomains: List<String>,
     /** Whether to apply theme styles to the editor. */
     val themeStyles: Boolean,
     /** Whether to load plugin assets. */
@@ -115,6 +117,7 @@ data class GBKitGlobal(
                 siteApiNamespace = configuration.siteApiNamespace.toList(),
                 namespaceExcludedPaths = configuration.namespaceExcludedPaths.toList(),
                 authHeader = configuration.authHeader,
+                authHeaderDomains = configuration.authHeaderDomains.toList(),
                 themeStyles = configuration.themeStyles,
                 plugins = configuration.plugins,
                 enableNativeBlockInserter = configuration.enableNativeBlockInserter,

android/Gutenberg/src/main/java/org/wordpress/gutenberg/services/EditorService.kt

@@ -12,6 +12,7 @@ import org.wordpress.gutenberg.EditorHTTPClientProtocol
 import org.wordpress.gutenberg.Paths
 import org.wordpress.gutenberg.RESTAPIRepository
 import org.wordpress.gutenberg.model.EditorAssetBundle
+import org.wordpress.gutenberg.model.EditorAuthorizationScope
 import org.wordpress.gutenberg.model.EditorCachePolicy
 import org.wordpress.gutenberg.model.EditorConfiguration
 import org.wordpress.gutenberg.model.EditorDependencies
@@ -95,7 +96,10 @@ class EditorService(
             tempStorageRoot: File? = null,
             cacheRoot: File? = null
         ): EditorService {
-            val client = httpClient ?: EditorHTTPClient(authHeader = configuration.authHeader)
+            val client = httpClient ?: EditorHTTPClient(
+                authHeader = configuration.authHeader,
+                authorizationScope = EditorAuthorizationScope(configuration)
+            )
 
             val restRepository = RESTAPIRepository(
                 configuration = configuration,

android/Gutenberg/src/test/java/org/wordpress/gutenberg/EditorAssetsManifestAuthorizationTest.kt

@@ -0,0 +1,68 @@
+package org.wordpress.gutenberg
+
+import kotlinx.coroutines.runBlocking
+import okhttp3.mockwebserver.MockResponse
+import okhttp3.mockwebserver.MockWebServer
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+import org.wordpress.gutenberg.model.EditorConfiguration
+
+/**
+ * The manifest request of [org.wordpress.gutenberg.EditorAssetsLibrary] — the one in this
+ * package, which makes its own connection rather than going through [EditorHTTPClient] — sends
+ * the site's credentials to the site, and not to a custom endpoint on another party's host.
+ */
+@RunWith(RobolectricTestRunner::class)
+class EditorAssetsManifestAuthorizationTest {
+
+    private lateinit var mockWebServer: MockWebServer
+    private lateinit var baseUrl: String
+
+    companion object {
+        private const val TEST_AUTH_HEADER = "Bearer test-token-12345"
+    }
+
+    @Before
+    fun setUp() {
+        mockWebServer = MockWebServer()
+        mockWebServer.start()
+        baseUrl = mockWebServer.url("/").toString()
+    }
+
+    @After
+    fun tearDown() {
+        mockWebServer.shutdown()
+    }
+
+    private fun makeLibrary(configuration: EditorConfiguration.Builder) = EditorAssetsLibrary(
+        RuntimeEnvironment.getApplication(),
+        configuration.setAuthHeader(TEST_AUTH_HEADER).build()
+    )
+
+    @Test
+    fun `the manifest request sends the Authorization header to the site's API`() = runBlocking {
+        mockWebServer.enqueue(MockResponse().setResponseCode(200).setBody("{}"))
+
+        makeLibrary(EditorConfiguration.builder(baseUrl, baseUrl)).loadManifestContent()
+
+        assertEquals(TEST_AUTH_HEADER, mockWebServer.takeRequest().getHeader("Authorization"))
+    }
+
+    @Test
+    fun `the manifest request sends no Authorization header to an endpoint on another party's host`() = runBlocking {
+        mockWebServer.enqueue(MockResponse().setResponseCode(200).setBody("{}"))
+
+        makeLibrary(
+            EditorConfiguration.builder("https://example.com", "https://example.com/wp-json/")
+                .setEditorAssetsEndpoint("${baseUrl}editor-assets")
+        ).loadManifestContent()
+
+        assertNull(mockWebServer.takeRequest().getHeader("Authorization"))
+    }
+}