diff --git a/app/controllers/openid_connect/logout_controller.rb b/app/controllers/openid_connect/logout_controller.rb index c774e9785d1..830b9348260 100644 --- a/app/controllers/openid_connect/logout_controller.rb +++ b/app/controllers/openid_connect/logout_controller.rb @@ -19,12 +19,10 @@ def index if result.success? && (redirect_uri = result.extra[:redirect_uri]) sign_out - unless logout_params[:prevent_logout_redirect] == 'true' - redirect_to( - redirect_uri, - allow_other_host: true, - ) - end + redirect_to( + redirect_uri, + allow_other_host: true, + ) else render :error end @@ -35,7 +33,6 @@ def logout_params :id_token_hint, :post_logout_redirect_uri, :state, - :prevent_logout_redirect, ] if IdentityConfig.store.accept_client_id_in_oidc_logout || diff --git a/spec/controllers/openid_connect/logout_controller_spec.rb b/spec/controllers/openid_connect/logout_controller_spec.rb index 98a3c06f8bb..2978a80f9f8 100644 --- a/spec/controllers/openid_connect/logout_controller_spec.rb +++ b/spec/controllers/openid_connect/logout_controller_spec.rb @@ -60,6 +60,15 @@ expect(response).to redirect_to(/^#{post_logout_redirect_uri}/) end + it 'includes CSP headers' do + add_sp_session_request_url + action + + expect( + response.request.content_security_policy.directives['form-action'], + ).to eq(['\'self\'', 'gov.gsa.openidconnect.test:']) + end + it 'tracks events' do stub_analytics expect(@analytics).to receive(:track_event). @@ -580,4 +589,21 @@ end end end + + def add_sp_session_request_url + params = { + acr_values: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, + client_id: service_provider, + nonce: SecureRandom.hex, + redirect_uri: 'gov.gsa.openidconnect.test://result', + response_type: 'code', + scope: 'openid profile', + state: SecureRandom.hex, + } + session[:sp] = { + request_url: URI.parse( + "http://#{IdentityConfig.store.domain_name}?#{URI.encode_www_form(params)}", + ).to_s, + } + end end diff --git a/spec/features/openid_connect/openid_connect_spec.rb b/spec/features/openid_connect/openid_connect_spec.rb index 603f2a6d4bd..4c5a3dea282 100644 --- a/spec/features/openid_connect/openid_connect_spec.rb +++ b/spec/features/openid_connect/openid_connect_spec.rb @@ -143,7 +143,7 @@ end context 'when sending id_token_hint' do - it 'logout includes redirect_uris in CSP headers and destroys the session' do + it 'logout destroys the session' do id_token = sign_in_get_id_token state = SecureRandom.hex @@ -152,16 +152,6 @@ post_logout_redirect_uri: 'gov.gsa.openidconnect.test://result/signout', state: state, id_token_hint: id_token, - prevent_logout_redirect: true, - ) - - current_url_no_port = URI(current_url).tap { |uri| uri.port = nil }.to_s - expect(current_url_no_port).to include( - "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:', ) visit account_path @@ -177,7 +167,7 @@ end context 'when sending client_id' do - it 'logout includes redirect_uris in CSP headers and destroys the session' do + it 'logout destroys the session' do client_id = 'urn:gov:gsa:openidconnect:test' sign_in_get_id_token(client_id: client_id) @@ -187,16 +177,6 @@ client_id: client_id, post_logout_redirect_uri: 'gov.gsa.openidconnect.test://result/signout', state: state, - prevent_logout_redirect: true, - ) - - current_url_no_port = URI(current_url).tap { |uri| uri.port = nil }.to_s - expect(current_url_no_port).to include( - "http://www.example.com/openid_connect/logout?client_id=#{URI.encode_www_form_component(client_id)}", - ) - - expect(page.response_headers['Content-Security-Policy']).to include( - 'form-action \'self\' gov.gsa.openidconnect.test:', ) visit account_path @@ -223,7 +203,6 @@ client_id: client_id, post_logout_redirect_uri: 'gov.gsa.openidconnect.test://result/signout', state: state, - prevent_logout_redirect: true, ) current_url_no_port = URI(current_url).tap { |uri| uri.port = nil }.to_s @@ -242,7 +221,7 @@ and_return(true) end - it 'logout includes redirect_uris in CSP headers and destroys the session' do + it 'logout destroys the session' do client_id = 'urn:gov:gsa:openidconnect:test' sign_in_get_id_token(client_id: client_id) @@ -252,16 +231,6 @@ client_id: client_id, post_logout_redirect_uri: 'gov.gsa.openidconnect.test://result/signout', state: state, - prevent_logout_redirect: true, - ) - - current_url_no_port = URI(current_url).tap { |uri| uri.port = nil }.to_s - expect(current_url_no_port).to include( - "http://www.example.com/openid_connect/logout?client_id=#{URI.encode_www_form_component(client_id)}", - ) - - expect(page.response_headers['Content-Security-Policy']).to include( - 'form-action \'self\' gov.gsa.openidconnect.test:', ) visit account_path @@ -278,7 +247,6 @@ id_token_hint: id_token, post_logout_redirect_uri: 'gov.gsa.openidconnect.test://result/signout', state: state, - prevent_logout_redirect: true, ) current_url_no_port = URI(current_url).tap { |uri| uri.port = nil }.to_s @@ -693,11 +661,6 @@ id_token_hint: id_token, ) - current_url_no_port = URI(current_url).tap { |uri| uri.port = nil }.to_s - expect(current_url_no_port).to eq( - "gov.gsa.openidconnect.test://result/signout?state=#{state}", - ) - 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'))