fix(webapp): don't mistake every 403 for an MFA challenge

api/desecapi/exception_handlers.py

@@ -46,4 +46,14 @@ def _500():
             metrics.get("desecapi_exception").labels(class_path).inc()
             return handler()
 
-    return drf_exception_handler(exc, context)
+    response = drf_exception_handler(exc, context)
+
+    # DRF renders only the human-readable detail, dropping the machine-readable
+    # code that tells denials apart (e.g. an MFA challenge from a used-up domain
+    # limit). Clients need it to know what to do next, so put it on the wire.
+    if response is not None and isinstance(response.data, dict):
+        code = getattr(response.data.get("detail"), "code", None)
+        if code is not None:
+            response.data["code"] = code
+
+    return response

api/desecapi/permissions.py

@@ -58,6 +58,18 @@ def has_permission(self, request, view):
         )
 
 
+class MFARequiredIfEnabledOrAPIToken(permissions.OR):
+    """
+    Allows access to API tokens, and to human tokens as per MFARequiredIfEnabled.
+    """
+
+    message = MFARequiredIfEnabled.message
+    code = MFARequiredIfEnabled.code
+
+    def __init__(self):
+        super().__init__(IsAPIToken(), MFARequiredIfEnabled())
+
+
 class IsOwner(permissions.BasePermission):
     """
     Custom permission to only allow owners of an object to view or edit it.

api/desecapi/tests/test_authentication.py

@@ -78,7 +78,7 @@ def assertAuthenticationStatus(self, code, plain=None, expired=False, **kwargs):
 
         response = self.client.get(self.reverse("v1:root"), **kwargs)
         body = (
-            json.dumps({"detail": "Invalid token."})
+            json.dumps({"detail": "Invalid token.", "code": "authentication_failed"})
             if code == HTTP_401_UNAUTHORIZED
             else None
         )

api/desecapi/tests/test_domain_limits.py

@@ -168,6 +168,8 @@ def test_securing_domains_raises_the_limit(self):
         self.assertContains(
             response, "Domain limit", status_code=status.HTTP_403_FORBIDDEN
         )
+        # Clients need to tell this apart from an MFA challenge
+        self.assertEqual(response.data["code"], "domain_limit_exceeded")
 
         # Two secure domains pay for their own slots, and the headroom floor
         # of 2 sits on top: 2 + max(2, round(sqrt(2))) = 4.

api/desecapi/tests/test_totp.py

@@ -34,6 +34,12 @@ def _decrement_timestep(self, offset):
         factor.save()
 
     def _test_MFA_permission_status(self, assertion):
+        def assert_status(response):
+            assertion(response.status_code, status.HTTP_403_FORBIDDEN)
+            if response.status_code == status.HTTP_403_FORBIDDEN:
+                # Clients need to tell this apart from other denials
+                self.assertEqual(response.data["code"], "mfa_required")
+
         for method, view_names in {
             self.client.get: [
                 "v1:account",
@@ -48,19 +54,19 @@ def _test_MFA_permission_status(self, assertion):
         }.items():
             for view_name in view_names:
                 response = method(self.reverse(view_name))
-                assertion(response.status_code, status.HTTP_403_FORBIDDEN)
+                assert_status(response)
         for view_name in [
             "v1:domain-detail",
             "v1:rrsets",
         ]:
             for method in [self.client.get, self.client.post]:
                 response = method(self.reverse(view_name, name=self.my_domain))
-                assertion(response.status_code, status.HTTP_403_FORBIDDEN)
+                assert_status(response)
         for method in [self.client.get, self.client.post]:
             response = method(
                 self.reverse("v1:rrset@", name=self.my_domain, subname="", type="NS")
             )
-            assertion(response.status_code, status.HTTP_403_FORBIDDEN)
+            assert_status(response)
 
     def test_workflow(self):
         # Request setting up TOTP factor

api/desecapi/views/domains.py

@@ -39,7 +39,7 @@ class DomainViewSet(
     def permission_classes(self):
         ret = [
             IsAuthenticated,
-            permissions.IsAPIToken | permissions.MFARequiredIfEnabled,
+            permissions.MFARequiredIfEnabledOrAPIToken,
             permissions.IsOwner,
         ]
         if self.request.method not in SAFE_METHODS:

api/desecapi/views/records.py

@@ -38,7 +38,7 @@ class RRsetView(DomainViewMixin):
     serializer_class = RRsetSerializer
     permission_classes = (
         IsAuthenticated,
-        permissions.IsAPIToken | permissions.MFARequiredIfEnabled,
+        permissions.MFARequiredIfEnabledOrAPIToken,
         permissions.IsDomainOwner,
     )
 

api/desecapi/views/tokens.py

@@ -24,7 +24,7 @@ class TokenViewSet(IdempotentDestroyMixin, viewsets.ModelViewSet):
     def permission_classes(self):
         ret = [
             IsAuthenticated,
-            permissions.IsAPIToken | permissions.MFARequiredIfEnabled,
+            permissions.MFARequiredIfEnabledOrAPIToken,
             permissions.HasManageTokensPermission,
         ]
         # The effective user may manage the token; its owner can only delete it
@@ -86,7 +86,7 @@ class TokenPoliciesRoot(RetrieveAPIView):
     throttle_scope = "account_management_passive"
     permission_classes = [
         IsAuthenticated,
-        permissions.IsAPIToken | permissions.MFARequiredIfEnabled,
+        permissions.MFARequiredIfEnabledOrAPIToken,
         permissions.HasManageTokensPermission
         | permissions.AuthTokenCorrespondsToViewToken,
     ]
@@ -113,7 +113,7 @@ class TokenDomainPolicyViewSet(IdempotentDestroyMixin, viewsets.ModelViewSet):
     def permission_classes(self):
         ret = [
             IsAuthenticated,
-            permissions.IsAPIToken | permissions.MFARequiredIfEnabled,
+            permissions.MFARequiredIfEnabledOrAPIToken,
         ]
         if self.request.method in SAFE_METHODS:
             ret.append(

api/desecapi/views/users.py

@@ -72,7 +72,7 @@ def create(self, request, *args, **kwargs):
 class AccountView(generics.RetrieveUpdateAPIView):
     permission_classes = (
         IsAuthenticated,
-        permissions.IsAPIToken | permissions.MFARequiredIfEnabled,
+        permissions.MFARequiredIfEnabledOrAPIToken,
         permissions.HasManageTokensPermission,
     )
     serializer_class = serializers.UserSerializer

docs/auth/account.rst

@@ -108,7 +108,8 @@ Request``::
     HTTP/1.1 400 Bad Request
 
     {
-        "detail": "Registration denied. If you believe this is an error, please contact support."
+        "detail": "Registration denied. If you believe this is an error, please contact support.",
+        "code": "registration_denied"
     }
 
 

www/webapp/src/utils.js

@@ -48,15 +48,11 @@ async function _digestError(error, app) {
         } else {
           return ['You are not logged in.'];
         }
-      } else if (error.response.status === 403) {
-          if (useUserStore().authenticated && !['change-email', 'delete-account'].includes(app.$route.name)) { // MFA
-            if (app.$route.name !== 'mfa') {
-              app.$router.push({name: 'mfa', query: {redirect: app.$route.fullPath}});
-            }
-            return [];
-          } else { // unauthenticated 403, i.e. login failure
-            return [error.response.data.detail]
-          }
+      } else if (error.response.status === 403 && error.response.data?.code === 'mfa_required' && app !== undefined) {
+        if (app.$route.name !== 'mfa') {
+          app.$router.push({name: 'mfa', query: {redirect: app.$route.fullPath}});
+        }
+        return [];
       } else if (error.response.status === 413) {
         return ['Too much data. Try to reduce the length of your inputs.'];
       } else if ('data' in error.response) {