Only return CORS headers in the response as required

- Split out responses of preflight and actual CORS requests
- If the request is not CORS valid, don't set the CORS response headers

Note that invalid CORS requests should not actually throw error
responses, the client should simply not handle the response because the
response does not have the right headers (it's a client side policy
error not an http error). We do throw a 403 for a mismatch with the
queryparam, but since that is 'outside' of the spec, that might be
appropriate.

Bug: T76701
Change-Id: Ib296c68babe5c0b380268ee7793b3d6d35b9c3e3

diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php
index a5287b6..ba22e39 100644 (file)
--- a/includes/api/ApiMain.php
+++ b/includes/api/ApiMain.php
@@ -487,6 +487,8 @@ class ApiMain extends ApiBase {
         * 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.
         * 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.

+        * http://www.w3.org/TR/cors/#resource-requests
+        * http://www.w3.org/TR/cors/#resource-preflight-requests

         *
         * @return bool False if the caller should abort (403 case), true otherwise (all other cases)
         */
         *
         * @return bool False if the caller should abort (403 case), true otherwise (all other cases)
         */


@@ -499,12 +501,14 @@ class ApiMain extends ApiBase {
 
                $request = $this->getRequest();
                $response = $request->response();
 
                $request = $this->getRequest();
                $response = $request->response();

+

                // Origin: header is a space-separated list of origins, check all of them
                $originHeader = $request->getHeader( 'Origin' );
                if ( $originHeader === false ) {
                        $origins = array();
                } else {
                // Origin: header is a space-separated list of origins, check all of them
                $originHeader = $request->getHeader( 'Origin' );
                if ( $originHeader === false ) {
                        $origins = array();
                } else {

-                       $origins = explode( ' ', $originHeader );
+                       $originHeader = trim( $originHeader );
+                       $origins = preg_split( '/\s+/', $originHeader );

                }
 
                if ( !in_array( $originParam, $origins ) ) {
                }
 
                if ( !in_array( $originParam, $origins ) ) {


@@ -519,19 +523,43 @@ class ApiMain extends ApiBase {
                }
 
                $config = $this->getConfig();
                }
 
                $config = $this->getConfig();

-               $matchOrigin = self::matchOrigin(
+               $matchOrigin = count( $origins ) === 1 && self::matchOrigin(

                        $originParam,
                        $config->get( 'CrossSiteAJAXdomains' ),
                        $config->get( 'CrossSiteAJAXdomainExceptions' )
                );
 
                if ( $matchOrigin ) {
                        $originParam,
                        $config->get( 'CrossSiteAJAXdomains' ),
                        $config->get( 'CrossSiteAJAXdomainExceptions' )
                );
 
                if ( $matchOrigin ) {

-                       $response->header( "Access-Control-Allow-Origin: $originParam" );
+                       $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.
+                                       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 ) ) {
+                                               return true;
+                                       }
+                                       $response->header( 'Access-Control-Allow-Headers: ' . $requestedHeaders );
+                               }
+
+                               // We only allow the actual request to be GET or POST
+                               $response->header( 'Access-Control-Allow-Methods: POST, GET' );
+                       }
+
+                       $response->header( "Access-Control-Allow-Origin: $originHeader" );

                        $response->header( 'Access-Control-Allow-Credentials: true' );
                        $response->header( 'Access-Control-Allow-Credentials: true' );

-                       $response->header( 'Access-Control-Allow-Headers: Api-User-Agent' );
-                       $this->getOutput()->addVaryHeader( 'Origin' );
+
+                       if ( !$preflight ) {
+                               $response->header( 'Access-Control-Expose-Headers: MediaWiki-API-Error, Retry-After, X-Database-Lag' );
+                       }

                }
 
                }
 

+               $this->getOutput()->addVaryHeader( 'Origin' );

                return true;
        }
 
                return true;
        }
 


@@ -560,6 +588,41 @@ class ApiMain extends ApiBase {
                return false;
        }
 
                return false;
        }
 

+       /**
+        * Attempt to validate the value of Access-Control-Request-Headers against a list
+        * of headers that we allow the follow up request to send.
+        *
+        * @param string $requestedHeaders Comma seperated list of HTTP headers
+        * @return bool True if all requested headers are in the list of allowed headers
+        */
+       protected static function matchRequestedHeaders( $requestedHeaders ) {
+               if ( trim( $requestedHeaders ) === '' ) {
+                       return true;
+               }
+               $requestedHeaders = explode( ',', $requestedHeaders );
+               $allowedAuthorHeaders = array_flip( array(
+                       /* simple headers (see spec) */
+                       'accept',
+                       'accept-language',
+                       'content-language',
+                       'content-type',
+                       /* non-authorable headers in XHR, which are however requested by some UAs */
+                       'accept-encoding',
+                       'dnt',
+                       'origin',
+                       /* MediaWiki whitelist */
+                       'api-user-agent',
+               ) );
+               foreach ( $requestedHeaders as $rHeader ) {
+                       $rHeader = strtolower( trim( $rHeader ) );
+                       if ( !isset( $allowedAuthorHeaders[$rHeader] ) ) {
+                               wfDebugLog( 'api', 'CORS preflight failed on requested header: ' . $rHeader );
+                               return false;
+                       }
+               }
+               return true;
+       }
+

        /**
         * Helper function to convert wildcard string into a regex
         * '*' => '.*?'
        /**
         * Helper function to convert wildcard string into a regex
         * '*' => '.*?'