From 33d55839bf4731ef01005d08ed0d217f80ae0cff Mon Sep 17 00:00:00 2001 From: Renzo Minelli Date: Fri, 25 Sep 2026 12:26:58 -0300 Subject: [PATCH 1/7] feat: add configurable challenge store for WebAuthn challenges Controllers and strategies now read and write challenges through `Devise::Webauthn.challenge_store`. The default session store keeps the existing session keys, so behaviour is unchanged. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 8 ++ ...sskey_authentication_options_controller.rb | 3 +- ...passkey_registration_options_controller.rb | 3 +- app/controllers/devise/passkeys_controller.rb | 4 +- ..._factor_webauthn_credentials_controller.rb | 4 +- ...y_key_authentication_options_controller.rb | 4 +- ...ity_key_registration_options_controller.rb | 3 +- .../strategies/passkey_authenticatable.rb | 15 ++-- .../webauthn_two_factor_authenticatable.rb | 15 ++-- lib/devise/webauthn.rb | 7 ++ .../webauthn/challenge_stores/session.rb | 31 +++++++ .../webauthn/challenge_stores/session_spec.rb | 36 ++++++++ .../devise/custom_challenge_store_spec.rb | 82 +++++++++++++++++++ 13 files changed, 189 insertions(+), 26 deletions(-) create mode 100644 lib/devise/webauthn/challenge_stores/session.rb create mode 100644 spec/devise/webauthn/challenge_stores/session_spec.rb create mode 100644 spec/requests/devise/custom_challenge_store_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index a1a01f7f..8092a9ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ ## Unreleased +### Added + +- Add `Devise::Webauthn.challenge_store` to configure where WebAuthn challenges are kept between the options request and the verification request. The default, `Devise::Webauthn::ChallengeStores::Session`, keeps them in the session as before. [@RenzoMinelli] + +### Changed + +- If you override `verify_and_save_passkey` or `verify_and_save_security_key`, read the challenge with `Devise::Webauthn.challenge_store_for(request).consume(:registration, params[:public_key_credential])` instead of `session[:webauthn_challenge]`. Otherwise the challenge is no longer cleared after use. [@RenzoMinelli] + ## [v0.5.0](https://github.com/cedarcode/devise-webauthn/compare/v0.4.0...v0.5.0/) - 2026-07-13 ### Added diff --git a/app/controllers/devise/passkey_authentication_options_controller.rb b/app/controllers/devise/passkey_authentication_options_controller.rb index 1db26e3b..cdfd3df3 100644 --- a/app/controllers/devise/passkey_authentication_options_controller.rb +++ b/app/controllers/devise/passkey_authentication_options_controller.rb @@ -10,8 +10,7 @@ def create user_verification: "required" ) - # Store challenge in session for later verification - session[:authentication_challenge] = passkey_options.challenge + Devise::Webauthn.challenge_store_for(request).write(:passkey_authentication, passkey_options.challenge) render json: passkey_options end diff --git a/app/controllers/devise/passkey_registration_options_controller.rb b/app/controllers/devise/passkey_registration_options_controller.rb index e4606783..d389c09d 100644 --- a/app/controllers/devise/passkey_registration_options_controller.rb +++ b/app/controllers/devise/passkey_registration_options_controller.rb @@ -21,8 +21,7 @@ def create } ) - # Store challenge in session for later verification - session[:webauthn_challenge] = passkey_options.challenge + Devise::Webauthn.challenge_store_for(request).write(:registration, passkey_options.challenge) render json: passkey_options end diff --git a/app/controllers/devise/passkeys_controller.rb b/app/controllers/devise/passkeys_controller.rb index ae67f5c3..626ccf05 100644 --- a/app/controllers/devise/passkeys_controller.rb +++ b/app/controllers/devise/passkeys_controller.rb @@ -18,8 +18,6 @@ def create rescue WebAuthn::Error set_flash_message! :alert, :passkey_verification_failed, scope: :"devise.failure" redirect_to after_update_path - ensure - session.delete(:webauthn_challenge) end def destroy @@ -41,7 +39,7 @@ def authenticate_scope! def verify_and_save_passkey(passkey_from_params) passkey_from_params.verify( - session[:webauthn_challenge], + Devise::Webauthn.challenge_store_for(request).consume(:registration, params[:public_key_credential]), user_verification: true ) diff --git a/app/controllers/devise/second_factor_webauthn_credentials_controller.rb b/app/controllers/devise/second_factor_webauthn_credentials_controller.rb index 64023c63..9db83c08 100644 --- a/app/controllers/devise/second_factor_webauthn_credentials_controller.rb +++ b/app/controllers/devise/second_factor_webauthn_credentials_controller.rb @@ -18,8 +18,6 @@ def create rescue WebAuthn::Error set_flash_message! :alert, :webauthn_credential_verification_failed, scope: :"devise.failure" redirect_to after_create_path - ensure - session.delete(:webauthn_challenge) end def update @@ -51,7 +49,7 @@ def authenticate_scope! def verify_and_save_security_key(security_key_from_params) security_key_from_params.verify( - session[:webauthn_challenge] + Devise::Webauthn.challenge_store_for(request).consume(:registration, params[:public_key_credential]) ) resource.second_factor_webauthn_credentials.create( diff --git a/app/controllers/devise/security_key_authentication_options_controller.rb b/app/controllers/devise/security_key_authentication_options_controller.rb index 3c1fddfb..3f105594 100644 --- a/app/controllers/devise/security_key_authentication_options_controller.rb +++ b/app/controllers/devise/security_key_authentication_options_controller.rb @@ -13,8 +13,8 @@ def create user_verification: "discouraged" ) - # Store challenge in session for later verification - session[:two_factor_authentication_challenge] = security_key_authentication_options.challenge + Devise::Webauthn.challenge_store_for(request) + .write(:two_factor_authentication, security_key_authentication_options.challenge) render json: security_key_authentication_options end diff --git a/app/controllers/devise/security_key_registration_options_controller.rb b/app/controllers/devise/security_key_registration_options_controller.rb index a536cc11..03dc9239 100644 --- a/app/controllers/devise/security_key_registration_options_controller.rb +++ b/app/controllers/devise/security_key_registration_options_controller.rb @@ -21,8 +21,7 @@ def create } ) - # Store challenge in session for later verification - session[:webauthn_challenge] = create_security_key_options.challenge + Devise::Webauthn.challenge_store_for(request).write(:registration, create_security_key_options.challenge) render json: create_security_key_options end diff --git a/lib/devise/strategies/passkey_authenticatable.rb b/lib/devise/strategies/passkey_authenticatable.rb index 78b61179..35d50cd5 100644 --- a/lib/devise/strategies/passkey_authenticatable.rb +++ b/lib/devise/strategies/passkey_authenticatable.rb @@ -4,10 +4,11 @@ module Devise module Strategies class PasskeyAuthenticatable < Devise::Strategies::Base def valid? - passkey_param.present? && session[:authentication_challenge].present? + passkey_param.present? && challenge_store.pending?(:passkey_authentication, passkey_param) end def authenticate! # rubocop:disable Metrics/AbcSize + challenge = challenge_store.consume(:passkey_authentication, passkey_param) passkey_from_params = WebAuthn::Credential.from_get(JSON.parse(passkey_param)) return fail!(:passkey_not_found) if passkey_from_params.user_handle.nil? @@ -17,14 +18,12 @@ def authenticate! # rubocop:disable Metrics/AbcSize return fail!(:passkey_not_found) if stored_passkey.blank? - verify_passkeys(passkey_from_params, stored_passkey) + verify_passkeys(passkey_from_params, stored_passkey, challenge) remember_me(resource) success!(resource) rescue WebAuthn::Error fail!(:passkey_verification_failed) - ensure - session.delete(:authentication_challenge) end private @@ -33,9 +32,13 @@ def passkey_param params[:public_key_credential] end - def verify_passkeys(passkey_from_params, stored_passkey) + def challenge_store + @challenge_store ||= Devise::Webauthn.challenge_store_for(request) + end + + def verify_passkeys(passkey_from_params, stored_passkey, challenge) passkey_from_params.verify( - session[:authentication_challenge], + challenge, public_key: stored_passkey.public_key, sign_count: stored_passkey.sign_count, user_verification: true diff --git a/lib/devise/strategies/webauthn_two_factor_authenticatable.rb b/lib/devise/strategies/webauthn_two_factor_authenticatable.rb index 23d4712f..107d381f 100644 --- a/lib/devise/strategies/webauthn_two_factor_authenticatable.rb +++ b/lib/devise/strategies/webauthn_two_factor_authenticatable.rb @@ -6,11 +6,12 @@ class WebauthnTwoFactorAuthenticatable < Devise::Strategies::Base def valid? credential_param.present? && session[:current_authentication_resource_id].present? && - session[:two_factor_authentication_challenge].present? + challenge_store.pending?(:two_factor_authentication, credential_param) end # rubocop:disable Metrics/AbcSize def authenticate! + challenge = challenge_store.consume(:two_factor_authentication, credential_param) credential_from_params = WebAuthn::Credential.from_get(JSON.parse(credential_param)) resource = resource_class.find_by(id: session[:current_authentication_resource_id]) stored_credential = resource&.webauthn_credentials&.find_by(external_id: credential_from_params.id) @@ -20,7 +21,7 @@ def authenticate! return fail!(:webauthn_credential_verification_failed) end - verify_credential(credential_from_params, stored_credential) + verify_credential(credential_from_params, stored_credential, challenge) resource.remember_me = session[:current_authentication_remember_me] if resource.respond_to?(:remember_me=) success!(resource) @@ -29,8 +30,6 @@ def authenticate! session.delete(:current_authentication_remember_me) rescue WebAuthn::Error fail!(:webauthn_credential_verification_failed) - ensure - session.delete(:two_factor_authentication_challenge) end # rubocop:enable Metrics/AbcSize @@ -40,9 +39,13 @@ def credential_param params[:public_key_credential] end - def verify_credential(credential_from_params, stored_credential) + def challenge_store + @challenge_store ||= Devise::Webauthn.challenge_store_for(request) + end + + def verify_credential(credential_from_params, stored_credential, challenge) credential_from_params.verify( - session[:two_factor_authentication_challenge], + challenge, public_key: stored_credential.public_key, sign_count: stored_credential.sign_count ) diff --git a/lib/devise/webauthn.rb b/lib/devise/webauthn.rb index c83ea382..1d61f684 100644 --- a/lib/devise/webauthn.rb +++ b/lib/devise/webauthn.rb @@ -4,6 +4,7 @@ require "webauthn" require_relative "webauthn/version" +require_relative "webauthn/challenge_stores/session" require_relative "webauthn/engine" require_relative "webauthn/helpers/credentials_helper" require_relative "webauthn/routes" @@ -11,6 +12,12 @@ module Devise module Webauthn + mattr_accessor :challenge_store, default: ChallengeStores::Session + + def self.challenge_store_for(request) + challenge_store.new(request) + end + module Test autoload :AuthenticatorHelpers, "devise/webauthn/test/authenticator_helpers" end diff --git a/lib/devise/webauthn/challenge_stores/session.rb b/lib/devise/webauthn/challenge_stores/session.rb new file mode 100644 index 00000000..d6a5e13b --- /dev/null +++ b/lib/devise/webauthn/challenge_stores/session.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Devise + module Webauthn + module ChallengeStores + class Session + KEYS = { + passkey_authentication: :authentication_challenge, + two_factor_authentication: :two_factor_authentication_challenge, + registration: :webauthn_challenge + }.freeze + + def initialize(request) + @session = request.session + end + + def write(purpose, challenge) + @session[KEYS.fetch(purpose)] = challenge + end + + def pending?(purpose, _credential_json) + @session[KEYS.fetch(purpose)].present? + end + + def consume(purpose, _credential_json) + @session.delete(KEYS.fetch(purpose)) + end + end + end + end +end diff --git a/spec/devise/webauthn/challenge_stores/session_spec.rb b/spec/devise/webauthn/challenge_stores/session_spec.rb new file mode 100644 index 00000000..3d42c94f --- /dev/null +++ b/spec/devise/webauthn/challenge_stores/session_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Devise::Webauthn::ChallengeStores::Session do + let(:session) { {} } + let(:store) { described_class.new(instance_double(ActionDispatch::Request, session: session)) } + + it "keeps each purpose under its own session key" do + store.write(:passkey_authentication, "passkey-challenge") + store.write(:two_factor_authentication, "2fa-challenge") + store.write(:registration, "registration-challenge") + + expect(session).to eq( + authentication_challenge: "passkey-challenge", + two_factor_authentication_challenge: "2fa-challenge", + webauthn_challenge: "registration-challenge" + ) + end + + it "reports a challenge as pending until it is consumed" do + store.write(:passkey_authentication, "challenge") + + expect(store.pending?(:passkey_authentication, "{}")).to be(true) + expect(store.consume(:passkey_authentication, "{}")).to eq("challenge") + expect(store.pending?(:passkey_authentication, "{}")).to be(false) + end + + it "returns nil when consuming a challenge that was never written" do + expect(store.consume(:registration, "{}")).to be_nil + end + + it "raises on an unknown purpose" do + expect { store.write(:unknown, "challenge") }.to raise_error(KeyError) + end +end diff --git a/spec/requests/devise/custom_challenge_store_spec.rb b/spec/requests/devise/custom_challenge_store_spec.rb new file mode 100644 index 00000000..225ff826 --- /dev/null +++ b/spec/requests/devise/custom_challenge_store_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require "spec_helper" +require "webauthn/fake_client" + +RSpec.describe "Custom challenge store", type: :request do + let(:memory_store_class) do + Class.new do + def self.challenges + @challenges ||= {} + end + + def initialize(_request); end + + def write(purpose, challenge) + self.class.challenges[purpose] = challenge + end + + def pending?(purpose, _credential_json) + self.class.challenges.key?(purpose) + end + + def consume(purpose, _credential_json) + self.class.challenges.delete(purpose) + end + end + end + + let(:user) { Account.create!(email: "test@example.com", password: "password123") } + let(:client) { WebAuthn::FakeClient.new(WebAuthn.configuration.allowed_origins.first) } + let!(:passkey) do + user.update!(webauthn_id: WebAuthn.generate_user_id) + credential = WebAuthn::Credential.from_create( + client.create(challenge: WebAuthn.configuration.encoder.encode(SecureRandom.random_bytes(32))) + ) + user.passkeys.create!(external_id: credential.id, name: "My Passkey", + public_key: credential.public_key, sign_count: credential.sign_count) + end + + around do |example| + original_store = Devise::Webauthn.challenge_store + Devise::Webauthn.challenge_store = memory_store_class + example.run + ensure + Devise::Webauthn.challenge_store = original_store + end + + def sign_in_with_passkey(challenge) + assertion = client.get( + challenge: challenge, + allow_credentials: [passkey.external_id], + user_verified: true, + user_handle: WebAuthn.configuration.encoder.decode(user.webauthn_id) + ) + + post account_session_path, params: { public_key_credential: assertion.to_json } + end + + it "signs in with a passkey without keeping the challenge in the session" do + post account_passkey_authentication_options_path + challenge = response.parsed_body["challenge"] + + expect(memory_store_class.challenges).to eq(passkey_authentication: challenge) + expect(session[:authentication_challenge]).to be_nil + + sign_in_with_passkey(challenge) + + expect(controller.current_account).to eq(user) + end + + it "consumes the challenge so it cannot be used twice" do + post account_passkey_authentication_options_path + challenge = response.parsed_body["challenge"] + sign_in_with_passkey(challenge) + expect(memory_store_class.challenges).to be_empty + delete destroy_account_session_path + + sign_in_with_passkey(challenge) + + expect(controller.current_account).to be_nil + end +end From 2937591a7ed8692a901a35dca4a44651e15f7cf3 Mon Sep 17 00:00:00 2001 From: Renzo Minelli Date: Fri, 25 Sep 2026 12:27:19 -0300 Subject: [PATCH 2/7] docs(`CHANGELOG`): link challenge store entries to #150 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8092a9ba..cdd944ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,11 @@ ### Added -- Add `Devise::Webauthn.challenge_store` to configure where WebAuthn challenges are kept between the options request and the verification request. The default, `Devise::Webauthn::ChallengeStores::Session`, keeps them in the session as before. [@RenzoMinelli] +- Add `Devise::Webauthn.challenge_store` to configure where WebAuthn challenges are kept between the options request and the verification request. The default, `Devise::Webauthn::ChallengeStores::Session`, keeps them in the session as before. [#150](https://github.com/cedarcode/devise-webauthn/pull/150) [@RenzoMinelli] ### Changed -- If you override `verify_and_save_passkey` or `verify_and_save_security_key`, read the challenge with `Devise::Webauthn.challenge_store_for(request).consume(:registration, params[:public_key_credential])` instead of `session[:webauthn_challenge]`. Otherwise the challenge is no longer cleared after use. [@RenzoMinelli] +- If you override `verify_and_save_passkey` or `verify_and_save_security_key`, read the challenge with `Devise::Webauthn.challenge_store_for(request).consume(:registration, params[:public_key_credential])` instead of `session[:webauthn_challenge]`. Otherwise the challenge is no longer cleared after use. [#150](https://github.com/cedarcode/devise-webauthn/pull/150) [@RenzoMinelli] ## [v0.5.0](https://github.com/cedarcode/devise-webauthn/compare/v0.4.0...v0.5.0/) - 2026-07-13 From 936fbca361a8f5b76279f591221ff896f3ab3a73 Mon Sep 17 00:00:00 2001 From: Renzo Minelli Date: Fri, 25 Sep 2026 12:30:57 -0300 Subject: [PATCH 3/7] refactor: expose the challenge store through a `challenge_store` method Controllers and strategies include `Devise::Webauthn::ChallengeStoreAccess` instead of calling `Devise::Webauthn.challenge_store_for(request)`. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- .../passkey_authentication_options_controller.rb | 4 +++- .../passkey_registration_options_controller.rb | 4 +++- app/controllers/devise/passkeys_controller.rb | 4 +++- ...second_factor_webauthn_credentials_controller.rb | 4 +++- ...ecurity_key_authentication_options_controller.rb | 5 +++-- .../security_key_registration_options_controller.rb | 4 +++- lib/devise/strategies/passkey_authenticatable.rb | 6 ++---- .../webauthn_two_factor_authenticatable.rb | 6 ++---- lib/devise/webauthn.rb | 5 +---- lib/devise/webauthn/challenge_store_access.rb | 13 +++++++++++++ 11 files changed, 37 insertions(+), 20 deletions(-) create mode 100644 lib/devise/webauthn/challenge_store_access.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index cdd944ce..f558ad43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ ### Changed -- If you override `verify_and_save_passkey` or `verify_and_save_security_key`, read the challenge with `Devise::Webauthn.challenge_store_for(request).consume(:registration, params[:public_key_credential])` instead of `session[:webauthn_challenge]`. Otherwise the challenge is no longer cleared after use. [#150](https://github.com/cedarcode/devise-webauthn/pull/150) [@RenzoMinelli] +- If you override `verify_and_save_passkey` or `verify_and_save_security_key`, read the challenge with `challenge_store.consume(:registration, params[:public_key_credential])` instead of `session[:webauthn_challenge]`. Otherwise the challenge is no longer cleared after use. [#150](https://github.com/cedarcode/devise-webauthn/pull/150) [@RenzoMinelli] ## [v0.5.0](https://github.com/cedarcode/devise-webauthn/compare/v0.4.0...v0.5.0/) - 2026-07-13 diff --git a/app/controllers/devise/passkey_authentication_options_controller.rb b/app/controllers/devise/passkey_authentication_options_controller.rb index cdfd3df3..734d8165 100644 --- a/app/controllers/devise/passkey_authentication_options_controller.rb +++ b/app/controllers/devise/passkey_authentication_options_controller.rb @@ -2,6 +2,8 @@ module Devise class PasskeyAuthenticationOptionsController < DeviseController + include Devise::Webauthn::ChallengeStoreAccess + skip_forgery_protection def create @@ -10,7 +12,7 @@ def create user_verification: "required" ) - Devise::Webauthn.challenge_store_for(request).write(:passkey_authentication, passkey_options.challenge) + challenge_store.write(:passkey_authentication, passkey_options.challenge) render json: passkey_options end diff --git a/app/controllers/devise/passkey_registration_options_controller.rb b/app/controllers/devise/passkey_registration_options_controller.rb index d389c09d..3e29cea0 100644 --- a/app/controllers/devise/passkey_registration_options_controller.rb +++ b/app/controllers/devise/passkey_registration_options_controller.rb @@ -2,6 +2,8 @@ module Devise class PasskeyRegistrationOptionsController < DeviseController + include Devise::Webauthn::ChallengeStoreAccess + skip_forgery_protection before_action :authenticate_scope! @@ -21,7 +23,7 @@ def create } ) - Devise::Webauthn.challenge_store_for(request).write(:registration, passkey_options.challenge) + challenge_store.write(:registration, passkey_options.challenge) render json: passkey_options end diff --git a/app/controllers/devise/passkeys_controller.rb b/app/controllers/devise/passkeys_controller.rb index 626ccf05..09bea47b 100644 --- a/app/controllers/devise/passkeys_controller.rb +++ b/app/controllers/devise/passkeys_controller.rb @@ -2,6 +2,8 @@ module Devise class PasskeysController < DeviseController + include Devise::Webauthn::ChallengeStoreAccess + before_action :authenticate_scope! def new; end @@ -39,7 +41,7 @@ def authenticate_scope! def verify_and_save_passkey(passkey_from_params) passkey_from_params.verify( - Devise::Webauthn.challenge_store_for(request).consume(:registration, params[:public_key_credential]), + challenge_store.consume(:registration, params[:public_key_credential]), user_verification: true ) diff --git a/app/controllers/devise/second_factor_webauthn_credentials_controller.rb b/app/controllers/devise/second_factor_webauthn_credentials_controller.rb index 9db83c08..0792afd7 100644 --- a/app/controllers/devise/second_factor_webauthn_credentials_controller.rb +++ b/app/controllers/devise/second_factor_webauthn_credentials_controller.rb @@ -2,6 +2,8 @@ module Devise class SecondFactorWebauthnCredentialsController < DeviseController + include Devise::Webauthn::ChallengeStoreAccess + before_action :authenticate_scope! def new; end @@ -49,7 +51,7 @@ def authenticate_scope! def verify_and_save_security_key(security_key_from_params) security_key_from_params.verify( - Devise::Webauthn.challenge_store_for(request).consume(:registration, params[:public_key_credential]) + challenge_store.consume(:registration, params[:public_key_credential]) ) resource.second_factor_webauthn_credentials.create( diff --git a/app/controllers/devise/security_key_authentication_options_controller.rb b/app/controllers/devise/security_key_authentication_options_controller.rb index 3f105594..aaa751ba 100644 --- a/app/controllers/devise/security_key_authentication_options_controller.rb +++ b/app/controllers/devise/security_key_authentication_options_controller.rb @@ -2,6 +2,8 @@ module Devise class SecurityKeyAuthenticationOptionsController < DeviseController + include Devise::Webauthn::ChallengeStoreAccess + skip_forgery_protection before_action :set_resource @@ -13,8 +15,7 @@ def create user_verification: "discouraged" ) - Devise::Webauthn.challenge_store_for(request) - .write(:two_factor_authentication, security_key_authentication_options.challenge) + challenge_store.write(:two_factor_authentication, security_key_authentication_options.challenge) render json: security_key_authentication_options end diff --git a/app/controllers/devise/security_key_registration_options_controller.rb b/app/controllers/devise/security_key_registration_options_controller.rb index 03dc9239..dfdf026f 100644 --- a/app/controllers/devise/security_key_registration_options_controller.rb +++ b/app/controllers/devise/security_key_registration_options_controller.rb @@ -2,6 +2,8 @@ module Devise class SecurityKeyRegistrationOptionsController < DeviseController + include Devise::Webauthn::ChallengeStoreAccess + skip_forgery_protection before_action :authenticate_scope! @@ -21,7 +23,7 @@ def create } ) - Devise::Webauthn.challenge_store_for(request).write(:registration, create_security_key_options.challenge) + challenge_store.write(:registration, create_security_key_options.challenge) render json: create_security_key_options end diff --git a/lib/devise/strategies/passkey_authenticatable.rb b/lib/devise/strategies/passkey_authenticatable.rb index 35d50cd5..f9991a8f 100644 --- a/lib/devise/strategies/passkey_authenticatable.rb +++ b/lib/devise/strategies/passkey_authenticatable.rb @@ -3,6 +3,8 @@ module Devise module Strategies class PasskeyAuthenticatable < Devise::Strategies::Base + include Devise::Webauthn::ChallengeStoreAccess + def valid? passkey_param.present? && challenge_store.pending?(:passkey_authentication, passkey_param) end @@ -32,10 +34,6 @@ def passkey_param params[:public_key_credential] end - def challenge_store - @challenge_store ||= Devise::Webauthn.challenge_store_for(request) - end - def verify_passkeys(passkey_from_params, stored_passkey, challenge) passkey_from_params.verify( challenge, diff --git a/lib/devise/strategies/webauthn_two_factor_authenticatable.rb b/lib/devise/strategies/webauthn_two_factor_authenticatable.rb index 107d381f..2445d39c 100644 --- a/lib/devise/strategies/webauthn_two_factor_authenticatable.rb +++ b/lib/devise/strategies/webauthn_two_factor_authenticatable.rb @@ -3,6 +3,8 @@ module Devise module Strategies class WebauthnTwoFactorAuthenticatable < Devise::Strategies::Base + include Devise::Webauthn::ChallengeStoreAccess + def valid? credential_param.present? && session[:current_authentication_resource_id].present? && @@ -39,10 +41,6 @@ def credential_param params[:public_key_credential] end - def challenge_store - @challenge_store ||= Devise::Webauthn.challenge_store_for(request) - end - def verify_credential(credential_from_params, stored_credential, challenge) credential_from_params.verify( challenge, diff --git a/lib/devise/webauthn.rb b/lib/devise/webauthn.rb index 1d61f684..04931b7f 100644 --- a/lib/devise/webauthn.rb +++ b/lib/devise/webauthn.rb @@ -5,6 +5,7 @@ require_relative "webauthn/version" require_relative "webauthn/challenge_stores/session" +require_relative "webauthn/challenge_store_access" require_relative "webauthn/engine" require_relative "webauthn/helpers/credentials_helper" require_relative "webauthn/routes" @@ -14,10 +15,6 @@ module Devise module Webauthn mattr_accessor :challenge_store, default: ChallengeStores::Session - def self.challenge_store_for(request) - challenge_store.new(request) - end - module Test autoload :AuthenticatorHelpers, "devise/webauthn/test/authenticator_helpers" end diff --git a/lib/devise/webauthn/challenge_store_access.rb b/lib/devise/webauthn/challenge_store_access.rb new file mode 100644 index 00000000..1b092873 --- /dev/null +++ b/lib/devise/webauthn/challenge_store_access.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Devise + module Webauthn + module ChallengeStoreAccess + private + + def challenge_store + Devise::Webauthn.challenge_store.new(request) + end + end + end +end From 05e8ad12719bad07ff9ae551d7e79f9b665ed1cc Mon Sep 17 00:00:00 2001 From: Renzo Minelli Date: Fri, 25 Sep 2026 15:41:21 -0300 Subject: [PATCH 4/7] fix: clear the registration challenge even when verification never runs The challenge is consumed inside `verify_and_save_*`, which is skipped when parsing the credential raises or when an app overrides it. The `ensure` restores the previous guarantee that the challenge is always cleared. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 ---- app/controllers/devise/passkeys_controller.rb | 2 ++ ...second_factor_webauthn_credentials_controller.rb | 2 ++ spec/requests/devise/passkeys_controller_spec.rb | 13 +++++++++++++ ...d_factor_webauthn_credentials_controller_spec.rb | 13 +++++++++++++ 5 files changed, 30 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f558ad43..623e6358 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,10 +6,6 @@ - Add `Devise::Webauthn.challenge_store` to configure where WebAuthn challenges are kept between the options request and the verification request. The default, `Devise::Webauthn::ChallengeStores::Session`, keeps them in the session as before. [#150](https://github.com/cedarcode/devise-webauthn/pull/150) [@RenzoMinelli] -### Changed - -- If you override `verify_and_save_passkey` or `verify_and_save_security_key`, read the challenge with `challenge_store.consume(:registration, params[:public_key_credential])` instead of `session[:webauthn_challenge]`. Otherwise the challenge is no longer cleared after use. [#150](https://github.com/cedarcode/devise-webauthn/pull/150) [@RenzoMinelli] - ## [v0.5.0](https://github.com/cedarcode/devise-webauthn/compare/v0.4.0...v0.5.0/) - 2026-07-13 ### Added diff --git a/app/controllers/devise/passkeys_controller.rb b/app/controllers/devise/passkeys_controller.rb index 09bea47b..91b4cf50 100644 --- a/app/controllers/devise/passkeys_controller.rb +++ b/app/controllers/devise/passkeys_controller.rb @@ -20,6 +20,8 @@ def create rescue WebAuthn::Error set_flash_message! :alert, :passkey_verification_failed, scope: :"devise.failure" redirect_to after_update_path + ensure + challenge_store.consume(:registration, params[:public_key_credential]) end def destroy diff --git a/app/controllers/devise/second_factor_webauthn_credentials_controller.rb b/app/controllers/devise/second_factor_webauthn_credentials_controller.rb index 0792afd7..d8c2e82b 100644 --- a/app/controllers/devise/second_factor_webauthn_credentials_controller.rb +++ b/app/controllers/devise/second_factor_webauthn_credentials_controller.rb @@ -20,6 +20,8 @@ def create rescue WebAuthn::Error set_flash_message! :alert, :webauthn_credential_verification_failed, scope: :"devise.failure" redirect_to after_create_path + ensure + challenge_store.consume(:registration, params[:public_key_credential]) end def update diff --git a/spec/requests/devise/passkeys_controller_spec.rb b/spec/requests/devise/passkeys_controller_spec.rb index 9711071d..bd839ebc 100644 --- a/spec/requests/devise/passkeys_controller_spec.rb +++ b/spec/requests/devise/passkeys_controller_spec.rb @@ -77,6 +77,19 @@ expect(session[:webauthn_challenge]).to be_nil end end + + context "when the credential cannot be parsed" do + before do + allow(WebAuthn::Credential).to receive(:from_create).and_raise(WebAuthn::Error) + end + + it "clears the challenge and redirects" do + post account_passkeys_path, params: { public_key_credential: "{}", name: "My Passkey" } + + expect(response).to redirect_to(new_account_passkey_path) + expect(session[:webauthn_challenge]).to be_nil + end + end end context "when user is not authenticated" do diff --git a/spec/requests/devise/second_factor_webauthn_credentials_controller_spec.rb b/spec/requests/devise/second_factor_webauthn_credentials_controller_spec.rb index 1bd4790a..11bc3df4 100644 --- a/spec/requests/devise/second_factor_webauthn_credentials_controller_spec.rb +++ b/spec/requests/devise/second_factor_webauthn_credentials_controller_spec.rb @@ -87,6 +87,19 @@ expect(session[:webauthn_challenge]).to be_nil end end + + context "when the credential cannot be parsed" do + before do + allow(WebAuthn::Credential).to receive(:from_create).and_raise(WebAuthn::Error) + end + + it "clears the challenge and redirects" do + post account_second_factor_webauthn_credentials_path, params: { public_key_credential: "{}", name: "Key" } + + expect(response).to redirect_to(new_account_second_factor_webauthn_credential_path) + expect(session[:webauthn_challenge]).to be_nil + end + end end end From dfbb8c2468533e6c38b9a82414bf29f8fd17bbeb Mon Sep 17 00:00:00 2001 From: Renzo Minelli Date: Fri, 2 Oct 2026 19:00:19 -0300 Subject: [PATCH 5/7] refactor: address review on challenge store Rename the verified challenge to expected_challenge and the unused store argument to _credential. Move the custom store specs into the passkey authentication spec and extract MemoryChallengeStore to spec/support. Co-Authored-By: Claude Opus 5.5 --- .../strategies/passkey_authenticatable.rb | 8 +- .../webauthn_two_factor_authenticatable.rb | 8 +- .../webauthn/challenge_stores/session.rb | 4 +- .../devise/custom_challenge_store_spec.rb | 82 ------------------- .../devise/passkey_authentication_spec.rb | 48 +++++++++++ spec/spec_helper.rb | 1 + spec/support/memory_challenge_store.rb | 25 ++++++ 7 files changed, 84 insertions(+), 92 deletions(-) delete mode 100644 spec/requests/devise/custom_challenge_store_spec.rb create mode 100644 spec/support/memory_challenge_store.rb diff --git a/lib/devise/strategies/passkey_authenticatable.rb b/lib/devise/strategies/passkey_authenticatable.rb index f9991a8f..aad1514e 100644 --- a/lib/devise/strategies/passkey_authenticatable.rb +++ b/lib/devise/strategies/passkey_authenticatable.rb @@ -10,7 +10,7 @@ def valid? end def authenticate! # rubocop:disable Metrics/AbcSize - challenge = challenge_store.consume(:passkey_authentication, passkey_param) + expected_challenge = challenge_store.consume(:passkey_authentication, passkey_param) passkey_from_params = WebAuthn::Credential.from_get(JSON.parse(passkey_param)) return fail!(:passkey_not_found) if passkey_from_params.user_handle.nil? @@ -20,7 +20,7 @@ def authenticate! # rubocop:disable Metrics/AbcSize return fail!(:passkey_not_found) if stored_passkey.blank? - verify_passkeys(passkey_from_params, stored_passkey, challenge) + verify_passkeys(passkey_from_params, stored_passkey, expected_challenge) remember_me(resource) success!(resource) @@ -34,9 +34,9 @@ def passkey_param params[:public_key_credential] end - def verify_passkeys(passkey_from_params, stored_passkey, challenge) + def verify_passkeys(passkey_from_params, stored_passkey, expected_challenge) passkey_from_params.verify( - challenge, + expected_challenge, public_key: stored_passkey.public_key, sign_count: stored_passkey.sign_count, user_verification: true diff --git a/lib/devise/strategies/webauthn_two_factor_authenticatable.rb b/lib/devise/strategies/webauthn_two_factor_authenticatable.rb index 2445d39c..6dff57e3 100644 --- a/lib/devise/strategies/webauthn_two_factor_authenticatable.rb +++ b/lib/devise/strategies/webauthn_two_factor_authenticatable.rb @@ -13,7 +13,7 @@ def valid? # rubocop:disable Metrics/AbcSize def authenticate! - challenge = challenge_store.consume(:two_factor_authentication, credential_param) + expected_challenge = challenge_store.consume(:two_factor_authentication, credential_param) credential_from_params = WebAuthn::Credential.from_get(JSON.parse(credential_param)) resource = resource_class.find_by(id: session[:current_authentication_resource_id]) stored_credential = resource&.webauthn_credentials&.find_by(external_id: credential_from_params.id) @@ -23,7 +23,7 @@ def authenticate! return fail!(:webauthn_credential_verification_failed) end - verify_credential(credential_from_params, stored_credential, challenge) + verify_credential(credential_from_params, stored_credential, expected_challenge) resource.remember_me = session[:current_authentication_remember_me] if resource.respond_to?(:remember_me=) success!(resource) @@ -41,9 +41,9 @@ def credential_param params[:public_key_credential] end - def verify_credential(credential_from_params, stored_credential, challenge) + def verify_credential(credential_from_params, stored_credential, expected_challenge) credential_from_params.verify( - challenge, + expected_challenge, public_key: stored_credential.public_key, sign_count: stored_credential.sign_count ) diff --git a/lib/devise/webauthn/challenge_stores/session.rb b/lib/devise/webauthn/challenge_stores/session.rb index d6a5e13b..f5924e34 100644 --- a/lib/devise/webauthn/challenge_stores/session.rb +++ b/lib/devise/webauthn/challenge_stores/session.rb @@ -18,11 +18,11 @@ def write(purpose, challenge) @session[KEYS.fetch(purpose)] = challenge end - def pending?(purpose, _credential_json) + def pending?(purpose, _credential) @session[KEYS.fetch(purpose)].present? end - def consume(purpose, _credential_json) + def consume(purpose, _credential) @session.delete(KEYS.fetch(purpose)) end end diff --git a/spec/requests/devise/custom_challenge_store_spec.rb b/spec/requests/devise/custom_challenge_store_spec.rb deleted file mode 100644 index 225ff826..00000000 --- a/spec/requests/devise/custom_challenge_store_spec.rb +++ /dev/null @@ -1,82 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" -require "webauthn/fake_client" - -RSpec.describe "Custom challenge store", type: :request do - let(:memory_store_class) do - Class.new do - def self.challenges - @challenges ||= {} - end - - def initialize(_request); end - - def write(purpose, challenge) - self.class.challenges[purpose] = challenge - end - - def pending?(purpose, _credential_json) - self.class.challenges.key?(purpose) - end - - def consume(purpose, _credential_json) - self.class.challenges.delete(purpose) - end - end - end - - let(:user) { Account.create!(email: "test@example.com", password: "password123") } - let(:client) { WebAuthn::FakeClient.new(WebAuthn.configuration.allowed_origins.first) } - let!(:passkey) do - user.update!(webauthn_id: WebAuthn.generate_user_id) - credential = WebAuthn::Credential.from_create( - client.create(challenge: WebAuthn.configuration.encoder.encode(SecureRandom.random_bytes(32))) - ) - user.passkeys.create!(external_id: credential.id, name: "My Passkey", - public_key: credential.public_key, sign_count: credential.sign_count) - end - - around do |example| - original_store = Devise::Webauthn.challenge_store - Devise::Webauthn.challenge_store = memory_store_class - example.run - ensure - Devise::Webauthn.challenge_store = original_store - end - - def sign_in_with_passkey(challenge) - assertion = client.get( - challenge: challenge, - allow_credentials: [passkey.external_id], - user_verified: true, - user_handle: WebAuthn.configuration.encoder.decode(user.webauthn_id) - ) - - post account_session_path, params: { public_key_credential: assertion.to_json } - end - - it "signs in with a passkey without keeping the challenge in the session" do - post account_passkey_authentication_options_path - challenge = response.parsed_body["challenge"] - - expect(memory_store_class.challenges).to eq(passkey_authentication: challenge) - expect(session[:authentication_challenge]).to be_nil - - sign_in_with_passkey(challenge) - - expect(controller.current_account).to eq(user) - end - - it "consumes the challenge so it cannot be used twice" do - post account_passkey_authentication_options_path - challenge = response.parsed_body["challenge"] - sign_in_with_passkey(challenge) - expect(memory_store_class.challenges).to be_empty - delete destroy_account_session_path - - sign_in_with_passkey(challenge) - - expect(controller.current_account).to be_nil - end -end diff --git a/spec/requests/devise/passkey_authentication_spec.rb b/spec/requests/devise/passkey_authentication_spec.rb index 9a345fe4..99cecc6b 100644 --- a/spec/requests/devise/passkey_authentication_spec.rb +++ b/spec/requests/devise/passkey_authentication_spec.rb @@ -150,4 +150,52 @@ def generate_assertion(fake_client, challenge:, credential:, user_handle: nil) expect(controller.current_account).to be_nil end end + + describe "sign-in with passkeys through a custom challenge store" do + let!(:passkey) { create_passkey_for(user, client) } + + around do |example| + original_store = Devise::Webauthn.challenge_store + Devise::Webauthn.challenge_store = MemoryChallengeStore + example.run + ensure + Devise::Webauthn.challenge_store = original_store + MemoryChallengeStore.reset + end + + def sign_in_with_passkey(challenge) + assertion = generate_assertion( + client, + challenge: challenge, + credential: passkey, + user_handle: WebAuthn.configuration.encoder.decode(user.webauthn_id) + ) + + post account_session_path, params: { public_key_credential: assertion.to_json } + end + + it "signs in without keeping the challenge in the session" do + post account_passkey_authentication_options_path + challenge = response.parsed_body["challenge"] + + expect(MemoryChallengeStore.challenges).to eq(passkey_authentication: challenge) + expect(session[:authentication_challenge]).to be_nil + + sign_in_with_passkey(challenge) + + expect(controller.current_account).to eq(user) + end + + it "consumes the challenge so it cannot be used twice" do + post account_passkey_authentication_options_path + challenge = response.parsed_body["challenge"] + sign_in_with_passkey(challenge) + expect(MemoryChallengeStore.challenges).to be_empty + delete destroy_account_session_path + + sign_in_with_passkey(challenge) + + expect(controller.current_account).to be_nil + end + end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9d69f4e7..785894c4 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -16,6 +16,7 @@ require "rspec/retry" require "capybara/rspec" require "support/page_load_helpers" +require "support/memory_challenge_store" RSpec.configure do |config| # Enable flags like --only-failures and --next-failure diff --git a/spec/support/memory_challenge_store.rb b/spec/support/memory_challenge_store.rb new file mode 100644 index 00000000..e6efad1a --- /dev/null +++ b/spec/support/memory_challenge_store.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class MemoryChallengeStore + def self.challenges + @challenges ||= {} + end + + def self.reset + challenges.clear + end + + def initialize(_request); end + + def write(purpose, challenge) + self.class.challenges[purpose] = challenge + end + + def pending?(purpose, _credential) + self.class.challenges.key?(purpose) + end + + def consume(purpose, _credential) + self.class.challenges.delete(purpose) + end +end From 316a49ffb5261ec154347712ee5c75685474ea31 Mon Sep 17 00:00:00 2001 From: Renzo Minelli Date: Tue, 6 Oct 2026 09:19:58 -0300 Subject: [PATCH 6/7] refactor: select the challenge store with a symbol Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 2 +- lib/devise/webauthn.rb | 2 +- lib/devise/webauthn/challenge_store_access.rb | 4 +- .../webauthn/install/templates/webauthn.rb | 3 ++ .../webauthn/challenge_store_access_spec.rb | 49 +++++++++++++++++++ 5 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 spec/devise/webauthn/challenge_store_access_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 623e6358..1ae55486 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Added -- Add `Devise::Webauthn.challenge_store` to configure where WebAuthn challenges are kept between the options request and the verification request. The default, `Devise::Webauthn::ChallengeStores::Session`, keeps them in the session as before. [#150](https://github.com/cedarcode/devise-webauthn/pull/150) [@RenzoMinelli] +- Add `Devise::Webauthn.challenge_store` to configure where WebAuthn challenges are kept between the options request and the verification request. It takes a symbol such as `:session`, or a store class. The default, `:session`, keeps them in the session as before. [#150](https://github.com/cedarcode/devise-webauthn/pull/150) [@RenzoMinelli] ## [v0.5.0](https://github.com/cedarcode/devise-webauthn/compare/v0.4.0...v0.5.0/) - 2026-07-13 diff --git a/lib/devise/webauthn.rb b/lib/devise/webauthn.rb index 04931b7f..0b9f71cb 100644 --- a/lib/devise/webauthn.rb +++ b/lib/devise/webauthn.rb @@ -13,7 +13,7 @@ module Devise module Webauthn - mattr_accessor :challenge_store, default: ChallengeStores::Session + mattr_accessor :challenge_store, default: :session module Test autoload :AuthenticatorHelpers, "devise/webauthn/test/authenticator_helpers" diff --git a/lib/devise/webauthn/challenge_store_access.rb b/lib/devise/webauthn/challenge_store_access.rb index 1b092873..e7bd1b2e 100644 --- a/lib/devise/webauthn/challenge_store_access.rb +++ b/lib/devise/webauthn/challenge_store_access.rb @@ -6,7 +6,9 @@ module ChallengeStoreAccess private def challenge_store - Devise::Webauthn.challenge_store.new(request) + store = Devise::Webauthn.challenge_store + store = Devise::Webauthn::ChallengeStores.const_get(store.to_s.camelize) if store.is_a?(Symbol) + store.new(request) end end end diff --git a/lib/generators/devise/webauthn/install/templates/webauthn.rb b/lib/generators/devise/webauthn/install/templates/webauthn.rb index d6ee138c..3f7d8367 100644 --- a/lib/generators/devise/webauthn/install/templates/webauthn.rb +++ b/lib/generators/devise/webauthn/install/templates/webauthn.rb @@ -35,3 +35,6 @@ # # config.algorithms << "ES384" end + +# Where challenges are kept between requests: `:session` (default) or a store class. +# Devise::Webauthn.challenge_store = :session diff --git a/spec/devise/webauthn/challenge_store_access_spec.rb b/spec/devise/webauthn/challenge_store_access_spec.rb new file mode 100644 index 00000000..66ba3bd0 --- /dev/null +++ b/spec/devise/webauthn/challenge_store_access_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Devise::Webauthn::ChallengeStoreAccess do + let(:request) { instance_double(ActionDispatch::Request, session: {}) } + let(:host) do + Class.new do + include Devise::Webauthn::ChallengeStoreAccess + + attr_reader :request + + def initialize(request) + @request = request + end + + public :challenge_store + end.new(request) + end + + around do |example| + original_store = Devise::Webauthn.challenge_store + example.run + ensure + Devise::Webauthn.challenge_store = original_store + end + + it "uses the session store by default" do + expect(host.challenge_store).to be_a(Devise::Webauthn::ChallengeStores::Session) + end + + it "resolves a symbol to the matching store" do + Devise::Webauthn.challenge_store = :session + + expect(host.challenge_store).to be_a(Devise::Webauthn::ChallengeStores::Session) + end + + it "uses a store class as given" do + Devise::Webauthn.challenge_store = MemoryChallengeStore + + expect(host.challenge_store).to be_a(MemoryChallengeStore) + end + + it "raises on an unknown symbol" do + Devise::Webauthn.challenge_store = :unknown + + expect { host.challenge_store }.to raise_error(NameError) + end +end From 46aaa6c1d79d18bf732dbacbf1873edfab779eca Mon Sep 17 00:00:00 2001 From: Renzo Minelli Date: Wed, 7 Oct 2026 10:03:14 -0300 Subject: [PATCH 7/7] refactor: drop the credential argument from the challenge store interface Co-Authored-By: Claude Sonnet 5.5 --- app/controllers/devise/passkeys_controller.rb | 4 +- ..._factor_webauthn_credentials_controller.rb | 4 +- .../strategies/passkey_authenticatable.rb | 4 +- .../webauthn_two_factor_authenticatable.rb | 4 +- .../webauthn/challenge_stores/session.rb | 4 +- .../webauthn/challenge_stores/session_spec.rb | 8 +- .../devise/passkey_authentication_spec.rb | 91 ++++++++----------- spec/support/memory_challenge_store.rb | 4 +- 8 files changed, 53 insertions(+), 70 deletions(-) diff --git a/app/controllers/devise/passkeys_controller.rb b/app/controllers/devise/passkeys_controller.rb index 91b4cf50..e1d749c8 100644 --- a/app/controllers/devise/passkeys_controller.rb +++ b/app/controllers/devise/passkeys_controller.rb @@ -21,7 +21,7 @@ def create set_flash_message! :alert, :passkey_verification_failed, scope: :"devise.failure" redirect_to after_update_path ensure - challenge_store.consume(:registration, params[:public_key_credential]) + challenge_store.consume(:registration) end def destroy @@ -43,7 +43,7 @@ def authenticate_scope! def verify_and_save_passkey(passkey_from_params) passkey_from_params.verify( - challenge_store.consume(:registration, params[:public_key_credential]), + challenge_store.consume(:registration), user_verification: true ) diff --git a/app/controllers/devise/second_factor_webauthn_credentials_controller.rb b/app/controllers/devise/second_factor_webauthn_credentials_controller.rb index d8c2e82b..266c2007 100644 --- a/app/controllers/devise/second_factor_webauthn_credentials_controller.rb +++ b/app/controllers/devise/second_factor_webauthn_credentials_controller.rb @@ -21,7 +21,7 @@ def create set_flash_message! :alert, :webauthn_credential_verification_failed, scope: :"devise.failure" redirect_to after_create_path ensure - challenge_store.consume(:registration, params[:public_key_credential]) + challenge_store.consume(:registration) end def update @@ -53,7 +53,7 @@ def authenticate_scope! def verify_and_save_security_key(security_key_from_params) security_key_from_params.verify( - challenge_store.consume(:registration, params[:public_key_credential]) + challenge_store.consume(:registration) ) resource.second_factor_webauthn_credentials.create( diff --git a/lib/devise/strategies/passkey_authenticatable.rb b/lib/devise/strategies/passkey_authenticatable.rb index aad1514e..06129b2b 100644 --- a/lib/devise/strategies/passkey_authenticatable.rb +++ b/lib/devise/strategies/passkey_authenticatable.rb @@ -6,11 +6,11 @@ class PasskeyAuthenticatable < Devise::Strategies::Base include Devise::Webauthn::ChallengeStoreAccess def valid? - passkey_param.present? && challenge_store.pending?(:passkey_authentication, passkey_param) + passkey_param.present? && challenge_store.pending?(:passkey_authentication) end def authenticate! # rubocop:disable Metrics/AbcSize - expected_challenge = challenge_store.consume(:passkey_authentication, passkey_param) + expected_challenge = challenge_store.consume(:passkey_authentication) passkey_from_params = WebAuthn::Credential.from_get(JSON.parse(passkey_param)) return fail!(:passkey_not_found) if passkey_from_params.user_handle.nil? diff --git a/lib/devise/strategies/webauthn_two_factor_authenticatable.rb b/lib/devise/strategies/webauthn_two_factor_authenticatable.rb index 6dff57e3..9e845278 100644 --- a/lib/devise/strategies/webauthn_two_factor_authenticatable.rb +++ b/lib/devise/strategies/webauthn_two_factor_authenticatable.rb @@ -8,12 +8,12 @@ class WebauthnTwoFactorAuthenticatable < Devise::Strategies::Base def valid? credential_param.present? && session[:current_authentication_resource_id].present? && - challenge_store.pending?(:two_factor_authentication, credential_param) + challenge_store.pending?(:two_factor_authentication) end # rubocop:disable Metrics/AbcSize def authenticate! - expected_challenge = challenge_store.consume(:two_factor_authentication, credential_param) + expected_challenge = challenge_store.consume(:two_factor_authentication) credential_from_params = WebAuthn::Credential.from_get(JSON.parse(credential_param)) resource = resource_class.find_by(id: session[:current_authentication_resource_id]) stored_credential = resource&.webauthn_credentials&.find_by(external_id: credential_from_params.id) diff --git a/lib/devise/webauthn/challenge_stores/session.rb b/lib/devise/webauthn/challenge_stores/session.rb index f5924e34..e08eb0f9 100644 --- a/lib/devise/webauthn/challenge_stores/session.rb +++ b/lib/devise/webauthn/challenge_stores/session.rb @@ -18,11 +18,11 @@ def write(purpose, challenge) @session[KEYS.fetch(purpose)] = challenge end - def pending?(purpose, _credential) + def pending?(purpose) @session[KEYS.fetch(purpose)].present? end - def consume(purpose, _credential) + def consume(purpose) @session.delete(KEYS.fetch(purpose)) end end diff --git a/spec/devise/webauthn/challenge_stores/session_spec.rb b/spec/devise/webauthn/challenge_stores/session_spec.rb index 3d42c94f..71c755cb 100644 --- a/spec/devise/webauthn/challenge_stores/session_spec.rb +++ b/spec/devise/webauthn/challenge_stores/session_spec.rb @@ -21,13 +21,13 @@ it "reports a challenge as pending until it is consumed" do store.write(:passkey_authentication, "challenge") - expect(store.pending?(:passkey_authentication, "{}")).to be(true) - expect(store.consume(:passkey_authentication, "{}")).to eq("challenge") - expect(store.pending?(:passkey_authentication, "{}")).to be(false) + expect(store.pending?(:passkey_authentication)).to be(true) + expect(store.consume(:passkey_authentication)).to eq("challenge") + expect(store.pending?(:passkey_authentication)).to be(false) end it "returns nil when consuming a challenge that was never written" do - expect(store.consume(:registration, "{}")).to be_nil + expect(store.consume(:registration)).to be_nil end it "raises on an unknown purpose" do diff --git a/spec/requests/devise/passkey_authentication_spec.rb b/spec/requests/devise/passkey_authentication_spec.rb index 99cecc6b..c28d27a2 100644 --- a/spec/requests/devise/passkey_authentication_spec.rb +++ b/spec/requests/devise/passkey_authentication_spec.rb @@ -9,6 +9,19 @@ let(:origin) { WebAuthn.configuration.allowed_origins.first } let(:client) { WebAuthn::FakeClient.new(origin) } + around do |example| + original_store = Devise::Webauthn.challenge_store + Devise::Webauthn.challenge_store = MemoryChallengeStore + example.run + ensure + Devise::Webauthn.challenge_store = original_store + MemoryChallengeStore.reset + end + + def stored_challenge + MemoryChallengeStore.challenges[:passkey_authentication] + end + def create_passkey_for(account, fake_client) account.update!(webauthn_id: WebAuthn.generate_user_id) challenge = WebAuthn.configuration.encoder.encode(SecureRandom.random_bytes(32)) @@ -36,7 +49,7 @@ def generate_assertion(fake_client, challenge:, credential:, user_handle: nil) let!(:passkey) { create_passkey_for(user, client) } before do - post account_passkey_authentication_options_path # To set the challenge in session + post account_passkey_authentication_options_path # To store the challenge end it "completes authentication with valid credential" do @@ -44,7 +57,7 @@ def generate_assertion(fake_client, challenge:, credential:, user_handle: nil) assertion = generate_assertion( client, - challenge: session[:authentication_challenge], + challenge: stored_challenge, credential: passkey, user_handle: WebAuthn.configuration.encoder.decode(user.webauthn_id) ) @@ -57,16 +70,34 @@ def generate_assertion(fake_client, challenge:, credential:, user_handle: nil) expect(response).to redirect_to(root_path) expect(flash[:notice]).to eq(I18n.t("devise.sessions.signed_in")) expect(controller.current_account).to eq(user) - expect(session[:authentication_challenge]).to be_nil + expect(stored_challenge).to be_nil end.to change { passkey.reload.sign_count }.by(1) end + it "consumes the challenge so it cannot be used twice" do + assertion = generate_assertion( + client, + challenge: stored_challenge, + credential: passkey, + user_handle: WebAuthn.configuration.encoder.decode(user.webauthn_id) + ) + params = { public_key_credential: assertion.to_json } + + post account_session_path, params: params + expect(controller.current_account).to eq(user) + delete destroy_account_session_path + + post account_session_path, params: params + + expect(controller.current_account).to be_nil + end + it "rejects sign-in with non-existent credential" do get new_account_session_path assertion = generate_assertion( client, - challenge: session[:authentication_challenge], + challenge: stored_challenge, credential: passkey, user_handle: WebAuthn.configuration.encoder.decode(user.webauthn_id) ) @@ -105,7 +136,7 @@ def generate_assertion(fake_client, challenge:, credential:, user_handle: nil) get new_account_session_path assertion = client.get( - challenge: session[:authentication_challenge], + challenge: stored_challenge, allow_credentials: [passkey.external_id], user_verified: true ) @@ -126,7 +157,7 @@ def generate_assertion(fake_client, challenge:, credential:, user_handle: nil) get new_account_session_path assertion = client.get( - challenge: session[:authentication_challenge], + challenge: stored_challenge, allow_credentials: [passkey.external_id], user_verified: true, user_handle: WebAuthn.configuration.encoder.decode(other_user.webauthn_id) @@ -150,52 +181,4 @@ def generate_assertion(fake_client, challenge:, credential:, user_handle: nil) expect(controller.current_account).to be_nil end end - - describe "sign-in with passkeys through a custom challenge store" do - let!(:passkey) { create_passkey_for(user, client) } - - around do |example| - original_store = Devise::Webauthn.challenge_store - Devise::Webauthn.challenge_store = MemoryChallengeStore - example.run - ensure - Devise::Webauthn.challenge_store = original_store - MemoryChallengeStore.reset - end - - def sign_in_with_passkey(challenge) - assertion = generate_assertion( - client, - challenge: challenge, - credential: passkey, - user_handle: WebAuthn.configuration.encoder.decode(user.webauthn_id) - ) - - post account_session_path, params: { public_key_credential: assertion.to_json } - end - - it "signs in without keeping the challenge in the session" do - post account_passkey_authentication_options_path - challenge = response.parsed_body["challenge"] - - expect(MemoryChallengeStore.challenges).to eq(passkey_authentication: challenge) - expect(session[:authentication_challenge]).to be_nil - - sign_in_with_passkey(challenge) - - expect(controller.current_account).to eq(user) - end - - it "consumes the challenge so it cannot be used twice" do - post account_passkey_authentication_options_path - challenge = response.parsed_body["challenge"] - sign_in_with_passkey(challenge) - expect(MemoryChallengeStore.challenges).to be_empty - delete destroy_account_session_path - - sign_in_with_passkey(challenge) - - expect(controller.current_account).to be_nil - end - end end diff --git a/spec/support/memory_challenge_store.rb b/spec/support/memory_challenge_store.rb index e6efad1a..e610197d 100644 --- a/spec/support/memory_challenge_store.rb +++ b/spec/support/memory_challenge_store.rb @@ -15,11 +15,11 @@ def write(purpose, challenge) self.class.challenges[purpose] = challenge end - def pending?(purpose, _credential) + def pending?(purpose) self.class.challenges.key?(purpose) end - def consume(purpose, _credential) + def consume(purpose) self.class.challenges.delete(purpose) end end