Merge pull request #3 from Flowpack/add-tests

TASK: add tests
Flowpack · Oct 23, 2024 · 14171d7 · 14171d7
2 parents d8731f9 + d71f649
commit 14171d7
Show file tree

Hide file tree

Showing 16 changed files with 537 additions and 51 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -0,0 +1,22 @@
+name: CI
+
+on: [ push ]
+
+jobs:
+  phpunit:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v3
+
+      - uses: php-actions/composer@v6
+
+      - name: PHPUnit Tests
+        uses: php-actions/phpunit@v4
+        with:
+          php_extensions: xdebug
+          bootstrap: vendor/autoload.php
+          configuration: phpunit.xml
+          coverage_text: true
+        env:
+          XDEBUG_MODE: coverage
diff --git a/.gitignore b/.gitignore
@@ -2,3 +2,4 @@
 composer.lock
 vendor
 Packages
+.phpunit.cache
diff --git a/Classes/Command/CspConfigCommandController.php b/Classes/Command/CspConfigCommandController.php
@@ -24,6 +24,11 @@ class CspConfigCommandController extends CommandController
      */
     protected Nonce $nonce;
 
+    /**
+     * @Flow\Inject
+     */
+    protected PolicyFactory $policyFactory;
+
     /**
      * @Flow\InjectConfiguration(path="content-security-policy")
      * @var mixed[]
@@ -39,13 +44,13 @@ class CspConfigCommandController extends CommandController
     public function showCommand(): void
     {
         try {
-            $backendPolicy = PolicyFactory::create(
+            $backendPolicy = $this->policyFactory->create(
                 $this->nonce,
                 $this->configuration['backend'],
                 $this->configuration['custom-backend']
             );
 
-            $frontendPolicy = PolicyFactory::create(
+            $frontendPolicy = $this->policyFactory->create(
                 $this->nonce,
                 $this->configuration['default'],
                 $this->configuration['custom']

diff --git a/Classes/Factory/PolicyFactory.php b/Classes/Factory/PolicyFactory.php
@@ -7,13 +7,19 @@
 use Flowpack\ContentSecurityPolicy\Exceptions\InvalidDirectiveException;
 use Flowpack\ContentSecurityPolicy\Model\Nonce;
 use Flowpack\ContentSecurityPolicy\Model\Policy;
+use Neos\Flow\Annotations as Flow;
 
+/**
+ * @Flow\Scope("singleton")
+ */
 class PolicyFactory
 {
     /**
+     * @param  string[][]  $defaultDirective
+     * @param  string[][]  $customDirective
      * @throws InvalidDirectiveException
      */
-    public static function create(Nonce $nonce, array $defaultDirective, array $customDirective): Policy
+    public function create(Nonce $nonce, array $defaultDirective, array $customDirective): Policy
     {
         $directiveCollections = [$defaultDirective, $customDirective];
         $defaultDirective = array_shift($directiveCollections);

diff --git a/Classes/Helpers/TagHelper.php b/Classes/Helpers/TagHelper.php
@@ -6,8 +6,6 @@
 
 class TagHelper
 {
-    public const NONCE = 'nonce';
-
     public static function tagHasAttribute(
         string $tag,
         string $name,
@@ -18,15 +16,15 @@ public static function tagHasAttribute(
                 self::buildMatchAttributeNameReqex($name),
                 $tag
             );
-        } else {
-            return ! ! preg_match(
-                self::buildMatchAttributeNameWithSpecificValueReqex(
-                    $name,
-                    $value
-                ),
-                $tag
-            );
         }
+
+        return ! ! preg_match(
+            self::buildMatchAttributeNameWithSpecificValueReqex(
+                $name,
+                $value
+            ),
+            $tag
+        );
     }
 
     public static function tagChangeAttributeValue(
@@ -63,9 +61,9 @@ function ($hits) use ($name, $value) {
                         $value.
                         '"'.
                         $hits["end"];
-                } else {
-                    return $hits["start"].' '.$name.$hits["end"];
                 }
+
+                return $hits["start"].' '.$name.$hits["end"];
             },
             $tag
         );
@@ -82,9 +80,8 @@ private static function buildMatchEndOfOpeningTagReqex(): string
         return '/(?<start><[a-z]+.*?)(?<end>>|\/>)/';
     }
 
-    private static function buildMatchAttributeNameWithAnyValueReqex(
-        string $name
-    ): string {
+    private static function buildMatchAttributeNameWithAnyValueReqex(string $name): string
+    {
         $nameQuoted = self::escapeReqexCharsInString($name);
 
         return '/(?<pre><.*? )(?<name>'.

diff --git a/Classes/Http/CspHeaderMiddleware.php b/Classes/Http/CspHeaderMiddleware.php
@@ -20,6 +20,8 @@
 
 class CspHeaderMiddleware implements MiddlewareInterface
 {
+    private const NONCE = 'nonce';
+
     /**
      * @Flow\InjectConfiguration(path="enabled")
      */
@@ -35,6 +37,11 @@ class CspHeaderMiddleware implements MiddlewareInterface
      */
     protected Nonce $nonce;
 
+    /**
+     * @Flow\Inject
+     */
+    protected PolicyFactory $policyFactory;
+
     /**
      * @Flow\InjectConfiguration(path="content-security-policy")
      * @var mixed[]
@@ -47,7 +54,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;
         }
 
@@ -79,14 +86,14 @@ private function getPolicyByCurrentContext(ServerRequestInterface $request): Pol
          * Neos\Neos\Domain\Service\ContentContext at this point as it throws an error.
          */
         if (str_starts_with($request->getUri()->getPath(), '/neos')) {
-            return PolicyFactory::create(
+            return $this->policyFactory->create(
                 $this->nonce,
                 $this->configuration['backend'],
                 $this->configuration['custom-backend']
             );
         }
 
-        return PolicyFactory::create(
+        return $this->policyFactory->create(
             $this->nonce,
             $this->configuration['default'],
             $this->configuration['custom']
@@ -101,11 +108,11 @@ private function addNonceToTags(string $markup): string
         return $this->checkTagAndReplaceUsingACallback($tagNames, $markup, function (
             $tagMarkup,
         ) {
-            if (TagHelper::tagHasAttribute($tagMarkup, TagHelper::NONCE)) {
-                return TagHelper::tagChangeAttributeValue($tagMarkup, TagHelper::NONCE, $this->nonce->getValue());
+            if (TagHelper::tagHasAttribute($tagMarkup, self::NONCE)) {
+                return TagHelper::tagChangeAttributeValue($tagMarkup, self::NONCE, $this->nonce->getValue());
             }
 
-            return TagHelper::tagAddAttribute($tagMarkup, TagHelper::NONCE, $this->nonce->getValue());
+            return TagHelper::tagAddAttribute($tagMarkup, self::NONCE, $this->nonce->getValue());
         });
     }
 

diff --git a/Classes/Model/Nonce.php b/Classes/Model/Nonce.php
@@ -29,9 +29,9 @@ public function getValue(): string
         return $this->value;
     }
 
-    public function __toString()
+    public function __toString(): string
     {
-        return $this->value;
+        return $this->getValue();
     }
 
     /**

diff --git a/Classes/Model/Policy.php b/Classes/Model/Policy.php
@@ -40,14 +40,9 @@ public function setNonce(Nonce $nonce): Policy
         return $this;
     }
 
-    public function isReportOnly(): bool
-    {
-        return $this->reportOnly;
-    }
-
     public function getSecurityHeaderKey(): string
     {
-        if ($this->isReportOnly()) {
+        if ($this->reportOnly) {
             return self::SECURITY_HEADER_KEY_REPORT_ONLY;
         }
 
@@ -59,10 +54,16 @@ public function getDirectives(): array
         return $this->directives;
     }
 
+    public function hasNonceDirectiveValue(): bool
+    {
+        return $this->hasNonceDirectiveValue;
+    }
+
     /**
+     * @param  string[]  $values
      * @throws InvalidDirectiveException
      */
-    public function addDirective(string $directive, $values): self
+    public function addDirective(string $directive, array $values): self
     {
         if (! Directive::isValidDirective($directive)) {
             throw new InvalidDirectiveException($directive);
@@ -74,16 +75,6 @@ public function addDirective(string $directive, $values): self
         return $this;
     }
 
-    public function getNonce(): Nonce
-    {
-        return $this->nonce;
-    }
-
-    public function hasNonceDirectiveValue(): bool
-    {
-        return $this->hasNonceDirectiveValue;
-    }
-
     public function __toString(): string
     {
         $directives = $this->getDirectives();
@@ -95,26 +86,21 @@ public function __toString(): string
             return "$directive $value";
         }, $directives, $keys);
 
-        return implode(';', $items).';';
+        return implode('; ', $items).';';
     }
 
     private function sanitizeValue(string $value): string
     {
-        if ($this->isSpecialValue($value)) {
+        if (in_array($value, self::SPECIAL_DIRECTIVES)) {
             return "'$value'";
         }
 
         if ($value === '{nonce}') {
             $this->hasNonceDirectiveValue = true;
 
-            return "'nonce-".$this->getNonce()->getValue()."'";
+            return "'nonce-".$this->nonce->getValue()."'";
         }
 
         return $value;
     }
-
-    private function isSpecialValue(string $directive): bool
-    {
-        return in_array($directive, self::SPECIAL_DIRECTIVES);
-    }
 }
diff --git a/Tests/Unit/Factory/PolicyFactoryTest.php b/Tests/Unit/Factory/PolicyFactoryTest.php
@@ -0,0 +1,53 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Unit\Factory;
+
+use Flowpack\ContentSecurityPolicy\Factory\PolicyFactory;
+use Flowpack\ContentSecurityPolicy\Model\Directive;
+use Flowpack\ContentSecurityPolicy\Model\Nonce;
+use Flowpack\ContentSecurityPolicy\Model\Policy;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\UsesClass;
+use PHPUnit\Framework\TestCase;
+
+#[CoversClass(PolicyFactory::class)]
+#[UsesClass(Policy::class)]
+#[UsesClass(Directive::class)]
+class PolicyFactoryTest extends TestCase
+{
+    public function testCreateShouldReturnPolicy(): void
+    {
+        $policyFactory = new PolicyFactory();
+        $nonceMock = $this->createMock(Nonce::class);
+
+        $defaultDirective = [
+            'base-uri' => [
+                'self',
+            ],
+            'script-src' => [
+                'self',
+            ],
+        ];
+        $customDirective = [
+            'script-src' => [
+                '{nonce}',
+            ],
+        ];
+
+        $expected = [
+            'base-uri' => [
+                "'self'",
+            ],
+            'script-src' => [
+                "'self'",
+                "'nonce-'",
+            ],
+        ];
+
+        $result = $policyFactory->create($nonceMock, $defaultDirective, $customDirective);
+
+        self::assertSame($expected, $result->getDirectives());
+    }
+}