Invalidate auth tokens on password change and reset #5128
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| from rest_framework_expiring_authtoken.models import ExpiringToken | ||
| from rest_framework_simplejwt.exceptions import TokenError | ||
| from rest_framework_simplejwt.token_blacklist.models import ( | ||
| BlacklistedToken, | ||
| OutstandingToken, | ||
| ) | ||
| from rest_framework_simplejwt.tokens import RefreshToken | ||
|
|
||
| from .models import JwtToken | ||
|
|
||
|
|
||
| def revoke_all_user_tokens(user): | ||
| """ | ||
| Invalidate every active credential for a user. | ||
|
|
||
| Called after password change or reset so existing browser sessions, | ||
| API tokens, and JWT refresh tokens cannot keep working. | ||
| """ | ||
| ExpiringToken.objects.filter(user=user).delete() | ||
| JwtToken.objects.filter(user=user).delete() | ||
|
|
||
| for outstanding_token in OutstandingToken.objects.filter(user=user): | ||
| if BlacklistedToken.objects.filter(token=outstanding_token).exists(): | ||
| continue | ||
| try: | ||
| RefreshToken(outstanding_token.token).blacklist() | ||
| except TokenError: | ||
| BlacklistedToken.objects.get_or_create(token=outstanding_token) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,159 @@ | ||
| from allauth.account.models import EmailAddress | ||
| from django.contrib.auth.models import User | ||
| from django.contrib.auth.tokens import default_token_generator | ||
| from django.urls import reverse_lazy | ||
| from django.utils.encoding import force_bytes | ||
| from django.utils.http import urlsafe_base64_encode | ||
| from rest_framework import status | ||
| from rest_framework.test import APIClient, APITestCase | ||
| from rest_framework_expiring_authtoken.models import ExpiringToken | ||
| from rest_framework_simplejwt.exceptions import TokenError | ||
| from rest_framework_simplejwt.token_blacklist.models import ( | ||
| BlacklistedToken, | ||
| OutstandingToken, | ||
| ) | ||
| from rest_framework_simplejwt.tokens import RefreshToken | ||
|
|
||
| from accounts.models import JwtToken | ||
| from accounts.token_revocation import revoke_all_user_tokens | ||
|
|
||
|
|
||
| class BaseAPITestClass(APITestCase): | ||
| def setUp(self): | ||
| self.client = APIClient(enforce_csrf_checks=True) | ||
|
|
||
| self.user = User.objects.create( | ||
| username="someuser", | ||
| email="user@test.com", | ||
| password="secret_password", | ||
| ) | ||
|
Comment on lines
+25
to
+29
There was a problem hiding this comment. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the relevant test file and nearby context
git ls-files tests/unit/accounts/test_token_revocation.py
wc -l tests/unit/accounts/test_token_revocation.py
sed -n '1,220p' tests/unit/accounts/test_token_revocation.py
# Find the user model / auth helpers used in this area
rg -n "create_user|set_password|check_password|old_password|token revocation|revocation" tests src . -g '!**/node_modules/**'Repository: Cloud-CV/EvalAI Length of output: 21870 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the password-change / reset serializers and any helper they use.
sed -n '300,390p' apps/accounts/serializers.py
# Inspect the token revocation helper for the side effects the tests assert.
sed -n '1,220p' apps/accounts/token_revocation.py
# Inspect the model used to store JWT tokens.
sed -n '1,220p' apps/accounts/models.pyRepository: Cloud-CV/EvalAI Length of output: 7547 Create the fixture user with a hashed password. 🧰 Tools🪛 Ruff (0.15.18)[error] 28-28: Possible hardcoded password assigned to argument: "password" (S106) 🤖 Prompt for AI Agents |
||
|
|
||
| EmailAddress.objects.create( | ||
| user=self.user, email="user@test.com", primary=True, verified=True | ||
| ) | ||
|
|
||
|
|
||
| class RevokeAllUserTokensTest(BaseAPITestClass): | ||
| def test_revoke_all_user_tokens_deletes_api_token(self): | ||
| api_token = ExpiringToken.objects.create(user=self.user) | ||
|
|
||
| revoke_all_user_tokens(self.user) | ||
|
|
||
| self.assertFalse(ExpiringToken.objects.filter(pk=api_token.pk).exists()) | ||
|
|
||
| def test_revoke_all_user_tokens_blacklists_jwt_and_deletes_record(self): | ||
| refresh = RefreshToken.for_user(self.user) | ||
| jwt_token = JwtToken.objects.create( | ||
| user=self.user, | ||
| refresh_token=str(refresh), | ||
| access_token=str(refresh.access_token), | ||
| ) | ||
| outstanding_token = OutstandingToken.objects.filter(user=self.user).latest( | ||
| "created_at" | ||
| ) | ||
|
|
||
| revoke_all_user_tokens(self.user) | ||
|
|
||
| self.assertFalse(JwtToken.objects.filter(pk=jwt_token.pk).exists()) | ||
| self.assertTrue( | ||
| BlacklistedToken.objects.filter(token=outstanding_token).exists() | ||
| ) | ||
| with self.assertRaises(TokenError): | ||
| RefreshToken(str(refresh)) | ||
|
|
||
|
|
||
| class PasswordChangeTokenRevocationTest(BaseAPITestClass): | ||
| url = reverse_lazy("rest_password_change") | ||
|
|
||
| def setUp(self): | ||
| super().setUp() | ||
| self.api_token = ExpiringToken.objects.create(user=self.user) | ||
| self.client.credentials(HTTP_AUTHORIZATION="Token {}".format(self.api_token.key)) | ||
|
|
||
| def test_password_change_revokes_existing_api_token(self): | ||
| response = self.client.post( | ||
| "/api/auth/password/change/", | ||
| { | ||
| "old_password": "secret_password", | ||
| "new_password1": "new_secret_password", | ||
| "new_password2": "new_secret_password", | ||
| }, | ||
| format="json", | ||
| ) | ||
|
|
||
| self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
| self.assertFalse( | ||
| ExpiringToken.objects.filter(pk=self.api_token.pk).exists() | ||
| ) | ||
| self.user.refresh_from_db() | ||
| self.assertTrue(self.user.check_password("new_secret_password")) | ||
|
|
||
| def test_password_change_revokes_jwt_credentials(self): | ||
| refresh = RefreshToken.for_user(self.user) | ||
| JwtToken.objects.create( | ||
| user=self.user, | ||
| refresh_token=str(refresh), | ||
| access_token=str(refresh.access_token), | ||
| ) | ||
| outstanding_token = OutstandingToken.objects.filter(user=self.user).latest( | ||
| "created_at" | ||
| ) | ||
|
|
||
| response = self.client.post( | ||
| "/api/auth/password/change/", | ||
| { | ||
| "old_password": "secret_password", | ||
| "new_password1": "new_secret_password", | ||
| "new_password2": "new_secret_password", | ||
| }, | ||
| format="json", | ||
| ) | ||
|
|
||
| self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
| self.assertFalse(JwtToken.objects.filter(user=self.user).exists()) | ||
| self.assertTrue( | ||
| BlacklistedToken.objects.filter(token=outstanding_token).exists() | ||
| ) | ||
|
|
||
|
|
||
| class PasswordResetConfirmTokenRevocationTest(BaseAPITestClass): | ||
| url = reverse_lazy("rest_password_reset_confirm") | ||
|
|
||
| def setUp(self): | ||
| super().setUp() | ||
| self.api_token = ExpiringToken.objects.create(user=self.user) | ||
| self.uid = urlsafe_base64_encode(force_bytes(self.user.pk)) | ||
| self.token = default_token_generator.make_token(self.user) | ||
|
|
||
| def test_password_reset_confirm_revokes_existing_credentials(self): | ||
| refresh = RefreshToken.for_user(self.user) | ||
| JwtToken.objects.create( | ||
| user=self.user, | ||
| refresh_token=str(refresh), | ||
| access_token=str(refresh.access_token), | ||
| ) | ||
| outstanding_token = OutstandingToken.objects.filter(user=self.user).latest( | ||
| "created_at" | ||
| ) | ||
|
|
||
| response = self.client.post( | ||
| "/api/auth/password/reset/confirm/", | ||
| { | ||
| "uid": self.uid, | ||
| "token": self.token, | ||
| "new_password1": "reset_secret_password", | ||
| "new_password2": "reset_secret_password", | ||
| }, | ||
| format="json", | ||
| ) | ||
|
|
||
| self.assertEqual(response.status_code, status.HTTP_200_OK) | ||
| self.assertFalse( | ||
| ExpiringToken.objects.filter(pk=self.api_token.pk).exists() | ||
| ) | ||
| self.assertFalse(JwtToken.objects.filter(user=self.user).exists()) | ||
| self.assertTrue( | ||
| BlacklistedToken.objects.filter(token=outstanding_token).exists() | ||
| ) | ||
| self.user.refresh_from_db() | ||
| self.assertTrue(self.user.check_password("reset_secret_password")) | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: Cloud-CV/EvalAI
Length of output: 153
🏁 Script executed:
Repository: Cloud-CV/EvalAI
Length of output: 4480
🏁 Script executed:
Repository: Cloud-CV/EvalAI
Length of output: 8491
Wrap password save/revocation in a transaction.
apps/accounts/serializers.py:350-362Both serializers commit the new password beforerevoke_all_user_tokens(). If any revocation write fails, the password change can persist while old credentials remain usable.🤖 Prompt for AI Agents