From c2d9ebd5e6f251c861bb82b0c98976311f627969 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 22 Feb 2023 10:09:01 -0500 Subject: [PATCH 1/8] changelog: Bug Fixes, Authentication, don't allow users to submit without nickname for security key --- app/controllers/users/webauthn_setup_controller.rb | 7 ++----- app/javascript/packs/webauthn-setup.ts | 5 +++++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/controllers/users/webauthn_setup_controller.rb b/app/controllers/users/webauthn_setup_controller.rb index b2d1346f14f..56cb7019190 100644 --- a/app/controllers/users/webauthn_setup_controller.rb +++ b/app/controllers/users/webauthn_setup_controller.rb @@ -50,7 +50,7 @@ def confirm ) properties = result.to_h.merge(analytics_properties) analytics.multi_factor_auth_setup(**properties) - + if @platform_authenticator irs_attempts_api_tracker.mfa_enroll_webauthn_platform(success: result.success?) else @@ -183,17 +183,14 @@ def process_invalid_webauthn(form) else flash.now[:error] = t('errors.webauthn_setup.unique_name') end - - render :new else if form.platform_authenticator? flash[:error] = t('errors.webauthn_platform_setup.general_error') else flash[:error] = t('errors.webauthn_setup.general_error') end - - redirect_to account_two_factor_authentication_path end + render :new end def mark_user_as_fully_authenticated diff --git a/app/javascript/packs/webauthn-setup.ts b/app/javascript/packs/webauthn-setup.ts index f0b68e2eda2..7a3e2bbd104 100644 --- a/app/javascript/packs/webauthn-setup.ts +++ b/app/javascript/packs/webauthn-setup.ts @@ -21,6 +21,11 @@ function webauthn() { } const continueButton = document.getElementById('continue-button')!; continueButton.addEventListener('click', () => { + // check if form is valid first + const form = document.getElementById('webauthn_form') as HTMLFormElement; + if(!form.checkValidity()){ + return; + } document.getElementById('spinner')!.classList.remove('display-none'); document.getElementById('continue-button')!.className = 'display-none'; From bc1c82cc1fb28392c567a22265e6d28ae8019740 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 22 Feb 2023 10:40:27 -0500 Subject: [PATCH 2/8] rubocop --- app/controllers/users/webauthn_setup_controller.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/controllers/users/webauthn_setup_controller.rb b/app/controllers/users/webauthn_setup_controller.rb index 56cb7019190..6974c1bd180 100644 --- a/app/controllers/users/webauthn_setup_controller.rb +++ b/app/controllers/users/webauthn_setup_controller.rb @@ -50,7 +50,7 @@ def confirm ) properties = result.to_h.merge(analytics_properties) analytics.multi_factor_auth_setup(**properties) - + if @platform_authenticator irs_attempts_api_tracker.mfa_enroll_webauthn_platform(success: result.success?) else @@ -183,12 +183,10 @@ def process_invalid_webauthn(form) else flash.now[:error] = t('errors.webauthn_setup.unique_name') end - else - if form.platform_authenticator? - flash[:error] = t('errors.webauthn_platform_setup.general_error') + elsif form.platform_authenticator? + flash[:error] = t('errors.webauthn_platform_setup.general_error') else flash[:error] = t('errors.webauthn_setup.general_error') - end end render :new end From 2855d7494f91a9711da82952a4de8e11c3f9a085 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 22 Feb 2023 11:00:15 -0500 Subject: [PATCH 3/8] fix spec for webauthn --- spec/features/webauthn/management_spec.rb | 4 ++-- spec/features/webauthn/sign_up_spec.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/features/webauthn/management_spec.rb b/spec/features/webauthn/management_spec.rb index e36e5877d9d..e8c25746930 100644 --- a/spec/features/webauthn/management_spec.rb +++ b/spec/features/webauthn/management_spec.rb @@ -21,7 +21,7 @@ def expect_webauthn_setup_success def expect_webauthn_setup_error expect(page).to have_content t('errors.webauthn_setup.general_error') - expect(current_path).to eq account_two_factor_authentication_path + expect(current_path).to eq webauthn_setup_path end def visit_webauthn_platform_setup @@ -41,7 +41,7 @@ def expect_webauthn_platform_setup_success def expect_webauthn_platform_setup_error expect(page).to have_content t('errors.webauthn_platform_setup.general_error') - expect(current_path).to eq account_two_factor_authentication_path + expect(current_path).to eq webauthn_setup_path end context 'with webauthn roaming associations' do diff --git a/spec/features/webauthn/sign_up_spec.rb b/spec/features/webauthn/sign_up_spec.rb index 74f09e95ea0..1770458506d 100644 --- a/spec/features/webauthn/sign_up_spec.rb +++ b/spec/features/webauthn/sign_up_spec.rb @@ -18,7 +18,7 @@ def expect_webauthn_setup_success def expect_webauthn_setup_error expect(page).to have_content t('errors.webauthn_setup.general_error') - expect(page).to have_current_path(authentication_methods_setup_path) + expect(page).to have_current_path(webauthn_setup_path) end it_behaves_like 'webauthn setup' From 049646cf15488c69ae5df5dc405c5565aa114e08 Mon Sep 17 00:00:00 2001 From: Malick Diarra Date: Wed, 22 Feb 2023 14:13:12 -0500 Subject: [PATCH 4/8] update javascript to be on form submit --- app/javascript/packs/webauthn-setup.ts | 22 ++------------------- app/views/users/webauthn_setup/new.html.erb | 11 +++++------ 2 files changed, 7 insertions(+), 26 deletions(-) diff --git a/app/javascript/packs/webauthn-setup.ts b/app/javascript/packs/webauthn-setup.ts index 7a3e2bbd104..88e048d74b9 100644 --- a/app/javascript/packs/webauthn-setup.ts +++ b/app/javascript/packs/webauthn-setup.ts @@ -19,13 +19,8 @@ function webauthn() { if (!isWebAuthnEnabled()) { reloadWithError('NotSupportedError'); } - const continueButton = document.getElementById('continue-button')!; - continueButton.addEventListener('click', () => { - // check if form is valid first - const form = document.getElementById('webauthn_form') as HTMLFormElement; - if(!form.checkValidity()){ - return; - } + const form = document.getElementById('webauthn_form') as HTMLFormElement; + form.addEventListener('submit', () => { document.getElementById('spinner')!.classList.remove('display-none'); document.getElementById('continue-button')!.className = 'display-none'; @@ -52,19 +47,6 @@ function webauthn() { }) .catch((err) => reloadWithError(err.name, { force: true })); }); - const input = document.getElementById('nickname') as HTMLInputElement; - input.addEventListener('keypress', function (event) { - if (event.keyCode === 13) { - // prevent form submit - event.preventDefault(); - } - }); - input.addEventListener('keyup', function (event) { - event.preventDefault(); - if (event.keyCode === 13 && input.value) { - continueButton.click(); - } - }); } if (process.env.NODE_ENV !== 'test') { diff --git a/app/views/users/webauthn_setup/new.html.erb b/app/views/users/webauthn_setup/new.html.erb index e5b741968b1..ed5456c516c 100644 --- a/app/views/users/webauthn_setup/new.html.erb +++ b/app/views/users/webauthn_setup/new.html.erb @@ -52,13 +52,12 @@ checked: @presenter.remember_device_box_checked?, }, ) %> - <%= submit_tag t('forms.buttons.submit.default'), id: 'submit-button', class: 'display-none' %> + <%= submit_tag( + @presenter.button_text, + id: 'continue-button', + class: 'display-block usa-button usa-button--big usa-button--wide margin-y-5' + ) %> <% end %> -<%= button_tag( - @presenter.button_text, - class: 'display-block usa-button usa-button--big usa-button--wide margin-y-5', - id: 'continue-button', - ) %>