feat(api): require User.is_active on auth actions, except activation

api/desecapi/authentication.py

@@ -149,16 +149,22 @@ class EmailPasswordPayloadAuthentication(BaseAuthentication):
 class AuthenticatedBasicUserActionAuthentication(BaseAuthentication):
     """
     Authenticates a request based on whether the serializer determines the validity of the given verification code
-    and additional data (using `serializer.is_valid()`). The serializer's input data will be determined by (a) the
-    view's 'code' kwarg and (b) the request payload for POST requests.
-
-    If the request is valid, the AuthenticatedAction instance will be attached to the request as `auth` attribute.
+    based on the view's 'code' kwarg and the view serializer's code validity period.
     """
     def authenticate(self, request):
         view = request.parser_context['view']
-        serializer = AuthenticatedBasicUserActionSerializer(data={}, context=view.get_serializer_context())
+        return self.authenticate_credentials(view.get_serializer_context())
+
+    def authenticate_credentials(self, context):
+        serializer = AuthenticatedBasicUserActionSerializer(data={}, context=context)
         serializer.is_valid(raise_exception=True)
-        return serializer.validated_data['user'], None
+        user = serializer.validated_data['user']
+
+        # When user.is_active is None, activation is pending.  We need to admit them to finish activation, so only
+        # reject strictly False.  There are permissions to make sure that such accounts can't do anything else.
+        if user.is_active == False:
+            raise exceptions.AuthenticationFailed('User inactive.')
+        return user, None
 
 
 class TokenHasher(PBKDF2PasswordHasher):

api/desecapi/permissions.py

@@ -3,6 +3,16 @@ from ipaddress import IPv4Address, IPv4Network
 from rest_framework import permissions
 
 
+class IsActiveUser(permissions.BasePermission):
+    """
+    Allows access only to activated users.
+    """
+
+    def has_permission(self, request, view):
+        # Authenticated users can have is_active = None (pending activation). Strictly require True here.
+        return request.user and request.user.is_active == True
+
+
 class IsOwner(permissions.BasePermission):
     """
     Custom permission to only allow owners of an object to view or edit it.

api/desecapi/tests/test_user_management.py

@@ -319,6 +319,13 @@ class UserManagementTestCase(DesecTestCase, PublicSuffixMockMixin):
             status_code=status.HTTP_200_OK
         )
 
+    def assertResetPasswordInactiveUserVerificationFailedResponse(self, response):
+        return self.assertContains(
+            response=response,
+            text="User inactive.",
+            status_code=status.HTTP_403_FORBIDDEN
+        )
+
     def assertChangeEmailSuccessResponse(self, response):
         return self.assertContains(
             response=response,
@@ -705,6 +712,20 @@ class HasUserAccountTestCase(UserManagementTestCase):
             self.assertResetPasswordSuccessResponse(self.reset_password(self.email))
             self.assertNoEmailSent()
 
+    def test_reset_password_inactive_user_old_confirmation_link(self):
+        user = User.objects.get(email=self.email)
+        user.needs_captcha = False
+
+        self.assertResetPasswordSuccessResponse(self.reset_password(self.email))
+        confirmation_link = self.assertResetPasswordEmail(self.email)
+
+        user.is_active = False
+        user.save()
+        new_password = self.random_password()
+        self.assertConfirmationLinkRedirect(confirmation_link)
+        self.assertResetPasswordInactiveUserVerificationFailedResponse(
+            self.client.verify(confirmation_link, data={'new_password': new_password}))
+
     def test_reset_password_multiple_times(self):
         for _ in range(3):
             self._test_reset_password(self.email)

api/desecapi/views.py

@@ -639,6 +639,10 @@ class AuthenticatedActionView(generics.GenericAPIView):
         # This prevents both auth action code evaluation and user-specific throttling when we only want a redirect
         return () if self.request.method in SAFE_METHODS else (auth.AuthenticatedBasicUserActionAuthentication,)
 
+    @property
+    def permission_classes(self):
+        return () if self.request.method in SAFE_METHODS else (permissions.IsActiveUser,)
+
     @property
     def throttle_scope(self):
         return 'account_management_passive' if self.request.method in SAFE_METHODS else 'account_management_active'
@@ -680,6 +684,7 @@ class AuthenticatedActionView(generics.GenericAPIView):
 
 class AuthenticatedActivateUserActionView(AuthenticatedActionView):
     html_url = '/confirm/activate-account/{code}/'
+    permission_classes = ()  # don't require that user is activated already
     serializer_class = serializers.AuthenticatedActivateUserActionSerializer
 
     def finalize(self):