From c8a75cb4101abde7449a90041360adb584aca114 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 14 Aug 2024 09:36:51 -0400 Subject: [PATCH 1/3] Use faster default driver for feature tests not requiring JavaScript changelog: Internal, Automated Testing, Use faster default driver for feature tests not requiring JavaScript --- spec/features/openid_connect/vtr_spec.rb | 17 +++++++------ .../profile_recovery_for_gpo_verified_spec.rb | 24 ++++--------------- spec/features/users/sign_up_spec.rb | 12 +++++----- spec/features/webauthn/sign_in_spec.rb | 6 ++--- 4 files changed, 22 insertions(+), 37 deletions(-) diff --git a/spec/features/openid_connect/vtr_spec.rb b/spec/features/openid_connect/vtr_spec.rb index 41420286001..b0f9cc77d88 100644 --- a/spec/features/openid_connect/vtr_spec.rb +++ b/spec/features/openid_connect/vtr_spec.rb @@ -10,7 +10,7 @@ and_return(true) end - scenario 'sign in with VTR request for authentication', :js do + scenario 'sign in with VTR request for authentication' do user = create(:user, :fully_registered) visit_idp_from_oidc_sp_with_vtr(vtr: ['C1']) @@ -25,10 +25,10 @@ click_agree_and_continue - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') end - scenario 'sign in with VTR request for AAL2 disables remember device', :js do + scenario 'sign in with VTR request for AAL2 disables remember device' do user = create(:user, :fully_registered) # Sign in and remember device @@ -48,10 +48,10 @@ click_agree_and_continue - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') end - scenario 'sign in with VTR for phishing-resistance requires phishing-resistanc auth', :js do + scenario 'sign in with VTR for phishing-resistance requires phishing-resistanc auth' do mock_webauthn_setup_challenge user = create(:user, :fully_registered) @@ -69,7 +69,7 @@ click_agree_and_continue - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') end scenario 'sign in with VTR request for HSDP12 auth requires PIV/CAC setup' do @@ -94,12 +94,11 @@ follow_piv_cac_redirect click_agree_and_continue - click_submit_default - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') end - scenario 'sign in with VTR request for idv requires idv', :js do + scenario 'sign in with VTR request for idv requires idv' do user = create(:user, :fully_registered) visit_idp_from_oidc_sp_with_vtr(vtr: ['P1']) diff --git a/spec/features/users/profile_recovery_for_gpo_verified_spec.rb b/spec/features/users/profile_recovery_for_gpo_verified_spec.rb index a228c3fc303..ab2b5f80fd1 100644 --- a/spec/features/users/profile_recovery_for_gpo_verified_spec.rb +++ b/spec/features/users/profile_recovery_for_gpo_verified_spec.rb @@ -4,30 +4,16 @@ allowed_extra_analytics: [:*] do include IdvStepHelper - let(:email) { 'cool_beagle@example.org' } - let(:password) { '!1a Z@6s' * 16 } # default password from user factory let(:new_password) { 'some really awesome new password' } - let(:user) { create(:user, :fully_registered, email: email, password: password) } - - before do - allow(FeatureManagement).to receive(:reveal_gpo_code?).and_return(true) - end - - scenario 'lets them reactivate their profile with their personal key', email: true, js: true do - complete_idv_steps_with_gpo_before_confirmation_step(user) - click_on t('doc_auth.buttons.continue') - - gpo_code = page.get_rack_session_key('last_gpo_confirmation_code') - page.go_back # get_rack_session_key navigates away. - - click_on t('links.sign_out') - - fill_in_credentials_and_submit(email, password) + scenario 'lets them reactivate their profile with their personal key', email: true do + user = create(:user, :fully_registered, :with_pending_gpo_profile) + visit new_user_session_path + fill_in_credentials_and_submit(user.email, user.password) fill_in I18n.t('components.one_time_code_input.label'), with: last_phone_otp click_submit_default - fill_in 'gpo_verify_form_otp', with: gpo_code + fill_in 'gpo_verify_form_otp', with: 'ABCDE12345' click_on t('idv.gpo.form.submit') personal_key = scrape_personal_key diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index c9457d18db0..cd81363a901 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -241,14 +241,14 @@ def clipboard_text expect(page).to have_current_path account_path end + end - it 'allows a user to sign up with backup codes and add methods without reauthentication' do - sign_in_user - select_2fa_option('backup_code') + it 'allows a user to sign up with backup codes and add methods without reauthentication' do + sign_in_user + select_2fa_option('backup_code') - visit phone_setup_path - expect(page).to have_current_path phone_setup_path - end + visit phone_setup_path + expect(page).to have_current_path phone_setup_path end context 'user accesses password screen with already confirmed token', email: true do diff --git a/spec/features/webauthn/sign_in_spec.rb b/spec/features/webauthn/sign_in_spec.rb index 117bd7d8407..ce98608adc9 100644 --- a/spec/features/webauthn/sign_in_spec.rb +++ b/spec/features/webauthn/sign_in_spec.rb @@ -44,7 +44,7 @@ expect(page).to have_current_path(login_two_factor_webauthn_path) end - it 'does not show error after successful challenge/secret reattempt', :js do + it 'does not show error after successful challenge/secret reattempt' do mock_webauthn_verification_challenge sign_in_user(user) @@ -57,7 +57,7 @@ expect(page).to_not have_content(general_error) end - it 'maintains correct platform attachment content if cancelled', :js do + it 'maintains correct platform attachment content if cancelled' do mock_webauthn_verification_challenge sign_in_user(user) @@ -71,7 +71,7 @@ create(:user, :with_webauthn_platform, with: { credential_id:, credential_public_key: }) end - it 'maintains correct platform attachment content if cancelled', :js do + it 'maintains correct platform attachment content if cancelled' do mock_webauthn_verification_challenge sign_in_user(user) From e1ef687f4008dcc97f09910825c5fcece9d2205d Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 14 Aug 2024 10:01:41 -0400 Subject: [PATCH 2/3] Enable JavaScript for at least one WebAuthn spec See rationale in #8761 --- spec/features/webauthn/sign_in_spec.rb | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/spec/features/webauthn/sign_in_spec.rb b/spec/features/webauthn/sign_in_spec.rb index ce98608adc9..b87decc8fc3 100644 --- a/spec/features/webauthn/sign_in_spec.rb +++ b/spec/features/webauthn/sign_in_spec.rb @@ -15,13 +15,18 @@ ) end - it 'allows the user to sign in if webauthn is successful' do - mock_webauthn_verification_challenge + context 'with javascript enabled', :js do + # While JavaScript tests are slower to run, these tests provide increased confidence in the + # real-world behavior of WebAuthn browser interaction for the critical pathways. - sign_in_user(user) - mock_successful_webauthn_authentication { click_webauthn_authenticate_button } + it 'allows the user to sign in if webauthn is successful' do + mock_webauthn_verification_challenge - expect(page).to have_current_path(account_path) + sign_in_user(user) + mock_successful_webauthn_authentication { click_webauthn_authenticate_button } + + expect(page).to have_current_path(account_path) + end end it 'does not allow the user to sign in if the challenge/secret is incorrect' do From 38ff8112460ab4361c5d8607da9b2bb49902e116 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 14 Aug 2024 10:02:40 -0400 Subject: [PATCH 3/3] Remove duplicate spec Same test case already exists in no-JavaScript context --- spec/features/users/sign_up_spec.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index cd81363a901..d54721b7631 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -243,14 +243,6 @@ def clipboard_text end end - it 'allows a user to sign up with backup codes and add methods without reauthentication' do - sign_in_user - select_2fa_option('backup_code') - - visit phone_setup_path - expect(page).to have_current_path phone_setup_path - end - context 'user accesses password screen with already confirmed token', email: true do it 'returns them to the home page' do create(:user, :fully_registered, confirmation_token: 'foo')