diff --git a/apps/accounts/serializers.py b/apps/accounts/serializers.py index 591616b57f..7b2d85fe3c 100644 --- a/apps/accounts/serializers.py +++ b/apps/accounts/serializers.py @@ -5,11 +5,16 @@ from participants.utils import ( has_participated_in_require_complete_profile_challenge, ) -from rest_auth.serializers import PasswordResetSerializer +from rest_auth.serializers import ( + PasswordChangeSerializer, + PasswordResetConfirmSerializer, + PasswordResetSerializer, +) from rest_framework import serializers from rest_framework.exceptions import ValidationError from .models import JwtToken, Profile +from .token_revocation import revoke_all_user_tokens LOCKED_PROFILE_FIELDS = [ "first_name", @@ -337,6 +342,26 @@ def create(self, validated_data): return email_address +class CustomPasswordChangeSerializer(PasswordChangeSerializer): + """ + Change password and revoke all existing API/JWT credentials. + """ + + def save(self): + self.set_password_form.save() + revoke_all_user_tokens(self.user) + + +class CustomPasswordResetConfirmSerializer(PasswordResetConfirmSerializer): + """ + Reset password from email link and revoke all existing credentials. + """ + + def save(self): + self.set_password_form.save() + revoke_all_user_tokens(self.user) + + class CustomPasswordResetSerializer(PasswordResetSerializer): """ Send password reset emails only for eligible accounts while always diff --git a/apps/accounts/token_revocation.py b/apps/accounts/token_revocation.py new file mode 100644 index 0000000000..60300dd7e3 --- /dev/null +++ b/apps/accounts/token_revocation.py @@ -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) diff --git a/frontend/src/js/controllers/changePwdCtrl.js b/frontend/src/js/controllers/changePwdCtrl.js index 8878eb6b3f..6a951cdefb 100644 --- a/frontend/src/js/controllers/changePwdCtrl.js +++ b/frontend/src/js/controllers/changePwdCtrl.js @@ -71,10 +71,11 @@ parameters.callback = { onSuccess: function() { vm.user.error = false; - $rootScope.notify("success", "Your password has been changed successfully!"); + $rootScope.notify("success", "Your password has been changed successfully! Please log in again."); vm.stopLoader(); - // navigate to challenge page - // $state.go('web.challenge-page.overview'); + utilities.resetStorage(); + $rootScope.isAuth = false; + $state.go('auth.login'); }, onError: function(response) { vm.user.error = "Failed"; diff --git a/frontend/src/js/controllers/profileCtrl.js b/frontend/src/js/controllers/profileCtrl.js index e81edba97f..58cf1870e6 100644 --- a/frontend/src/js/controllers/profileCtrl.js +++ b/frontend/src/js/controllers/profileCtrl.js @@ -376,8 +376,10 @@ parameters.callback = { onSuccess: function() { vm.user.error = false; - $rootScope.notify("success", "Your password has been changed successfully!"); - $state.go('web.profile.AuthToken'); + $rootScope.notify("success", "Your password has been changed successfully! Please log in again."); + utilities.resetStorage(); + $rootScope.isAuth = false; + $state.go('auth.login'); }, onError: function(response) { vm.user.error = "Failed"; diff --git a/frontend/tests/controllers-test/changePwdCtrl.test.js b/frontend/tests/controllers-test/changePwdCtrl.test.js index b46599888e..74a937e168 100644 --- a/frontend/tests/controllers-test/changePwdCtrl.test.js +++ b/frontend/tests/controllers-test/changePwdCtrl.test.js @@ -104,11 +104,16 @@ describe('Unit tests for change password controller', function () { var resetconfirmFormValid = true; success = true; $state.params.user_id = 1; + spyOn(utilities, 'resetStorage'); + spyOn($state, 'go'); vm.changePassword(resetconfirmFormValid); expect(vm.user.error).toBeFalsy(); - expect($rootScope.notify).toHaveBeenCalledWith("success", "Your password has been changed successfully!"); + expect($rootScope.notify).toHaveBeenCalledWith("success", "Your password has been changed successfully! Please log in again."); expect(vm.stopLoader).toHaveBeenCalled(); + expect(utilities.resetStorage).toHaveBeenCalled(); + expect($rootScope.isAuth).toBeFalsy(); + expect($state.go).toHaveBeenCalledWith('auth.login'); }); it('when old password is valid', function () { diff --git a/frontend/tests/controllers-test/profileCtrl.test.js b/frontend/tests/controllers-test/profileCtrl.test.js index 187a871294..a88efdd85e 100644 --- a/frontend/tests/controllers-test/profileCtrl.test.js +++ b/frontend/tests/controllers-test/profileCtrl.test.js @@ -572,14 +572,17 @@ describe('Unit tests for profile controller', function () { $state.params = { user_id: 1 }; }); - it('should call notify and go to AuthToken on success', function () { + it('should log out and redirect to login on success', function () { spyOn(utilities, 'sendRequest').and.callFake(function (params) { params.callback.onSuccess(); }); + spyOn(utilities, 'resetStorage'); vm.changePassword(true); expect(vm.user.error).toBe(false); - expect($rootScope.notify).toHaveBeenCalledWith("success", "Your password has been changed successfully!"); - expect($state.go).toHaveBeenCalledWith('web.profile.AuthToken'); + expect($rootScope.notify).toHaveBeenCalledWith("success", "Your password has been changed successfully! Please log in again."); + expect(utilities.resetStorage).toHaveBeenCalled(); + expect($rootScope.isAuth).toBe(false); + expect($state.go).toHaveBeenCalledWith('auth.login'); }); it('should handle old_password error', function () { diff --git a/settings/common.py b/settings/common.py index 48506de9f3..50b05b4e1c 100755 --- a/settings/common.py +++ b/settings/common.py @@ -342,6 +342,12 @@ REST_AUTH_SERIALIZERS = { "USER_DETAILS_SERIALIZER": "accounts.serializers.ProfileSerializer", "PASSWORD_RESET_SERIALIZER": "accounts.serializers.CustomPasswordResetSerializer", + "PASSWORD_CHANGE_SERIALIZER": ( + "accounts.serializers.CustomPasswordChangeSerializer" + ), + "PASSWORD_RESET_CONFIRM_SERIALIZER": ( + "accounts.serializers.CustomPasswordResetConfirmSerializer" + ), } # Default email for sending emails.. diff --git a/tests/unit/accounts/test_token_revocation.py b/tests/unit/accounts/test_token_revocation.py new file mode 100644 index 0000000000..937d7bcf8f --- /dev/null +++ b/tests/unit/accounts/test_token_revocation.py @@ -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", + ) + + 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"))