From 2c3b821e969c65e770d712a6a37bc27e19d81585 Mon Sep 17 00:00:00 2001 From: Santiago Rodriguez <46354312+santiagorodriguez96@users.noreply.github.com> Date: Thu, 12 Mar 2026 17:13:30 -0300 Subject: [PATCH 1/4] feat: validate `userHandle` in `PasskeyAuthenticatable` strategy --- .../strategies/passkey_authenticatable.rb | 8 +-- .../devise/passkey_authentication_spec.rb | 53 +++++++++++++++++-- 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/lib/devise/strategies/passkey_authenticatable.rb b/lib/devise/strategies/passkey_authenticatable.rb index 746083a9..56ca3909 100644 --- a/lib/devise/strategies/passkey_authenticatable.rb +++ b/lib/devise/strategies/passkey_authenticatable.rb @@ -9,13 +9,13 @@ def valid? def authenticate! passkey_from_params = WebAuthn::Credential.from_get(JSON.parse(passkey_param)) - stored_passkey = WebauthnCredential.passkey.find_by(external_id: passkey_from_params.id) + resource = resource_class.find_by(webauthn_id: passkey_from_params.user_handle) + stored_passkey = resource&.passkeys&.find_by(external_id: passkey_from_params.id) return fail!(:passkey_not_found) if stored_passkey.blank? verify_passkeys(passkey_from_params, stored_passkey) - resource = stored_passkey.public_send(resource_name) success!(resource) rescue WebAuthn::Error fail!(:passkey_verification_failed) @@ -40,8 +40,8 @@ def verify_passkeys(passkey_from_params, stored_passkey) stored_passkey.update!(sign_count: passkey_from_params.sign_count) end - def resource_name - mapping.to.name.underscore + def resource_class + mapping.to end end end diff --git a/spec/requests/devise/passkey_authentication_spec.rb b/spec/requests/devise/passkey_authentication_spec.rb index c7fdd14d..4840054e 100644 --- a/spec/requests/devise/passkey_authentication_spec.rb +++ b/spec/requests/devise/passkey_authentication_spec.rb @@ -22,11 +22,12 @@ def create_passkey_for(account, fake_client) ) end - def generate_assertion(fake_client, challenge:, credential:) + def generate_assertion(fake_client, challenge:, credential:, user_handle: nil) fake_client.get( challenge: challenge, allow_credentials: [credential.external_id], - user_verified: true + user_verified: true, + user_handle: user_handle ) end @@ -43,7 +44,8 @@ def generate_assertion(fake_client, challenge:, credential:) assertion = generate_assertion( client, challenge: session[:authentication_challenge], - credential: passkey + credential: passkey, + user_handle: WebAuthn.configuration.encoder.decode(user.webauthn_id) ) expect do @@ -64,7 +66,8 @@ def generate_assertion(fake_client, challenge:, credential:) assertion = generate_assertion( client, challenge: session[:authentication_challenge], - credential: passkey + credential: passkey, + user_handle: WebAuthn.configuration.encoder.decode(user.webauthn_id) ) passkey.destroy! @@ -83,7 +86,8 @@ def generate_assertion(fake_client, challenge:, credential:) assertion = client.get( challenge: WebAuthn.configuration.encoder.encode("invalid_challenge"), - allow_credentials: [passkey.external_id] + allow_credentials: [passkey.external_id], + user_handle: WebAuthn.configuration.encoder.decode(user.webauthn_id) ) post account_session_path, params: { @@ -96,6 +100,45 @@ def generate_assertion(fake_client, challenge:, credential:) expect(controller.current_account).to be_nil end + it "rejects sign-in when userHandle is missing from the response" do + get new_account_session_path + + assertion = client.get( + challenge: session[:authentication_challenge], + allow_credentials: [passkey.external_id], + user_verified: true + ) + + post account_session_path, params: { + public_key_credential: assertion.to_json + } + + expect(response).to have_http_status(:unprocessable_entity) + expect(flash[:alert]).to eq(I18n.t("devise.failure.passkey_not_found")) + expect(controller.current_account).to be_nil + end + + it "rejects sign-in when userHandle does not match the passkey owner" do + other_user = Account.create!(email: "other@example.com", password: password) + + get new_account_session_path + + assertion = client.get( + challenge: session[:authentication_challenge], + allow_credentials: [passkey.external_id], + user_verified: true, + user_handle: WebAuthn.configuration.encoder.decode(other_user.webauthn_id) + ) + + post account_session_path, params: { + public_key_credential: assertion.to_json + } + + expect(response).to have_http_status(:unprocessable_entity) + expect(flash[:alert]).to eq(I18n.t("devise.failure.passkey_not_found")) + expect(controller.current_account).to be_nil + end + it "fails when credential param is missing" do post account_session_path, params: {} From 8992975248ee35853f2c205c64d4030c20e1e930 Mon Sep 17 00:00:00 2001 From: Santiago Rodriguez <46354312+santiagorodriguez96@users.noreply.github.com> Date: Thu, 12 Mar 2026 17:13:35 -0300 Subject: [PATCH 2/4] feat: validate `userHandle` in `WebauthnTwoFactorAuthenticatable` strategy --- .../webauthn_two_factor_authenticatable.rb | 8 +++ .../devise/two_factor_authentication_spec.rb | 54 +++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/lib/devise/strategies/webauthn_two_factor_authenticatable.rb b/lib/devise/strategies/webauthn_two_factor_authenticatable.rb index 43bc5754..23d4712f 100644 --- a/lib/devise/strategies/webauthn_two_factor_authenticatable.rb +++ b/lib/devise/strategies/webauthn_two_factor_authenticatable.rb @@ -16,6 +16,9 @@ def authenticate! stored_credential = resource&.webauthn_credentials&.find_by(external_id: credential_from_params.id) return fail!(:webauthn_credential_not_found) if stored_credential.blank? + if user_handle_mismatch?(credential_from_params, resource) + return fail!(:webauthn_credential_verification_failed) + end verify_credential(credential_from_params, stored_credential) @@ -47,6 +50,11 @@ def verify_credential(credential_from_params, stored_credential) stored_credential.update!(sign_count: credential_from_params.sign_count) end + def user_handle_mismatch?(credential_from_params, resource) + credential_from_params.user_handle.present? && + credential_from_params.user_handle != resource.webauthn_id + end + def resource_class mapping.to end diff --git a/spec/requests/devise/two_factor_authentication_spec.rb b/spec/requests/devise/two_factor_authentication_spec.rb index 5d12bf2b..51b89892 100644 --- a/spec/requests/devise/two_factor_authentication_spec.rb +++ b/spec/requests/devise/two_factor_authentication_spec.rb @@ -164,6 +164,60 @@ def generate_assertion(fake_client, challenge:, credential:) expect(controller.current_account).to be_nil end + it "completes authentication when userHandle matches the authenticated user" do + post account_session_path, params: { account: { email: user.email, password: password } } + + expect(response).to redirect_to(new_account_two_factor_authentication_path) + + follow_redirect! + + expect(response).to have_http_status(:ok) + get account_security_key_authentication_options_path + + assertion = client.get( + challenge: session[:two_factor_authentication_challenge], + allow_credentials: [security_key.external_id], + user_handle: WebAuthn.configuration.encoder.decode(user.webauthn_id) + ) + + expect do + post account_two_factor_authentication_path, params: { + public_key_credential: assertion.to_json + } + + expect(response).to redirect_to(root_path) + expect(controller.current_account).to eq(user) + end.to change { security_key.reload.sign_count }.by(1) + end + + it "rejects 2FA when userHandle does not match the authenticated user" do + other_user = Account.create!(email: "other@example.com", password: password) + + post account_session_path, params: { account: { email: user.email, password: password } } + + expect(response).to redirect_to(new_account_two_factor_authentication_path) + + follow_redirect! + + expect(response).to have_http_status(:ok) + get account_security_key_authentication_options_path + + assertion = client.get( + challenge: session[:two_factor_authentication_challenge], + allow_credentials: [security_key.external_id], + user_handle: WebAuthn.configuration.encoder.decode(other_user.webauthn_id) + ) + + post account_two_factor_authentication_path, params: { + public_key_credential: assertion.to_json + } + + expect(response).to have_http_status(:unprocessable_entity) + expect(response.body).to include("Use security key") + expect(flash[:alert]).to eq(I18n.t("devise.failure.webauthn_credential_verification_failed")) + expect(controller.current_account).to be_nil + end + it "re-renders 2FA page when credential param is missing" do post account_session_path, params: { account: { email: user.email, password: password } } From bb2e7b6fa0863ecc6bebebff52c032d946f34c4d Mon Sep 17 00:00:00 2001 From: Santiago Rodriguez <46354312+santiagorodriguez96@users.noreply.github.com> Date: Tue, 17 Mar 2026 17:30:34 -0300 Subject: [PATCH 3/4] feat: fail early when `response.userHandle` is not present --- lib/devise/strategies/passkey_authenticatable.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/devise/strategies/passkey_authenticatable.rb b/lib/devise/strategies/passkey_authenticatable.rb index 56ca3909..541ea8b0 100644 --- a/lib/devise/strategies/passkey_authenticatable.rb +++ b/lib/devise/strategies/passkey_authenticatable.rb @@ -9,6 +9,9 @@ def valid? def authenticate! passkey_from_params = WebAuthn::Credential.from_get(JSON.parse(passkey_param)) + + return fail!(:passkey_not_found) if passkey_from_params.user_handle.nil? + resource = resource_class.find_by(webauthn_id: passkey_from_params.user_handle) stored_passkey = resource&.passkeys&.find_by(external_id: passkey_from_params.id) From 19534290c4614b71f36fad55ee2e2617723f59ba Mon Sep 17 00:00:00 2001 From: Santiago Rodriguez <46354312+santiagorodriguez96@users.noreply.github.com> Date: Tue, 17 Mar 2026 17:32:40 -0300 Subject: [PATCH 4/4] chore: disable `Metrics/AbcSize` for `PasskeyAuthenticatable#authenticate!` --- lib/devise/strategies/passkey_authenticatable.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/devise/strategies/passkey_authenticatable.rb b/lib/devise/strategies/passkey_authenticatable.rb index 541ea8b0..3b12d8f2 100644 --- a/lib/devise/strategies/passkey_authenticatable.rb +++ b/lib/devise/strategies/passkey_authenticatable.rb @@ -7,7 +7,7 @@ def valid? passkey_param.present? && session[:authentication_challenge].present? end - def authenticate! + def authenticate! # rubocop:disable Metrics/AbcSize passkey_from_params = WebAuthn::Credential.from_get(JSON.parse(passkey_param)) return fail!(:passkey_not_found) if passkey_from_params.user_handle.nil?