DEV: Refactor webauthn to support passkeys (1/3) (#23586)

This is part 1 of 3, split up of PR #23529. This PR refactors the
webauthn code to support passkey authentication/registration.

Passkeys aren't used yet, that is coming in PRs 2 and 3.

Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>

app/controllers/users_controller.rb

@@ -1578,13 +1578,12 @@ class UsersController < ApplicationController
     params.require(:attestation)
     params.require(:clientData)
 
-    ::DiscourseWebauthn::SecurityKeyRegistrationService.new(
+    ::DiscourseWebauthn::RegistrationService.new(
       current_user,
       params,
-      challenge: DiscourseWebauthn.challenge(current_user, secure_session),
+      session: secure_session,
-      rp_id: DiscourseWebauthn.rp_id,
+      factor_type: UserSecurityKey.factor_types[:second_factor],
-      origin: Discourse.base_url,
+    ).register_security_key
-    ).register_second_factor_security_key
     render json: success_json
   rescue ::DiscourseWebauthn::SecurityKeyError => err
     render json: failed_json.merge(error: err.message)
@@ -1631,7 +1630,7 @@ class UsersController < ApplicationController
   def disable_second_factor
     # delete all second factors for a user
     current_user.user_second_factors.destroy_all
-    current_user.security_keys.destroy_all
+    current_user.second_factor_security_keys.destroy_all
 
     Jobs.enqueue(
       :critical_user_email,

app/models/concerns/second_factor_manager.rb

@@ -163,12 +163,11 @@ module SecondFactorManager
   end
 
   def authenticate_security_key(secure_session, security_key_credential)
-    ::DiscourseWebauthn::SecurityKeyAuthenticationService.new(
+    ::DiscourseWebauthn::AuthenticationService.new(
       self,
       security_key_credential,
-      challenge: DiscourseWebauthn.challenge(self, secure_session),
+      session: secure_session,
-      rp_id: DiscourseWebauthn.rp_id,
+      factor_type: UserSecurityKey.factor_types[:second_factor],
-      origin: Discourse.base_url,
     ).authenticate_security_key
   end
 

app/models/user.rb

@@ -1722,11 +1722,18 @@ class User < ActiveRecord::Base
     new_secure_identifier
   end
 
+  def second_factor_security_keys
+    security_keys.where(factor_type: UserSecurityKey.factor_types[:second_factor])
+  end
+
   def second_factor_security_key_credential_ids
-    security_keys
+    second_factor_security_keys.pluck(:credential_id)
-      .select(:credential_id)
+  end
-      .where(factor_type: UserSecurityKey.factor_types[:second_factor])
+
-      .pluck(:credential_id)
+  def passkey_credential_ids
+    security_keys.where(factor_type: UserSecurityKey.factor_types[:first_factor]).pluck(
+      :credential_id,
+    )
   end
 
   def encoded_username(lower: false)

config/locales/server.en.yml

@@ -1012,6 +1012,7 @@ en:
       invalid_origin_error: "The origin of the authentication request does not match the server origin."
       malformed_attestation_error: "There was an error decoding the attestation data."
       invalid_relying_party_id_error: "The Relying Party ID of the authentication request does not match the server Relying Party ID."
+      user_presence_error: "User presence is required."
       user_verification_error: "User verification is required."
       unsupported_public_key_algorithm_error: "The provided public key algorithm is not supported by the server."
       unsupported_attestation_format_error: "The attestation format is not supported by the server."
@@ -1020,6 +1021,7 @@ en:
       ownership_error: "The security key is not owned by the user."
       not_found_error: "A security key with the provided credential ID could not be found."
       unknown_cose_algorithm_error: "The algorithm used for the security key is not recognized."
+      malformed_public_key_credential_error: "The provided public key is invalid."
 
   topic_flag_types:
     spam:

lib/discourse_webauthn.rb

@@ -1,8 +1,4 @@
 # frozen_string_literal: true
-require "webauthn/challenge_generator"
-require "webauthn/security_key_base_validation_service"
-require "webauthn/security_key_registration_service"
-require "webauthn/security_key_authentication_service"
 
 module DiscourseWebauthn
   ACCEPTABLE_REGISTRATION_TYPE = "webauthn.create"
@@ -22,6 +18,8 @@ module DiscourseWebauthn
   end
   class UserVerificationError < SecurityKeyError
   end
+  class UserPresenceError < SecurityKeyError
+  end
   class ChallengeMismatchError < SecurityKeyError
   end
   class InvalidTypeError < SecurityKeyError
@@ -34,7 +32,9 @@ module DiscourseWebauthn
   end
   class MalformedAttestationError < SecurityKeyError
   end
-  class NotFoundError < SecurityKeyError
+  class KeyNotFoundError < SecurityKeyError
+  end
+  class MalformedPublicKeyCredentialError < SecurityKeyError
   end
   class OwnershipError < SecurityKeyError
   end
@@ -68,7 +68,21 @@ module DiscourseWebauthn
   end
 
   def self.rp_id
-    Discourse.current_hostname
+    Rails.env.production? ? Discourse.current_hostname : "localhost"
+  end
+
+  def self.origin
+    case Rails.env
+    when "development"
+      # defaults to the Ember CLI local port
+      # you might need to change this and the rp_id above
+      # if you are using a non-default port/hostname locally
+      "http://localhost:4200"
+    when "test"
+      "http://localhost:3000"
+    else
+      Discourse.base_url
+    end
   end
 
   def self.rp_name

lib/discourse_webauthn/authentication_service.rb

@@ -2,21 +2,26 @@
 require "cose"
 
 module DiscourseWebauthn
-  class SecurityKeyAuthenticationService < SecurityKeyBaseValidationService
+  class AuthenticationService < BaseValidationService
     ##
     # See https://w3c.github.io/webauthn/#sctn-verifying-assertion for
     # the steps followed here. Memoized methods are called in their
     # place in the step flow to make the process clearer.
     def authenticate_security_key
       if @params.blank? || (!@params.is_a?(Hash) && !@params.is_a?(ActionController::Parameters))
-        return false
+        raise(
+          MalformedPublicKeyCredentialError,
+          I18n.t("webauthn.validation.malformed_public_key_credential_error"),
+        )
       end
 
+      security_key = UserSecurityKey.find_by(credential_id: @params[:credentialId])
+      raise(KeyNotFoundError, I18n.t("webauthn.validation.not_found_error")) if security_key.blank?
+
       # 3. Identify the user being authenticated and verify that this user is the
       #    owner of the public key credential source credentialSource identified by credential.id:
-      security_key = UserSecurityKey.find_by(credential_id: @params[:credentialId])
+      if @factor_type == UserSecurityKey.factor_types[:second_factor] &&
-      raise(NotFoundError, I18n.t("webauthn.validation.not_found_error")) if security_key.blank?
+           (@current_user == nil || security_key.user == nil || security_key.user != @current_user)
-      if security_key.user != @current_user
         raise(OwnershipError, I18n.t("webauthn.validation.ownership_error"))
       end
 
@@ -49,11 +54,12 @@ module DiscourseWebauthn
       # 13. Verify that the User Present bit of the flags in authData is set.
       # https://blog.bigbinary.com/2011/07/20/ruby-pack-unpack.html
       #
-      # bit 0 is the least significant bit - LSB first
+      validate_user_presence
+
       #
       # 14. If user verification is required for this registration, verify that
       #     the User Verified bit of the flags in authData is set.
-      validate_user_verification
+      validate_user_verification if @factor_type == UserSecurityKey.factor_types[:first_factor]
 
       # 15. Verify that the values of the client extension outputs in clientExtensionResults and the authenticator
       #     extension outputs in the extensions in authData are as expected, considering the client extension input
@@ -86,6 +92,9 @@ module DiscourseWebauthn
 
       # Success! Update the last used at time for the key.
       security_key.update(last_used: Time.zone.now)
+
+      # Return security key record so controller can use it to update the session
+      security_key
     rescue OpenSSL::PKey::PKeyError
       raise(PublicKeyError, I18n.t("webauthn.validation.public_key_error"))
     end

lib/discourse_webauthn/base_validation_service.rb

@@ -1,11 +1,12 @@
 # frozen_string_literal: true
 
 module DiscourseWebauthn
-  class SecurityKeyBaseValidationService
+  class BaseValidationService
-    def initialize(current_user, params, challenge_params)
+    def initialize(current_user, params, session:, factor_type:)
       @current_user = current_user
       @params = params
-      @challenge_params = challenge_params
+      @factor_type = factor_type
+      @session = session
     end
 
     def validate_webauthn_type(type_to_check)
@@ -31,9 +32,26 @@ module DiscourseWebauthn
       )
     end
 
+    ## flags per specification
+    # https://www.w3.org/TR/webauthn-2/#sctn-authenticator-data
+    # bit 0 - user presence
+    # bit 1 - reserved for future use
+    # bit 2 - user verification
+    # bit 3-5 - reserved for future use
+    # bit 6 - attested credential data
+    # bit 7 - extension data
+
+    def validate_user_presence
+      flags = auth_data[32].unpack("b*")[0].split("")
+      # bit 0 - user presence
+      return if flags[0] == "1"
+      raise(UserPresenceError, I18n.t("webauthn.validation.user_presence_error"))
+    end
+
     def validate_user_verification
       flags = auth_data[32].unpack("b*")[0].split("")
-      return if flags[0] == "1"
+      # bit 2 - user verification
+      return if flags[2] == "1"
       raise(UserVerificationError, I18n.t("webauthn.validation.user_verification_error"))
     end
 
@@ -52,15 +70,16 @@ module DiscourseWebauthn
     end
 
     def challenge_match?
-      Base64.decode64(client_data["challenge"]) == @challenge_params[:challenge]
+      Base64.decode64(client_data["challenge"]) ==
+        DiscourseWebauthn.challenge(@current_user, @session)
     end
 
     def origin_match?
-      client_data["origin"] == @challenge_params[:origin]
+      client_data["origin"] == DiscourseWebauthn.origin
     end
 
     def rp_id_hash_match?
-      auth_data[0..31] == OpenSSL::Digest::SHA256.digest(@challenge_params[:rp_id])
+      auth_data[0..31] == OpenSSL::Digest::SHA256.digest(DiscourseWebauthn.rp_id)
     end
 
     def client_data_hash

lib/discourse_webauthn/registration_service.rb

@@ -3,12 +3,12 @@ require "cbor"
 require "cose"
 
 module DiscourseWebauthn
-  class SecurityKeyRegistrationService < SecurityKeyBaseValidationService
+  class RegistrationService < BaseValidationService
     ##
     # See https://w3c.github.io/webauthn/#sctn-registering-a-new-credential for
     # the registration steps followed here. Memoized methods are called in their
     # place in the step flow to make the process clearer.
-    def register_second_factor_security_key
+    def register_security_key
       # 4. Verify that the value of C.type is webauthn.create.
       validate_webauthn_type(::DiscourseWebauthn::ACCEPTABLE_REGISTRATION_TYPE)
 
@@ -38,11 +38,12 @@ module DiscourseWebauthn
       # 11. Verify that the User Present bit of the flags in authData is set.
       # https://blog.bigbinary.com/2011/07/20/ruby-pack-unpack.html
       #
-      # bit 0 is the least significant bit - LSB first
+      validate_user_presence
+
       #
       # 12. If user verification is required for this registration, verify that
       #     the User Verified bit of the flags in authData is set.
-      validate_user_verification
+      validate_user_verification if @factor_type == UserSecurityKey.factor_types[:first_factor]
 
       # 13. Verify that the "alg" parameter in the credential public key in authData matches the alg
       #     attribute of one of the items in options.pubKeyCredParams.
@@ -100,7 +101,7 @@ module DiscourseWebauthn
       #     the Relying Party SHOULD fail this registration ceremony, or it MAY decide to accept
       #     the registration, e.g. while deleting the older registration.
       encoded_credential_id = Base64.strict_encode64(credential_id)
-      endcoded_public_key = Base64.strict_encode64(credential_public_key_bytes)
+      encoded_public_key = Base64.strict_encode64(credential_public_key_bytes)
       if UserSecurityKey.exists?(credential_id: encoded_credential_id)
         raise(CredentialIdInUseError, I18n.t("webauthn.validation.credential_id_in_use_error"))
       end
@@ -112,9 +113,9 @@ module DiscourseWebauthn
       UserSecurityKey.create!(
         user: @current_user,
         credential_id: encoded_credential_id,
-        public_key: endcoded_public_key,
+        public_key: encoded_public_key,
         name: @params[:name],
-        factor_type: UserSecurityKey.factor_types[:second_factor],
+        factor_type: @factor_type,
       )
     rescue CBOR::UnpackError, CBOR::TypeError, CBOR::MalformedFormatError, CBOR::StackError
       raise MalformedAttestationError, I18n.t("webauthn.validation.malformed_attestation_error")

spec/fabricators/user_security_key_fabricator.rb

@@ -24,3 +24,9 @@ Fabricator(:user_security_key_with_random_credential, from: :user_security_key)
   credential_id { SecureRandom.base64(40) }
   public_key { SecureRandom.base64(40) }
 end
+
+Fabricator(:passkey_with_random_credential, from: :user_security_key) do
+  credential_id { SecureRandom.base64(40) }
+  public_key { SecureRandom.base64(40) }
+  factor_type { UserSecurityKey.factor_types[:first_factor] }
+end

spec/lib/discourse_webauthn/authentication_service_spec.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 require "discourse_webauthn"
-require "webauthn/security_key_registration_service"
 
 ##
 # These tests use the following parameters generated on a local discourse
@@ -12,7 +11,6 @@ require "webauthn/security_key_registration_service"
 # - signature
 # - authenticator_data
 # - client_data_origin
-# - challenge_params_origin
 #
 # To create another test (e.g. for a different COSE algorithm) you need to:
 #
@@ -24,21 +22,23 @@ require "webauthn/security_key_registration_service"
 # you need to add puts debugger statements (or use binding.pry) like so:
 #
 # puts client_data
-# puts signature
+# puts @params
-# puts auth_data
 #
-# The auth_data will have the challenge param, but you must Base64.decode64 to
+# The client_data will have the challenge param, but you must Base64.decode64 to
-# use it in the let(:challenge) variable. The signature and auth_data params
+# use it in the let(:challenge) variable.
-# can be used as is.
+#
+# puts Base64.decode64(client_data["challenge"])
 #
 # You also need to make sure that client_data_param has the exact same structure
-# and order of keys as auth_data, otherwise even with everything else right the
+# and order of keys, otherwise even with everything else right the
 # public key verification will fail.
 #
-# The origin params just need to be whatever your localhost URL for Discourse is.
+# @params will contain authenticatorData and signature which you can use as is.
+#
+# The origin param needs to be http://localhost:3000 (that's the port tests run on)
 
-RSpec.describe DiscourseWebauthn::SecurityKeyAuthenticationService do
+RSpec.describe DiscourseWebauthn::AuthenticationService do
-  subject(:service) { described_class.new(current_user, params, challenge_params) }
+  subject(:service) { described_class.new(current_user, params, **options) }
 
   let(:security_key_user) { current_user }
   let!(:security_key) do
@@ -47,7 +47,9 @@ RSpec.describe DiscourseWebauthn::SecurityKeyAuthenticationService do
       credential_id: credential_id,
       public_key: public_key,
       user: security_key_user,
+      factor_type: UserSecurityKey.factor_types[:second_factor],
       last_used: nil,
+      name: "Some key",
     )
   end
   let(:public_key) do
@@ -56,6 +58,7 @@ RSpec.describe DiscourseWebauthn::SecurityKeyAuthenticationService do
   let(:credential_id) do
     "mJAJ4CznTO0SuLkJbYwpgK75ao4KMNIPlU5KWM92nq39kRbXzI9mSv6GxTcsMYoiPgaouNw7b7zBiS4vsQaO6A=="
   end
+  let(:secure_session) { SecureSession.new("tester") }
   let(:challenge) { "81d4acfbd69eafa8f02bc2ecbec5267be8c9b28c1e0ba306d52b79f0f13d" }
   let(:client_data_challenge) { Base64.strict_encode64(challenge) }
   let(:client_data_webauthn_type) { "webauthn.get" }
@@ -87,29 +90,39 @@ RSpec.describe DiscourseWebauthn::SecurityKeyAuthenticationService do
       signature: signature,
     }
   end
-  ##
+
-  # The original key was generated in localhost
+  let(:options) do
-  let(:rp_id) { "localhost" }
+    { session: secure_session, factor_type: UserSecurityKey.factor_types[:second_factor] }
-  let(:challenge_params_origin) { "http://localhost:3000" }
+  end
-  let(:challenge_params) { { challenge: challenge, rp_id: rp_id, origin: challenge_params_origin } }
   let(:current_user) { Fabricate(:user) }
 
+  before do
+    # we have to stub here because the public key was created using this specific challenge
+    DiscourseWebauthn.stubs(:challenge).returns(challenge)
+  end
+
   it "updates last_used when the security key and params are valid" do
-    expect(service.authenticate_security_key).to eq(true)
+    expect(service.authenticate_security_key).to eq(security_key)
     expect(security_key.reload.last_used).not_to eq(nil)
   end
 
   context "when params is blank" do
     let(:params) { nil }
-    it "returns false with no validation" do
+    it "raises a MalformedPublicKeyCredentialError" do
-      expect(service.authenticate_security_key).to eq(false)
+      expect { service.authenticate_security_key }.to raise_error(
+        DiscourseWebauthn::MalformedPublicKeyCredentialError,
+        I18n.t("webauthn.validation.malformed_public_key_credential_error"),
+      )
     end
   end
 
   context "when params is not blank and not a hash" do
     let(:params) { "test" }
-    it "returns false with no validation" do
+    it "raises a MalformedPublicKeyCredentialError" do
-      expect(service.authenticate_security_key).to eq(false)
+      expect { service.authenticate_security_key }.to raise_error(
+        DiscourseWebauthn::MalformedPublicKeyCredentialError,
+        I18n.t("webauthn.validation.malformed_public_key_credential_error"),
+      )
     end
   end
 
@@ -118,7 +131,7 @@ RSpec.describe DiscourseWebauthn::SecurityKeyAuthenticationService do
 
     it "raises a NotFoundError" do
       expect { service.authenticate_security_key }.to raise_error(
-        DiscourseWebauthn::NotFoundError,
+        DiscourseWebauthn::KeyNotFoundError,
         I18n.t("webauthn.validation.not_found_error"),
       )
     end
@@ -135,6 +148,18 @@ RSpec.describe DiscourseWebauthn::SecurityKeyAuthenticationService do
     end
   end
 
+  context "when the second-factor authentication is initiated without a user" do
+    let(:current_user) { nil }
+    let(:security_key_user) { Fabricate(:user) }
+
+    it "raises an OwnershipError" do
+      expect { service.authenticate_security_key }.to raise_error(
+        DiscourseWebauthn::OwnershipError,
+        I18n.t("webauthn.validation.ownership_error"),
+      )
+    end
+  end
+
   context "when the client data webauthn type is not webauthn.get" do
     let(:client_data_webauthn_type) { "webauthn.explode" }
 
@@ -169,9 +194,9 @@ RSpec.describe DiscourseWebauthn::SecurityKeyAuthenticationService do
   end
 
   context "when the sha256 hash of the relaying party ID does not match the one in attestation.authData" do
-    let(:rp_id) { "bad_rp_id" }
-
     it "raises a InvalidRelyingPartyIdError" do
+      DiscourseWebauthn.stubs(:rp_id).returns("bad_rp_id")
+
       expect { service.authenticate_security_key }.to raise_error(
         DiscourseWebauthn::InvalidRelyingPartyIdError,
         I18n.t("webauthn.validation.invalid_relying_party_id_error"),
@@ -214,7 +239,7 @@ RSpec.describe DiscourseWebauthn::SecurityKeyAuthenticationService do
     let(:public_key) do
       "pAEDAzkBACBZAQCqsl50KrR5zVm/QT9vWkeGTGxby32m0QRtCRh2UWseqoG0ZmBhGeWEYvkdoYlB1jObQKEHsAeB+1NBf5q69/88AA5zv4fzrvCydCtL41EUsHYFEbaPGnB61zZmYVLTPI7BYa+fu4F4MzFa924s36tVlU/L7n04peviJVZW2C1YIQfwOGDZJSvUpqJoZMQtw1vGRfrb4cQKlHfrpDZUpa3QLE8phh4ce4nwtX1tUnUGgCy8sOaFVkDNufENGTNr8HdAIHcinUiax3yy/Q8LjSZb8UR2ha6oXSe1vRHhj001B/P/mr5AdVMxSrOT1sUNXWkHv8L8IzS/iTBQpsC8CADZIUMBAAE="
     end
-    let(:challenge_params_origin) { "http://localhost:4200" }
+    # This key was generated using this specific origin
     let(:client_data_origin) { "http://localhost:4200" }
 
     # This has to be in the exact same order with the same data as it was originally
@@ -231,7 +256,9 @@ RSpec.describe DiscourseWebauthn::SecurityKeyAuthenticationService do
     end
 
     it "updates last_used when the security key and params are valid" do
-      expect(service.authenticate_security_key).to eq(true)
+      DiscourseWebauthn.stubs(:origin).returns("http://localhost:4200")
+
+      expect(service.authenticate_security_key).to eq(security_key)
       expect(security_key.reload.last_used).not_to eq(nil)
     end
   end
@@ -241,4 +268,56 @@ RSpec.describe DiscourseWebauthn::SecurityKeyAuthenticationService do
       expect(COSE::Algorithm.find(alg)).not_to be_nil
     end
   end
+
+  describe "authenticating a valid passkey" do
+    let(:options) do
+      { factor_type: UserSecurityKey.factor_types[:first_factor], session: secure_session }
+    end
+
+    ##
+    # These are sourced from an actual key, see instructions at the top of this spec for details
+    #
+    let(:public_key) { valid_passkey_data[:public_key] }
+    let(:credential_id) { valid_passkey_data[:credential_id] }
+    let(:signature) { valid_passkey_auth_data[:signature] }
+    let(:authenticator_data) { valid_passkey_auth_data[:authenticatorData] }
+    let(:challenge) { valid_passkey_challenge }
+
+    let(:client_data_param) { passkey_client_data_param("webauthn.get") }
+
+    let!(:security_key) do
+      Fabricate(
+        :user_security_key,
+        credential_id: credential_id,
+        public_key: public_key,
+        user: security_key_user,
+        factor_type: UserSecurityKey.factor_types[:first_factor],
+        last_used: nil,
+        name: "A key",
+      )
+    end
+
+    it "works and returns the correct key credential" do
+      key = service.authenticate_security_key
+      expect(key).to eq(security_key)
+      expect(key.factor_type).to eq(UserSecurityKey.factor_types[:first_factor])
+    end
+
+    context "when the user verification flag in the key is false" do
+      it "raises a UserVerificationError" do
+        # simulate missing user verification in the key data
+        # by setting third bit to 0
+        flags = "10000010" # correct flag sequence is "10100010"
+        overriden_auth_data = service.send(:auth_data)
+        overriden_auth_data[32] = [flags].pack("b*")
+
+        service.instance_variable_set(:@auth_data, overriden_auth_data)
+
+        expect { service.authenticate_security_key }.to raise_error(
+          DiscourseWebauthn::UserVerificationError,
+          I18n.t("webauthn.validation.user_verification_error"),
+        )
+      end
+    end
+  end
 end

spec/lib/discourse_webauthn/registration_service_spec.rb

@@ -1,10 +1,10 @@
 # frozen_string_literal: true
 require "discourse_webauthn"
-require "webauthn/security_key_registration_service"
 
-RSpec.describe DiscourseWebauthn::SecurityKeyRegistrationService do
+RSpec.describe DiscourseWebauthn::RegistrationService do
-  subject(:service) { described_class.new(current_user, params, challenge_params) }
+  subject(:service) { described_class.new(current_user, params, **options) }
 
+  let(:secure_session) { SecureSession.new("tester") }
   let(:client_data_challenge) { Base64.encode64(challenge) }
   let(:client_data_webauthn_type) { "webauthn.create" }
   let(:client_data_origin) { "http://localhost:3000" }
@@ -18,7 +18,8 @@ RSpec.describe DiscourseWebauthn::SecurityKeyRegistrationService do
   ##
   # This attestation object was sourced by manually registering
   # a key with `navigator.credentials.create` and capturing the
-  # results in localhost.
+  # results in localhost. It does not have a user verification
+  # flag set (i.e. it is only usable as 2FA).
   let(:attestation) do
     "o2NmbXRkbm9uZWdhdHRTdG10oGhhdXRoRGF0YVjESZYN5YgOjGh0NBcPZHZgW4/krrmihjLHmVzzuoMdl2NBAAAAAAAAAAAAAAAAAAAAAAAAAAAAQFmvayWc8OPJ4jj4sevfxBmvUglDMZrFalyokYrdnqOVvudC0lQialaGQv72eBzJM2Qn1GfJI7lpBgFJMprisLSlAQIDJiABIVgg+23/BZux7LK0/KQgCiQGtdr51ar+vfTtHWpRtN17gOwiWCBstV918mugVBexg/rdZjTs0wN/upHFoyBiAJCaGVD8OA=="
   end
@@ -32,16 +33,19 @@ RSpec.describe DiscourseWebauthn::SecurityKeyRegistrationService do
   ##
   # The above attestation was generated in localhost; Discourse.current_hostname
   # returns test.localhost which we do not want
-  let(:rp_id) { "localhost" }
+  let(:options) do
-  let(:challenge_params) { { challenge: challenge, rp_id: rp_id, origin: "http://localhost:3000" } }
+    { session: secure_session, factor_type: UserSecurityKey.factor_types[:second_factor] }
-  let(:challenge) { "f1e04530f34a1b6a08d032d8550e23eb8330be04e4166008f26c0e1b42ad" }
+  end
+
+  let(:challenge) { DiscourseWebauthn.stage_challenge(current_user, secure_session).challenge }
+
   let(:current_user) { Fabricate(:user) }
 
   context "when the client data webauthn type is not webauthn.create" do
     let(:client_data_webauthn_type) { "webauthn.explode" }
 
     it "raises an InvalidTypeError" do
-      expect { service.register_second_factor_security_key }.to raise_error(
+      expect { service.register_security_key }.to raise_error(
         DiscourseWebauthn::InvalidTypeError,
         I18n.t("webauthn.validation.invalid_type_error"),
       )
@@ -52,7 +56,7 @@ RSpec.describe DiscourseWebauthn::SecurityKeyRegistrationService do
     let(:client_data_challenge) { Base64.encode64("invalid challenge") }
 
     it "raises a ChallengeMismatchError" do
-      expect { service.register_second_factor_security_key }.to raise_error(
+      expect { service.register_security_key }.to raise_error(
         DiscourseWebauthn::ChallengeMismatchError,
         I18n.t("webauthn.validation.challenge_mismatch_error"),
       )
@@ -63,7 +67,7 @@ RSpec.describe DiscourseWebauthn::SecurityKeyRegistrationService do
     let(:client_data_origin) { "https://someothersite.com" }
 
     it "raises a InvalidOriginError" do
-      expect { service.register_second_factor_security_key }.to raise_error(
+      expect { service.register_security_key }.to raise_error(
         DiscourseWebauthn::InvalidOriginError,
         I18n.t("webauthn.validation.invalid_origin_error"),
       )
@@ -71,10 +75,10 @@ RSpec.describe DiscourseWebauthn::SecurityKeyRegistrationService do
   end
 
   context "when the sha256 hash of the relaying party ID does not match the one in attestation.authData" do
-    let(:rp_id) { "bad_rp_id" }
-
     it "raises a InvalidRelyingPartyIdError" do
-      expect { service.register_second_factor_security_key }.to raise_error(
+      DiscourseWebauthn.stubs(:rp_id).returns("bad_rp_id")
+
+      expect { service.register_security_key }.to raise_error(
         DiscourseWebauthn::InvalidRelyingPartyIdError,
         I18n.t("webauthn.validation.invalid_relying_party_id_error"),
       )
@@ -88,7 +92,7 @@ RSpec.describe DiscourseWebauthn::SecurityKeyRegistrationService do
     end
 
     it "raises a UnsupportedPublicKeyAlgorithmError" do
-      expect { service.register_second_factor_security_key }.to raise_error(
+      expect { service.register_security_key }.to raise_error(
         DiscourseWebauthn::UnsupportedPublicKeyAlgorithmError,
         I18n.t("webauthn.validation.unsupported_public_key_algorithm_error"),
       )
@@ -106,7 +110,7 @@ RSpec.describe DiscourseWebauthn::SecurityKeyRegistrationService do
     end
 
     it "raises a UnsupportedAttestationFormatError" do
-      expect { service.register_second_factor_security_key }.to raise_error(
+      expect { service.register_security_key }.to raise_error(
         DiscourseWebauthn::UnsupportedAttestationFormatError,
         I18n.t("webauthn.validation.unsupported_attestation_format_error"),
       )
@@ -122,14 +126,14 @@ RSpec.describe DiscourseWebauthn::SecurityKeyRegistrationService do
   context "when the credential id is already in use for any user" do
     it "raises a CredentialIdInUseError" do
       # register the key to the current user
-      security_key = service.register_second_factor_security_key
+      security_key = service.register_security_key
 
       # update the key to be on a different user
       other_user = Fabricate(:user)
       security_key.update(user: other_user)
 
       # error!
-      expect { service.register_second_factor_security_key }.to raise_error(
+      expect { service.register_security_key }.to raise_error(
         DiscourseWebauthn::CredentialIdInUseError,
         I18n.t("webauthn.validation.credential_id_in_use_error"),
       )
@@ -142,10 +146,83 @@ RSpec.describe DiscourseWebauthn::SecurityKeyRegistrationService do
     end
 
     it "raises a MalformedAttestationError" do
-      expect { service.register_second_factor_security_key }.to raise_error(
+      expect { service.register_security_key }.to raise_error(
         DiscourseWebauthn::MalformedAttestationError,
         I18n.t("webauthn.validation.malformed_attestation_error"),
       )
     end
   end
+
+  context "when the user presence flag is false" do
+    it "raises a UserPresenceError" do
+      # simulate missing user presence by flipping first bit to 0
+      flags = "00000010"
+      overridenAuthData = service.send(:auth_data)
+      overridenAuthData[32] = [flags].pack("b*")
+
+      service.instance_variable_set(:@auth_data, overridenAuthData)
+
+      expect { service.register_security_key }.to raise_error(
+        DiscourseWebauthn::UserPresenceError,
+        I18n.t("webauthn.validation.user_presence_error"),
+      )
+    end
+  end
+
+  it "registers a valid second-factor key" do
+    key = service.register_security_key
+    expect(key).to be_a(UserSecurityKey)
+    expect(key.user).to eq(current_user)
+    expect(key.factor_type).to eq(UserSecurityKey.factor_types[:second_factor])
+  end
+
+  describe "registering a second factor key as first factor" do
+    let(:options) do
+      { factor_type: UserSecurityKey.factor_types[:first_factor], session: secure_session }
+    end
+
+    it "does not work since second-factor key does not have the user verification flag" do
+      expect { service.register_security_key }.to raise_error(
+        DiscourseWebauthn::UserVerificationError,
+        I18n.t("webauthn.validation.user_verification_error"),
+      )
+    end
+  end
+
+  describe "registering a passkey" do
+    let(:options) do
+      { factor_type: UserSecurityKey.factor_types[:first_factor], session: secure_session }
+    end
+
+    ##
+    # key registered locally using
+    # - localhost:3000 as the origin (via an origin override in discourse_webauthn.rb)
+    # - frontend webauthn.create has user verification flag enabled
+    let(:attestation) do
+      "o2NmbXRkbm9uZWdhdHRTdG10oGhhdXRoRGF0YVikSZYN5YgOjGh0NBcPZHZgW4/krrmihjLHmVzzuoMdl2NFAAAAAK3OAAI1vMYKZIsLJfHwVQMAICRXq4sFZ9XpWZOzfJ8EguJmoEPMzNVyFMUWQfT5u1QzpQECAyYgASFYILjOiAHAwNrXkCk/tmyYRiE87QyV/15wUvhcXhr1JfwtIlggClQywgQvSxTsqV/FSK0cNHTTmuwfzzREqE6eLDmPxmI="
+    end
+
+    it "works with a valid key" do
+      key = service.register_security_key
+      expect(key).to be_a(UserSecurityKey)
+      expect(key.user).to eq(current_user)
+      expect(key.factor_type).to eq(UserSecurityKey.factor_types[:first_factor])
+    end
+
+    context "when the user verification flag in the key is false" do
+      it "raises a UserVerificationError" do
+        # simulate missing user verification by flipping third bit to 0
+        flags = "10000010" # correct flag sequence is "10100010"
+        overriden_auth_data = service.send(:auth_data)
+        overriden_auth_data[32] = [flags].pack("b*")
+
+        service.instance_variable_set(:@auth_data, overriden_auth_data)
+
+        expect { service.register_security_key }.to raise_error(
+          DiscourseWebauthn::UserVerificationError,
+          I18n.t("webauthn.validation.user_verification_error"),
+        )
+      end
+    end
+  end
 end

spec/requests/session_controller_spec.rb

@@ -120,7 +120,7 @@ RSpec.describe SessionController do
           expect(response_body_parsed["challenge"]).to eq(
             DiscourseWebauthn.challenge(user, secure_session),
           )
-          expect(DiscourseWebauthn.rp_id).to eq(Discourse.current_hostname)
+          expect(DiscourseWebauthn.rp_id).to eq("localhost")
         end
       end
     end
@@ -2041,7 +2041,9 @@ RSpec.describe SessionController do
             expect(session[:current_user_id]).to eq(nil)
             response_body = response.parsed_body
             expect(response_body["failed"]).to eq("FAILED")
-            expect(response_body["error"]).to eq(I18n.t("login.invalid_security_key"))
+            expect(response_body["error"]).to eq(
+              I18n.t("webauthn.validation.malformed_public_key_credential_error"),
+            )
           end
         end
 

spec/requests/users_controller_spec.rb

@@ -5835,7 +5835,6 @@ RSpec.describe UsersController do
 
     context "when the creation parameters are invalid" do
       it "shows a security key error and does not create a key" do
-        stub_as_dev_localhost
         create_second_factor_security_key
         _response_parsed = response.parsed_body
 
@@ -5872,9 +5871,10 @@ RSpec.describe UsersController do
               user: user1,
               factor_type: UserSecurityKey.factor_types[:second_factor],
             )
+          Fabricate(:passkey_with_random_credential, user: user1)
         end
 
-        it "should disable all totp and security keys" do
+        it "should disable all totp and security keys (but not passkeys)" do
           expect_enqueued_with(
             job: :critical_user_email,
             args: {
@@ -5887,7 +5887,12 @@ RSpec.describe UsersController do
             expect(response.status).to eq(200)
 
             expect(user1.reload.user_second_factors).to be_empty
-            expect(user1.security_keys).to be_empty
+            expect(user1.second_factor_security_keys).to be_empty
+            expect(user1.security_keys.length).to eq(1)
+            expect(user1.security_keys[0].factor_type).to eq(
+              UserSecurityKey.factor_types[:first_factor],
+            )
+            expect(user1.passkey_credential_ids.length).to eq(1)
           end
         end
       end

spec/support/webauthn_integration_helpers.rb

@@ -17,6 +17,10 @@ module DiscourseWebauthnIntegrationHelpers
   # This is because the challenge is embedded
   # in the post data's authenticatorData and must match up. See
   # simulate_localhost_webauthn_challenge for a real example.
+
+  # All of the valid security key data is sourced from a localhost
+  # login (with origin http://localhost:3000).
+
   def valid_security_key_data
     {
       credential_id:
@@ -55,19 +59,45 @@ module DiscourseWebauthnIntegrationHelpers
     }
   end
 
-  # all of the valid security key data is sourced from a localhost
+  def simulate_localhost_webauthn_challenge
-  # login, if this is not set the specs for webauthn WILL NOT WORK
+    DiscourseWebauthn.stubs(:challenge).returns(valid_security_key_challenge_data[:challenge])
-  def stub_as_dev_localhost
-    Discourse.stubs(:current_hostname).returns("localhost")
-    Discourse.stubs(:base_url).returns("http://localhost:3000")
   end
 
-  def simulate_localhost_webauthn_challenge
+  # Passkey data sourced from a key generated in a local browser
-    stub_as_dev_localhost
+  # with webauthn.create that includes the user verification flag on localhost:3000
-    DiscourseWebauthn::ChallengeGenerator.stubs(:generate).returns(
+  # usin puts statements in the passkeys session controllers
-      DiscourseWebauthn::ChallengeGenerator::ChallengeSession.new(
+  def valid_passkey_challenge
-        challenge: valid_security_key_challenge_data[:challenge],
+    "66b47014ef72937d8320ed893dc797e8a9a6d5098b89b185ca3d439b3656"
-      ),
+  end
-    )
+
+  def passkey_client_data_param(type)
+    {
+      type: type,
+      challenge: Base64.strict_encode64(valid_passkey_challenge),
+      origin: "http://localhost:3000",
+      crossOrigin: false,
+    }
+  end
+
+  def valid_passkey_auth_data
+    {
+      clientData: Base64.strict_encode64(passkey_client_data_param("webauthn.get").to_json),
+      credentialId: "JFeriwVn1elZk7N8nwSC4magQ8zM1XIUxRZB9Pm7VDM=",
+      authenticatorData: "SZYN5YgOjGh0NBcPZHZgW4/krrmihjLHmVzzuoMdl2MFAAAAAA==",
+      signature:
+        "MEUCIG5AFaw2Nfy69hHjeRLqm3LzQRMFb+TRbUAz19WJymegAiEAyEEyGdAMB2/NBwRCHM47IwtjKWCLEtabAX2BaK6fD8g=",
+    }
+  end
+
+  def valid_passkey_data
+    {
+      credential_id: "JFeriwVn1elZk7N8nwSC4magQ8zM1XIUxRZB9Pm7VDM=",
+      public_key:
+        "pQECAyYgASFYILjOiAHAwNrXkCk/tmyYRiE87QyV/15wUvhcXhr1JfwtIlggClQywgQvSxTsqV/FSK0cNHTTmuwfzzREqE6eLDmPxmI=",
+    }
+  end
+
+  def simulate_localhost_passkey_challenge
+    DiscourseWebauthn.stubs(:challenge).returns(valid_passkey_challenge)
   end
 end

spec/system/user_page/user_preferences_security_spec.rb

@@ -14,6 +14,9 @@ describe "User preferences for Security", type: :system do
 
   describe "Security keys" do
     it "adds a 2F security key and logs in with it" do
+      # system specs run on their own host + port
+      DiscourseWebauthn.stubs(:origin).returns(current_host + ":" + Capybara.server_port.to_s)
+
       # simulate browser credential authorization
       options = ::Selenium::WebDriver::VirtualAuthenticatorOptions.new
       page.driver.browser.add_virtual_authenticator(options)