Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Classes/Factory/PolicyFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@ class PolicyFactory
*/

/**
* @param array<string, array<string|int, string|bool>> $defaultDirectives
* @param array<string, array<string|int, string|bool>> $customDirectives
* @param array<string, array<int|string, mixed>|null> $defaultDirectives
* @param array<string, array<int|string, mixed>|null> $customDirectives
* @throws InvalidDirectiveException
* @throws DirectivesNormalizerException
*/
public function create(Nonce $nonce, array $defaultDirectives, array $customDirectives): Policy
{
$normalizedDefaultDirectives = DirectivesNormalizer::normalize($defaultDirectives, $this->logger);
$normalizedCustomDirectives = DirectivesNormalizer::normalize($customDirectives, $this->logger);
$normalizedDefaultDirectives = DirectivesNormalizer::normalize($defaultDirectives);
$normalizedCustomDirectives = DirectivesNormalizer::normalize($customDirectives);

$resultDirectives = $normalizedDefaultDirectives;
foreach ($normalizedCustomDirectives as $key => $customDirective) {
Expand Down
52 changes: 14 additions & 38 deletions Classes/Helpers/DirectivesNormalizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,70 +5,46 @@
namespace Flowpack\ContentSecurityPolicy\Helpers;

use Flowpack\ContentSecurityPolicy\Exceptions\DirectivesNormalizerException;
use Psr\Log\LoggerInterface;

/**
* Helper to support normalization of directives from different formats.
* The old format supported yaml lists. Now key-value pairs should be used for directives.
* In the future we will deprecate the list format!
*
* We also cleanup of empty directives and entries here before further processing.
* Normalizes CSP directives from yaml key-value pairs (e.g. example.com: true) to string arrays.
* Also removes empty directives and entries before further processing.
*/
final class DirectivesNormalizer
{
/**
* @param array<string, ?array<string|int, string|bool>> $directives
* @param array<string, array<int|string, mixed>|null> $directives
* @return string[][]
* @throws DirectivesNormalizerException
*/
public static function normalize(array $directives, LoggerInterface $logger): array
public static function normalize(array $directives): array
{
$result = [];
// directives e.g. script-src:
foreach ($directives as $directive => $values) {
if (!is_array($values) || count($values) === 0) {
continue;
}

$normalizedValues = [];
$firstKeyType = null;
// values e.g. 'self', 'unsafe-inline' OR key-value pairs e.g. example.com: true
foreach ($values as $key => $value) {
if ($firstKeyType === null) {
$firstKeyType = gettype($key);
} else {
if (gettype($key) !== $firstKeyType) {
// we do not allow mixed key types -> this should be marked as an error in the IDE as well
// as Flow should throw an exception here. But just to be sure, we add this check.
throw new DirectivesNormalizerException(
'Directives must be defined as a list OR an object.'
);
}
}

if (is_int($key) && is_string($value) && trim($value) !== '') {
// old configuration format using list
$normalizedValues[] = $value;
$logger->warning(
'Using list format for CSP directives is deprecated and will be removed in future versions. Please use key-value pairs with boolean values instead.'
if (!is_string($key)) {
throw new DirectivesNormalizerException(
'Directives must be defined as an object with string keys and boolean values.'
);
} elseif (is_string($key)) {
// new configuration format using key-value pairs
if (is_bool($value)) {
if ($value === true && trim($key) !== '') {
$normalizedValues[] = $key;
}
continue;
}
}

// We chose a format similar to NodeType constraints yaml configuration.
if (!is_bool($value)) {
throw new DirectivesNormalizerException(
'When using keys in your yaml, the values must be boolean.'
);
}

if ($value === true && trim($key) !== '') {
$normalizedValues[] = $key;
}
}

if ($normalizedValues !== []) {
// we also clean up empty directives here
$result[$directive] = $normalizedValues;
}
}
Expand Down
19 changes: 0 additions & 19 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
* [Flowpack.ContentSecurityPolicy](#flowpackcontentsecuritypolicy)
* [Introduction](#introduction)
* [Usage](#usage)
* [Deprecated Configuration](#deprecated-configuration)
* [Custom directives and values](#custom-directives-and-values)
* [Show CSP configuration](#show-csp-configuration)
* [Disable or report only](#disable-or-report-only)
Expand Down Expand Up @@ -70,24 +69,6 @@ Flowpack:
Now only resources from the same origin are allowed for the most common directives.
It is enabled by default and the report-only mode is disabled.
## Deprecated Configuration
Make sure to change any old configuration to the new object format. **Support for the old list format will be removed in future versions.**
The new config allows to merge configurations from different packages/yaml files.
```yaml
frame-src:
# Deprecated list format
# - 'https://www.youtube.com':
# - 'https://staticxx.facebook.com':

# New object format
'https://www.youtube.com': true
'https://staticxx.facebook.com': true
```
## Custom directives and values
If you want to override the default config don't forget to add this package as a dependency in the composer.json file
Expand Down
119 changes: 32 additions & 87 deletions Tests/Unit/Factory/PolicyFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,32 +49,19 @@ public function testCreateShouldReturnPolicyAndMergeCustomWithDefaultDirective()
$nonceMock = $this->createMock(Nonce::class);

$defaultDirective = [
'base-uri' => [
'test.com',
],
'script-src' => [
'test.com',
],
'base-uri' => ['test.com' => true],
'script-src' => ['test.com' => true],
];
$customDirective = [
'script-src' => [
'custom.com',
],
];

$expected = [
'base-uri' => [
'test.com',
],
'script-src' => [
'test.com',
'custom.com',
],
'script-src' => ['custom.com' => true],
];

$result = $this->policyFactory->create($nonceMock, $defaultDirective, $customDirective);

self::assertSame($expected, $result->getDirectives());
self::assertSame([
'base-uri' => ['test.com'],
'script-src' => ['test.com', 'custom.com'],
], $result->getDirectives());
}

public function testCreateShouldReturnPolicyAndHandleSpecialDirectives(): void
Expand All @@ -83,35 +70,26 @@ public function testCreateShouldReturnPolicyAndHandleSpecialDirectives(): void

$defaultDirective = [
'script-src' => [
'{nonce}',
'self',
'{nonce}' => true,
'self' => true,
],
];
$customDirective = [];

$expected = [
'script-src' => [
"'nonce-'",
"'self'",
],
];

$result = $this->policyFactory->create($nonceMock, $defaultDirective, $customDirective);

self::assertSame($expected, $result->getDirectives());
self::assertSame([
'script-src' => ["'nonce-'", "'self'"],
], $result->getDirectives());
}

public function testCreateShouldFailWithInvalidDirective(): void
{
$nonceMock = $this->createMock(Nonce::class);

$defaultDirective = [
'invalid' => [
'self',
],
'script-src' => [
'self',
],
'invalid' => ['self' => true],
'script-src' => ['self' => true],
];
$customDirective = [];

Expand All @@ -128,12 +106,8 @@ public function testCreateShouldLogInvalidDirectiveInProduction(): void
);

$defaultDirective = [
'invalid' => [
'self',
],
'script-src' => [
'self',
],
'invalid' => ['self' => true],
'script-src' => ['self' => true],
];
$customDirective = [];

Expand All @@ -151,69 +125,40 @@ public function testCreateShouldReturnPolicyWithUniqueValues(): void
$nonceMock = $this->createMock(Nonce::class);

$defaultDirective = [
'base-uri' => [
'test.com',
],
'script-src' => [
'test.com',
],
'base-uri' => ['test.com' => true],
'script-src' => ['test.com' => true],
];
$customDirective = [
'base-uri' => [
'test.com',
'test.com',
],
'script-src' => [
'test.com',
],
];

$expected = [
'base-uri' => [
'test.com',
],
'script-src' => [
'test.com',
],
'base-uri' => ['test.com' => true],
'script-src' => ['test.com' => true],
];

$result = $this->policyFactory->create($nonceMock, $defaultDirective, $customDirective);

self::assertSame($expected, $result->getDirectives());
self::assertSame([
'base-uri' => ['test.com'],
'script-src' => ['test.com'],
], $result->getDirectives());
}

public function testCreateShouldAddDirectiveWhichIsPresentInCustomButNotDefaultConfiguration(): void
{
$nonceMock = $this->createMock(Nonce::class);

$defaultDirective = [
'base-uri' => [
'test.com',
],
'script-src' => [
'test.com',
],
'base-uri' => ['test.com' => true],
'script-src' => ['test.com' => true],
];
$customDirective = [
'worker-src' => [
'test.com',
],
];

$expected = [
'base-uri' => [
"test.com",
],
'script-src' => [
"test.com",
],
'worker-src' => [
"test.com",
],
'worker-src' => ['test.com' => true],
];

$result = $this->policyFactory->create($nonceMock, $defaultDirective, $customDirective);

self::assertSame($expected, $result->getDirectives());
self::assertSame([
'base-uri' => ['test.com'],
'script-src' => ['test.com'],
'worker-src' => ['test.com'],
], $result->getDirectives());
}
}
Loading
Loading