change!: rename config throw-invalid-directive-exception to throw-exception-on-configuration-error (#18)

#16

Author: t-heuser

Classes/Factory/PolicyFactory.php

@@ -18,9 +18,9 @@
 class PolicyFactory
 {
     /**
-     * @Flow\InjectConfiguration(path="throw-invalid-directive-exception")
+     * @Flow\InjectConfiguration(path="throw-exception-on-configuration-error")
      */
-    protected bool $throwInvalidDirectiveException;
+    protected bool $throwExceptionOnConfigurationError;
 
     /**
      * @Flow\Inject
@@ -60,15 +60,14 @@ public function create(Nonce $nonce, array $defaultDirectives, array $customDire
         foreach ($resultDirectives as $directive => $values) {
             try {
                 $policy->addDirective($directive, $values);
-            } catch (InvalidDirectiveException $e
-            ) {
-                if ($this->throwInvalidDirectiveException) {
+            } catch (InvalidDirectiveException $exception) {
+                if ($this->throwExceptionOnConfigurationError) {
                     // For development we want to make sure directives are configured correctly.
-                    throw $e;
+                    throw $exception;
                 } else {
                     // In production we just log the error and continue. If a directive is invalid, we still
                     // want to apply the rest of the policy.
-                    $this->logger->critical($e->getMessage());
+                    $this->logger->critical($exception->getMessage());
                     continue;
                 }
             }

Classes/Http/CspHeaderMiddleware.php

@@ -50,11 +50,10 @@ class CspHeaderMiddleware implements MiddlewareInterface
      */
     protected array $policies;
 
-    // TODO: rename to throw-on-configuration-error in next major version
     /**
-     * @Flow\InjectConfiguration(path="throw-invalid-directive-exception")
+     * @Flow\InjectConfiguration(path="throw-exception-on-configuration-error")
      */
-    protected bool $throwInvalidDirectiveException;
+    protected bool $throwExceptionOnConfigurationError;
 
     /**
      * @Flow\Inject
@@ -69,7 +68,7 @@ class CspHeaderMiddleware implements MiddlewareInterface
     public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
     {
         $response = $handler->handle($request);
-        if (! $this->enabled) {
+        if (!$this->enabled) {
             return $response;
         }
 
@@ -102,7 +101,7 @@ private function getPolicyByCurrentContext(ServerRequestInterface $request): Pol
             $result = preg_match('#' . str_replace('#', '\#', $pattern) . '#', $path);
             if ($result === false) {
                 $message = sprintf('Invalid matchUri pattern "%s": %s', $pattern, preg_last_error_msg());
-                if ($this->throwInvalidDirectiveException) {
+                if ($this->throwExceptionOnConfigurationError) {
                     throw new InvalidArgumentException($message);
                 }
                 $this->logger->critical($message);
@@ -141,14 +140,14 @@ private function addNonceToTags(string $markup): string
     }
 
     /**
-     * @param  string[]  $tagNames
+     * @param string[] $tagNames
      */
     private function checkTagAndReplaceUsingACallback(
         array $tagNames,
         string $contentMarkup,
         callable $hitCallback
     ): string {
-        $regex = '/<('.implode('|', $tagNames).').*?>/';
+        $regex = '/<(' . implode('|', $tagNames) . ').*?>/';
 
         return preg_replace_callback(
             $regex,

Configuration/Production/Settings.yaml

@@ -1,3 +1,3 @@
 Flowpack:
   ContentSecurityPolicy:
-    throw-invalid-directive-exception: false
+    throw-exception-on-configuration-error: false

Configuration/Settings.yaml

@@ -2,7 +2,7 @@ Flowpack:
   ContentSecurityPolicy:
     enabled: true
     report-only: false
-    throw-invalid-directive-exception: true
+    throw-exception-on-configuration-error: true
     content-security-policy:
       default:
         base-uri:

README.md

@@ -1,16 +1,19 @@
 # Flowpack.ContentSecurityPolicy
 
 <!-- TOC -->
+
 * [Flowpack.ContentSecurityPolicy](#flowpackcontentsecuritypolicy)
-  * [Introduction](#introduction)
-  * [Usage](#usage)
-  * [Custom directives and values](#custom-directives-and-values)
-    * [Show CSP configuration](#show-csp-configuration)
-  * [Disable or report only](#disable-or-report-only)
-  * [Nonce](#nonce)
-  * [Backend](#backend)
-    * [Custom backend routes](#custom-backend-routes)
-  * [Thank you](#thank-you)
+    * [Introduction](#introduction)
+    * [Usage](#usage)
+    * [Custom directives and values](#custom-directives-and-values)
+        * [Show CSP configuration](#show-csp-configuration)
+    * [Configuration errors](#configuration-errors)
+    * [Disable or report only](#disable-or-report-only)
+    * [Nonce](#nonce)
+    * [Backend](#backend)
+        * [Custom backend routes](#custom-backend-routes)
+    * [Thank you](#thank-you)
+
 <!-- TOC -->
 
 ## Introduction
@@ -94,6 +97,20 @@ If you fully want to override the entire default config then just override the d
 To show the parsed configuration, the built-in command `./flow cspconfig:show` can be used.
 It shows all directives used by the frontend and the backend.
 
+## Throwing exceptions on configuration errors
+
+By default, in production, invalid directives are logged as critical errors and skipped so the rest of the policy is
+still applied.
+Also invalid custom backend routes are logged and ignored in production.
+In development, exceptions are throw instead, which surfaces misconfiguration immediately.
+You can change that behavior like this:
+
+```yaml
+Flowpack:
+  ContentSecurityPolicy:
+    throw-exception-on-configuration-error: true
+```
+
 ## Disable or report only
 
 To disable the header simply set `enabled` to false.

Tests/Unit/Factory/PolicyFactoryTest.php

@@ -38,7 +38,7 @@ protected function setUp(): void
 
         $this->policyFactoryReflection = new ReflectionClass($this->policyFactory);
         $this->policyFactoryReflection->getProperty('logger')->setValue($this->policyFactory, $this->loggerMock);
-        $this->policyFactoryReflection->getProperty('throwInvalidDirectiveException')->setValue(
+        $this->policyFactoryReflection->getProperty('throwExceptionOnConfigurationError')->setValue(
             $this->policyFactory,
             true
         );
@@ -100,7 +100,7 @@ public function testCreateShouldFailWithInvalidDirective(): void
     public function testCreateShouldLogInvalidDirectiveInProduction(): void
     {
         $nonceMock = $this->createMock(Nonce::class);
-        $this->policyFactoryReflection->getProperty('throwInvalidDirectiveException')->setValue(
+        $this->policyFactoryReflection->getProperty('throwExceptionOnConfigurationError')->setValue(
             $this->policyFactory,
             false
         );
@@ -114,7 +114,7 @@ public function testCreateShouldLogInvalidDirectiveInProduction(): void
         $this->loggerMock->expects($this->once())->method('critical');
         $this->policyFactory->create($nonceMock, $defaultDirective, $customDirective);
 
-        $this->policyFactoryReflection->getProperty('throwInvalidDirectiveException')->setValue(
+        $this->policyFactoryReflection->getProperty('throwExceptionOnConfigurationError')->setValue(
             $this->policyFactory,
             true
         );

Tests/Unit/Http/CspHeaderMiddlewareTest.php

@@ -21,6 +21,7 @@
 use Psr\Log\LoggerInterface;
 use ReflectionClass;
 use Throwable;
+
 use function PHPUnit\Framework\once;
 
 #[CoversClass(CspHeaderMiddleware::class)]
@@ -78,7 +79,7 @@ protected function setUp(): void
             ['backend' => ['matchUris' => ['^/neos']], 'custom-backend' => ['matchUris' => []]]
         );
 
-        $reflectionProperty = $this->middlewareReflection->getProperty('throwInvalidDirectiveException');
+        $reflectionProperty = $this->middlewareReflection->getProperty('throwExceptionOnConfigurationError');
         $reflectionProperty->setValue($this->middleware, true);
 
         $reflectionProperty = $this->middlewareReflection->getProperty('logger');
@@ -205,7 +206,7 @@ public function testProcessThrowsOnInvalidMatchUriPattern(): void
 
     public function testProcessLogsInvalidMatchUriPatternInProduction(): void
     {
-        $reflectionProperty = $this->middlewareReflection->getProperty('throwInvalidDirectiveException');
+        $reflectionProperty = $this->middlewareReflection->getProperty('throwExceptionOnConfigurationError');
         $reflectionProperty->setValue($this->middleware, false);
 
         $reflectionProperty = $this->middlewareReflection->getProperty('policies');