search

krakenjs / lusca

Application security for express apps.
Other
1.79k stars 122 forks source link

Correcting header value and implementation of report-uri #17

Closed bluelnkd closed 10 years ago

bluelnkd commented 10 years ago

Report-uri was inconsistent with the header values specified by CSP documentation: https://developer.mozilla.org/en-US/docs/Security/CSP/CSP_policy_directives#report-uri

It should post a header value of report-uri rather than reportUri

Also the option was mismatching latest Kraken docs under middleware: 

report-uri - URI the browser should send the report to.
bluelnkd commented 10 years ago

Few additional updates -- CSP is a POST not a GET. Updated the unit test.

Added the ability to whitelist URLs in CSRF. Currently an app cannot have a CSP report-uri that is also protected by CSRF. Whitelisting CSP reporters seems to be an appropriate workaround to allow a POST without a CSRF token.

jeffharrell commented 10 years ago

Hey Stephen,

This won't cleanly merge now that the refactored PR was pulled in. Can you take a look at that?

Also, do you mind breaking the whitelisting out into it's own PR so it can be dealt with separately? Specifically on that note, we've been trying to avoid having white listing params for each set of functionality and instead include middleware-route-specific functionality at the meddleware level (see enabled option which can be set at runtime). Would this satisfy that requirement? If so, this will be available in the upcoming kraken 1.0 release.

bluelnkd commented 10 years ago

@jeffharrell Updated! Meddleware's gonna be awesome! :+1:

I'll update the Kraken-JS readme after this gets merged... it's still referencing report-uri as the option value, and probably should stay as reportUri to be consistent with how we're camelCase'ing for all the other options.

jeffharrell commented 10 years ago

Coolio, thanks!

jeffharrell commented 10 years ago

Merging as it's just a header typo change.