Skip to content

Commit 9576028

Browse files
t-heusert-heuser
authored
change!: remove deprecated config support (#17)
#12
1 parent b15aff6 commit 9576028

5 files changed

Lines changed: 103 additions & 311 deletions

File tree

‎Classes/Factory/PolicyFactory.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,15 @@ class PolicyFactory
3333
*/
3434

3535
/**
36-
* @param array<string, array<string|int, string|bool>> $defaultDirectives
37-
* @param array<string, array<string|int, string|bool>> $customDirectives
36+
* @param array<string, array<int|string, mixed>|null> $defaultDirectives
37+
* @param array<string, array<int|string, mixed>|null> $customDirectives
3838
* @throws InvalidDirectiveException
3939
* @throws DirectivesNormalizerException
4040
*/
4141
public function create(Nonce $nonce, array $defaultDirectives, array $customDirectives): Policy
4242
{
43-
$normalizedDefaultDirectives = DirectivesNormalizer::normalize($defaultDirectives, $this->logger);
44-
$normalizedCustomDirectives = DirectivesNormalizer::normalize($customDirectives, $this->logger);
43+
$normalizedDefaultDirectives = DirectivesNormalizer::normalize($defaultDirectives);
44+
$normalizedCustomDirectives = DirectivesNormalizer::normalize($customDirectives);
4545

4646
$resultDirectives = $normalizedDefaultDirectives;
4747
foreach ($normalizedCustomDirectives as $key => $customDirective) {

‎Classes/Helpers/DirectivesNormalizer.php‎

Lines changed: 14 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -5,70 +5,46 @@
55
namespace Flowpack\ContentSecurityPolicy\Helpers;
66

77
use Flowpack\ContentSecurityPolicy\Exceptions\DirectivesNormalizerException;
8-
use Psr\Log\LoggerInterface;
98

109
/**
11-
* Helper to support normalization of directives from different formats.
12-
* The old format supported yaml lists. Now key-value pairs should be used for directives.
13-
* In the future we will deprecate the list format!
14-
*
15-
* We also cleanup of empty directives and entries here before further processing.
10+
* Normalizes CSP directives from yaml key-value pairs (e.g. example.com: true) to string arrays.
11+
* Also removes empty directives and entries before further processing.
1612
*/
1713
final class DirectivesNormalizer
1814
{
1915
/**
20-
* @param array<string, ?array<string|int, string|bool>> $directives
16+
* @param array<string, array<int|string, mixed>|null> $directives
2117
* @return string[][]
2218
* @throws DirectivesNormalizerException
2319
*/
24-
public static function normalize(array $directives, LoggerInterface $logger): array
20+
public static function normalize(array $directives): array
2521
{
2622
$result = [];
27-
// directives e.g. script-src:
2823
foreach ($directives as $directive => $values) {
2924
if (!is_array($values) || count($values) === 0) {
3025
continue;
3126
}
3227

3328
$normalizedValues = [];
34-
$firstKeyType = null;
35-
// values e.g. 'self', 'unsafe-inline' OR key-value pairs e.g. example.com: true
3629
foreach ($values as $key => $value) {
37-
if ($firstKeyType === null) {
38-
$firstKeyType = gettype($key);
39-
} else {
40-
if (gettype($key) !== $firstKeyType) {
41-
// we do not allow mixed key types -> this should be marked as an error in the IDE as well
42-
// as Flow should throw an exception here. But just to be sure, we add this check.
43-
throw new DirectivesNormalizerException(
44-
'Directives must be defined as a list OR an object.'
45-
);
46-
}
47-
}
48-
49-
if (is_int($key) && is_string($value) && trim($value) !== '') {
50-
// old configuration format using list
51-
$normalizedValues[] = $value;
52-
$logger->warning(
53-
'Using list format for CSP directives is deprecated and will be removed in future versions. Please use key-value pairs with boolean values instead.'
30+
if (!is_string($key)) {
31+
throw new DirectivesNormalizerException(
32+
'Directives must be defined as an object with string keys and boolean values.'
5433
);
55-
} elseif (is_string($key)) {
56-
// new configuration format using key-value pairs
57-
if (is_bool($value)) {
58-
if ($value === true && trim($key) !== '') {
59-
$normalizedValues[] = $key;
60-
}
61-
continue;
62-
}
34+
}
6335

64-
// We chose a format similar to NodeType constraints yaml configuration.
36+
if (!is_bool($value)) {
6537
throw new DirectivesNormalizerException(
6638
'When using keys in your yaml, the values must be boolean.'
6739
);
6840
}
41+
42+
if ($value === true && trim($key) !== '') {
43+
$normalizedValues[] = $key;
44+
}
6945
}
46+
7047
if ($normalizedValues !== []) {
71-
// we also clean up empty directives here
7248
$result[$directive] = $normalizedValues;
7349
}
7450
}

‎README.md‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
* [Flowpack.ContentSecurityPolicy](#flowpackcontentsecuritypolicy)
55
* [Introduction](#introduction)
66
* [Usage](#usage)
7-
* [Deprecated Configuration](#deprecated-configuration)
87
* [Custom directives and values](#custom-directives-and-values)
98
* [Show CSP configuration](#show-csp-configuration)
109
* [Disable or report only](#disable-or-report-only)
@@ -70,24 +69,6 @@ Flowpack:
7069
Now only resources from the same origin are allowed for the most common directives.
7170
It is enabled by default and the report-only mode is disabled.
7271
73-
## Deprecated Configuration
74-
75-
Make sure to change any old configuration to the new object format. **Support for the old list format will be removed in future versions.**
76-
77-
The new config allows to merge configurations from different packages/yaml files.
78-
79-
```yaml
80-
frame-src:
81-
# Deprecated list format
82-
# - 'https://www.youtube.com':
83-
# - 'https://staticxx.facebook.com':
84-
85-
# New object format
86-
'https://www.youtube.com': true
87-
'https://staticxx.facebook.com': true
88-
```
89-
90-
9172
## Custom directives and values
9273
9374
If you want to override the default config don't forget to add this package as a dependency in the composer.json file

‎Tests/Unit/Factory/PolicyFactoryTest.php‎

Lines changed: 32 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -49,32 +49,19 @@ public function testCreateShouldReturnPolicyAndMergeCustomWithDefaultDirective()
4949
$nonceMock = $this->createMock(Nonce::class);
5050

5151
$defaultDirective = [
52-
'base-uri' => [
53-
'test.com',
54-
],
55-
'script-src' => [
56-
'test.com',
57-
],
52+
'base-uri' => ['test.com' => true],
53+
'script-src' => ['test.com' => true],
5854
];
5955
$customDirective = [
60-
'script-src' => [
61-
'custom.com',
62-
],
63-
];
64-
65-
$expected = [
66-
'base-uri' => [
67-
'test.com',
68-
],
69-
'script-src' => [
70-
'test.com',
71-
'custom.com',
72-
],
56+
'script-src' => ['custom.com' => true],
7357
];
7458

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

77-
self::assertSame($expected, $result->getDirectives());
61+
self::assertSame([
62+
'base-uri' => ['test.com'],
63+
'script-src' => ['test.com', 'custom.com'],
64+
], $result->getDirectives());
7865
}
7966

8067
public function testCreateShouldReturnPolicyAndHandleSpecialDirectives(): void
@@ -83,35 +70,26 @@ public function testCreateShouldReturnPolicyAndHandleSpecialDirectives(): void
8370

8471
$defaultDirective = [
8572
'script-src' => [
86-
'{nonce}',
87-
'self',
73+
'{nonce}' => true,
74+
'self' => true,
8875
],
8976
];
9077
$customDirective = [];
9178

92-
$expected = [
93-
'script-src' => [
94-
"'nonce-'",
95-
"'self'",
96-
],
97-
];
98-
9979
$result = $this->policyFactory->create($nonceMock, $defaultDirective, $customDirective);
10080

101-
self::assertSame($expected, $result->getDirectives());
81+
self::assertSame([
82+
'script-src' => ["'nonce-'", "'self'"],
83+
], $result->getDirectives());
10284
}
10385

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

10890
$defaultDirective = [
109-
'invalid' => [
110-
'self',
111-
],
112-
'script-src' => [
113-
'self',
114-
],
91+
'invalid' => ['self' => true],
92+
'script-src' => ['self' => true],
11593
];
11694
$customDirective = [];
11795

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

130108
$defaultDirective = [
131-
'invalid' => [
132-
'self',
133-
],
134-
'script-src' => [
135-
'self',
136-
],
109+
'invalid' => ['self' => true],
110+
'script-src' => ['self' => true],
137111
];
138112
$customDirective = [];
139113

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

153127
$defaultDirective = [
154-
'base-uri' => [
155-
'test.com',
156-
],
157-
'script-src' => [
158-
'test.com',
159-
],
128+
'base-uri' => ['test.com' => true],
129+
'script-src' => ['test.com' => true],
160130
];
161131
$customDirective = [
162-
'base-uri' => [
163-
'test.com',
164-
'test.com',
165-
],
166-
'script-src' => [
167-
'test.com',
168-
],
169-
];
170-
171-
$expected = [
172-
'base-uri' => [
173-
'test.com',
174-
],
175-
'script-src' => [
176-
'test.com',
177-
],
132+
'base-uri' => ['test.com' => true],
133+
'script-src' => ['test.com' => true],
178134
];
179135

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

182-
self::assertSame($expected, $result->getDirectives());
138+
self::assertSame([
139+
'base-uri' => ['test.com'],
140+
'script-src' => ['test.com'],
141+
], $result->getDirectives());
183142
}
184143

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

189148
$defaultDirective = [
190-
'base-uri' => [
191-
'test.com',
192-
],
193-
'script-src' => [
194-
'test.com',
195-
],
149+
'base-uri' => ['test.com' => true],
150+
'script-src' => ['test.com' => true],
196151
];
197152
$customDirective = [
198-
'worker-src' => [
199-
'test.com',
200-
],
201-
];
202-
203-
$expected = [
204-
'base-uri' => [
205-
"test.com",
206-
],
207-
'script-src' => [
208-
"test.com",
209-
],
210-
'worker-src' => [
211-
"test.com",
212-
],
153+
'worker-src' => ['test.com' => true],
213154
];
214155

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

217-
self::assertSame($expected, $result->getDirectives());
158+
self::assertSame([
159+
'base-uri' => ['test.com'],
160+
'script-src' => ['test.com'],
161+
'worker-src' => ['test.com'],
162+
], $result->getDirectives());
218163
}
219164
}

0 commit comments

Comments
 (0)