From a7bf3d3be9db60cf7ba3e0b3455822fffff79761 Mon Sep 17 00:00:00 2001
From: Antoine Nguyen <tonio@ngyn.org>
Date: Sat, 6 Jul 2024 16:22:03 +0200
Subject: [PATCH] Improved webauthn support

---
 .../components/account/BackupCodeAuthForm.vue |  3 +-
 .../src/components/account/FidoAuthForm.vue   |  3 +-
 .../account/RecoveryCodesResetDialog.vue      | 12 +++---
 .../src/components/account/TotpAuthForm.vue   |  1 +
 frontend/src/views/account/SettingsView.vue   |  2 +-
 modoboa/core/api/v2/viewsets.py               |  4 +-
 modoboa/core/fido2_auth.py                    | 42 +++++++++----------
 modoboa/core/views/auth.py                    |  6 ++-
 .../registration/twofactor_code_verify.html   |  2 +-
 9 files changed, 39 insertions(+), 36 deletions(-)

diff --git a/frontend/src/components/account/BackupCodeAuthForm.vue b/frontend/src/components/account/BackupCodeAuthForm.vue
index e94d1f41f..a4132db29 100644
--- a/frontend/src/components/account/BackupCodeAuthForm.vue
+++ b/frontend/src/components/account/BackupCodeAuthForm.vue
@@ -11,7 +11,7 @@
           <v-alert type="info" class="mb-2">
             {{
               $gettext(
-                "You already have generated backup codes for this account. If you don't remember them or have used them all, you generate new ones."
+                "You already have generated backup codes for this account. If you don't remember them or have used them all, you can generate new ones."
               )
             }}
           </v-alert>
@@ -22,6 +22,7 @@
               autocomplete="new-password"
               type="password"
               variant="outlined"
+              density="compact"
               :rules="[rules.required]"
               :error-messages="passwordError"
             />
diff --git a/frontend/src/components/account/FidoAuthForm.vue b/frontend/src/components/account/FidoAuthForm.vue
index 2c1995e6b..014c36f29 100644
--- a/frontend/src/components/account/FidoAuthForm.vue
+++ b/frontend/src/components/account/FidoAuthForm.vue
@@ -12,7 +12,7 @@
             <div tag="p" class="my-4">
               {{
                 $gettext(
-                  "You don't have any WebAuthN registered as a second authentication method."
+                  "You don't have any WebAuthN device registered as a second authentication method."
                 )
               }}
             </div>
@@ -22,7 +22,6 @@
             <v-data-table-virtual
               :headers="headers"
               :items="fidoCreds"
-              height="400"
               item-value="id"
             >
               <template #[`item.added_on`]="{ item }">
diff --git a/frontend/src/components/account/RecoveryCodesResetDialog.vue b/frontend/src/components/account/RecoveryCodesResetDialog.vue
index af4eac6ec..775d0d903 100644
--- a/frontend/src/components/account/RecoveryCodesResetDialog.vue
+++ b/frontend/src/components/account/RecoveryCodesResetDialog.vue
@@ -11,11 +11,13 @@
           )
         }}
       </v-alert>
-      <ul>
-        <li v-for="token in tokens" :key="token">
-          {{ token }}
-        </li>
-      </ul>
+      <v-table class="mt-4" density="compact">
+        <tbody>
+          <tr v-for="token in tokens" :key="token">
+            <td>{{ token }}</td>
+          </tr>
+        </tbody>
+      </v-table>
     </v-card-text>
     <v-card-actions>
       <v-spacer></v-spacer>
diff --git a/frontend/src/components/account/TotpAuthForm.vue b/frontend/src/components/account/TotpAuthForm.vue
index f9860dee2..e2c98d4f5 100644
--- a/frontend/src/components/account/TotpAuthForm.vue
+++ b/frontend/src/components/account/TotpAuthForm.vue
@@ -87,6 +87,7 @@
               autocomplete="new-password"
               type="password"
               variant="outlined"
+              density="compact"
               :rules="[rules.required]"
               :error-messages="passwordError"
             />
diff --git a/frontend/src/views/account/SettingsView.vue b/frontend/src/views/account/SettingsView.vue
index eab7b06b4..f2f6f678d 100644
--- a/frontend/src/views/account/SettingsView.vue
+++ b/frontend/src/views/account/SettingsView.vue
@@ -26,8 +26,8 @@
       </v-window-item>
       <v-window-item value="security">
         <TotpAuthForm />
-        <BackupCodeAuthForm />
         <FidoAuthForm />
+        <BackupCodeAuthForm />
       </v-window-item>
       <v-window-item v-if="authStore.userHasMailbox" value="forward">
         <ForwardForm />
diff --git a/modoboa/core/api/v2/viewsets.py b/modoboa/core/api/v2/viewsets.py
index aa17fee7d..e616664b8 100644
--- a/modoboa/core/api/v2/viewsets.py
+++ b/modoboa/core/api/v2/viewsets.py
@@ -200,9 +200,7 @@ class FIDOViewSet(
         """An Api View to complete the registration process of a fido key."""
         serializer = serializers.FidoRegistrationSerializer(data=request.data)
         serializer.is_valid(raise_exception=True)
-        credential_data = f2_auth.end_registration(
-            request.data, request.session.pop("fido2_state"), request.user.id
-        )
+        credential_data = f2_auth.end_registration(request)
         models.UserFidoKeys.objects.create(
             name=request.data["name"],
             credential_data=credential_data,
diff --git a/modoboa/core/fido2_auth.py b/modoboa/core/fido2_auth.py
index 084c81378..d86873e0f 100644
--- a/modoboa/core/fido2_auth.py
+++ b/modoboa/core/fido2_auth.py
@@ -20,28 +20,28 @@ def set_json_mapping():
         pass
 
 
-def create_fido2_server():
-    rp = PublicKeyCredentialRpEntity(name="Demo server", id="localhost")
+def create_fido2_server(rp_id: str) -> Fido2Server:
+    rp = PublicKeyCredentialRpEntity(name="Modoboa", id=rp_id)
     return Fido2Server(rp)
 
 
-def get_creds_from_user(user_id):
-    return [
-        AttestedCredentialData(websafe_decode(uf.credential_data))
-        for uf in UserFidoKeys.objects.filter(user=user_id)
-    ]
+def get_creds_from_user(user_id: int) -> dict:
+    return {
+        key: AttestedCredentialData(websafe_decode(key.credential_data))
+        for key in UserFidoKeys.objects.filter(user=user_id)
+    }
 
 
 def begin_registration(request):
     set_json_mapping()
-    server = create_fido2_server()
+    server = create_fido2_server(request.localconfig.site.domain)
     options, state = server.register_begin(
         PublicKeyCredentialUserEntity(
             id=request.user.pk,
             name=request.user.username.encode("utf-8"),
             display_name=request.user.username,
         ),
-        get_creds_from_user(request.user.pk),
+        list(get_creds_from_user(request.user.pk).values()),
         user_verification=UserVerificationRequirement.DISCOURAGED,
         extensions={"credentialProtectionPolicy": "userVerificationOptional"},
     )
@@ -49,31 +49,31 @@ def begin_registration(request):
     return options
 
 
-def end_registration(data, fido2_state, user_id):
+def end_registration(request):
     set_json_mapping()
-    server = create_fido2_server()
-    auth_data = server.register_complete(fido2_state, data)
+    server = create_fido2_server(request.localconfig.site.domain)
+    auth_data = server.register_complete(
+        request.session.pop("fido2_state"), request.data
+    )
     return websafe_encode(auth_data.credential_data)
 
 
-def begin_authentication(user_id: int):
+def begin_authentication(request, user_id: int):
     set_json_mapping()
-    server = create_fido2_server()
-    return server.authenticate_begin(get_creds_from_user(user_id))
+    server = create_fido2_server(request.localconfig.site.domain)
+    return server.authenticate_begin(list(get_creds_from_user(user_id).values()))
 
 
-def end_authentication(user: User, state: str, data: dict):
+def end_authentication(user: User, state: str, data: dict, rp_id: str):
     set_json_mapping()
-    server = create_fido2_server()
-    # FIXME: review the next function...
+    server = create_fido2_server(rp_id)
     credentials = get_creds_from_user(user.pk)
     result = server.authenticate_complete(
         state,
-        credentials,
+        list(credentials.values()),
         data,
     )
-    for key in user.userfidokeys_set.all():
-        cred = AttestedCredentialData(websafe_decode(key.credential_data))
+    for key, cred in credentials.items():
         if cred.credential_id == result.credential_id:
             key.last_used = timezone.now()
             key.use_count += 1
diff --git a/modoboa/core/views/auth.py b/modoboa/core/views/auth.py
index d9eaa157e..881001119 100644
--- a/modoboa/core/views/auth.py
+++ b/modoboa/core/views/auth.py
@@ -300,7 +300,7 @@ class FidoAuthenticationBeginView(generic.View):
         user_id = request.session.get(constants.TFA_PRE_VERIFY_USER_PK)
         if not user_id:
             raise PermissionDenied
-        options, state = fido2_auth.begin_authentication(user_id)
+        options, state = fido2_auth.begin_authentication(request, user_id)
         request.session["fido_state"] = state
         return JsonResponse(dict(options))
 
@@ -315,6 +315,8 @@ class FidoAuthenticationEndView(LoginViewMixin, APIView):
             raise PermissionDenied
         serializer = serializers.FidoAuthenticationSerializer(data=request.data)
         serializer.is_valid(raise_exception=True)
-        fido2_auth.end_authentication(user, fido_state, serializer.validated_data)
+        fido2_auth.end_authentication(
+            user, fido_state, serializer.validated_data, request.localconfig.site.domain
+        )
         response = self.login(user, self.request.session.pop("rememberme", False))
         return JsonResponse({"next": response.url})
diff --git a/modoboa/templates/registration/twofactor_code_verify.html b/modoboa/templates/registration/twofactor_code_verify.html
index 41bb8a9f7..8eab21d6a 100644
--- a/modoboa/templates/registration/twofactor_code_verify.html
+++ b/modoboa/templates/registration/twofactor_code_verify.html
@@ -46,7 +46,7 @@
       {% endif %}
     </div>
     <div id="webautn_form" {% if show_method_selection or not webauthn_device %}style="display:none;"{% endif %}>
-      <button class="btn" onclick="useWebAuthn()">{% translate "Log using Webauthn" %}</button>
+      <button class="btn btn-primary" onclick="useWebAuthn()">{% translate "Log using Webauthn" %}</button>
       {% if show_method_selection %}
         <button class="mt btn" onclick="backToSelection()">{% translate "Back" %}</button>
       {% endif %}