OAuth2 Authentication #693

dulacp · 2013-03-01T01:18:52Z

After the OAuth1 PR, here is the OAuth2Authencation class as discussed in #8

I've incorporated some doc on "how to get started" with django-oauth2-provider but maybe you don't want to incorporate those kind of third-party-documentation. I made that as a temporary way for people to get started, waiting for the official doc to be ready.

tomchristie · 2013-03-01T09:21:34Z

docs/css/bootstrap.css

@@ -859,7 +859,7 @@ pre {
 }

 code {
-  padding: 2px 4px;
+  padding: 1px 4px;


Keep any CSS changes to a separate pull req.

Allright, I'll move them right now.

tomchristie · 2013-03-01T09:30:27Z

Great stuff! Really excellent. Comments inline. 👍 :)

dulacp · 2013-03-01T10:23:18Z

Thanks :)
And thank you for the comments too!

Conflicts: rest_framework/tests/authentication.py

to avoid naming collision with `oauth2` used for OAuth 1

dulacp · 2013-03-01T20:21:10Z

@thedrow I just forgot to add the "pip install" command to the .travis.yaml file.
I'm doing it right now!

tomchristie · 2013-03-01T21:16:35Z

rest_framework/tests/authentication.py

 from rest_framework.tests.utils import RequestFactory
 from rest_framework.views import APIView
 import json
 import base64
+import datetime
+import unittest


Use 'from django.utils import unittest' for compatibility across different py versions

thanks @tomchristie for this tip!

for a cross python versions compatibility

dulacp · 2013-03-01T23:36:50Z

Well, the pull request is still failing !
For Python 3 Travis Jobs it's due to the shortuuid dependency of django-oauth2-provider that doesn't seem to be compatible with python 3. I'll investigate that.

I've just found out about tox, so I will make more tests before submitting another commit, promise !

tomchristie · 2013-03-02T07:57:50Z

django-oauth2-provider isn't yet py3k compatible, so may as well drop it from tox and Travis for the moment. Just leaves Django 1.3 errors which are a bit more mysterious.

On 1 Mar 2013, at 23:36, Pierre Dulac notifications@github.com wrote:

Well, the pull request is still failing !
For Python 3 Travis Jobs it's due to the shortuuid dependency of django-oauth2-provider that doesn't seem to be compatible with python 3. I'll investigate that.

I've just found out about tox, so I will make more tests before submitting another commit, promise !

—
Reply to this email directly or view it on GitHub.

dulacp · 2013-03-02T10:51:46Z

I agree, the Django 1.3 errors are pretty mysterious, but I've already came across a similar issue while implementing the OAuth2Authentication class. And it was because I used an undefined method... and instead of raising an AttributeError exception concerning the undefined method called, it mysteriously raised an AttributeError concerning the successful_authenticator method not found, which was silly.

But I don't know the reasons that can explain why python is mixing up exceptions like that.

dulacp · 2013-03-02T14:45:46Z

I've finally cracked it ! :)

The mysterious error is due to the fact that the oauth2_provider module imported in compat.py#L1R433 does not allow submodule access like if want to do form = oauth2_provider.forms.ClientAuthForm().

So, when I create the form in authentication.py#L3R192 in fact it fails and raise an exception.

The thing I don't know is why the AttributeError exception raised is referencing to the successful_authenticator method and not to the original exception which is

AttributeError: 'module' object has no attribute 'forms'

tomchristie · 2013-03-03T08:46:56Z

.travis.yml

@@ -14,6 +14,7 @@ env:
 install:
  - pip install $DJANGO
  - pip install defusedxml==0.3
+  - "if [[ ${TRAVIS_PYTHON_VERSION::1} != '3' ]]; then pip install -e git+git://github.com/dulaccc/django-oauth2-provider.git#egg=django-oauth2-provider; fi"


What's different in your fork?

Hi used my fork, temporary, because I didn't get any return from the repo owner concerning my pull requests and other pull requesters too.

And in my fork, I made a fix to add the submodules import statements into the provider.oauth2.__init__ file to fix the error we were getting with Django 1.3
Moreover, I made changes to the README in order to point to the new documentation I'm refactoring.

Okay. Can't really merge this until it's pointed at the PyPI version. If we can use the PyPI version and workaround any import issues inside the compat module that'd be great.

I understand. I will change that.
But as @thedrow suggested, do you want me to then rebase the commit into the corresponding one where I initially use my fork ?

Yes, I perfectly understand, and fortunately no one has forked my public fork so no harm :) I'll forced my commit to take into account the rebase, you're right this is the correct way to fix "commit mistakes" if no one fork those commits.

I don't care about history so much, so don't worry about rebase unless you want too.

On 6 Mar 2013, at 17:37, Pierre Dulac notifications@github.com wrote:

In .travis.yml:

@@ -14,6 +14,7 @@ env:
install:

pip install $DJANGO

pip install defusedxml==0.3

"if [[ ${TRAVIS_PYTHON_VERSION::1} != '3' ]]; then pip install -e git+git://github.com/dulaccc/django-oauth2-provider.git#egg=django-oauth2-provider; fi"
I understand. I will change that.
But as @thedrow suggested, do you want me to then rebase the commit into the corresponding one where I initially use my fork ?

—
Reply to this email directly or view it on GitHub.

Changes pushed. I've done a rebase to clean a little and avoid "doing this, undoing this, redoing this" ^^ and to learn in the same time the better practices.
The only thing is that it didn't seem to trigger the Travis builds.

tomchristie · 2013-03-06T17:15:50Z

Correct.

On 6 March 2013 17:09, Omer Katz notifications@github.com wrote:

So this will only allow authenticating with your own oauth service right?
There's no client functionality that will allow you to authenticate with
other oauth providers as far as I can see. Am I correct?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/693#issuecomment-14512833
.

tomchristie · 2013-03-06T17:15:56Z

local3/src/pip-delete-this-directory.txt

+here by pip.
+
+Once this package is successfully installed this source code will be
+deleted (unless you remove this file).


Drop this file.

Oops... this shouldn't be here

dulacp · 2013-03-06T17:17:42Z

Actually, the OAuth authentication class is pretty short. So, if you want to use another oauth provider you can easily adapt yours and write your own Authentication class.

dulacp · 2013-03-06T17:29:19Z

Allright, I will learn how to use git rebase then.

otherwise log a warning

@tomchristie

with a warning for incompatibility with Python 3 and taking @tomchristie advice into account on how to reformulate some sentences

dulacp · 2013-03-06T22:11:56Z

Travis build isn't showing up but it was triggered build#5296716

For the awesome OAuth 2 support in #693.

tomchristie · 2013-03-08T20:28:29Z

Merged into #709.

michaelmior · 2013-03-21T12:45:23Z

Awesome! Thanks a lot folks :)

dulacp added 5 commits March 1, 2013 02:06

Add tests for OAuth2 authentication

468b5e4

Add django-oauth2-provider to the installed apps

4e1f77d

Add django-oauth2-provider to optionals.txt

592a0a5

Add the OAuth2Authentication class

da9d7fb

Add OAuth2Authentication documentation

d8f455b

tomchristie reviewed Mar 1, 2013
View reviewed changes

dulacp added 4 commits March 1, 2013 11:50

Merge branch 'master' into oauth2-authentication

aed3c13

Conflicts: rest_framework/tests/authentication.py

Improve the django-oauth2-provider import block

9d5c306

to avoid naming collision with `oauth2` used for OAuth 1

Use the correct doc link style

653fcf7

Clean up some print and comments

d4c2267

Add django-oauth2-provider to .travis.yml

182edb3

tomchristie reviewed Mar 1, 2013
View reviewed changes

Use django.utils to import the unittest module

721dc51

for a cross python versions compatibility

dulacp added 2 commits March 2, 2013 20:16

Add new OAuth2 tests

8809c46

Properly fail to wrong Authorization token type

c449dd4

tomchristie reviewed Mar 3, 2013
View reviewed changes

tomchristie reviewed Mar 6, 2013
View reviewed changes

dulacp added 5 commits March 6, 2013 19:15

Use the PyPI django-oauth2-provider version

6f57641

Only add the django-oauth2-provider apps if the module is installed

cda21a3

otherwise log a warning

Update the documentation

30e3775

with a warning for incompatibility with Python 3 and taking @tomchristie advice into account on how to reformulate some sentences

Fix import errors

8845c0b

Update the package dependency url style in tox.ini

5a56f92

dulacp mentioned this pull request Mar 6, 2013

Rebased pull requests do not trigger new builds travis-ci/travis-ci#719

Closed

tomchristie added a commit that referenced this pull request Mar 8, 2013

Added @dulaccc.

1016c14

For the awesome OAuth 2 support in #693.

tomchristie closed this Mar 8, 2013

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OAuth2 Authentication #693

OAuth2 Authentication #693

dulacp commented Mar 1, 2013

tomchristie Mar 1, 2013

dulacp Mar 1, 2013

tomchristie commented Mar 1, 2013

dulacp commented Mar 1, 2013

dulacp commented Mar 1, 2013

tomchristie Mar 1, 2013

dulacp Mar 1, 2013

dulacp commented Mar 1, 2013

tomchristie commented Mar 2, 2013

dulacp commented Mar 2, 2013

dulacp commented Mar 2, 2013

tomchristie Mar 3, 2013

dulacp Mar 3, 2013

tomchristie Mar 6, 2013

dulacp Mar 6, 2013

dulacp Mar 6, 2013

tomchristie Mar 6, 2013

dulacp Mar 6, 2013

tomchristie commented Mar 6, 2013

tomchristie Mar 6, 2013

dulacp Mar 6, 2013

dulacp commented Mar 6, 2013

dulacp commented Mar 6, 2013

dulacp commented Mar 6, 2013

tomchristie commented Mar 8, 2013

michaelmior commented Mar 21, 2013

OAuth2 Authentication #693

OAuth2 Authentication #693

Conversation

dulacp commented Mar 1, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomchristie commented Mar 1, 2013

dulacp commented Mar 1, 2013

dulacp commented Mar 1, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dulacp commented Mar 1, 2013

tomchristie commented Mar 2, 2013

dulacp commented Mar 2, 2013

dulacp commented Mar 2, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomchristie commented Mar 6, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dulacp commented Mar 6, 2013

dulacp commented Mar 6, 2013

dulacp commented Mar 6, 2013

tomchristie commented Mar 8, 2013

michaelmior commented Mar 21, 2013