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

app/controllers/users_controller.rb

@@ -1578,13 +1578,12 @@ def register_second_factor_security_key
     params.require(:attestation)
     params.require(:clientData)
 
-    ::DiscourseWebauthn::SecurityKeyRegistrationService.new(
+    ::DiscourseWebauthn::RegistrationService.new(
       current_user,
       params,
-      challenge: DiscourseWebauthn.challenge(current_user, secure_session),
-      rp_id: DiscourseWebauthn.rp_id,
-      origin: Discourse.base_url,
-    ).register_second_factor_security_key
+      session: secure_session,
+      factor_type: UserSecurityKey.factor_types[:second_factor],
+    ).register_security_key
     render json: success_json
   rescue ::DiscourseWebauthn::SecurityKeyError => err
     render json: failed_json.merge(error: err.message)
@@ -1631,7 +1630,7 @@ def enable_second_factor_totp
   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 @@ def valid_second_factor_method_for_user?(method)
   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),
-      rp_id: DiscourseWebauthn.rp_id,
-      origin: Discourse.base_url,
+      session: secure_session,
+      factor_type: UserSecurityKey.factor_types[:second_factor],
     ).authenticate_security_key
   end
 

app/models/user.rb

@@ -1722,10 +1722,20 @@ def create_or_fetch_secure_identifier
     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.where(factor_type: UserSecurityKey.factor_types[:second_factor]).pluck(
+      :credential_id,
+    )
+  end
+
+  def passkey_credential_ids
     security_keys
       .select(:credential_id)
-      .where(factor_type: UserSecurityKey.factor_types[:second_factor])
+      .where(factor_type: UserSecurityKey.factor_types[:first_factor])
       .pluck(:credential_id)
   end
 

config/locales/server.en.yml

@@ -1010,6 +1010,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."
@@ -1018,6 +1019,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 @@ class InvalidRelyingPartyIdError < SecurityKeyError
   end
   class UserVerificationError < SecurityKeyError
   end
+  class UserPresenceError < SecurityKeyError
+  end
   class ChallengeMismatchError < SecurityKeyError
   end
   class InvalidTypeError < SecurityKeyError
@@ -34,7 +32,9 @@ class CredentialIdInUseError < SecurityKeyError
   end
   class MalformedAttestationError < SecurityKeyError
   end
-  class NotFoundError < SecurityKeyError
+  class KeyNotFoundError < SecurityKeyError
+  end
+  class MalformedPublicKeyCredentialError < SecurityKeyError
   end
   class OwnershipError < SecurityKeyError
   end
@@ -68,7 +68,18 @@ def self.challenge(user, secure_session)
   end
 
   def self.rp_id
-    Discourse.current_hostname
+    Rails.env.production? ? Discourse.current_hostname : "localhost"
+  end
+
+  def self.origin
+    case Rails.env
+    when "development"
+      "http://localhost:4200"
+    when "test"
+      "http://localhost:3000"
+    else
+      Discourse.base_url
+    end
   end
 
   def self.rp_name

lib/webauthn/security_key_authentication_service.rb → lib/discourse_webauthn/authentication_service.rb

@@ -2,21 +2,25 @@
 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
 
       # 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])
-      raise(NotFoundError, I18n.t("webauthn.validation.not_found_error")) if security_key.blank?
-      if security_key.user != @current_user
+      raise(KeyNotFoundError, I18n.t("webauthn.validation.not_found_error")) if security_key.blank?
+      if @factor_type == UserSecurityKey.factor_types[:second_factor] &&
+           security_key.user != @current_user
         raise(OwnershipError, I18n.t("webauthn.validation.ownership_error"))
       end
 
@@ -49,11 +53,12 @@ def authenticate_security_key
       # 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 +91,9 @@ def authenticate_security_key
 
       # 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/webauthn/security_key_base_validation_service.rb → lib/discourse_webauthn/base_validation_service.rb

@@ -1,11 +1,12 @@
 # frozen_string_literal: true
 
 module DiscourseWebauthn
-  class SecurityKeyBaseValidationService
-    def initialize(current_user, params, challenge_params)
+  class BaseValidationService
+    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 @@ def validate_rp_id_hash
       )
     end
 
-    def validate_user_verification
+    ## 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("")
+      # bit 2 - user verification
+      return if flags[2] == "1"
       raise(UserVerificationError, I18n.t("webauthn.validation.user_verification_error"))
     end
 
@@ -52,15 +70,16 @@ def client_data
     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/webauthn/security_key_registration_service.rb → lib/discourse_webauthn/registration_service.rb

@@ -3,12 +3,12 @@
 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 @@ def register_second_factor_security_key
       # 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 @@ def register_second_factor_security_key
       #     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 @@ def register_second_factor_security_key
       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 @@
   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