Page MenuHomePhabricator

OAuth: Authorisation should not fail because you don't have an account on central wiki
Closed, ResolvedPublic

Description

Steps to reproduce:

  1. Create a totally new account on the English Wikipedia.
  2. Go to https://tools.wmflabs.org/oauth-hello-world/
  3. Click "Authorize this application"

Expected result: I am able to authorise the application
Actual result: Error "Unified login needed, E008"

The error is caused because the totally new user doesn't have a local account on mediawiki.org. One is created immediately afterwards, but it's that little bit too late. If you refresh the page showing you the error, it works just fine.


See also T94885

Details

Reference
bz72469

Related Objects

View Standalone Graph
This task is connected to more than 200 other tasks. Only direct parents and subtasks are shown here. Use View Standalone Graph to show more of the graph.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
  • Bug 72791 has been marked as a duplicate of this bug. ***

A user ran into this with Quarry at the Community Data Science Workshop I was at today, and it was really confusing trying to clear whatever caches were stopping the user from successfully logging in even after visiting another wiki.

Given that OAuth apps are increasingly often the entry point for new users, this bug really hamstrings OAuth.

greg added a subscriber: greg.Jan 16 2015, 11:59 PM

Note that if you create an account on a wiki and then try to log in with an OAuth app that connects to that same wiki, then even after refreshing it will still not work.

Eloquence added a comment.EditedMar 4 2015, 9:00 AM

If they go to mediawiki.org to authorize the app, then they will be autocreated and everything will work.

That is the theory. I tried reproducing the sequence described in the bug three times. Two out of three times, it worked fine. One out of three times, I got an error message when I tried to authorize (on mediawiki.org, as is the default for the Hello World app), and had to refresh. So there appears to be a race condition of some kind.

I want to make sure we're focusing on the right issue here. Having to send users to mediawiki.org is an inconvenience -- and one we should rectify in time. Having users sometimes get confusing error messages when they try to authorize an application is a very severe bug and one we should address much sooner.

Tgr claimed this task.Mar 30 2015, 7:51 PM
Tgr added a comment.Apr 2 2015, 12:25 AM

Seems fairly frequent, I could reproduce on first try.

The error is caused because the totally new user doesn't have a local account on mediawiki.org.

That wasn't the case for me:

Relevant logs:

tgr@fluorine:~$ ack-grep Testuser-T74469-2 /a/mw-log/centralauth*
/a/mw-log/centralauth-bug39996.log
40602:2015-04-01 21:58:40 mw1028 loginwiki: CentralAuthHooks::attemptAddUser: creating new user (Testuser-T74469-2) - from: /wiki/Special:CentralAutoLogin/refreshCookies?type=1x1&wikiid=enwiki&proto=https
40604:2015-04-01 21:58:56 mw1175 metawiki: CentralAuthHooks::attemptAddUser: creating new user (Testuser-T74469-2) - from: /w/index.php?title=Special:RecordImpression&country=HU&uselang=en&project=wikipedia&db=enwiki&anonymous=false&device=desktop&banner=WMHU_1percent_2015&campaign=WMHU_1percent_2015&bucket=1&bucketStart=1427716800&bucketEnd=1430870340&reason=viewLimit&banner_count=2&result=hide&sampleRate=1
40616:2015-04-01 21:59:40 mw1078 mediawikiwiki: CentralAuthHooks::attemptAddUser: creating new user (Testuser-T74469-2) - from: /wiki/Special:OAuth/authorize?oauth_token=12bbcbddc19e7f669f43ac643eb8adac&oauth_consumer_key=8f08a9cc98d3a2dd95c7708c38194a92
40617:2015-04-01 21:59:41 mw1255 specieswiki: CentralAuthHooks::attemptAddUser: creating new user (Testuser-T74469-2) - from: /wiki/Special:CentralAutoLogin/start?type=1x1&from=mediawikiwiki
40618:2015-04-01 21:59:41 mw1244 enwikiquote: CentralAuthHooks::attemptAddUser: creating new user (Testuser-T74469-2) - from: /wiki/Special:CentralAutoLogin/start?type=1x1&from=mediawikiwiki
40619:2015-04-01 21:59:41 mw1251 enwikisource: CentralAuthHooks::attemptAddUser: creating new user (Testuser-T74469-2) - from: /wiki/Special:CentralAutoLogin/start?type=1x1&from=mediawikiwiki
40620:2015-04-01 21:59:41 mw1086 enwiktionary: CentralAuthHooks::attemptAddUser: creating new user (Testuser-T74469-2) - from: /wiki/Special:CentralAutoLogin/start?type=1x1&from=mediawikiwiki
40621:2015-04-01 21:59:41 mw1179 enwikiversity: CentralAuthHooks::attemptAddUser: creating new user (Testuser-T74469-2) - from: /wiki/Special:CentralAutoLogin/start?type=1x1&from=mediawikiwiki
40622:2015-04-01 21:59:41 mw1187 enwikivoyage: CentralAuthHooks::attemptAddUser: creating new user (Testuser-T74469-2) - from: /wiki/Special:CentralAutoLogin/start?type=1x1&from=mediawikiwiki
40623:2015-04-01 21:59:41 mw1066 incubatorwiki: CentralAuthHooks::attemptAddUser: creating new user (Testuser-T74469-2) - from: /wiki/Special:CentralAutoLogin/start?type=1x1&from=mediawikiwiki
40624:2015-04-01 21:59:41 mw1238 enwikinews: CentralAuthHooks::attemptAddUser: creating new user (Testuser-T74469-2) - from: /wiki/Special:CentralAutoLogin/start?type=1x1&from=mediawikiwiki
40625:2015-04-01 21:59:41 mw1211 wikidatawiki: CentralAuthHooks::attemptAddUser: creating new user (Testuser-T74469-2) - from: /wiki/Special:CentralAutoLogin/start?type=1x1&from=mediawikiwiki
40626:2015-04-01 21:59:41 mw1065 commonswiki: CentralAuthHooks::attemptAddUser: creating new user (Testuser-T74469-2) - from: /wiki/Special:CentralAutoLogin/start?type=1x1&from=mediawikiwiki
40627:2015-04-01 21:59:41 mw1070 enwikibooks: CentralAuthHooks::attemptAddUser: creating new user (Testuser-T74469-2) - from: /wiki/Special:CentralAutoLogin/start?type=1x1&from=mediawikiwiki

/a/mw-log/centralauth.log
364404:2015-04-01 21:58:35 mw1083 enwiki: registered global account 'Testuser-T74469-2'
364405:2015-04-01 21:58:35 mw1083 enwiki: Attaching local user Testuser-T74469-2@enwiki by 'new'
364414:2015-04-01 21:58:37 mw1083 enwiki: Set global password for 'Testuser-T74469-2'
364424:2015-04-01 21:58:40 mw1028 loginwiki: Attaching local user Testuser-T74469-2@loginwiki by 'login'
364494:2015-04-01 21:58:56 mw1175 metawiki: Attaching local user Testuser-T74469-2@metawiki by 'login'
364682:2015-04-01 21:59:40 mw1078 mediawikiwiki: Attaching local user Testuser-T74469-2@mediawikiwiki by 'login'
364683:2015-04-01 21:59:40 mw1078 mediawikiwiki: CentralAuthHooks::onOAuthGetCentralIdFromLocalUser: user 'Testuser-T74469-2' cannot use OAuth on mediawikiwiki
364687:2015-04-01 21:59:41 mw1255 specieswiki: Attaching local user Testuser-T74469-2@specieswiki by 'login'
364688:2015-04-01 21:59:41 mw1244 enwikiquote: Attaching local user Testuser-T74469-2@enwikiquote by 'login'
364689:2015-04-01 21:59:41 mw1086 enwiktionary: Attaching local user Testuser-T74469-2@enwiktionary by 'login'
364690:2015-04-01 21:59:41 mw1251 enwikisource: Attaching local user Testuser-T74469-2@enwikisource by 'login'
364691:2015-04-01 21:59:41 mw1179 enwikiversity: Attaching local user Testuser-T74469-2@enwikiversity by 'login'
364692:2015-04-01 21:59:41 mw1187 enwikivoyage: Attaching local user Testuser-T74469-2@enwikivoyage by 'login'
364693:2015-04-01 21:59:41 mw1066 incubatorwiki: Attaching local user Testuser-T74469-2@incubatorwiki by 'login'
364694:2015-04-01 21:59:41 mw1238 enwikinews: Attaching local user Testuser-T74469-2@enwikinews by 'login'
364695:2015-04-01 21:59:41 mw1065 commonswiki: Attaching local user Testuser-T74469-2@commonswiki by 'login'
364696:2015-04-01 21:59:41 mw1211 wikidatawiki: Attaching local user Testuser-T74469-2@wikidatawiki by 'login'
364698:2015-04-01 21:59:41 mw1070 enwikibooks: Attaching local user Testuser-T74469-2@enwikibooks by 'login'
tgr@terbium:~$ echo "select * from localuser where lu_wiki = 'mediawikiwiki' and lu_name = 'Testuser-T74469-2';" | mwscript sql.php --wiki mediawikiwiki --slave any --wikidb centralauth
stdClass Object
(
    [lu_wiki] => mediawikiwiki
    [lu_name] => Testuser-T74469-2
    [lu_attached_timestamp] => 20150401215940
    [lu_attached_method] => login
)

The error message on the screenshot can be traced back to the execution path SpecialMWOAuth::execute( 'authorize' ) --> SpecialMWOAuth::handleAuthorizationForm() --> MWOAuthServer::getCurrentAuthorization() --> MWOAuthUtils::getCentralIdFromLocalUser() --> CentralAuthHooks::onOAuthGetCentralIdFromLocalUser(), which apparently returns false.

Attaching the new user seems to happen via CentralAuthHooks::onUserLoadFromSession() (whenever the user id of the lazy-loaded user object is first accessed) --> CentralAuthHooks::attemptAddUser() --> CentralAuthPlugin::initUser() --> CentralAuthUser::attach().

There seems to be a race condition between these two, but I do not see how - they both use the master DB and attach() does invalidate the cache.

@Tgr For the case where you're not authenticating on mediawiki.org, at least, it doesn't seem to be a race condition. If you create a new account on en.wiki and then try to log in to an OAuth app that is authenticates on en.wiki (for example, http://wizard.wikiedu.org), you will get that same error every time until you visit another wiki.

Tgr added a comment.EditedApr 2 2015, 1:16 PM

@Ragesoss: moved that issue into T94885.

Tgr added a comment.Apr 2 2015, 6:55 PM

The problem is MWOAuthUtils::getCentralIdFromLocalUser() which provides a hook (OAuthGetCentralIdFromLocalUser) for unified login extensions to look up the user id on the central wiki, and caches the results as a PHP variable. There is no way to invalidate that cache, which in this case stores the unattached state of the local account, just before the auto-attachment happens.

One simple way to fix this would be to not use the hook when we are on the central wiki, since the central id is the same as the local id in that case. That would have the side effect though that it would be possible to use OAuth with an unconnected mediawwiki.org account - I imagine that would be unwanted. (It won't make any difference for WMF sites after the SUL finalization - are there any other sites using CentralAuth?)

Anomie added a comment.Apr 2 2015, 7:05 PM
In T74469#1175065, @Tgr wrote:

One simple way to fix this would be to not use the hook when we are on the central wiki, since the central id is the same as the local id in that case.

Is that necessarily true, or could CentralAuth use the global user_id rather than the local id on any particular wiki if it wanted to?

Tgr added a comment.Apr 2 2015, 7:26 PM

Is that necessarily true, or could CentralAuth use the global user_id rather than the local id on any particular wiki if it wanted to?

The central id (which is mainly used for MWOAuthConsumerAcceptance::$userId) is documented as Publisher user ID (on central wiki), so it's true by definition right now. That said, as far as I can see, using the global id (or any unique, unchanging, never-reassigned identifier) would work as well.

Tgr added a comment.Apr 2 2015, 7:35 PM

How much of the current logic is going to be discarded after the SUL finalization? Will auto-attachments still happen the same way, on first visit to a new wiki? Right now, the only robust fix I can think of is moving the cache (which is currently attached to random user objects, which are not singletons, so hard to deal with) into a global local id -> central id map and making sure that the relevant CentralAuth operations invalidate or update it. It feels a bit like overengineering though.

Tgr updated the task description. (Show Details)Apr 2 2015, 7:36 PM
Tgr set Security to None.
In T74469#1175226, @Tgr wrote:

How much of the current logic is going to be discarded after the SUL finalization?

Currently there are no plans (AFAIK) to change how CentralAuth works in regards to attachments, but there's nothing stopping us from making those kinds of changes once SULF is complete.

Tgr added a comment.Apr 7 2015, 8:04 PM
In T74469#1175065, @Tgr wrote:

One simple way to fix this would be to not use the hook when we are on the central wiki, since the central id is the same as the local id in that case.

Is that necessarily true, or could CentralAuth use the global user_id rather than the local id on any particular wiki if it wanted to?

Eh, I misread the code. It does actually use the global id, despite what the documentation claims.

Tgr added a comment.Apr 7 2015, 8:07 PM

After SULF, do we need to support the old use cases forever since theoretically some third party could be using CentralAuth + OAuth with the old setup, or do we only require new code to work in a WMF-like setup? In the latter case, I would mark this task blocked by SULF, and get rid of the account attachment checks in onOAuthGetCentralIdFromLocalUser afterwards.

Tgr removed Tgr as the assignee of this task.Apr 8 2015, 2:24 PM
Tgr added a subscriber: Tgr.

Per above, will return to this after SUL finalization is over.

Trying to force an autocreation on the central wiki as part of the authorization process is probably possible.

Sounds like that would go against fixing T18864.

Tgr added a comment.Apr 13 2015, 7:45 PM

Sounds like that would go against fixing T18864.

Not really. An account is supposed to be autocreated when you visit the wiki, which is what you do when you do the OAuth authorization process. Anyway, autocreation happens already, it just does not get registered due to a stale in-process cache.

Change 204059 had a related patch set uploaded (by Gergő Tisza):
Autocreate some local accounts when global account is created

https://gerrit.wikimedia.org/r/204059

Tgr claimed this task.Apr 22 2015, 3:52 PM

@Legoktm @csteipp is this still relevant now? SULF is done

Still relevant. Even though accounts are unique, it doesn't mean all accounts actually exist on all wikis.

What's the blocker on this? Tgr's patch has been waiting for a while now.

Poke! I've returned to user testing with some new features of dashboard.wikiedu.org, and I'm getting the painful reminder of how big a blocker this is for new users being onboarded via an OAuth app.

Who do I need to send whiskey / money / flowers / pleas to, to get this back on the radar?

Both @Legoktm and I reviewed it on Monday. I'll give lego another day to comment again, otherwise I can merge it.

Change 204059 merged by jenkins-bot:
Autocreate some local accounts when global account is created

https://gerrit.wikimedia.org/r/204059

\o/

Thanks all!

Change 220970 had a related patch set uploaded (by Gergő Tisza):
Autocreate local versions of global accounts on meta, mw.o

https://gerrit.wikimedia.org/r/220970

The bug will still affect people who registered before this patch was deployed but never visited mw.org (or meta, once we switch it to be the central wiki). If that's a big enough problem, we can run the job manually for everyone who does not have an account there yet.

Adding notice tag; this will significantly increase the volume of account creations on meta (and mw.org but I doubt anyone cares about that).

this will significantly increase the volume of account creations on meta

Unlikely, given T18864.

Tgr added a comment.Jun 26 2015, 7:59 AM

THat bug is only triggered when you visit multiple wikis or stick around for 30+ days. Most people who register an account probably don't do that.

gpaumier moved this task from Backlog to Triaged on the Notice board.
gpaumier moved this task from To Triage to Announce in next Tech/News on the User-notice board.
gpaumier moved this task from Triaged to Archive on the Notice board.Jul 2 2015, 8:19 PM

Anything else blocking the deployment of the config change? The dashboard.wikiedu.org system hoping to launch of July 15 and inviting a bunch of new users to log in at the same time they are creating accounts.

Change 220970 merged by jenkins-bot:
Autocreate accounts on meta, mediawiki.org, loginwiki

https://gerrit.wikimedia.org/r/220970

Tgr added a comment.Jul 6 2015, 11:36 PM

Anything else blocking the deployment of the config change? The dashboard.wikiedu.org system hoping to launch of July 15 and inviting a bunch of new users to log in at the same time they are creating accounts.

No, I just didn't want to deploy this on Thursday since Friday was a holiday and a long weekend is not the best time to break account creation :)

In hindsight, there was no need to wait for group2 wikis so this could have been deployed on Wednesday... oh well.

Tgr added a comment.EditedJul 6 2015, 11:38 PM

Account autocreation is broken:

  • create account on enwiki
    • account created on mw.org, correctly attached per centralauth.localuser
  • log in on mw.org
    • error: wrong password
  • log out on enwiki
  • log in on mw.org
    • works

No change in the user record in the mediawikiwiki DB between the two attempts.

Change 223209 had a related patch set uploaded (by Gergő Tisza):
Invalidate cache after account autocreation from job

https://gerrit.wikimedia.org/r/223209

Change 223209 merged by jenkins-bot:
Invalidate cache after account autocreation from job

https://gerrit.wikimedia.org/r/223209

Change 223211 had a related patch set uploaded (by Gergő Tisza):
Invalidate cache after account autocreation from job

https://gerrit.wikimedia.org/r/223211

Change 223211 merged by jenkins-bot:
Invalidate cache after account autocreation from job

https://gerrit.wikimedia.org/r/223211

Tgr closed this task as Resolved.Jul 7 2015, 12:41 AM

Note that if the authorization happens on mw.org (as it does e.g. for https://tools.wmflabs.org/oauth-hello-world/) you need to log in again. If it happens on the same wiki where you registered (as for enwiki + http://wizard.wikiedu.org), you don't. That's slightly jarring, but that's how central auth works now (see T18864). I guess since we are autocreating accounts on mw.org/meta anyway, we could autologin there as well, it would make the OAuth workflow smoother.

Filed as T104932.

For my app that auths to en.wikipedia.org, I also had to log out and then log in again after creating a new account, before the OAuth login would work. Before that, it still gives the E008 error.

As a short term workaround, we should probably change the text for the E008 error to tell people to log out and then in again if they just created their account.

Tgr added a comment.Jul 7 2015, 11:37 PM

@Ragesoss I went through the reproduction steps of both this bug and T74328 and it worked correctly. Can you open a new bug with exact reproduction steps?

Here's the new bug with reproduction steps: T105105

Tgr reopened this task as Open.Sep 24 2015, 11:57 PM

Reopening, still need to deal with old accounts.

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 24 2015, 11:57 PM
Tgr closed this task as Resolved.Sep 25 2015, 12:03 AM

Nevermind, I created T94885 for that. Too many similar bugs...