add setting MAX_KEYS_PER_ACCOUNT

Author: xi

mfa/settings.py

@@ -17,3 +17,12 @@
 # `user_verification` parameter passed to python-fido2
 # See https://www.w3.org/TR/webauthn/#enum-userVerificationRequirement
 FIDO2_USER_VERIFICATION = getattr(settings, 'MFA_FIDO2_USER_VERIFICATION', None)
+
+# An account might end up having a large number of keys
+# if it is shared by multiple users or if lost keys are not
+# deleted. Both are potential security issues, so this is
+# restricted to a reasonable (but small) number by default.
+#
+# This setting only applies when adding new keys.
+# To allow an arbitrary number of keys, set this to `None`.
+MAX_KEYS_PER_ACCOUNT = getattr(settings, 'MFA_MAX_KEYS_PER_ACCOUNT', 3)

mfa/templates/mfa/mfakey_list.html

@@ -18,6 +18,13 @@ <h1>{% if object_list %}Login keys configured{% else %}No login keys configured{
     {% endfor %}
 </ul>
 
-<a href="{% url 'mfa:create' 'TOTP' %}">Create TOTP key</a>
-<a href="{% url 'mfa:create' 'FIDO2' %}">Create FIDO2 key</a>
-<a href="{% url 'mfa:create' 'recovery' %}">Create recovery code</a>
+{% if max_keys and user.mfakey_set.count >= max_keys %}
+    <p>
+        You cannot have more than {{ max_keys }} keys.
+        Please delete one of your existing keys before adding a new one.
+    </p>
+{% else %}
+    <a href="{% url 'mfa:create' 'TOTP' %}">Create TOTP key</a>
+    <a href="{% url 'mfa:create' 'FIDO2' %}">Create FIDO2 key</a>
+    <a href="{% url 'mfa:create' 'recovery' %}">Create recovery code</a>
+{% endif %}

mfa/views.py

@@ -10,6 +10,7 @@
 from django.urls import reverse
 from django.utils.decorators import method_decorator
 from django.utils.functional import cached_property
+from django.utils.text import format_lazy
 from django.utils.translation import gettext_lazy as _
 from django.views.generic import DeleteView
 from django.views.generic import ListView
@@ -49,6 +50,11 @@ class MFAListView(LoginRequiredMixin, ListView):
     def get_queryset(self):
         return super().get_queryset().filter(user=self.request.user)
 
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data(**kwargs)
+        context['max_keys'] = settings.MAX_KEYS_PER_ACCOUNT
+        return context
+
 
 class MFADeleteView(LoginRequiredMixin, DeleteView):
     model = MFAKey
@@ -76,6 +82,14 @@ def complete(self, code):
         return self.method.register_complete(self.challenge[1], code)
 
     def form_valid(self, form):
+        if settings.MAX_KEYS_PER_ACCOUNT:
+            count = self.request.user.mfakey_set.count()
+            if count >= settings.MAX_KEYS_PER_ACCOUNT:
+                form.add_error(None, format_lazy(_(
+                    'You cannot have more than {} keys. Please delete '
+                    'one of your existing keys before adding a new one.'
+                ), settings.MAX_KEYS_PER_ACCOUNT))
+                return self.form_invalid(form)
         MFAKey.objects.create(
             user=self.request.user,
             method=self.method.name,

tests/tests.py

@@ -166,6 +166,28 @@ def test_keep_challenge_on_validation(self):
         })
         self.assertEqual(MFAKey.objects.count(), 1)
 
+    def test_max_keys(self):
+        for i in range(3):
+            self.user.mfakey_set.create(
+                method='recovery',
+                name=f'test-{i}',
+                secret='dummy',
+            )
+
+        self.client.force_login(self.user)
+
+        res = self.client.get('/mfa/create/TOTP/')
+        self.assertEqual(res.status_code, 200)
+        totp = pyotp.TOTP(res.context['mfa_data']['secret'])
+
+        with self.settings(MFA_MAX_KEYS_PER_ACCOUNT=3):
+            res = self.client.post('/mfa/create/TOTP/', {
+                'name': 'test',
+                'code': totp.now()
+            })
+        self.assertEqual(res.status_code, 200)
+        self.assertEqual(MFAKey.objects.count(), 3)
+
 
 class FIDO2Test(MFATestCase):
     # I have no clue how to simulate a FIDO2 authenticator,