Deployment to staging: patch for 1.008

app/controllers/application_controller.rb

@@ -46,7 +46,10 @@ def create_user_event(event_type, user = current_user)
 
   def decorated_session
     @_decorated_session ||= DecoratedSession.new(
-      sp: current_sp, view_context: view_context, sp_session: sp_session
+      sp: current_sp,
+      view_context: view_context,
+      sp_session: sp_session,
+      service_provider_request: service_provider_request
     ).call
   end
 
@@ -74,11 +77,14 @@ def sp_from_sp_session
   end
 
   def sp_from_request_id
-    issuer = ServiceProviderRequest.from_uuid(params[:request_id]).issuer
-    sp = ServiceProvider.from_issuer(issuer)
+    sp = ServiceProvider.from_issuer(service_provider_request.issuer)
     sp if sp.is_a? ServiceProvider
   end
 
+  def service_provider_request
+    @service_provider_request ||= ServiceProviderRequest.from_uuid(params[:request_id])
+  end
+
   def after_sign_in_path_for(user)
     stored_location_for(user) || sp_session[:request_url] || signed_in_path
   end

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

app/controllers/sign_out_controller.rb

@@ -0,0 +1,13 @@
+class SignOutController < ApplicationController
+  include FullyAuthenticatable
+
+  skip_before_action :handle_two_factor_authentication
+
+  def destroy
+    path_after_cancellation = decorated_session.cancel_link_path
+    sign_out
+    flash[:success] = t('devise.sessions.signed_out')
+    redirect_to path_after_cancellation
+    delete_branded_experience
+  end
+end

app/decorators/service_provider_session_decorator.rb

@@ -3,10 +3,11 @@ class ServiceProviderSessionDecorator
 
   DEFAULT_LOGO = 'generic.svg'.freeze
 
-  def initialize(sp:, view_context:, sp_session:)
+  def initialize(sp:, view_context:, sp_session:, service_provider_request:)
     @sp = sp
     @view_context = view_context
     @sp_session = sp_session
+    @service_provider_request = service_provider_request
   end
 
   def sp_logo
@@ -71,10 +72,10 @@ def cancel_link_path
 
   private
 
-  attr_reader :sp, :view_context, :sp_session
+  attr_reader :sp, :view_context, :sp_session, :service_provider_request
 
   def request_url
-    sp_session[:request_url]
+    sp_session[:request_url] || service_provider_request.url
   end
 
   def openid_connect_redirector

app/presenters/two_factor_auth_code/authenticator_delivery_presenter.rb

@@ -22,7 +22,7 @@ def cancel_link
       if reauthn
         account_path
       else
-        destroy_user_session_path
+        sign_out_path
       end
     end
 

app/presenters/two_factor_auth_code/phone_delivery_presenter.rb

@@ -22,7 +22,7 @@ def cancel_link
       if confirmation_for_phone_change || reauthn
         account_path
       else
-        destroy_user_session_path
+        sign_out_path
       end
     end
 

app/services/decorated_session.rb

@@ -1,14 +1,18 @@
 class DecoratedSession
-  def initialize(sp:, view_context:, sp_session:)
+  def initialize(sp:, view_context:, sp_session:, service_provider_request:)
     @sp = sp
     @view_context = view_context
     @sp_session = sp_session
+    @service_provider_request = service_provider_request
   end
 
   def call
     if sp.is_a? ServiceProvider
       ServiceProviderSessionDecorator.new(
-        sp: sp, view_context: view_context, sp_session: sp_session
+        sp: sp,
+        view_context: view_context,
+        sp_session: sp_session,
+        service_provider_request: service_provider_request
       )
     else
       SessionDecorator.new
@@ -17,5 +21,5 @@ def call
 
   private
 
-  attr_reader :sp, :view_context, :sp_session
+  attr_reader :sp, :view_context, :sp_session, :service_provider_request
 end

app/views/two_factor_authentication/personal_key_verification/show.html.slim

@@ -8,4 +8,4 @@ p.mt-tiny.mb0 = t('devise.two_factor_authentication.personal_key_prompt')
   = render 'partials/personal_key/entry_fields', f: f, attribute_name: :personal_key
   = f.button :submit, t('forms.buttons.submit.default'), class: 'btn btn-primary'
 
-= render 'shared/cancel', link: destroy_user_session_path
+= render 'shared/cancel', link: sign_out_path

config/routes.rb

@@ -114,6 +114,8 @@
   get '/sign_up/completed' => 'sign_up/completions#show', as: :sign_up_completed
   post '/sign_up/completed' => 'sign_up/completions#update'
 
+  match '/sign_out' => 'sign_out#destroy', via: %i[get post delete]
+
   delete '/users' => 'users#destroy', as: :destroy_user
 
   if FeatureManagement.enable_identity_verification?
@@ -151,5 +153,7 @@
   # The line below will route all requests that aren't
   # defined route to the 404 page. Therefore, anything you put after this rule
   # will be ignored.
-  match '*path', via: :all, to: 'pages#page_not_found'
+  constraints(format: /html/) do
+    match '*path', via: :all, to: 'pages#page_not_found'
+  end
 end

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/controllers/sign_out_controller_spec.rb

@@ -0,0 +1,22 @@
+require 'rails_helper'
+
+describe SignOutController do
+  describe '#destroy' do
+    it 'redirects to decorated_session.cancel_link_path with flash message' do
+      stub_sign_in_before_2fa
+      allow(controller.decorated_session).to receive(:cancel_link_path).and_return('foo')
+
+      get :destroy
+
+      expect(response).to redirect_to 'foo'
+      expect(flash[:success]).to eq t('devise.sessions.signed_out')
+    end
+
+    it 'calls #sign_out and #delete_branded_experience' do
+      expect(controller).to receive(:sign_out).and_call_original
+      expect(controller).to receive(:delete_branded_experience)
+
+      get :destroy
+    end
+  end
+end

spec/decorators/service_provider_session_decorator_spec.rb

@@ -3,7 +3,12 @@
 RSpec.describe ServiceProviderSessionDecorator do
   let(:view_context) { ActionController::Base.new.view_context }
   subject do
-    ServiceProviderSessionDecorator.new(sp: sp, view_context: view_context, sp_session: {})
+    ServiceProviderSessionDecorator.new(
+      sp: sp,
+      view_context: view_context,
+      sp_session: {},
+      service_provider_request: ServiceProviderRequest.new
+    )
   end
   let(:sp) { build_stubbed(:service_provider) }
   let(:sp_name) { subject.sp_name }
@@ -59,7 +64,10 @@
     it 'returns the agency name if friendly name is not present' do
       sp = build_stubbed(:service_provider, friendly_name: nil)
       subject = ServiceProviderSessionDecorator.new(
-        sp: sp, view_context: view_context, sp_session: {}
+        sp: sp,
+        view_context: view_context,
+        sp_session: {},
+        service_provider_request: ServiceProviderRequest.new
       )
       expect(subject.sp_name).to eq sp.agency
       expect(subject.sp_name).to_not be_nil
@@ -73,7 +81,10 @@
         sp = build_stubbed(:service_provider, logo: sp_logo)
 
         subject = ServiceProviderSessionDecorator.new(
-          sp: sp, view_context: view_context, sp_session: {}
+          sp: sp,
+          view_context: view_context,
+          sp_session: {},
+          service_provider_request: ServiceProviderRequest.new
         )
 
         expect(subject.sp_logo).to eq sp_logo
@@ -85,7 +96,10 @@
         sp = build_stubbed(:service_provider, logo: nil)
 
         subject = ServiceProviderSessionDecorator.new(
-          sp: sp, view_context: view_context, sp_session: {}
+          sp: sp,
+          view_context: view_context,
+          sp_session: {},
+          service_provider_request: ServiceProviderRequest.new
         )
 
         expect(subject.sp_logo).to eq 'generic.svg'
@@ -96,7 +110,10 @@
   describe '#cancel_link_path' do
     it 'returns sign_up_start_url with the request_id as a param' do
       subject = ServiceProviderSessionDecorator.new(
-        sp: sp, view_context: view_context, sp_session: { request_id: 'foo' }
+        sp: sp,
+        view_context: view_context,
+        sp_session: { request_id: 'foo' },
+        service_provider_request: ServiceProviderRequest.new
       )
 
       expect(subject.cancel_link_path).