Don't set session[:sp] if no issuer is present

app/controllers/sign_out_controller.rb

@@ -2,6 +2,7 @@ class SignOutController < ApplicationController
   include FullyAuthenticatable
 
   def destroy
+    analytics.track_event(Analytics::LOGOUT_INITIATED, method: 'cancel link')
     url_after_cancellation = decorated_session.cancel_link_url
     sign_out
     flash[:success] = t('devise.sessions.signed_out')

app/controllers/sign_up/completions_controller.rb

@@ -31,7 +31,7 @@ def update
     private
 
     def show_completions_page?
-      service_providers = session[:sp].present? || @view_model.user_has_identities?
+      service_providers = sp_session[:issuer].present? || @view_model.user_has_identities?
       user_fully_authenticated? && service_providers
     end
 

app/forms/password_form.rb

@@ -8,6 +8,7 @@ def initialize(user)
 
   def submit(params)
     submitted_password = params[:password]
+    @request_id = params[:request_id]
 
     self.password = submitted_password
 
@@ -16,9 +17,12 @@ def submit(params)
 
   private
 
+  attr_reader :request_id
+
   def extra_analytics_attributes
     {
       user_id: user.uuid,
+      request_id_present: request_id.present?,
     }
   end
 end

app/services/store_sp_metadata_in_session.rb

@@ -5,23 +5,39 @@ def initialize(session:, request_id:)
   end
 
   def call
-    session[:sp] = {
-      issuer: sp_request.issuer,
-      loa3: loa3_requested?,
-      request_url: sp_request.url,
-      request_id: sp_request.uuid,
-      requested_attributes: sp_request.requested_attributes,
-    }
+    Rails.logger.info(event_attributes)
+
+    return if sp_request.is_a?(NullServiceProviderRequest)
+
+    update_session
   end
 
   private
 
   attr_reader :session, :request_id
 
+  def event_attributes
+    {
+      event: 'StoreSpMetadataInSession',
+      request_id_present: request_id.present?,
+      sp_request_class: sp_request.class.to_s,
+    }.to_json
+  end
+
   def sp_request
     @sp_request ||= ServiceProviderRequest.from_uuid(request_id)
   end
 
+  def update_session
+    session[:sp] = {
+      issuer: sp_request.issuer,
+      loa3: loa3_requested?,
+      request_url: sp_request.url,
+      request_id: sp_request.uuid,
+      requested_attributes: sp_request.requested_attributes,
+    }
+  end
+
   def loa3_requested?
     sp_request.loa == Saml::Idp::Constants::LOA3_AUTHN_CONTEXT_CLASSREF
   end

spec/controllers/sign_out_controller_spec.rb

@@ -18,5 +18,16 @@
 
       get :destroy
     end
+
+    it 'tracks the event' do
+      stub_sign_in_before_2fa
+      stub_analytics
+      allow(controller.decorated_session).to receive(:cancel_link_url).and_return('foo')
+
+      expect(@analytics).
+        to receive(:track_event).with(Analytics::LOGOUT_INITIATED, method: 'cancel link')
+
+      get :destroy
+    end
   end
 end

spec/controllers/sign_up/completions_controller_spec.rb

@@ -11,7 +11,7 @@
       context 'LOA1' do
         it 'tracks page visit' do
           stub_sign_in
-          subject.session[:sp] = { loa3: false }
+          subject.session[:sp] = { issuer: 'awesome sp', loa3: false }
           get :show
 
           expect(@analytics).to have_received(:track_event).with(
@@ -25,7 +25,7 @@
         it 'tracks page visit' do
           user = create(:user, profiles: [create(:profile, :verified, :active)])
           stub_sign_in(user)
-          subject.session[:sp] = { loa3: true }
+          subject.session[:sp] = { issuer: 'awesome sp', loa3: true }
 
           get :show
 
@@ -59,9 +59,18 @@
       expect(response).to redirect_to(account_url)
     end
 
+    it 'requires service provider issuer in session' do
+      stub_sign_in
+      subject.session[:sp] = { issuer: nil }
+
+      get :show
+
+      expect(response).to redirect_to(account_url)
+    end
+
     it 'renders show if the user has an sp in the active session' do
       stub_sign_in
-      subject.session[:sp] = { loa3: false }
+      subject.session[:sp] = { issuer: 'awesome sp', loa3: false }
       get :show
 
       expect(response).to render_template(:show)

spec/controllers/sign_up/passwords_controller_spec.rb

@@ -12,6 +12,7 @@
         success: true,
         errors: {},
         user_id: user.uuid,
+        request_id_present: false,
       }
 
       expect(@analytics).to receive(:track_event).
@@ -38,6 +39,7 @@
         success: false,
         errors: { password: ['is too short (minimum is 8 characters)'] },
         user_id: user.uuid,
+        request_id_present: false,
       }
 
       expect(@analytics).to receive(:track_event).

spec/forms/password_form_spec.rb

@@ -16,6 +16,7 @@
 
         extra = {
           user_id: user.uuid,
+          request_id_present: false,
         }
 
         result = instance_double(FormResponse)
@@ -40,6 +41,7 @@
 
         extra = {
           user_id: '123',
+          request_id_present: false,
         }
 
         result = instance_double(FormResponse)
@@ -70,6 +72,7 @@
         passwords.each do |password|
           extra = {
             user_id: '123',
+            request_id_present: false,
           }
           result = instance_double(FormResponse)
 
@@ -79,5 +82,22 @@
         end
       end
     end
+
+    context 'when the request_id is passed in the params' do
+      it 'tracks that it is present' do
+        user = build_stubbed(:user)
+        form = PasswordForm.new(user)
+        password = 'valid password'
+        extra = {
+          user_id: user.uuid,
+          request_id_present: true,
+        }
+        result = instance_double(FormResponse)
+
+        expect(FormResponse).to receive(:new).
+          with(success: true, errors: {}, extra: extra).and_return(result)
+        expect(form.submit(password: password, request_id: 'foo')).to eq result
+      end
+    end
   end
 end

spec/services/store_sp_metadata_in_session_spec.rb

@@ -0,0 +1,55 @@
+require 'rails_helper'
+
+describe StoreSpMetadataInSession do
+  describe '#call' do
+    context 'when a ServiceProviderRequest is not found' do
+      it 'does not set the session[:sp] hash' do
+        allow(Rails.logger).to receive(:info)
+        app_session = {}
+        instance = StoreSpMetadataInSession.new(session: app_session, request_id: 'foo')
+        info_hash = {
+          event: 'StoreSpMetadataInSession',
+          request_id_present: true,
+          sp_request_class: 'NullServiceProviderRequest',
+        }.to_json
+
+        expect { instance.call }.to_not change(app_session, :keys)
+        expect(Rails.logger).to have_received(:info).with(info_hash)
+      end
+    end
+
+    context 'when a ServiceProviderRequest is found' do
+      it 'sets the session[:sp] hash' do
+        allow(Rails.logger).to receive(:info)
+
+        app_session = {}
+        request_id = SecureRandom.uuid
+        ServiceProviderRequest.find_or_create_by(uuid: request_id) do |sp_request|
+          sp_request.issuer = 'issuer'
+          sp_request.loa = 'loa1'
+          sp_request.url = 'http://issuer.gov'
+          sp_request.requested_attributes = %w[email]
+        end
+        instance = StoreSpMetadataInSession.new(session: app_session, request_id: request_id)
+
+        info_hash = {
+          event: 'StoreSpMetadataInSession',
+          request_id_present: true,
+          sp_request_class: 'ServiceProviderRequest',
+        }.to_json
+
+        app_session_hash = {
+          issuer: 'issuer',
+          loa3: false,
+          request_url: 'http://issuer.gov',
+          request_id: request_id,
+          requested_attributes: %w[email],
+        }
+
+        instance.call
+        expect(Rails.logger).to have_received(:info).with(info_hash)
+        expect(app_session[:sp]).to eq app_session_hash
+      end
+    end
+  end
+end