Page MenuHomePhabricator

Support Attribute Exchange
Closed, ResolvedPublic

Assigned To
None
Authored By
bzimport
Dec 10 2009, 9:51 AM
Referenced Files
F6121: check-email.patch
Nov 21 2014, 10:50 PM
F6120: 21810-patch-against-r62177.patch
Nov 21 2014, 10:50 PM
F6119: ax-enhancements.patch
Nov 21 2014, 10:50 PM
Subscribers
None

Description

Author: craig.box

Description:
The OpenID extension currently requests e-mail address by simple registration (SREG). It should also attempt to request it by attribute exchange (AX), as this is supported by more providers (most importantly, Google.


Version: unspecified
Severity: enhancement

Details

Reference
bz21810

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 10:50 PM
bzimport set Reference to bz21810.

craig.box wrote:

Attribute Exchange support & autogeneration of username by e-mail address (optional)

Patch for the 1.15.1 line.

It also includes a new config option, $wgOpenIDUseEmailAsNickname, which will use the part before the @ as your e-mail address. You should only use this if you are using a single OpenID provider, like Google Apps, where everyone is guaranteed to not have a conflict.

attachment ax.patch ignored as obsolete

sergey.chernyshev wrote:

Looks great! Can you provide a patch for the latest trunk version?
http://svn.wikimedia.org/svnroot/mediawiki/trunk/extensions/OpenID/

Comment on attachment 6866
Attribute Exchange support & autogeneration of username by e-mail address (optional)

[tweaking attachment for inline view]

craig.box wrote:

Version against trunk with further configuration enhancements

OK, this one is a bit of a monster (sorry) but adds the following:

  • Attribute exchange to get full name and e-mail address (note: php-openid 2.0 in Debian Etch has a bug in its AX libraries; the version in Sid seems to be OK)
  • (configurably) Automatically using e-mail address user@ part as nickname
  • (configurably) Hard-coding a single provider

Attached:

sergey.chernyshev wrote:

Hmm. Applying this patch disabled OpenID menu item for mediawikiwidgets.org

Can you check if there is some version dependency in your code?
http://www.mediawikiwidgets.org/Special:Version

craig.box wrote:

I don't see the problem and didn't introduce anything serious in the code (nothing relating to that area either). I was only testing on 1.15 though.

craig.box wrote:

New patch against latest trunk

Here's a new patch against the current trunk (r62177). It works well for me, and other patches I build will rely on it.

Now preferences have been backported, I can't see the error you referred to with the OpenID menu. Can you test again, Sergey?

Attached:

sergey.chernyshev wrote:

Yeah, I don't see any problems - it's possible that it I just missed some permission problems after patching last time.

What's the easiest way to test this new patch?

craig.box wrote:

Log into Google and see if you are prompted for e-mail address and real name, and check that they are set on your profile after you log in.

sergey.chernyshev wrote:

How do I know if it uses AX instead of SREG?

craig.box wrote:

Google doesn't do SREG.

sergey.chernyshev wrote:

I patched my test wiki and it doesn't seem to be grabbing AX data even though it is indeed requested and returned from Google.

http://wiki.sergeychernyshev.com/Main_Page

Logs report that data was properly returned from Google, but there are no errors in there and resulting form doesn't get populated with any data from AX.

Can you take a look and confirm that it doesn't work as intended?

craig.box wrote:

I logged in with my Gmail account, never having had an account on the wiki in the past, and according to http://wiki.sergeychernyshev.com/test/index.php/Special:Preferences, both my real name and my e-mail address are filled in, and a test e-mail to that address was generated.

sergey.chernyshev wrote:

Was the username suggested to you or you picked one yourself?

craig.box wrote:

I opted to go for my suggested name ("Craig Box"), but can test again with a generated name if you want?

sergey.chernyshev wrote:

That's the problem - it didn't suggest a name for me when I registered with my Google account.

craig.box wrote:

Previously, the name would have been taken from the 'fullname' attribute in SREG.

Now, AX is checked also:

if ( array_key_exists( 'fullname', $sreg ) )
        $fullname = $sreg['fullname'];
 
if ( array_key_exists( 'http://axschema.org/namePerson/first', $ax ) || array_key_exists( 'http://axschema.org/namePerson/last', $ax ) )
        $fullname = $ax['http://axschema.org/namePerson/first'][0] . " " . $ax['http://axschema.org/namePerson/last'][0];

if ( $fullname && $this->userNameOK( $fullname ) ) {
        // offer the name

You therefore have to have either piece of the full name included, and the resulting concatenated string needs to be a valid username, for it to be offered to you.

If you don't have a valid real name set on your Google account, then you will not be offered the choice to use it.

Does this help?

craig.box wrote:

(Ahh, assuming your real name on Google is "Sergey Chernyshev", that user already exists on your wiki.)

sergey.chernyshev wrote:

Yeah, that might be a reason for this. Why didn't it suggest "Sergey Chernyshev2" or tell me, at least, that it already exists?

In any case, that's a separate question - this patch seems to be working then. I need to figure out how to test it in full though.

craig.box wrote:

Because it didn't do that before with only SREG. :) If you choose your own name, and it already exists, you are silently returned to this page also.

For testing - I've been running with AX in production for some time now, and I'm pretty confident I've ironed out all the results. It works great when given AX from someone like Google - I haven't tested with anyone who does SREG /and/ AX, but seeing as AX only does real name and e-mail address, I'm confident those cases are well covered.

sergey.chernyshev wrote:

OK, the only thing that I'm usually worrying about is contributors having special case and not catching other possible issue, that's the only reason I'm testing the code.

I'll go ahead and commit it. Thanks for contribution, BTW!

sergey.chernyshev wrote:

Added in r62390

craig.box wrote:

Fix AX patch (against r62444)

There was an extraneous } in the checked in patch, and also a bug where it didn't check that the username it was going to automatically create an account for you, existed already. Both are fixed in this patch.

Attached:

sergey.chernyshev wrote:

Patch applied, please update and test.