diff --git a/app/controllers/concerns/fully_authenticatable.rb b/app/controllers/concerns/fully_authenticatable.rb index bd0ce49dbcb..3b746f8b833 100644 --- a/app/controllers/concerns/fully_authenticatable.rb +++ b/app/controllers/concerns/fully_authenticatable.rb @@ -1,6 +1,7 @@ module FullyAuthenticatable def delete_branded_experience ServiceProviderRequestProxy.delete(request_id) + session.delete(:sp) end def request_id diff --git a/app/controllers/openid_connect/authorization_controller.rb b/app/controllers/openid_connect/authorization_controller.rb index 34cad9a6011..e978908321d 100644 --- a/app/controllers/openid_connect/authorization_controller.rb +++ b/app/controllers/openid_connect/authorization_controller.rb @@ -32,7 +32,7 @@ def check_sp_active end def check_sp_handoff_bounced - return unless SpHandoffBounce::IsBounced.call(sp_session) + return unless SpHandoffBounce.is_bounced?(session) analytics.track_event(Analytics::SP_HANDOFF_BOUNCED_DETECTED) redirect_to bounced_url true @@ -56,7 +56,7 @@ def link_identity_to_service_provider def handle_successful_handoff track_events - SpHandoffBounce::AddHandoffTimeToSession.call(sp_session) + SpHandoffBounce.add_handoff_time_to_session(session) redirect_to @authorize_form.success_redirect_uri delete_branded_experience end diff --git a/app/controllers/openid_connect/logout_controller.rb b/app/controllers/openid_connect/logout_controller.rb index fadd49041bc..f4cdc83e21e 100644 --- a/app/controllers/openid_connect/logout_controller.rb +++ b/app/controllers/openid_connect/logout_controller.rb @@ -1,8 +1,5 @@ module OpenidConnect class LogoutController < ApplicationController - include SecureHeadersConcern - - before_action :apply_secure_headers_override, only: [:index] def index @logout_form = OpenidConnectLogoutForm.new(params) diff --git a/app/services/sp_handoff_bounce.rb b/app/services/sp_handoff_bounce.rb new file mode 100644 index 00000000000..ffcf919bab0 --- /dev/null +++ b/app/services/sp_handoff_bounce.rb @@ -0,0 +1,13 @@ +class SpHandoffBounce + def self.is_bounced?(session) + start_time = session[:sp_handoff_start_time] + return if start_time.blank? + tz = Time.zone + start_time = tz.parse(start_time) if start_time.class == String + tz.now <= (start_time + AppConfig.env.sp_handoff_bounce_max_seconds.to_i.seconds) + end + + def self.add_handoff_time_to_session(session) + session[:sp_handoff_start_time] = Time.zone.now + end +end diff --git a/app/services/sp_handoff_bounce/add_handoff_time_to_session.rb b/app/services/sp_handoff_bounce/add_handoff_time_to_session.rb deleted file mode 100644 index feaa37377eb..00000000000 --- a/app/services/sp_handoff_bounce/add_handoff_time_to_session.rb +++ /dev/null @@ -1,7 +0,0 @@ -module SpHandoffBounce - class AddHandoffTimeToSession - def self.call(session) - session[:sp_handoff_start_time] = Time.zone.now - end - end -end diff --git a/app/services/sp_handoff_bounce/is_bounced.rb b/app/services/sp_handoff_bounce/is_bounced.rb deleted file mode 100644 index a0673c48804..00000000000 --- a/app/services/sp_handoff_bounce/is_bounced.rb +++ /dev/null @@ -1,11 +0,0 @@ -module SpHandoffBounce - class IsBounced - def self.call(session) - start_time = session[:sp_handoff_start_time] - return if start_time.blank? - tz = Time.zone - start_time = tz.parse(start_time) if start_time.class == String - tz.now <= (start_time + AppConfig.env.sp_handoff_bounce_max_seconds.to_i.seconds) - end - end -end diff --git a/spec/features/openid_connect/openid_connect_spec.rb b/spec/features/openid_connect/openid_connect_spec.rb index c1cd2ca74e5..5f655101d65 100644 --- a/spec/features/openid_connect/openid_connect_spec.rb +++ b/spec/features/openid_connect/openid_connect_spec.rb @@ -89,7 +89,6 @@ click_submit_default expect(current_url).to start_with('http://localhost:7654/auth/result') - expect(page.get_rack_session.keys).to include('sp') end it 'auto-allows and includes redirect_uris in CSP headers after an incorrect OTP' do @@ -116,7 +115,6 @@ click_submit_default expect(current_url).to start_with('http://localhost:7654/auth/result') - expect(page.get_rack_session.keys).to include('sp') end end @@ -137,11 +135,6 @@ "http://www.example.com/openid_connect/logout?id_token_hint=#{id_token}", ) - expect(page.response_headers['Content-Security-Policy']).to include( - 'form-action \'self\' gov.gsa.openidconnect.test://result '\ - 'gov.gsa.openidconnect.test://result/signout', - ) - visit account_path expect(page).to_not have_content(t('headings.account.login_info')) expect(page).to have_content(t('headings.sign_in_without_sp')) @@ -488,7 +481,6 @@ continue_as(email) redirect_uri = URI(current_url) expect(redirect_uri.to_s).to start_with('gov.gsa.openidconnect.test://result') - expect(page.get_rack_session.keys).to include('sp') end end end diff --git a/spec/features/saml/ial1_sso_spec.rb b/spec/features/saml/ial1_sso_spec.rb index e72d5b186df..e9b3e3bd32a 100644 --- a/spec/features/saml/ial1_sso_spec.rb +++ b/spec/features/saml/ial1_sso_spec.rb @@ -32,7 +32,6 @@ continue_as(email) expect(current_url).to eq authn_request - expect(page.get_rack_session.keys).to include('sp') end end diff --git a/spec/features/two_factor_authentication/multiple_tabs_spec.rb b/spec/features/two_factor_authentication/multiple_tabs_spec.rb deleted file mode 100644 index 0593d891557..00000000000 --- a/spec/features/two_factor_authentication/multiple_tabs_spec.rb +++ /dev/null @@ -1,9 +0,0 @@ -require 'rails_helper' - -feature 'user interacts with 2FA across multiple browser tabs' do - include SpAuthHelper - include SamlAuthHelper - - it_behaves_like 'visiting 2fa when fully authenticated', :oidc - it_behaves_like 'visiting 2fa when fully authenticated', :saml -end diff --git a/spec/features/users/sign_in_spec.rb b/spec/features/users/sign_in_spec.rb index c99a4416974..ca4f724757a 100644 --- a/spec/features/users/sign_in_spec.rb +++ b/spec/features/users/sign_in_spec.rb @@ -479,6 +479,25 @@ end end + context 'adds phone number after IAL1 sign in' do + it 'redirects to account page and not the SP' do + user = create(:user, :signed_up) + visit_idp_from_oidc_sp_with_loa1_prompt_login + fill_in_credentials_and_submit(user.email, user.password) + fill_in_code_with_last_phone_otp + click_submit_default + click_agree_and_continue + + visit account_path + click_on "+ #{t('account.index.phone_add')}" + fill_in :new_phone_form_phone, with: '415-555-0199' + click_continue + fill_in_code_with_last_phone_otp + click_submit_default + expect(current_path).to eq account_path + end + end + context 'invalid request_id' do it 'allows the user to sign in and does not try to redirect to any SP' do user = create(:user, :signed_up) diff --git a/spec/support/shared_examples/account_creation.rb b/spec/support/shared_examples/account_creation.rb index dff1d2258a6..556cddcfabd 100644 --- a/spec/support/shared_examples/account_creation.rb +++ b/spec/support/shared_examples/account_creation.rb @@ -150,7 +150,6 @@ expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') end - expect(page.get_rack_session.keys).to include('sp') end perform_in_browser(:one) do @@ -171,7 +170,6 @@ expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') end - expect(page.get_rack_session.keys).to include('sp') end end end diff --git a/spec/support/shared_examples/sign_in.rb b/spec/support/shared_examples/sign_in.rb index 7f1d5cf76b4..57f4f966254 100644 --- a/spec/support/shared_examples/sign_in.rb +++ b/spec/support/shared_examples/sign_in.rb @@ -41,27 +41,6 @@ end end -shared_examples 'visiting 2fa when fully authenticated' do |sp| - before { Timecop.freeze Time.zone.now } - after { Timecop.return } - - it 'redirects to SP after visiting a 2fa screen when fully authenticated', email: true do - ial1_sign_in_with_personal_key_goes_to_sp(sp) - - visit login_two_factor_options_path - - click_continue - continue_as - expect(current_url).to eq @saml_authn_request if sp == :saml - - if sp == :oidc - redirect_uri = URI(current_url) - - expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') - end - end -end - shared_examples 'signing in as IAL2 with personal key' do |sp| before { Timecop.freeze Time.zone.now } after { Timecop.return }