New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Phabricator backend #169

Merged
merged 2 commits into from Dec 30, 2017

Conversation

Projects
None yet
3 participants
@divadsn
Contributor

divadsn commented Dec 26, 2017

This adds a OAuth2 backend for Phabricator to allow users to authenticate using Phabricator login.

See: https://phabricator.wikimedia.org/T179202

ACCESS_TOKEN_URL = 'https://secure.phabricator.com/oauthserver/token/'
ACCESS_TOKEN_METHOD = 'POST'
REDIRECT_STATE = False
DEFAULT_SCOPE = []

This comment has been minimized.

@jayvdb

jayvdb Dec 27, 2017

If there is no default scope, this could also be omitted, or better set to None same as the super class, but ...

I suspect that the default 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.

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.

'email': response.get('primaryEmail', ''),
'fullname': fullname,
'first_name': first_name,
'last_name': last_name

This comment has been minimized.

@jayvdb

jayvdb Dec 27, 2017

trailing commas are a good idea ; look at where else you might add them

divadsn added some commits Dec 29, 2017

Add PhabricatorOAuth2 backend
Signed-off-by: David Sn <divad.nnamtdeis@gmail.com>
Add test for PhabricatorOAuth2 backend
Signed-off-by: David Sn <divad.nnamtdeis@gmail.com>

@divadsn divadsn force-pushed the divadsn:phabricator-oauth branch from 2c563e6 to 09aa154 Dec 29, 2017

@divadsn

This comment has been minimized.

Contributor

divadsn commented Dec 29, 2017

I squashed those styling edits back into two commits @jayvdb :)

@jayvdb

This comment has been minimized.

jayvdb commented Dec 29, 2017

It is one feature; only one commit is necessary

@divadsn

This comment has been minimized.

Contributor

divadsn commented Dec 29, 2017

Hm, I was taking #162 as an example for my commits, but I can squash them later into one, I still have not figured out how to use git on a Chromebook :D

@omab

omab approved these changes Dec 30, 2017

Looks good.

@omab omab merged commit a938ea4 into python-social-auth:master Dec 30, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@omab

This comment has been minimized.

Contributor

omab commented Dec 30, 2017

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment