BUGFIX: valid custom directive is not added to csp header

Author: t-heuser

Classes/Factory/PolicyFactory.php

@@ -15,27 +15,28 @@
 class PolicyFactory
 {
     /**
-     * @param  string[][]  $defaultDirective
-     * @param  string[][]  $customDirective
+     * @param string[][] $defaultDirectives
+     * @param string[][] $customDirectives
      * @throws InvalidDirectiveException
      */
-    public function create(Nonce $nonce, array $defaultDirective, array $customDirective): Policy
+    public function create(Nonce $nonce, array $defaultDirectives, array $customDirectives): Policy
     {
-        $directiveCollections = [$defaultDirective, $customDirective];
-        $defaultDirective = array_shift($directiveCollections);
-
-        array_walk($defaultDirective, function (array &$item, string $key) use ($directiveCollections) {
-            foreach ($directiveCollections as $collection) {
-                if (array_key_exists($key, $collection)) {
-                    $item = array_unique([...$item, ...$collection[$key]]);
-                }
+        $resultDirectives = $defaultDirectives;
+        foreach ($customDirectives as $key => $customDirective) {
+            if (array_key_exists($key, $resultDirectives)) {
+                $resultDirectives[$key] = array_merge($resultDirectives[$key], $customDirective);
+            } else {
+                // Custom directive is not present in default, still needs to be added.
+                $resultDirectives[$key] = $customDirective;
             }
-        });
+
+            $resultDirectives[$key] = array_unique($resultDirectives[$key]);
+        }
 
         $policy = new Policy();
         $policy->setNonce($nonce);
 
-        foreach ($defaultDirective as $directive => $values) {
+        foreach ($resultDirectives as $directive => $values) {
             $policy->addDirective($directive, $values);
         }
 

Tests/Unit/Factory/PolicyFactoryTest.php

@@ -4,6 +4,7 @@
 
 namespace Unit\Factory;
 
+use Flowpack\ContentSecurityPolicy\Exceptions\InvalidDirectiveException;
 use Flowpack\ContentSecurityPolicy\Factory\PolicyFactory;
 use Flowpack\ContentSecurityPolicy\Model\Directive;
 use Flowpack\ContentSecurityPolicy\Model\Nonce;
@@ -15,6 +16,7 @@
 #[CoversClass(PolicyFactory::class)]
 #[UsesClass(Policy::class)]
 #[UsesClass(Directive::class)]
+#[UsesClass(InvalidDirectiveException::class)]
 class PolicyFactoryTest extends TestCase
 {
     public function testCreateShouldReturnPolicy(): void
@@ -50,4 +52,68 @@ public function testCreateShouldReturnPolicy(): void
 
         self::assertSame($expected, $result->getDirectives());
     }
+
+    public function testCreateShouldFailWithInvalidDirective(): void
+    {
+        $policyFactory = new PolicyFactory();
+        $nonceMock = $this->createMock(Nonce::class);
+
+        $defaultDirective = [
+            'base-uri' => [
+                'self',
+            ],
+            'script-src' => [
+                'self',
+            ],
+        ];
+        $customDirective = [
+            'invalid' => [
+                '{nonce}',
+            ],
+        ];
+
+        $this->expectException(InvalidDirectiveException::class);
+        $policyFactory->create($nonceMock, $defaultDirective, $customDirective);
+    }
+
+    public function testCreateShouldAddDirectiveWhichIsPresentInCustomButNotDefaultConfiguration(): void
+    {
+        $policyFactory = new PolicyFactory();
+        $nonceMock = $this->createMock(Nonce::class);
+
+        $defaultDirective = [
+            'base-uri' => [
+                'self',
+            ],
+            'script-src' => [
+                'self',
+            ],
+        ];
+        $customDirective = [
+            'script-src' => [
+                '{nonce}',
+            ],
+            'worker-src' => [
+                'self',
+                'self',
+            ],
+        ];
+
+        $expected = [
+            'base-uri' => [
+                "'self'",
+            ],
+            'script-src' => [
+                "'self'",
+                "'nonce-'",
+            ],
+            'worker-src' => [
+                "'self'",
+            ],
+        ];
+
+        $result = $policyFactory->create($nonceMock, $defaultDirective, $customDirective);
+
+        self::assertSame($expected, $result->getDirectives());
+    }
 }