socialcast / devise_oauth2_providable

Rails3 engine integrating OAuth2 authentication with Devise
MIT License
219 stars 102 forks source link

redirect_uri dissapears after "Approve/Deny" form #28

Closed shemusok closed 12 years ago

shemusok commented 12 years ago

after I send GET

http://server/oauth2/authorize?response_type=code&client_id=someid&redirect_uri=callbackuri

to server, which use devise_oauth2_providable, I got approve/deny page in my browser I click 'approve' and got

Invalid Authorization Request Invalid redirect_uri is given

I found (with debugger) that

  1. at first run of Devise::Oauth2Providable::AuthorizationsController#authorize_endpoint req.redirect_uri is OK (also I see it in the browser address line:) and it matches @client.redirect_uri
  2. at second run (after I click 'Approve') req.redirect_uri is empty (also, no value= property in code of "approve/deny" form)

I'm not sure that it's an issue, but in any case I really need a help:) thanx rails 3.1.3 ruby 1.8.7

shemusok commented 12 years ago

I found, that Devise::Oauth2Providable::AuthorizationsController#authorize_endpoint doesn't set @redirect_uri, which used from app/views/devise/oauth2_providable/authorizations/new.html.erb. After adding

@redirect_uri = res.redirect_uri

all (I hope) works good.

I'll not create a patch, because I'm still not sure 'bout "rails way" to solve this problem, and it'll be good to see any comments)

gregwinn commented 12 years ago

shemusok, This does not relate to your question but how did you generate the views for oauth2_providable? I had tried "rails g devise_oauth2_providable:install" but no luck...

Thanks

shemusok commented 12 years ago

in fact, I hadn't generate any views at all, I was just playing with https://github.com/ZenCocoon/testoauth2provider, and it already had some) I'm not sure if such generators are exists

gregwinn commented 12 years ago

I do not see app/views/devise/oauth2_providable/authorizations/new.html.erb in the test project https://github.com/ZenCocoon/testoauth2provider

Did you just create it??

shemusok commented 12 years ago

so, u wanna this views:) u could find 'em in the https://github.com/socialcast/devise_oauth2_providable app/views/etc.

wireframe commented 12 years ago

there's definitely a bug if the @redirect_uri variable is not being set. i'll whip up a patch shortly.

shemusok commented 12 years ago

or may be just use @client.redirect_uri in views? What is "rails way"?;)

wireframe commented 12 years ago

fixed in v1.0.4

jonbuffington commented 12 years ago

Unfortunately, the 1.0.4 fix does not help with Ruby 1.9.2. Any interest in in using ntodd's assignment suggestion in the commit note?

wireframe commented 12 years ago

@jonbuffington I'm ruby 1.9.3 and not seeing an issue. can you reproduce via a testcase?

jonbuffington commented 12 years ago

@wireframe Hmm. As of rack-oauth2-0.11.0 and Ruby 1.9.2p290, @redirect_uri will always be nil. I think the issue is easier to illustrate with the following example since Request#verify_redirect_uri! only returns one value, a string.

def foo 'hello' end

res.redirect_uri, @redirect_uri = foo

res.redirect_uri => "hello" @redirect_uri => nil

In contrast to: res.redirect_uri = @redirect_uri = foo res.redirect_uri => "hello" @redirect_uri => "hello"

Am I missing something? Does the second form of assignment not work in 1.9.3? Unfortunately, I do not have access to 1.9.3 at the moment to test.

gregwinn commented 12 years ago

Adding @client.redirect_uri to the view worked for me.

wireframe commented 12 years ago

should be fixed in v1.0.5.

a new testcase has been added and verified that it is fixed.