mmodell wrote on 2014-06-02 15:59:47 (UTC)

I guess I've taken this over from @csteipp and it's nearly finished.

qgil wrote on 2014-06-02 21:47:15 (UTC)

Wikimedia SUL is a requirement for the launch of the Wikimedia Trusted User Tool, scheduled "as soon as possible". This is a small project compared to Day 1. It will be useful to be assured that Wikimedia SUL works like a charm for Day 1.

qgil wrote on 2014-06-02 23:19:03 (UTC)

Just an idea to make sure we have rock solid Wikimedia SUL implemented in our Phabricator: deploy Phabricator in production only with Wikimedia SUL and the People app enabled, allowing users to log in and fill their profiles. Before enabling Legalpad and anything else. Make a call for testers in wikitech-l and the Engineering mailing lists. See what happens.

If everything goes well after a couple of weeks, champagne! If we run into any problems, let's fix them accordingly keeping the champagne in the fridge.

In the worst case scenario we can throw the Phabricator instance to the river and start from scratch again, without compromising e.g. real community contracts signed by users with the Trusted User Tool.

mmodell wrote on 2014-06-02 23:23:25 (UTC)

This sounds good to me. Give me one more day to have the auth rock solid. And I'm still waiting to hear back from Chase re:production hardware.

Waldir wrote on 2014-06-10 16:05:17 (UTC)

There should be an option to merge a WMF SUL account to an existing account when registering via OAuth. At least a warning, during OAuth registration, of the correct procedure (e.g. cancel this registration, and instead login manually to the existing Phabricator account, then go to Settings > add OAuth account -- I'm just guessing, I don't know if that's how it's supposed to work)

Alternatively, since according to T40, local accounts won't be allowed anymore, we can simply ensure that all current local accounts are merged to their SUL equivalent, in a one-off process. Still, if we decide to allow GitHub OAuth (T337), the merge process should be available for those having both a SUL and a GitHub account, to prevent duplicate accounts being created (that, or having an easy way for users to merge their accounts after the fact).

csteipp wrote on 2014-06-10 17:35:56 (UTC)

In T314#25, @Waldir wrote:

There should be an option to merge a WMF SUL account to an existing account when registering via OAuth. At least a warning, during OAuth registration, of the correct procedure (e.g. cancel this registration, and instead login manually to the existing Phabricator account, then go to Settings > add OAuth account -- I'm just guessing, I don't know if that's how it's supposed to work)

That would be a nice feature.

@mmodell, do you think we could add a warning message on the registration completion page that reminds users they are registering a new account, and if they just want to link an existing account, <give some directions>?

Alternatively, since according to T40, local accounts won't be allowed anymore,

Sortof. Phabricator still has Phabricator accounts with their own usernames. The setting that we will most likely configure is to disallow password authentication against the Phabricator database. But regardless of whether you use MediaWiki OAuth, github's, or LDAP, those are just external authentication mechanisms for your local Phabricator account (which has a single username, that may or may not look anything like the account name you use to login to mediawiki/github/ldap).

I originally was under the impression the integration was tighter, but that's how Phabricator does things.

csteipp wrote on 2014-06-10 17:50:31 (UTC)

In T314#25, @Waldir wrote:

There should be an option to merge a WMF SUL account to an existing account

And forgot to mention, I don't think this is possible. Phabricator doesn't seem to have the concept (unless they have an addon app to do it?) to merge multiple accounts. Instead you would need to unlink the local account from the external authentication account, then re-link the correct account. This is complicated since you can't unlink an external authentication that is the only authentication mechanism for an account. And (hmm.. phabricator bug?) you can't even add a password to an account that used OAuth to register, because it can't validate the "Old Password".

So we'll need pretty good documentation about how to do the account generation and conversion from gerrit. We may need to have an extra admin tool to remove an external MediaWiki account so users who get in the situation where their account is linked with the wrong phabricator account, we can at least get them working in the correct account (even if they can never again login to the other account).

csteipp wrote on 2014-06-11 17:11:06 (UTC)

@mmodell, I was talking with @Qgil this mornign, and wondering-- would it be possible on the account registration screen (after a user registers with MediaWiki OAuth), can we prevent the user from changing the suggested username? Usernames in SUL are unique (and antispoof'ed), so we won't have clashes.

The thinking is we then make an provider other than MediaWiki OAuth a login only (can't register with it), so all usernames in Phabricator would match the user's mediawiki.org account.

mmodell wrote on 2014-06-11 18:19:27 (UTC)

@csteipp: that sounds like a very reasonable way to do it, and yes that should work just fine. I can prevent them from changing usernames, surely without much effort.

Rush wrote on 2014-06-13 15:59:04 (UTC)

just a note from the call yesterday. The next steps for the SUL authentication provider are to go through proper review in Gerrit and get included in the master branch of the relevant local phabricator/ repositories. This will enable us to test further in production. I think @mmodell was going to translate some upstream phabricator differential into a gerrit changeset?

thanks guys!

Rush wrote on 2014-06-13 16:47:11 (UTC)

The New-New update to this exciting situation :)

It seems like:

a. this legalpad thing is an organizational necessity (legal needs it for serious business reasons)
b. we want to remove as many unnecessary pre-reqs from it as possible
c. the race conditions surrounding legal doing their thing at the same time as testing ticket migration are many

adjusted course of action:

I talked to Mark Bergsma and the idea is to better service legal's needs we stand up a legal only phabricator server with only legalpad that can have SUL and doesn't need to worry about rt data or anything. This data then is folded into the main phab once we have everything in order.

This seems to resolve the dependency dilemma's surrounding, get legal going asap, and doesn't jeopardize sensitive data. The downside is I'm not sure about porting accounts registered with SUL from this instance, but it's a small thing to re-register if necessary I think.

qgil wrote on 2014-06-13 17:06:51 (UTC)

In T314#32, @Rush wrote:

The New-New update to this exciting situation :)
(...)
stand up a legal only phabricator server with only legalpad that can have SUL

Works for me, and I would sign this in a Legalpad as well. :)

Thank you!

mmodell wrote on 2014-06-13 18:27:20 (UTC)

changes in gerrit:

https://gerrit.wikimedia.org/r/#/c/139438/
https://gerrit.wikimedia.org/r/#/c/139442/

aklapper wrote on 2014-06-13 22:00:52 (UTC)

In T314#32, @Rush wrote:

The New-New update to this exciting situation :)

Sounds good. I'll update https://www.mediawiki.org/wiki/Phabricator/Plan#Migration_plan on Monday if nobody is faster (nearly midnight here).

Rush wrote on 2014-06-19 18:00:38 (UTC)

The changesets are still languishing in gerrit. Any chance someone could move the forward?

csteipp wrote on 2014-06-19 18:11:19 (UTC)

In T314#37, @Rush wrote:

The changesets are still languishing in gerrit. Any chance someone could move the forward?

@mmodell, let me know if you need anything from me. I *think* you already have code for at least one of my comments, so I was going to let you make changes, and I'm happy to +2 when they're close enough. But let me know if you need me to work on any parts.

mmodell wrote on 2014-06-19 18:59:34 (UTC)

I still don't find this to be reliable and I don't quite understand why. Almost every time I try to use it from my iphone the authentication handshake fails, something is always lost between the point where I am handed to the mediawiki part of the process and I either fail to get redirected back to phabricator or I get redirected back to a page which indicates no auth session was started.

I'm not sure if it's something with my phone but it's definitely flaky on the phone and seems more reliable on a desktop browser. It doesn't feel like first class user experience though, that's for sure.

greg wrote on 2014-06-20 20:26:59 (UTC)

@mmodell: is the code that is running on this instance the latest you're testing/playing with? It works fine for me on desktop, which I think we can be ok with for now until we get more testing/users.

If you're stuck with the mobile bit (I haven't tested it myself), let's rope in Chris McMahon to help out.

csteipp wrote on 2014-06-20 20:40:37 (UTC)

In T314#39, @mmodell wrote:

I still don't find this to be reliable and I don't quite understand why. Almost every time I try to use it from my iphone the authentication handshake fails, something is always lost between the point where I am handed to the mediawiki part of the process and I either fail to get redirected back to phabricator or I get redirected back to a page which indicates no auth session was started.

Doh! I just realized what problem you're hitting. The issue is that for mobile to work, the urls need to be in clean format, instead of index.php?title=Special:OAuth/authorize... I made a fix for it in my php oauth library last weekend (https://bugzilla.wikimedia.org/show_bug.cgi?id=60034). So the answer there is that we need to support clean urls by adding in a faux title = 'Special:OAuth/<action>' pair before we sign the OAuth request.

@mmodell, if are you working off the version in gerrit? If I can work on the same code, I can make the OAuth updates so mobile works correctly.

mmodell wrote on 2014-06-25 00:18:22 (UTC)

@csteipp why would an iphone care about the url having a query string like that? seems like it shouldn't matter.

mmodell wrote on 2014-06-25 00:23:05 (UTC)

@csteipp I just read the bug you linked... I think I understand now. sorry for not reading before I asked.

csteipp wrote on 2014-06-25 23:26:48 (UTC)

In T314#43, @mmodell wrote:

@csteipp I just read the bug you linked... I think I understand now. sorry for not reading before I asked.

Yeah, it's kinda silly, but that's how they did it.

Let me know if you still have problems with it!

mmodell wrote on 2014-06-26 21:48:00 (UTC)

This is done, just waiting on code review:

https://gerrit.wikimedia.org/r/#/c/139442/
https://gerrit.wikimedia.org/r/#/c/139438/

mmodell wrote on 2014-06-26 21:54:03 (UTC)

@csteipp, @Aklapper:
review pls?

mmodell wrote on 2014-06-27 21:58:52 (UTC)

The differential revisions are still pending review upstream, however, this is merged locally in gerrit so I'm marking it complete.

importing issue status