diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3cd020fef7b..8cd801eaf7b 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -425,23 +425,18 @@ def sp_session # Conditionally sets the final_auth_request service provider session attribute # when applicable (the original SP request is SAML) def sp_session_request_url_with_updated_params - # Temporarily place SAML route update behind a feature flag - if IdentityConfig.store.saml_internal_post - return unless sp_session[:request_url].present? - request_url = URI(sp_session[:request_url]) - url = if request_url.path.match?('saml') - sp_session[:final_auth_request] = true - complete_saml_url - else - # Login.gov redirects to the orginal request_url after a user authenticates - # replace prompt=login with prompt=select_account to prevent sign_out - # which should only ever occur once when the user - # lands on Login.gov with prompt=login - sp_session[:request_url]&.gsub('prompt=login', 'prompt=select_account') - end - else - url = sp_session[:request_url]&.gsub('prompt=login', 'prompt=select_account') - end + return unless sp_session[:request_url].present? + request_url = URI(sp_session[:request_url]) + url = if request_url.path.match?('saml') + sp_session[:final_auth_request] = true + complete_saml_url + else + # Login.gov redirects to the orginal request_url after a user authenticates + # replace prompt=login with prompt=select_account to prevent sign_out + # which should only ever occur once when the user + # lands on Login.gov with prompt=login + sp_session[:request_url]&.gsub('prompt=login', 'prompt=select_account') + end # If the user has changed the locale, we should preserve that as well if url && locale_url_param && UriService.params(url)[:locale] != locale_url_param diff --git a/app/controllers/concerns/saml_idp_auth_concern.rb b/app/controllers/concerns/saml_idp_auth_concern.rb index d7df3b73e8c..4e776a913a8 100644 --- a/app/controllers/concerns/saml_idp_auth_concern.rb +++ b/app/controllers/concerns/saml_idp_auth_concern.rb @@ -20,12 +20,8 @@ module SamlIdpAuthConcern def sign_out_if_forceauthn_is_true_and_user_is_signed_in return unless user_signed_in? && saml_request.force_authn? - if IdentityConfig.store.saml_internal_post - sign_out unless sp_session[:final_auth_request] - sp_session[:final_auth_request] = false - else - sign_out unless sp_session[:request_url] == request.original_url - end + sign_out unless sp_session[:final_auth_request] + sp_session[:final_auth_request] = false end def check_sp_active diff --git a/config/application.yml.default b/config/application.yml.default index 25a606864d5..f9774d25260 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -293,7 +293,6 @@ rules_of_use_horizon_years: 6 rules_of_use_updated_at: '2022-01-19T00:00:00Z' # Production has a newer timestamp than this, update directly in S3 s3_public_reports_enabled: false s3_reports_enabled: false -saml_internal_post: true saml_secret_rotation_enabled: false seed_agreements_data: true service_provider_request_ttl_hours: 24 @@ -570,7 +569,6 @@ test: s3_report_bucket_prefix: '' s3_report_public_bucket_prefix: '' saml_endpoint_configs: '[{"suffix":"2023","secret_key_passphrase":"trust-but-verify"}]' - saml_internal_post: true scrypt_cost: 800$8$1$ secret_key_base: test_secret_key_base session_encryption_key: 27bad3c25711099429c1afdfd1890910f3b59f5a4faec1c85e945cb8b02b02f261ba501d99cfbb4fab394e0102de6fecf8ffe260f322f610db3e96b2a775c120 diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 8b6cbfa4aec..8b7f8a1c1b6 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -398,7 +398,6 @@ def self.build_store(config_map) config.add(:secret_key_base, type: :string) config.add(:seed_agreements_data, type: :boolean) config.add(:service_provider_request_ttl_hours, type: :integer) - config.add(:saml_internal_post, type: :boolean) config.add(:session_check_delay, type: :integer) config.add(:session_check_frequency, type: :integer) config.add(:session_encryption_key, type: :string) diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 215f3da082f..f6382e59923 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -436,38 +436,6 @@ def index expect(url_with_updated_params).to eq('/authorize?locale=es') end end - - context 'with saml_internal_post feature flag set to false' do - before { allow(IdentityConfig.store).to receive(:saml_internal_post).and_return false } - context 'with a SAML request' do - let(:sp_session_request_url) { '/api/saml/auth2023?SAMLRequest=blah' } - it 'returns the original request url' do - expect(url_with_updated_params).to eq '/api/saml/auth2023?SAMLRequest=blah' - end - end - - context 'with an OIDC request' do - let(:sp_session_request_url) { '/openid_connect/authorize' } - it 'returns the original request' do - expect(url_with_updated_params).to eq '/openid_connect/authorize' - end - end - - context 'with a url that has prompt=login' do - let(:sp_session_request_url) { '/authorize?prompt=login' } - it 'changes it to prompt=select_account' do - expect(url_with_updated_params).to eq('/authorize?prompt=select_account') - end - end - - context 'when the locale has been changed' do - before { I18n.locale = :es } - let(:sp_session_request_url) { '/authorize' } - it 'adds the locale to the url' do - expect(url_with_updated_params).to eq('/authorize?locale=es') - end - end - end end def expect_user_event_to_have_been_created(user, event_type) diff --git a/spec/controllers/saml_idp_controller_spec.rb b/spec/controllers/saml_idp_controller_spec.rb index 27f9df1d08a..0b6f8c2ffc2 100644 --- a/spec/controllers/saml_idp_controller_spec.rb +++ b/spec/controllers/saml_idp_controller_spec.rb @@ -873,7 +873,7 @@ def name_id_version(format_urn) end end - context 'saml_internal_post feature configuration is set to true with ForceAuthn' do + context 'with ForceAuthn' do let(:user) { create(:user, :signed_up) } it 'signs user out if a session is active and sp_session[:final_auth_request] is falsey' do @@ -902,23 +902,6 @@ def name_id_version(format_urn) end end - context 'saml_internal_post feature configuration is set to false' do - let(:user) { create(:user, :signed_up) } - - before { allow(IdentityConfig.store).to receive(:saml_internal_post).and_return(false) } - - context 'ForceAuthn set to true' do - it 'signs the user out if a session is active' do - sign_in(user) - generate_saml_response(user, saml_settings(overrides: { force_authn: true })) - # would be 200 if the user's session persists - expect(response.status).to eq(302) - # implicit test of request storage since request_id would be missing otherwise - expect(response.location).to match(%r{#{root_url}\?request_id=.+}) - end - end - end - context 'service provider is inactive' do it 'responds with an error page' do user = create(:user, :signed_up)