refactor(middleware): Refactor internals of CSPMiddleware so that it's easier to extend existing logic without copy/pasting it into subclass #237
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current implementation of
csp.middleware.CSPMiddleware
doesn't lend itself to extending the functionality of it or thecsp.contrib.rate_limiting.RateLimitedCSPMiddleware
without copy/pasting the implementation of.build_policy()
and.build_policy_ro()
This is a bit of an issue, because it means any updates to the base implementation of the superclass has to be manually copied into any subclasses.
For example, for my multi-tenant webapp project, we use the
RateLimitedCSPMiddleware
and have an additional requirement to generate a CSP that contains some per-tenant configuration. So I've created a custom middleware by copying the code fromRateLimitedCSPMiddleware
and adding my extra logic in.This isn't great because if the implementation of
RateLimitedCSPMiddleware
changes, for example to fix this issue #231, then I have to remember to check for changes and manually copy them into my middleware.So, I'm proposing a change to how CSPMiddleware works to provide a better hook into the existing implementation and allow subclasses to modify things before the policy string is built.
As you can see from the diff, this reduces the lines of code required to create a CSPMiddleware subclass, even more so when extending the
RateLimitedCSPMiddleware
.The
build_policy
andbuild_policy_ro
methods are essentially redundant with this PR, but I wasn't too sure about removing outright, since it would a breaking change, so would like the maintainers' input on that.Finally, I haven't attempted to address any of the known issues already raised in other issues/PRs