Skip to content

Commit 140a3dc

Browse files
SeldaekSeldaek
authored
Merge pull request #348 from martijnc/feature/directive-set-builder
Introduce `DirectiveSetBuilderInterface` to allow runtime modification
2 parents 23d3bc1 + dace871 commit 140a3dc

7 files changed

+222
-27
lines changed

‎src/ContentSecurityPolicy/ConfigurationDirectiveSetBuilder.php

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* This file is part of the Nelmio SecurityBundle.
7+
*
8+
* (c) Nelmio <[email protected]>
9+
*
10+
* For the full copyright and license information, please view the LICENSE
11+
* file that was distributed with this source code.
12+
*/
13+
14+
namespace Nelmio\SecurityBundle\ContentSecurityPolicy;
15+
16+
class ConfigurationDirectiveSetBuilder implements DirectiveSetBuilderInterface
17+
{
18+
private PolicyManager $policyManager;
19+
20+
/**
21+
* @phpstan-var array{
22+
* enforce?: array<string, mixed>,
23+
* report?: array<string, mixed>,
24+
* } $config
25+
*/
26+
private array $config;
27+
28+
/**
29+
* @phpstan-var 'enforce'|'report' $kind
30+
*/
31+
private string $kind;
32+
33+
/**
34+
* @phpstan-param array{
35+
* enforce?: array<string, mixed>,
36+
* report?: array<string, mixed>,
37+
* } $config
38+
* @phpstan-param 'enforce'|'report' $kind
39+
*/
40+
public function __construct(PolicyManager $policyManager, array $config, string $kind)
41+
{
42+
$this->policyManager = $policyManager;
43+
$this->config = $config;
44+
$this->kind = $kind;
45+
}
46+
47+
public function buildDirectiveSet(): DirectiveSet
48+
{
49+
return DirectiveSet::fromConfig($this->policyManager, $this->config, $this->kind);
50+
}
51+
52+
/**
53+
* @phpstan-param array{
54+
* enforce?: array<string, mixed>,
55+
* report?: array<string, mixed>,
56+
* } $config
57+
* @phpstan-param 'enforce'|'report' $kind
58+
*/
59+
public static function create(PolicyManager $policyManager, array $config, string $kind): self
60+
{
61+
return new self($policyManager, $config, $kind);
62+
}
63+
}

‎src/ContentSecurityPolicy/DirectiveSetBuilderInterface.php

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* This file is part of the Nelmio SecurityBundle.
7+
*
8+
* (c) Nelmio <[email protected]>
9+
*
10+
* For the full copyright and license information, please view the LICENSE
11+
* file that was distributed with this source code.
12+
*/
13+
14+
namespace Nelmio\SecurityBundle\ContentSecurityPolicy;
15+
16+
interface DirectiveSetBuilderInterface
17+
{
18+
public function buildDirectiveSet(): DirectiveSet;
19+
}

‎src/ContentSecurityPolicy/LegacyDirectiveSetBuilder.php

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* This file is part of the Nelmio SecurityBundle.
7+
*
8+
* (c) Nelmio <[email protected]>
9+
*
10+
* For the full copyright and license information, please view the LICENSE
11+
* file that was distributed with this source code.
12+
*/
13+
14+
namespace Nelmio\SecurityBundle\ContentSecurityPolicy;
15+
16+
/**
17+
* @internal
18+
*/
19+
class LegacyDirectiveSetBuilder implements DirectiveSetBuilderInterface
20+
{
21+
private DirectiveSet $directiveSet;
22+
23+
public function __construct(DirectiveSet $directiveSet)
24+
{
25+
$this->directiveSet = $directiveSet;
26+
}
27+
28+
public function buildDirectiveSet(): DirectiveSet
29+
{
30+
return clone $this->directiveSet;
31+
}
32+
}

‎src/DependencyInjection/NelmioSecurityExtension.php

+16-9
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@
1313

1414
namespace Nelmio\SecurityBundle\DependencyInjection;
1515

16-
use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSet;
16+
use Nelmio\SecurityBundle\ContentSecurityPolicy\ConfigurationDirectiveSetBuilder;
17+
use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSetBuilderInterface;
1718
use Symfony\Component\Config\Definition\Processor;
1819
use Symfony\Component\Config\FileLocator;
1920
use Symfony\Component\DependencyInjection\ContainerBuilder;
@@ -59,11 +60,17 @@ public function load(array $configs, ContainerBuilder $container): void
5960

6061
$cspConfig = $config['csp'];
6162

62-
$enforceDefinition = $this->buildDirectiveSetDefinition($container, $cspConfig, 'enforce');
63-
$reportDefinition = $this->buildDirectiveSetDefinition($container, $cspConfig, 'report');
63+
$enforceDefinition = $this->createDirectiveSetBuilder($container, $cspConfig, 'enforce');
64+
$reportDefinition = $this->createDirectiveSetBuilder($container, $cspConfig, 'report');
65+
66+
$container->addDefinitions([
67+
'nelmio_security.directive_set_builder.report' => $reportDefinition,
68+
'nelmio_security.directive_set_builder.enforce' => $enforceDefinition,
69+
]);
6470

6571
$cspListenerDefinition = $container->getDefinition('nelmio_security.csp_listener');
66-
$cspListenerDefinition->setArguments([$reportDefinition, $enforceDefinition, new Reference('nelmio_security.nonce_generator'), new Reference('nelmio_security.sha_computer'), (bool) $cspConfig['compat_headers'], $cspConfig['hosts'], $cspConfig['content_types']]);
72+
$cspListenerDefinition->setArguments([new Reference('nelmio_security.directive_set_builder.report'), new Reference('nelmio_security.directive_set_builder.enforce'), new Reference('nelmio_security.nonce_generator'), new Reference('nelmio_security.sha_computer'), (bool) $cspConfig['compat_headers'], $cspConfig['hosts'], $cspConfig['content_types']]);
73+
6774
$container->setParameter('nelmio_security.csp.hash_algorithm', $cspConfig['hash']['algorithm']);
6875

6976
if (isset($cspConfig['request_matcher'])) {
@@ -188,11 +195,11 @@ public function load(array $configs, ContainerBuilder $container): void
188195
* } $config
189196
* @phpstan-param 'enforce'|'report' $type
190197
*/
191-
private function buildDirectiveSetDefinition(ContainerBuilder $container, array $config, string $type): Definition
198+
private function createDirectiveSetBuilder(ContainerBuilder $container, array $config, string $type): Definition
192199
{
193-
$directiveDefinition = new Definition(DirectiveSet::class);
200+
$builderDefinition = new Definition(DirectiveSetBuilderInterface::class);
194201

195-
$directiveDefinition->setFactory([DirectiveSet::class, 'fromConfig']);
202+
$builderDefinition->setFactory([ConfigurationDirectiveSetBuilder::class, 'create']);
196203

197204
$pmDefinition = $container->getDefinition('nelmio_security.policy_manager');
198205

@@ -207,8 +214,8 @@ private function buildDirectiveSetDefinition(ContainerBuilder $container, array
207214
$pmDefinition->setArguments([new Reference('nelmio_security.ua_parser')]);
208215
}
209216

210-
$directiveDefinition->setArguments([$pmDefinition, $config, $type]);
217+
$builderDefinition->setArguments([$pmDefinition, $config, $type]);
211218

212-
return $directiveDefinition;
219+
return $builderDefinition;
213220
}
214221
}

‎src/EventListener/ContentSecurityPolicyListener.php

+49-12
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
namespace Nelmio\SecurityBundle\EventListener;
1515

1616
use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSet;
17+
use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSetBuilderInterface;
18+
use Nelmio\SecurityBundle\ContentSecurityPolicy\LegacyDirectiveSetBuilder;
1719
use Nelmio\SecurityBundle\ContentSecurityPolicy\NonceGeneratorInterface;
1820
use Nelmio\SecurityBundle\ContentSecurityPolicy\ShaComputerInterface;
1921
use Symfony\Component\HttpFoundation\Request;
@@ -24,8 +26,8 @@
2426

2527
final class ContentSecurityPolicyListener extends AbstractContentTypeRestrictableListener
2628
{
27-
private DirectiveSet $report;
28-
private DirectiveSet $enforce;
29+
private DirectiveSetBuilderInterface $reportDirectiveSetBuilder;
30+
private DirectiveSetBuilderInterface $enforceDirectiveSetBuilder;
2931
private bool $compatHeaders;
3032

3133
/**
@@ -45,12 +47,14 @@ final class ContentSecurityPolicyListener extends AbstractContentTypeRestrictabl
4547
private ?RequestMatcherInterface $requestMatcher;
4648

4749
/**
48-
* @param list<string> $hosts
49-
* @param list<string> $contentTypes
50+
* @param DirectiveSetBuilderInterface|DirectiveSet $reportDirectiveSetBuilder
51+
* @param DirectiveSetBuilderInterface|DirectiveSet $enforceDirectiveSetBuilder
52+
* @param list<string> $hosts
53+
* @param list<string> $contentTypes
5054
*/
5155
public function __construct(
52-
DirectiveSet $report,
53-
DirectiveSet $enforce,
56+
$reportDirectiveSetBuilder,
57+
$enforceDirectiveSetBuilder,
5458
NonceGeneratorInterface $nonceGenerator,
5559
ShaComputerInterface $shaComputer,
5660
bool $compatHeaders = true,
@@ -59,8 +63,8 @@ public function __construct(
5963
?RequestMatcherInterface $requestMatcher = null
6064
) {
6165
parent::__construct($contentTypes);
62-
$this->report = $report;
63-
$this->enforce = $enforce;
66+
$this->reportDirectiveSetBuilder = $this->ensureDirectiveSetBuilder($reportDirectiveSetBuilder);
67+
$this->enforceDirectiveSetBuilder = $this->ensureDirectiveSetBuilder($enforceDirectiveSetBuilder);
6468
$this->compatHeaders = $compatHeaders;
6569
$this->hosts = $hosts;
6670
$this->nonceGenerator = $nonceGenerator;
@@ -110,14 +114,20 @@ public function addStyle(string $html): void
110114
$this->sha['style-src'][] = $this->shaComputer->computeForStyle($html);
111115
}
112116

117+
/**
118+
* @deprecated Use `nelmio_security.directive_set_builder.report` instead.
119+
*/
113120
public function getReport(): DirectiveSet
114121
{
115-
return $this->report;
122+
return $this->reportDirectiveSetBuilder->buildDirectiveSet();
116123
}
117124

125+
/**
126+
* @deprecated Use `nelmio_security.directive_set_builder.enforce` instead.
127+
*/
118128
public function getEnforcement(): DirectiveSet
119129
{
120-
return $this->enforce;
130+
return $this->enforceDirectiveSetBuilder->buildDirectiveSet();
121131
}
122132

123133
public function getNonce(string $usage): string
@@ -169,10 +179,10 @@ public function onKernelResponse(ResponseEvent $e): void
169179
}
170180

171181
if (!$response->headers->has('Content-Security-Policy-Report-Only')) {
172-
$response->headers->add($this->buildHeaders($request, $this->report, true, $this->compatHeaders, $signatures));
182+
$response->headers->add($this->buildHeaders($request, $this->reportDirectiveSetBuilder->buildDirectiveSet(), true, $this->compatHeaders, $signatures));
173183
}
174184
if (!$response->headers->has('Content-Security-Policy')) {
175-
$response->headers->add($this->buildHeaders($request, $this->enforce, false, $this->compatHeaders, $signatures));
185+
$response->headers->add($this->buildHeaders($request, $this->enforceDirectiveSetBuilder->buildDirectiveSet(), false, $this->compatHeaders, $signatures));
176186
}
177187
}
178188

@@ -233,4 +243,31 @@ private function buildHeaders(
233243

234244
return $headers;
235245
}
246+
247+
/**
248+
* @param DirectiveSetBuilderInterface|DirectiveSet $builderOrDirectiveSet
249+
*/
250+
private function ensureDirectiveSetBuilder($builderOrDirectiveSet): DirectiveSetBuilderInterface
251+
{
252+
if ($builderOrDirectiveSet instanceof DirectiveSetBuilderInterface) {
253+
return $builderOrDirectiveSet;
254+
}
255+
256+
if ($builderOrDirectiveSet instanceof DirectiveSet) {
257+
trigger_deprecation(
258+
'nelmio/security-bundle',
259+
'3.5',
260+
sprintf(
261+
'Passing %s directly to the %s constructor is deprecated and will be removed in 4.0. Pass a %s instead.',
262+
DirectiveSet::class,
263+
self::class,
264+
DirectiveSetBuilderInterface::class
265+
)
266+
);
267+
268+
return new LegacyDirectiveSetBuilder($builderOrDirectiveSet);
269+
}
270+
271+
throw new \InvalidArgumentException(sprintf('The %s constructor %s expects a or %s.', self::class, DirectiveSetBuilderInterface::class, DirectiveSet::class));
272+
}
236273
}

‎tests/Listener/ContentSecurityPolicyListenerTest.php

+30-2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
namespace Nelmio\SecurityBundle\Tests\Listener;
1515

1616
use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSet;
17+
use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSetBuilderInterface;
1718
use Nelmio\SecurityBundle\ContentSecurityPolicy\NonceGeneratorInterface;
1819
use Nelmio\SecurityBundle\ContentSecurityPolicy\PolicyManager;
1920
use Nelmio\SecurityBundle\ContentSecurityPolicy\ShaComputerInterface;
@@ -464,6 +465,22 @@ public function testHeadersAreNotOverwrittenIfPresent(): void
464465
);
465466
}
466467

468+
public function testLegacyConstructorCreatesDirectiveSetBuilder(): void
469+
{
470+
$reportDirectiveSet = new DirectiveSet(new PolicyManager());
471+
$reportDirectiveSet->setDirective('script-src', 'https://report.deprecation-test.example.com');
472+
473+
$enforceDirectiveSet = new DirectiveSet(new PolicyManager());
474+
$enforceDirectiveSet->setDirective('script-src', 'https://enforce.deprecation-test.example.com');
475+
476+
$listener = new ContentSecurityPolicyListener($reportDirectiveSet, $enforceDirectiveSet, $this->nonceGenerator, $this->shaComputer, true, []);
477+
478+
$response = $this->callListener($listener, '/', true);
479+
480+
$this->assertSame('script-src https://report.deprecation-test.example.com', $response->headers->get('Content-Security-Policy-Report-Only'));
481+
$this->assertSame('script-src https://enforce.deprecation-test.example.com', $response->headers->get('Content-Security-Policy'));
482+
}
483+
467484
/**
468485
* @param array<string, string|true> $directives
469486
* @param list<string> $contentTypes
@@ -473,11 +490,22 @@ private function buildSimpleListener(array $directives, bool $reportOnly = false
473490
$directiveSet = new DirectiveSet(new PolicyManager());
474491
$directiveSet->setDirectives($directives);
475492

493+
$builder = $this->createDirectiveSetBuilderMock($directiveSet);
494+
$dummyBuilder = $this->createDirectiveSetBuilderMock(new DirectiveSet(new PolicyManager()));
495+
476496
if ($reportOnly) {
477-
return new ContentSecurityPolicyListener($directiveSet, new DirectiveSet(new PolicyManager()), $this->nonceGenerator, $this->shaComputer, $compatHeaders, $contentTypes);
497+
return new ContentSecurityPolicyListener($builder, $dummyBuilder, $this->nonceGenerator, $this->shaComputer, $compatHeaders, $contentTypes);
478498
}
479499

480-
return new ContentSecurityPolicyListener(new DirectiveSet(new PolicyManager()), $directiveSet, $this->nonceGenerator, $this->shaComputer, $compatHeaders, $contentTypes);
500+
return new ContentSecurityPolicyListener($dummyBuilder, $builder, $this->nonceGenerator, $this->shaComputer, $compatHeaders, $contentTypes);
501+
}
502+
503+
private function createDirectiveSetBuilderMock(DirectiveSet $directiveSet): DirectiveSetBuilderInterface
504+
{
505+
$mock = $this->createMock(DirectiveSetBuilderInterface::class);
506+
$mock->method('buildDirectiveSet')->willReturn($directiveSet);
507+
508+
return $mock;
481509
}
482510

483511
/**

‎tests/Twig/IntegrationTest.php

+13-4
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
namespace Nelmio\SecurityBundle\Tests\Twig;
1515

1616
use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSet;
17+
use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSetBuilderInterface;
1718
use Nelmio\SecurityBundle\ContentSecurityPolicy\NonceGeneratorInterface;
1819
use Nelmio\SecurityBundle\ContentSecurityPolicy\PolicyManager;
1920
use Nelmio\SecurityBundle\ContentSecurityPolicy\ShaComputerInterface;
@@ -45,8 +46,8 @@ public function testItWorksDynamically(): void
4546
$policyManager = new PolicyManager();
4647

4748
$listener = new ContentSecurityPolicyListener(
48-
new DirectiveSet($policyManager),
49-
new DirectiveSet($policyManager),
49+
$this->createDirectiveSetBuilderMock(new DirectiveSet($policyManager)),
50+
$this->createDirectiveSetBuilderMock(new DirectiveSet($policyManager)),
5051
$this->createStub(NonceGeneratorInterface::class),
5152
$shaComputer
5253
);
@@ -94,8 +95,8 @@ public function testItWorksStatically(): void
9495
$policyManager = new PolicyManager();
9596

9697
$listener = new ContentSecurityPolicyListener(
97-
new DirectiveSet($policyManager),
98-
new DirectiveSet($policyManager),
98+
$this->createDirectiveSetBuilderMock(new DirectiveSet($policyManager)),
99+
$this->createDirectiveSetBuilderMock(new DirectiveSet($policyManager)),
99100
$this->createStub(NonceGeneratorInterface::class),
100101
$shaComputer
101102
);
@@ -127,4 +128,12 @@ public function testItWorksStatically(): void
127128

128129
$this->assertSame(['script-src' => ['sha-script'], 'style-src' => ['sha-style']], $getSha($listener));
129130
}
131+
132+
private function createDirectiveSetBuilderMock(DirectiveSet $directiveSet): DirectiveSetBuilderInterface
133+
{
134+
$mock = $this->createMock(DirectiveSetBuilderInterface::class);
135+
$mock->method('buildDirectiveSet')->willReturn($directiveSet);
136+
137+
return $mock;
138+
}
130139
}

0 commit comments

Comments
 (0)