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

Author: michalsn

system/Filters/Cors.php

@@ -100,6 +100,24 @@ private function createCorsService(?array $arguments): void
      */
     public function after(RequestInterface $request, ResponseInterface $response, $arguments = null)
     {
-        return null;
+        if (! $request instanceof IncomingRequest) {
+            return null;
+        }
+
+        $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
+        // have this header, so it will be cached separately.
+        if ($request->is('OPTIONS')) {
+            $response->appendHeader('Vary', 'Access-Control-Request-Method');
+        }
+
+        return $this->cors->addResponseHeaders($request, $response);
     }
 }

system/HTTP/Cors.php

@@ -227,4 +227,28 @@ private function setExposeHeaders(ResponseInterface $response): void
             );
         }
     }
+
+    /**
+     * 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;
+        }
+
+        if ($this->config['exposedHeaders'] !== [] && (! $response->hasHeader('Access-Control-Expose-Headers') || ! str_contains(
+            $response->getHeaderLine('Access-Control-Expose-Headers'),
+            implode(', ', $this->config['exposedHeaders']),
+        ))) {
+            return false;
+        }
+
+        return true;
+    }
 }

tests/system/Filters/CorsTest.php

@@ -16,6 +16,7 @@
 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 @@ private function handle(RequestInterface $request): ResponseInterface
         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 @@ public function testItAddsVaryAccessControlRequestMethodHeaderEvenIfItIsNormalOp
         // 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 @@ public function testAddResponseHeadersMultipleAllowedOriginsNotAllowed(): void
             $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

@@ -22,8 +22,6 @@ Message Changes
 Changes
 *******
 
-- **Cors:** From now on only the ``before`` filter is used. You can remove all the ``after`` filter occurrences from your configuration for CORS.
-
 ************
 Deprecations
 ************
@@ -33,7 +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. From now on all CORS headers are added in the ``before`` filter and the ``after`` filter is no longer used.
+- **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>`_

user_guide_src/source/libraries/cors/002.php

@@ -13,6 +13,7 @@ class Filters extends BaseFilters
         // ...
         'cors' => [
             'before' => ['api/*'],
+            'after'  => ['api/*'],
         ],
     ];
 }