From 9049d6350f88f2a67ceff582b84bfb1c1d2a0ee2 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 3 Nov 2013 13:07:10 -0800 Subject: [PATCH 1/2] implement OAUTH_CLEAN_EXPIRED, clean as you go this avoids keeping around unneeded/no-longer usable objects. it's optional defaulting to False, the current behavior. adds a setting OAUTH_CLEAN_EXPIRED that optionally cleans out objects as they're expired which should keep the size of the grant, access, and refresh token tables in check. once grants are used they are deleted. and once a RefreshToken is used both it and its corresponding AccessToken are deleted. --- provider/oauth2/tests.py | 76 +++++++++++++++++++++++++++++++++++++++- provider/oauth2/views.py | 22 ++++++++---- 2 files changed, 91 insertions(+), 7 deletions(-) diff --git a/provider/oauth2/tests.py b/provider/oauth2/tests.py index 6a051bc..988af23 100644 --- a/provider/oauth2/tests.py +++ b/provider/oauth2/tests.py @@ -12,7 +12,7 @@ from ..compat import skipIfCustomUser from ..templatetags.scope import scopes from ..utils import now as date_now from .forms import ClientForm -from .models import Client, Grant, AccessToken +from .models import Client, Grant, AccessToken, RefreshToken from .backends import BasicClientBackend, RequestParamsClientBackend from .backends import AccessTokenBackend @@ -531,3 +531,77 @@ class ScopeTest(TestCase): names.sort() self.assertEqual('read read+write write', ' '.join(names)) + + +class CleanExpiredTest(BaseOAuth2TestCase): + fixtures = ['test_oauth2'] + + def setUp(self): + self._old_oauth_clean_expired = getattr(settings, + 'OAUTH_CLEAN_EXPIRED', None) + settings.OAUTH_CLEAN_EXPIRED = True + + def tearDown(self): + if self._old_oauth_clean_expired is not None: + settings.OAUTH_CLEAN_EXPIRED = self._old_oauth_clean_expired + else: + delattr(settings, 'OAUTH_CLEAN_EXPIRED') + + def test_clear_expired(self): + self.login() + + self._login_and_authorize() + + response = self.client.get(self.redirect_url()) + + self.assertEqual(302, response.status_code) + location = response['Location'] + self.assertFalse('error' in location) + self.assertTrue('code' in location) + + # verify that Grant with code exists + code = urlparse.parse_qs(location)['code'][0] + self.assertTrue(Grant.objects.filter(code=code).exists()) + + from pprint import pprint + + # use the code/grant + response = self.client.post(self.access_token_url(), { + 'grant_type': 'authorization_code', + 'client_id': self.get_client().client_id, + 'client_secret': self.get_client().client_secret, + 'code': code}) + self.assertEquals(200, response.status_code) + token = json.loads(response.content) + self.assertTrue('access_token' in token) + access_token = token['access_token'] + self.assertTrue('refresh_token' in token) + refresh_token = token['refresh_token'] + + # make sure the grant is gone + self.assertFalse(Grant.objects.filter(code=code).exists()) + # and verify that the AccessToken and RefreshToken exist + self.assertTrue(AccessToken.objects.filter(token=access_token) + .exists()) + self.assertTrue(RefreshToken.objects.filter(token=refresh_token) + .exists()) + + # refresh the token + response = self.client.post(self.access_token_url(), { + 'grant_type': 'refresh_token', + 'refresh_token': token['refresh_token'], + 'client_id': self.get_client().client_id, + 'client_secret': self.get_client().client_secret, + }) + self.assertEqual(200, response.status_code) + token = json.loads(response.content) + self.assertTrue('access_token' in token) + self.assertNotEquals(access_token, token['access_token']) + self.assertTrue('refresh_token' in token) + self.assertNotEquals(refresh_token, token['refresh_token']) + + # make sure the orig AccessToken and RefreshToken are gone + self.assertFalse(AccessToken.objects.filter(token=access_token) + .exists()) + self.assertFalse(RefreshToken.objects.filter(token=refresh_token) + .exists()) diff --git a/provider/oauth2/views.py b/provider/oauth2/views.py index 0c3af84..fa9fbb1 100644 --- a/provider/oauth2/views.py +++ b/provider/oauth2/views.py @@ -1,4 +1,5 @@ from datetime import timedelta +from django.conf import settings from django.core.urlresolvers import reverse from ..views import Capture, Authorize, Redirect from ..views import AccessToken as AccessTokenView, OAuthError @@ -116,13 +117,22 @@ class AccessTokenView(AccessTokenView): ) def invalidate_grant(self, grant): - grant.expires = now() - timedelta(days=1) - grant.save() + if getattr(settings, 'OAUTH_CLEAN_EXPIRED', False): + grant.delete() + else: + grant.expires = now() - timedelta(days=1) + grant.save() def invalidate_refresh_token(self, rt): - rt.expired = True - rt.save() + if getattr(settings, 'OAUTH_CLEAN_EXPIRED', False): + rt.delete() + else: + rt.expired = True + rt.save() def invalidate_access_token(self, at): - at.expires = now() - timedelta(days=1) - at.save() + if getattr(settings, 'OAUTH_CLEAN_EXPIRED', False): + at.delete() + else: + at.expires = now() - timedelta(days=1) + at.save() From 0c0236b389a0a3827573c091be39d07fac725eda Mon Sep 17 00:00:00 2001 From: Evan Culver Date: Tue, 5 Nov 2013 15:17:44 -0800 Subject: [PATCH 2/2] Use `constants` instead of going directly through settings when invalidating tokens and grants. This also replaces 'clean' with 'delete' for the new setting to be more clear as to what's being done under the hood. --- docs/api.rst | 14 +++++++++++--- provider/constants.py | 4 ++++ provider/oauth2/models.py | 2 +- provider/oauth2/tests.py | 14 ++++---------- provider/oauth2/views.py | 8 ++++---- provider/views.py | 1 + 6 files changed, 25 insertions(+), 18 deletions(-) diff --git a/docs/api.rst b/docs/api.rst index ff35369..3d10d5e 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -28,7 +28,7 @@ :settings: `OAUTH_EXPIRE_DELTA` :default: `datetime.timedelta(days=365)` - + The time to expiry for access tokens as outlined in :rfc:`4.2.2` and :rfc:`5.1`. @@ -36,9 +36,17 @@ :settings: `OAUTH_EXPIRE_CODE_DELTA` :default: `datetime.timedelta(seconds=10*60)` - + The time to expiry for an authorization code grant as outlined in :rfc:`4.1.2`. - + +.. attribute:: DELETE_EXPIRED + + :settings: `OAUTH_DELETE_EXPIRED` + :default: `False` + + To remove expired tokens immediately instead of letting them persist, set + to `True`. + .. attribute:: ENFORCE_SECURE :settings: `OAUTH_ENFORCE_SECURE` diff --git a/provider/constants.py b/provider/constants.py index 82587ab..7f94b8e 100644 --- a/provider/constants.py +++ b/provider/constants.py @@ -26,11 +26,15 @@ DEFAULT_SCOPES = ( SCOPES = getattr(settings, 'OAUTH_SCOPES', DEFAULT_SCOPES) EXPIRE_DELTA = getattr(settings, 'OAUTH_EXPIRE_DELTA', timedelta(days=365)) + # Expiry delta for public clients (which typically have shorter lived tokens) EXPIRE_DELTA_PUBLIC = getattr(settings, 'OAUTH_EXPIRE_DELTA_PUBLIC', timedelta(days=30)) EXPIRE_CODE_DELTA = getattr(settings, 'OAUTH_EXPIRE_CODE_DELTA', timedelta(seconds=10 * 60)) +# Remove expired tokens immediately instead of letting them persist. +DELETE_EXPIRED = getattr(settings, 'OAUTH_DELETE_EXPIRED', False) + ENFORCE_SECURE = getattr(settings, 'OAUTH_ENFORCE_SECURE', False) ENFORCE_CLIENT_SECURE = getattr(settings, 'OAUTH_ENFORCE_CLIENT_SECURE', True) diff --git a/provider/oauth2/models.py b/provider/oauth2/models.py index d6e6f8a..e4ab484 100644 --- a/provider/oauth2/models.py +++ b/provider/oauth2/models.py @@ -7,7 +7,7 @@ views in :attr:`provider.views`. from django.db import models from django.conf import settings from .. import constants -from ..constants import CLIENT_TYPES +from ..constants import CLIENT_TYPES, DELETE_EXPIRED from ..utils import short_token, long_token, get_token_expiry from ..utils import get_code_expiry from ..utils import now diff --git a/provider/oauth2/tests.py b/provider/oauth2/tests.py index 988af23..247f66e 100644 --- a/provider/oauth2/tests.py +++ b/provider/oauth2/tests.py @@ -533,19 +533,15 @@ class ScopeTest(TestCase): self.assertEqual('read read+write write', ' '.join(names)) -class CleanExpiredTest(BaseOAuth2TestCase): +class DeleteExpiredTest(BaseOAuth2TestCase): fixtures = ['test_oauth2'] def setUp(self): - self._old_oauth_clean_expired = getattr(settings, - 'OAUTH_CLEAN_EXPIRED', None) - settings.OAUTH_CLEAN_EXPIRED = True + self._delete_expired = constants.DELETE_EXPIRED + constants.DELETE_EXPIRED = True def tearDown(self): - if self._old_oauth_clean_expired is not None: - settings.OAUTH_CLEAN_EXPIRED = self._old_oauth_clean_expired - else: - delattr(settings, 'OAUTH_CLEAN_EXPIRED') + constants.DELETE_EXPIRED = self._delete_expired def test_clear_expired(self): self.login() @@ -563,8 +559,6 @@ class CleanExpiredTest(BaseOAuth2TestCase): code = urlparse.parse_qs(location)['code'][0] self.assertTrue(Grant.objects.filter(code=code).exists()) - from pprint import pprint - # use the code/grant response = self.client.post(self.access_token_url(), { 'grant_type': 'authorization_code', diff --git a/provider/oauth2/views.py b/provider/oauth2/views.py index fa9fbb1..d0de51d 100644 --- a/provider/oauth2/views.py +++ b/provider/oauth2/views.py @@ -1,6 +1,6 @@ from datetime import timedelta -from django.conf import settings from django.core.urlresolvers import reverse +from .. import constants from ..views import Capture, Authorize, Redirect from ..views import AccessToken as AccessTokenView, OAuthError from ..utils import now @@ -117,21 +117,21 @@ class AccessTokenView(AccessTokenView): ) def invalidate_grant(self, grant): - if getattr(settings, 'OAUTH_CLEAN_EXPIRED', False): + if constants.DELETE_EXPIRED: grant.delete() else: grant.expires = now() - timedelta(days=1) grant.save() def invalidate_refresh_token(self, rt): - if getattr(settings, 'OAUTH_CLEAN_EXPIRED', False): + if constants.DELETE_EXPIRED: rt.delete() else: rt.expired = True rt.save() def invalidate_access_token(self, at): - if getattr(settings, 'OAUTH_CLEAN_EXPIRED', False): + if constants.DELETE_EXPIRED: at.delete() else: at.expires = now() - timedelta(days=1) diff --git a/provider/views.py b/provider/views.py index 7c2a266..3b0bb1d 100644 --- a/provider/views.py +++ b/provider/views.py @@ -492,6 +492,7 @@ class AccessToken(OAuthView, Mixin): """ rt = self.get_refresh_token_grant(request, data, client) + # this must be called first in case we need to purge expired tokens self.invalidate_refresh_token(rt) self.invalidate_access_token(rt.access_token)