Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions lib/devise/strategies/passkey_authenticatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,18 @@ 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))
stored_passkey = WebauthnCredential.passkey.find_by(external_id: passkey_from_params.id)

return fail!(:passkey_not_found) if passkey_from_params.user_handle.nil?

resource = resource_class.find_by(webauthn_id: passkey_from_params.user_handle)
Comment thread
RenzoMinelli marked this conversation as resolved.
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)
Expand All @@ -40,8 +43,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
Comment thread
RenzoMinelli marked this conversation as resolved.
end
end
end
Expand Down
8 changes: 8 additions & 0 deletions lib/devise/strategies/webauthn_two_factor_authenticatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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? &&
Comment thread
RenzoMinelli marked this conversation as resolved.
credential_from_params.user_handle != resource.webauthn_id
end

def resource_class
mapping.to
end
Expand Down
53 changes: 48 additions & 5 deletions spec/requests/devise/passkey_authentication_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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!

Expand All @@ -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: {
Expand All @@ -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: {}

Expand Down
54 changes: 54 additions & 0 deletions spec/requests/devise/two_factor_authentication_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 } }

Expand Down