Skip to content

Instantly share code, notes, and snippets.

Created May 24, 2011 22:18
Show Gist options
  • Save anonymous/989872 to your computer and use it in GitHub Desktop.
Save anonymous/989872 to your computer and use it in GitHub Desktop.
Ok!
If you (like me) are wondering what's up with there being 3 separate OpenID critical issues between D8 and D7 and what on earth it all means, and more importantly how to fix it, you're in luck! wojtha kindly spent about an hour with me in IRC today walking me through the state of things.
Here's the skinny:
There are two main issues where core's OpenID module is violating spec, and this leads to consequences ranging from certain OpenID providers just plain not working to possible impersonation risks:
[#575810]: When you enter an OpenID provider like "webchick.openid.com", Drupal tries to helpfully change it into a fully-qualified domain for you, like "http://webchick.openid.com". The problem is that providers in many cases specific what URLs like this should redirect to (often https://webchick.openid.com) and Drupal is <strong>completely ignoring</strong> this recommendation and instead doing its own thing. We need to fix that.
[#1076366]: Another issue is related to Claim IDs. When you attach a new OpenID provider to your account, you're given a fragment identifier, something like a form token, to tie it uniquely to your request. For example, webchick.openid.com/#FUSGDKS. Drupal is erroneously stripping this fragment identifier from the external auth tables, which could lead to impersonation attacks.
THEN you have the problem of existing D6 and D7 sites where these values have already been stored all screwed up, and they need to be fixed (D7 needs a workaround for both of these issues; D6 only the redirect one because [#728278] did not yet get committed to D6). The only reasonable place to cut in and do this is during login, because it requires manual intervention with a user's OpenID provider. But this involves sticking in some pretty nasty code in openid_authenticate() that needs to run on each login request, as well as an API change for D7.
So. That is where we're at. The order of operations in terms of reviewing/committing these patches is:
1. Redirects for D8
2. Fragments for D8
3. Redirects for D7
4. Fragments for D7
5. Workarounds for D7 (must be committed along #3 and #4, or else no one can log in)
6. Redirects for D6
7. Workaround for D6 (must be committed along with
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment