diff --git a/app/controllers/concerns/fully_authenticatable.rb b/app/controllers/concerns/fully_authenticatable.rb index bd0ce49dbcb..baef9a3ebb9 100644 --- a/app/controllers/concerns/fully_authenticatable.rb +++ b/app/controllers/concerns/fully_authenticatable.rb @@ -1,6 +1,8 @@ module FullyAuthenticatable - def delete_branded_experience + def delete_branded_experience(logout: false) ServiceProviderRequestProxy.delete(request_id) + session[:sp] = {} if logout + nil end def request_id diff --git a/app/controllers/openid_connect/logout_controller.rb b/app/controllers/openid_connect/logout_controller.rb index 3aed416bf98..b43a72b3b31 100644 --- a/app/controllers/openid_connect/logout_controller.rb +++ b/app/controllers/openid_connect/logout_controller.rb @@ -1,6 +1,7 @@ module OpenidConnect class LogoutController < ApplicationController include SecureHeadersConcern + include FullyAuthenticatable before_action :apply_secure_headers_override, only: [:index, :delete] before_action :confirm_two_factor_authenticated, only: [:delete] @@ -62,6 +63,8 @@ def handle_successful_logout_request(result, redirect_uri) } @params[:state] = logout_params[:state] if !logout_params[:state].nil? @service_provider_name = @logout_form.service_provider&.friendly_name + delete_branded_experience(logout: true) + render :index else analytics.logout_initiated(**result.to_h.except(:redirect_uri)) diff --git a/app/controllers/users/service_provider_inactive_controller.rb b/app/controllers/users/service_provider_inactive_controller.rb index 0c10625ce0e..1be7cb73cf7 100644 --- a/app/controllers/users/service_provider_inactive_controller.rb +++ b/app/controllers/users/service_provider_inactive_controller.rb @@ -8,8 +8,7 @@ def index @sp_name = sp_from_sp_session&.friendly_name || I18n.t('service_providers.errors.generic_sp_name') - delete_branded_experience - session[:sp] = {} + delete_branded_experience(logout: true) end end end diff --git a/spec/features/openid_connect/openid_connect_spec.rb b/spec/features/openid_connect/openid_connect_spec.rb index 0646a7c2e35..b29b985bfb8 100644 --- a/spec/features/openid_connect/openid_connect_spec.rb +++ b/spec/features/openid_connect/openid_connect_spec.rb @@ -242,6 +242,7 @@ click_link t('openid_connect.logout.deny') expect(page).to have_content(t('headings.account.login_info')) + expect(page).not_to have_content(service_provider.friendly_name) end end end @@ -299,6 +300,30 @@ expect(page).to have_content(t('headings.sign_in_without_sp')) end + it 'does not destroy the session and redirects to account page when denying logout' do + service_provider = ServiceProvider.find_by(issuer: 'urn:gov:gsa:openidconnect:test') + sign_in_get_id_token(client_id: service_provider.issuer) + + state = SecureRandom.hex + + visit openid_connect_logout_path( + client_id: service_provider.issuer, + post_logout_redirect_uri: 'gov.gsa.openidconnect.test://result/signout', + state: state, + ) + expect(page).to have_content( + t( + 'openid_connect.logout.heading_with_sp', + app_name: APP_NAME, + service_provider_name: service_provider.friendly_name, + ), + ) + click_link t('openid_connect.logout.deny') + + expect(page).to have_content(t('headings.account.login_info')) + expect(page).not_to have_content(service_provider.friendly_name) + end + it 'logout rejects requests that include id_token_hint' do id_token = sign_in_get_id_token