From 26b6e220437b3900e96303a9e80e1034f6b3688e Mon Sep 17 00:00:00 2001 From: Marc LaBelle Date: Wed, 1 Apr 2020 18:54:16 -0400 Subject: [PATCH 1/9] blacklist refresh token on logout if REST_USE_JWT and added .idea to gitignore --- .gitignore | 3 +++ dj_rest_auth/views.py | 20 +++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 84cc2de..136132c 100644 --- a/.gitignore +++ b/.gitignore @@ -72,6 +72,9 @@ target/ # Jupyter Notebook .ipynb_checkpoints +# IDE +.idea + # pyenv .python-version diff --git a/dj_rest_auth/views.py b/dj_rest_auth/views.py index c5ba7fc..7f2e651 100644 --- a/dj_rest_auth/views.py +++ b/dj_rest_auth/views.py @@ -11,6 +11,8 @@ from rest_framework.generics import GenericAPIView, RetrieveUpdateAPIView from rest_framework.permissions import AllowAny, IsAuthenticated from rest_framework.response import Response from rest_framework.views import APIView +from rest_framework_simplejwt.exceptions import TokenError +from rest_framework_simplejwt.tokens import RefreshToken from .app_settings import (JWTSerializer, LoginSerializer, PasswordChangeSerializer, @@ -134,13 +136,29 @@ class LogoutView(APIView): pass if getattr(settings, 'REST_SESSION_LOGIN', True): django_logout(request) - response = Response({"detail": _("Successfully logged out.")}, status=status.HTTP_200_OK) if getattr(settings, 'REST_USE_JWT', False): cookie_name = getattr(settings, 'JWT_AUTH_COOKIE', None) if cookie_name: response.delete_cookie(cookie_name) + # add refresh token to blacklist + try: + token = RefreshToken(request.data['refresh']) + token.blacklist() + except KeyError: + response = Response({"detail": _("Refresh token was not included.")}, + status=status.HTTP_401_UNAUTHORIZED) + except TokenError as e: + if e.args[0] == 'Token is blacklisted': + response = Response({"detail": _("Token is already blacklisted.")}, + status=status.HTTP_404_NOT_FOUND) + except AttributeError as e: + # warn user blacklist is not enabled if not using JWT_AUTH_COOKIE + if not cookie_name: + if e.args[0] == "'RefreshToken' object has no attribute 'blacklist'": + response = Response({"detail": _("Blacklist is not enabled in INSTALLED_APPS.")}, + status=status.HTTP_501_NOT_IMPLEMENTED) return response From 241011a353e0a3f4b7a62890bda71deb803c78bc Mon Sep 17 00:00:00 2001 From: Marc LaBelle Date: Wed, 1 Apr 2020 18:56:41 -0400 Subject: [PATCH 2/9] attempt to blacklist token if no JWT_AUTH_COOKIE is found --- dj_rest_auth/views.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/dj_rest_auth/views.py b/dj_rest_auth/views.py index 7f2e651..365545e 100644 --- a/dj_rest_auth/views.py +++ b/dj_rest_auth/views.py @@ -142,20 +142,20 @@ class LogoutView(APIView): cookie_name = getattr(settings, 'JWT_AUTH_COOKIE', None) if cookie_name: response.delete_cookie(cookie_name) - # add refresh token to blacklist - try: - token = RefreshToken(request.data['refresh']) - token.blacklist() - except KeyError: - response = Response({"detail": _("Refresh token was not included.")}, - status=status.HTTP_401_UNAUTHORIZED) - except TokenError as e: - if e.args[0] == 'Token is blacklisted': - response = Response({"detail": _("Token is already blacklisted.")}, - status=status.HTTP_404_NOT_FOUND) - except AttributeError as e: - # warn user blacklist is not enabled if not using JWT_AUTH_COOKIE - if not cookie_name: + else: + # add refresh token to blacklist + try: + token = RefreshToken(request.data['refresh']) + token.blacklist() + except KeyError: + response = Response({"detail": _("Refresh token was not included.")}, + status=status.HTTP_401_UNAUTHORIZED) + except TokenError as e: + if e.args[0] == 'Token is blacklisted': + response = Response({"detail": _("Token is already blacklisted.")}, + status=status.HTTP_404_NOT_FOUND) + except AttributeError as e: + # warn user blacklist is not enabled if e.args[0] == "'RefreshToken' object has no attribute 'blacklist'": response = Response({"detail": _("Blacklist is not enabled in INSTALLED_APPS.")}, status=status.HTTP_501_NOT_IMPLEMENTED) From aaab91f82bb81c74f8df99c274983f5be27cfefc Mon Sep 17 00:00:00 2001 From: Marc LaBelle Date: Wed, 1 Apr 2020 21:28:02 -0400 Subject: [PATCH 3/9] updated exceptions to raise error if not not handled --- dj_rest_auth/tests/test_api.py | 11 +++++++++++ dj_rest_auth/views.py | 14 +++++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/dj_rest_auth/tests/test_api.py b/dj_rest_auth/tests/test_api.py index 0134560..f373dcc 100644 --- a/dj_rest_auth/tests/test_api.py +++ b/dj_rest_auth/tests/test_api.py @@ -555,3 +555,14 @@ class APIBasicTests(TestsMixin, TestCase): self.assertEqual(['jwt-auth'], list(resp.cookies.keys())) resp = self.get('/protected-view/') self.assertEquals(resp.status_code, 200) + + @override_settings(REST_USE_JWT=True) + def test_blacklisting(self): + payload = { + "username": self.USERNAME, + "password": self.PASS + } + get_user_model().objects.create_user(self.USERNAME, '', self.PASS) + self.post(self.login_url, data=payload, status_code=200) + resp = self.post(self.logout_url, status=200) + pass diff --git a/dj_rest_auth/views.py b/dj_rest_auth/views.py index 365545e..c79423f 100644 --- a/dj_rest_auth/views.py +++ b/dj_rest_auth/views.py @@ -147,18 +147,26 @@ class LogoutView(APIView): try: token = RefreshToken(request.data['refresh']) token.blacklist() + except KeyError: - response = Response({"detail": _("Refresh token was not included.")}, + response = Response({"detail": _("Refresh token was not included in request data.")}, status=status.HTTP_401_UNAUTHORIZED) + except TokenError as e: - if e.args[0] == 'Token is blacklisted': + if hasattr(e, 'args') and 'Token is blacklisted' in e.args: response = Response({"detail": _("Token is already blacklisted.")}, status=status.HTTP_404_NOT_FOUND) + else: + raise + except AttributeError as e: # warn user blacklist is not enabled - if e.args[0] == "'RefreshToken' object has no attribute 'blacklist'": + if hasattr(e, 'args') and "'RefreshToken' object has no attribute 'blacklist'" in e.args: response = Response({"detail": _("Blacklist is not enabled in INSTALLED_APPS.")}, status=status.HTTP_501_NOT_IMPLEMENTED) + else: + raise + return response From 8f97cbc6179675bd14a587c8c471ff574d446710 Mon Sep 17 00:00:00 2001 From: Marc LaBelle Date: Thu, 2 Apr 2020 10:01:07 -0400 Subject: [PATCH 4/9] added rest_framework_simplejwt.token_blacklist to settings for tests, return 500 if error occurs instead of raising, added unit tests for blacklist --- Makefile | 2 ++ dj_rest_auth/tests/settings.py | 2 ++ dj_rest_auth/tests/test_api.py | 34 ++++++++++++++++++++++++++++++---- dj_rest_auth/views.py | 28 +++++++++++++++------------- 4 files changed, 49 insertions(+), 17 deletions(-) create mode 100644 Makefile diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..103ee60 --- /dev/null +++ b/Makefile @@ -0,0 +1,2 @@ +test: + coverage run --source=dj_rest_auth setup.py test diff --git a/dj_rest_auth/tests/settings.py b/dj_rest_auth/tests/settings.py index 3d02309..5f57f91 100644 --- a/dj_rest_auth/tests/settings.py +++ b/dj_rest_auth/tests/settings.py @@ -94,6 +94,8 @@ INSTALLED_APPS = [ 'dj_rest_auth', 'dj_rest_auth.registration', + + 'rest_framework_simplejwt.token_blacklist' ] SECRET_KEY = "38dh*skf8sjfhs287dh&^hd8&3hdg*j2&sd" diff --git a/dj_rest_auth/tests/test_api.py b/dj_rest_auth/tests/test_api.py index f373dcc..0e5a3c6 100644 --- a/dj_rest_auth/tests/test_api.py +++ b/dj_rest_auth/tests/test_api.py @@ -1,6 +1,7 @@ +import json +from unittest.mock import patch + from allauth.account import app_settings as account_app_settings -from dj_rest_auth.registration.app_settings import register_permission_classes -from dj_rest_auth.registration.views import RegisterView from django.conf import settings from django.contrib.auth import get_user_model from django.core import mail @@ -9,6 +10,8 @@ from django.utils.encoding import force_text from rest_framework import status from rest_framework.test import APIRequestFactory +from dj_rest_auth.registration.app_settings import register_permission_classes +from dj_rest_auth.registration.views import RegisterView from .mixins import CustomPermissionClass, TestsMixin try: @@ -556,6 +559,20 @@ class APIBasicTests(TestsMixin, TestCase): resp = self.get('/protected-view/') self.assertEquals(resp.status_code, 200) + @override_settings(REST_USE_JWT=True) + @patch('rest_framework_simplejwt.tokens.BlacklistMixin.blacklist') + def test_blacklisting_not_installed(self, mocked_blacklist): + mocked_blacklist.side_effect = AttributeError(f"'RefreshToken' object has no attribute 'blacklist'") + payload = { + "username": self.USERNAME, + "password": self.PASS + } + get_user_model().objects.create_user(self.USERNAME, '', self.PASS) + resp = self.post(self.login_url, data=payload, status_code=200) + token = resp.data['refresh_token'] + resp = self.post(self.logout_url, status=200, data={'refresh': token}) + self.assertEqual(resp.status_code, 501) + @override_settings(REST_USE_JWT=True) def test_blacklisting(self): payload = { @@ -563,6 +580,15 @@ class APIBasicTests(TestsMixin, TestCase): "password": self.PASS } get_user_model().objects.create_user(self.USERNAME, '', self.PASS) - self.post(self.login_url, data=payload, status_code=200) + resp = self.post(self.login_url, data=payload, status_code=200) + token = resp.data['refresh_token'] resp = self.post(self.logout_url, status=200) - pass + self.assertEqual(resp.status_code, 401) + resp = self.post(self.logout_url, status=200, data={'refresh': '1'}) + self.assertEqual(resp.status_code, 404) + resp = self.post(self.logout_url, status=200, data={'refresh': token}) + self.assertEqual(resp.status_code, 200) + resp = self.post(self.logout_url, status=200, data={'refresh': token}) + self.assertEqual(resp.status_code, 404) + resp = self.post(self.logout_url, status=200, data=json.dumps({'refresh': token})) + self.assertEqual(resp.status_code, 500) diff --git a/dj_rest_auth/views.py b/dj_rest_auth/views.py index c79423f..f3bb3fe 100644 --- a/dj_rest_auth/views.py +++ b/dj_rest_auth/views.py @@ -147,25 +147,27 @@ class LogoutView(APIView): try: token = RefreshToken(request.data['refresh']) token.blacklist() - except KeyError: response = Response({"detail": _("Refresh token was not included in request data.")}, status=status.HTTP_401_UNAUTHORIZED) - except TokenError as e: - if hasattr(e, 'args') and 'Token is blacklisted' in e.args: - response = Response({"detail": _("Token is already blacklisted.")}, - status=status.HTTP_404_NOT_FOUND) - else: - raise + except (TokenError, AttributeError, TypeError) as error: + if hasattr(error, 'args'): + if 'Token is blacklisted' in error.args or 'Token is invalid or expired' in error.args: + response = Response({"detail": _(error.args[0])}, + status=status.HTTP_404_NOT_FOUND) + + # warn user blacklist is not enabled + elif "'RefreshToken' object has no attribute 'blacklist'" in error.args: + response = Response({"detail": _("Blacklist is not enabled in INSTALLED_APPS.")}, + status=status.HTTP_501_NOT_IMPLEMENTED) + else: + response = Response({"detail": _("An error has occurred.")}, + status=status.HTTP_500_INTERNAL_SERVER_ERROR) - except AttributeError as e: - # warn user blacklist is not enabled - if hasattr(e, 'args') and "'RefreshToken' object has no attribute 'blacklist'" in e.args: - response = Response({"detail": _("Blacklist is not enabled in INSTALLED_APPS.")}, - status=status.HTTP_501_NOT_IMPLEMENTED) else: - raise + response = Response({"detail": _("No attr error has occurred.")}, + status=status.HTTP_500_INTERNAL_SERVER_ERROR) return response From 8b284f8adf86b2ff29c49aa4c66aaa26325d67f9 Mon Sep 17 00:00:00 2001 From: Marc LaBelle Date: Thu, 2 Apr 2020 10:07:23 -0400 Subject: [PATCH 5/9] fixed typo in 500 response --- dj_rest_auth/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dj_rest_auth/views.py b/dj_rest_auth/views.py index f3bb3fe..f3d6c72 100644 --- a/dj_rest_auth/views.py +++ b/dj_rest_auth/views.py @@ -166,7 +166,7 @@ class LogoutView(APIView): status=status.HTTP_500_INTERNAL_SERVER_ERROR) else: - response = Response({"detail": _("No attr error has occurred.")}, + response = Response({"detail": _("An error has occurred.")}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) return response From b55fcc2361f0e0ff46933cbed79b0851675d924a Mon Sep 17 00:00:00 2001 From: Marc LaBelle Date: Tue, 7 Apr 2020 20:24:15 -0400 Subject: [PATCH 6/9] deleted make file and adding testing section to README --- Makefile | 2 -- README.md | 5 +++++ 2 files changed, 5 insertions(+), 2 deletions(-) delete mode 100644 Makefile diff --git a/Makefile b/Makefile deleted file mode 100644 index 103ee60..0000000 --- a/Makefile +++ /dev/null @@ -1,2 +0,0 @@ -test: - coverage run --source=dj_rest_auth setup.py test diff --git a/README.md b/README.md index b87ba30..deefdb5 100644 --- a/README.md +++ b/README.md @@ -44,7 +44,12 @@ REST_USE_JWT = True JWT_AUTH_COOKIE = 'jwt-auth' ``` +### Testing +To run the tests within a virtualenv, run `python runtests.py` from the repository directory. +The easiest way to run test coverage is with [`coverage`](https://pypi.org/project/coverage/), +which runs the tests against all supported Django installs. To run the test coverage +within a virtualenv, run `coverage run ./runtests.py` from the repository directory then run `coverage report`. ### Documentation From d5d9c69aa3aaea8c1a5d77d20439bb62f8420a03 Mon Sep 17 00:00:00 2001 From: Marc LaBelle Date: Thu, 9 Apr 2020 20:53:04 -0400 Subject: [PATCH 7/9] check if blacklist is installed and warn user to delete client side if cookies and blacklist are not enabled --- dj_rest_auth/tests/test_api.py | 11 ++++++----- dj_rest_auth/views.py | 16 +++++++++++----- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/dj_rest_auth/tests/test_api.py b/dj_rest_auth/tests/test_api.py index 0e5a3c6..4c4de9e 100644 --- a/dj_rest_auth/tests/test_api.py +++ b/dj_rest_auth/tests/test_api.py @@ -1,5 +1,4 @@ import json -from unittest.mock import patch from allauth.account import app_settings as account_app_settings from django.conf import settings @@ -560,9 +559,8 @@ class APIBasicTests(TestsMixin, TestCase): self.assertEquals(resp.status_code, 200) @override_settings(REST_USE_JWT=True) - @patch('rest_framework_simplejwt.tokens.BlacklistMixin.blacklist') - def test_blacklisting_not_installed(self, mocked_blacklist): - mocked_blacklist.side_effect = AttributeError(f"'RefreshToken' object has no attribute 'blacklist'") + def test_blacklisting_not_installed(self): + settings.INSTALLED_APPS.remove('rest_framework_simplejwt.token_blacklist') payload = { "username": self.USERNAME, "password": self.PASS @@ -571,7 +569,10 @@ class APIBasicTests(TestsMixin, TestCase): resp = self.post(self.login_url, data=payload, status_code=200) token = resp.data['refresh_token'] resp = self.post(self.logout_url, status=200, data={'refresh': token}) - self.assertEqual(resp.status_code, 501) + self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.data["detail"], + "Neither cookies or blacklist are enabled, so the token has not been deleted server side. " + "Please make sure the token is deleted client side.") @override_settings(REST_USE_JWT=True) def test_blacklisting(self): diff --git a/dj_rest_auth/views.py b/dj_rest_auth/views.py index f3d6c72..325466e 100644 --- a/dj_rest_auth/views.py +++ b/dj_rest_auth/views.py @@ -134,19 +134,23 @@ class LogoutView(APIView): request.user.auth_token.delete() except (AttributeError, ObjectDoesNotExist): pass + if getattr(settings, 'REST_SESSION_LOGIN', True): django_logout(request) response = Response({"detail": _("Successfully logged out.")}, status=status.HTTP_200_OK) + if getattr(settings, 'REST_USE_JWT', False): cookie_name = getattr(settings, 'JWT_AUTH_COOKIE', None) if cookie_name: response.delete_cookie(cookie_name) - else: + + elif 'rest_framework_simplejwt.token_blacklist' in settings.INSTALLED_APPS: # add refresh token to blacklist try: token = RefreshToken(request.data['refresh']) token.blacklist() + except KeyError: response = Response({"detail": _("Refresh token was not included in request data.")}, status=status.HTTP_401_UNAUTHORIZED) @@ -157,10 +161,6 @@ class LogoutView(APIView): response = Response({"detail": _(error.args[0])}, status=status.HTTP_404_NOT_FOUND) - # warn user blacklist is not enabled - elif "'RefreshToken' object has no attribute 'blacklist'" in error.args: - response = Response({"detail": _("Blacklist is not enabled in INSTALLED_APPS.")}, - status=status.HTTP_501_NOT_IMPLEMENTED) else: response = Response({"detail": _("An error has occurred.")}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) @@ -169,6 +169,12 @@ class LogoutView(APIView): response = Response({"detail": _("An error has occurred.")}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) + else: + response = Response({ + "detail": _("Neither cookies or blacklist are enabled, so the token has not been deleted server " + "side. Please make sure the token is deleted client side." + )}, status=status.HTTP_200_OK) + return response From 91c052fe47c1d98579260e6606fb254a5e6c34db Mon Sep 17 00:00:00 2001 From: Marc LaBelle Date: Thu, 9 Apr 2020 21:00:48 -0400 Subject: [PATCH 8/9] changed invalid or expired and blacklisted errors to 401 --- dj_rest_auth/tests/test_api.py | 9 +++++++-- dj_rest_auth/views.py | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/dj_rest_auth/tests/test_api.py b/dj_rest_auth/tests/test_api.py index 4c4de9e..c53be66 100644 --- a/dj_rest_auth/tests/test_api.py +++ b/dj_rest_auth/tests/test_api.py @@ -583,13 +583,18 @@ class APIBasicTests(TestsMixin, TestCase): get_user_model().objects.create_user(self.USERNAME, '', self.PASS) resp = self.post(self.login_url, data=payload, status_code=200) token = resp.data['refresh_token'] + # test refresh token not included in request data resp = self.post(self.logout_url, status=200) self.assertEqual(resp.status_code, 401) + # test token is invalid or expired resp = self.post(self.logout_url, status=200, data={'refresh': '1'}) - self.assertEqual(resp.status_code, 404) + self.assertEqual(resp.status_code, 401) + # test successful logout resp = self.post(self.logout_url, status=200, data={'refresh': token}) self.assertEqual(resp.status_code, 200) + # test token is blacklisted resp = self.post(self.logout_url, status=200, data={'refresh': token}) - self.assertEqual(resp.status_code, 404) + self.assertEqual(resp.status_code, 401) + # test other TokenError, AttributeError, TypeError (invalid format) resp = self.post(self.logout_url, status=200, data=json.dumps({'refresh': token})) self.assertEqual(resp.status_code, 500) diff --git a/dj_rest_auth/views.py b/dj_rest_auth/views.py index 325466e..25ebe31 100644 --- a/dj_rest_auth/views.py +++ b/dj_rest_auth/views.py @@ -159,7 +159,7 @@ class LogoutView(APIView): if hasattr(error, 'args'): if 'Token is blacklisted' in error.args or 'Token is invalid or expired' in error.args: response = Response({"detail": _(error.args[0])}, - status=status.HTTP_404_NOT_FOUND) + status=status.HTTP_401_UNAUTHORIZED) else: response = Response({"detail": _("An error has occurred.")}, From 1c64c0d398615b3a9590658837a7e7a68ea051ac Mon Sep 17 00:00:00 2001 From: Marc LaBelle Date: Thu, 9 Apr 2020 21:03:41 -0400 Subject: [PATCH 9/9] changed spacing for better readability --- dj_rest_auth/views.py | 1 + 1 file changed, 1 insertion(+) diff --git a/dj_rest_auth/views.py b/dj_rest_auth/views.py index 25ebe31..114856b 100644 --- a/dj_rest_auth/views.py +++ b/dj_rest_auth/views.py @@ -137,6 +137,7 @@ class LogoutView(APIView): if getattr(settings, 'REST_SESSION_LOGIN', True): django_logout(request) + response = Response({"detail": _("Successfully logged out.")}, status=status.HTTP_200_OK)