search

hapijs / crumb

CSRF crumb generation and validation for hapi
Other
171 stars 51 forks source link

Restful validation with autoGenerate set to false not working #94

Closed davidkaminsky closed 6 years ago

davidkaminsky commented 7 years ago

Hello, I'm having some issues getting crumb to work. I'm using hapi as my API server and serving the front-end via an express server that does an initial server render of React.

I'm going about adding CSRF protection to the API server by using restful validation where I create a route to generate a crumb. That route is called by the express server to get the crumb and injected into the DOM to then be read by the client-side JS. The client-side JS adds the crumb to the X-CSRF-Token header for all calls to the API. This all seems to work fine but it's not actually validating anything. It seems that when the restful option is set to true and the autoGenerate option to false, it just skips validation.

It does so because request.route.settings.plugins._crumb is false on this line: https://github.com/hapijs/crumb/blob/master/lib/index.js#L118

That seems to be false because settings.autoGenerate is false on this line: https://github.com/hapijs/crumb/blob/master/lib/index.js#L70

I'm not 100% sure what the best fix is to this or if I'm just doing something fundamentally wrong to begin with. I'd really appreciate some guidance here. Thanks!

davidkaminsky commented 7 years ago

Would really appreciate some help here if anyone has any insight into how to proceed. Thanks!

ryanwilliamquinn commented 7 years ago

I'm curious about this issue too. Why does the autogenerate toggle impact whether or not a route is validated?

ryanwilliamquinn commented 7 years ago

I think the restful flag is irrelevant for the issue, the behavior is the same regardless of its value.

jonathansamines commented 7 years ago

@davidkaminsky did you solved this issue? If not, could you provide a reduced reproduction of your issue?

davidkaminsky commented 7 years ago

No, taking another look at this now... will update.

davidkaminsky commented 7 years ago

Ok, so it appears that allowing for autogenerating the crumb isn't a problem. I don't need it but it doesn't hurt anything so I've moved on leaving the default of it autogenerating on. That said, you should be able to reproduce this by basically using the example: https://github.com/hapijs/crumb/blob/master/example/restful.js and setting the autoGenerate option to false.

spanditcaa commented 6 years ago

closing this as a duplicate of #108 (more detail/possible fix in that issue)

lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.