krakenjs / lusca

Application security for express apps.
Other
1.78k stars 123 forks source link

Add support for arrays in CSP #72

Closed giladgo closed 7 years ago

giladgo commented 8 years ago

Configuring CSP in lusca makes the config file look unreadable. Oftentimes, you'll have more than 2-3 entries in default-src or img-src, and these can be quite long.

This PR offers the users a chance to specify entries in the policy with an array, which will allow them to break it into lines, and make the config files much more readable and debuggable.

Compare

"policy": {
    "default-src": "'self' https://*.google-analytics.com https://*.mydomain.com https://*.mydomain2.com  http://*.mydomain2.com ",
    "script-src": "'self' https://*.google-analytics.com https://*.mydomain.com https://*.mydomain2.com ",
}

to

"policy": {
    "default-src": ["'self'", 
                "https://*.google-analytics.com",
                "https://*.mydomain.com",
                "https://*.mydomain2.com",
                "http://*.mydomain2.com"],

    "script-src": ["'self'",
               "https://*.google-analytics.com",
               "https://*.mydomain.com",
               "https://*.mydomain2.com"]
}

For backwards compatability, the ability to specify it as a string like before was retained.

giladgo commented 8 years ago

Let's merge?

turboMaCk commented 8 years ago

related: https://github.com/krakenjs/lusca/pull/71

giladgo commented 8 years ago

Ah, didn't notice that #71 takes care of this as well. In that case, let's merge #71 and close this.

turboMaCk commented 8 years ago

@giladgo In fact my PR do not really implement exactly the same functionality, but It's pretty closed. I'm fan of this one too. For me it makes sense to: A) Merge this two together first and refactor same logic – this thow should share some functions. Then we can create one PR including both. B) Merge one of these at first, rebase second branch and refactor that PR.

linkRace commented 7 years ago

Doing some cleanup here, closing this since #79 has this change and more