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

Unable to complete OpenID login with ruby-openid 2.9.0/2.9.1 #125

Open
madsolar8582 opened this issue Oct 11, 2019 · 4 comments
Open

Unable to complete OpenID login with ruby-openid 2.9.0/2.9.1 #125

madsolar8582 opened this issue Oct 11, 2019 · 4 comments

Comments

@madsolar8582
Copy link

@madsolar8582 madsolar8582 commented Oct 11, 2019

We've been testing the latest version of ruby-openid in our development environment and found that we are no longer able to successfully complete login. We started seeing

Unexpected OpenID response: #<OpenID::Consumer::FailureResponse:0x000055e490435880 @endpoint=nil, @message="Unable to contact OpenID server: bad URI(is not URI?): nil", @contact=nil, @reference=nil>

in our logs, which lead us to find out that #121 broke our login flow (note endpoint is nil).

Here is what we are doing:

  1. Discover the OpenID endpoint to login to (OpenID::OpenIDServiceEndpoint).
  2. Create a OpenID::Consumer to generate a OpenID::Consumer::CheckIDRequest (without discovery).
  3. Add the attributes we want about the user Attribute Exchange extension to the request.
  4. Add the OAuth (OpenID::OAuth::Request) extension to the request.
  5. Redirect to the constructed OpenID login URL.
  6. Receive the callback.
  7. Call the complete method on the consumer.
  8. Validate the OpenID response and that we received all of the requested attributes.

As it turns out, the call to complete, which then calls, handle_idres, which calls id_res and causes the problem as the verification makes assumptions that are no longer true. When verify_discovery_results was before check_signature, the @endpoint was set or it would perform discovery and set it. However, now that check_signature is getting called first, the @store is nil, so the assoc is set to nil and that triggers a call to check_auth. But, check_auth will never succeed because the make_kv_post method has no endpoint to call, thus preventing successful response validation and preventing login.

We were able to work around this by locking down to 2.8.0. I do realize that the change made in #121 was for security reasons, but we're not sure how to proceed.

@tobiashm

This comment has been minimized.

Copy link
Contributor

@tobiashm tobiashm commented Oct 16, 2019

Okay, it seems that some parts of the code is dependent on the execution order.

If anyone has the time to look into this and make a proposal for how to fix it, it would be much appreciated. I might take a look at it, but I unfortunately have very limit time available.

@utkarsh2102

This comment has been minimized.

Copy link
Contributor

@utkarsh2102 utkarsh2102 commented Oct 20, 2019

Hi @madsolar8582,
Could you tell me how badly is your workflow affected with #126 being merged and released as v2.9.2? Is the problem same or somewhat different?

@madsolar8582

This comment has been minimized.

Copy link
Author

@madsolar8582 madsolar8582 commented Oct 20, 2019

It’s the same, we are unable to complete login.

@utkarsh2102

This comment has been minimized.

Copy link
Contributor

@utkarsh2102 utkarsh2102 commented Oct 20, 2019

Hm, so #126 doesn't really affect anything. Strange.
Thank you! I'll try to figure out what's happening.
However, if you get time @tobiashm to get it working, that'd be great :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.