Fix deactivate failure, backend coverage

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)

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)

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")