Page MenuHomePhabricator

Create Phabricator backend for python-social-auth
Closed, ResolvedPublic

Description

There is a MediaWiki backend for python-social-auth (github), however it would also be useful to allow authentication using Phabricator, as the later contains a lot of extra information about our people accessible via a proper API (something MediaWiki does not do) , and allow less clunky setup for tools using https://pypi.python.org/pypi/phabricator .

The initial assessment of this task will consist of a fork of python-social-auth/social-core with an extra backend for Phabricator and tests.

(Submitting a pull request on GitHub can be done after the task is accepted)

See php version: https://github.com/ofbeaton/oauth2-phabricator

Mentors: @jayvdb (gitter), @01tonythomas

Event Timeline

jayvdb created this task.Oct 27 2017, 6:54 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 27 2017, 6:54 PM
divadsn claimed this task.Dec 26 2017, 9:06 AM
divadsn added a subscriber: divadsn.

I will do it :)

jayvdb updated the task description. (Show Details)Dec 26 2017, 9:33 AM
jayvdb added a comment.EditedDec 26 2017, 12:10 PM

First challenge is getting a test instance of Phabricator up.
https://secure.phabricator.com/ says NOTE: You can launch a test installation of Phabricator on Phacility if you want to poke around and try things out.

divadsn added a comment.EditedDec 26 2017, 12:28 PM

Oh, thanks for the tip, I was going to use Docker to create a local Phabricator instance :)

Edit: Seems like there are limitations when it comes to creating new OAuth app.

Alright, I finally could get my local Docker instance running and added a testapp in OAuth Server which will then return the data I want to my dummy script.

jayvdb updated the task description. (Show Details)Dec 26 2017, 1:46 PM
jayvdb updated the task description. (Show Details)
jayvdb added a subscriber: 01tonythomas.

Looking good so far. The most recent PRs included a unit test module.

https://github.com/python-social-auth/social-core/pull/162/files

But some other recent backends havent. e.g.
https://github.com/python-social-auth/social-core/commit/9f6955d2d7b7211963d0ec0beb5c5f4c68a30923
https://github.com/python-social-auth/social-core/commit/7043ad009807ebbccd36e66413f28c51b0de6988

Could you set up a test module for Phabricator? They have a mock'ing system that can be used.

Sure! Will do that in about 1 hour :)

jayvdb added a comment.EditedDec 27 2017, 4:06 AM

Repeating some of what I put into the code review of https://github.com/python-social-auth/social-core/pull/169

I suspect that the default OAuth scope is actually scope.always. Check out https://secure.phabricator.com/D15621 , which removed previous scopes offline_access and whoami as no longer needed, but added a new one.

Phabricator has no published defined scopes, as noted at https://github.com/ofbeaton/oauth2-phabricator#managing-scopes , and "This section has not been written yet." on https://secure.phabricator.com/book/phabcontrib/article/using_oauthserver/ .

If true, this deserves special mention as Phabricator is very odd in that regard; not many other OAuth providers have no scopes at all.

If we do figure out the scopes, maybe there is a documentation task to be done ... :P

But as I mentioned above, it looks like the scope scope.always exists, and I wouldnt be surprised if the myriad other scopes in phabricator also work via OAuth.

A bit more investigation needed (however the GCI task criteria have been met, and so it is approved).

divadsn added a comment.EditedDec 28 2017, 1:53 AM

But as I mentioned above, it looks like the scope scope.always exists, and I wouldnt be surprised if the myriad other scopes in phabricator also work via OAuth.
A bit more investigation needed (however the GCI task criteria have been met, and so it is approved).

I could do that investigation and propose some changes to the docs, if no one gets the task before me of course :P

@divadsn , you have three commits on https://github.com/python-social-auth/social-core/pull/169/commits ; that is unnecessary , and in gerrit world it would be considered messy work. Please squash them.

@jayvdb but isn't it how it's supposed to be done on GitHub where you squash those commits during merge? I mean, people are always telling you not to force push, right? :/

divadsn closed this task as Resolved.Jan 11 2018, 3:24 PM

Closing as the original PR got merged a week ago.