Page MenuHomePhabricator

Configurable MediaWiki oauth1 provider
AbandonedPublic

Authored by 20after4 on May 19 2014, 7:15 PM.

Details

Summary

The second part of T5096, see also D9202 for the libphutil oauth adaptor

Test Plan

Enabled mediawiki provider and associated it with my account, logged in via mediawiki credentials

Event Timeline

20after4 retitled this revision from to initial proof of concept implementation of mediawiki oauth provider.
20after4 updated this object.
20after4 edited the test plan for this revision. (Show Details)
20after4 added reviewers: qgil, epriestley.

Is this stuff bound to WMF, or can I theoretically auth against any local install of Mediawiki? I've been assuming the latter, but maybe that's not true?

In D9203#6, @epriestley wrote:

Is this stuff bound to WMF, or can I theoretically auth against any local install of Mediawiki? I've been assuming the latter, but maybe that's not true?

I think it should be able to auth against any mediawiki, with a bit of generalization. I wish I could pin down the original author of this bit of code but he doesn't have an account on secure.phabricator.com, Hopefully he will rectify that problem and join in the discussion here.

@epriestley so apparently there are plans to build a proper oauth feature into mediawiki's core. Currently the auth stuff is just a bit hacky and provided via a plugin. Once we clean out the hacks and make it a core feature, then it would make sense to have a phabricator adaptor that anyone could use with their own instance of mediawiki.

This rudimentary version might not be terribly suitable for upstreaming right now and I may have jumped the gun a bit in submitting this.

In D9203#7, @20after4 wrote:
In D9203#6, @epriestley wrote:

Is this stuff bound to WMF, or can I theoretically auth against any local install of Mediawiki? I've been assuming the latter, but maybe that's not true?

I think it should be able to auth against any mediawiki, with a bit of generalization. I wish I could pin down the original author of this bit of code but he doesn't have an account on secure.phabricator.com, Hopefully he will rectify that problem and join in the discussion here.

I wasn't sure what would be most useful, so I did the easiest thing as a PoC. Having it go against any mediawiki install would probably have more potential users.

Unless it is a load of extra work, it would be useful to make the plugin available for any MediaWiki with https://www.mediawiki.org/wiki/Extension:OAuth installed. This functionality is not in MediaWiki Core, but the extension is deployed in all Wikimedia servers, so I guess most needs of your average MediaWiki instance would be covered as well.

qgil removed a reviewer: qgil.

@qgil: yeah I'm going to make the oauth url configurable so that this will work with any mediawiki.

20after4 edited edge metadata.

Now configurable with custom mediawiki url. Borrowed some code from the jira provider to make it work.

20after4 edited edge metadata.

Fixed some lint errors (long lines)

20after4 retitled this revision from initial proof of concept implementation of mediawiki oauth provider to Configurable MediaWiki oauth1 provider.Jun 26 2014, 6:16 PM
20after4 edited the test plan for this revision. (Show Details)
20after4 added a project: Wikimedia.

Updated with @csteipp's changes

This has passed wikimedia code review
at https://gerrit.wikimedia.org/r/#/c/139442/

Couple of inlines, cookie thing is only real issue I want to sort out.

src/applications/auth/provider/PhabricatorAuthProvider.php
306 ↗(On Diff #23435)

This should be changed only in the subclass, not here.

src/applications/auth/provider/PhabricatorAuthProviderOAuth1MediaWiki.php
54–55 ↗(On Diff #23435)

This URI should be generic.

85–86 ↗(On Diff #23435)

I want to genericize the cookie stuff so we don't need to duplicate this function. It's pretty much a direct copy/paste of the method in the parent class.

258–268 ↗(On Diff #23435)

If you don't plan to need this, just omit it. Some of the providers have a method like this because we use "is Phabricator connected to this thing?" to adjust other behavior (like turning on the "JIRA Issues" field in Differential).

Assuming I'm roughly on target with D9760, you should be able to remove the entire processLoginRequest() implementation after it lands. The rest of this looks reasonable -- I'll set up a local MW install and run through it to see if I catch anything.

Make the CSRF check optional. See related changes in D9202

Assuming I'm roughly on target with D9760, you should be able to remove the entire processLoginRequest() implementation after it lands. The rest of this looks reasonable -- I'll set up a local MW install and run through it to see if I catch anything.

If you're setting up a mediawiki, https://github.com/wikimedia/mediawiki-vagrant would probably save you some time.

That repo includes our customized vagrant setup which allows you to do something like vagrant enable-role oauth to turn on the oauth plugin which isn't included or enabled by default normally.

epriestley edited edge metadata.

There's feedback on D9202 which affects this (mostly: real two-phase setup process), aggregated there for clarity or confusion.

This revision now requires changes to proceed.Aug 25 2014, 4:11 PM
20after4 edited edge metadata.

Addresses @epriestley's concerns from D9202.

Note: This also depends on the changes from D9202

This is now published as part of our wikimedia/phabricator-extensions repository. There isn't much demand (or need) for it to be in the upstream.