Fix on CORS implementation.

Added logic to discard same Origin requests.
Implements: blueprint openid-oauth2-user-service

Change-Id: Idee3c3e5498c83b62eb81af42e9627e17f6b5eb8

app/config/cors.php

@@ -15,7 +15,7 @@ return array(
     /**
      * http://www.w3.org/TR/cors/#access-control-allow-headers-response-header
      */
-    'AllowedHeaders'      => 'origin, content-type, accept, authorization',
+    'AllowedHeaders'      => 'origin, content-type, accept, authorization, x-requested-with',
     /**
      * http://www.w3.org/TR/cors/#access-control-allow-methods-response-header
      */

app/services/oauth2/CORS/CORSMiddleware.php

@@ -21,11 +21,10 @@ use Config;
  */
 class CORSMiddleware {
 
-    private $app;
     private $endpoint_service;
     private $cache_service;
     private $origin_service;
-    private $modify_response = false;
+    private $actual_request = false;
     private $headers = array();
     private $allowed_headers;
     private $allowed_methods;
@@ -39,7 +38,7 @@ class CORSMiddleware {
         'origin',
     );
 
-    const DefaultAllowedHeaders =  'origin, content-type, accept, authorization';
+    const DefaultAllowedHeaders = 'origin, content-type, accept, authorization, x-requested-with';
     const DefaultAllowedMethods = 'GET, POST, OPTIONS, PUT, DELETE';
 
     public function __construct(IApiEndpointService $endpoint_service,
@@ -49,50 +48,73 @@ class CORSMiddleware {
         $this->endpoint_service  = $endpoint_service;
         $this->cache_service     = $cache_service;
         $this->origin_service    = $origin_service;
+
         $this->allowed_headers   = Config::get('cors.AllowedHeaders',self::DefaultAllowedHeaders);
         $this->allowed_methods   = Config::get('cors.AllowedMethods',self::DefaultAllowedMethods);
     }
 
+    /**
+     * User agents can discover via a preflight request whether a cross-origin resource is prepared to
+     * accept requests, using a non-simple method, from a given origin.
+     * @param Request $request
+     * @param IApiEndpoint $endpoint
+     * @return Response
+     */
     private function makePreflightResponse(Request $request, IApiEndpoint $endpoint){
 
         $response = new Response();
 
         $allow_credentials = Config::get('cors.AllowCredentials', '');
+
         if(!empty($allow_credentials)){
+            // The Access-Control-Allow-Credentials header indicates whether the response to request
+            // can be exposed when the omit credentials flag is unset. When part of the response to a preflight request
+            // it indicates that the actual request can include user credentials.
             $response->headers->set('Access-Control-Allow-Credentials',$allow_credentials );
         }
 
         if(Config::get('cors.UsePreflightCaching', false)){
+            // The Access-Control-Max-Age header indicates how long the response can be cached, so that for
+            // subsequent requests, within the specified time, no preflight request has to be made.
             $response->headers->set('Access-Control-Max-Age', Config::get('cors.MaxAge', 32000));
         }
-
+        // The Access-Control-Allow-Headers header indicates, as part of the response to a preflight request,
+        // which header field names can be used during the actual request
         $response->headers->set('Access-Control-Allow-Headers', $this->allowed_headers);
 
-
         if (!$this->checkOrigin($request)) {
             $response->headers->set('Access-Control-Allow-Origin', 'null');
             return $response;
         }
+
         $response->headers->set('Access-Control-Allow-Origin', $request->headers->get('Origin'));
 
+        // The Access-Control-Request-Method header indicates which method will be used in the actual
+        // request as part of the preflight request
         // check request method
+
         if ($request->headers->get('Access-Control-Request-Method') != $endpoint->getHttpMethod()) {
             $response->setStatusCode(405);
             return $response;
         }
-
+        //The Access-Control-Allow-Methods header indicates, as part of the response to a preflight request,
+        // which methods can be used during the actual request.
         $response->headers->set('Access-Control-Allow-Methods', $this->allowed_methods);
 
-        // check request headers
+        // The Access-Control-Request-Headers header indicates which headers will be used in the actual request
-        $allow_headers = explode(', ',$this->allowed_headers);
+        // as part of the preflight request.
 
         $headers = $request->headers->get('Access-Control-Request-Headers');
+
         if ($headers) {
-            $headers = trim(strtolower($headers));
+            $headers       = trim(strtolower($headers));
+            $allow_headers = explode(', ',$this->allowed_headers);
             foreach (preg_split('{, *}', $headers) as $header) {
+                //if they are simple headers then skip them
                 if (in_array($header, self::$simple_headers, true)) {
                     continue;
                 }
+                //check is the requested header is on the list of allowed headers
                 if (!in_array($header, $allow_headers, true)) {
                     $response->setStatusCode(400);
                     $response->setContent('Unauthorized header '.$header);
@@ -100,7 +122,7 @@ class CORSMiddleware {
                 }
             }
         }
-
+        //OK - No Content
         $response->setStatusCode(204);
         return $response;
     }
@@ -120,10 +142,44 @@ class CORSMiddleware {
 
     public function verifyRequest($request){
         try{
-            // skip if not a CORS request
+            /**
+             * The presence of the Origin header does not necessarily mean that the request is a cross-origin request.
+             * While all cross-origin requests will contain an Origin header,
+             * some same-origin requests might have one as well. For example, Firefox doesn't include an
+             * Origin header on same-origin requests. But Chrome and Safari include an Origin header on
+             * same-origin POST/PUT/DELETE requests (same-origin GET requests will not have an Origin header).
+             */
+
             if (!$request->headers->has('Origin')) {
                 return;
             }
+            //https://www.owasp.org/index.php/CORS_OriginHeaderScrutiny
+            $origin      = $request->headers->get('Origin',null,false);
+            $host        = $request->headers->get('Host',null,false);
+            if(is_array($origin) && count($origin)>1){
+                // If we reach this point it means that we have multiple instance of the "Origin" header
+                $response = new Response();
+                $response->setStatusCode(403);
+                return $response;
+            }
+            //now get the first one
+            $origin      = $request->headers->get('Origin');
+            $server_name = isset($_SERVER['SERVER_NAME'])?$_SERVER['SERVER_NAME']:null;
+            $origin_host = @parse_url($origin,PHP_URL_HOST);
+
+            //Have only one and non empty instance of the host header,
+            if(is_array($host) && count($host)>1){
+                // If we reach this point it means that we have multiple instance of the "Host" header
+                $response = new Response();
+                $response->setStatusCode(403);
+                return;
+            }
+            //now get the first one
+            $host = $request->headers->get('Host');
+
+            if(is_null($host) || $server_name != $host || is_null($origin_host) || $origin_host == $server_name){
+                return;
+            }
 
             $method    = $request->getMethod();
             $preflight = false;
@@ -131,21 +187,18 @@ class CORSMiddleware {
             //preflight checks
             if ($method === 'OPTIONS') {
                 $request_method  = $request->headers->get('Access-Control-Request-Method');
-                if(is_null($request_method)){
+                if(!is_null($request_method)){
-                    Log::warning('CORS: not a valid preflight request!');
+                    // sets the original method on request in order to be able to find the
-                    return;
+                    // correct route
+                    $request->setMethod($request_method);
+                    $preflight = true;
                 }
-                // sets the original method on request in order to be able to find the
-                // correct route
-                $request->setMethod($request_method);
-                $preflight = true;
             }
 
             //gets routes from container and try to find the route
             $router    = App::make('router');
             $routes    = $router->getRoutes();
             $route     = $routes->match($request);
-
             $url       = $route->getPath();
 
             if(strpos($url, '/') != 0){
@@ -165,33 +218,58 @@ class CORSMiddleware {
                 else if(!$endpoint->supportCORS()){
                     Log::warning(sprintf("endpoint %s does not support CORS.",$url));
                 }
-
                 return;
             }
 
-            // perform preflight checks
+            //perform preflight checks
             if ($preflight) {
                 return $this->makePreflightResponse($request,$endpoint);
             }
-
+            //Actual Request
             if (!$this->checkOrigin($request)) {
                 return new Response('', 403, array('Access-Control-Allow-Origin' => 'null'));
             }
 
-            $this->modify_response = true;
+            $this->actual_request = true;
 
             // Save response headers
             $this->headers['Access-Control-Allow-Origin'] =  $request->headers->get('Origin');
-            $this->headers['Access-Control-Allow-Credentials'] = 'true';
+            $allow_credentials = Config::get('cors.AllowCredentials', '');
+            if(!empty($allow_credentials)){
+                // The Access-Control-Allow-Credentials header indicates whether the response to request
+                // can be exposed when the omit credentials flag is unset. When part of the response to a preflight request
+                // it indicates that the actual request can include user credentials.
+                $this->headers['Access-Control-Allow-Credentials'] = $allow_credentials ;
+            }
+
+            /**
+             * During a CORS request, the getResponseHeader() method can only access simple response headers.
+             * Simple response headers are defined as follows:
+             ** Cache-Control
+             ** Content-Language
+             ** Content-Type
+             ** Expires
+             ** Last-Modified
+             ** Pragma
+             * If you want clients to be able to access other headers,
+             * you have to use the Access-Control-Expose-Headers header.
+             * The value of this header is a comma-delimited list of response headers you want to expose
+             * to the client.
+             */
+            $exposed_headers = Config::get('cors.ExposedHeaders', '');
+            if(!empty($exposed_headers)){
+                $this->headers['Access-Control-Expose-Headers'] = $exposed_headers ;
+            }
+
         }
         catch(Exception $ex){
             Log::error($ex);
         }
     }
 
-    public function modifyResponse($request,$response)
+    public function modifyResponse($request, $response)
     {
-        if(!$this->modify_response){
+        if(!$this->actual_request){
             return $response;
         }
         // add CORS response headers

app/tests/OAuth2UserServiceApiTest.php

@@ -100,4 +100,20 @@ class OAuth2UserServiceApiTest  extends TestCase {
         $content   = $response->getContent();
         $user_info = json_decode($content);
     }
+
+    public function testGetInfoCORS(){
+        $response = $this->action("OPTION", "OAuth2UserApiController@me",
+            array(),
+            array(),
+            array(),
+            array(
+                "HTTP_Authorization" => " Bearer " .$this->access_token,
+                'HTTP_Origin' => array('www.test.com','www.test1.com'),
+                'HTTP_Access-Control-Request-Method'=>'GET',
+            ));
+
+        $this->assertResponseStatus(403);
+        $content   = $response->getContent();
+        $user_info = json_decode($content);
+    }
 }