Skip to content

Commit

Permalink
Introduce DirectiveSetBuilderInterface to allow runtime modification
Browse files Browse the repository at this point in the history
  • Loading branch information
martijnc committed Jul 3, 2024
1 parent 6a12626 commit e86b401
Show file tree
Hide file tree
Showing 7 changed files with 222 additions and 27 deletions.
63 changes: 63 additions & 0 deletions src/ContentSecurityPolicy/ConfigurationDirectiveSetBuilder.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?php

declare(strict_types=1);

/*
* This file is part of the Nelmio SecurityBundle.
*
* (c) Nelmio <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Nelmio\SecurityBundle\ContentSecurityPolicy;

class ConfigurationDirectiveSetBuilder implements DirectiveSetBuilderInterface
{
private PolicyManager $policyManager;

/**
* @phpstan-var array{
* enforce?: array<string, mixed>,
* report?: array<string, mixed>,
* } $config
*/
private array $config;

/**
* @phpstan-var 'enforce'|'report' $kind
*/
private string $kind;

/**
* @phpstan-param array{
* enforce?: array<string, mixed>,
* report?: array<string, mixed>,
* } $config
* @phpstan-param 'enforce'|'report' $kind
*/
public function __construct(PolicyManager $policyManager, array $config, string $kind)
{
$this->policyManager = $policyManager;
$this->config = $config;
$this->kind = $kind;
}

public function buildDirectiveSet(): DirectiveSet
{
return DirectiveSet::fromConfig($this->policyManager, $this->config, $this->kind);
}

/**
* @phpstan-param array{
* enforce?: array<string, mixed>,
* report?: array<string, mixed>,
* } $config
* @phpstan-param 'enforce'|'report' $kind
*/
public static function create(PolicyManager $policyManager, array $config, string $kind): self
{
return new self($policyManager, $config, $kind);
}
}
19 changes: 19 additions & 0 deletions src/ContentSecurityPolicy/DirectiveSetBuilderInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

declare(strict_types=1);

/*
* This file is part of the Nelmio SecurityBundle.
*
* (c) Nelmio <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Nelmio\SecurityBundle\ContentSecurityPolicy;

interface DirectiveSetBuilderInterface
{
public function buildDirectiveSet(): DirectiveSet;
}
32 changes: 32 additions & 0 deletions src/ContentSecurityPolicy/LegacyDirectiveSetBuilder.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

declare(strict_types=1);

/*
* This file is part of the Nelmio SecurityBundle.
*
* (c) Nelmio <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Nelmio\SecurityBundle\ContentSecurityPolicy;

/**
* @internal
*/
class LegacyDirectiveSetBuilder implements DirectiveSetBuilderInterface
{
private DirectiveSet $directiveSet;

public function __construct(DirectiveSet $directiveSet)
{
$this->directiveSet = $directiveSet;
}

public function buildDirectiveSet(): DirectiveSet
{
return clone $this->directiveSet;
}
}
25 changes: 16 additions & 9 deletions src/DependencyInjection/NelmioSecurityExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@

namespace Nelmio\SecurityBundle\DependencyInjection;

use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSet;
use Nelmio\SecurityBundle\ContentSecurityPolicy\ConfigurationDirectiveSetBuilder;
use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSetBuilderInterface;
use Symfony\Component\Config\Definition\Processor;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\ContainerBuilder;
Expand Down Expand Up @@ -50,11 +51,17 @@ public function load(array $configs, ContainerBuilder $container): void

$cspConfig = $config['csp'];

$enforceDefinition = $this->buildDirectiveSetDefinition($container, $cspConfig, 'enforce');
$reportDefinition = $this->buildDirectiveSetDefinition($container, $cspConfig, 'report');
$enforceDefinition = $this->createDirectiveSetBuilder($container, $cspConfig, 'enforce');
$reportDefinition = $this->createDirectiveSetBuilder($container, $cspConfig, 'report');

$container->addDefinitions([
'nelmio_security.directive_set_builder.report' => $reportDefinition,
'nelmio_security.directive_set_builder.enforce' => $enforceDefinition,
]);

$cspListenerDefinition = $container->getDefinition('nelmio_security.csp_listener');
$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']]);
$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']]);

$container->setParameter('nelmio_security.csp.hash_algorithm', $cspConfig['hash']['algorithm']);

if (isset($cspConfig['request_matcher'])) {
Expand Down Expand Up @@ -179,11 +186,11 @@ public function load(array $configs, ContainerBuilder $container): void
* } $config
* @phpstan-param 'enforce'|'report' $type
*/
private function buildDirectiveSetDefinition(ContainerBuilder $container, array $config, string $type): Definition
private function createDirectiveSetBuilder(ContainerBuilder $container, array $config, string $type): Definition
{
$directiveDefinition = new Definition(DirectiveSet::class);
$builderDefinition = new Definition(DirectiveSetBuilderInterface::class);

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

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

Expand All @@ -198,8 +205,8 @@ private function buildDirectiveSetDefinition(ContainerBuilder $container, array
$pmDefinition->setArguments([new Reference('nelmio_security.ua_parser')]);
}

$directiveDefinition->setArguments([$pmDefinition, $config, $type]);
$builderDefinition->setArguments([$pmDefinition, $config, $type]);

return $directiveDefinition;
return $builderDefinition;
}
}
61 changes: 49 additions & 12 deletions src/EventListener/ContentSecurityPolicyListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
namespace Nelmio\SecurityBundle\EventListener;

use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSet;
use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSetBuilderInterface;
use Nelmio\SecurityBundle\ContentSecurityPolicy\LegacyDirectiveSetBuilder;
use Nelmio\SecurityBundle\ContentSecurityPolicy\NonceGeneratorInterface;
use Nelmio\SecurityBundle\ContentSecurityPolicy\ShaComputerInterface;
use Symfony\Component\HttpFoundation\Request;
Expand All @@ -24,8 +26,8 @@

final class ContentSecurityPolicyListener extends AbstractContentTypeRestrictableListener
{
private DirectiveSet $report;
private DirectiveSet $enforce;
private DirectiveSetBuilderInterface $reportDirectiveSetBuilder;
private DirectiveSetBuilderInterface $enforceDirectiveSetBuilder;
private bool $compatHeaders;

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

/**
* @param list<string> $hosts
* @param list<string> $contentTypes
* @param DirectiveSetBuilderInterface|DirectiveSet $reportDirectiveSetBuilder
* @param DirectiveSetBuilderInterface|DirectiveSet $enforceDirectiveSetBuilder
* @param list<string> $hosts
* @param list<string> $contentTypes
*/
public function __construct(
DirectiveSet $report,
DirectiveSet $enforce,
$reportDirectiveSetBuilder,
$enforceDirectiveSetBuilder,
NonceGeneratorInterface $nonceGenerator,
ShaComputerInterface $shaComputer,
bool $compatHeaders = true,
Expand All @@ -59,8 +63,8 @@ public function __construct(
?RequestMatcherInterface $requestMatcher = null
) {
parent::__construct($contentTypes);
$this->report = $report;
$this->enforce = $enforce;
$this->reportDirectiveSetBuilder = $this->ensureDirectiveSetBuilder($reportDirectiveSetBuilder);
$this->enforceDirectiveSetBuilder = $this->ensureDirectiveSetBuilder($enforceDirectiveSetBuilder);
$this->compatHeaders = $compatHeaders;
$this->hosts = $hosts;
$this->nonceGenerator = $nonceGenerator;
Expand Down Expand Up @@ -110,14 +114,20 @@ public function addStyle(string $html): void
$this->sha['style-src'][] = $this->shaComputer->computeForStyle($html);
}

/**
* @deprecated Use `nelmio_security.directive_set_builder.report` instead.
*/
public function getReport(): DirectiveSet
{
return $this->report;
return $this->reportDirectiveSetBuilder->buildDirectiveSet();
}

/**
* @deprecated Use `nelmio_security.directive_set_builder.enforce` instead.
*/
public function getEnforcement(): DirectiveSet
{
return $this->enforce;
return $this->enforceDirectiveSetBuilder->buildDirectiveSet();
}

public function getNonce(string $usage): string
Expand Down Expand Up @@ -169,10 +179,10 @@ public function onKernelResponse(ResponseEvent $e): void
}

if (!$response->headers->has('Content-Security-Policy-Report-Only')) {
$response->headers->add($this->buildHeaders($request, $this->report, true, $this->compatHeaders, $signatures));
$response->headers->add($this->buildHeaders($request, $this->reportDirectiveSetBuilder->buildDirectiveSet(), true, $this->compatHeaders, $signatures));
}
if (!$response->headers->has('Content-Security-Policy')) {
$response->headers->add($this->buildHeaders($request, $this->enforce, false, $this->compatHeaders, $signatures));
$response->headers->add($this->buildHeaders($request, $this->enforceDirectiveSetBuilder->buildDirectiveSet(), false, $this->compatHeaders, $signatures));
}
}

Expand Down Expand Up @@ -233,4 +243,31 @@ private function buildHeaders(

return $headers;
}

/**
* @param DirectiveSetBuilderInterface|DirectiveSet $builderOrDirectiveSet
*/
private function ensureDirectiveSetBuilder($builderOrDirectiveSet): DirectiveSetBuilderInterface
{
if ($builderOrDirectiveSet instanceof DirectiveSetBuilderInterface) {
return $builderOrDirectiveSet;
}

if ($builderOrDirectiveSet instanceof DirectiveSet) {
trigger_deprecation(
'nelmio/security-bundle',
'3.3',
sprintf(
'Passing %s directly to the %s constructor is deprecated and will be removed in 4.0. Pass a %s instead.',
DirectiveSet::class,
self::class,
DirectiveSetBuilderInterface::class
)
);

return new LegacyDirectiveSetBuilder($builderOrDirectiveSet);
}

throw new \InvalidArgumentException(sprintf('The %s constructor %s expects a or %s.', self::class, DirectiveSetBuilderInterface::class, DirectiveSet::class));
}
}
32 changes: 30 additions & 2 deletions tests/Listener/ContentSecurityPolicyListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
namespace Nelmio\SecurityBundle\Tests\Listener;

use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSet;
use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSetBuilderInterface;
use Nelmio\SecurityBundle\ContentSecurityPolicy\NonceGeneratorInterface;
use Nelmio\SecurityBundle\ContentSecurityPolicy\PolicyManager;
use Nelmio\SecurityBundle\ContentSecurityPolicy\ShaComputerInterface;
Expand Down Expand Up @@ -464,6 +465,22 @@ public function testHeadersAreNotOverwrittenIfPresent(): void
);
}

public function testLegacyConstructorCreatesDirectiveSetBuilder(): void
{
$reportDirectiveSet = new DirectiveSet(new PolicyManager());
$reportDirectiveSet->setDirective('script-src', 'https://report.deprecation-test.example.com');

$enforceDirectiveSet = new DirectiveSet(new PolicyManager());
$enforceDirectiveSet->setDirective('script-src', 'https://enforce.deprecation-test.example.com');

$listener = new ContentSecurityPolicyListener($reportDirectiveSet, $enforceDirectiveSet, $this->nonceGenerator, $this->shaComputer, true, []);

$response = $this->callListener($listener, '/', true);

$this->assertSame('script-src https://report.deprecation-test.example.com', $response->headers->get('Content-Security-Policy-Report-Only'));
$this->assertSame('script-src https://enforce.deprecation-test.example.com', $response->headers->get('Content-Security-Policy'));
}

/**
* @param array<string, string|true> $directives
* @param list<string> $contentTypes
Expand All @@ -473,11 +490,22 @@ private function buildSimpleListener(array $directives, bool $reportOnly = false
$directiveSet = new DirectiveSet(new PolicyManager());
$directiveSet->setDirectives($directives);

$builder = $this->createDirectiveSetBuilderMock($directiveSet);
$dummyBuilder = $this->createDirectiveSetBuilderMock(new DirectiveSet(new PolicyManager()));

if ($reportOnly) {
return new ContentSecurityPolicyListener($directiveSet, new DirectiveSet(new PolicyManager()), $this->nonceGenerator, $this->shaComputer, $compatHeaders, $contentTypes);
return new ContentSecurityPolicyListener($builder, $dummyBuilder, $this->nonceGenerator, $this->shaComputer, $compatHeaders, $contentTypes);
}

return new ContentSecurityPolicyListener(new DirectiveSet(new PolicyManager()), $directiveSet, $this->nonceGenerator, $this->shaComputer, $compatHeaders, $contentTypes);
return new ContentSecurityPolicyListener($dummyBuilder, $builder, $this->nonceGenerator, $this->shaComputer, $compatHeaders, $contentTypes);
}

private function createDirectiveSetBuilderMock(DirectiveSet $directiveSet): DirectiveSetBuilderInterface
{
$mock = $this->createMock(DirectiveSetBuilderInterface::class);
$mock->method('buildDirectiveSet')->willReturn($directiveSet);

return $mock;
}

/**
Expand Down
17 changes: 13 additions & 4 deletions tests/Twig/IntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
namespace Nelmio\SecurityBundle\Tests\Twig;

use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSet;
use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSetBuilderInterface;
use Nelmio\SecurityBundle\ContentSecurityPolicy\NonceGeneratorInterface;
use Nelmio\SecurityBundle\ContentSecurityPolicy\PolicyManager;
use Nelmio\SecurityBundle\ContentSecurityPolicy\ShaComputerInterface;
Expand Down Expand Up @@ -45,8 +46,8 @@ public function testItWorksDynamically(): void
$policyManager = new PolicyManager();

$listener = new ContentSecurityPolicyListener(
new DirectiveSet($policyManager),
new DirectiveSet($policyManager),
$this->createDirectiveSetBuilderMock(new DirectiveSet($policyManager)),
$this->createDirectiveSetBuilderMock(new DirectiveSet($policyManager)),
$this->createStub(NonceGeneratorInterface::class),
$shaComputer
);
Expand Down Expand Up @@ -94,8 +95,8 @@ public function testItWorksStatically(): void
$policyManager = new PolicyManager();

$listener = new ContentSecurityPolicyListener(
new DirectiveSet($policyManager),
new DirectiveSet($policyManager),
$this->createDirectiveSetBuilderMock(new DirectiveSet($policyManager)),
$this->createDirectiveSetBuilderMock(new DirectiveSet($policyManager)),
$this->createStub(NonceGeneratorInterface::class),
$shaComputer
);
Expand Down Expand Up @@ -127,4 +128,12 @@ public function testItWorksStatically(): void

$this->assertSame(['script-src' => ['sha-script'], 'style-src' => ['sha-style']], $getSha($listener));
}

private function createDirectiveSetBuilderMock(DirectiveSet $directiveSet): DirectiveSetBuilderInterface
{
$mock = $this->createMock(DirectiveSetBuilderInterface::class);
$mock->method('buildDirectiveSet')->willReturn($directiveSet);

return $mock;
}
}

0 comments on commit e86b401

Please sign in to comment.