Skip to content

Commit 8fe5640

Browse files
Validate userHandle from authenticator response against webauthn_id (#131)
* feat: validate `userHandle` in `PasskeyAuthenticatable` strategy * feat: validate `userHandle` in `WebauthnTwoFactorAuthenticatable` strategy * feat: fail early when `response.userHandle` is not present * chore: disable `Metrics/AbcSize` for `PasskeyAuthenticatable#authenticate!`
1 parent 99b3b3f commit 8fe5640

4 files changed

Lines changed: 118 additions & 10 deletions

File tree

lib/devise/strategies/passkey_authenticatable.rb

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,18 @@ def valid?
77
passkey_param.present? && session[:authentication_challenge].present?
88
end
99

10-
def authenticate!
10+
def authenticate! # rubocop:disable Metrics/AbcSize
1111
passkey_from_params = WebAuthn::Credential.from_get(JSON.parse(passkey_param))
12-
stored_passkey = WebauthnCredential.passkey.find_by(external_id: passkey_from_params.id)
12+
13+
return fail!(:passkey_not_found) if passkey_from_params.user_handle.nil?
14+
15+
resource = resource_class.find_by(webauthn_id: passkey_from_params.user_handle)
16+
stored_passkey = resource&.passkeys&.find_by(external_id: passkey_from_params.id)
1317

1418
return fail!(:passkey_not_found) if stored_passkey.blank?
1519

1620
verify_passkeys(passkey_from_params, stored_passkey)
1721

18-
resource = stored_passkey.public_send(resource_name)
1922
success!(resource)
2023
rescue WebAuthn::Error
2124
fail!(:passkey_verification_failed)
@@ -40,8 +43,8 @@ def verify_passkeys(passkey_from_params, stored_passkey)
4043
stored_passkey.update!(sign_count: passkey_from_params.sign_count)
4144
end
4245

43-
def resource_name
44-
mapping.to.name.underscore
46+
def resource_class
47+
mapping.to
4548
end
4649
end
4750
end

lib/devise/strategies/webauthn_two_factor_authenticatable.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ def authenticate!
1616
stored_credential = resource&.webauthn_credentials&.find_by(external_id: credential_from_params.id)
1717

1818
return fail!(:webauthn_credential_not_found) if stored_credential.blank?
19+
if user_handle_mismatch?(credential_from_params, resource)
20+
return fail!(:webauthn_credential_verification_failed)
21+
end
1922

2023
verify_credential(credential_from_params, stored_credential)
2124

@@ -47,6 +50,11 @@ def verify_credential(credential_from_params, stored_credential)
4750
stored_credential.update!(sign_count: credential_from_params.sign_count)
4851
end
4952

53+
def user_handle_mismatch?(credential_from_params, resource)
54+
credential_from_params.user_handle.present? &&
55+
credential_from_params.user_handle != resource.webauthn_id
56+
end
57+
5058
def resource_class
5159
mapping.to
5260
end

spec/requests/devise/passkey_authentication_spec.rb

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,12 @@ def create_passkey_for(account, fake_client)
2222
)
2323
end
2424

25-
def generate_assertion(fake_client, challenge:, credential:)
25+
def generate_assertion(fake_client, challenge:, credential:, user_handle: nil)
2626
fake_client.get(
2727
challenge: challenge,
2828
allow_credentials: [credential.external_id],
29-
user_verified: true
29+
user_verified: true,
30+
user_handle: user_handle
3031
)
3132
end
3233

@@ -43,7 +44,8 @@ def generate_assertion(fake_client, challenge:, credential:)
4344
assertion = generate_assertion(
4445
client,
4546
challenge: session[:authentication_challenge],
46-
credential: passkey
47+
credential: passkey,
48+
user_handle: WebAuthn.configuration.encoder.decode(user.webauthn_id)
4749
)
4850

4951
expect do
@@ -64,7 +66,8 @@ def generate_assertion(fake_client, challenge:, credential:)
6466
assertion = generate_assertion(
6567
client,
6668
challenge: session[:authentication_challenge],
67-
credential: passkey
69+
credential: passkey,
70+
user_handle: WebAuthn.configuration.encoder.decode(user.webauthn_id)
6871
)
6972
passkey.destroy!
7073

@@ -83,7 +86,8 @@ def generate_assertion(fake_client, challenge:, credential:)
8386

8487
assertion = client.get(
8588
challenge: WebAuthn.configuration.encoder.encode("invalid_challenge"),
86-
allow_credentials: [passkey.external_id]
89+
allow_credentials: [passkey.external_id],
90+
user_handle: WebAuthn.configuration.encoder.decode(user.webauthn_id)
8791
)
8892

8993
post account_session_path, params: {
@@ -96,6 +100,45 @@ def generate_assertion(fake_client, challenge:, credential:)
96100
expect(controller.current_account).to be_nil
97101
end
98102

103+
it "rejects sign-in when userHandle is missing from the response" do
104+
get new_account_session_path
105+
106+
assertion = client.get(
107+
challenge: session[:authentication_challenge],
108+
allow_credentials: [passkey.external_id],
109+
user_verified: true
110+
)
111+
112+
post account_session_path, params: {
113+
public_key_credential: assertion.to_json
114+
}
115+
116+
expect(response).to have_http_status(:unprocessable_entity)
117+
expect(flash[:alert]).to eq(I18n.t("devise.failure.passkey_not_found"))
118+
expect(controller.current_account).to be_nil
119+
end
120+
121+
it "rejects sign-in when userHandle does not match the passkey owner" do
122+
other_user = Account.create!(email: "other@example.com", password: password)
123+
124+
get new_account_session_path
125+
126+
assertion = client.get(
127+
challenge: session[:authentication_challenge],
128+
allow_credentials: [passkey.external_id],
129+
user_verified: true,
130+
user_handle: WebAuthn.configuration.encoder.decode(other_user.webauthn_id)
131+
)
132+
133+
post account_session_path, params: {
134+
public_key_credential: assertion.to_json
135+
}
136+
137+
expect(response).to have_http_status(:unprocessable_entity)
138+
expect(flash[:alert]).to eq(I18n.t("devise.failure.passkey_not_found"))
139+
expect(controller.current_account).to be_nil
140+
end
141+
99142
it "fails when credential param is missing" do
100143
post account_session_path, params: {}
101144

spec/requests/devise/two_factor_authentication_spec.rb

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,60 @@ def generate_assertion(fake_client, challenge:, credential:)
164164
expect(controller.current_account).to be_nil
165165
end
166166

167+
it "completes authentication when userHandle matches the authenticated user" do
168+
post account_session_path, params: { account: { email: user.email, password: password } }
169+
170+
expect(response).to redirect_to(new_account_two_factor_authentication_path)
171+
172+
follow_redirect!
173+
174+
expect(response).to have_http_status(:ok)
175+
get account_security_key_authentication_options_path
176+
177+
assertion = client.get(
178+
challenge: session[:two_factor_authentication_challenge],
179+
allow_credentials: [security_key.external_id],
180+
user_handle: WebAuthn.configuration.encoder.decode(user.webauthn_id)
181+
)
182+
183+
expect do
184+
post account_two_factor_authentication_path, params: {
185+
public_key_credential: assertion.to_json
186+
}
187+
188+
expect(response).to redirect_to(root_path)
189+
expect(controller.current_account).to eq(user)
190+
end.to change { security_key.reload.sign_count }.by(1)
191+
end
192+
193+
it "rejects 2FA when userHandle does not match the authenticated user" do
194+
other_user = Account.create!(email: "other@example.com", password: password)
195+
196+
post account_session_path, params: { account: { email: user.email, password: password } }
197+
198+
expect(response).to redirect_to(new_account_two_factor_authentication_path)
199+
200+
follow_redirect!
201+
202+
expect(response).to have_http_status(:ok)
203+
get account_security_key_authentication_options_path
204+
205+
assertion = client.get(
206+
challenge: session[:two_factor_authentication_challenge],
207+
allow_credentials: [security_key.external_id],
208+
user_handle: WebAuthn.configuration.encoder.decode(other_user.webauthn_id)
209+
)
210+
211+
post account_two_factor_authentication_path, params: {
212+
public_key_credential: assertion.to_json
213+
}
214+
215+
expect(response).to have_http_status(:unprocessable_entity)
216+
expect(response.body).to include("Use security key")
217+
expect(flash[:alert]).to eq(I18n.t("devise.failure.webauthn_credential_verification_failed"))
218+
expect(controller.current_account).to be_nil
219+
end
220+
167221
it "re-renders 2FA page when credential param is missing" do
168222
post account_session_path, params: { account: { email: user.email, password: password } }
169223

0 commit comments

Comments
 (0)