Skip to content

Commit 2e4d15e

Browse files
phillbakerjleclanche
authored andcommitted
Revoke refresh tokens instead of deleting them
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
1 parent 107e5ea commit 2e4d15e

File tree

8 files changed

+216
-31
lines changed

8 files changed

+216
-31
lines changed

docs/settings.rst

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,16 @@ REFRESH_TOKEN_EXPIRE_SECONDS
100100
The number of seconds before a refresh token gets removed from the database by
101101
the ``cleartokens`` management command. Check :ref:`cleartokens` management command for further info.
102102

103+
REFRESH_TOKEN_GRACE_PERIOD_SECONDS
104+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
105+
The number of seconds between when a refresh token is first used when it is
106+
expired. The most common case of this for this is native mobile applications
107+
that run into issues of network connectivity during the refresh cycle and are
108+
unable to complete the full request/response life cycle. Without a grace
109+
period the application, the app then has only a consumed refresh token and the
110+
only recourse is to have the user re-authenticate. A suggested value, if this
111+
is enabled, is 2 minutes.
112+
103113
REFRESH_TOKEN_MODEL
104114
~~~~~~~~~~~~~~~~~~~
105115
The import string of the class (model) representing your refresh tokens. Overwrite
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# -*- coding: utf-8 -*-
2+
# Generated by Django 1.11.1 on 2017-05-14 11:41
3+
from __future__ import unicode_literals
4+
5+
from oauth2_provider.settings import oauth2_settings
6+
from django.conf import settings
7+
from django.db import migrations, models
8+
import django.db.models.deletion
9+
10+
11+
class Migration(migrations.Migration):
12+
13+
dependencies = [
14+
('oauth2_provider', '0005_auto_20170514_1141'),
15+
]
16+
17+
operations = [
18+
migrations.AddField(
19+
model_name='accesstoken',
20+
name='source_refresh_token',
21+
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'),
22+
preserve_default=False,
23+
),
24+
migrations.AddField(
25+
model_name='refreshtoken',
26+
name='revoked',
27+
field=models.DateTimeField(null=True, default=django.utils.timezone.now),
28+
preserve_default=False,
29+
),
30+
migrations.AlterField(
31+
model_name='refreshtoken',
32+
name='token',
33+
field=models.CharField(max_length=255),
34+
),
35+
migrations.AlterField(
36+
model_name='refreshtoken',
37+
name='access_token',
38+
field=models.OneToOneField(blank=True, null=True, related_name='refresh_token', to=oauth2_settings.ACCESS_TOKEN_MODEL, on_delete=models.SET_NULL),
39+
),
40+
migrations.AlterUniqueTogether(
41+
name='refreshtoken',
42+
unique_together=set([('token', 'revoked')]),
43+
),
44+
]

oauth2_provider/models.py

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ class AbstractAccessToken(models.Model):
221221
Fields:
222222
223223
* :attr:`user` The Django user representing resources' owner
224+
* :attr:`source_refresh_token` If from a refresh, the consumed RefeshToken
224225
* :attr:`token` Access token
225226
* :attr:`application` Application instance
226227
* :attr:`expires` Date and time of token expiration, in DateTime format
@@ -231,6 +232,11 @@ class AbstractAccessToken(models.Model):
231232
settings.AUTH_USER_MODEL, on_delete=models.CASCADE, blank=True, null=True,
232233
related_name="%(app_label)s_%(class)s"
233234
)
235+
source_refresh_token = models.OneToOneField(
236+
# unique=True implied by the OneToOneField
237+
oauth2_settings.REFRESH_TOKEN_MODEL, on_delete=models.SET_NULL, blank=True, null=True,
238+
related_name="refreshed_access_token"
239+
)
234240
token = models.CharField(max_length=255, unique=True, )
235241
application = models.ForeignKey(
236242
oauth2_settings.APPLICATION_MODEL, on_delete=models.CASCADE, blank=True, null=True,
@@ -313,38 +319,41 @@ class AbstractRefreshToken(models.Model):
313319
* :attr:`application` Application instance
314320
* :attr:`access_token` AccessToken instance this refresh token is
315321
bounded to
322+
* :attr:`revoked` Timestamp of when this refresh token was revoked
316323
"""
317324
id = models.BigAutoField(primary_key=True)
318325
user = models.ForeignKey(
319326
settings.AUTH_USER_MODEL, on_delete=models.CASCADE,
320327
related_name="%(app_label)s_%(class)s"
321328
)
322-
token = models.CharField(max_length=255, unique=True)
329+
token = models.CharField(max_length=255)
323330
application = models.ForeignKey(
324331
oauth2_settings.APPLICATION_MODEL, on_delete=models.CASCADE)
325332
access_token = models.OneToOneField(
326-
oauth2_settings.ACCESS_TOKEN_MODEL, on_delete=models.CASCADE,
333+
oauth2_settings.ACCESS_TOKEN_MODEL, on_delete=models.SET_NULL, blank=True, null=True,
327334
related_name="refresh_token"
328335
)
329336

330337
created = models.DateTimeField(auto_now_add=True)
331338
updated = models.DateTimeField(auto_now=True)
339+
revoked = models.DateTimeField(null=True)
332340

333341
def revoke(self):
334342
"""
335-
Delete this refresh token along with related access token
343+
Mark this refresh token revoked and revoke related access token
336344
"""
337345
access_token_model = get_access_token_model()
338-
token = access_token_model.objects.get(id=self.access_token.id)
339-
# Avoid cascade by deleting self first.
340-
self.delete()
341-
token.revoke()
346+
access_token_model.objects.get(id=self.access_token_id).revoke()
347+
self.access_token = None
348+
self.revoked = timezone.now()
349+
self.save()
342350

343351
def __str__(self):
344352
return self.token
345353

346354
class Meta:
347355
abstract = True
356+
unique_together = ("token", "revoked",)
348357

349358

350359
class RefreshToken(AbstractRefreshToken):
@@ -390,6 +399,7 @@ def clear_expired():
390399

391400
with transaction.atomic():
392401
if refresh_expire_at:
402+
refresh_token_model.objects.filter(revoked__lt=refresh_expire_at).delete()
393403
refresh_token_model.objects.filter(access_token__expires__lt=refresh_expire_at).delete()
394404
access_token_model.objects.filter(refresh_token__isnull=True, expires__lt=now).delete()
395405
grant_model.objects.filter(expires__lt=now).delete()

oauth2_provider/oauth2_validators.py

Lines changed: 51 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from django.contrib.auth import authenticate, get_user_model
1111
from django.core.exceptions import ObjectDoesNotExist
1212
from django.db import transaction
13+
from django.db.models import Q
1314
from django.utils import timezone
1415
from django.utils.timezone import make_aware
1516
from oauthlib.oauth2 import RequestValidator
@@ -449,24 +450,43 @@ def save_bearer_token(self, token, request, *args, **kwargs):
449450

450451
# else create fresh with access & refresh tokens
451452
else:
452-
# revoke existing tokens if possible
453+
# revoke existing tokens if possible to allow reuse of grant
453454
if isinstance(refresh_token_instance, RefreshToken):
455+
previous_access_token = AccessToken.objects.filter(
456+
source_refresh_token=refresh_token_instance
457+
).first()
454458
try:
455459
refresh_token_instance.revoke()
456460
except (AccessToken.DoesNotExist, RefreshToken.DoesNotExist):
457461
pass
458462
else:
459463
setattr(request, "refresh_token_instance", None)
464+
else:
465+
previous_access_token = None
466+
467+
# If the refresh token has already been used to create an
468+
# access token (ie it's within the grace period), return that
469+
# access token
470+
if not previous_access_token:
471+
access_token = self._create_access_token(
472+
expires,
473+
request,
474+
token,
475+
source_refresh_token=refresh_token_instance,
476+
)
460477

461-
access_token = self._create_access_token(expires, request, token)
462-
463-
refresh_token = RefreshToken(
464-
user=request.user,
465-
token=refresh_token_code,
466-
application=request.client,
467-
access_token=access_token
468-
)
469-
refresh_token.save()
478+
refresh_token = RefreshToken(
479+
user=request.user,
480+
token=refresh_token_code,
481+
application=request.client,
482+
access_token=access_token
483+
)
484+
refresh_token.save()
485+
else:
486+
# make sure that the token data we're returning matches
487+
# the existing token
488+
token["access_token"] = previous_access_token.token
489+
token["scope"] = previous_access_token.scope
470490

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

478-
def _create_access_token(self, expires, request, token):
498+
def _create_access_token(self, expires, request, token, source_refresh_token=None):
479499
access_token = AccessToken(
480500
user=request.user,
481501
scope=token["scope"],
482502
expires=expires,
483503
token=token["access_token"],
484-
application=request.client
504+
application=request.client,
505+
source_refresh_token=source_refresh_token,
485506
)
486507
access_token.save()
487508
return access_token
@@ -524,20 +545,29 @@ def get_original_scopes(self, refresh_token, request, *args, **kwargs):
524545
# Avoid second query for RefreshToken since this method is invoked *after*
525546
# validate_refresh_token.
526547
rt = request.refresh_token_instance
548+
if not rt.access_token_id:
549+
return AccessToken.objects.get(source_refresh_token_id=rt.id).scope
550+
527551
return rt.access_token.scope
528552

529553
def validate_refresh_token(self, refresh_token, client, request, *args, **kwargs):
530554
"""
531555
Check refresh_token exists and refers to the right client.
532556
Also attach User instance to the request object
533557
"""
534-
try:
535-
rt = RefreshToken.objects.get(token=refresh_token)
536-
request.user = rt.user
537-
request.refresh_token = rt.token
538-
# Temporary store RefreshToken instance to be reused by get_original_scopes.
539-
request.refresh_token_instance = rt
540-
return rt.application == client
541-
542-
except RefreshToken.DoesNotExist:
558+
559+
null_or_recent = Q(revoked__isnull=True) | Q(
560+
revoked__gt=timezone.now() - timedelta(
561+
seconds=oauth2_settings.REFRESH_TOKEN_GRACE_PERIOD_SECONDS
562+
)
563+
)
564+
rt = RefreshToken.objects.filter(null_or_recent, token=refresh_token).first()
565+
566+
if not rt:
543567
return False
568+
569+
request.user = rt.user
570+
request.refresh_token = rt.token
571+
# Temporary store RefreshToken instance to be reused by get_original_scopes and save_bearer_token.
572+
request.refresh_token_instance = rt
573+
return rt.application == client

oauth2_provider/settings.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
"AUTHORIZATION_CODE_EXPIRE_SECONDS": 60,
4646
"ACCESS_TOKEN_EXPIRE_SECONDS": 36000,
4747
"REFRESH_TOKEN_EXPIRE_SECONDS": None,
48+
"REFRESH_TOKEN_GRACE_PERIOD_SECONDS": 0,
4849
"ROTATE_REFRESH_TOKEN": True,
4950
"ERROR_RESPONSE_WITH_SCOPES": False,
5051
"APPLICATION_MODEL": APPLICATION_MODEL,

tests/test_authorization_code.py

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -580,6 +580,54 @@ def test_refresh(self):
580580
content = json.loads(response.content.decode("utf-8"))
581581
self.assertTrue("invalid_grant" in content.values())
582582

583+
def test_refresh_with_grace_period(self):
584+
"""
585+
Request an access token using a refresh token
586+
"""
587+
oauth2_settings.REFRESH_TOKEN_GRACE_PERIOD_SECONDS = 120
588+
self.client.login(username="test_user", password="123456")
589+
authorization_code = self.get_auth()
590+
591+
token_request_data = {
592+
"grant_type": "authorization_code",
593+
"code": authorization_code,
594+
"redirect_uri": "http://example.org"
595+
}
596+
auth_headers = get_basic_auth_header(self.application.client_id, self.application.client_secret)
597+
598+
response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data, **auth_headers)
599+
content = json.loads(response.content.decode("utf-8"))
600+
self.assertTrue("refresh_token" in content)
601+
602+
# make a second token request to be sure the previous refresh token remains valid, see #65
603+
authorization_code = self.get_auth()
604+
token_request_data = {
605+
"grant_type": "authorization_code",
606+
"code": authorization_code,
607+
"redirect_uri": "http://example.org"
608+
}
609+
response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data, **auth_headers)
610+
611+
token_request_data = {
612+
"grant_type": "refresh_token",
613+
"refresh_token": content["refresh_token"],
614+
"scope": content["scope"],
615+
}
616+
response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data, **auth_headers)
617+
self.assertEqual(response.status_code, 200)
618+
619+
content = json.loads(response.content.decode("utf-8"))
620+
self.assertTrue("access_token" in content)
621+
first_access_token = content["access_token"]
622+
623+
# check refresh token returns same data if used twice, see #497
624+
response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data, **auth_headers)
625+
self.assertEqual(response.status_code, 200)
626+
content = json.loads(response.content.decode("utf-8"))
627+
self.assertTrue("access_token" in content)
628+
self.assertEqual(content["access_token"], first_access_token)
629+
oauth2_settings.REFRESH_TOKEN_GRACE_PERIOD_SECONDS = 0
630+
583631
def test_refresh_invalidates_old_tokens(self):
584632
"""
585633
Ensure existing refresh tokens are cleaned up when issuing new ones
@@ -608,7 +656,8 @@ def test_refresh_invalidates_old_tokens(self):
608656
response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data, **auth_headers)
609657
self.assertEqual(response.status_code, 200)
610658

611-
self.assertFalse(RefreshToken.objects.filter(token=rt).exists())
659+
refresh_token = RefreshToken.objects.filter(token=rt).first()
660+
self.assertIsNotNone(refresh_token.revoked)
612661
self.assertFalse(AccessToken.objects.filter(token=at).exists())
613662

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

745+
def test_refresh_repeating_requests(self):
746+
"""
747+
Trying to refresh an access token with the same refresh token more than
748+
once succeeds in the grace period and fails outside
749+
"""
750+
oauth2_settings.REFRESH_TOKEN_GRACE_PERIOD_SECONDS = 120
751+
self.client.login(username="test_user", password="123456")
752+
authorization_code = self.get_auth()
753+
754+
token_request_data = {
755+
"grant_type": "authorization_code",
756+
"code": authorization_code,
757+
"redirect_uri": "http://example.org"
758+
}
759+
auth_headers = get_basic_auth_header(self.application.client_id, self.application.client_secret)
760+
761+
response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data, **auth_headers)
762+
content = json.loads(response.content.decode("utf-8"))
763+
self.assertTrue("refresh_token" in content)
764+
765+
token_request_data = {
766+
"grant_type": "refresh_token",
767+
"refresh_token": content["refresh_token"],
768+
"scope": content["scope"],
769+
}
770+
response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data, **auth_headers)
771+
self.assertEqual(response.status_code, 200)
772+
response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data, **auth_headers)
773+
self.assertEqual(response.status_code, 200)
774+
775+
# try refreshing outside the refresh window, see #497
776+
rt = RefreshToken.objects.get(token=content["refresh_token"])
777+
self.assertIsNotNone(rt.revoked)
778+
rt.revoked = timezone.now() - datetime.timedelta(minutes=10) # instead of mocking out datetime
779+
rt.save()
780+
781+
response = self.client.post(reverse("oauth2_provider:token"), data=token_request_data, **auth_headers)
782+
self.assertEqual(response.status_code, 401)
783+
oauth2_settings.REFRESH_TOKEN_GRACE_PERIOD_SECONDS = 0
784+
696785
def test_refresh_repeating_requests_non_rotating_tokens(self):
697786
"""
698787
Try refreshing an access token with the same refresh token more than once when not rotating tokens.

tests/test_oauth2_validators.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ def test_save_bearer_token__with_new_token_equal_to_existing_token__revokes_old_
236236

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

239-
self.assertEqual(1, RefreshToken.objects.count())
239+
self.assertEqual(1, RefreshToken.objects.filter(revoked__isnull=True).count())
240240
self.assertEqual(1, AccessToken.objects.count())
241241

242242
def test_save_bearer_token__with_no_refresh_token__creates_new_access_token_only(self):

tests/test_token_revocation.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,8 @@ def test_revoke_refresh_token(self):
149149
url = "{url}?{qs}".format(url=reverse("oauth2_provider:revoke-token"), qs=query_string)
150150
response = self.client.post(url)
151151
self.assertEqual(response.status_code, 200)
152-
self.assertFalse(RefreshToken.objects.filter(id=rtok.id).exists())
152+
refresh_token = RefreshToken.objects.filter(id=rtok.id).first()
153+
self.assertIsNotNone(refresh_token.revoked)
153154
self.assertFalse(AccessToken.objects.filter(id=rtok.access_token.id).exists())
154155

155156
def test_revoke_token_with_wrong_hint(self):

0 commit comments

Comments
 (0)