Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce DirectiveSetBuilderInterface to allow runtime modification #348

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

martijnc
Copy link
Contributor

@martijnc martijnc commented Jun 1, 2024

This PR introduces the DirectiveSetBuilderInterface and the default implementation ConfigurationDirectiveSetBuilder proposed in #347, to allow for runtime modification of the CSP directive sets. The major changes are:

  • Introduction of DirectiveSetBuilderInterface and ConfigurationDirectiveSetBuilder;
  • The ContentSecurityPolicyListener constructor now takes DirectiveSetBuilderInterface instead of the directive sets directly;
  • Changes to NelmioSecurityExtension to provide the configuration to the ConfigurationDirectiveSetBuilders, which in turn are injected into ContentSecurityPolicyListener (instead of the directive sets).

This adds a layer between the configuration (and the directive sets built from it) and ContentSecurityPolicyListener. This layer provides an integration point for application code to modify the directive sets based on the request (e.g., in a controller or a kernel event listener).

@martijnc martijnc force-pushed the feature/directive-set-builder branch from 8ea7fa0 to 7126387 Compare June 1, 2024 18:47
@martijnc martijnc force-pushed the feature/directive-set-builder branch from 7126387 to 65a8e55 Compare June 14, 2024 18:25
@martijnc martijnc force-pushed the feature/directive-set-builder branch from 65a8e55 to e86b401 Compare July 3, 2024 18:07
@Seldaek
Copy link
Member

Seldaek commented Jul 5, 2024

Sorry but I won't manage to review this one right now, seems solid at first sight but I'd rather give it some more thought.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants