Loading…
Use state to mitigate CSRF #612
4 participants
Someone, please get assigned, or we'll get hundred pull-requests at once :)
@AlexanderPavlenko we need fix asap :)
omniauth should automatically generate random state, save it in user session and check equality on callback_phase
10 LOC + tests..
@homakov could you please test it? https://github.com/AlexanderPavlenko/omniauth-oauth2/tree/csrf
it looks good solution. 3.times.map{ rand.to_s[2..-1] }.reduce(&:concat) strange random..
strange random
nice one, for me :)
About solution — the only thing I fear is some dumb providers that just wouldn't pass back that state param. But if it's rare and code really works (I didn't have a chance to test it with the real OAuth providers, that's why I asked for your help), we can create pull request for it.
To read more, go here and search for "state" (keep the double quotes).
@AlexanderPavlenko Seems good. Tried it and works :)
Why not use ruby's built in SecureRandom instead of generating the random by hand? Something like SecureRandom.hex(24)? (just be sure to require 'securerandom')
@kitop fixed
Excellent!
Hope this gets merged! Are you going to send a pull request?
Here is it intridea/omniauth-oauth2#18
@homakov aaand it's merged, so this issue can be closed
good job guys
I have a question, why omniauth doesn't provide 'state' param to prevent CSRF? For example facebook supports it and returns it back to make sure you are who started auth process. There should be 2 params: code and state. Now it's only code. And it is vulnerable. Should I add one?