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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, openid-ax-1.patch, failed testing.

aspilicious’s picture

Status: Needs work » Needs review

openid-ax-1.patch queued for re-testing.

Dries’s picture

This looks reasonable to me.

c960657’s picture

FileSize
10.58 KB

Reroll.

Damien Tournoud’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
8.23 KB

That 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.

Dries’s picture

Dries’s picture

#5: 740036-openid-ax-followup.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 740036-openid-ax-followup.patch, failed testing.

Damien Tournoud’s picture

Status: Needs work » Needs review
FileSize
8.48 KB

Free reroll.

Status: Needs review » Needs work

The last submitted patch, 740036-openid-ax-followup.patch, failed testing.

c960657’s picture

Status: Needs work » Needs review
FileSize
12.97 KB

submitLoginForm() was used in both of the new test classes, so I added a common base class.

Status: Needs review » Needs work

The last submitted patch, 740036-openid-ax-followup-2.patch, failed testing.

c960657’s picture

Status: Needs work » Needs review

#11: 740036-openid-ax-followup-2.patch queued for re-testing.

bcn’s picture

Status: Needs review » Reviewed & tested by the community

Back to RTBC- All tests are passing.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD.

c960657’s picture

Category: feature » bug
Status: Fixed » Needs review
FileSize
5.79 KB

Yahoo 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().

Damien Tournoud’s picture

I 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:

The fetch response message supplies the information requested in the fetch request. Each attribute is supplied with the assigned alias prefixed by "openid.ax.value." as the lvalue and the attribute value as the rvalue. Attribute types are also returned in the "openid.ax.type." parameters. The supported length for attribute aliases MUST be at least 32 characters.

Granted, this is slightly ambiguous, but saying that the behavior of Yahoo is valid is a very long shot.

c960657’s picture

I 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.”

Damien Tournoud’s picture

Status: Needs review » Reviewed & tested by the community

Anyway. Let's not forget this patch, which still applies with offset.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.