Reimplement CORS properly, addressing Tim's concerns

Tim's concerns (listed at
https://bugzilla.wikimedia.org/show_bug.cgi?id=20814#c6) were:
* Lack of Vary: Origin breaks Squid caching
* Vary: Origin on everything would be disastrous, so add an origin param
* Origin header is space-separated list, wasn't treated as such

This commit:
* Remove CORS code from api.php and reimplement it in ApiMain.php
* Add 'origin' parameter to ApiMain
* If 'origin' parameter doesn't match Origin header, send a 403
* If origin is whitelisted, set CORS headers and set Vary: Origin
* Add https?:// to wildcard matching logic, wasn't there but is needed

CORS now works :) you can test it locally as follows:
Set $wgCrossSiteAJAXdomains[] = '*.wikipedia.org';
Log into MediaWiki on localhost
Go to Wikipedia, open a JS console, and run:
$.ajax( {
	'url': 'http://localhost/w/api.php',
	'data': {
		'action': 'query',
		'meta': 'userinfo',
		'format': 'json',
		'origin': 'https://en.wikipedia.org'
		// or whichever domain you're on; must be correct!
	},
	'xhrFields': {
		'withCredentials': true
	},
	'success': function( data ) {
		alert( 'Foreign user ' + data.query.userinfo.name +
			' (ID ' + data.query.userinfo.id + ')' );
	}
} );

Change-Id: I725ce176866d7c81dd9ad6d7bc4a86b7160f2458

diff --git a/api.php b/api.php
index 1cd5497..1d75857 100644 (file)
--- a/api.php
+++ b/api.php
@@ -62,43 +62,6 @@ if ( !$wgEnableAPI ) {
        die(1);
 }
 
-// Selectively allow cross-site AJAX
-
-/**
- * Helper function to convert wildcard string into a regex
- * '*' => '.*?'
- * '?' => '.'
- *
- * @param $search string
- * @return string
- */
-function convertWildcard( $search ) {
-       $search = preg_quote( $search, '/' );
-       $search = str_replace(
-               array( '\*', '\?' ),
-               array( '.*?', '.' ),
-               $search
-       );
-       return "/$search/";
-}
-
-if ( $wgCrossSiteAJAXdomains && isset( $_SERVER['HTTP_ORIGIN'] ) ) {
-       $exceptions = array_map( 'convertWildcard', $wgCrossSiteAJAXdomainExceptions );
-       $regexes = array_map( 'convertWildcard', $wgCrossSiteAJAXdomains );
-       foreach ( $regexes as $regex ) {
-               if ( preg_match( $regex, $_SERVER['HTTP_ORIGIN'] ) ) {
-                       foreach ( $exceptions as $exc ) { // Check against exceptions
-                               if ( preg_match( $exc, $_SERVER['HTTP_ORIGIN'] ) ) {
-                                       break 2;
-                               }
-                       }
-                       header( "Access-Control-Allow-Origin: {$_SERVER['HTTP_ORIGIN']}" );
-                       header( 'Access-Control-Allow-Credentials: true' );
-                       break;
-               }
-       }
-}
-
 // Set a dummy $wgTitle, because $wgTitle == null breaks various things
 // In a perfect world this wouldn't be necessary
 $wgTitle = Title::makeTitle( NS_MAIN, 'API' );

diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php
index 30e9fb6..695eccd 100644 (file)
--- a/includes/api/ApiMain.php
+++ b/includes/api/ApiMain.php
@@ -354,6 +354,12 @@ class ApiMain extends ApiBase {
         * have been accumulated, and replace it with an error message and a help screen.
         */
        protected function executeActionWithErrorHandling() {
+               // Verify the CORS header before executing the action
+               if ( !$this->handleCORS() ) {
+                       // handleCORS() has sent a 403, abort
+                       return;
+               }
+
                // In case an error occurs during data output,
                // clear the output buffer and print just the error information
                ob_start();
@@ -402,6 +408,86 @@ class ApiMain extends ApiBase {
                ob_end_flush();
        }
 
+       /**
+        * Check the &origin= query parameter against the Origin: HTTP header and respond appropriately.
+        *
+        * If no origin parameter is present, nothing happens.
+        * If an origin parameter is present but doesn't match the Origin header, a 403 status code
+        * is set and false is returned.
+        * If the parameter and the header do match, the header is checked against $wgCrossSiteAJAXdomains
+        * and $wgCrossSiteAJAXdomainExceptions, and if the origin qualifies, the appropriate CORS
+        * headers are set.
+        *
+        * @return bool False if the caller should abort (403 case), true otherwise (all other cases)
+        */
+       protected function handleCORS() {
+               global $wgCrossSiteAJAXdomains, $wgCrossSiteAJAXdomainExceptions;
+               $response = $this->getRequest()->response();
+               $originParam = $this->getParameter( 'origin' ); // defaults to null
+               if ( $originParam === null ) {
+                       // No origin parameter, nothing to do
+                       return true;
+               }
+               // Origin: header is a space-separated list of origins, check all of them
+               $originHeader = isset( $_SERVER['HTTP_ORIGIN'] ) ? $_SERVER['HTTP_ORIGIN'] : '';
+               $origins = explode( ' ', $originHeader );
+               if ( !in_array( $originParam, $origins ) ) {
+                       // origin parameter set but incorrect
+                       // Send a 403 response
+                       $message = HttpStatus::getMessage( 403 );
+                       $response->header( "HTTP/1.1 403 $message", true, 403 );
+                       $response->header( 'Cache-Control: no-cache' );
+                       echo "'origin' parameter does not match Origin header\n";
+                       return false;
+               }
+               if ( self::matchOrigin( $originParam, $wgCrossSiteAJAXdomains, $wgCrossSiteAJAXdomainExceptions ) ) {
+                       $response->header( "Access-Control-Allow-Origin: $originParam" );
+                       $response->header( 'Access-Control-Allow-Credentials: true' );
+                       $this->getOutput()->addVaryHeader( 'Origin' );
+               }
+               return true;
+       }
+
+       /**
+        * Attempt to match an Origin header against a set of rules and a set of exceptions
+        * @param $value string Origin header
+        * @param $rules array Set of wildcard rules
+        * @param $exceptions array Set of wildcard rules
+        * @return bool True if $value matches a rule in $rules and doesn't match any rules in $exceptions, false otherwise
+        */
+       protected static function matchOrigin( $value, $rules, $exceptions ) {
+               foreach ( $rules as $rule ) {
+                       if ( preg_match( self::wildcardToRegex( $rule ), $value ) ) {
+                               // Rule matches, check exceptions
+                               foreach ( $exceptions as $exc ) {
+                                       if ( preg_match( self::wildcardToRegex( $exc ), $value ) ) {
+                                               return false;
+                                       }
+                               }
+                               return true;
+                       }
+               }
+               return false;
+       }
+
+       /**
+        * Helper function to convert wildcard string into a regex
+        * '*' => '.*?'
+        * '?' => '.'
+        *
+        * @param $wildcard string String with wildcards
+        * @return string Regular expression
+        */
+       protected static function wildcardToRegex( $wildcard ) {
+               $wildcard = preg_quote( $wildcard, '/' );
+               $wildcard = str_replace(
+                       array( '\*', '\?' ),
+                       array( '.*?', '.' ),
+                       $wildcard
+               );
+               return "/https?:\/\/$wildcard/";
+       }
+
        protected function sendCacheHeaders() {
                global $wgUseXVO, $wgVaryOnXFP;
                $response = $this->getRequest()->response();
@@ -784,6 +870,7 @@ class ApiMain extends ApiBase {
                        ),
                        'requestid' => null,
                        'servedby'  => false,
+                       'origin' => null,
                );
        }
 
@@ -809,6 +896,12 @@ class ApiMain extends ApiBase {
                        'maxage' => 'Set the max-age header to this many seconds. Errors are never cached',
                        'requestid' => 'Request ID to distinguish requests. This will just be output back to you',
                        'servedby' => 'Include the hostname that served the request in the results. Unconditionally shown on error',
+                       'origin' => array(
+                               'When accessing the API using a cross-domain AJAX request (CORS), set this to the originating domain.',
+                               'This must match one of the origins in the Origin: header exactly, so it has to be set to something like http://en.wikipedia.org or https://meta.wikimedia.org .',
+                               'If this parameter does not match the Origin: header, a 403 response will be returned.',
+                               'If this parameter matches the Origin: header and the origin is whitelisted, an Access-Control-Allow-Origin header will be set.',
+                       ),
                );
        }