diff --git a/CHANGELOG.md b/CHANGELOG.md index a1a01f7f..1ae55486 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Added + +- 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 ### Added diff --git a/app/controllers/devise/passkey_authentication_options_controller.rb b/app/controllers/devise/passkey_authentication_options_controller.rb index 1db26e3b..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,8 +12,7 @@ def create user_verification: "required" ) - # Store challenge in session for later verification - session[:authentication_challenge] = 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 e4606783..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,8 +23,7 @@ def create } ) - # Store challenge in session for later verification - session[:webauthn_challenge] = 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 ae67f5c3..e1d749c8 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 @@ -19,7 +21,7 @@ def create set_flash_message! :alert, :passkey_verification_failed, scope: :"devise.failure" redirect_to after_update_path ensure - session.delete(:webauthn_challenge) + challenge_store.consume(:registration) end def destroy @@ -41,7 +43,7 @@ def authenticate_scope! def verify_and_save_passkey(passkey_from_params) passkey_from_params.verify( - session[:webauthn_challenge], + 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 64023c63..266c2007 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 @@ -19,7 +21,7 @@ def create set_flash_message! :alert, :webauthn_credential_verification_failed, scope: :"devise.failure" redirect_to after_create_path ensure - session.delete(:webauthn_challenge) + challenge_store.consume(:registration) end def update @@ -51,7 +53,7 @@ def authenticate_scope! def verify_and_save_security_key(security_key_from_params) security_key_from_params.verify( - session[:webauthn_challenge] + challenge_store.consume(:registration) ) 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..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" ) - # Store challenge in session for later verification - session[:two_factor_authentication_challenge] = 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 a536cc11..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,8 +23,7 @@ def create } ) - # Store challenge in session for later verification - session[:webauthn_challenge] = 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 78b61179..06129b2b 100644 --- a/lib/devise/strategies/passkey_authenticatable.rb +++ b/lib/devise/strategies/passkey_authenticatable.rb @@ -3,11 +3,14 @@ module Devise module Strategies class PasskeyAuthenticatable < Devise::Strategies::Base + include Devise::Webauthn::ChallengeStoreAccess + def valid? - passkey_param.present? && session[:authentication_challenge].present? + passkey_param.present? && challenge_store.pending?(:passkey_authentication) end def authenticate! # rubocop:disable Metrics/AbcSize + 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? @@ -17,14 +20,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, expected_challenge) remember_me(resource) success!(resource) rescue WebAuthn::Error fail!(:passkey_verification_failed) - ensure - session.delete(:authentication_challenge) end private @@ -33,9 +34,9 @@ def passkey_param params[:public_key_credential] end - def verify_passkeys(passkey_from_params, stored_passkey) + def verify_passkeys(passkey_from_params, stored_passkey, expected_challenge) passkey_from_params.verify( - session[:authentication_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 23d4712f..9e845278 100644 --- a/lib/devise/strategies/webauthn_two_factor_authenticatable.rb +++ b/lib/devise/strategies/webauthn_two_factor_authenticatable.rb @@ -3,14 +3,17 @@ module Devise module Strategies class WebauthnTwoFactorAuthenticatable < Devise::Strategies::Base + include Devise::Webauthn::ChallengeStoreAccess + def valid? credential_param.present? && session[:current_authentication_resource_id].present? && - session[:two_factor_authentication_challenge].present? + challenge_store.pending?(:two_factor_authentication) end # rubocop:disable Metrics/AbcSize def authenticate! + 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) @@ -20,7 +23,7 @@ def authenticate! return fail!(:webauthn_credential_verification_failed) end - verify_credential(credential_from_params, stored_credential) + 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) @@ -29,8 +32,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 +41,9 @@ def credential_param params[:public_key_credential] end - def verify_credential(credential_from_params, stored_credential) + def verify_credential(credential_from_params, stored_credential, expected_challenge) credential_from_params.verify( - session[:two_factor_authentication_challenge], + expected_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..0b9f71cb 100644 --- a/lib/devise/webauthn.rb +++ b/lib/devise/webauthn.rb @@ -4,6 +4,8 @@ require "webauthn" 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" @@ -11,6 +13,8 @@ module Devise module Webauthn + mattr_accessor :challenge_store, default: :session + 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..e7bd1b2e --- /dev/null +++ b/lib/devise/webauthn/challenge_store_access.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Devise + module Webauthn + module ChallengeStoreAccess + private + + def challenge_store + 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 +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..e08eb0f9 --- /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) + @session[KEYS.fetch(purpose)].present? + end + + def consume(purpose) + @session.delete(KEYS.fetch(purpose)) + end + end + 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 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..71c755cb --- /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/passkey_authentication_spec.rb b/spec/requests/devise/passkey_authentication_spec.rb index 9a345fe4..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) 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 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..e610197d --- /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) + self.class.challenges.key?(purpose) + end + + def consume(purpose) + self.class.challenges.delete(purpose) + end +end