Skip to content

Commit 42b8a66

Browse files
t-heusert-heuser
committed
BUGFIX: valid custom directive is not added to csp header
1 parent ec3e054 commit 42b8a66

File tree

2 files changed

+138
-18
lines changed

2 files changed

+138
-18
lines changed

‎Classes/Factory/PolicyFactory.php‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,27 +15,28 @@
1515
class PolicyFactory
1616
{
1717
/**
18-
* @param string[][] $defaultDirective
19-
* @param string[][] $customDirective
18+
* @param string[][] $defaultDirectives
19+
* @param string[][] $customDirectives
2020
* @throws InvalidDirectiveException
2121
*/
22-
public function create(Nonce $nonce, array $defaultDirective, array $customDirective): Policy
22+
public function create(Nonce $nonce, array $defaultDirectives, array $customDirectives): Policy
2323
{
24-
$directiveCollections = [$defaultDirective, $customDirective];
25-
$defaultDirective = array_shift($directiveCollections);
26-
27-
array_walk($defaultDirective, function (array &$item, string $key) use ($directiveCollections) {
28-
foreach ($directiveCollections as $collection) {
29-
if (array_key_exists($key, $collection)) {
30-
$item = array_unique([...$item, ...$collection[$key]]);
31-
}
24+
$resultDirectives = $defaultDirectives;
25+
foreach ($customDirectives as $key => $customDirective) {
26+
if (array_key_exists($key, $resultDirectives)) {
27+
$resultDirectives[$key] = array_merge($resultDirectives[$key], $customDirective);
28+
} else {
29+
// Custom directive is not present in default, still needs to be added.
30+
$resultDirectives[$key] = $customDirective;
3231
}
33-
});
32+
33+
$resultDirectives[$key] = array_unique($resultDirectives[$key]);
34+
}
3435

3536
$policy = new Policy();
3637
$policy->setNonce($nonce);
3738

38-
foreach ($defaultDirective as $directive => $values) {
39+
foreach ($resultDirectives as $directive => $values) {
3940
$policy->addDirective($directive, $values);
4041
}
4142

‎Tests/Unit/Factory/PolicyFactoryTest.php‎

Lines changed: 124 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace Unit\Factory;
66

7+
use Flowpack\ContentSecurityPolicy\Exceptions\InvalidDirectiveException;
78
use Flowpack\ContentSecurityPolicy\Factory\PolicyFactory;
89
use Flowpack\ContentSecurityPolicy\Model\Directive;
910
use Flowpack\ContentSecurityPolicy\Model\Nonce;
@@ -15,34 +16,152 @@
1516
#[CoversClass(PolicyFactory::class)]
1617
#[UsesClass(Policy::class)]
1718
#[UsesClass(Directive::class)]
19+
#[UsesClass(InvalidDirectiveException::class)]
1820
class PolicyFactoryTest extends TestCase
1921
{
20-
public function testCreateShouldReturnPolicy(): void
22+
public function testCreateShouldReturnPolicyAndMergeCustomWithDefaultDirective(): void
2123
{
2224
$policyFactory = new PolicyFactory();
2325
$nonceMock = $this->createMock(Nonce::class);
2426

2527
$defaultDirective = [
2628
'base-uri' => [
29+
'test.com',
30+
],
31+
'script-src' => [
32+
'test.com',
33+
],
34+
];
35+
$customDirective = [
36+
'script-src' => [
37+
'custom.com',
38+
],
39+
];
40+
41+
$expected = [
42+
'base-uri' => [
43+
'test.com',
44+
],
45+
'script-src' => [
46+
'test.com',
47+
'custom.com',
48+
],
49+
];
50+
51+
$result = $policyFactory->create($nonceMock, $defaultDirective, $customDirective);
52+
53+
self::assertSame($expected, $result->getDirectives());
54+
}
55+
56+
public function testCreateShouldReturnPolicyAndHandleSpecialDirectives(): void
57+
{
58+
$policyFactory = new PolicyFactory();
59+
$nonceMock = $this->createMock(Nonce::class);
60+
61+
$defaultDirective = [
62+
'script-src' => [
63+
'{nonce}',
2764
'self',
2865
],
66+
];
67+
$customDirective = [];
68+
69+
$expected = [
2970
'script-src' => [
71+
"'nonce-'",
72+
"'self'",
73+
],
74+
];
75+
76+
$result = $policyFactory->create($nonceMock, $defaultDirective, $customDirective);
77+
78+
self::assertSame($expected, $result->getDirectives());
79+
}
80+
81+
public function testCreateShouldFailWithInvalidDirective(): void
82+
{
83+
$policyFactory = new PolicyFactory();
84+
$nonceMock = $this->createMock(Nonce::class);
85+
86+
$defaultDirective = [
87+
'invalid' => [
3088
'self',
3189
],
90+
'script-src' => [
91+
'self',
92+
],
93+
];
94+
$customDirective = [];
95+
96+
$this->expectException(InvalidDirectiveException::class);
97+
$policyFactory->create($nonceMock, $defaultDirective, $customDirective);
98+
}
99+
100+
public function testCreateShouldReturnPolicyWithUniqueValues(): void
101+
{
102+
$policyFactory = new PolicyFactory();
103+
$nonceMock = $this->createMock(Nonce::class);
104+
105+
$defaultDirective = [
106+
'base-uri' => [
107+
'test.com',
108+
],
109+
'script-src' => [
110+
'test.com',
111+
],
32112
];
33113
$customDirective = [
114+
'base-uri' => [
115+
'test.com',
116+
'test.com',
117+
],
34118
'script-src' => [
35-
'{nonce}',
119+
'test.com',
36120
],
37121
];
38122

39123
$expected = [
40124
'base-uri' => [
41-
"'self'",
125+
'test.com',
42126
],
43127
'script-src' => [
44-
"'self'",
45-
"'nonce-'",
128+
'test.com',
129+
],
130+
];
131+
132+
$result = $policyFactory->create($nonceMock, $defaultDirective, $customDirective);
133+
134+
self::assertSame($expected, $result->getDirectives());
135+
}
136+
137+
public function testCreateShouldAddDirectiveWhichIsPresentInCustomButNotDefaultConfiguration(): void
138+
{
139+
$policyFactory = new PolicyFactory();
140+
$nonceMock = $this->createMock(Nonce::class);
141+
142+
$defaultDirective = [
143+
'base-uri' => [
144+
'test.com',
145+
],
146+
'script-src' => [
147+
'test.com',
148+
],
149+
];
150+
$customDirective = [
151+
'worker-src' => [
152+
'test.com',
153+
],
154+
];
155+
156+
$expected = [
157+
'base-uri' => [
158+
"test.com",
159+
],
160+
'script-src' => [
161+
"test.com",
162+
],
163+
'worker-src' => [
164+
"test.com",
46165
],
47166
];
48167

0 commit comments

Comments
 (0)