Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New option for csrf #2121

Merged
merged 3 commits into from
Aug 18, 2014
Merged

New option for csrf #2121

merged 3 commits into from
Aug 18, 2014

Conversation

pjpoirson
Copy link
Contributor

You can disable csrf for a route with module.exports.csrf.routesDisabled

Hi,

I had a problem with our facebook app and csrf. To display the homepage a form is used in Post

The _csrf param can't be passed here. So I need to disable this protection for the page https://game.zombie-outbreak.eu/pregame/facebook/. So I made the change and I add this option in my config/csrf.js : routesDisabled: '/pregame/facebook/'. And now it's ok for my first route call.

Before making these changes I found this "issue": #2096
So I tried to use the solution give by @sgress454

The only way to disable it for a particular route would be to add some custom middleware after the session middleware that checks req.url, and if it matches /upload/image, places the CSRF token in req.params as if it had been send with the request.

It's doesn't work for me.

If I put my custom middleware here

order: [
  'startRequestTimer',
  'cookieParser',
  'session',      
  'bodyParser', 
  'handleBodyParserError',
  'compress',
  'methodOverride',
  'poweredBy',
  '$custom',
  'disableCSFRtoken', // <===
  'router',          
  'www',
  'favicon',
  '404',
  '500'
],

disableCSFRtoken() si called but I can't access to req.csrfToken() or res.locals._csrf too add the csrf value in req.param/body

If I put it after the router middlware disableCSFRtoken() is not called in my case

'router',
'disableCSFRtoken', // <===

You can disable csrf for a route with module.exports.csrf.routesDisabled
@sgress454
Copy link
Member

Cool...if you can write a test and put it in here in test/integration/hooks.cors_csrf.test.js, we'd be happy to merge.

CSRF config ::
    ...
    with CSRF set to {protectionEnabled: true, routesDisabled: '/user'}
        ✓ a POST request on /user without a CSRF token should result in a 200 response
        ✓ a POST request on /test without a CSRF token should result in a 403 response
@pjpoirson
Copy link
Contributor Author

Ok, done

CSRF config ::
...
  with CSRF set to {protectionEnabled: true, routesDisabled: '/user'}
    ✓ a POST request on /user without a CSRF token should result in a 200 response 
    ✓ a POST request on /test without a CSRF token should result in a 403 response

@sgress454
Copy link
Member

Brilliant, thanks!

sgress454 added a commit that referenced this pull request Aug 18, 2014
@sgress454 sgress454 merged commit 0ad8691 into balderdashy:master Aug 18, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants