In #634562: OpenID AX support Drupal got support for account registration based on profile fields fetched via OpenID Attribute Exchange (AX) in addition to the existing support for OpenID Simple Registration (SREG). This is a follow-up issue.
The AX request does not ask for the suggested username from the OpenID Provider, like the SREG request does. Instead it used the string before @ in the email address. If no nickname is supplied, the user should be prompted to enter one, just like with SREG.
(In #216101: OpenID fails to auto-register account: Username invalid, email required it was discussed whether we should autogenerate a username if one was not provided by SREG. In #637850: Show confirmation page during OpenID auto-registration is is suggested that the user is always given a change to enter the values provided by the OpenID Provider, before the account is created).
Also, the OpenID AX spec mentions that value types are identified using URIs but it does not specify an authoritative list of common types, though it mentions http://www.axschema.org URIs as an example. Apparently implementors disagree on which URIs to use *sigh*. This post claims that the http://schema.openid.net URIs are out-dated and that http://www.axschema.org URIs should be used. However, at least myopenid.com only supports the former, while Yahoo only supports the latter.
With this patch Drupal now asks the OpenID Provider for a suggested nickname. The request for email and nickname uses both types of URIs. Also, I split the OpenID test class into two so the auto-registration stuff can be tested a bit faster.
Comment | File | Size | Author |
---|---|---|---|
#16 | 740036-openid-ax-2nd-followup-1.patch | 5.79 KB | c960657 |
#11 | 740036-openid-ax-followup-2.patch | 12.97 KB | c960657 |
#9 | 740036-openid-ax-followup.patch | 8.48 KB | Damien Tournoud |
#5 | 740036-openid-ax-followup.patch | 8.23 KB | Damien Tournoud |
#4 | openid-ax-2.patch | 10.58 KB | c960657 |
Comments
Comment #2
aspilicious CreditAttribution: aspilicious commentedopenid-ax-1.patch queued for re-testing.
Comment #3
Dries CreditAttribution: Dries commentedThis looks reasonable to me.
Comment #4
c960657 CreditAttribution: c960657 commentedReroll.
Comment #5
Damien Tournoud CreditAttribution: Damien Tournoud commentedThat looks perfectly ok, but I had to open two specs to understand the patch :)
Rerolled with a bunch of additional comments. Absolutely no code change, so RTBCing at the same time.
Comment #6
Dries CreditAttribution: Dries commentedAsking for a retest because #395340: Email verification not enforced with OpenID auto-registration was committed.
Comment #7
Dries CreditAttribution: Dries commented#5: 740036-openid-ax-followup.patch queued for re-testing.
Comment #9
Damien Tournoud CreditAttribution: Damien Tournoud commentedFree reroll.
Comment #11
c960657 CreditAttribution: c960657 commentedsubmitLoginForm() was used in both of the new test classes, so I added a common base class.
Comment #13
c960657 CreditAttribution: c960657 commented#11: 740036-openid-ax-followup-2.patch queued for re-testing.
Comment #14
bcn CreditAttribution: bcn commentedBack to RTBC- All tests are passing.
Comment #15
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD.
Comment #16
c960657 CreditAttribution: c960657 commentedYahoo recently changed the behavior of their OpenID service, and that revealed another problem:
The OpenID Provider does not necessarily use the same aliases in the response as Drupal used in the request. E.g. Google uses our returns the email address in openid.ext1.value.mail_ao, and so did Yahoo until recently (I think), but now Yahoo uses the openid.ax.value.email key. This is perfectly legal but is not supported by openid_extract_ax_values().
Comment #17
Damien Tournoud CreditAttribution: Damien Tournoud commentedI actually studied this while doing the first AX patch, and came to the conclusion that the alias in the response *must be* the same as the alias in the request.
From the definition of the fetch response:
Granted, this is slightly ambiguous, but saying that the behavior of Yahoo is valid is a very long shot.
Comment #18
c960657 CreditAttribution: c960657 commentedI agree that the spec isn't very clear here, but I believe the intention is to allow the provider to choose its own aliases.
For instance, the description of openid.ax.type.<alias> in the response mentions that “aliases MUST NOT contain newline and colon characters” — this description seems odd if the provider is expected to just use whatever alias it has received from the client.
Also, it seems consistent with the way the provider is free to choose its own extension aliases (i.e. other than “ax”) in the response, i.e. the response may contain a openid.<extension_alias>.type.<alias> lvalue with another extension_alias than was used in the request. This is explicitly allowed by OpenID Authentication 2.0, section 12: “If a message is a response to another message, the response MAY use a different alias to refer to the same namespace.”
Comment #19
Damien Tournoud CreditAttribution: Damien Tournoud commentedAnyway. Let's not forget this patch, which still applies with offset.
Comment #20
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.