Skip to content

Commit

Permalink
Revoke refresh tokens instead of deleting them
Browse files Browse the repository at this point in the history
To handle the case where a refresh token is used to generate a new access
token but the new access/refresh token is never received, allow a refresh
token to be re-used within a grace period, returning the same access token
(appearing as if the request/response was 'cached').

This now revokes the token with a timestamp instead of deleting it.

Refs #497
  • Loading branch information
phillbaker authored and jleclanche committed Feb 16, 2018
1 parent 107e5ea commit 2e4d15e
Show file tree
Hide file tree
Showing 8 changed files with 216 additions and 31 deletions.
10 changes: 10 additions & 0 deletions docs/settings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,16 @@ REFRESH_TOKEN_EXPIRE_SECONDS
The number of seconds before a refresh token gets removed from the database by
the ``cleartokens`` management command. Check :ref:`cleartokens` management command for further info.

REFRESH_TOKEN_GRACE_PERIOD_SECONDS
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The number of seconds between when a refresh token is first used when it is
expired. The most common case of this for this is native mobile applications
that run into issues of network connectivity during the refresh cycle and are
unable to complete the full request/response life cycle. Without a grace
period the application, the app then has only a consumed refresh token and the
only recourse is to have the user re-authenticate. A suggested value, if this
is enabled, is 2 minutes.

REFRESH_TOKEN_MODEL
~~~~~~~~~~~~~~~~~~~
The import string of the class (model) representing your refresh tokens. Overwrite
Expand Down
44 changes: 44 additions & 0 deletions oauth2_provider/migrations/0006_auto_20171214_2232.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.1 on 2017-05-14 11:41
from __future__ import unicode_literals

from oauth2_provider.settings import oauth2_settings
from django.conf import settings
from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
('oauth2_provider', '0005_auto_20170514_1141'),
]

operations = [
migrations.AddField(
model_name='accesstoken',
name='source_refresh_token',
field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to=oauth2_settings.REFRESH_TOKEN_MODEL, related_name='refreshed_access_token'),
preserve_default=False,
),
migrations.AddField(
model_name='refreshtoken',
name='revoked',
field=models.DateTimeField(null=True, default=django.utils.timezone.now),
preserve_default=False,
),
migrations.AlterField(
model_name='refreshtoken',
name='token',
field=models.CharField(max_length=255),
),
migrations.AlterField(
model_name='refreshtoken',
name='access_token',
field=models.OneToOneField(blank=True, null=True, related_name='refresh_token', to=oauth2_settings.ACCESS_TOKEN_MODEL, on_delete=models.SET_NULL),
),
migrations.AlterUniqueTogether(
name='refreshtoken',
unique_together=set([('token', 'revoked')]),
),
]
24 changes: 17 additions & 7 deletions oauth2_provider/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ class AbstractAccessToken(models.Model):
Fields:
* :attr:`user` The Django user representing resources' owner
* :attr:`source_refresh_token` If from a refresh, the consumed RefeshToken
* :attr:`token` Access token
* :attr:`application` Application instance
* :attr:`expires` Date and time of token expiration, in DateTime format
Expand All @@ -231,6 +232,11 @@ class AbstractAccessToken(models.Model):
settings.AUTH_USER_MODEL, on_delete=models.CASCADE, blank=True, null=True,
related_name="%(app_label)s_%(class)s"
)
source_refresh_token = models.OneToOneField(
# unique=True implied by the OneToOneField
oauth2_settings.REFRESH_TOKEN_MODEL, on_delete=models.SET_NULL, blank=True, null=True,
related_name="refreshed_access_token"
)
token = models.CharField(max_length=255, unique=True, )
application = models.ForeignKey(
oauth2_settings.APPLICATION_MODEL, on_delete=models.CASCADE, blank=True, null=True,
Expand Down Expand Up @@ -313,38 +319,41 @@ class AbstractRefreshToken(models.Model):
* :attr:`application` Application instance
* :attr:`access_token` AccessToken instance this refresh token is
bounded to
* :attr:`revoked` Timestamp of when this refresh token was revoked
"""
id = models.BigAutoField(primary_key=True)
user = models.ForeignKey(
settings.AUTH_USER_MODEL, on_delete=models.CASCADE,
related_name="%(app_label)s_%(class)s"
)
token = models.CharField(max_length=255, unique=True)
token = models.CharField(max_length=255)
application = models.ForeignKey(
oauth2_settings.APPLICATION_MODEL, on_delete=models.CASCADE)
access_token = models.OneToOneField(
oauth2_settings.ACCESS_TOKEN_MODEL, on_delete=models.CASCADE,
oauth2_settings.ACCESS_TOKEN_MODEL, on_delete=models.SET_NULL, blank=True, null=True,
related_name="refresh_token"
)

created = models.DateTimeField(auto_now_add=True)
updated = models.DateTimeField(auto_now=True)
revoked = models.DateTimeField(null=True)

def revoke(self):
"""
Delete this refresh token along with related access token
Mark this refresh token revoked and revoke related access token
"""
access_token_model = get_access_token_model()
token = access_token_model.objects.get(id=self.access_token.id)
# Avoid cascade by deleting self first.
self.delete()
token.revoke()
access_token_model.objects.get(id=self.access_token_id).revoke()
self.access_token = None
self.revoked = timezone.now()
self.save()

def __str__(self):
return self.token

class Meta:
abstract = True
unique_together = ("token", "revoked",)


class RefreshToken(AbstractRefreshToken):
Expand Down Expand Up @@ -390,6 +399,7 @@ def clear_expired():

with transaction.atomic():
if refresh_expire_at:
refresh_token_model.objects.filter(revoked__lt=refresh_expire_at).delete()
refresh_token_model.objects.filter(access_token__expires__lt=refresh_expire_at).delete()
access_token_model.objects.filter(refresh_token__isnull=True, expires__lt=now).delete()
grant_model.objects.filter(expires__lt=now).delete()
72 changes: 51 additions & 21 deletions oauth2_provider/oauth2_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from django.contrib.auth import authenticate, get_user_model
from django.core.exceptions import ObjectDoesNotExist
from django.db import transaction
from django.db.models import Q
from django.utils import timezone
from django.utils.timezone import make_aware
from oauthlib.oauth2 import RequestValidator
Expand Down Expand Up @@ -449,24 +450,43 @@ def save_bearer_token(self, token, request, *args, **kwargs):

# else create fresh with access & refresh tokens
else:
# revoke existing tokens if possible
# revoke existing tokens if possible to allow reuse of grant
if isinstance(refresh_token_instance, RefreshToken):
previous_access_token = AccessToken.objects.filter(
source_refresh_token=refresh_token_instance
).first()
try:
refresh_token_instance.revoke()
except (AccessToken.DoesNotExist, RefreshToken.DoesNotExist):
pass
else:
setattr(request, "refresh_token_instance", None)
else:
previous_access_token = None

# If the refresh token has already been used to create an
# access token (ie it's within the grace period), return that
# access token
if not previous_access_token:
access_token = self._create_access_token(
expires,
request,
token,
source_refresh_token=refresh_token_instance,
)

access_token = self._create_access_token(expires, request, token)

refresh_token = RefreshToken(
user=request.user,
token=refresh_token_code,
application=request.client,
access_token=access_token
)
refresh_token.save()
refresh_token = RefreshToken(
user=request.user,
token=refresh_token_code,
application=request.client,
access_token=access_token
)
refresh_token.save()
else:
# make sure that the token data we're returning matches
# the existing token
token["access_token"] = previous_access_token.token
token["scope"] = previous_access_token.scope

# No refresh token should be created, just access token
else:
Expand All @@ -475,13 +495,14 @@ def save_bearer_token(self, token, request, *args, **kwargs):
# TODO: check out a more reliable way to communicate expire time to oauthlib
token["expires_in"] = oauth2_settings.ACCESS_TOKEN_EXPIRE_SECONDS

def _create_access_token(self, expires, request, token):
def _create_access_token(self, expires, request, token, source_refresh_token=None):
access_token = AccessToken(
user=request.user,
scope=token["scope"],
expires=expires,
token=token["access_token"],
application=request.client
application=request.client,
source_refresh_token=source_refresh_token,
)
access_token.save()
return access_token
Expand Down Expand Up @@ -524,20 +545,29 @@ def get_original_scopes(self, refresh_token, request, *args, **kwargs):
# Avoid second query for RefreshToken since this method is invoked *after*
# validate_refresh_token.
rt = request.refresh_token_instance
if not rt.access_token_id:
return AccessToken.objects.get(source_refresh_token_id=rt.id).scope

return rt.access_token.scope

def validate_refresh_token(self, refresh_token, client, request, *args, **kwargs):
"""
Check refresh_token exists and refers to the right client.
Also attach User instance to the request object
"""
try:
rt = RefreshToken.objects.get(token=refresh_token)
request.user = rt.user
request.refresh_token = rt.token
# Temporary store RefreshToken instance to be reused by get_original_scopes.
request.refresh_token_instance = rt
return rt.application == client

except RefreshToken.DoesNotExist:

null_or_recent = Q(revoked__isnull=True) | Q(
revoked__gt=timezone.now() - timedelta(
seconds=oauth2_settings.REFRESH_TOKEN_GRACE_PERIOD_SECONDS
)
)
rt = RefreshToken.objects.filter(null_or_recent, token=refresh_token).first()

if not rt:
return False

request.user = rt.user
request.refresh_token = rt.token
# Temporary store RefreshToken instance to be reused by get_original_scopes and save_bearer_token.
request.refresh_token_instance = rt
return rt.application == client
1 change: 1 addition & 0 deletions oauth2_provider/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
"AUTHORIZATION_CODE_EXPIRE_SECONDS": 60,
"ACCESS_TOKEN_EXPIRE_SECONDS": 36000,
"REFRESH_TOKEN_EXPIRE_SECONDS": None,
"REFRESH_TOKEN_GRACE_PERIOD_SECONDS": 0,
"ROTATE_REFRESH_TOKEN": True,
"ERROR_RESPONSE_WITH_SCOPES": False,
"APPLICATION_MODEL": APPLICATION_MODEL,
Expand Down
91 changes: 90 additions & 1 deletion tests/test_authorization_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,54 @@ def test_refresh(self):
content = json.loads(response.content.decode("utf-8"))
self.assertTrue("invalid_grant" in content.values())

def test_refresh_with_grace_period(self):
"""
Request an access token using a refresh token
"""
oauth2_settings.REFRESH_TOKEN_GRACE_PERIOD_SECONDS = 120
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.org"
}
auth_headers = 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"))
self.assertTrue("refresh_token" in content)

# make a second token request to be sure the previous refresh token remains valid, see #65
authorization_code = self.get_auth()
token_request_data = {
"grant_type": "authorization_code",
"code": authorization_code,
"redirect_uri": "http://example.org"
}
response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data, **auth_headers)

token_request_data = {
"grant_type": "refresh_token",
"refresh_token": content["refresh_token"],
"scope": content["scope"],
}
response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data, **auth_headers)
self.assertEqual(response.status_code, 200)

content = json.loads(response.content.decode("utf-8"))
self.assertTrue("access_token" in content)
first_access_token = content["access_token"]

# check refresh token returns same data if used twice, see #497
response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data, **auth_headers)
self.assertEqual(response.status_code, 200)
content = json.loads(response.content.decode("utf-8"))
self.assertTrue("access_token" in content)
self.assertEqual(content["access_token"], first_access_token)
oauth2_settings.REFRESH_TOKEN_GRACE_PERIOD_SECONDS = 0

def test_refresh_invalidates_old_tokens(self):
"""
Ensure existing refresh tokens are cleaned up when issuing new ones
Expand Down Expand Up @@ -608,7 +656,8 @@ def test_refresh_invalidates_old_tokens(self):
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())
refresh_token = RefreshToken.objects.filter(token=rt).first()
self.assertIsNotNone(refresh_token.revoked)
self.assertFalse(AccessToken.objects.filter(token=at).exists())

def test_refresh_no_scopes(self):
Expand Down Expand Up @@ -693,6 +742,46 @@ def test_refresh_fail_repeating_requests(self):
response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data, **auth_headers)
self.assertEqual(response.status_code, 401)

def test_refresh_repeating_requests(self):
"""
Trying to refresh an access token with the same refresh token more than
once succeeds in the grace period and fails outside
"""
oauth2_settings.REFRESH_TOKEN_GRACE_PERIOD_SECONDS = 120
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.org"
}
auth_headers = 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"))
self.assertTrue("refresh_token" in content)

token_request_data = {
"grant_type": "refresh_token",
"refresh_token": content["refresh_token"],
"scope": content["scope"],
}
response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data, **auth_headers)
self.assertEqual(response.status_code, 200)
response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data, **auth_headers)
self.assertEqual(response.status_code, 200)

# try refreshing outside the refresh window, see #497
rt = RefreshToken.objects.get(token=content["refresh_token"])
self.assertIsNotNone(rt.revoked)
rt.revoked = timezone.now() - datetime.timedelta(minutes=10) # instead of mocking out datetime
rt.save()

response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data, **auth_headers)
self.assertEqual(response.status_code, 401)
oauth2_settings.REFRESH_TOKEN_GRACE_PERIOD_SECONDS = 0

def test_refresh_repeating_requests_non_rotating_tokens(self):
"""
Try refreshing an access token with the same refresh token more than once when not rotating tokens.
Expand Down
2 changes: 1 addition & 1 deletion tests/test_oauth2_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ def test_save_bearer_token__with_new_token_equal_to_existing_token__revokes_old_

self.validator.save_bearer_token(token, self.request)

self.assertEqual(1, RefreshToken.objects.count())
self.assertEqual(1, RefreshToken.objects.filter(revoked__isnull=True).count())
self.assertEqual(1, AccessToken.objects.count())

def test_save_bearer_token__with_no_refresh_token__creates_new_access_token_only(self):
Expand Down
3 changes: 2 additions & 1 deletion tests/test_token_revocation.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ def test_revoke_refresh_token(self):
url = "{url}?{qs}".format(url=reverse("oauth2_provider:revoke-token"), qs=query_string)
response = self.client.post(url)
self.assertEqual(response.status_code, 200)
self.assertFalse(RefreshToken.objects.filter(id=rtok.id).exists())
refresh_token = RefreshToken.objects.filter(id=rtok.id).first()
self.assertIsNotNone(refresh_token.revoked)
self.assertFalse(AccessToken.objects.filter(id=rtok.access_token.id).exists())

def test_revoke_token_with_wrong_hint(self):
Expand Down

1 comment on commit 2e4d15e

@amirRamirfatahi
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this in the latest release of project?

Please sign in to comment.