Current Drupal OpenID implementation supports only SREG for requesting/receiving credential from OpenID provider. This patch adds AX support as well. In practice this means that now Drupal OpenID authentication requests email from Google, when it is used as OpenID provider.

CommentFileSizeAuthor
#16 634562-support-attribute-exchange-d6.28.patch14 KBtuffnatty
#15 634562-support-attribute-exchange-d6.patch22.13 KBvgarvardt
#9 634562-support-attribute-exchange.patch10.13 KBdamien tournoud
#8 634562-support-attribute-exchange.patch9.82 KBdamien tournoud
#4 openid-ax-support-d7-3.patch5.5 KBvgarvardt
#1 openid-ax-support-d7-2.patch6.15 KBvgarvardt
openid-ax-support-d7.patch6.16 KBvgarvardt

Comments

vgarvardt’s picture

vgarvardt commented
StatusFileSize
new openid-ax-support-d7-2.patch6.15 KB

Sorry, invalid patch from Mercurial diff. Here is updated patch

lilou’s picture

lilou commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch failed testing.

vgarvardt’s picture

vgarvardt commented
StatusFileSize
new openid-ax-support-d7-3.patch5.5 KB

Here is fixed version of the patch.
It passes all OpenID tests on my dev environment.

vgarvardt’s picture

vgarvardt commented
Status: Needs work » Needs review
moshe weitzman’s picture

moshe weitzman commented

subscribe. we need some justification about why this deserves a code freeze exception. otherwise, this has to live in contrib for d7.

vgarvardt’s picture

vgarvardt commented

OpenID is very useful technology and widely used. Drupal interface implementation is not the best in case of usability but it works. Usability can be improved with 3rd party modules (currently my module that should do this is in code review state), but there are some problems with OpenID backend implementation.

Currently there are 2 main OpenID extensions that implements attributes exchange between OpenID provider and consumer - SREG and AX. E.g. SREG is implemented in yandex.ru (nr 1 Russian search engine and nr 2 email hosting) and AX is implemented in google.com. This extensions allow consumer to request some info from provider (email, nickname, etc.).

OpenID implementation in Drupal supports only one extension - SREG. This means that it's impossible to request and fill in some info on registration in Drupal for providers, that does not implement SREG. Another thing about current Drupal implementation is that SREG request is always sent, does not matter if provider implements SREG or not.

My patch adds 2 things:
- support for AX OpenID extension
- verification for OpenID provider supported extensions (current Drupal implementation receives, but omits this) and data preparation for request

The only way I see this features can live as contributed module is to copy all OpenID module code, rename function names (to avoid conflicts with function names) and apply my patch. This is because OpenID module API is only 1 function - openid_begin(), all another functions are used internally. Also it provides 2 hooks - hook_openid to add request parameters, but does not provide info about provider supportes extensions, another one is hook_openid_response, that provides response from provider, but can not be used to override form values on registration form after response was received.

damien tournoud’s picture

damien tournoud commented
StatusFileSize
new 634562-support-attribute-exchange.patch9.82 KB

Improved patch, loosely based on vgarvardt work:

  • Ask the Endpoint for SREG (unconditionally, as the specification doesn't mandate SREG to be advertised in the service description), an AX (if available). Contributed modules can ask for more fields using hook_openid() if necessary.
  • Properly parse the response: there is no requirement that the endpoint uses the same prefixes for extensions as the initial request (for example, Google's Endpoint always replies with openid.ns.ext1 = http://openid.net/srv/ax/1.0, even if the original request was openid.ns.ax = http://openid.net/srv/ax/1.0)
  • Extended the tests to account for AX

This is absolutely necessary if we want to support Google Federated Login (which would be just awesome, given the huge installed user base), and is, all things considered, quite a minor change. See #727650: Support Google-specific OpenID discovery protocol.

damien tournoud’s picture

damien tournoud commented
StatusFileSize
new 634562-support-attribute-exchange.patch10.13 KB

Same patch, improved code comments.

dries’s picture

dries commented
Status: Needs review » Fixed

Committed to CVS HEAD.

c960657’s picture

c960657 commented

Cool feature!

Follow-up issue: #740036: OpenID AX: Request nickname + support alternative namespaces

Status: Fixed » Closed (fixed)

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

anarcat’s picture

anarcat commented
Version: 7.x-dev » 6.x-dev
Status: Closed (fixed) » Patch (to be ported)

Can we backport this patch 6.x, as recommended in #339600: support Google Federated Login OpenID?

vgarvardt’s picture

vgarvardt commented

I'll update my D6 patch to support latest Drupal 6 code. Will post updated patch as soon as I can.

vgarvardt’s picture

vgarvardt commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new 634562-support-attribute-exchange-d6.patch22.13 KB

Here is patch for D6 backported from #9 to 6.20.
I tested it with Google - works fine for me.

tuffnatty’s picture

tuffnatty commented
StatusFileSize
new 634562-support-attribute-exchange-d6.28.patch14 KB

Here is patch from #15 ported to D6.28.

Status: Needs review » Needs work

The last submitted patch, 634562-support-attribute-exchange-d6.28.patch, failed testing.

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.