Merge pull request #72 from Oliboy50/rework-forced-option

rework forced_allow_origin_value option to always set the header
nelmio · Jan 22, 2017 · 3c7a643 · 3c7a643
2 parents 58c2ae3 + f24d106
commit 3c7a643
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 24 deletions.
diff --git a/EventListener/CorsListener.php b/EventListener/CorsListener.php
@@ -55,12 +55,17 @@ public function onKernelRequest(GetResponseEvent $event)
 
         $request = $event->getRequest();
 
-        // skip if not a CORS request
-        if (!$request->headers->has('Origin') || $request->headers->get('Origin') == $request->getSchemeAndHttpHost()) {
+        if (!$options = $this->configurationResolver->getOptions($request)) {
             return;
         }
 
-        if (!$options = $this->configurationResolver->getOptions($request)) {
+        // if the "forced_allow_origin_value" option is set, add a listener which will set or override the "Access-Control-Allow-Origin" header
+        if (!empty($options['forced_allow_origin_value'])) {
+            $this->dispatcher->addListener('kernel.response', array($this, 'forceAccessControlAllowOriginHeader'), -1);
+        }
+
+        // skip if not a CORS request
+        if (!$request->headers->has('Origin') || $request->headers->get('Origin') == $request->getSchemeAndHttpHost()) {
             return;
         }
 
@@ -75,7 +80,7 @@ public function onKernelRequest(GetResponseEvent $event)
             return;
         }
 
-        $this->dispatcher->addListener('kernel.response', array($this, 'onKernelResponse'));
+        $this->dispatcher->addListener('kernel.response', array($this, 'onKernelResponse'), 0);
     }
 
     public function onKernelResponse(FilterResponseEvent $event)
@@ -90,11 +95,7 @@ public function onKernelResponse(FilterResponseEvent $event)
 
         $response = $event->getResponse();
         // add CORS response headers
-        $response->headers->set('Access-Control-Allow-Origin',
-            !empty($options['forced_allow_origin_value'])
-                ? $options['forced_allow_origin_value']
-                : $request->headers->get('Origin')
-        );
+        $response->headers->set('Access-Control-Allow-Origin', $request->headers->get('Origin'));
         if ($options['allow_credentials']) {
             $response->headers->set('Access-Control-Allow-Credentials', 'true');
         }
@@ -103,6 +104,15 @@ public function onKernelResponse(FilterResponseEvent $event)
         }
     }
 
+    public function forceAccessControlAllowOriginHeader(FilterResponseEvent $event)
+    {
+        if (!$options = $this->configurationResolver->getOptions($request = $event->getRequest())) {
+            return;
+        }
+
+        $event->getResponse()->headers->set('Access-Control-Allow-Origin', $options['forced_allow_origin_value']);
+    }
+
     protected function getPreflightResponse(Request $request, array $options)
     {
         $response = new Response();
@@ -132,11 +142,7 @@ protected function getPreflightResponse(Request $request, array $options)
             return $response;
         }
 
-        $response->headers->set('Access-Control-Allow-Origin',
-            !empty($options['forced_allow_origin_value'])
-                ? $options['forced_allow_origin_value']
-                : $request->headers->get('Origin')
-        );
+        $response->headers->set('Access-Control-Allow-Origin', $request->headers->get('Origin'));
 
         // check request method
         if (!in_array(strtoupper($request->headers->get('Access-Control-Request-Method')), $options['allow_methods'], true)) {

diff --git a/Tests/CorsListenerTest.php b/Tests/CorsListenerTest.php
@@ -82,7 +82,7 @@ public function testPreflightedRequest()
         $callback = null;
         $dispatcher = m::mock('Symfony\Component\EventDispatcher\EventDispatcherInterface');
         $dispatcher->shouldReceive('addListener')->once()
-            ->with('kernel.response', m::type('callable'))
+            ->with('kernel.response', m::type('callable'), 0)
             ->andReturnUsing(function ($cb) use (&$callback) {
                 $callback = $cb;
             });
@@ -136,16 +136,20 @@ public function testPreflightedRequestWithForcedAllowOriginValue()
         $req->headers->set('Access-Control-Request-Method', 'GET');
 
         $dispatcher = m::mock('Symfony\Component\EventDispatcher\EventDispatcherInterface');
+        $dispatcher->shouldReceive('addListener')->once()->with('kernel.response', m::type('callable'), -1);
+
         $event = new GetResponseEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MASTER_REQUEST);
         $this->getListener($dispatcher, $options)->onKernelRequest($event);
+        $event = new FilterResponseEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MASTER_REQUEST, $event->getResponse());
+        $this->getListener($dispatcher, $options)->forceAccessControlAllowOriginHeader($event);
         $resp = $event->getResponse();
         $this->assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp);
         $this->assertEquals(200, $resp->getStatusCode());
         $this->assertEquals('*', $resp->headers->get('Access-Control-Allow-Origin'));
         $this->assertEquals('GET', $resp->headers->get('Access-Control-Allow-Methods'));
 
         // allow_origin does not match origin header
-        // => 'Access-Control-Allow-Origin' should be 'null'
+        // => 'Access-Control-Allow-Origin' should be equal to "forced_allow_origin_value" (i.e. '*')
         $options = array(
             'allow_origin' => array(),
             'allow_methods' => array('GET'),
@@ -157,12 +161,16 @@ public function testPreflightedRequestWithForcedAllowOriginValue()
         $req->headers->set('Access-Control-Request-Method', 'GET');
 
         $dispatcher = m::mock('Symfony\Component\EventDispatcher\EventDispatcherInterface');
+        $dispatcher->shouldReceive('addListener')->once()->with('kernel.response', m::type('callable'), -1);
+
         $event = new GetResponseEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MASTER_REQUEST);
         $this->getListener($dispatcher, $options)->onKernelRequest($event);
+        $event = new FilterResponseEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MASTER_REQUEST, $event->getResponse());
+        $this->getListener($dispatcher, $options)->forceAccessControlAllowOriginHeader($event);
         $resp = $event->getResponse();
         $this->assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp);
         $this->assertEquals(200, $resp->getStatusCode());
-        $this->assertEquals('null', $resp->headers->get('Access-Control-Allow-Origin'));
+        $this->assertEquals('*', $resp->headers->get('Access-Control-Allow-Origin'));
         $this->assertEquals('GET', $resp->headers->get('Access-Control-Allow-Methods'));
     }
 
@@ -221,28 +229,36 @@ public function testRequestWithForcedAllowOriginValue()
         $req->headers->set('Origin', 'http://example.com');
 
         $dispatcher = m::mock('Symfony\Component\EventDispatcher\EventDispatcherInterface');
-        $dispatcher->shouldReceive('addListener')->once()->with('kernel.response', m::type('callable'));
+        $dispatcher->shouldReceive('addListener')->twice()->with('kernel.response', m::type('callable'), m::type('integer'));
 
         $event = new GetResponseEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MASTER_REQUEST);
         $this->getListener($dispatcher, $options)->onKernelRequest($event);
         $event = new FilterResponseEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MASTER_REQUEST, new Response());
         $this->getListener($dispatcher, $options)->onKernelResponse($event);
+        $this->getListener($dispatcher, $options)->forceAccessControlAllowOriginHeader($event);
         $resp = $event->getResponse();
         $this->assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp);
         $this->assertEquals(200, $resp->getStatusCode());
         $this->assertEquals('http://example.com http://huh-lala.foobar', $resp->headers->get('Access-Control-Allow-Origin'));
 
-        // allow_origin does not match origin header
-        // => CorsListener should not interfere with the response
+        // request without "Origin" header
+        // => 'Access-Control-Allow-Origin' should be equal to "forced_allow_origin_value" (i.e. '*')
+        $options = array(
+            'forced_allow_origin_value' => '*',
+        );
+
         $req = Request::create('/foo', 'GET');
-        $req->headers->set('Origin', 'http://evil.com');
 
         $dispatcher = m::mock('Symfony\Component\EventDispatcher\EventDispatcherInterface');
-        $dispatcher->shouldReceive('addListener')->times(0);
+        $dispatcher->shouldReceive('addListener')->once()->with('kernel.response', m::type('callable'), -1);
 
         $event = new GetResponseEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MASTER_REQUEST);
         $this->getListener($dispatcher, $options)->onKernelRequest($event);
-
-        $this->assertNull($event->getResponse());
+        $event = new FilterResponseEvent(m::mock('Symfony\Component\HttpKernel\HttpKernelInterface'), $req, HttpKernelInterface::MASTER_REQUEST, new Response());
+        $this->getListener($dispatcher, $options)->forceAccessControlAllowOriginHeader($event);
+        $resp = $event->getResponse();
+        $this->assertInstanceOf('Symfony\Component\HttpFoundation\Response', $resp);
+        $this->assertEquals(200, $resp->getStatusCode());
+        $this->assertEquals('*', $resp->headers->get('Access-Control-Allow-Origin'));
     }
 }