diff --git a/Classes/Factory/PolicyFactory.php b/Classes/Factory/PolicyFactory.php index bdab27d..d45a572 100644 --- a/Classes/Factory/PolicyFactory.php +++ b/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; } } diff --git a/Classes/Http/CspHeaderMiddleware.php b/Classes/Http/CspHeaderMiddleware.php index c608adc..402b30a 100644 --- a/Classes/Http/CspHeaderMiddleware.php +++ b/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, diff --git a/Configuration/Production/Settings.yaml b/Configuration/Production/Settings.yaml index a958078..4e9f5a9 100644 --- a/Configuration/Production/Settings.yaml +++ b/Configuration/Production/Settings.yaml @@ -1,3 +1,3 @@ Flowpack: ContentSecurityPolicy: - throw-invalid-directive-exception: false + throw-exception-on-configuration-error: false diff --git a/Configuration/Settings.yaml b/Configuration/Settings.yaml index c797146..f703666 100644 --- a/Configuration/Settings.yaml +++ b/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: diff --git a/README.md b/README.md index caf4056..ce02fca 100644 --- a/README.md +++ b/README.md @@ -1,16 +1,19 @@ # Flowpack.ContentSecurityPolicy + * [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) + ## 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. diff --git a/Tests/Unit/Factory/PolicyFactoryTest.php b/Tests/Unit/Factory/PolicyFactoryTest.php index 3d57c54..6363c2b 100644 --- a/Tests/Unit/Factory/PolicyFactoryTest.php +++ b/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 ); diff --git a/Tests/Unit/Http/CspHeaderMiddlewareTest.php b/Tests/Unit/Http/CspHeaderMiddlewareTest.php index 10e08be..331bf6d 100644 --- a/Tests/Unit/Http/CspHeaderMiddlewareTest.php +++ b/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');