From c2d34a78050584271fec14de5c2495aacefa6d35 Mon Sep 17 00:00:00 2001
From: Stephen Nelson <stephen@katalyst.com.au>
Date: Thu, 21 Nov 2024 14:26:46 +1030
Subject: [PATCH] Improve passkey UX

---
 .../webauthn_registration_controller.js       | 22 ++++++++-----
 app/assets/stylesheets/koi/base/_flow.scss    |  8 +++++
 app/assets/stylesheets/koi/base/_index.scss   |  2 ++
 app/assets/stylesheets/koi/base/_repel.scss   | 23 ++++++++++++++
 .../concerns/koi/controller/has_webauthn.rb   |  3 +-
 app/views/admin/admin_users/index.html.erb    |  3 ++
 .../admin/admin_users/show.html+self.erb      |  9 +++---
 .../credentials/_credentials.html+self.erb    | 10 ++++++
 .../admin/credentials/_credentials.html.erb   |  9 +++---
 app/views/admin/credentials/new.html.erb      | 31 ++++++++++++++++---
 10 files changed, 98 insertions(+), 22 deletions(-)
 create mode 100644 app/assets/stylesheets/koi/base/_flow.scss
 create mode 100644 app/assets/stylesheets/koi/base/_repel.scss
 create mode 100644 app/views/admin/credentials/_credentials.html+self.erb

diff --git a/app/assets/javascripts/koi/controllers/webauthn_registration_controller.js b/app/assets/javascripts/koi/controllers/webauthn_registration_controller.js
index b0ff5b924..258836399 100644
--- a/app/assets/javascripts/koi/controllers/webauthn_registration_controller.js
+++ b/app/assets/javascripts/koi/controllers/webauthn_registration_controller.js
@@ -6,8 +6,11 @@ import {
 } from "@github/webauthn-json/browser-ponyfill";
 
 export default class WebauthnRegistrationController extends Controller {
-  static values = { options: Object };
-  static targets = ["response"];
+  static values = {
+    options: Object,
+    response: String,
+  };
+  static targets = ["intro", "nickname", "response"];
 
   submit(e) {
     if (this.responseTarget.value === "") {
@@ -16,12 +19,17 @@ export default class WebauthnRegistrationController extends Controller {
     }
   }
 
-  createCredential() {
-    create(this.options).then((response) => {
-      this.responseTarget.value = JSON.stringify(response);
+  async createCredential() {
+    const response = await create(this.options);
 
-      this.element.requestSubmit();
-    });
+    this.responseValue = JSON.stringify(response);
+    this.responseTarget.value = JSON.stringify(response);
+  }
+
+  responseValueChanged(response) {
+    const responsePresent = response !== "";
+    this.introTarget.toggleAttribute("hidden", responsePresent);
+    this.nicknameTarget.toggleAttribute("hidden", !responsePresent);
   }
 
   get options() {
diff --git a/app/assets/stylesheets/koi/base/_flow.scss b/app/assets/stylesheets/koi/base/_flow.scss
new file mode 100644
index 000000000..7630019b6
--- /dev/null
+++ b/app/assets/stylesheets/koi/base/_flow.scss
@@ -0,0 +1,8 @@
+/*
+FLOW COMPOSITION
+Like the Every Layout stack: https://every-layout.dev/layouts/stack/
+Info about this implementation: https://piccalil.li/quick-tip/flow-utility/
+*/
+.flow > * + * {
+  margin-top: var(--flow-space, 1em);
+}
diff --git a/app/assets/stylesheets/koi/base/_index.scss b/app/assets/stylesheets/koi/base/_index.scss
index eae4b74dd..ef550c0f2 100644
--- a/app/assets/stylesheets/koi/base/_index.scss
+++ b/app/assets/stylesheets/koi/base/_index.scss
@@ -1,8 +1,10 @@
 @use "button";
 @use "icon";
 @use "input";
+@use "flow";
 @use "link";
 @use "list";
+@use "repel";
 @use "tables";
 @use "typography";
 
diff --git a/app/assets/stylesheets/koi/base/_repel.scss b/app/assets/stylesheets/koi/base/_repel.scss
new file mode 100644
index 000000000..78179a7e1
--- /dev/null
+++ b/app/assets/stylesheets/koi/base/_repel.scss
@@ -0,0 +1,23 @@
+/*
+REPEL
+A little layout that pushes items away from each other where
+there is space in the viewport and stacks on small viewports
+
+CUSTOM PROPERTIES AND CONFIGURATION
+--gutter (var(--space-s-m)): This defines the space
+between each item.
+
+--repel-vertical-alignment How items should align vertically.
+Can be any acceptable flexbox alignment value.
+*/
+.repel {
+  display: flex;
+  flex-wrap: wrap;
+  justify-content: space-between;
+  align-items: var(--repel-vertical-alignment, center);
+  gap: var(--gutter, var(--space-s-m));
+}
+
+.repel[data-nowrap] {
+  flex-wrap: nowrap;
+}
diff --git a/app/controllers/concerns/koi/controller/has_webauthn.rb b/app/controllers/concerns/koi/controller/has_webauthn.rb
index c18326a64..2b7346b34 100644
--- a/app/controllers/concerns/koi/controller/has_webauthn.rb
+++ b/app/controllers/concerns/koi/controller/has_webauthn.rb
@@ -36,7 +36,8 @@ def webauthn_authenticate!
           Admin::Credential.find_by!(external_id: credential.id)
         end
 
-        stored_credential.update!(sign_count: webauthn_credential.sign_count)
+        stored_credential.update(sign_count: webauthn_credential.sign_count)
+        stored_credential.touch
 
         stored_credential.admin
       end
diff --git a/app/views/admin/admin_users/index.html.erb b/app/views/admin/admin_users/index.html.erb
index 202e317c0..8bedd81ab 100644
--- a/app/views/admin/admin_users/index.html.erb
+++ b/app/views/admin/admin_users/index.html.erb
@@ -15,6 +15,9 @@
   <% row.select %>
   <% row.link :name, url: :admin_admin_user_path %>
   <% row.text :email %>
+  <% row.boolean :credentials, label: "Passkey" do |cell| %>
+    <%= cell.value.any? ? "Yes" : "No" %>
+  <% end %>
 <% end %>
 
 <%= table_pagination_with(collection:) %>
diff --git a/app/views/admin/admin_users/show.html+self.erb b/app/views/admin/admin_users/show.html+self.erb
index 3a8b88b68..d68197b73 100644
--- a/app/views/admin/admin_users/show.html+self.erb
+++ b/app/views/admin/admin_users/show.html+self.erb
@@ -11,10 +11,9 @@
   <%= builder.date :last_sign_in_at, label: { text: "Last sign in" } %>
 <% end %>
 
-<h3>Passkeys</h3>
+<div class="repel">
+  <h3>Passkeys</h3>
+  <%= kpop_link_to "New passkey", new_admin_admin_user_credential_path(admin), class: "button button--primary" %>
+</div>
 
 <%= render "admin/credentials/credentials", admin: %>
-
-<div class="actions-group">
-  <%= kpop_link_to "Add this device", new_admin_admin_user_credential_path(admin), class: "button button--primary" %>
-</div>
diff --git a/app/views/admin/credentials/_credentials.html+self.erb b/app/views/admin/credentials/_credentials.html+self.erb
new file mode 100644
index 000000000..3165c4920
--- /dev/null
+++ b/app/views/admin/credentials/_credentials.html+self.erb
@@ -0,0 +1,10 @@
+<%= table_with(id: dom_id(admin, :credentials), collection: admin.credentials) do |t, c| %>
+  <% t.text :nickname, label: "Name" %>
+  <% t.date :updated_at, label: "Last use" do |date| %>
+    <%= date unless c.created_at == c.updated_at %>
+  <% end %>
+  <% t.cell :actions, label: "" do %>
+    <%= link_to("Remove passkey", admin_admin_user_credential_path(admin, c),
+                data: { turbo_method: :delete }) %>
+  <% end %>
+<% end %>
diff --git a/app/views/admin/credentials/_credentials.html.erb b/app/views/admin/credentials/_credentials.html.erb
index 4780404f8..50c6cd00e 100644
--- a/app/views/admin/credentials/_credentials.html.erb
+++ b/app/views/admin/credentials/_credentials.html.erb
@@ -1,7 +1,6 @@
 <%= table_with(id: dom_id(admin, :credentials), collection: admin.credentials) do |t, c| %>
-  <% t.text :nickname %>
-  <% t.number :sign_count %>
-  <% t.cell :actions, label: "" do %>
-    <%= button_to "Remove device", admin_admin_user_credential_path(admin, c), method: :delete, class: "button button--text" %>
-  <% end if admin == current_admin %>
+  <% t.text :nickname, label: "Name" %>
+  <% t.date :updated_at, label: "Last use" do |date| %>
+    <%= date unless c.created_at == c.updated_at %>
+  <% end %>
 <% end %>
diff --git a/app/views/admin/credentials/new.html.erb b/app/views/admin/credentials/new.html.erb
index 02b2bc93a..0946b2236 100644
--- a/app/views/admin/credentials/new.html.erb
+++ b/app/views/admin/credentials/new.html.erb
@@ -1,14 +1,37 @@
-<%= render Kpop::ModalComponent.new(title: "Register device") do %>
+<%= render Kpop::ModalComponent.new(title: "New passkey") do %>
   <%= form_with model: admin.credentials.new,
                 url:   admin_admin_user_credentials_path(admin),
+                class: "flow prose",
                 data:  {
                   controller:                          "webauthn-registration",
                   action:                              "submit->webauthn-registration#submit",
                   webauthn_registration_options_value: { publicKey: options },
                 } do |form| %>
-    <%= form.govuk_text_field :nickname %>
     <%= form.hidden_field :response, data: { webauthn_registration_target: "response" } %>
-
-    <%= form.admin_save %>
+    <section class="flow prose" data-webauthn-registration-target="intro">
+      <p>
+        Passkeys are secure secrets that are stored by your device.
+        You will need the device where your passkey is stored to log in.
+      </p>
+      <p>
+        Unlike a password, your password doesn't get sent to the server when you log
+        in and can't be stolen in a data breach. When you log in with a passkey,
+        your operating system will prompt you for permission to use the passkey
+        secret to authenticate the login attempt.
+      </p>
+      <p>
+        We recommend that you store your passkey on your phone or cloud account.
+        Depending on your browser, you may need to choose "more options" to see
+        a QR code that you can scan with your phone.
+      </p>
+    </section>
+    <section class="flow" data-webauthn-registration-target="nickname" hidden>
+      <%= form.govuk_text_field :nickname, label: { text: "Passkey name" } do %>
+        Enter a name for this passkey to help you distinguish it from other passkeys you may have for this site.
+        <br>
+        Example: My Phone, Chrome, iCloud, 1Password
+      <% end %>
+    </section>
+    <%= form.admin_save("Next") %>
   <% end %>
 <% end %>