From 96823a7f90bebe68d8b39924a977b6e84f082047 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C4=93teris=20Caune?= <cuu508@gmail.com>
Date: Fri, 17 Nov 2023 16:06:39 +0200
Subject: [PATCH] Add logging for failed webauthn key registrations
---
hc/accounts/tests/test_add_webauthn.py | 8 ++++++--
hc/accounts/views.py | 9 +++++++--
hc/lib/webauthn.py | 9 +++------
static/css/admin/records.css | 8 +++++++-
4 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/hc/accounts/tests/test_add_webauthn.py b/hc/accounts/tests/test_add_webauthn.py
index 2a4a1efe..2162129a 100644
--- a/hc/accounts/tests/test_add_webauthn.py
+++ b/hc/accounts/tests/test_add_webauthn.py
@@ -73,9 +73,10 @@ class AddWebauthnTestCase(BaseTestCase):
r = self.client.post(self.url, payload)
self.assertEqual(r.status_code, 400)
+ @patch("hc.accounts.views.logger")
@patch("hc.accounts.views.CreateHelper.verify")
- def test_it_handles_verification_failure(self, mock_verify: Mock) -> None:
- mock_verify.return_value = None
+ def test_it_handles_verification_failure(self, verify: Mock, logger: Mock) -> None:
+ verify.side_effect = ValueError
self.client.login(username="alice@example.org", password="password")
self.set_sudo_flag()
@@ -88,3 +89,6 @@ class AddWebauthnTestCase(BaseTestCase):
r = self.client.post(self.url, payload, follow=True)
self.assertEqual(r.status_code, 400)
+
+ # It should log the verification failure
+ self.assertTrue(logger.exception.called)
diff --git a/hc/accounts/views.py b/hc/accounts/views.py
index 37de47c7..ac9a39fe 100644
--- a/hc/accounts/views.py
+++ b/hc/accounts/views.py
@@ -1,5 +1,6 @@
from __future__ import annotations
+import logging
import time
from datetime import timedelta as td
from secrets import token_urlsafe
@@ -42,6 +43,8 @@ from hc.lib.tz import all_timezones
from hc.lib.webauthn import CreateHelper, GetHelper
from hc.payments.models import Subscription
+logger = logging.getLogger(__name__)
+
POST_LOGIN_ROUTES = (
"hc-checks",
"hc-details",
@@ -727,8 +730,10 @@ def add_webauthn(request: AuthenticatedHttpRequest) -> HttpResponse:
return HttpResponseBadRequest()
state = request.session["state"]
- credential_bytes = helper.verify(state, form.cleaned_data["response"])
- if credential_bytes is None:
+ try:
+ credential_bytes = helper.verify(state, form.cleaned_data["response"])
+ except ValueError as e:
+ logger.exception("CreateHelper.verify failed, form: %s", form.cleaned_data)
return HttpResponseBadRequest()
c = Credential(user=request.user)
diff --git a/hc/lib/webauthn.py b/hc/lib/webauthn.py
index f37c6ca8..366da523 100644
--- a/hc/lib/webauthn.py
+++ b/hc/lib/webauthn.py
@@ -47,12 +47,9 @@ class CreateHelper(object):
return dict(options), state
def verify(self, state: Any, response_json: str) -> bytes | None:
- try:
- doc = json.loads(response_json)
- auth_data = self.server.register_complete(state, doc)
- return auth_data.credential_data
- except ValueError:
- return None
+ doc = json.loads(response_json)
+ auth_data = self.server.register_complete(state, doc)
+ return auth_data.credential_data
class GetHelper(object):
diff --git a/static/css/admin/records.css b/static/css/admin/records.css
index 0256f0db..c2526690 100644
--- a/static/css/admin/records.css
+++ b/static/css/admin/records.css
@@ -39,4 +39,10 @@
.field-traceback .readonly {
font-family: monospace;
-}
\ No newline at end of file
+}
+
+.field-message .readonly {
+ width: 90%;
+ font-family: monospace;
+
+}