API: Don't handle non-preflight OPTIONS as CORS

If it's not a preflight, it's either a simple request or an actual
request after a preflight. But simple requests can only be GET, HEAD, or
POST, and we only return a successful response for a preflight if the
actual request is going to be GET or POST. So either way it shouldn't be
handled as CORS.

This also adds a response header to indicate to the end user why
something they probably intended as a CORS request wasn't handled as
one, to help with debugging.

And it renames a local variable that confused me when looking back at
this code.

Bug: T168558
Change-Id: Ia66c74e1cc8536c6c478237540abb4d7a98b9803

diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php
index d7586e0..5cb7967 100644 (file)
--- a/includes/api/ApiMain.php
+++ b/includes/api/ApiMain.php
@@ -704,13 +704,17 @@ class ApiMain extends ApiBase {
                $request = $this->getRequest();
                $response = $request->response();
 
-               $matchOrigin = false;
+               $matchedOrigin = false;
                $allowTiming = false;
                $varyOrigin = true;
 
                if ( $originParam === '*' ) {
                        // Request for anonymous CORS
-                       $matchOrigin = true;
+                       // Technically we should check for the presence of an Origin header
+                       // and not process it as CORS if it's not set, but that would
+                       // require us to vary on Origin for all 'origin=*' requests which
+                       // we don't want to do.
+                       $matchedOrigin = true;
                        $allowOrigin = '*';
                        $allowCredentials = 'false';
                        $varyOrigin = false; // No need to vary
@@ -737,7 +741,7 @@ class ApiMain extends ApiBase {
                        }
 
                        $config = $this->getConfig();
-                       $matchOrigin = count( $origins ) === 1 && self::matchOrigin(
+                       $matchedOrigin = count( $origins ) === 1 && self::matchOrigin(
                                $originParam,
                                $config->get( 'CrossSiteAJAXdomains' ),
                                $config->get( 'CrossSiteAJAXdomainExceptions' )
@@ -748,19 +752,21 @@ class ApiMain extends ApiBase {
                        $allowTiming = $originHeader;
                }
 
-               if ( $matchOrigin ) {
+               if ( $matchedOrigin ) {
                        $requestedMethod = $request->getHeader( 'Access-Control-Request-Method' );
                        $preflight = $request->getMethod() === 'OPTIONS' && $requestedMethod !== false;
                        if ( $preflight ) {
                                // This is a CORS preflight request
                                if ( $requestedMethod !== 'POST' && $requestedMethod !== 'GET' ) {
                                        // If method is not a case-sensitive match, do not set any additional headers and terminate.
+                                       $response->header( 'MediaWiki-CORS-Rejection: Unsupported method requested in preflight' );
                                        return true;
                                }
                                // We allow the actual request to send the following headers
                                $requestedHeaders = $request->getHeader( 'Access-Control-Request-Headers' );
                                if ( $requestedHeaders !== false ) {
                                        if ( !self::matchRequestedHeaders( $requestedHeaders ) ) {
+                                               $response->header( 'MediaWiki-CORS-Rejection: Unsupported header requested in preflight' );
                                                return true;
                                        }
                                        $response->header( 'Access-Control-Allow-Headers: ' . $requestedHeaders );
@@ -768,6 +774,12 @@ class ApiMain extends ApiBase {
 
                                // We only allow the actual request to be GET or POST
                                $response->header( 'Access-Control-Allow-Methods: POST, GET' );
+                       } elseif ( $request->getMethod() !== 'POST' && $request->getMethod() !== 'GET' ) {
+                               // Unsupported non-preflight method, don't handle it as CORS
+                               $response->header(
+                                       'MediaWiki-CORS-Rejection: Unsupported method for simple request or actual request'
+                               );
+                               return true;
                        }
 
                        $response->header( "Access-Control-Allow-Origin: $allowOrigin" );
@@ -783,6 +795,8 @@ class ApiMain extends ApiBase {
                                        . 'MediaWiki-Login-Suppressed'
                                );
                        }
+               } else {
+                       $response->header( 'MediaWiki-CORS-Rejection: Origin mismatch' );
                }
 
                if ( $varyOrigin ) {