openid/ruby-openid

Breaking google change, results in "Server https://www.google.com/accounts/o8/ud responds that the 'check_authentication' call is not valid"

cortfr opened this issue · 18 comments

I believe the fix is to no longer include 'assoc_handle' when creating requests with mode checkid_immediate or checkid_setup. See https://groups.google.com/forum/?fromgroups=#!topic/google-federated-login-api/qXZDD7_K7jU .

I can confirm the same error. Traced it to the same Google Group message...

Okay, I'm certain this patch demonstrates a gross misunderstanding of the OpenID specifications, but it's a fix (for me, at least):

kendagriff@ef88f27

(Additional update: kendagriff@79beaa4)

Let me know if this is worthy of a pull request (somehow I doubt it).

Thanks for this @kendagriff! Your patch is working for us at https://rapportive.com/ :D. I've emailed our contact at Google to see if they know what's causing this, will let you know if I get a shareable response.

Thanks for the patch @kendagriff..

Hey @kendagriff. Thanks for the patch!

We ran into this change as well, and after a lot of trying to figure out what was going on (especially with the unfortunately small amount of detail on the google group), it seems that an association is determined during "step 1" of going through OpenID (ie. building the request to send to google), and if one is found, its handle is included in the URL. Having this association handle in the URL is what causes Google to return invalid.

I'm not sure if @kendagriff's patch will cause problems for any other OpenID providers, and possibly will make the OpenID store completely useless? I'm not exactly sure how that part works.

Anyways, before I saw this issue, this was the monkey patch I wrote, which is limited to OpenID requests for google:

module OpenID
  class Consumer
    class AssociationManager
      def get_association_with_skip_for_google
        # Google no longer wants the assoc_handle parameter when authenticating by OpenID, and passing it in causes
        # it to always respond that the request is invalid. To get around this, if we're using google, just don't
        # collect/create an association, so no handle will be used in the request.
        # See: https://groups.google.com/forum/#!topic/google-federated-login-api/qXZDD7_K7jU
        return nil if @server_url == "https://www.google.com/accounts/o8/ud"

        get_association_without_skip_for_google
      end

      alias_method_chain :get_association, :skip_for_google
    end
  end
end

I'm not really sure how to determine in a universal way if the assoc_handle is desired or not.

@kendagriff's patch is working for us, in production. We currently use OpenID exclusively for Google sign on so I'm not sure whether the patch affects other OpenID providers, as @dvandersluis suggested. Hopefully enough folks can try this out over the next few days to get a solid patch integrated into this gem ASAP.

Thanks @dvandersluis and @kendagriff! I'm using @dvandersluis's monkey patch in production for now, at least until there is clarity about how other openid providers expect this to be handled. Thanks again!

Thanks @kendagriff, Working for us at http://www.piesync.com. Cost me quite some time to figure out this was the issue...

Thanks again! See what google comes up with now ...

@dvandersluis as best I can tell, yesterday's Google change is limited to OpenID 2 requests in stateless mode - that is, requests that by definition shouldn't be using a store. For stateful mode, the whole point is to avoid having the RP (you and me) make an extra server-side request to the OP (Google) to validate the assertion. That's probably the proper universal way.

We (Heroku) are using @kendagriff's patch in a couple places internally, though not in prod. We've so far limited our deployment to just apps that were set up in stateless mode initially. Our ruby-openid using apps that are doing stateful OpenID (as a dependency from OmniAuth or Warden) weren't impacted by Google's change.

Can the contributors chime in? IMO, Google is a sufficiently important OpenID provider – and given that they appear to implement the specification correctly – that ruby-openid should support OpenID2 requests out of the box – i.e. without a monkey patch.

Or, perhaps ruby-openid could support "compatibility" mode for providers who require the association?

@dennisreimann could you comment on this one?

Update on using the patch posted by @dvandersluis:

I realized today that I needed to add a check for https://www.google.com/accounts/o8/site-xrds to fix our Google Apps OpenID Login.

So:

module OpenID
  class Consumer
    class AssociationManager
      def get_association_with_skip_for_google
        # Google no longer wants the assoc_handle parameter when authenticating by OpenID, and passing it in causes
        # it to always respond that the request is invalid. To get around this, if we're using google, just don't
        # collect/create an association, so no handle will be used in the request.
        # See: https://groups.google.com/forum/#!topic/google-federated-login-api/qXZDD7_K7jU
        return nil if @server_url == "https://www.google.com/accounts/o8/ud"

        return nil if @server_url and @server_url.starts_with?("https://www.google.com/accounts/o8/site-xrds")

        get_association_without_skip_for_google
      end

      alias_method_chain :get_association, :skip_for_google
    end
  end
end
kcm commented

This bit us too. Can this get reviewed/merged ASAP? Though, this monkeypatch from cortfr worked for us instead of pegging the ruby-openid version.

I can confirm @cortfr's monkey patch also works for us, thanks!

So this patch has not been merged to the latest version of the gem?

I finally found the time to fix the tests and merge this and other pull requests.

Please try v2.3.0 which was just released and see if it works in production :)