Fixed merge conflicts

.travis.yml

@@ -9,12 +9,15 @@ env:
   - TOX_ENV=py27-django15
   - TOX_ENV=py27-django16
   - TOX_ENV=py27-django17
+  - TOX_ENV=py27-django18
   - TOX_ENV=py33-django15
   - TOX_ENV=py33-django16
   - TOX_ENV=py33-django17
+  - TOX_ENV=py33-django18
   - TOX_ENV=py34-django15
   - TOX_ENV=py34-django16
   - TOX_ENV=py34-django17
+  - TOX_ENV=py34-django18
   - TOX_ENV=docs
 
 install:

README.rst

@@ -41,7 +41,7 @@ Requirements
 ------------
 
 * Python 2.6, 2.7, 3.3, 3.4
-* Django 1.4, 1.5, 1.6, 1.7
+* Django 1.4, 1.5, 1.6, 1.7, 1.8
 
 Installation
 ------------
@@ -95,6 +95,8 @@ master branch
 
 * ``oauthlib_backend_class`` is now pluggable through Django settings
 * #127: ``application/json`` Content-Type is now supported using ``JSONOAuthLibCore``
+* #238: Fixed redirect uri handling in case of error
+* #229: Invalidate access tokens when getting a new refresh token
 
 0.8.1 [2015-04-27]
 ~~~~~~~~~~~~~~~~~~

docs/changelog.rst

@@ -6,6 +6,8 @@ master branch
 
 * ``oauthlib_backend_class`` is now pluggable through Django settings
 * #127: ``application/json`` Content-Type is now supported using ``JSONOAuthLibCore``
+* #238: Fixed redirect uri handling in case of error
+* #229: Invalidate access tokens when getting a new refresh token
 
 
 0.8.1 [2015-04-27]

oauth2_provider/models.py

@@ -219,6 +219,13 @@ class AccessToken(AbstractAccessToken):
     """
     token = models.CharField(max_length=255, db_index=True)
 
+    def revoke(self):
+        """
+        Convenience method to uniform tokens' interface, for now
+        simply remove this token from the database in order to revoke it.
+        """
+        self.delete()
+
     def __str__(self):
         return self.token
 
@@ -252,6 +259,13 @@ class RefreshToken(AbstractRefreshToken):
     """
     token = models.CharField(max_length=255, db_index=True)
 
+    def revoke(self):
+        """
+        Delete this refresh token along with related access token
+        """
+        AccessToken.objects.get(id=self.access_token.id).revoke()
+        self.delete()
+
     def __str__(self):
         return self.token
 

oauth2_provider/oauth2_validators.py

@@ -296,7 +296,7 @@ def save_bearer_token(self, token, request, *args, **kwargs):
         if request.refresh_token:
             # remove used refresh token
             try:
-                RefreshToken.objects.get(token=request.refresh_token).delete()
+                RefreshToken.objects.get(token=request.refresh_token).revoke()
             except RefreshToken.DoesNotExist:
                 assert()  # TODO though being here would be very strange, at least log the error
 
@@ -348,10 +348,11 @@ def revoke_token(self, token, token_type_hint, request, *args, **kwargs):
 
         token_type = token_types.get(token_type_hint, AccessToken)
         try:
-            token_type.objects.get(token=token).delete()
+            token_type.objects.get(token=token).revoke()
         except ObjectDoesNotExist:
             for other_type in [_t for _t in token_types.values() if _t != token_type]:
-                other_type.objects.filter(token=token).delete()
+                # slightly inefficient on Python2, but the queryset contains only one instance
+                list(map(lambda t: t.revoke(), other_type.objects.filter(token=token)))
 
     def validate_user(self, username, password, client, request, *args, **kwargs):
         """

oauth2_provider/tests/test_authorization_code.py

@@ -10,7 +10,7 @@
 from django.utils import timezone
 
 from ..compat import urlparse, parse_qs, urlencode, get_user_model
-from ..models import get_application_model, Grant, get_access_token_model
+from ..models import get_application_model, Grant, get_access_token_model, get_refresh_token_model
 from ..settings import oauth2_settings
 from ..views import ProtectedResourceView
 
@@ -19,6 +19,7 @@
 
 Application = get_application_model()
 AccessToken = get_access_token_model()
+RefreshToken = get_access_token_model()
 UserModel = get_user_model()
 
 
@@ -424,6 +425,27 @@ def test_code_post_auth_redirection_uri_with_querystring(self):
         self.assertIn("http://example.com?foo=bar", response['Location'])
         self.assertIn("code=", response['Location'])
 
+    def test_code_post_auth_failing_redirection_uri_with_querystring(self):
+        """
+        Test that in case of error the querystring of the redirection uri is preserved
+
+        See https://github.com/evonove/django-oauth-toolkit/issues/238
+        """
+        self.client.login(username="test_user", password="123456")
+
+        form_data = {
+            'client_id': self.application.client_id,
+            'state': 'random_state_string',
+            'scope': 'read write',
+            'redirect_uri': 'http://example.com?foo=bar',
+            'response_type': 'code',
+            'allow': False,
+        }
+
+        response = self.client.post(reverse('oauth2_provider:authorize'), data=form_data)
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual("http://example.com?foo=bar&error=access_denied", response['Location'])
+
     def test_code_post_auth_fails_when_redirect_uri_path_is_invalid(self):
         """
         Tests that a redirection uri is matched using scheme + netloc + path
@@ -527,6 +549,37 @@ def test_refresh(self):
         content = json.loads(response.content.decode("utf-8"))
         self.assertTrue('invalid_grant' in content.values())
 
+    def test_refresh_invalidates_old_tokens(self):
+        """
+        Ensure existing refresh tokens are cleaned up when issuing new ones
+        """
+        self.client.login(username="test_user", password="123456")
+        authorization_code = self.get_auth()
+
+        token_request_data = {
+            'grant_type': 'authorization_code',
+            'code': authorization_code,
+            'redirect_uri': 'http://example.it'
+        }
+        auth_headers = self.get_basic_auth_header(self.application.client_id, self.application.client_secret)
+
+        response = self.client.post(reverse('oauth2_provider:token'), data=token_request_data, **auth_headers)
+        content = json.loads(response.content.decode("utf-8"))
+
+        rt = content['refresh_token']
+        at = content['access_token']
+
+        token_request_data = {
+            'grant_type': 'refresh_token',
+            'refresh_token': rt,
+            'scope': content['scope'],
+        }
+        response = self.client.post(reverse('oauth2_provider:token'), data=token_request_data, **auth_headers)
+        self.assertEqual(response.status_code, 200)
+
+        self.assertFalse(RefreshToken.objects.filter(token=rt).exists())
+        self.assertFalse(AccessToken.objects.filter(token=at).exists())
+
     def test_refresh_no_scopes(self):
         """
         Request an access token using a refresh token without passing any scope

oauth2_provider/tests/test_client_credential.py

@@ -111,6 +111,7 @@ class TestExtendedRequest(BaseTest):
     @classmethod
     def setUpClass(cls):
         cls.request_factory = RequestFactory()
+        super(TestExtendedRequest, cls).setUpClass()
 
     def test_extended_request(self):
         class TestView(OAuthLibMixin, View):

oauth2_provider/tests/test_decorators.py

@@ -20,6 +20,7 @@ class TestProtectedResourceDecorator(TestCase, TestCaseUtils):
     @classmethod
     def setUpClass(cls):
         cls.request_factory = RequestFactory()
+        super(TestProtectedResourceDecorator, cls).setUpClass()
 
     def setUp(self):
         self.user = UserModel.objects.create_user("test_user", "test@user.com", "123456")

oauth2_provider/tests/test_mixins.py

@@ -16,6 +16,7 @@ class BaseTest(TestCase):
     @classmethod
     def setUpClass(cls):
         cls.request_factory = RequestFactory()
+        super(BaseTest, cls).setUpClass()
 
 
 class TestOAuthLibMixin(BaseTest):

oauth2_provider/tests/test_models.py

@@ -27,7 +27,7 @@ def setUp(self):
 
     def test_allow_scopes(self):
         self.client.login(username="test_user", password="123456")
-        app = Application(
+        app = Application.objects.create(
             name="test_app",
             redirect_uris="http://localhost http://example.com http://example.it",
             user=self.user,
@@ -98,10 +98,12 @@ def test_related_objects(self):
         See issue #90 (https://github.com/evonove/django-oauth-toolkit/issues/90)
         """
         # Django internals caches the related objects.
-        del UserModel._meta._related_objects_cache
+        if django.VERSION < (1, 8):
+            del UserModel._meta._related_objects_cache
         related_object_names = [ro.name for ro in UserModel._meta.get_all_related_objects()]
         self.assertNotIn('oauth2_provider:application', related_object_names)
-        self.assertIn('tests:testapplication', related_object_names)
+        self.assertIn('tests%stestapplication' % (':' if django.VERSION < (1, 8) else '_'),
+                      related_object_names)
 
 
 class TestGrantModel(TestCase):

oauth2_provider/tests/test_token_revocation.py

@@ -120,6 +120,7 @@ def test_revoke_refresh_token(self):
         response = self.client.post(url)
         self.assertEqual(response.status_code, 200)
         self.assertFalse(RefreshToken.objects.filter(id=rtok.id).exists())
+        self.assertFalse(AccessToken.objects.filter(id=rtok.access_token.id).exists())
 
     def test_revoke_token_with_wrong_hint(self):
         """

oauth2_provider/views/mixins.py

@@ -155,9 +155,13 @@ def error_response(self, error, **kwargs):
         :param error: :attr:`OAuthToolkitError`
         """
         oauthlib_error = error.oauthlib_error
+
+        redirect_uri = oauthlib_error.redirect_uri or ""
+        separator = '&' if '?' in redirect_uri else '?'
+
         error_response = {
             'error': oauthlib_error,
-            'url': "{0}?{1}".format(oauthlib_error.redirect_uri, oauthlib_error.urlencoded)
+            'url': "{0}{1}{2}".format(oauthlib_error.redirect_uri, separator, oauthlib_error.urlencoded)
         }
         error_response.update(kwargs)
 

tox.ini

@@ -1,9 +1,9 @@
 [tox]
 envlist =
     py26-django14, py26-django15, py26-django16,
-    py27-django14, py27-django15, py27-django16, py27-django17,
-    py33-django15, py33-django16, py33-django17,
-    py34-django15, py34-django16, py34-django17,
+    py27-django14, py27-django15, py27-django16, py27-django17, py27-django18,
+    py33-django15, py33-django16, py33-django17, py33-django18,
+    py34-django15, py34-django16, py34-django17, py34-django18,
     docs,
     flake8
 
@@ -59,6 +59,12 @@ deps =
     Django<1.8
     {[testenv]deps}
 
+[testenv:py27-django18]
+basepython = python2.7
+deps =
+    Django<1.9
+    {[testenv]deps}
+
 [testenv:py33-django15]
 basepython = python3.3
 deps =
@@ -78,6 +84,12 @@ deps =
     Django<1.8
     {[testenv]deps}
 
+[testenv:py33-django18]
+basepython = python3.3
+deps =
+    Django<1.9
+    {[testenv]deps}
+
 [testenv:py34-django15]
 basepython = python3.4
 deps =
@@ -97,6 +109,12 @@ deps =
     Django<1.8
     {[testenv]deps}
 
+[testenv:py34-django18]
+basepython = python3.4
+deps =
+    Django<1.9
+    {[testenv]deps}
+
 [testenv:docs]
 basepython=python
 changedir=docs