From ca3afa33f99dd759b9108a2283024f060884c9bf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C4=93teris=20Caune?= <cuu508@gmail.com>
Date: Thu, 5 Aug 2021 16:25:52 +0300
Subject: [PATCH] Add auth method selection step

This has dual purpose:

* if user has both WebAuthn and TOTP set up, they can choose
  between the two as equal options.
* we initiate WebAuthn flow only after an explicit user action
  (button press). This may help with authentication failures
  on recent MacOS, iOS and iPadOS versions [1]

[1] https://support.yubico.com/hc/en-us/articles/360022004600-No-reaction-when-using-WebAuthn-on-macOS-iOS-and-iPadOS
---
 hc/accounts/tests/test_login_webauthn.py |  4 ++--
 static/js/login_tfa.js                   |  5 ++--
 templates/accounts/login_totp.html       |  4 ++--
 templates/accounts/login_webauthn.html   | 29 +++++++++++++++++-------
 4 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/hc/accounts/tests/test_login_webauthn.py b/hc/accounts/tests/test_login_webauthn.py
index fad949b5..f74e55d6 100644
--- a/hc/accounts/tests/test_login_webauthn.py
+++ b/hc/accounts/tests/test_login_webauthn.py
@@ -21,7 +21,7 @@ class LoginWebAuthnTestCase(BaseTestCase):
     def test_it_shows_form(self):
         r = self.client.get(self.url)
         self.assertContains(r, "Waiting for security key")
-        self.assertNotContains(r, "Use the authenticator app instead?")
+        self.assertNotContains(r, "Use authenticator app")
 
         # It should put a "state" key in the session:
         self.assertIn("state", self.client.session)
@@ -31,7 +31,7 @@ class LoginWebAuthnTestCase(BaseTestCase):
         self.profile.save()
 
         r = self.client.get(self.url)
-        self.assertContains(r, "Use the authenticator app instead?")
+        self.assertContains(r, "Use authenticator app")
 
     def test_it_requires_unauthenticated_user(self):
         self.client.login(username="alice@example.org", password="password")
diff --git a/static/js/login_tfa.js b/static/js/login_tfa.js
index bbba6fc8..57a1548a 100644
--- a/static/js/login_tfa.js
+++ b/static/js/login_tfa.js
@@ -9,6 +9,7 @@ $(function() {
     }
 
     function authenticate() {
+        $("#pick-method").addClass("hide");
         $("#waiting").removeClass("hide");
         $("#error").addClass("hide");
 
@@ -30,8 +31,6 @@ $(function() {
         });
     }
 
+    $("#use-key-btn").click(authenticate);
     $("#retry").click(authenticate);
-
-    authenticate();
-
 });
diff --git a/templates/accounts/login_totp.html b/templates/accounts/login_totp.html
index 0cb59f2b..07eb5c16 100644
--- a/templates/accounts/login_totp.html
+++ b/templates/accounts/login_totp.html
@@ -17,8 +17,8 @@
             type="text"
             name="code"
             pattern="[0-9]{6}"
-            title="six-digit code"
-            placeholder="123456"
+            title="6-digit code"
+            placeholder="6-digit code"
             class="form-control input-lg" />
         {% if form.code.errors %}
         <div class="help-block">
diff --git a/templates/accounts/login_webauthn.html b/templates/accounts/login_webauthn.html
index edcbb469..37e6007d 100644
--- a/templates/accounts/login_webauthn.html
+++ b/templates/accounts/login_webauthn.html
@@ -12,6 +12,27 @@
     encrypt="multipart/form-data">
     <h1>Two-factor Authentication</h1>
 
+    <div id="pick-method">
+        {% if offer_totp %}
+        <p>Please select how you want to authenticate.</p>
+        {% else %}
+        <p>
+            Please authenticate using your security key.<br />
+            When you are ready, press the button below.
+        </p>
+        {% endif %}
+
+        <button
+            id="use-key-btn"
+            type="button"
+            class="btn btn-primary">Use security key</button>
+        {% if offer_totp %}
+        <a href="{% url 'hc-login-totp' %}" class="btn btn-default">
+            Use authenticator app
+        </a>
+        {% endif %}
+    </div>
+
     {% csrf_token %}
     <input id="credential_id" type="hidden" name="credential_id">
     <input id="authenticator_data" type="hidden" name="authenticator_data">
@@ -44,14 +65,6 @@
         </div>
     </div>
 
-    {% if offer_totp %}
-    <p>
-        <a href="{% url 'hc-login-totp' %}">
-            Use the authenticator app instead?
-        </a>
-    </p>
-    {% endif %}
-
     <div id="success" class="hide">
         <div class="alert alert-success">
             <strong>Success!</strong>