fix: set headers for CORS (#9437)

* fix: set everything in the before filter for CORS

* fix append vary header

* cors: apply response headers in the after filter only if they are not already set

* cs fix

system/Filters/Cors.php

@@ -58,13 +58,10 @@ class Cors implements FilterInterface
 
         $this->createCorsService($arguments);
 
-        if (! $this->cors->isPreflightRequest($request)) {
-            return null;
-        }
-
         /** @var ResponseInterface $response */
         $response = service('response');
 
+        if ($this->cors->isPreflightRequest($request)) {
             $response = $this->cors->handlePreflightRequest($request, $response);
 
             // Always adds `Vary: Access-Control-Request-Method` header for cacheability.
@@ -76,6 +73,19 @@ class Cors implements FilterInterface
             return $response;
         }
 
+        if ($request->is('OPTIONS')) {
+            // Always adds `Vary: Access-Control-Request-Method` header for cacheability.
+            // If there is an intermediate cache server such as a CDN, if a plain
+            // OPTIONS request is sent, it may be cached. But valid preflight requests
+            // have this header, so it will be cached separately.
+            $response->appendHeader('Vary', 'Access-Control-Request-Method');
+        }
+
+        $this->cors->addResponseHeaders($request, $response);
+
+        return null;
+    }
+
     /**
      * @param list<string>|null $arguments
      */
@@ -87,8 +97,6 @@ class Cors implements FilterInterface
 
     /**
      * @param list<string>|null $arguments
-     *
-     * @return ResponseInterface|null
      */
     public function after(RequestInterface $request, ResponseInterface $response, $arguments = null)
     {
@@ -98,6 +106,10 @@ class Cors implements FilterInterface
 
         $this->createCorsService($arguments);
 
+        if ($this->cors->hasResponseHeaders($request, $response)) {
+            return null;
+        }
+
         // Always adds `Vary: Access-Control-Request-Method` header for cacheability.
         // If there is an intermediate cache server such as a CDN, if a plain
         // OPTIONS request is sent, it may be cached. But valid preflight requests

system/HTTP/Cors.php

@@ -227,4 +227,24 @@ class Cors
             );
         }
     }
+
+    /**
+     * Check if response headers were set
+     */
+    public function hasResponseHeaders(RequestInterface $request, ResponseInterface $response): bool
+    {
+        if (! $response->hasHeader('Access-Control-Allow-Origin')) {
+            return false;
+        }
+
+        if ($this->config['supportsCredentials']
+            && ! $response->hasHeader('Access-Control-Allow-Credentials')) {
+            return false;
+        }
+
+        return ! ($this->config['exposedHeaders'] !== [] && (! $response->hasHeader('Access-Control-Expose-Headers') || ! str_contains(
+            $response->getHeaderLine('Access-Control-Expose-Headers'),
+            implode(', ', $this->config['exposedHeaders']),
+        )));
+    }
 }

tests/system/Filters/CorsTest.php

@@ -16,6 +16,7 @@ namespace CodeIgniter\Filters;
 use CodeIgniter\Exceptions\ConfigException;
 use CodeIgniter\HTTP\CLIRequest;
 use CodeIgniter\HTTP\IncomingRequest;
+use CodeIgniter\HTTP\RedirectResponse;
 use CodeIgniter\HTTP\RequestInterface;
 use CodeIgniter\HTTP\ResponseInterface;
 use CodeIgniter\HTTP\SiteURI;
@@ -154,6 +155,25 @@ final class CorsTest extends CIUnitTestCase
         return $response;
     }
 
+    private function handleRedirect(RequestInterface $request): ResponseInterface
+    {
+        $response = $this->cors->before($request);
+        if ($response instanceof ResponseInterface) {
+            $this->response = $response;
+
+            return $response;
+        }
+
+        $response = service('redirectresponse');
+
+        $response = $this->cors->after($request, $response);
+        $response ??= service('redirectresponse');
+
+        $this->response = $response;
+
+        return $response;
+    }
+
     public function testItDoesModifyOnARequestWithSameOrigin(): void
     {
         $this->cors = $this->createCors(['allowedOrigins' => ['*']]);
@@ -461,4 +481,29 @@ final class CorsTest extends CIUnitTestCase
         // Always adds `Vary: Access-Control-Request-Method` header.
         $this->assertHeader('Vary', 'Access-Control-Request-Method');
     }
+
+    public function testItReturnsAllowOriginHeaderOnValidActualRequestWithRedirect(): void
+    {
+        $this->cors = $this->createCors();
+        $request    = $this->createValidActualRequest();
+
+        $response = $this->handleRedirect($request);
+
+        $this->assertInstanceOf(RedirectResponse::class, $response);
+        $this->assertTrue($response->hasHeader('Access-Control-Allow-Origin'));
+        $this->assertHeader('Access-Control-Allow-Origin', 'http://localhost');
+    }
+
+    public function testItReturnsAllowOriginHeaderOnAllowAllOriginRequestWithRedirect(): void
+    {
+        $this->cors = $this->createCors(['allowedOrigins' => ['*']]);
+        $request    = $this->createRequest();
+        $request->setHeader('Origin', 'http://localhost');
+
+        $response = $this->handleRedirect($request);
+
+        $this->assertInstanceOf(RedirectResponse::class, $response);
+        $this->assertTrue($response->hasHeader('Access-Control-Allow-Origin'));
+        $this->assertHeader('Access-Control-Allow-Origin', '*');
+    }
 }

tests/system/HTTP/CorsTest.php

@@ -521,4 +521,41 @@ final class CorsTest extends CIUnitTestCase
             $response->hasHeader('Access-Control-Allow-Methods'),
         );
     }
+
+    public function testHasResponseHeadersFalse(): void
+    {
+        $config                   = $this->getDefaultConfig();
+        $config['allowedOrigins'] = ['http://localhost:8080'];
+        $config['allowedMethods'] = ['GET', 'POST', 'PUT'];
+        $cors                     = $this->createCors($config);
+
+        $request = $this->createRequest()
+            ->withMethod('GET')
+            ->setHeader('Origin', 'http://localhost:8080');
+
+        $response = service('redirectresponse', null, false);
+
+        $this->assertFalse(
+            $cors->hasResponseHeaders($request, $response),
+        );
+    }
+
+    public function testHasResponseHeadersTrue(): void
+    {
+        $config                   = $this->getDefaultConfig();
+        $config['allowedOrigins'] = ['http://localhost:8080'];
+        $config['allowedMethods'] = ['GET', 'POST'];
+        $cors                     = $this->createCors($config);
+
+        $request = $this->createRequest()
+            ->withMethod('GET')
+            ->setHeader('Origin', 'http://localhost:8080');
+
+        $response = service('redirectresponse', null, false);
+        $response = $cors->addResponseHeaders($request, $response);
+
+        $this->assertTrue(
+            $cors->hasResponseHeaders($request, $response),
+        );
+    }
 }

user_guide_src/source/changelogs/v4.6.1.rst

@@ -31,6 +31,7 @@ Bugs Fixed
 **********
 
 - **CURLRequest:** Fixed an issue where multiple header sections appeared in the CURL response body during multiple redirects from the target server.
+- **Cors:** Fixed a bug in the Cors filter that caused the appropriate headers to not be added when another filter returned a response object in the ``before`` filter.
 
 See the repo's
 `CHANGELOG.md <https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md>`_