NIFI-5366 - Added ContentSecurityPolicyFilter which stops framing of … #2989
Conversation
| </dependency> | ||
| <dependency> | ||
| <groupId>org.eclipse.jetty</groupId> | ||
| <artifactId>jetty-servlet</artifactId> |
There was a problem hiding this comment.
Let's move the non-test dependencies above so they are all together with the compile dependencies and the test dependencies are together. Not a technical necessity, but good for logical grouping and identification.
There was a problem hiding this comment.
Removed one unnecessary dependency and changed the other to scope test (as it's only used in the unit test).
|
Reviewing... |
|
Verified that the header is present on responses. Ran |
…NiFi resources. It applies the Content-Security-Policy header. This protects against clickjacking. NIFI-5366 - Added unit test. Added single quotes around 'self' for frame-ancestors CSP header. NIFI-5366 - Fixed dependencies.
thenatog
force-pushed
the
NIFI-5366-rebased
branch
from
5ba4798
to
b6d065d
Compare
| @@ -502,6 +503,11 @@ private WebAppContext loadWar(final File warFile, final String contextPath, fina | |||
| // add a filter to set the X-Frame-Options filter | |||
| webappContext.addFilter(new FilterHolder(FRAME_OPTIONS_FILTER), "/*", EnumSet.allOf(DispatcherType.class)); | |||
|
|
|||
| // add a filter to set the Content Security Policy frame-ancestors directive | |||
| FilterHolder cspFilter = new FilterHolder(new ContentSecurityPolicyFilter()); | |||
There was a problem hiding this comment.
From what I can tell by tracing the Jetty code, this is almost equivalent to just assigning the Filter directly via addFilter(). Do you have a resource or documentation that indicates why using a FilterHolder is preferable?
There was a problem hiding this comment.
I think I thought there was no way to set the Filter name without passing the FilterHolder but it looks like if you pass the classname as a string it will set the filter name. Setting the filter name is helpful for debugging. Looks like most filters being added use the FilterHolder method definition, not exactly sure why however.
|
Thanks for making the requested changes. In general, please try to avoid rebasing in PRs as it loses the history of the interactions. The reviewer/committer should handle that when merging to master. +1, merged. |
…NiFi resources. It applies the Content-Security-Policy header. This protects against clickjacking.
NIFI-5366 - Added unit test. Added single quotes around 'self' for frame-ancestors CSP header.
Thank you for submitting a contribution to Apache NiFi.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically master)?
Is your initial contribution a single, squashed commit?
For code changes:
For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.