DEV: Update webauthn authentication documentation (#23787)

Also adds a `userHandle` check for first factor verification, though this is not yet implemented in Rails controllers and UI.

app/assets/javascripts/discourse/app/lib/webauthn.js

@@ -1,4 +1,5 @@
 import I18n from "I18n";
+
 export function stringToBuffer(str) {
   let buffer = new ArrayBuffer(str.length);
   let byteView = new Uint8Array(buffer);
@@ -33,7 +34,10 @@ export function getWebauthnCredential(
       type: "public-key",
     };
   });
+  // See https://w3c.github.io/webauthn/#sctn-verifying-assertion for the steps followed here.
 
+  // 1. Let options be a new PublicKeyCredentialRequestOptions structure configured to the Relying Party's needs
+  // 2. Call navigator.credentials.get() and pass options as the publicKey option.
   navigator.credentials
     .get({
       publicKey: {
@@ -48,8 +52,19 @@ export function getWebauthnCredential(
       },
     })
     .then((credential) => {
-      // 1. if there is a credential, check if the raw ID base64 matches
+      // 3. If credential.response is not an instance of AuthenticatorAssertionResponse, abort the ceremony.
-      // any of the allowed credential ids
+      // eslint-disable-next-line no-undef
+      if (!(credential.response instanceof AuthenticatorAssertionResponse)) {
+        return errorCallback(
+          I18n.t("login.security_key_invalid_response_error")
+        );
+      }
+
+      // 4. Let clientExtensionResults be the result of calling credential.getClientExtensionResults().
+      // We are not using this
+
+      // 5. If options.allowCredentials is not empty, verify that credential.id identifies one of the public key
+      // credentials listed in options.allowCredentials.
       if (
         !allowedCredentialIds.some(
           (credentialId) => bufferToBase64(credential.rawId) === credentialId
@@ -68,7 +83,9 @@ export function getWebauthnCredential(
         ),
         credentialId: bufferToBase64(credential.rawId),
       };
+
       successCallback(credentialData);
+      // steps 6+ of this flow are handled by lib/discourse_webauthn/authentication_service.rb
     })
     .catch((err) => {
       if (err.name === "NotAllowedError") {

config/locales/client.en.yml

@@ -2169,6 +2169,7 @@ en:
       security_key_not_allowed_error: "The security key authentication process either timed out or was cancelled."
       security_key_no_matching_credential_error: "No matching credentials could be found in the provided security key."
       security_key_support_missing_error: "Your current device or browser does not support the use of security keys. Please use a different method."
+      security_key_invalid_response_error: "The security key authentication process failed due to an invalid response."
       email_placeholder: "Email / Username"
       caps_lock_warning: "Caps Lock is on"
       error: "Unknown error"

lib/discourse_webauthn/authentication_service.rb

@@ -8,6 +8,7 @@ module DiscourseWebauthn
     # the steps followed here. Memoized methods are called in their
     # place in the step flow to make the process clearer.
     def authenticate_security_key
+      # Steps 1-5 of this authentication flow are in the frontend at lib/webauthn.js
       if @params.blank? || (!@params.is_a?(Hash) && !@params.is_a?(ActionController::Parameters))
         raise(
           MalformedPublicKeyCredentialError,
@@ -15,63 +16,75 @@ module DiscourseWebauthn
         )
       end
 
+      # 6. Identify the user being authenticated and verify that this user is the
+      #    owner of the public key credential source credentialSource identified by credential.id:
+
+      # 6a. If the user was identified before the authentication ceremony was initiated,
+      #     verify that the identified user account contains a credential record whose id equals credential.rawId.
       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:
       if @factor_type == UserSecurityKey.factor_types[:second_factor] &&
            (@current_user == nil || security_key.user == nil || security_key.user != @current_user)
         raise(OwnershipError, I18n.t("webauthn.validation.ownership_error"))
       end
 
-      # 4. Using credential.id (or credential.rawId, if base64url encoding is inappropriate for your use case),
+      # 6b. If the user was not identified before the authentication ceremony was initiated,
-      #    look up the corresponding credential public key and let credentialPublicKey be that credential public key.
+      #     verify that response.userHandle is present. Verify that the user account identified by response.userHandle
-      public_key = security_key.public_key
+      #     contains a credential record whose id equals credential.rawId
+      if @factor_type == UserSecurityKey.factor_types[:first_factor] &&
+           Base64.decode64(@params[:userHandle]) != @current_user.secure_identifier
+        raise(OwnershipError, I18n.t("webauthn.validation.ownership_error"))
+      end
 
-      # 5. Let cData, authData and sig denote the value of credential’s response's clientDataJSON, authenticatorData, and signature respectively.
+      # 7. No upstream step
-      # 6. Let JSONtext be the result of running UTF-8 decode on the value of cData.
+      # 8. No upstream step
-      # 7. Let C, the client data claimed as used for the signature, be the result of running an implementation-specific JSON parser on JSONtext.
+
+      # 9. Let cData, authData and sig denote the value of credential’s response's clientDataJSON, authenticatorData, and signature respectively.
+      # 10. Let JSONtext be the result of running UTF-8 decode on the value of cData.
+      # 11. Let C, the client data claimed as used for the signature, be the result of running an implementation-specific JSON parser on JSONtext.
       client_data
 
-      # 8. Verify that the value of C.type is the string webauthn.get.
+      # 12. Verify that the value of C.type is the string webauthn.get.
       validate_webauthn_type(::DiscourseWebauthn::ACCEPTABLE_AUTHENTICATION_TYPE)
 
-      # 9. Verify that the value of C.challenge equals the base64url encoding of options.challenge.
+      # 13. Verify that the value of C.challenge equals the base64url encoding of options.challenge.
       validate_challenge
 
-      # 10. Verify that the value of C.origin matches the Relying Party's origin.
+      # 14. Verify that the value of C.origin matches the Relying Party's origin.
       validate_origin
 
-      # 11. Verify that the value of C.tokenBinding.status matches the state of Token Binding for the TLS connection
+      # 15. If C.topOrigin is present:
-      #     over which the attestation was obtained. If Token Binding was used on that TLS connection, also verify
+      # - Verify that the Relying Party expects this credential to be used within an iframe that is not same-origin with its ancestors.
-      #     that C.tokenBinding.id matches the base64url encoding of the Token Binding ID for the connection.
+      # - Verify that the value of C.topOrigin matches the origin of a page that the Relying Party expects to be sub-framed within.
-      #     Not using this right now.
+      # We are not using this.
 
-      # 12. Verify that the rpIdHash in authData is the SHA-256 hash of the RP ID expected by the Relying Party.
+      # 16. Verify that the rpIdHash in authData is the SHA-256 hash of the RP ID expected by the Relying Party.
       validate_rp_id_hash
 
-      # 13. Verify that the User Present bit of the flags in authData is set.
+      # 17. Verify that the User Present bit of the flags in authData is set.
       # https://blog.bigbinary.com/2011/07/20/ruby-pack-unpack.html
       #
       validate_user_presence
 
       #
-      # 14. If user verification is required for this registration, verify that
+      # 18. Determine whether user verification is required for this assertion.
-      #     the User Verified bit of the flags in authData is set.
+      #     User verification SHOULD be required if, and only if, options.userVerification is set to required.
+      #     If user verification was determined to be required, verify that the UV bit of the flags in authData is set.
+      #     Otherwise, ignore the value of the UV flag.
       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
+      # 19. If the BE bit of the flags in authData is not set, verify that the BS bit is not set.
-      #     extension outputs in the extensions in authData are as expected, considering the client extension input
+      #     Not using this right now.
-      #     values that were given in options.extensions and any specific policy of the Relying Party regarding
+      # 20. If the credential backup state is used as part of Relying Party business logic or policy...
-      #     unsolicited extensions, i.e., those that were not specified as part of options.extensions. In the
+      #     Not using this right now.
-      #     general case, the meaning of "are as expected" is specific to the Relying Party and which extensions are in use.
+      # 21. Verify that the values of the client extension outputs in clientExtensionResults...
       #     Not using this right now.
 
-      # 16. Let hash be the result of computing a hash over response.clientDataJSON using SHA-256.
+      # 22. Let hash be the result of computing a hash over response.clientDataJSON using SHA-256.
       client_data_hash
 
-      # 17. Using credentialPublicKey, verify that sig is a valid signature over the binary concatenation of authData and hash.
+      # 23. Using credentialPublicKey, verify that sig is a valid signature over the binary concatenation of authData and hash.
       cose_key = COSE::Key.deserialize(Base64.decode64(security_key.public_key))
       cose_algorithm = COSE::Algorithm.find(cose_key.alg)
 
@@ -90,7 +103,13 @@ module DiscourseWebauthn
         raise(PublicKeyError, I18n.t("webauthn.validation.public_key_error"))
       end
 
-      # Success! Update the last used at time for the key.
+      # 24. If authData.signCount is nonzero or credentialRecord.signCount is nonzero...
+      #     Not using this right now.
+
+      # 25. If response.attestationObject is present and the Relying Party wishes to verify the attestation...
+      #     Not using this right now.
+
+      # 26. Success! Update the last used at time for the key (credentialRecord).
       security_key.update(last_used: Time.zone.now)
 
       # Return security key record so controller can use it to update the session

spec/lib/discourse_webauthn/authentication_service_spec.rb

@@ -269,7 +269,7 @@ RSpec.describe DiscourseWebauthn::AuthenticationService do
     end
   end
 
-  describe "authenticating a valid passkey" do
+  describe "authenticating passkeys" do
     let(:options) do
       { factor_type: UserSecurityKey.factor_types[:first_factor], session: secure_session }
     end
@@ -297,10 +297,20 @@ RSpec.describe DiscourseWebauthn::AuthenticationService do
       )
     end
 
-    it "works and returns the correct key credential" do
+    before do
-      key = service.authenticate_security_key
+      # this essentially bypasses the user handle check for this key
-      expect(key).to eq(security_key)
+      # a real test would need to go through the full registration/authentication flow in one go
-      expect(key.factor_type).to eq(UserSecurityKey.factor_types[:first_factor])
+      params[:userHandle] = Base64.strict_encode64(
+        security_key_user.create_or_fetch_secure_identifier,
+      )
+    end
+
+    context "with a valid passkey" do
+      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
     end
 
     context "when the user verification flag in the key is false" do
@@ -319,5 +329,16 @@ RSpec.describe DiscourseWebauthn::AuthenticationService do
         )
       end
     end
+
+    context "when the user handle does not match" do
+      it "raises an OwnershipError" do
+        params[:userHandle] = Base64.strict_encode64(SecureRandom.hex(20))
+
+        expect { service.authenticate_security_key }.to raise_error(
+          DiscourseWebauthn::OwnershipError,
+          I18n.t("webauthn.validation.ownership_error"),
+        )
+      end
+    end
   end
 end