Fix adding new phone number redirects to SP (LG-3784)

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

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

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)

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

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

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
 

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)

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

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 }