krakenjs / lusca

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

Allow CSRF cookie options to be set #104

Closed stgogm closed 7 years ago

stgogm commented 7 years ago

This fix allows us to set cookie options and maintain compatibility with current configurations.

Example configurations:

// Using AngularJS with cookie options
lusca.csrf({
  angular: true,
  cookie: {
    options: {
      httpOnly: true,
      secure: true
    }
  }
});
// Using custom cookie name and options.
lusca.csrf({
  cookie: {
    name: 'MyCustomCSRFCookieName',
    options: {
      httpOnly: true,
      secure: true
    }
  }
});
// Using this also works
lusca.csrf({
  cookie: 'MyCustomCSRFCookieName'
});
// Using this works too
lusca.csrf({
  cookie: {
    name: 'MyCustomCSRFCookieName'
  }
});
doublerebel commented 7 years ago

Without being able to set cookie options, I receive warnings when running the OWASP ZAP test because httpOnly and secure are not set on my csrf cookie. LGTM!

EDIT: OWASP reference https://www.owasp.org/index.php/Testing_for_cookies_attributes_(OTG-SESS-002)

stgogm commented 7 years ago

@doublerebel That's the main reason behind this PR as we performed an Acunetix audit on our applications and the only warning we had was with an insecure cookie (XSRF).

Sadly, it's been quite a while since I made this pull request.

Anyway, I've added the respective tests and improved cookie options validations.

If you like, you can install lusca with this fix until it's merged using npm i https://github.com/stgogm/lusca.git#patch-2

grawk commented 7 years ago

Hey @stgogm. We'll assess this PR this week. Thanks for submitting it.

grawk commented 7 years ago

Seems fine from my perspective :+1:

stgogm commented 7 years ago

@grawk Awesome! Let me know if you need anything else.

stgogm commented 7 years ago

Hi @grawk

Any news regarding this?

stgogm commented 7 years ago

Finally decided to take my own approach on this matter. If anyone else is interested, you're welcome to try Fi Aegis. We've forked this project and added some functionalities, improved documentation and made minor code optimizations.

linkRace commented 7 years ago

👍 , will get this in with 1.5.0 release. sorry this repo has been so neglected :(