diff --git a/app/forms/webauthn_setup_form.rb b/app/forms/webauthn_setup_form.rb index 7148c980d33..297b6fa71c1 100644 --- a/app/forms/webauthn_setup_form.rb +++ b/app/forms/webauthn_setup_form.rb @@ -61,7 +61,7 @@ def generic_error_message private - attr_reader :success, :transports, :invalid_transports, :protocol + attr_reader :success, :transports, :aaguid, :invalid_transports, :protocol attr_accessor :user, :challenge, :attestation_object, :client_data_json, :name, :platform_authenticator, :authenticator_data_flags, :device_name @@ -110,6 +110,7 @@ def valid_attestation_response?(protocol) ) begin + @aaguid = attestation_response.authenticator_data.aaguid attestation_response.valid?(@challenge.pack('c*'), original_origin) rescue StandardError false @@ -141,6 +142,7 @@ def create_webauthn_configuration platform_authenticator: platform_authenticator, transports: transports.presence, authenticator_data_flags: authenticator_data_flags, + aaguid: aaguid, ) end @@ -172,6 +174,7 @@ def extra_analytics_attributes pii_like_keypaths: [[:mfa_method_counts, :phone]], authenticator_data_flags: authenticator_data_flags, unknown_transports: invalid_transports.presence, + aaguid: aaguid, }.compact end end diff --git a/app/forms/webauthn_verification_form.rb b/app/forms/webauthn_verification_form.rb index 9dec32f69fe..686770007ff 100644 --- a/app/forms/webauthn_verification_form.rb +++ b/app/forms/webauthn_verification_form.rb @@ -92,6 +92,7 @@ def valid_assertion_response? client_data_json.blank? || signature.blank? || challenge.blank? + WebAuthn::AuthenticatorAssertionResponse.new( authenticator_data: Base64.decode64(authenticator_data), client_data_json: Base64.decode64(client_data_json), @@ -165,6 +166,7 @@ def extra_analytics_attributes { webauthn_configuration_id: webauthn_configuration&.id, frontend_error: webauthn_error.presence, + webauthn_aaguid: webauthn_configuration&.aaguid, }.compact end end diff --git a/db/primary_migrate/20240828182041_add_aaguid_to_webauthn_configuration.rb b/db/primary_migrate/20240828182041_add_aaguid_to_webauthn_configuration.rb new file mode 100644 index 00000000000..bed1e8ca51c --- /dev/null +++ b/db/primary_migrate/20240828182041_add_aaguid_to_webauthn_configuration.rb @@ -0,0 +1,5 @@ +class AddAaguidToWebauthnConfiguration < ActiveRecord::Migration[7.1] + def change + add_column :webauthn_configurations, :aaguid, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 565747b4d73..385018287e3 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_08_22_122355) do +ActiveRecord::Schema[7.1].define(version: 2024_08_28_182041) do # These are extensions that must be enabled in order to support this database enable_extension "citext" enable_extension "pg_stat_statements" @@ -656,6 +656,7 @@ t.boolean "platform_authenticator" t.string "transports", array: true t.jsonb "authenticator_data_flags" + t.string "aaguid" t.index ["user_id"], name: "index_webauthn_configurations_on_user_id" end diff --git a/spec/forms/webauthn_setup_form_spec.rb b/spec/forms/webauthn_setup_form_spec.rb index 21aa0aa9c29..64fac942c8f 100644 --- a/spec/forms/webauthn_setup_form_spec.rb +++ b/spec/forms/webauthn_setup_form_spec.rb @@ -7,9 +7,10 @@ let(:user_session) { { webauthn_challenge: webauthn_challenge } } let(:device_name) { 'Chrome 119 on macOS' } let(:domain_name) { 'localhost:3000' } + let(:attestation) { attestation_object } let(:params) do { - attestation_object: attestation_object, + attestation_object: attestation, client_data_json: setup_client_data_json, name: 'mykey', platform_authenticator: false, @@ -26,42 +27,51 @@ describe '#submit' do context 'when the input is valid' do - it 'returns FormResponse with success: true and creates a webauthn configuration' do - extra_attributes = { - enabled_mfa_methods_count: 1, - mfa_method_counts: { webauthn: 1 }, - multi_factor_auth_method: 'webauthn', - authenticator_data_flags: { - up: true, - uv: false, - be: true, - bs: true, - at: false, - ed: true, - }, - pii_like_keypaths: [[:mfa_method_counts, :phone]], - } + context 'security key' do + it 'returns FormResponse with success: true and creates a webauthn configuration' do + extra_attributes = { + enabled_mfa_methods_count: 1, + mfa_method_counts: { webauthn: 1 }, + multi_factor_auth_method: 'webauthn', + authenticator_data_flags: { + up: true, + uv: false, + be: true, + bs: true, + at: false, + ed: true, + }, + pii_like_keypaths: [[:mfa_method_counts, :phone]], + } - expect(subject.submit(params).to_h).to eq( - success: true, - errors: {}, - **extra_attributes, - ) + expect(subject.submit(params).to_h).to eq( + success: true, + errors: {}, + **extra_attributes, + ) - user.reload + user.reload - expect(user.webauthn_configurations.roaming_authenticators.count).to eq(1) - expect(user.webauthn_configurations.roaming_authenticators.first.transports).to eq(['usb']) - end + expect(user.webauthn_configurations.roaming_authenticators.count).to eq(1) + expect(user.webauthn_configurations.roaming_authenticators.first.transports). + to eq(['usb']) + end - it 'sends a recovery information changed event' do - expect(PushNotification::HttpPush).to receive(:deliver). - with(PushNotification::RecoveryInformationChangedEvent.new(user: user)) + it 'sends a recovery information changed event' do + expect(PushNotification::HttpPush).to receive(:deliver). + with(PushNotification::RecoveryInformationChangedEvent.new(user: user)) - subject.submit(params) + subject.submit(params) + end + + it 'does not contains uuid' do + result = subject.submit(params) + expect(result.extra[:aaguid]).to eq nil + end end context 'with platform authenticator' do + let(:attestation) { platform_auth_attestation_object } let(:params) do super().merge(platform_authenticator: true, transports: 'internal,hybrid') end @@ -114,6 +124,11 @@ expect(result.to_h[:authenticator_data_flags]).to be_nil end end + + it 'contains uuid' do + result = subject.submit(params) + expect(result.extra[:aaguid]).to eq aaguid + end end context 'with invalid transports' do diff --git a/spec/forms/webauthn_verification_form_spec.rb b/spec/forms/webauthn_verification_form_spec.rb index d01339b6544..1f298720bc6 100644 --- a/spec/forms/webauthn_verification_form_spec.rb +++ b/spec/forms/webauthn_verification_form_spec.rb @@ -10,6 +10,7 @@ let(:screen_lock_error) { nil } let(:platform_authenticator) { false } let(:client_data_json) { verification_client_data_json } + let(:webauthn_aaguid) { nil } let!(:webauthn_configuration) do return if !user create( @@ -18,6 +19,7 @@ credential_id: credential_id, credential_public_key: credential_public_key, platform_authenticator: platform_authenticator, + aaguid: webauthn_aaguid, ) end @@ -45,20 +47,24 @@ subject(:result) { form.submit } context 'when the input is valid' do - it 'returns successful result' do - expect(result.to_h).to eq( - success: true, - webauthn_configuration_id: webauthn_configuration.id, - ) + context 'security key' do + it 'returns successful result' do + expect(result.to_h).to eq( + success: true, + webauthn_configuration_id: webauthn_configuration.id, + ) + end end context 'for platform authenticator' do let(:platform_authenticator) { true } + let(:webauthn_aaguid) { aaguid } it 'returns successful result' do expect(result.to_h).to eq( success: true, webauthn_configuration_id: webauthn_configuration.id, + webauthn_aaguid: aaguid, ) end end diff --git a/spec/support/features/webauthn_helper.rb b/spec/support/features/webauthn_helper.rb index 45278d60a9f..19735b95f2c 100644 --- a/spec/support/features/webauthn_helper.rb +++ b/spec/support/features/webauthn_helper.rb @@ -172,6 +172,15 @@ def attestation_object HEREDOC end + def platform_auth_attestation_object + <<~HEREDOC.chomp + o2NmbXRkbm9uZWdhdHRTdG10oGhhdXRoRGF0YVikSZYN5YgOjGh0NBcPZHZgW4/krrmihjLHmVzz + uoMdl2NFAAAAAK3OAAI1vMYKZIsLJfHwVQMAIOa31Ugh6EPoj4z6b+ibq6rVF1CZ9ygzSNvMrFmY + aPLtpQECAyYgASFYIO6a1uIfDkbqg/pm7bHZG0oRGyCEuWZrCWd2v/2IqXCaIlggKQEHbAiyBZxS + 1HSBwwdjNCE4prYoHdzJWQILvDrIySo= + HEREDOC + end + def setup_client_data_json <<~HEREDOC.chomp eyJjaGFsbGVuZ2UiOiJncjEycndSVVVIWnFvNkZFSV9ZbEFnIiwibmV3X2tleXNfbWF5X2JlX2 @@ -198,6 +207,10 @@ def authenticator_data 'SZYN5YgOjGh0NBcPZHZgW4/krrmihjLHmVzzuoMdl2MBAAAAcQ==' end + def aaguid + 'adce0002-35bc-c60a-648b-0b25f1f05503' + end + def signature <<~HEREDOC.chomp MEYCIQC7VHQpZasv8URBC/VYKWcuv4MrmV82UfsESKTGgV3r+QIhAO8iAduYC7XDHJjpKkrSKb