Page MenuHomePhabricator

Update TranslationNotifications to use AuthManager
Closed, ResolvedPublic

Description

See T110433 and T110291.

Details

Related Gerrit Patches:
mediawiki/extensions/TranslationNotifications : REL1_27Remove publishHere
mediawiki/extensions/TranslationNotifications : masterRemove publishHere
operations/mediawiki-config : masterUse bot password for TNBot
mediawiki/extensions/TranslationNotifications : masterUpdate login token fetching
mediawiki/extensions/TranslationNotifications : masterUpdate csrf token handling to use modern API

Related Objects

Event Timeline

Tgr created this task.Aug 29 2015, 12:32 AM
Tgr raised the priority of this task from to Needs Triage.
Tgr updated the task description. (Show Details)
Tgr added subscribers: Aklapper, Tgr.
Liuxinyu970226 set Security to None.Sep 6 2015, 4:02 AM
Anomie added a subscriber: Anomie.May 11 2016, 9:45 PM

The whole setup with respect to TranslationNotificationJob posting to other wikis is pretty ugly and I'd recommend replacing it with something else if you can:

  1. Ideally would be done with Echo cross-wiki notifications or something like that.
  2. If you want to keep the bot-like user talk page posts, I'd recommend using an OAuth owner-only consumer.
  3. If you don't want to use OAuth either, I'd recommend you make some sort of TranslationNotification-specific SessionProvider (that would have to be installed on every remote wiki) that uses the HTTP Authorization header in a manner similar to OAuth.
  4. If you insist on continuing to do it with requests to API action=login and action=edit, you'll need to set up BotPasswords logins because action=login isn't guaranteed to continue working for the "main" account password in an AuthManager world.

In any case, that should allow you to get rid of the $user->setPassword() call. Ideally things would even work if you use User::newSystemUser() to properly steal your notification account.

For prioritization @Amire80 is a better person to talk to. According to wikiapiary this extension hardly has any third party users, so making it use Echo instead seems best option.

I don't understand point (4): are you going to break normal login with password via API? If not, we have global account on WMF sites, so we are not creating any users and thus I do not see anything urgent here to fix that would break.

I don't understand point (4): are you going to break normal login with password via API?

Sort of. Logging in using action=login with the main account password will fail under AuthManager in various cases, such as two-factor auth being enabled for the account, or the account needing a password change, or a captcha being required (although that one isn't new). You can hope nothing like these starts applying to your extension's account, but we can't promise it won't.

Bots should use OAuth or BotPasswords (details in this mailing list post), while apps interacting with a human should use action=clientlogin instead which has the ability to instruct the app how to ask for the captcha or second factor or new password (details in this mailing list post). Extensions have a few additional options (detials on mediawiki.org now, but the relevant bits are about the same as I wrote in T110766#2287230).

Arrbee triaged this task as High priority.Jun 1 2016, 7:22 AM

Change 292120 had a related patch set uploaded (by Nikerabbit):
Remove publishHere

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

Change 292121 had a related patch set uploaded (by Nikerabbit):
Update csrf token handling to use modern API

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

Change 292122 had a related patch set uploaded (by Nikerabbit):
Update login token fetching

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

I did some cleanups on the code, removing parts that would be problematic with auth manager, review would be welcome.

I also created a bot password. I guess that the name change can be SWAT-deployed and the corresponding password updated at the same time.

Nikerabbit moved this task from Backlog to In Progress on the Language-Q4-2016-Sprint 4 board.

By the way, T48310: Deliver notification using Echo where available already exists for using Echo notifications, which I consider to be better long term solution.

Change 292898 had a related patch set uploaded (by Nikerabbit):
Use BotPassword for TNBot

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

Change 292120 merged by jenkins-bot:
Remove publishHere

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

Change 292121 merged by jenkins-bot:
Update csrf token handling to use modern API

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

Change 292122 merged by jenkins-bot:
Update login token fetching

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

Change 292898 merged by jenkins-bot:
Use bot password for TNBot

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

Tgr added a comment.Jun 7 2016, 11:17 PM

Is there anything left to do?

Nikerabbit added a subscriber: Nemo_bis.

@Nemo_bis found a bug about incorrect username when posting to local wiki, which should be fixed by my patch to use same method for local and other wikis. This is to be confirmed after meta is on MW-1.28-release (WMF-deploy-2016-06-07_(1.28.0-wmf.5))

Nikerabbit moved this task from QA to Done on the Language-Q4-2016-Sprint 4 board.Jun 9 2016, 6:45 PM

I confirmed that the mentioned bug was fixed on meta.

Anomie closed this task as Resolved.Jun 13 2016, 3:55 PM

Let's call this "resolved" then, since T48310 exists for the longer-term solution.

Change 296509 had a related patch set uploaded (by Gergő Tisza):
Remove publishHere

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

Change 296509 merged by jenkins-bot:
Remove publishHere

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