diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/users/sessions_controller.rb index 9079c17975a..c8be461e9f5 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/users/sessions_controller.rb @@ -40,11 +40,15 @@ def create end def destroy - analytics.logout_initiated(sp_initiated: false, oidc: false) - irs_attempts_api_tracker.logout_initiated( - success: true, - ) - super + if request.method == 'GET' && IdentityConfig.store.disable_logout_get_request + redirect_to root_path + else + analytics.logout_initiated(sp_initiated: false, oidc: false) + irs_attempts_api_tracker.logout_initiated( + success: true, + ) + super + end end private diff --git a/app/views/accounts/_nav_auth.html.erb b/app/views/accounts/_nav_auth.html.erb index 07b123a7bf5..ba6e5f67fed 100644 --- a/app/views/accounts/_nav_auth.html.erb +++ b/app/views/accounts/_nav_auth.html.erb @@ -16,7 +16,9 @@ - <%= link_to t('links.sign_out'), destroy_user_session_path %> + + <%= button_to t('links.sign_out'), logout_path, method: :delete, class: 'usa-button usa-button--unstyled' %> + <% if enable_mobile_nav %> <% end %> diff --git a/app/views/session_timeout/_warning.html.erb b/app/views/session_timeout/_warning.html.erb index 96ab50b783b..abf656dd841 100644 --- a/app/views/session_timeout/_warning.html.erb +++ b/app/views/session_timeout/_warning.html.erb @@ -49,7 +49,10 @@ t('continue', scope: modal_presenter.translation_scope), ) %> <%= render ButtonComponent.new( - action: ->(**tag_options, &block) { link_to(destroy_user_session_path, **tag_options, &block) }, + action: ->(**tag_options, &block) do + button_to(logout_path, **tag_options, &block) + end, + method: :delete, big: true, full_width: true, outline: true, diff --git a/app/views/shared/_cancel.html.erb b/app/views/shared/_cancel.html.erb index f56594f9868..93673e50d00 100644 --- a/app/views/shared/_cancel.html.erb +++ b/app/views/shared/_cancel.html.erb @@ -2,6 +2,10 @@ <% if user_signing_up? %> <%= link_to cancel_link_text, sign_up_cancel_path, method: :get, class: 'usa-button usa-button--unstyled' %> <% else %> - <%= link_to cancel_link_text, link || return_to_sp_cancel_path %> + <% if defined?(link_method) %> + <%= button_to cancel_link_text, link, method: link_method, class: 'usa-button usa-button--unstyled' %> + <% else %> + <%= link_to cancel_link_text, link %> + <% end %> <% end %> <% end %> diff --git a/app/views/sign_up/passwords/new.html.erb b/app/views/sign_up/passwords/new.html.erb index 285d5ef4231..d6cbb3bee95 100644 --- a/app/views/sign_up/passwords/new.html.erb +++ b/app/views/sign_up/passwords/new.html.erb @@ -25,6 +25,6 @@ <%= render 'shared/password_accordion' %> -<%= render 'shared/cancel', link: destroy_user_session_path %> +<%= render 'shared/cancel' %> <%= javascript_packs_tag_once 'pw-strength' %> diff --git a/app/views/users/piv_cac_setup_from_sign_in/success.html.erb b/app/views/users/piv_cac_setup_from_sign_in/success.html.erb index ea8b3bc741b..1d91863a67b 100644 --- a/app/views/users/piv_cac_setup_from_sign_in/success.html.erb +++ b/app/views/users/piv_cac_setup_from_sign_in/success.html.erb @@ -7,4 +7,4 @@ <%= button_to t('forms.buttons.continue'), login_add_piv_cac_success_path, class: 'usa-button usa-button--big usa-button--wide', method: :post %> -<%= render 'shared/cancel', link: destroy_user_session_path %> +<%= render 'shared/cancel', link: logout_path, link_method: :delete %> diff --git a/app/views/users/two_factor_authentication_setup/index.html.erb b/app/views/users/two_factor_authentication_setup/index.html.erb index 74c49fa5625..0070c58eb7b 100644 --- a/app/views/users/two_factor_authentication_setup/index.html.erb +++ b/app/views/users/two_factor_authentication_setup/index.html.erb @@ -36,4 +36,4 @@ <%= f.submit t('forms.buttons.continue'), class: 'margin-bottom-1' %> <% end %> -<%= render 'shared/cancel', link: destroy_user_session_path %> +<%= render 'shared/cancel', link: logout_path, link_method: :delete %> diff --git a/config/application.yml.default b/config/application.yml.default index e23bbd9a247..f42d2a75039 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -78,6 +78,7 @@ deleted_user_accounts_report_configs: '[]' development_mailer_deliver_method: letter_opener disable_csp_unsafe_inline: true disable_email_sending: true +disable_logout_get_request: true disallow_all_web_crawlers: true disposable_email_services: '[]' doc_auth_attempt_window_in_minutes: 360 @@ -438,6 +439,7 @@ production: database_worker_jobs_host: '' database_worker_jobs_password: '' disable_email_sending: false + disable_logout_get_request: false disallow_all_web_crawlers: false doc_auth_vendor: 'acuant' doc_auth_vendor_randomize: false diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 7932d8a3425..71f7735079c 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -178,6 +178,7 @@ def self.build_store(config_map) config.add(:development_mailer_deliver_method, type: :symbol, enum: [:file, :letter_opener]) config.add(:disable_csp_unsafe_inline, type: :boolean) config.add(:disable_email_sending, type: :boolean) + config.add(:disable_logout_get_request, type: :boolean) config.add(:disallow_all_web_crawlers, type: :boolean) config.add(:disposable_email_services, type: :json) config.add(:doc_auth_attempt_window_in_minutes, type: :integer) diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index 959b30bc98b..f10c9aff527 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -5,25 +5,11 @@ let(:mock_valid_site) { 'http://example.com' } describe 'GET /logout' do - it 'tracks a logout event' do - stub_analytics - stub_attempts_tracker - expect(@analytics).to receive(:track_event).with( - 'Logout Initiated', - hash_including( - sp_initiated: false, - oidc: false, - ), - ) - + it 'does not log user out and redirects to root' do sign_in_as_user - - expect(@irs_attempts_api_tracker).to receive(:logout_initiated).with( - success: true, - ) - get :destroy - expect(controller.current_user).to be nil + expect(controller.current_user).to_not be nil + expect(response).to redirect_to root_url end end diff --git a/spec/features/idv/clearing_and_restarting_spec.rb b/spec/features/idv/clearing_and_restarting_spec.rb index 6a7e2a36f7e..7b187af2e95 100644 --- a/spec/features/idv/clearing_and_restarting_spec.rb +++ b/spec/features/idv/clearing_and_restarting_spec.rb @@ -22,7 +22,7 @@ context 'after signing out' do before do visit account_path - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click start_idv_from_sp sign_in_live_with_2fa(user) end diff --git a/spec/features/idv/steps/gpo_step_spec.rb b/spec/features/idv/steps/gpo_step_spec.rb index b6a34f3416a..ab9437601aa 100644 --- a/spec/features/idv/steps/gpo_step_spec.rb +++ b/spec/features/idv/steps/gpo_step_spec.rb @@ -126,7 +126,7 @@ def complete_idv_and_sign_out click_continue visit root_path click_on t('idv.gpo.return_to_profile') - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click end def complete_idv_and_return_to_gpo_step diff --git a/spec/features/multiple_emails/sign_in_spec.rb b/spec/features/multiple_emails/sign_in_spec.rb index cbd291241ac..e047eb50bbb 100644 --- a/spec/features/multiple_emails/sign_in_spec.rb +++ b/spec/features/multiple_emails/sign_in_spec.rb @@ -13,7 +13,7 @@ expect(page).to have_current_path(account_path) - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click signin(email2, user.password) fill_in_code_with_last_phone_otp diff --git a/spec/features/openid_connect/openid_connect_spec.rb b/spec/features/openid_connect/openid_connect_spec.rb index 581edfa1d8e..f6f6048aec3 100644 --- a/spec/features/openid_connect/openid_connect_spec.rb +++ b/spec/features/openid_connect/openid_connect_spec.rb @@ -797,7 +797,9 @@ uncheck(t('forms.messages.remember_device')) fill_in_code_with_last_phone_otp click_submit_default - visit destroy_user_session_url + + visit account_path + click_button t('links.sign_out') visit_idp_from_ial1_oidc_sp(prompt: 'select_account') fill_in_credentials_and_submit(user.email, user.password) diff --git a/spec/features/remember_device/phone_spec.rb b/spec/features/remember_device/phone_spec.rb index 89a133b9db4..5b16b37a8af 100644 --- a/spec/features/remember_device/phone_spec.rb +++ b/spec/features/remember_device/phone_spec.rb @@ -16,7 +16,7 @@ def remember_device_and_sign_out_user check t('forms.messages.remember_device') fill_in_code_with_last_phone_otp click_submit_default - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click user end @@ -36,7 +36,7 @@ def remember_device_and_sign_out_user click_submit_default skip_second_mfa_prompt - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click user end diff --git a/spec/features/remember_device/revocation_spec.rb b/spec/features/remember_device/revocation_spec.rb index e5396827904..1c83a366484 100644 --- a/spec/features/remember_device/revocation_spec.rb +++ b/spec/features/remember_device/revocation_spec.rb @@ -17,7 +17,7 @@ find_sidenav_forget_browsers_link.click click_on(t('forms.buttons.confirm')) - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click expect_mfa_to_be_required_for_user(user) end @@ -34,7 +34,7 @@ find_sidenav_forget_browsers_link.click click_on(t('forms.buttons.confirm')) - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click expect_mfa_to_be_required_for_user(user) end @@ -51,7 +51,7 @@ def sign_in_with_remember_device_and_sign_out check t('forms.messages.remember_device') fill_in_code_with_last_phone_otp click_submit_default - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click end def expect_mfa_to_be_required_for_user(user) diff --git a/spec/features/remember_device/session_expiration_spec.rb b/spec/features/remember_device/session_expiration_spec.rb index d6d12484038..1fa3c1b6551 100644 --- a/spec/features/remember_device/session_expiration_spec.rb +++ b/spec/features/remember_device/session_expiration_spec.rb @@ -15,7 +15,7 @@ check t('forms.messages.remember_device') fill_in_code_with_last_phone_otp click_submit_default - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click IdentityLinker.new( user, build(:service_provider, issuer: OidcAuthHelper::OIDC_IAL1_ISSUER) diff --git a/spec/features/remember_device/sp_expiration_spec.rb b/spec/features/remember_device/sp_expiration_spec.rb index d7307451a8a..8395e480c3e 100644 --- a/spec/features/remember_device/sp_expiration_spec.rb +++ b/spec/features/remember_device/sp_expiration_spec.rb @@ -102,7 +102,7 @@ def visit_sp(protocol, aal) click_submit_default skip_second_mfa_prompt - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click user_record end diff --git a/spec/features/remember_device/totp_spec.rb b/spec/features/remember_device/totp_spec.rb index 9679edcc9bb..ddbce2befad 100644 --- a/spec/features/remember_device/totp_spec.rb +++ b/spec/features/remember_device/totp_spec.rb @@ -13,7 +13,7 @@ def remember_device_and_sign_out_user fill_in :code, with: generate_totp_code(user.auth_app_configurations.first.otp_secret_key) check t('forms.messages.remember_device') click_submit_default - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click user end @@ -32,7 +32,7 @@ def remember_device_and_sign_out_user click_submit_default skip_second_mfa_prompt - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click user end @@ -52,7 +52,7 @@ def remember_device_and_sign_out_user check t('forms.messages.remember_device') click_submit_default expect(page).to have_current_path(account_two_factor_authentication_path) - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click user end diff --git a/spec/features/remember_device/user_opted_preference_spec.rb b/spec/features/remember_device/user_opted_preference_spec.rb index 99f5218b8eb..177bace973c 100644 --- a/spec/features/remember_device/user_opted_preference_spec.rb +++ b/spec/features/remember_device/user_opted_preference_spec.rb @@ -17,7 +17,7 @@ click_button 'Submit' skip_second_mfa_prompt - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click sign_in_user(user) end @@ -44,7 +44,7 @@ click_continue skip_second_mfa_prompt - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click sign_in_user(user) end @@ -72,7 +72,7 @@ click_submit_default skip_second_mfa_prompt - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click sign_in_user(user) end @@ -92,7 +92,7 @@ fill_in :code, with: generate_totp_code(user.auth_app_configurations.first.otp_secret_key) uncheck t('forms.messages.remember_device') click_submit_default - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click sign_in_user(user) end @@ -119,7 +119,7 @@ sign_in_user(user) uncheck(:remember_device) mock_successful_webauthn_authentication { click_webauthn_authenticate_button } - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click sign_in_user(user) end @@ -138,7 +138,7 @@ uncheck t('forms.messages.remember_device') fill_in_code_with_last_phone_otp click_submit_default - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click sign_in_user(user) end diff --git a/spec/features/remember_device/webauthn_spec.rb b/spec/features/remember_device/webauthn_spec.rb index dbe6a964564..7dd1aef60df 100644 --- a/spec/features/remember_device/webauthn_spec.rb +++ b/spec/features/remember_device/webauthn_spec.rb @@ -26,7 +26,7 @@ def remember_device_and_sign_out_user sign_in_user(user) check t('forms.messages.remember_device') mock_successful_webauthn_authentication { click_webauthn_authenticate_button } - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click user end @@ -46,7 +46,7 @@ def remember_device_and_sign_out_user mock_press_button_on_hardware_key_on_setup skip_second_mfa_prompt - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click user end @@ -64,7 +64,7 @@ def remember_device_and_sign_out_user check t('forms.messages.remember_device') mock_press_button_on_hardware_key_on_setup expect(page).to have_current_path(account_two_factor_authentication_path) - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click user end @@ -89,7 +89,7 @@ def remember_device_and_sign_out_user sign_in_user(user) check t('forms.messages.remember_device') mock_successful_webauthn_authentication { click_webauthn_authenticate_button } - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click user end @@ -140,7 +140,7 @@ def remember_device_and_sign_out_user fill_in_code_with_last_phone_otp click_submit_default - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click user end @@ -157,7 +157,7 @@ def remember_device_and_sign_out_user check t('forms.messages.remember_device') mock_press_button_on_hardware_key_on_setup expect(page).to have_current_path(account_two_factor_authentication_path) - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click user end diff --git a/spec/features/saml/ial2_sso_spec.rb b/spec/features/saml/ial2_sso_spec.rb index cf7d1192cf0..137b2af20bd 100644 --- a/spec/features/saml/ial2_sso_spec.rb +++ b/spec/features/saml/ial2_sso_spec.rb @@ -47,7 +47,7 @@ def update_mailing_address end def sign_out_user - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click end context 'First time registration' do diff --git a/spec/features/saml/saml_logout_spec.rb b/spec/features/saml/saml_logout_spec.rb index ec897261769..ff655ff321e 100644 --- a/spec/features/saml/saml_logout_spec.rb +++ b/spec/features/saml/saml_logout_spec.rb @@ -18,7 +18,7 @@ # Sign out of the IDP visit account_path - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click expect(current_path).to eq root_path # SAML logout request diff --git a/spec/features/two_factor_authentication/backup_code_sign_up_spec.rb b/spec/features/two_factor_authentication/backup_code_sign_up_spec.rb index 0071e8826f6..76b117b2cef 100644 --- a/spec/features/two_factor_authentication/backup_code_sign_up_spec.rb +++ b/spec/features/two_factor_authentication/backup_code_sign_up_spec.rb @@ -88,6 +88,6 @@ end def sign_out_user - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click end end diff --git a/spec/features/users/sign_in_spec.rb b/spec/features/users/sign_in_spec.rb index 46551e2266a..a44a596e527 100644 --- a/spec/features/users/sign_in_spec.rb +++ b/spec/features/users/sign_in_spec.rb @@ -336,7 +336,7 @@ end scenario 'user can continue browsing with refreshed CSRF token' do - token = find('[name=authenticity_token]', visible: false).value + token = first('[name=authenticity_token]', visible: false).value click_button t('notices.timeout_warning.signed_in.continue') expect(page).not_to have_css('.usa-js-modal--active') expect(page).to have_css( @@ -347,7 +347,7 @@ end scenario 'user has option to sign out' do - click_link(t('notices.timeout_warning.signed_in.sign_out')) + click_button(t('notices.timeout_warning.signed_in.sign_out')) expect(page).to have_content t('devise.sessions.signed_out') expect(current_path).to eq new_user_session_path @@ -394,7 +394,7 @@ it 'fails to sign in the user, with CSRF error' do user = sign_in_and_2fa_user - click_link(t('links.sign_out'), match: :first) + click_button(t('links.sign_out'), match: :first) travel(Devise.timeout_in + 1.minute) do expect(page).to_not have_content(t('forms.buttons.continue')) diff --git a/spec/features/users/sign_out_spec.rb b/spec/features/users/sign_out_spec.rb index 1798dffde9c..b23c74ac240 100644 --- a/spec/features/users/sign_out_spec.rb +++ b/spec/features/users/sign_out_spec.rb @@ -3,7 +3,7 @@ RSpec.feature 'Sign out' do scenario 'user signs out successfully' do sign_in_and_2fa_user - click_link(t('links.sign_out'), match: :first) + click_button(t('links.sign_out'), match: :first) expect(page).to have_content t('devise.sessions.signed_out') end diff --git a/spec/features/visitors/email_confirmation_spec.rb b/spec/features/visitors/email_confirmation_spec.rb index c043c69030c..5b243d5e2c8 100644 --- a/spec/features/visitors/email_confirmation_spec.rb +++ b/spec/features/visitors/email_confirmation_spec.rb @@ -72,8 +72,8 @@ it 'redirects to sign in page with message that user is already confirmed' do allow(IdentityConfig.store).to receive(:participate_in_dap).and_return(true) sign_up_and_set_password + logout(:user) - visit destroy_user_session_url visit sign_up_create_email_confirmation_url(confirmation_token: @raw_confirmation_token) expect(page.html).to include(t('notices.dap_participation')) diff --git a/spec/support/idv_examples/sp_handoff.rb b/spec/support/idv_examples/sp_handoff.rb index 9e159ccb569..8350f068124 100644 --- a/spec/support/idv_examples/sp_handoff.rb +++ b/spec/support/idv_examples/sp_handoff.rb @@ -72,7 +72,7 @@ fill_in 'Password', with: user.password click_continue acknowledge_and_confirm_personal_key - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click end it 'does not require verification and hands off successfully' do @@ -107,7 +107,7 @@ acknowledge_and_confirm_personal_key click_agree_and_continue visit account_path - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click end it 'does not require idv or requested attribute verification and hands off successfully' do diff --git a/spec/support/idv_examples/sp_requested_attributes.rb b/spec/support/idv_examples/sp_requested_attributes.rb index 741705fae4c..53e942c1d2f 100644 --- a/spec/support/idv_examples/sp_requested_attributes.rb +++ b/spec/support/idv_examples/sp_requested_attributes.rb @@ -55,7 +55,7 @@ acknowledge_and_confirm_personal_key click_agree_and_continue visit account_path - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click end it 'does not require the user to verify attributes' do diff --git a/spec/support/shared_examples/remember_device.rb b/spec/support/shared_examples/remember_device.rb index e9e10d20801..6a4948935a8 100644 --- a/spec/support/shared_examples/remember_device.rb +++ b/spec/support/shared_examples/remember_device.rb @@ -31,7 +31,7 @@ click_submit_default # Sign out second user - first(:link, t('links.sign_out')).click + first(:button, t('links.sign_out')).click # Sign in as first user again and expect otp confirmation sign_in_user(first_user) diff --git a/spec/views/accounts/_nav_auth.html.erb_spec.rb b/spec/views/accounts/_nav_auth.html.erb_spec.rb index f5acafd8636..35e42c67a37 100644 --- a/spec/views/accounts/_nav_auth.html.erb_spec.rb +++ b/spec/views/accounts/_nav_auth.html.erb_spec.rb @@ -19,11 +19,14 @@ end it 'does not contain link to cancel the auth process' do - expect(rendered).not_to have_link(t('links.cancel'), href: destroy_user_session_path) + expect(rendered).not_to have_link(t('links.cancel')) end it 'contains sign out link' do - expect(rendered).to have_link(t('links.sign_out'), href: destroy_user_session_path) + expect(rendered).to have_button(t('links.sign_out')) + expect(rendered).to have_selector('form') do |f| + expect(f['action']).to eq logout_path + end end end diff --git a/spec/views/layouts/application.html.erb_spec.rb b/spec/views/layouts/application.html.erb_spec.rb index f1f53f1fb05..b4799cdb66d 100644 --- a/spec/views/layouts/application.html.erb_spec.rb +++ b/spec/views/layouts/application.html.erb_spec.rb @@ -25,7 +25,8 @@ expect(rendered).to have_css('.page-header--basic') expect(rendered).to_not have_content(t('account.welcome')) - expect(rendered).to_not have_link(t('links.sign_out'), href: destroy_user_session_path) + expect(rendered).to_not have_button(t('links.sign_out')) + expect(rendered).to_not have_selector('form') end end diff --git a/spec/views/shared/_nav_lite.html.erb_spec.rb b/spec/views/shared/_nav_lite.html.erb_spec.rb index a7445a84ceb..fa3c151c914 100644 --- a/spec/views/shared/_nav_lite.html.erb_spec.rb +++ b/spec/views/shared/_nav_lite.html.erb_spec.rb @@ -9,7 +9,7 @@ it 'does not contain sign out link' do render - expect(rendered).to_not have_link(t('links.sign_out'), href: destroy_user_session_path) + expect(rendered).to_not have_button(t('links.sign_out')) end end end