-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
OAuth2 Authentication #693
Changes from 10 commits
468b5e4
4e1f77d
592a0a5
da9d7fb
d8f455b
aed3c13
9d5c306
653fcf7
d4c2267
182edb3
721dc51
8809c46
c449dd4
6f57641
cda21a3
30e3775
8845c0b
5a56f92
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
from django.utils.encoding import DjangoUnicodeDecodeError | ||
from rest_framework import exceptions, HTTP_HEADER_ENCODING | ||
from rest_framework.compat import CsrfViewMiddleware | ||
from rest_framework.compat import oauth2_provider | ||
from rest_framework.authtoken.models import Token | ||
import base64 | ||
|
||
|
@@ -155,4 +156,80 @@ def authenticate_header(self, request): | |
return 'Token' | ||
|
||
|
||
# TODO: OAuthAuthentication | ||
class OAuth2Authentication(BaseAuthentication): | ||
""" | ||
OAuth 2 authentication backend using `django-oauth2-provider` | ||
""" | ||
require_active = True | ||
|
||
def __init__(self, **kwargs): | ||
super(OAuth2Authentication, self).__init__(**kwargs) | ||
if oauth2_provider is None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we check for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So if I understand what you're suggesting, it's to replace if oauth2_provider is None: with a |
||
raise ImproperlyConfigured("The 'django-oauth2-provider' package could not be imported. It is required for use with the 'OAuth2Authentication' class.") | ||
|
||
def authenticate(self, request): | ||
""" | ||
The Bearer type is the only finalized type | ||
|
||
Read the spec for more details | ||
http://tools.ietf.org/html/rfc6749#section-7.1 | ||
""" | ||
auth = request.META.get('HTTP_AUTHORIZATION', '').split() | ||
if not auth or auth[0].lower() != "bearer": | ||
return None | ||
|
||
if len(auth) != 2: | ||
raise exceptions.AuthenticationFailed('Invalid token header') | ||
|
||
return self.authenticate_credentials(request, auth[1]) | ||
|
||
def authenticate_credentials(self, request, access_token): | ||
""" | ||
:returns: two-tuple of (user, auth) if authentication succeeds, or None otherwise. | ||
""" | ||
|
||
# authenticate the client | ||
oauth2_client_form = oauth2_provider.forms.ClientAuthForm(request.REQUEST) | ||
if not oauth2_client_form.is_valid(): | ||
raise exceptions.AuthenticationFailed("Client could not be validated") | ||
client = oauth2_client_form.cleaned_data.get('client') | ||
|
||
# retrieve the `oauth2_provider.models.OAuth2AccessToken` instance from the access_token | ||
auth_backend = oauth2_provider.backends.AccessTokenBackend() | ||
token = auth_backend.authenticate(access_token, client) | ||
if token is None: | ||
raise exceptions.AuthenticationFailed("Invalid token") # does not exist or is expired | ||
|
||
# TODO check scope | ||
|
||
if not self.check_active(token.user): | ||
raise exceptions.AuthenticationFailed('User not active: %s' % token.user.username) | ||
|
||
if client and token: | ||
request.user = token.user | ||
return (request.user, None) | ||
|
||
raise exceptions.AuthenticationFailed( | ||
'You are not allowed to access this resource.') | ||
|
||
return None | ||
|
||
def authenticate_header(self, request): | ||
""" | ||
Bearer is the only finalized type currently | ||
|
||
Check details on the `OAuth2Authentication.authenticate` method | ||
""" | ||
return 'Bearer' | ||
|
||
def check_active(self, user): | ||
""" | ||
Ensures the user has an active account. | ||
|
||
Optimized for the ``django.contrib.auth.models.User`` case. | ||
""" | ||
if not self.require_active: | ||
# Ignore & move on. | ||
return True | ||
|
||
return user.is_active |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -426,3 +426,10 @@ def apply_markdown(text): | |
import defusedxml.ElementTree as etree | ||
except ImportError: | ||
etree = None | ||
|
||
|
||
# OAuth 2 support is optional | ||
try: | ||
import provider.oauth2 as oauth2_provider | ||
except ImportError: | ||
oauth2_provider = None | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given that this import isn't really required by the codebase can we just drop it and rely on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah you're right, initially I added it to manage the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll need to talk about scopes & permissions, but let's leave that aside for a moment. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
from __future__ import unicode_literals | ||
from django.core.urlresolvers import reverse | ||
from django.contrib.auth.models import User | ||
from django.http import HttpResponse | ||
from django.test import Client, TestCase | ||
|
@@ -11,13 +12,17 @@ | |
BaseAuthentication, | ||
TokenAuthentication, | ||
BasicAuthentication, | ||
SessionAuthentication | ||
SessionAuthentication, | ||
OAuth2Authentication | ||
) | ||
from rest_framework.compat import patterns | ||
from rest_framework.compat import patterns, url, include | ||
from rest_framework.compat import oauth2_provider | ||
from rest_framework.tests.utils import RequestFactory | ||
from rest_framework.views import APIView | ||
import json | ||
import base64 | ||
import datetime | ||
import unittest | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use 'from django.utils import unittest' for compatibility across different py versions There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks @tomchristie for this tip! |
||
|
||
|
||
factory = RequestFactory() | ||
|
@@ -41,6 +46,8 @@ def put(self, request): | |
(r'^basic/$', MockView.as_view(authentication_classes=[BasicAuthentication])), | ||
(r'^token/$', MockView.as_view(authentication_classes=[TokenAuthentication])), | ||
(r'^auth-token/$', 'rest_framework.authtoken.views.obtain_auth_token'), | ||
url(r'^oauth2/', include('provider.oauth2.urls', namespace = 'oauth2')), | ||
url(r'^oauth2-test/$', MockView.as_view(authentication_classes=[OAuth2Authentication])), | ||
) | ||
|
||
|
||
|
@@ -222,3 +229,99 @@ def authenticate(self, request): | |
response = view(request) | ||
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) | ||
self.assertEqual(response.data, {'detail': 'Bad credentials'}) | ||
|
||
|
||
class OAuth2Tests(TestCase): | ||
"""OAuth 2.0 authentication""" | ||
urls = 'rest_framework.tests.authentication' | ||
|
||
def setUp(self): | ||
self.csrf_client = Client(enforce_csrf_checks=True) | ||
self.username = 'john' | ||
self.email = '[email protected]' | ||
self.password = 'password' | ||
self.user = User.objects.create_user(self.username, self.email, self.password) | ||
|
||
self.CLIENT_ID = 'client_key' | ||
self.CLIENT_SECRET = 'client_secret' | ||
self.ACCESS_TOKEN = "access_token" | ||
self.REFRESH_TOKEN = "refresh_token" | ||
|
||
self.oauth2_client = oauth2_provider.models.Client.objects.create( | ||
client_id=self.CLIENT_ID, | ||
client_secret=self.CLIENT_SECRET, | ||
redirect_uri='', | ||
client_type=0, | ||
name='example', | ||
user=None, | ||
) | ||
|
||
self.access_token = oauth2_provider.models.AccessToken.objects.create( | ||
token=self.ACCESS_TOKEN, | ||
client=self.oauth2_client, | ||
user=self.user, | ||
) | ||
self.refresh_token = oauth2_provider.models.RefreshToken.objects.create( | ||
user=self.user, | ||
access_token=self.access_token, | ||
client=self.oauth2_client | ||
) | ||
|
||
def _create_authorization_header(self, token=None): | ||
return "Bearer {0}".format(token or self.access_token.token) | ||
|
||
def _client_credentials_params(self): | ||
return {'client_id': self.CLIENT_ID, 'client_secret': self.CLIENT_SECRET} | ||
|
||
@unittest.skipUnless(oauth2_provider, 'django-oauth2-provider not installed') | ||
def test_get_form_with_wrong_client_data_failing_auth(self): | ||
"""Ensure GETing form over OAuth with incorrect client credentials fails""" | ||
auth = self._create_authorization_header() | ||
params = self._client_credentials_params() | ||
params['client_id'] += 'a' | ||
response = self.csrf_client.get('/oauth2-test/', params, HTTP_AUTHORIZATION=auth) | ||
self.assertEqual(response.status_code, 401) | ||
|
||
@unittest.skipUnless(oauth2_provider, 'django-oauth2-provider not installed') | ||
def test_get_form_passing_auth(self): | ||
"""Ensure GETing form over OAuth with correct client credentials succeed""" | ||
auth = self._create_authorization_header() | ||
params = self._client_credentials_params() | ||
response = self.csrf_client.get('/oauth2-test/', params, HTTP_AUTHORIZATION=auth) | ||
self.assertEqual(response.status_code, 200) | ||
|
||
@unittest.skipUnless(oauth2_provider, 'django-oauth2-provider not installed') | ||
def test_post_form_passing_auth(self): | ||
"""Ensure POSTing form over OAuth with correct credentials passes and does not require CSRF""" | ||
auth = self._create_authorization_header() | ||
params = self._client_credentials_params() | ||
response = self.csrf_client.post('/oauth2-test/', params, HTTP_AUTHORIZATION=auth) | ||
self.assertEqual(response.status_code, 200) | ||
|
||
@unittest.skipUnless(oauth2_provider, 'django-oauth2-provider not installed') | ||
def test_post_form_token_removed_failing_auth(self): | ||
"""Ensure POSTing when there is no OAuth access token in db fails""" | ||
self.access_token.delete() | ||
auth = self._create_authorization_header() | ||
params = self._client_credentials_params() | ||
response = self.csrf_client.post('/oauth2-test/', params, HTTP_AUTHORIZATION=auth) | ||
self.assertIn(response.status_code, (status.HTTP_401_UNAUTHORIZED, status.HTTP_403_FORBIDDEN)) | ||
|
||
@unittest.skipUnless(oauth2_provider, 'django-oauth2-provider not installed') | ||
def test_post_form_with_refresh_token_failing_auth(self): | ||
"""Ensure POSTing with refresh token instead of access token fails""" | ||
auth = self._create_authorization_header(token=self.refresh_token.token) | ||
params = self._client_credentials_params() | ||
response = self.csrf_client.post('/oauth2-test/', params, HTTP_AUTHORIZATION=auth) | ||
self.assertIn(response.status_code, (status.HTTP_401_UNAUTHORIZED, status.HTTP_403_FORBIDDEN)) | ||
|
||
@unittest.skipUnless(oauth2_provider, 'django-oauth2-provider not installed') | ||
def test_post_form_with_expired_access_token_failing_auth(self): | ||
"""Ensure POSTing with expired access token fails with an 'Invalid token' error""" | ||
self.access_token.expires = datetime.datetime.now() - datetime.timedelta(seconds=10) # 10 seconds late | ||
self.access_token.save() | ||
auth = self._create_authorization_header() | ||
params = self._client_credentials_params() | ||
response = self.csrf_client.post('/oauth2-test/', params, HTTP_AUTHORIZATION=auth) | ||
self.assertIn(response.status_code, (status.HTTP_401_UNAUTHORIZED, status.HTTP_403_FORBIDDEN)) | ||
self.assertIn('Invalid token', response.content) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll make some changes to the tone of these docs, but looks good so far.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah ^^ I understand what you mean when I read it again this morning.
And, English is not my native language so I've probably made mistakes or use weird ways to tell thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally fine :), just some things like "And hopefully," - sounds a bit uncertain - we'd like to be a bit more assertive than that. :p
Also, we'll use something less opinionated than "Unfortunately, there isn't a lot of documentation...".
Finally, we probably shouldn't state that HTTPS is highly recommended, but instead simply say that in production you must use HTTPS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will "refactor" the documentation very soon.
And just to let you know, I've made a pull request on the
django-oauth2-provider
project to improve its documentation. I learn to usesphinx
by the way, that was fun.I've configured a temporary project on readthedocs.org to let you see the work. Because I don't know if I will have a reply for my pull request.