search

isaacsanders / omniauth-stripe-connect

Stripe Connect OAuth2 Strategy for OmniAuth 1.0
MIT License
130 stars 74 forks source link

provider_ignores_state: true possible #34

Closed romeo4934 closed 9 years ago

romeo4934 commented 9 years ago

Hi! I noticed that if the user is edit several times the stripe page we can get

Authentication failure! csrf_detected: OmniAuth::Strategies::OAuth2::CallbackError, csrf_detected | CSRF detected

How should we handle this case ?

Kind regards, Antoine

isaacsanders commented 9 years ago

What version of the gem are you working with?

bolom commented 9 years ago

@isaacsanders i have got the same issue

isaacsanders commented 9 years ago

What version of the gem are you using? I believe that this is fixed in the most recent version.

mgraulich commented 9 years ago

@isaacsanders I'm using 2.7.0 and getting the same error.

I had to rollback to 2.6.0 to get this to work. I tried different configuration changes with 2.7.0 to no prevail (did not want to set provider_ignores_state to true).

Using rails 4.1.10.

isaacsanders commented 9 years ago

Okay. Anyone online to work with me on this? I am ready to devote some hours to this, but I will need someone to help me understand the problem more.

(Update: I have no internet at home, but I will update again, and mention people, when I get to internet again)

mgraulich commented 9 years ago

I had to update back up to 2.7.0... When i rolled back to 2.6.0 I started seeing the csrf_detected issue with my facebook oAuth flow. I didn't have this issue until i updated rails from 4.0.5 to 4.1.10. I currently have facebook and stripe_connect declared in my omniauth.rb initializer (i'm not using the devise flow). I had to add the provider_ignores_state to the initializer to get everything to work together on 4.1.10.

isaacsanders commented 9 years ago

So, the Stripe security people contacted me to remove the provider_ignores_state option. I imagine it is important to them that it doesn't ignore state. I don't know why it would break things.

I will try to loop in my contact on this.

On Jun 13, 2015, at 6:33 PM, mgraulich notifications@github.com wrote:

I had to update back up to 2.7.0... When i rolled back to 2.6.0 I started seeing the csrf_detected issue with my facebook oAuth flow. I didn't have this issue until i updated rails from 4.0.5 to 4.1.10. I currently have facebook and stripe_connect declared in my omniauth.rb initializer (i'm not using the devise flow). I had to add the provider_ignores_state to the initializer to get everything to work together on 4.1.10.

— Reply to this email directly or view it on GitHub.

mgraulich commented 9 years ago

Sounds good. Yeah, it's kind of strange. I was passing in a state param (https://stripe.com/docs/connect/reference) but it was still failing when i removed that.

isaacsanders commented 9 years ago

@antoineherzog @bolom @mgraulich

Hey. I just found a bug in omniauth-oauth2 that might have been the cause of this bug. I am so sorry it has taken this long to get this resolved. I hope that a new version of omniauth-oauth2 will come out soon with my fix in it, then I will release another version of this gem, hopefully fixing all of your problems.

Thank you for your patience.

– Isaac

isaacsanders commented 9 years ago

@antoineherzog @bolom @mgraulich

Just released v2.8.0.

Please tell me if the issue persists. We can reopen from there.

ghost commented 9 years ago

I have also gotten this issue with the latest version:

E, [2015-07-25T22:29:12.253165 #30793] ERROR -- omniauth: (stripe_connect) Authentication failure! csrf_detected: OmniAuth::Strategies::OAuth2::CallbackError, csrf_detected | CSRF detected

What i ended up doing is downgrading to 2.6.0 and removing everything from omniauth.rb, and the account creation went through.

isaacsanders commented 9 years ago

I have just run through the login flow with out the provider_ignores_state parameter using https://github.com/PracticallyGreen/omniauth-test-harness

I had no issues on 2.8.0. Please try to recreate the error using the test harness mentioned above.

Thanks for your help in fixing this issue.

isaacsanders commented 9 years ago

Realized that Stripe might have misunderstood this parameter's purpose. Adding it back in and telling the Stripe folks.

isaacsanders commented 9 years ago

Version 2.9.0 going out today. Let me know if this works for you.

eric-norcross commented 6 years ago

@isaacsanders - can you elaborate on what the misunderstanding was and why the provider_ignores_state parameter was added back in? Doesn't disregarding the state/csrf token create a vulnerability? (or am I misunderstanding it's purpose as well?)