Merge pull request #4 from Flowpack/bugfix/handlePreflightInCorsMiddl…

…eware BUGFIX: The CORS middleware will create the response for an OPTIONS request without passing the request down through the process chain
Flowpack · Dec 11, 2024 · 7212c85 · 7212c85
2 parents 8ca6c1b + 10e5bf8
commit 7212c85
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 7 deletions.
diff --git a/Classes/Http/CorsHeaderMiddleware.php b/Classes/Http/CorsHeaderMiddleware.php
@@ -6,6 +6,7 @@
 
 use Lmc\HttpConstants\Header;
 use Neos\Flow\Annotations as Flow;
+use Psr\Http\Message\ResponseFactoryInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\MiddlewareInterface;
@@ -14,6 +15,12 @@
 
 class CorsHeaderMiddleware implements MiddlewareInterface
 {
+    /**
+     * @Flow\Inject
+     * @var ResponseFactoryInterface
+     */
+    protected $responseFactory;
+
     /**
      * @Flow\InjectConfiguration("enabled")
      */
@@ -73,15 +80,16 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
         }
 
         $this->initializeConfiguration();
-
-        $response = $handler->handle($request);
         $method = $request->getMethod();
 
-        // method type is not options, return early
-        if ($method == 'OPTIONS') {
+        // method type is OPTIONS, return early
+        if ($method === 'OPTIONS') {
             $this->logger->debug('CORS Component: Preflight request');
+            $response = $this->responseFactory->createResponse();
             return $this->handlePreflight($request, $response);
         }
+
+        $response = $handler->handle($request);
         return $this->handleRequest($request, $response);
     }
 

diff --git a/Tests/Unit/Http/CorsHeaderMiddlewareTest.php b/Tests/Unit/Http/CorsHeaderMiddlewareTest.php
@@ -10,6 +10,7 @@
 use PHPUnit\Framework\MockObject\Exception;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
+use Psr\Http\Message\ResponseFactoryInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\RequestHandlerInterface;
@@ -24,6 +25,7 @@ class CorsHeaderMiddlewareTest extends TestCase
     private readonly ResponseInterface&MockObject $responseMock;
     private readonly RequestHandlerInterface&MockObject $handlerMock;
     private readonly LoggerInterface&MockObject $logger;
+    private readonly ResponseFactoryInterface&MockObject $responseFactoryMock;
 
     /**
      * @throws Exception
@@ -35,12 +37,12 @@ protected function setUp(): void
         $this->requestMock = $this->createMock(ServerRequestInterface::class);
         $this->responseMock = $this->createMock(ResponseInterface::class);
         $this->handlerMock = $this->createMock(RequestHandlerInterface::class);
+        $this->responseFactoryMock = $this->createMock(ResponseFactoryInterface::class);
         $this->logger = $this->createMock(LoggerInterface::class);
 
         ObjectAccess::setProperty($this->middleware, 'enabled', true, true);
         ObjectAccess::setProperty($this->middleware, 'logger', $this->logger, true);
-
-        $this->handlerMock->expects($this->once())->method('handle')->willReturn($this->responseMock);
+        ObjectAccess::setProperty($this->middleware, 'responseFactory', $this->responseFactoryMock, true);
     }
 
     public function testMiddlewareIsNotEnabled(): void
@@ -65,6 +67,9 @@ public function testMiddlewarePreflightWithConfig(): void
             };
         });
 
+        // the process is not passed down the chain and a fresh response is created
+        $this->handlerMock->expects($this->never())->method('handle');
+        $this->responseFactoryMock->expects($this->once())->method('createResponse')->willReturn($this->responseMock);
         $this->responseMock->expects($this->exactly(5))->method('withHeader')->willReturnSelf();
 
         $this->middleware->process($this->requestMock, $this->handlerMock);
@@ -83,9 +88,12 @@ public function testMiddlewarePreflightWithWildcardConfig(): void
             };
         });
 
+        // the process is not passed down the chain and a fresh response is created
+        $this->handlerMock->expects($this->never())->method('handle');
+        $this->responseFactoryMock->expects($this->once())->method('createResponse')->willReturn($this->responseMock);
         $this->responseMock->expects($this->exactly(4))->method('withHeader')->willReturnSelf();
 
-         $this->middleware->process($this->requestMock, $this->handlerMock);
+        $this->middleware->process($this->requestMock, $this->handlerMock);
     }
 
     public function testMiddlewareActualRequestWithConfig(): void
@@ -100,6 +108,8 @@ public function testMiddlewareActualRequestWithConfig(): void
             };
         });
 
+        // request is passed down the chain
+        $this->handlerMock->expects($this->once())->method('handle')->willReturn($this->responseMock);
         $this->responseMock->expects($this->exactly(4))->method('withHeader')->willReturnSelf();
 
         $this->middleware->process($this->requestMock, $this->handlerMock);
@@ -117,6 +127,8 @@ public function testMiddlewareActualRequestWithWildcardConfig(): void
             };
         });
 
+        // request is passed down the chain
+        $this->handlerMock->expects($this->once())->method('handle')->willReturn($this->responseMock);
         $this->responseMock->expects($this->exactly(4))->method('withHeader')->willReturnSelf();
 
         $this->middleware->process($this->requestMock, $this->handlerMock);
@@ -142,6 +154,8 @@ public function testMiddlewareActualRequestWithWildcardOrigin(): void
             };
         });
 
+        // request is passed down the chain
+        $this->handlerMock->expects($this->once())->method('handle')->willReturn($this->responseMock);
         $this->responseMock->expects($this->exactly(4))->method('withHeader')->willReturnSelf();
 
         $this->middleware->process($this->requestMock, $this->handlerMock);
@@ -167,6 +181,8 @@ public function testMiddlewareActualRequestWithNotAllowedOrigin(): void
             };
         });
 
+        // request is passed down the chain
+        $this->handlerMock->expects($this->once())->method('handle')->willReturn($this->responseMock);
         $this->responseMock->expects($this->never())->method('withHeader')->willReturnSelf();
 
         $this->logger->expects($this->exactly(2))->method('debug');