From da243cde82c16f9adcf3881a950f9b4f7f7c594c Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Fri, 25 Oct 2024 12:02:10 -0700 Subject: [PATCH] Fix deactivate failure, backend coverage --- src/documents/tests/test_api_permissions.py | 84 +++------------------ src/documents/tests/test_api_profile.py | 80 ++++++++++++++++++++ src/paperless/views.py | 24 +++--- 3 files changed, 101 insertions(+), 87 deletions(-) diff --git a/src/documents/tests/test_api_permissions.py b/src/documents/tests/test_api_permissions.py index 2f14ab951..6bc616397 100644 --- a/src/documents/tests/test_api_permissions.py +++ b/src/documents/tests/test_api_permissions.py @@ -1,5 +1,4 @@ import json -from unittest import mock from allauth.mfa.models import Authenticator from django.contrib.auth.models import Group @@ -609,8 +608,10 @@ class TestApiUser(DirectoriesMixin, APITestCase): - Existing user account with TOTP enabled WHEN: - API request by a superuser is made to deactivate TOTP + - API request by a regular user is made to deactivate TOTP THEN: - - TOTP is deactivated + - TOTP is deactivated, if exists + - Regular user is forbidden from deactivating TOTP """ user1 = User.objects.create( @@ -632,6 +633,12 @@ class TestApiUser(DirectoriesMixin, APITestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(Authenticator.objects.filter(user=user1).count(), 0) + # fail if already deactivated + response = self.client.post( + f"{self.ENDPOINT}{user1.pk}/deactivate_totp/", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + regular_user = User.objects.create_user(username="regular_user") regular_user.user_permissions.add( *Permission.objects.all(), @@ -1059,76 +1066,3 @@ class TestBulkEditObjectPermissions(APITestCase): ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - - -class TestApiTOTPViews(APITestCase): - ENDPOINT = "/api/profile/totp/" - - def setUp(self): - super().setUp() - - self.user = User.objects.create_superuser(username="temp_admin") - self.client.force_authenticate(user=self.user) - - def test_get_totp(self): - """ - GIVEN: - - Existing user account - WHEN: - - API request is made to TOTP endpoint - THEN: - - TOTP is generated - """ - response = self.client.get( - self.ENDPOINT, - ) - - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertIn("qr_svg", response.data) - self.assertIn("secret", response.data) - - @mock.patch("allauth.mfa.totp.internal.auth.validate_totp_code") - def test_activate_totp(self, mock_validate_totp_code): - """ - GIVEN: - - Existing user account - WHEN: - - API request is made to activate TOTP - THEN: - - TOTP is activated, recovery codes are returned - """ - mock_validate_totp_code.return_value = True - - response = self.client.post( - self.ENDPOINT, - data={ - "secret": "123", - "code": "456", - }, - ) - - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertTrue(Authenticator.objects.filter(user=self.user).exists()) - self.assertIn("recovery_codes", response.data) - - def test_deactivate_totp(self): - """ - GIVEN: - - Existing user account with TOTP enabled - WHEN: - - API request is made to deactivate TOTP - THEN: - - TOTP is deactivated - """ - Authenticator.objects.create( - user=self.user, - type=Authenticator.Type.TOTP, - data={}, - ) - - response = self.client.delete( - self.ENDPOINT, - ) - - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(Authenticator.objects.filter(user=self.user).count(), 0) diff --git a/src/documents/tests/test_api_profile.py b/src/documents/tests/test_api_profile.py index eede0d2b0..e8bd2f151 100644 --- a/src/documents/tests/test_api_profile.py +++ b/src/documents/tests/test_api_profile.py @@ -1,5 +1,6 @@ from unittest import mock +from allauth.mfa.models import Authenticator from allauth.socialaccount.models import SocialAccount from allauth.socialaccount.models import SocialApp from django.contrib.auth.models import User @@ -299,3 +300,82 @@ class TestApiProfile(DirectoriesMixin, APITestCase): len(self.user.socialaccount_set.filter(pk=social_account_id)), 0, ) + + +class TestApiTOTPViews(APITestCase): + ENDPOINT = "/api/profile/totp/" + + def setUp(self): + super().setUp() + + self.user = User.objects.create_superuser(username="temp_admin") + self.client.force_authenticate(user=self.user) + + def test_get_totp(self): + """ + GIVEN: + - Existing user account + WHEN: + - API request is made to TOTP endpoint + THEN: + - TOTP is generated + """ + response = self.client.get( + self.ENDPOINT, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIn("qr_svg", response.data) + self.assertIn("secret", response.data) + + @mock.patch("allauth.mfa.totp.internal.auth.validate_totp_code") + def test_activate_totp(self, mock_validate_totp_code): + """ + GIVEN: + - Existing user account + WHEN: + - API request is made to activate TOTP + THEN: + - TOTP is activated, recovery codes are returned + """ + mock_validate_totp_code.return_value = True + + response = self.client.post( + self.ENDPOINT, + data={ + "secret": "123", + "code": "456", + }, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertTrue(Authenticator.objects.filter(user=self.user).exists()) + self.assertIn("recovery_codes", response.data) + + def test_deactivate_totp(self): + """ + GIVEN: + - Existing user account with TOTP enabled + WHEN: + - API request is made to deactivate TOTP + THEN: + - TOTP is deactivated + """ + Authenticator.objects.create( + user=self.user, + type=Authenticator.Type.TOTP, + data={}, + ) + + response = self.client.delete( + self.ENDPOINT, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(Authenticator.objects.filter(user=self.user).count(), 0) + + # test fails + response = self.client.delete( + self.ENDPOINT, + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) diff --git a/src/paperless/views.py b/src/paperless/views.py index a7dff8282..45286898d 100644 --- a/src/paperless/views.py +++ b/src/paperless/views.py @@ -116,14 +116,14 @@ class UserViewSet(ModelViewSet): return HttpResponseForbidden( "You do not have permission to deactivate TOTP for this user", ) - try: - authenticator = Authenticator.objects.filter( - user=user, - type=Authenticator.Type.TOTP, - ).first() + authenticator = Authenticator.objects.filter( + user=user, + type=Authenticator.Type.TOTP, + ).first() + if authenticator is not None: delete_and_cleanup(request, authenticator) return Response(True) - except Authenticator.DoesNotExist: + else: return HttpResponseBadRequest("TOTP not found") @@ -230,14 +230,14 @@ class TOTPView(GenericAPIView): Deactivates the TOTP authenticator """ user = self.request.user - try: - authenticator = Authenticator.objects.filter( - user=user, - type=Authenticator.Type.TOTP, - ).first() + authenticator = Authenticator.objects.filter( + user=user, + type=Authenticator.Type.TOTP, + ).first() + if authenticator is not None: delete_and_cleanup(request, authenticator) return Response(True) - except Authenticator.DoesNotExist: + else: return HttpResponseBadRequest("TOTP not found")