Support report-to CSP directive and Reporting-Endpoints header #7374
Conversation
…Sync var names with report-to parameter names.
labkey-jeckels
left a comment
labkey-jeckels
left a comment
There was a problem hiding this comment.
Some suggestions, but not sure if they're all viable.
| public static final MimeType CSP = new MimeType("application/csp-report", false, true); | ||
| public static final MimeType CSP_REPORT = new MimeType("application/reports+json", false, true); |
There was a problem hiding this comment.
Consider CSP_V1 and CSP_V3 or similar, as they both seems like CSP reports.
| { | ||
| // Generate the Reporting-Endpoints header value now since its value is static. Use an absolute URL so we | ||
| // always post reports to https:, even when the violating request happens to be http: | ||
| String violationEndpoint = substituteReportParams(baseServerUrl + "/admin-contentSecurityPolicyReportTo.api?${CSP.REPORT.PARAMS}"); |
There was a problem hiding this comment.
Could this use ActionURL, at least for the parts other than the substitution syntax? It could add the CSP version too, which could theoretically need URI encoding
There was a problem hiding this comment.
I tried, but it's quite awkward given the substitution has to be in the middle of the URL. I took another stab. I guess it's okay.
| } | ||
| } | ||
|
|
||
| String baseServerUrl = AppProps.getInstance().getBaseServerUrl(); |
There was a problem hiding this comment.
Does this work when bootstrapping a server? And what happens if the base URL is changed after startup?
There was a problem hiding this comment.
I'll check. Can probably switch to populating this lazily and any time the base server URL changes.
Rationale
Security scanners like to see
report-todirectives, even though not all browsers support them. https://github.com/LabKey/internal-issues/issues/794Related Pull Requests