From f07b69cf1379ec48cd338d307a4dbdfd60e20577 Mon Sep 17 00:00:00 2001
From: Lucas Werkmeister <lucas.werkmeister@wikimedia.de>
Date: Mon, 9 Sep 2019 16:55:35 +0200
Subject: [PATCH] =?utf8?q?mw.ForeignApi:=20don=E2=80=99t=20set=20origin=20?=
 =?utf8?q?for=20same-origin=20requests?=
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

If the foreign API has the same host as the current page (e. g. if it’s
actually the same wiki, or multiple wikis are installed under different
paths on the same host), then the browser will not send an Origin
header, and if we still set an origin parameter, then the API will
complain that the two don’t match. Detect this and unset the origin
parameter in that case.

Bug: T208601
Change-Id: Ia006f3dc3283ce3f81d4d72cbe9676a00797c4d0
---
 resources/Resources.php                       |  1 +
 resources/src/mediawiki.ForeignApi.core.js    | 24 ++++++++++++------
 .../mediawiki.ForeignApi.test.js              | 25 +++++++++++++++++++
 3 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/resources/Resources.php b/resources/Resources.php
index 1388128fc0..a9e29c9827 100644
--- a/resources/Resources.php
+++ b/resources/Resources.php
@@ -895,6 +895,7 @@ return [
 		'dependencies' => [
 			'mediawiki.api',
 			'oojs',
+			'mediawiki.Uri',
 		],
 		'targets' => [ 'desktop', 'mobile' ],
 	],
diff --git a/resources/src/mediawiki.ForeignApi.core.js b/resources/src/mediawiki.ForeignApi.core.js
index 4b6313b93c..83ea0ce003 100644
--- a/resources/src/mediawiki.ForeignApi.core.js
+++ b/resources/src/mediawiki.ForeignApi.core.js
@@ -59,7 +59,6 @@
 					}
 				},
 				parameters: {
-					// Add 'origin' query parameter to all requests.
 					origin: this.getOrigin()
 				}
 			},
@@ -77,17 +76,26 @@
 	 * any).
 	 *
 	 * @protected
-	 * @return {string}
+	 * @return {string|undefined}
 	 */
 	CoreForeignApi.prototype.getOrigin = function () {
-		var origin;
+		var origin, apiUri, apiOrigin;
 		if ( this.anonymous ) {
 			return '*';
 		}
+
 		origin = location.protocol + '//' + location.hostname;
 		if ( location.port ) {
 			origin += ':' + location.port;
 		}
+
+		apiUri = new mw.Uri( this.apiUrl );
+		apiOrigin = apiUri.protocol + '://' + apiUri.getAuthority();
+		if ( origin === apiOrigin ) {
+			// requests are not cross-origin, omit parameter
+			return undefined;
+		}
+
 		return origin;
 	};
 
@@ -101,10 +109,12 @@
 		if ( ajaxOptions.type === 'POST' ) {
 			url = ( ajaxOptions && ajaxOptions.url ) || this.defaults.ajax.url;
 			origin = ( parameters && parameters.origin ) || this.defaults.parameters.origin;
-			url += ( url.indexOf( '?' ) !== -1 ? '&' : '?' ) +
-				// Depending on server configuration, MediaWiki may forbid periods in URLs, due to an IE 6
-				// XSS bug. So let's escape them here. See WebRequest::checkUrlExtension() and T30235.
-				'origin=' + encodeURIComponent( origin ).replace( /\./g, '%2E' );
+			if ( origin !== undefined ) {
+				url += ( url.indexOf( '?' ) !== -1 ? '&' : '?' ) +
+					// Depending on server configuration, MediaWiki may forbid periods in URLs, due to an IE 6
+					// XSS bug. So let's escape them here. See WebRequest::checkUrlExtension() and T30235.
+					'origin=' + encodeURIComponent( origin ).replace( /\./g, '%2E' );
+			}
 			newAjaxOptions = $.extend( {}, ajaxOptions, { url: url } );
 		} else {
 			newAjaxOptions = ajaxOptions;
diff --git a/tests/qunit/suites/resources/mediawiki.api/mediawiki.ForeignApi.test.js b/tests/qunit/suites/resources/mediawiki.api/mediawiki.ForeignApi.test.js
index 541c61072d..22a3a4b436 100644
--- a/tests/qunit/suites/resources/mediawiki.api/mediawiki.ForeignApi.test.js
+++ b/tests/qunit/suites/resources/mediawiki.api/mediawiki.ForeignApi.test.js
@@ -29,4 +29,29 @@
 		return api.post( {} );
 	} );
 
+	QUnit.test( 'origin is not included in same-origin GET requests', function ( assert ) {
+		var apiUrl = location.protocol + '//' + location.host + '/w/api.php',
+			api = new mw.ForeignApi( apiUrl );
+
+		this.server.respond( function ( request ) {
+			assert.strictEqual( request.url.match( /origin=.*?(?:&|$)/ ), null, 'origin is not included in GET requests' );
+			request.respond( 200, { 'Content-Type': 'application/json' }, '[]' );
+		} );
+
+		return api.get( {} );
+	} );
+
+	QUnit.test( 'origin is not included in same-origin POST requests', function ( assert ) {
+		var apiUrl = location.protocol + '//' + location.host + '/w/api.php',
+			api = new mw.ForeignApi( apiUrl );
+
+		this.server.respond( function ( request ) {
+			assert.strictEqual( request.requestBody.match( /origin=.*?(?:&|$)/ ), null, 'origin is not included in POST request body' );
+			assert.strictEqual( request.url.match( /origin=.*?(?:&|$)/ ), null, 'origin is not included in POST request URL, either' );
+			request.respond( 200, { 'Content-Type': 'application/json' }, '[]' );
+		} );
+
+		return api.post( {} );
+	} );
+
 }() );
-- 
2.20.1