Allow multiple account creation in same session

app/controllers/concerns/saml_idp_auth_concern.rb

@@ -23,8 +23,6 @@ def validate_service_provider_and_authn_context
   end
 
   def store_saml_request
-    return if sp_session[:request_id]
-
     @request_id = SecureRandom.uuid
     ServiceProviderRequest.find_or_create_by(uuid: @request_id) do |sp_request|
       sp_request.issuer = current_issuer
@@ -35,15 +33,7 @@ def store_saml_request
   end
 
   def add_sp_metadata_to_session
-    return if sp_session[:request_id]
-
-    session[:sp] = {
-      issuer: current_issuer,
-      loa3: loa3_requested?,
-      request_id: @request_id,
-      request_url: request.original_url,
-      requested_attributes: requested_attributes,
-    }
+    StoreSpMetadataInSession.new(session: session, request_id: @request_id).call
   end
 
   def requested_authn_context

app/controllers/openid_connect/authorization_controller.rb

@@ -76,8 +76,6 @@ def validate_authorize_form
     end
 
     def store_request
-      return if sp_session[:request_id]
-
       client_id = @authorize_form.client_id
 
       @request_id = SecureRandom.uuid
@@ -90,15 +88,7 @@ def store_request
     end
 
     def add_sp_metadata_to_session
-      return if sp_session[:request_id]
-
-      session[:sp] = {
-        issuer: @authorize_form.client_id,
-        loa3: @authorize_form.loa3_requested?,
-        request_id: @request_id,
-        request_url: request.original_url,
-        requested_attributes: requested_attributes,
-      }
+      StoreSpMetadataInSession.new(session: session, request_id: @request_id).call
     end
 
     def requested_attributes

spec/controllers/saml_idp_controller_spec.rb

@@ -292,11 +292,11 @@
         sp_request_id = ServiceProviderRequest.last.uuid
 
         expect(session[:sp]).to eq(
-          loa3: false,
           issuer: saml_settings.issuer,
-          request_id: sp_request_id,
+          loa3: false,
           request_url: @saml_request.request.original_url,
-          requested_attributes: [:email]
+          request_id: sp_request_id,
+          requested_attributes: ['email']
         )
       end
 

spec/features/openid_connect/openid_connect_spec.rb

@@ -102,7 +102,6 @@
         prompt: 'select_account'
       )
 
-      sp_request_id = ServiceProviderRequest.last.uuid
       allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true)
       sign_in_user(user)
 
@@ -112,8 +111,6 @@
       click_submit_default
 
       expect(current_url).to start_with('http://localhost:7654/auth/result')
-      expect(ServiceProviderRequest.from_uuid(sp_request_id)).
-        to be_a NullServiceProviderRequest
       expect(page.get_rack_session.keys).to_not include('sp')
     end
 
@@ -134,7 +131,6 @@
         prompt: 'select_account'
       )
 
-      sp_request_id = ServiceProviderRequest.last.uuid
       allow(FeatureManagement).to receive(:prefill_otp_codes?).and_return(true)
       sign_in_user(user)
 
@@ -150,8 +146,6 @@
       click_submit_default
 
       expect(current_url).to start_with('http://localhost:7654/auth/result')
-      expect(ServiceProviderRequest.from_uuid(sp_request_id)).
-        to be_a NullServiceProviderRequest
       expect(page.get_rack_session.keys).to_not include('sp')
     end
   end
@@ -238,16 +232,12 @@
         sign_up_user_from_sp_without_confirming_email(email)
       end
 
-      sp_request_id = ServiceProviderRequest.last.uuid
-
       perform_in_browser(:two) do
         confirm_email_in_a_different_browser(email)
 
         click_button t('forms.buttons.continue')
         redirect_uri = URI(current_url)
         expect(redirect_uri.to_s).to start_with('gov.gsa.openidconnect.test://result')
-        expect(ServiceProviderRequest.from_uuid(sp_request_id)).
-          to be_a NullServiceProviderRequest
         expect(page.get_rack_session.keys).to_not include('sp')
       end
     end
@@ -423,15 +413,14 @@
   end
 
   context 'visiting IdP via SP, then going back to SP and visiting IdP again' do
-    it 'maintains the request_id in the params' do
+    it 'displays the branded page' do
       visit_idp_from_sp_with_loa1
-      sp_request_id = ServiceProviderRequest.last.uuid
 
-      expect(current_url).to eq sign_up_start_url(request_id: sp_request_id)
+      expect(current_url).to match(%r{http://www.example.com/sign_up/start\?request_id=.+})
 
       visit_idp_from_sp_with_loa1
 
-      expect(current_url).to eq sign_up_start_url(request_id: sp_request_id)
+      expect(current_url).to match(%r{http://www.example.com/sign_up/start\?request_id=.+})
     end
   end
 
@@ -496,6 +485,41 @@
     end
   end
 
+  context 'creating two accounts during the same session' do
+    it 'allows the second account creation process to complete fully', email: true do
+      first_email = 'test1@test.com'
+      second_email = 'test2@test.com'
+
+      perform_in_browser(:one) do
+        visit_idp_from_sp_with_loa1
+        sign_up_user_from_sp_without_confirming_email(first_email)
+      end
+
+      perform_in_browser(:two) do
+        confirm_email_in_a_different_browser(first_email)
+        click_button t('forms.buttons.continue')
+        redirect_uri = URI(current_url)
+
+        expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result')
+        expect(page.get_rack_session.keys).to_not include('sp')
+      end
+
+      perform_in_browser(:one) do
+        visit_idp_from_sp_with_loa1
+        sign_up_user_from_sp_without_confirming_email(second_email)
+      end
+
+      perform_in_browser(:two) do
+        confirm_email_in_a_different_browser(second_email)
+        click_button t('forms.buttons.continue')
+        redirect_uri = URI(current_url)
+
+        expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result')
+        expect(page.get_rack_session.keys).to_not include('sp')
+      end
+    end
+  end
+
   def visit_idp_from_sp_with_loa1(state: SecureRandom.hex)
     client_id = 'urn:gov:gsa:openidconnect:sp:server'
     nonce = SecureRandom.hex

spec/features/saml/loa1_sso_spec.rb

@@ -13,8 +13,6 @@
         sign_up_user_from_sp_without_confirming_email(email)
       end
 
-      sp_request_id = ServiceProviderRequest.last.uuid
-
       perform_in_browser(:two) do
         confirm_email_in_a_different_browser(email)
 
@@ -32,8 +30,6 @@
         click_on t('forms.buttons.continue')
 
         expect(current_url).to eq authn_request
-        expect(ServiceProviderRequest.from_uuid(sp_request_id)).
-          to be_a NullServiceProviderRequest
         expect(page.get_rack_session.keys).to_not include('sp')
       end
     end
@@ -43,12 +39,9 @@
       saml_authn_request = auth_request.create(saml_settings)
 
       visit saml_authn_request
-      sp_request_id = ServiceProviderRequest.last.uuid
       sign_in_live_with_2fa(user)
 
       expect(current_url).to eq saml_authn_request
-      expect(ServiceProviderRequest.from_uuid(sp_request_id)).
-        to be_a NullServiceProviderRequest
       expect(page.get_rack_session.keys).to_not include('sp')
 
       visit root_path
@@ -139,16 +132,15 @@
   end
 
   context 'visiting IdP via SP, then going back to SP and visiting IdP again' do
-    it 'maintains the request_id in the params' do
+    it 'displays the branded page' do
       authn_request = auth_request.create(saml_settings)
       visit authn_request
-      sp_request_id = ServiceProviderRequest.last.uuid
 
-      expect(current_url).to eq sign_up_start_url(request_id: sp_request_id)
+      expect(current_url).to match(%r{http://www.example.com/sign_up/start\?request_id=.+})
 
       visit authn_request
 
-      expect(current_url).to eq sign_up_start_url(request_id: sp_request_id)
+      expect(current_url).to match(%r{http://www.example.com/sign_up/start\?request_id=.+})
     end
   end
 
@@ -169,6 +161,40 @@
     end
   end
 
+  context 'creating two accounts during the same session' do
+    it 'allows the second account creation process to complete fully', email: true do
+      first_email = 'test1@test.com'
+      second_email = 'test2@test.com'
+      authn_request = auth_request.create(saml_settings)
+
+      perform_in_browser(:one) do
+        visit authn_request
+        sign_up_user_from_sp_without_confirming_email(first_email)
+      end
+
+      perform_in_browser(:two) do
+        confirm_email_in_a_different_browser(first_email)
+        click_button t('forms.buttons.continue')
+
+        expect(current_url).to eq authn_request
+        expect(page.get_rack_session.keys).to_not include('sp')
+      end
+
+      perform_in_browser(:one) do
+        visit authn_request
+        sign_up_user_from_sp_without_confirming_email(second_email)
+      end
+
+      perform_in_browser(:two) do
+        confirm_email_in_a_different_browser(second_email)
+        click_button t('forms.buttons.continue')
+
+        expect(current_url).to eq authn_request
+        expect(page.get_rack_session.keys).to_not include('sp')
+      end
+    end
+  end
+
   def sign_in_and_require_viewing_personal_key(user)
     login_as(user, scope: :user, run_callbacks: false)
     Warden.on_next_request do |proxy|

spec/support/features/session_helper.rb

@@ -243,7 +243,7 @@ def sign_up_user_from_sp_without_confirming_email(email)
       expect(current_url).to eq sign_up_email_url(request_id: sp_request_id)
       expect(page).to have_css('img[src*=sp-logos]')
 
-      submit_form_with_valid_email
+      submit_form_with_valid_email(email)
 
       expect(current_url).to eq sign_up_verify_email_url(request_id: sp_request_id)
       expect(last_email.html_part.body).to have_content "?_request_id=#{sp_request_id}"