Make WebAuthn clone detection actually block, and fix false positives

Two problems with sign-count clone detection:

- suspicious_sign_count? flagged the case where both the stored and presented
  counts are 0. Most synced passkeys (Apple/Google) report 0 every time, so every
  legitimate sign-in was flagged — drowning real signals in noise. Per WebAuthn
  §6.1.1 a 0 counter means "no counter"; only flag when BOTH counts are non-zero
  and the new one does not advance.

- On a suspicious count the controller only logged a warning and then continued
  to authenticate and overwrite the stored counter. A cloned credential therefore
  worked indefinitely. webauthn_verify now rejects the sign-in (no session, no
  counter update) and emails the user via a new SecurityMailer#suspicious_passkey_used.

Tests cover the corrected classification (synced/first-use/normal vs equal/
decreasing) and the new alert email.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

app/controllers/sessions_controller.rb

@@ -295,10 +295,14 @@ class SessionsController < ApplicationController
         sign_count: stored_credential.sign_count
       )
 
-      # Check for suspicious sign count (possible clone)
+      # Clone detection: a non-advancing signature counter signals the credential
+      # may have been copied. Reject the sign-in (do NOT create a session or update
+      # the stored counter) and alert the user, per WebAuthn §6.1.1.
       if stored_credential.suspicious_sign_count?(webauthn_credential.sign_count)
-        Rails.logger.warn "Suspicious WebAuthn sign count for user #{user.id}, credential #{stored_credential.id}"
-        # You might want to notify admins or temporarily disable the credential
+        Rails.logger.warn "Suspicious WebAuthn sign count for user #{user.id}, credential #{stored_credential.id} (stored=#{stored_credential.sign_count}, presented=#{webauthn_credential.sign_count})"
+        SecurityMailer.suspicious_passkey_used(user, nickname: stored_credential.display_name, **security_event_context).deliver_later
+        render json: {error: "Passkey authentication could not be completed. Please contact support."}, status: :unprocessable_entity
+        return
       end
 
       # Update credential usage

app/mailers/security_mailer.rb

@@ -40,6 +40,12 @@ class SecurityMailer < ApplicationMailer
     mail subject: "#{SUBJECT_PREFIX}An API key was revoked on your account", to: user.email_address
   end
 
+  def suspicious_passkey_used(user, nickname:, ip:, user_agent:, occurred_at:)
+    assign_context(user, ip, user_agent, occurred_at)
+    @nickname = nickname
+    mail subject: "#{SUBJECT_PREFIX}A passkey sign-in was blocked", to: user.email_address
+  end
+
   def email_address_changed(user, recipient:, old_email:, new_email:, ip:, user_agent:, occurred_at:)
     assign_context(user, ip, user_agent, occurred_at)
     @recipient = recipient

app/models/webauthn_credential.rb

@@ -52,13 +52,17 @@ class WebauthnCredential < ApplicationRecord
     end
   end
 
-  # Check if sign count is suspicious (clone detection)
+  # Check if sign count is suspicious (clone detection).
+  #
+  # Per WebAuthn §6.1.1, a signature counter of 0 means the authenticator does
+  # not implement a counter (true of most synced passkeys — Apple/Google report
+  # 0 every time), so it cannot be used for clone detection. Only when BOTH the
+  # stored and presented counts are non-zero does a non-increasing value signal
+  # a possible clone.
   def suspicious_sign_count?(new_sign_count)
-    return false if sign_count.zero? && new_sign_count > 0 # First use
-    return false if new_sign_count > sign_count # Normal increment
+    return false if sign_count.zero? || new_sign_count.zero?
 
-    # Sign count didn't increase - possible clone
-    true
+    new_sign_count <= sign_count
   end
 
   # Format for display in UI

app/views/security_mailer/suspicious_passkey_used.html.erb

@@ -0,0 +1,16 @@
+<p>Hello,</p>
+
+<p>
+  A sign-in to your Clinch account (<strong><%= @user.email_address %></strong>)
+  using your passkey (<strong><%= @nickname %></strong>) was <strong>blocked</strong>
+  because its security counter did not advance as expected. This can indicate the
+  passkey has been copied (cloned).
+</p>
+
+<p>
+  If this was you and you are unable to sign in, remove this passkey and register
+  a new one. If you do not recognise this activity, treat it as a compromise:
+  remove the passkey and review your account security.
+</p>
+
+<%= render "event_metadata" %>

app/views/security_mailer/suspicious_passkey_used.text.erb

@@ -0,0 +1,11 @@
+Hello,
+
+A sign-in to your Clinch account (<%= @user.email_address %>) using your passkey
+("<%= @nickname %>") was BLOCKED because its security counter did not advance as
+expected. This can indicate the passkey has been copied (cloned).
+
+If this was you and you are unable to sign in, remove this passkey and register a
+new one. If you do not recognise this activity, treat it as a compromise: remove
+the passkey and review your account security.
+
+<%= render "event_metadata" %>

test/mailers/security_mailer_test.rb

@@ -54,6 +54,15 @@ class SecurityMailerTest < ActionMailer::TestCase
     assert_bodies_contain email, "Old MacBook"
   end
 
+  test "suspicious_passkey_used warns about a blocked clone sign-in" do
+    email = SecurityMailer.suspicious_passkey_used(@user, nickname: "Yubikey-5", **CONTEXT)
+
+    assert_equal [@user.email_address], email.to
+    assert_match(/blocked/i, email.subject)
+    assert_bodies_contain email, "Yubikey-5"
+    assert_bodies_match email, /clon/i
+  end
+
   test "api_key_created includes the key name" do
     email = SecurityMailer.api_key_created(@user, name: "CI bot", **CONTEXT)
 

test/models/webauthn_credential_test.rb

@@ -0,0 +1,29 @@
+require "test_helper"
+
+class WebauthnCredentialTest < ActiveSupport::TestCase
+  # suspicious_sign_count?(new_sign_count) — clone detection per WebAuthn §6.1.1.
+  # Build an in-memory credential with a given stored sign_count; no persistence
+  # needed since the method only reads self.sign_count.
+  def credential(stored:)
+    WebauthnCredential.new(sign_count: stored)
+  end
+
+  test "does not flag when the authenticator reports no counter (synced passkeys)" do
+    # Both 0 -> authenticator doesn't implement a counter; must NOT be suspicious.
+    refute credential(stored: 0).suspicious_sign_count?(0)
+    # Stored 0, first real use.
+    refute credential(stored: 0).suspicious_sign_count?(5)
+    # Stored non-zero but authenticator now reports 0 -> no counter, not a clone.
+    refute credential(stored: 5).suspicious_sign_count?(0)
+  end
+
+  test "does not flag a normal increasing counter" do
+    refute credential(stored: 5).suspicious_sign_count?(6)
+    refute credential(stored: 1).suspicious_sign_count?(1000)
+  end
+
+  test "flags a non-advancing counter as a possible clone" do
+    assert credential(stored: 5).suspicious_sign_count?(5), "equal count is suspicious"
+    assert credential(stored: 5).suspicious_sign_count?(3), "decreasing count is suspicious"
+  end
+end