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
14 changes: 10 additions & 4 deletions app/controllers/users/webauthn_setup_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def process_valid_webauthn(form)
success: true,
)
handle_remember_device_preference(params[:remember_device])
if form.platform_authenticator?
if form.setup_as_platform_authenticator?
handle_valid_verification_for_confirmation_context(
auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN_PLATFORM,
)
Expand All @@ -144,7 +144,7 @@ def process_valid_webauthn(form)
analytics,
threatmetrix_attrs,
)
flash[:success] = t('notices.webauthn_platform_configured')
flash[:success] = t('notices.webauthn_platform_configured') if !form.transports_mismatch?
else
handle_valid_verification_for_confirmation_context(
auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN,
Expand All @@ -155,9 +155,15 @@ def process_valid_webauthn(form)
analytics,
threatmetrix_attrs,
)
flash[:success] = t('notices.webauthn_configured')
flash[:success] = t('notices.webauthn_configured') if !form.transports_mismatch?
end

if form.transports_mismatch?
user_session[:webauthn_mismatch_id] = form.webauthn_configuration.id
redirect_to webauthn_setup_mismatch_path
else
redirect_to next_setup_path || after_mfa_setup_path
end
redirect_to next_setup_path || after_mfa_setup_path
end

def analytics_properties
Expand Down
39 changes: 30 additions & 9 deletions app/forms/webauthn_setup_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class WebauthnSetupForm
validate :name_is_unique
validate :validate_attestation_response

attr_reader :attestation_response
attr_reader :attestation_response, :webauthn_configuration

def initialize(user:, user_session:, device_name:)
@user = user
Expand Down Expand Up @@ -48,6 +48,18 @@ def platform_authenticator?
!!@platform_authenticator
end

def setup_as_platform_authenticator?
if transports.present?
platform_authenticator_transports?
else
platform_authenticator?
end
end

def transports_mismatch?
transports.present? && platform_authenticator_transports? != platform_authenticator?
end

def generic_error_message
if platform_authenticator?
I18n.t('errors.webauthn_platform_setup.general_error')
Expand Down Expand Up @@ -134,11 +146,11 @@ def create_webauthn_configuration
credential = attestation_response.credential
public_key = Base64.strict_encode64(credential.public_key)
id = Base64.strict_encode64(credential.id)
user.webauthn_configurations.create(
@webauthn_configuration = user.webauthn_configurations.create(
credential_public_key: public_key,
credential_id: id,
name: name,
platform_authenticator: platform_authenticator,
platform_authenticator: setup_as_platform_authenticator?,
transports: transports.presence,
authenticator_data_flags: authenticator_data_flags,
aaguid: aaguid,
Expand Down Expand Up @@ -166,20 +178,29 @@ def aaguid
nil
end

def platform_authenticator_transports?
(transports & WebauthnConfiguration::PLATFORM_AUTHENTICATOR_TRANSPORTS.to_a).present?
end

def multi_factor_auth_method
if setup_as_platform_authenticator?
'webauthn_platform'
else
'webauthn'
end
end

def extra_analytics_attributes
auth_method = if platform_authenticator?
'webauthn_platform'
else
'webauthn'
end
{
mfa_method_counts: mfa_user.enabled_two_factor_configuration_counts_hash,
enabled_mfa_methods_count: mfa_user.enabled_mfa_methods_count,
multi_factor_auth_method: auth_method,
multi_factor_auth_method:,
pii_like_keypaths: [[:mfa_method_counts, :phone]],
authenticator_data_flags: authenticator_data_flags,
unknown_transports: invalid_transports.presence,
aaguid: aaguid,
transports: transports,
transports_mismatch: transports_mismatch?,
}
end
end
6 changes: 6 additions & 0 deletions app/models/webauthn_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ class WebauthnConfiguration < ApplicationRecord
validates :credential_public_key, presence: true
validate :valid_transports

# https://w3c.github.io/webauthn/#enum-transport
PLATFORM_AUTHENTICATOR_TRANSPORTS = %w[
hybrid
internal
].to_set.freeze

# https://w3c.github.io/webauthn/#enum-transport
VALID_TRANSPORTS = %w[
usb
Expand Down
23 changes: 17 additions & 6 deletions app/services/analytics_events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5834,6 +5834,10 @@ def multi_factor_auth_phone_setup(
# @param [String, nil] aaguid AAGUID value of WebAuthn device
# @param [String[], nil] unknown_transports Array of unrecognized WebAuthn transports, intended to
# be used in case of future specification changes.
# @param [String[], nil] transports WebAuthn transports associated with registration.
# @param [Boolean, nil] transports_mismatch Whether the WebAuthn transports associated with
# registration contradict the authenticator attachment for user setup. For example, a user can
# set up a platform authenticator through the Security Key setup flow.
# @param [:authentication, :account_creation, nil] webauthn_platform_recommended A/B test for
# recommended Face or Touch Unlock setup, if applicable.
def multi_factor_auth_setup(
Expand All @@ -5859,6 +5863,8 @@ def multi_factor_auth_setup(
attempts: nil,
aaguid: nil,
unknown_transports: nil,
transports: nil,
transports_mismatch: nil,
webauthn_platform_recommended: nil,
**extra
)
Expand Down Expand Up @@ -5886,6 +5892,8 @@ def multi_factor_auth_setup(
attempts:,
aaguid:,
unknown_transports:,
transports:,
transports_mismatch:,
webauthn_platform_recommended:,
**extra,
)
Expand Down Expand Up @@ -7753,9 +7761,12 @@ def webauthn_setup_mismatch_visited(
)
end

# @param [Boolean] platform_authenticator
# @param [Boolean] success
# @param [Hash, nil] errors
# @param [Boolean] platform_authenticator Whether submission is for setting up a platform
# authenticator. This aligns to what the user experienced in setting up the authenticator.
# However, if `transports_mismatch` is true, the authentication method is created as the
# opposite of this value.
# @param [Boolean] success Whether the submission was successful
# @param [Hash, nil] errors Errors resulting from form validation, or nil if successful.
# @param [Boolean] in_account_creation_flow Whether user is going through account creation flow
# Tracks whether or not Webauthn setup was successful
def webauthn_setup_submitted(
Expand All @@ -7767,10 +7778,10 @@ def webauthn_setup_submitted(
)
track_event(
:webauthn_setup_submitted,
platform_authenticator: platform_authenticator,
success: success,
platform_authenticator:,
success:,
errors:,
in_account_creation_flow:,
errors: errors,
**extra,
)
end
Expand Down
116 changes: 102 additions & 14 deletions spec/controllers/users/webauthn_setup_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@
end
end

describe 'when signed in and not account creation' do
context 'when signed in and not account creation' do
let(:user) { create(:user, :fully_registered, :with_authentication_app) }

before do
stub_analytics
stub_sign_in(user)
end

describe 'GET new' do
describe '#new' do
it 'tracks page visit' do
stub_sign_in
stub_analytics
Expand Down Expand Up @@ -77,7 +77,9 @@
end
end

describe 'patch confirm' do
describe '#confirm' do
subject(:response) { patch :confirm, params: params }

let(:params) do
{
attestation_object: attestation_object,
Expand All @@ -103,6 +105,16 @@
controller.user_session[:webauthn_challenge] = webauthn_challenge
end

it 'should flash a success message after successfully creating' do
response

expect(flash[:success]).to eq(t('notices.webauthn_configured'))
end

it 'redirects to next setup path' do
expect(response).to redirect_to(account_url)
end

it 'tracks the submission' do
Funnel::Registration::AddMfa.call(user.id, 'phone', @analytics, threatmetrix_attrs)

Expand Down Expand Up @@ -130,6 +142,8 @@
ed: false,
},
attempts: 1,
transports: ['usb'],
transports_mismatch: false,
)
expect(@analytics).to have_logged_event(
:webauthn_setup_submitted,
Expand All @@ -139,6 +153,74 @@
)
end

context 'with transports mismatch' do
let(:params) { super().merge(transports: 'internal') }

it 'handles as successful setup for platform authenticator' do
expect(controller).to receive(:handle_valid_verification_for_confirmation_context).with(
auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN_PLATFORM,
)

response
end

it 'does not flash success message' do
response

expect(flash[:success]).to be_nil
end

it 'redirects to mismatch confirmation' do
expect(response).to redirect_to(webauthn_setup_mismatch_url)
end

it 'sets session value for mismatched configuration id' do
response

expect(controller.user_session[:webauthn_mismatch_id])
.to eq(user.webauthn_configurations.last.id)
end
end

context 'with platform authenticator set up' do
let(:params) { super().merge(platform_authenticator: true, transports: 'internal') }

it 'should flash a success message after successfully creating' do
response

expect(flash[:success]).to eq(t('notices.webauthn_platform_configured'))
end

context 'with transports mismatch' do
let(:params) { super().merge(transports: 'usb') }

it 'handles as successful setup for cross-platform authenticator' do
expect(controller).to receive(:handle_valid_verification_for_confirmation_context).with(
auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN,
)

response
end

it 'does not flash success message' do
response

expect(flash[:success]).to be_nil
end

it 'redirects to mismatch confirmation' do
expect(response).to redirect_to(webauthn_setup_mismatch_url)
end

it 'sets session value for mismatched configuration id' do
response

expect(controller.user_session[:webauthn_mismatch_id])
.to eq(user.webauthn_configurations.last.id)
end
end
end

context 'with setup from sms recommendation' do
before do
controller.user_session[:webauthn_platform_recommended] = :authentication
Expand Down Expand Up @@ -283,6 +365,8 @@
multi_factor_auth_method: 'webauthn',
success: true,
attempts: 1,
transports: ['usb'],
transports_mismatch: false,
)
expect(@analytics).to have_logged_event(
:webauthn_setup_submitted,
Expand Down Expand Up @@ -344,6 +428,8 @@
multi_factor_auth_method: 'webauthn_platform',
success: true,
attempts: 1,
transports: ['internal', 'hybrid'],
transports_mismatch: false,
)
end

Expand Down Expand Up @@ -380,18 +466,18 @@

expect(@analytics).to have_logged_event(
'Multi-Factor Authentication Setup',
{
enabled_mfa_methods_count: 0,
errors: {
attestation_object: [I18n.t('errors.webauthn_platform_setup.general_error')],
},
error_details: { attestation_object: { invalid: true } },
in_account_creation_flow: false,
mfa_method_counts: {},
multi_factor_auth_method: 'webauthn_platform',
success: false,
attempts: 1,
enabled_mfa_methods_count: 0,
errors: {
attestation_object: [I18n.t('errors.webauthn_platform_setup.general_error')],
},
error_details: { attestation_object: { invalid: true } },
in_account_creation_flow: false,
mfa_method_counts: {},
multi_factor_auth_method: 'webauthn_platform',
success: false,
attempts: 1,
transports: ['internal', 'hybrid'],
transports_mismatch: false,
)
end
end
Expand Down Expand Up @@ -448,6 +534,8 @@
ed: false,
},
attempts: 2,
transports: ['usb'],
transports_mismatch: false,
)
expect(@analytics).to have_logged_event(
:webauthn_setup_submitted,
Expand Down
Loading