Merge pull request #61 from moebrowne/feature/none-override

None Overrides
spatie · Mar 3, 2020 · b43acec · b43acec
2 parents dcbd4ac + 6e16736
commit b43acec
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 0 deletions.
diff --git a/src/Exceptions/InvalidValueSet.php b/src/Exceptions/InvalidValueSet.php
@@ -0,0 +1,13 @@
+<?php
+
+namespace Spatie\Csp\Exceptions;
+
+use Exception;
+
+class InvalidValueSet extends Exception
+{
+    public static function noneMustBeOnlyValue(): self
+    {
+        return new self('The keyword none can only be used on its own');
+    }
+}
diff --git a/src/Policies/Policy.php b/src/Policies/Policy.php
@@ -8,6 +8,7 @@
 use ReflectionClass;
 use Spatie\Csp\Directive;
 use Spatie\Csp\Exceptions\InvalidDirective;
+use Spatie\Csp\Exceptions\InvalidValueSet;
 use Spatie\Csp\Keyword;
 use Spatie\Csp\Value;
 use Symfony\Component\HttpFoundation\Response;
@@ -27,10 +28,12 @@ abstract public function configure();
      * @return \Spatie\Csp\Policies\Policy
      *
      * @throws \Spatie\Csp\Exceptions\InvalidDirective
+     * @throws \Spatie\Csp\Exceptions\InvalidValueSet
      */
     public function addDirective(string $directive, $values): self
     {
         $this->guardAgainstInvalidDirectives($directive);
+        $this->guardAgainstInvalidValues(Arr::wrap($values));
 
         if ($values === Value::NO_VALUE) {
             $this->directives[$directive][] = Value::NO_VALUE;
@@ -42,6 +45,16 @@ public function addDirective(string $directive, $values): self
             return explode(' ', $value);
         }, Arr::wrap($values))));
 
+        if (in_array(Keyword::NONE, $values, true)) {
+            $this->directives[$directive] = [$this->sanitizeValue(Keyword::NONE)];
+
+            return $this;
+        }
+
+        $this->directives[$directive] = array_filter($this->directives[$directive] ?? [], function ($value) {
+            return $value !== $this->sanitizeValue(Keyword::NONE);
+        });
+
         foreach ($values as $value) {
             $sanitizedValue = $this->sanitizeValue($value);
 
@@ -117,6 +130,13 @@ protected function guardAgainstInvalidDirectives(string $directive)
         }
     }
 
+    protected function guardAgainstInvalidValues(array $values)
+    {
+        if (in_array(Keyword::NONE, $values, true) && count($values) > 1) {
+            throw InvalidValueSet::noneMustBeOnlyValue();
+        }
+    }
+
     protected function isHash(string $value): bool
     {
         $acceptableHashingAlgorithms = [

diff --git a/tests/AddCspHeadersTest.php b/tests/AddCspHeadersTest.php
@@ -7,6 +7,7 @@
 use Spatie\Csp\AddCspHeaders;
 use Spatie\Csp\Directive;
 use Spatie\Csp\Exceptions\InvalidCspPolicy;
+use Spatie\Csp\Exceptions\InvalidValueSet;
 use Spatie\Csp\Keyword;
 use Spatie\Csp\Policies\Basic;
 use Spatie\Csp\Policies\Policy;
@@ -93,6 +94,26 @@ public function using_an_invalid_policy_class_will_throw_an_exception()
         $this->getResponseHeaders();
     }
 
+    /** @test */
+    public function passing_none_with_other_values_will_throw_an_exception()
+    {
+        $this->withoutExceptionHandling();
+
+        $invalidPolicy = new class extends Policy {
+            public function configure()
+            {
+                $this
+                    ->addDirective(Directive::CONNECT, [Keyword::NONE, 'connect']);
+            }
+        };
+
+        config(['csp.policy' => get_class($invalidPolicy)]);
+
+        $this->expectException(InvalidValueSet::class);
+
+        $this->getResponseHeaders();
+    }
+
     /** @test */
     public function it_can_use_multiple_values_for_the_same_directive()
     {
@@ -117,6 +138,52 @@ public function configure()
         );
     }
 
+    /** @test */
+    public function none_overrides_other_values_for_the_same_directive()
+    {
+        $policy = new class extends Policy {
+            public function configure()
+            {
+                $this
+                    ->addDirective(Directive::CONNECT, 'connect-1')
+                    ->addDirective(Directive::FRAME, 'src-1')
+                    ->addDirective(Directive::CONNECT, Keyword::NONE);
+            }
+        };
+
+        config(['csp.policy' => get_class($policy)]);
+
+        $headers = $this->getResponseHeaders();
+
+        $this->assertEquals(
+            'connect-src \'none\';frame-src src-1',
+            $headers->get('Content-Security-Policy')
+        );
+    }
+
+    /** @test */
+    public function values_override_none_value_for_the_same_directive()
+    {
+        $policy = new class extends Policy {
+            public function configure()
+            {
+                $this
+                    ->addDirective(Directive::CONNECT, Keyword::NONE)
+                    ->addDirective(Directive::FRAME, 'src-1')
+                    ->addDirective(Directive::CONNECT, Keyword::SELF);
+            }
+        };
+
+        config(['csp.policy' => get_class($policy)]);
+
+        $headers = $this->getResponseHeaders();
+
+        $this->assertEquals(
+            'connect-src \'self\';frame-src src-1',
+            $headers->get('Content-Security-Policy')
+        );
+    }
+
     /** @test */
     public function a_policy_can_be_put_in_report_only_mode()
     {