TASK: add tests

Flowpack · Oct 22, 2024 · 8c53c4f · 8c53c4f
1 parent d8731f9
commit 8c53c4f
Show file tree

Hide file tree

Showing 15 changed files with 444 additions and 45 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -0,0 +1,26 @@
+name: CI-coverage
+
+on: [ push ]
+
+jobs:
+  build-test:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        php-version: [ 8.2, 8.3 ]
+
+    steps:
+      - uses: actions/checkout@v3
+
+      - uses: php-actions/composer@v6
+
+      - name: PHPUnit Tests
+        uses: php-actions/phpunit@v3
+        with:
+          php_version: ${{ matrix.php-version }}
+          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/Factory/PolicyFactory.php b/Classes/Factory/PolicyFactory.php
@@ -12,6 +12,8 @@ class PolicyFactory
 {
     /**
      * @throws InvalidDirectiveException
+     * @param string[][] $defaultDirective
+     * @param string[][] $customDirective
      */
     public static function create(Nonce $nonce, array $defaultDirective, array $customDirective): Policy
     {

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")
      */
@@ -101,11 +103,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,52 @@
+<?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
+    {
+        $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());
+    }
+}
diff --git a/Tests/Unit/Helpers/TagHelperTest.php b/Tests/Unit/Helpers/TagHelperTest.php
@@ -0,0 +1,77 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Unit\Helpers;
+
+use Flowpack\ContentSecurityPolicy\Helpers\TagHelper;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\TestCase;
+
+#[CoversClass(TagHelper::class)]
+class TagHelperTest extends TestCase
+{
+    public function testTagHasAttributeWithoutValueShouldReturnTrue(): void
+    {
+        $tag = '<script src="https://google.com"></script>';
+        self::assertTrue(TagHelper::tagHasAttribute($tag, 'src'));
+    }
+
+    public function testTagHasAttributeWithoutValueShouldReturnFalse(): void
+    {
+        $tag = '<script src="https://google.com"></script>';
+        self::assertFalse(TagHelper::tagHasAttribute($tag, 'bar'));
+    }
+
+    public function testTagHasAttributeWithValueShouldReturnTrue(): void
+    {
+        $tag = '<script src="https://google.com"></script>';
+        self::assertTrue(TagHelper::tagHasAttribute($tag, 'src', 'https://google.com'));
+    }
+
+    public function testTagHasAttributeWithValueShouldReturnFalse(): void
+    {
+        $tag = '<script src="https://google.com"></script>';
+        self::assertFalse(TagHelper::tagHasAttribute($tag, 'src', 'another value'));
+    }
+
+    public function testTagChangeAttributeValueShouldChangeValue(): void
+    {
+        $tag = '<script src="https://google.com"></script>';
+
+        self::assertSame(
+            '<script src="https://test.com"></script>',
+            TagHelper::tagChangeAttributeValue($tag, 'src', 'https://test.com')
+        );
+    }
+
+    public function testTagChangeAttributeValueShouldDoNothingIfAttributeDoesntExist(): void
+    {
+        $tag = '<script src="https://google.com"></script>';
+
+        self::assertSame(
+            '<script src="https://google.com"></script>',
+            TagHelper::tagChangeAttributeValue($tag, 'nonce', 'da65sf1g')
+        );
+    }
+
+    public function testTagAddAttributeShouldAddAttributeWithValue(): void
+    {
+        $tag = '<script src="https://google.com"></script>';
+
+        self::assertSame(
+            '<script src="https://google.com" nonce="da65sf1g"></script>',
+            TagHelper::tagAddAttribute($tag, 'nonce', 'da65sf1g')
+        );
+    }
+
+    public function testTagAddAttributeShouldAddAttributeWithoutValue(): void
+    {
+        $tag = '<script src="https://google.com"></script>';
+
+        self::assertSame(
+            '<script src="https://google.com" defer></script>',
+            TagHelper::tagAddAttribute($tag, 'defer')
+        );
+    }
+}