Skip to content

Commit 4d6e4fd

Browse files
feat(auth): Revoke refresh token on password change (#928)
* feat(auth): Revoke refresh token on password change * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * refactor(serializers): Correct validation order in TokenRefreshSerializer * refactor: centralize password changed error messages in error dictionaries * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * feat(serializers): Add full user validation to sliding token refresh Implements the same user validation logic (active status, password change) in to ensure consistent behavior with the standard . * refactor: Inline password hash comparison in serializers Simplifies the conditional check by removing temporary variables for the token hash and user password hash. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * BREAKING: return 401 AuthenticationFailed instead of 404 DoesNotExist for missing users --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent 930cf85 commit 4d6e4fd

File tree

4 files changed

+215
-12
lines changed

4 files changed

+215
-12
lines changed

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
## [Unreleased]
2+
3+
### Changed
4+
- **BREAKING:** In `serializers.py`, when a user linked to a token is missing or deleted, the code now raises `AuthenticationFailed("no_active_account")` instead of allowing `DoesNotExist` to propagate.
5+
- Response changed from **404 Not Found****401 Unauthorized**.
6+
- Improves security by not leaking whether a user/token exists.
7+
- Follows RFC 7235, where authentication failures should return 401.
8+
- Clearer for clients: signals an auth issue instead of suggesting the endpoint is missing.
9+
10+
111
## 5.5.1
212

313
Missing Migration for rest_framework_simplejwt.token_blacklist app. A previously missing migration (0013_blacklist) has now been added. This issue arose because the migration file was mistakenly not generated earlier. This migration was never part of an official release, but users following the latest master branch may have encountered it.

rest_framework_simplejwt/authentication.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ class JWTAuthentication(authentication.BaseAuthentication):
3333
www_authenticate_realm = "api"
3434
media_type = "application/json"
3535

36+
default_error_messages = {
37+
"password_changed": _("The user's password has been changed."),
38+
}
39+
3640
def __init__(self, *args, **kwargs) -> None:
3741
super().__init__(*args, **kwargs)
3842
self.user_model = get_user_model()
@@ -143,7 +147,8 @@ def get_user(self, validated_token: Token) -> AuthUser:
143147
api_settings.REVOKE_TOKEN_CLAIM
144148
) != get_md5_hash_password(user.password):
145149
raise AuthenticationFailed(
146-
_("The user's password has been changed."), code="password_changed"
150+
self.default_error_messages["password_changed"],
151+
code="password_changed",
147152
)
148153

149154
return user

rest_framework_simplejwt/serializers.py

Lines changed: 76 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from .models import TokenUser
1111
from .settings import api_settings
1212
from .tokens import RefreshToken, SlidingToken, Token, UntypedToken
13+
from .utils import get_md5_hash_password
1314

1415
AuthUser = TypeVar("AuthUser", AbstractBaseUser, TokenUser)
1516

@@ -105,27 +106,51 @@ class TokenRefreshSerializer(serializers.Serializer):
105106
token_class = RefreshToken
106107

107108
default_error_messages = {
108-
"no_active_account": _("No active account found for the given token.")
109+
"no_active_account": _("No active account found for the given token."),
110+
"password_changed": _("The user's password has been changed."),
109111
}
110112

111113
def validate(self, attrs: dict[str, Any]) -> dict[str, str]:
112114
refresh = self.token_class(attrs["refresh"])
113115

114116
user_id = refresh.payload.get(api_settings.USER_ID_CLAIM, None)
115117
if user_id:
116-
user = (
117-
get_user_model()
118-
.objects.filter(**{api_settings.USER_ID_FIELD: user_id})
119-
.first()
120-
)
121-
if not user or not api_settings.USER_AUTHENTICATION_RULE(user):
118+
try:
119+
user = get_user_model().objects.get(
120+
**{api_settings.USER_ID_FIELD: user_id}
121+
)
122+
except get_user_model().DoesNotExist:
123+
# This handles the case where the user has been deleted.
122124
raise AuthenticationFailed(
123-
self.error_messages["no_active_account"],
124-
"no_active_account",
125+
self.error_messages["no_active_account"], "no_active_account"
125126
)
126127

127-
data = {"access": str(refresh.access_token)}
128+
if not api_settings.USER_AUTHENTICATION_RULE(user):
129+
raise AuthenticationFailed(
130+
self.error_messages["no_active_account"], "no_active_account"
131+
)
128132

133+
if api_settings.CHECK_REVOKE_TOKEN:
134+
if refresh.payload.get(
135+
api_settings.REVOKE_TOKEN_CLAIM
136+
) != get_md5_hash_password(user.password):
137+
# If the password has changed, we blacklist the token
138+
# to prevent any further use.
139+
if (
140+
"rest_framework_simplejwt.token_blacklist"
141+
in settings.INSTALLED_APPS
142+
):
143+
try:
144+
refresh.blacklist()
145+
except AttributeError:
146+
pass
147+
148+
raise AuthenticationFailed(
149+
self.error_messages["password_changed"],
150+
code="password_changed",
151+
)
152+
153+
data = {"access": str(refresh.access_token)}
129154
if api_settings.ROTATE_REFRESH_TOKENS:
130155
if api_settings.BLACKLIST_AFTER_ROTATION:
131156
try:
@@ -150,8 +175,49 @@ class TokenRefreshSlidingSerializer(serializers.Serializer):
150175
token = serializers.CharField()
151176
token_class = SlidingToken
152177

178+
default_error_messages = {
179+
"no_active_account": _("No active account found for the given token."),
180+
"password_changed": _("The user's password has been changed."),
181+
}
182+
153183
def validate(self, attrs: dict[str, Any]) -> dict[str, str]:
154184
token = self.token_class(attrs["token"])
185+
user_id = token.payload.get(api_settings.USER_ID_CLAIM, None)
186+
if user_id:
187+
try:
188+
user = get_user_model().objects.get(
189+
**{api_settings.USER_ID_FIELD: user_id}
190+
)
191+
except get_user_model().DoesNotExist:
192+
# This handles the case where the user has been deleted.
193+
raise AuthenticationFailed(
194+
self.error_messages["no_active_account"], "no_active_account"
195+
)
196+
197+
if not api_settings.USER_AUTHENTICATION_RULE(user):
198+
raise AuthenticationFailed(
199+
self.error_messages["no_active_account"], "no_active_account"
200+
)
201+
202+
if api_settings.CHECK_REVOKE_TOKEN:
203+
if token.payload.get(
204+
api_settings.REVOKE_TOKEN_CLAIM
205+
) != get_md5_hash_password(user.password):
206+
# If the password has changed, we blacklist the token
207+
# to prevent any further use.
208+
if (
209+
"rest_framework_simplejwt.token_blacklist"
210+
in settings.INSTALLED_APPS
211+
):
212+
try:
213+
token.blacklist()
214+
except AttributeError:
215+
pass
216+
217+
raise AuthenticationFailed(
218+
self.error_messages["password_changed"],
219+
code="password_changed",
220+
)
155221

156222
# Check that the timestamp in the "refresh_exp" claim has not
157223
# passed

tests/test_serializers.py

Lines changed: 123 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,15 @@ def test_it_should_produce_a_json_web_token_when_valid(self):
187187

188188

189189
class TestTokenRefreshSlidingSerializer(TestCase):
190+
def setUp(self):
191+
self.username = "test_user"
192+
self.password = "test_password"
193+
194+
self.user = User.objects.create_user(
195+
username=self.username,
196+
password=self.password,
197+
)
198+
190199
def test_it_should_not_validate_if_token_invalid(self):
191200
token = SlidingToken()
192201
del token["exp"]
@@ -268,6 +277,74 @@ def test_it_should_update_token_exp_claim_if_everything_ok(self):
268277

269278
self.assertTrue(old_exp < new_exp)
270279

280+
def test_it_should_raise_error_for_deleted_users(self):
281+
token = SlidingToken.for_user(self.user)
282+
self.user.delete()
283+
284+
s = TokenRefreshSlidingSerializer(data={"token": str(token)})
285+
286+
# It should raise AuthenticationFailed instead of ObjectDoesNotExist
287+
with self.assertRaises(drf_exceptions.AuthenticationFailed) as e:
288+
s.is_valid()
289+
290+
self.assertEqual(e.exception.get_codes(), "no_active_account")
291+
292+
def test_it_should_raise_error_for_inactive_users(self):
293+
token = SlidingToken.for_user(self.user)
294+
self.user.is_active = False
295+
self.user.save()
296+
297+
s = TokenRefreshSlidingSerializer(data={"token": str(token)})
298+
299+
with self.assertRaises(drf_exceptions.AuthenticationFailed) as e:
300+
s.is_valid()
301+
302+
self.assertEqual(e.exception.get_codes(), "no_active_account")
303+
304+
@override_api_settings(
305+
CHECK_REVOKE_TOKEN=True,
306+
REVOKE_TOKEN_CLAIM="hash_password",
307+
BLACKLIST_AFTER_ROTATION=False,
308+
)
309+
def test_sliding_token_should_fail_after_password_change(self):
310+
"""
311+
Tests that sliding token refresh fails if CHECK_REVOKE_TOKEN is True and the
312+
user's password has changed.
313+
"""
314+
token = SlidingToken.for_user(self.user)
315+
self.user.set_password("new_password")
316+
self.user.save()
317+
318+
s = TokenRefreshSlidingSerializer(data={"token": str(token)})
319+
320+
with self.assertRaises(drf_exceptions.AuthenticationFailed) as e:
321+
s.is_valid(raise_exception=True)
322+
323+
self.assertEqual(e.exception.get_codes(), "password_changed")
324+
325+
@override_api_settings(
326+
CHECK_REVOKE_TOKEN=True,
327+
REVOKE_TOKEN_CLAIM="hash_password",
328+
BLACKLIST_AFTER_ROTATION=True,
329+
)
330+
def test_sliding_token_should_blacklist_after_password_change(self):
331+
"""
332+
Tests that if sliding token refresh fails due to a password change, the
333+
offending token is blacklisted.
334+
"""
335+
token = SlidingToken.for_user(self.user)
336+
self.user.set_password("new_password")
337+
self.user.save()
338+
339+
s = TokenRefreshSlidingSerializer(data={"token": str(token)})
340+
with self.assertRaises(drf_exceptions.AuthenticationFailed):
341+
s.is_valid(raise_exception=True)
342+
343+
# Check that the token is now in the blacklist
344+
jti = token[api_settings.JTI_CLAIM]
345+
self.assertTrue(OutstandingToken.objects.filter(jti=jti).exists())
346+
self.assertTrue(BlacklistedToken.objects.filter(token__jti=jti).exists())
347+
271348

272349
class TestTokenRefreshSerializer(TestCase):
273350
def setUp(self):
@@ -285,10 +362,11 @@ def test_it_should_raise_error_for_deleted_users(self):
285362

286363
s = TokenRefreshSerializer(data={"refresh": str(refresh)})
287364

365+
# It should raise AuthenticationFailed instead of ObjectDoesNotExist
288366
with self.assertRaises(drf_exceptions.AuthenticationFailed) as e:
289367
s.is_valid()
290368

291-
self.assertIn("No active account", str(e.exception))
369+
self.assertEqual(e.exception.get_codes(), "no_active_account")
292370

293371
def test_it_should_raise_error_for_inactive_users(self):
294372
refresh = RefreshToken.for_user(self.user)
@@ -482,6 +560,50 @@ def test_blacklist_app_not_installed_should_pass(self):
482560
reload(tokens)
483561
reload(serializers)
484562

563+
@override_api_settings(
564+
CHECK_REVOKE_TOKEN=True,
565+
REVOKE_TOKEN_CLAIM="hash_password",
566+
BLACKLIST_AFTER_ROTATION=False,
567+
)
568+
def test_refresh_token_should_fail_after_password_change(self):
569+
"""
570+
Tests that token refresh fails if CHECK_REVOKE_TOKEN is True and the
571+
user's password has changed.
572+
"""
573+
refresh = RefreshToken.for_user(self.user)
574+
self.user.set_password("new_password")
575+
self.user.save()
576+
577+
s = TokenRefreshSerializer(data={"refresh": str(refresh)})
578+
579+
with self.assertRaises(drf_exceptions.AuthenticationFailed) as e:
580+
s.is_valid(raise_exception=True)
581+
582+
self.assertEqual(e.exception.get_codes(), "password_changed")
583+
584+
@override_api_settings(
585+
CHECK_REVOKE_TOKEN=True,
586+
REVOKE_TOKEN_CLAIM="hash_password",
587+
BLACKLIST_AFTER_ROTATION=True,
588+
)
589+
def test_refresh_token_should_blacklist_after_password_change(self):
590+
"""
591+
Tests that if token refresh fails due to a password change, the
592+
offending refresh token is blacklisted.
593+
"""
594+
refresh = RefreshToken.for_user(self.user)
595+
self.user.set_password("new_password")
596+
self.user.save()
597+
598+
s = TokenRefreshSerializer(data={"refresh": str(refresh)})
599+
with self.assertRaises(drf_exceptions.AuthenticationFailed):
600+
s.is_valid(raise_exception=True)
601+
602+
# Check that the token is now in the blacklist
603+
jti = refresh[api_settings.JTI_CLAIM]
604+
self.assertTrue(OutstandingToken.objects.filter(jti=jti).exists())
605+
self.assertTrue(BlacklistedToken.objects.filter(token__jti=jti).exists())
606+
485607

486608
class TestTokenVerifySerializer(TestCase):
487609
def test_it_should_raise_token_error_if_token_invalid(self):

0 commit comments

Comments
 (0)