diff --git a/app/controllers/concerns/secure_headers_concern.rb b/app/controllers/concerns/secure_headers_concern.rb index d7e7e3496b1..e0aecab4451 100644 --- a/app/controllers/concerns/secure_headers_concern.rb +++ b/app/controllers/concerns/secure_headers_concern.rb @@ -3,6 +3,7 @@ module SecureHeadersConcern def apply_secure_headers_override return if stored_url_for_user.blank? + return unless IdentityConfig.store.openid_connect_content_security_form_action_enabled authorize_form = OpenidConnectAuthorizeForm.new(authorize_params) return unless authorize_form.valid? diff --git a/app/controllers/openid_connect/authorization_controller.rb b/app/controllers/openid_connect/authorization_controller.rb index de08e231166..e2113dda38d 100644 --- a/app/controllers/openid_connect/authorization_controller.rb +++ b/app/controllers/openid_connect/authorization_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module OpenidConnect class AuthorizationController < ApplicationController include FullyAuthenticatable @@ -73,7 +75,16 @@ def ial_context def handle_successful_handoff track_events SpHandoffBounce::AddHandoffTimeToSession.call(sp_session) - redirect_to @authorize_form.success_redirect_uri, allow_other_host: true + if IdentityConfig.store.openid_connect_redirect_interstitial_enabled + @oidc_redirect_uri = @authorize_form.success_redirect_uri + render( + 'openid_connect/shared/redirect', + layout: false, + ) + else + redirect_to @authorize_form.success_redirect_uri, allow_other_host: true + end + delete_branded_experience end @@ -97,6 +108,7 @@ def build_authorize_form_from_params end def secure_headers_override + return unless IdentityConfig.store.openid_connect_content_security_form_action_enabled csp_uris = SecureHeadersAllowList.csp_with_sp_redirect_uris( @authorize_form.redirect_uri, @authorize_form.service_provider.redirect_uris, @@ -117,11 +129,18 @@ def pre_validate_authorize_form ), ) return if result.success? + redirect_uri = result.extra[:redirect_uri] - if (redirect_uri = result.extra[:redirect_uri]) - redirect_to redirect_uri, allow_other_host: true - else + if redirect_uri.nil? render :error + elsif IdentityConfig.store.openid_connect_redirect_interstitial_enabled + @oidc_redirect_uri = redirect_uri + render( + 'openid_connect/shared/redirect', + layout: false, + ) + else + redirect_to redirect_uri, allow_other_host: true end end diff --git a/app/controllers/openid_connect/logout_controller.rb b/app/controllers/openid_connect/logout_controller.rb index c438f3420a2..5da023013ba 100644 --- a/app/controllers/openid_connect/logout_controller.rb +++ b/app/controllers/openid_connect/logout_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module OpenidConnect class LogoutController < ApplicationController include SecureHeadersConcern @@ -29,10 +31,18 @@ def delete irs_attempts_api_tracker.logout_initiated(success: result.success?) sign_out - redirect_to( - redirect_uri, - allow_other_host: true, - ) + if IdentityConfig.store.openid_connect_redirect_interstitial_enabled + @oidc_redirect_uri = redirect_uri + render( + 'openid_connect/shared/redirect', + layout: false, + ) + else + redirect_to( + redirect_uri, + allow_other_host: true, + ) + end else render :error end @@ -42,6 +52,7 @@ def delete def apply_logout_secure_headers_override(redirect_uri, service_provider) return if service_provider.nil? || redirect_uri.nil? + return unless IdentityConfig.store.openid_connect_content_security_form_action_enabled uris = SecureHeadersAllowList.csp_with_sp_redirect_uris( redirect_uri, @@ -82,10 +93,19 @@ def handle_successful_logout_request(result, redirect_uri) irs_attempts_api_tracker.logout_initiated(success: result.success?) sign_out - redirect_to( - redirect_uri, - allow_other_host: true, - ) + + if IdentityConfig.store.openid_connect_redirect_interstitial_enabled + @oidc_redirect_uri = redirect_uri + render( + 'openid_connect/shared/redirect', + layout: false, + ) + else + redirect_to( + redirect_uri, + allow_other_host: true, + ) + end end end diff --git a/app/views/openid_connect/shared/redirect.html.erb b/app/views/openid_connect/shared/redirect.html.erb new file mode 100644 index 00000000000..75531cb75e1 --- /dev/null +++ b/app/views/openid_connect/shared/redirect.html.erb @@ -0,0 +1,12 @@ + + + + + <%= t('headings.redirecting') %> | <%= APP_NAME %> + <%= stylesheet_link_tag 'application', media: 'all' %> + <%= render_stylesheet_once_tags %> + + + + + diff --git a/app/views/saml_idp/shared/saml_post_binding.html.erb b/app/views/saml_idp/shared/saml_post_binding.html.erb index fad016f2439..eb827e26bf8 100644 --- a/app/views/saml_idp/shared/saml_post_binding.html.erb +++ b/app/views/saml_idp/shared/saml_post_binding.html.erb @@ -2,7 +2,7 @@ - <%= t('.redirecting') %> | <%= APP_NAME %> + <%= t('headings.redirecting') %> | <%= APP_NAME %> <%= csrf_meta_tags %> <%= stylesheet_link_tag 'application', media: 'all' %> <%= render_stylesheet_once_tags %> diff --git a/config/application.yml.default b/config/application.yml.default index 00d196751b0..55bbb2f81b5 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -211,6 +211,8 @@ multi_region_kms_migration_jobs_profile_timeout: 120 multi_region_kms_migration_jobs_user_count: 1000 multi_region_kms_migration_jobs_user_timeout: 120 mx_timeout: 3 +openid_connect_redirect_interstitial_enabled: true +openid_connect_content_security_form_action_enabled: false otp_delivery_blocklist_maxretry: 10 otp_valid_for: 10 otp_expiration_warning_seconds: 150 @@ -470,6 +472,8 @@ production: logins_per_ip_track_only_mode: true newrelic_license_key: '' nonessential_email_banlist: '[]' + openid_connect_redirect_interstitial_enabled: false + openid_connect_content_security_form_action_enabled: true otp_delivery_blocklist_findtime: 5 participate_in_dap: true password_pepper: diff --git a/config/locales/headings/en.yml b/config/locales/headings/en.yml index 5066fd0a215..f88741dd91d 100644 --- a/config/locales/headings/en.yml +++ b/config/locales/headings/en.yml @@ -56,6 +56,7 @@ en: piv_cac_setup: already_associated: The PIV/CAC you presented is associated with another user. new: Use your PIV/CAC card to secure your account + redirecting: Redirecting residential_address: Current residential address session_timeout_warning: Need more time? sign_in_existing_users: Sign in for existing users diff --git a/config/locales/headings/es.yml b/config/locales/headings/es.yml index 301d7ebd5db..4b98a646142 100644 --- a/config/locales/headings/es.yml +++ b/config/locales/headings/es.yml @@ -56,6 +56,7 @@ es: piv_cac_setup: already_associated: La PIV/CAC que has presentado está asociada a otro usuario. new: Use su tarjeta PIV/CAC para asegurar su cuenta + redirecting: Redirigiendo residential_address: Dirección residencial actual session_timeout_warning: '¿Necesita más tiempo?' sign_in_existing_users: Iniciar sesión para usuarios existentes diff --git a/config/locales/headings/fr.yml b/config/locales/headings/fr.yml index 3f2100eebe1..7353464d25e 100644 --- a/config/locales/headings/fr.yml +++ b/config/locales/headings/fr.yml @@ -59,6 +59,7 @@ fr: already_associated: La carte PIV/CAC que vous avez présentée est associée à un autre utilisateur. new: Utilisez votre carte PIV/CAC pour sécuriser votre compte + redirecting: Redirection residential_address: Adresse de résidence actuelle session_timeout_warning: Vous avez besoin de plus de temps? sign_in_existing_users: S’identifier pour les utilisateurs existants diff --git a/config/locales/saml_idp/en.yml b/config/locales/saml_idp/en.yml index f273cd0af06..6c273e15a40 100644 --- a/config/locales/saml_idp/en.yml +++ b/config/locales/saml_idp/en.yml @@ -10,4 +10,3 @@ en: no_js: JavaScript seems to be turned off in your browser. Normally this step happens automatically, but because you have JavaScript turned off, please click the submit button to continue signing in or signing out. - redirecting: Redirecting diff --git a/config/locales/saml_idp/es.yml b/config/locales/saml_idp/es.yml index 9e57fd4c088..a408d15022e 100644 --- a/config/locales/saml_idp/es.yml +++ b/config/locales/saml_idp/es.yml @@ -11,4 +11,3 @@ es: paso se realiza automáticamente, pero debido a que tiene JavaScript desactivado, haga clic en el botón Enviar para continuar iniciando o cerrando la sesión. - redirecting: Redirigiendo diff --git a/config/locales/saml_idp/fr.yml b/config/locales/saml_idp/fr.yml index 4b31787ed91..7c10525b20d 100644 --- a/config/locales/saml_idp/fr.yml +++ b/config/locales/saml_idp/fr.yml @@ -11,4 +11,3 @@ fr: cette étape se déroule automatiquement, mais parce que vous avez désactivé le JavaScript, veuillez cliquer sur le lien « soumettre » pour continuer ou pour vous déconnecter. - redirecting: Redirection diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 31a7ed683c3..87f6f4f9b93 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -322,6 +322,8 @@ def self.build_store(config_map) config.add(:mx_timeout, type: :integer) config.add(:newrelic_license_key, type: :string) config.add(:nonessential_email_banlist, type: :json) + config.add(:openid_connect_redirect_interstitial_enabled, type: :boolean) + config.add(:openid_connect_content_security_form_action_enabled, type: :boolean) config.add(:otp_delivery_blocklist_findtime, type: :integer) config.add(:otp_delivery_blocklist_maxretry, type: :integer) config.add(:otp_expiration_warning_seconds, type: :integer) diff --git a/spec/controllers/openid_connect/authorization_controller_spec.rb b/spec/controllers/openid_connect/authorization_controller_spec.rb index 25e49e3a381..0d4634cbeb5 100644 --- a/spec/controllers/openid_connect/authorization_controller_spec.rb +++ b/spec/controllers/openid_connect/authorization_controller_spec.rb @@ -1,3 +1,4 @@ +# rubocop:disable Layout/LineLength require 'rails_helper' RSpec.describe OpenidConnect::AuthorizationController do @@ -47,7 +48,9 @@ end context 'with valid params' do - it 'redirects back to the client app with a code' do + it 'redirects back to the client app with a code if client-side redirect is disabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(false) IdentityLinker.new(user, service_provider).link_identity(ial: 1) user.identities.last.update!(verified_attributes: %w[given_name family_name birthdate]) action @@ -60,6 +63,22 @@ expect(redirect_params[:state]).to eq(params[:state]) end + it 'renders a client-side redirect back to the client app with a code if it is enabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(true) + IdentityLinker.new(user, service_provider).link_identity(ial: 1) + user.identities.last.update!(verified_attributes: %w[given_name family_name birthdate]) + action + + expect(controller).to render_template('openid_connect/shared/redirect') + expect(assigns(:oidc_redirect_uri)).to start_with(params[:redirect_uri]) + + redirect_params = UriService.params(assigns(:oidc_redirect_uri)) + + expect(redirect_params[:code]).to be_present + expect(redirect_params[:state]).to eq(params[:state]) + end + it 'tracks IAL1 authentication event' do stub_analytics expect(@analytics).to receive(:track_event). @@ -108,7 +127,9 @@ ).user end - it 'redirects to the redirect_uri immediately when pii is unlocked' do + it 'redirects to the redirect_uri immediately when pii is unlocked if client-side redirect is disabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(false) IdentityLinker.new(user, service_provider).link_identity(ial: 3) user.identities.last.update!( verified_attributes: %w[given_name family_name birthdate verified_at], @@ -119,6 +140,20 @@ expect(response).to redirect_to(/^#{params[:redirect_uri]}/) end + it 'renders a client-side redirect back to the client app immediately if it is enabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(true) + IdentityLinker.new(user, service_provider).link_identity(ial: 3) + user.identities.last.update!( + verified_attributes: %w[given_name family_name birthdate verified_at], + ) + allow(controller).to receive(:pii_requested_but_locked?).and_return(false) + action + + expect(controller).to render_template('openid_connect/shared/redirect') + expect(assigns(:oidc_redirect_uri)).to start_with(params[:redirect_uri]) + end + it 'redirects to the password capture url when pii is locked' do IdentityLinker.new(user, service_provider).link_identity(ial: 3) user.identities.last.update!( @@ -272,7 +307,9 @@ ).user end - it 'redirects to the redirect_uri immediately when pii is unlocked' do + it 'redirects to the redirect_uri immediately when pii is unlocked if client-side redirect is disabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(false) IdentityLinker.new(user, service_provider).link_identity(ial: 3) user.identities.last.update!( verified_attributes: %w[given_name family_name birthdate verified_at], @@ -283,6 +320,21 @@ expect(response).to redirect_to(/^#{params[:redirect_uri]}/) end + it 'renders client-side redirect to the client app immediately if PII is unlocked and it is enabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(true) + + IdentityLinker.new(user, service_provider).link_identity(ial: 3) + user.identities.last.update!( + verified_attributes: %w[given_name family_name birthdate verified_at], + ) + allow(controller).to receive(:pii_requested_but_locked?).and_return(false) + action + + expect(controller).to render_template('openid_connect/shared/redirect') + expect(assigns(:oidc_redirect_uri)).to start_with(params[:redirect_uri]) + end + it 'redirects to the password capture url when pii is locked' do IdentityLinker.new(user, service_provider).link_identity(ial: 3) user.identities.last.update!( @@ -336,16 +388,33 @@ end context 'account is not already verified' do - it 'redirects to the redirect_uri immediately without proofing' do + it 'redirects to the redirect_uri immediately without proofing if client-side redirect is disabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(false) IdentityLinker.new(user, service_provider).link_identity(ial: 1) user.identities.last.update!( verified_attributes: %w[given_name family_name birthdate verified_at], ) action + expect(response).to redirect_to(/^#{params[:redirect_uri]}/) end + it 'renders client-side redirect to the client app immediately if client-side redirect is enabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(true) + + IdentityLinker.new(user, service_provider).link_identity(ial: 1) + user.identities.last.update!( + verified_attributes: %w[given_name family_name birthdate verified_at], + ) + + action + expect(controller).to render_template('openid_connect/shared/redirect') + expect(assigns(:oidc_redirect_uri)).to start_with(params[:redirect_uri]) + end + it 'tracks IAL1 authentication event' do stub_analytics expect(@analytics).to receive(:track_event). @@ -389,16 +458,34 @@ context 'profile is reset' do let(:user) { create(:profile, :verified, :password_reset).user } - it 'redirects to the redirect_uri immediately without proofing' do + it 'redirects to the redirect_uri immediately without proofing if client-side redirect is disabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(false) + IdentityLinker.new(user, service_provider).link_identity(ial: 1) user.identities.last.update!( verified_attributes: %w[given_name family_name birthdate verified_at], ) action + expect(response).to redirect_to(/^#{params[:redirect_uri]}/) end + it 'renders client-side redirect to the client app immediately if client-side redirect is enabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(true) + + IdentityLinker.new(user, service_provider).link_identity(ial: 1) + user.identities.last.update!( + verified_attributes: %w[given_name family_name birthdate verified_at], + ) + + action + expect(controller).to render_template('openid_connect/shared/redirect') + expect(assigns(:oidc_redirect_uri)).to start_with(params[:redirect_uri]) + end + it 'tracks IAL1 authentication event' do stub_analytics expect(@analytics).to receive(:track_event). @@ -460,7 +547,9 @@ user.identities.last.update!(verified_attributes: %w[given_name family_name birthdate]) end - it 'redirects back to the client app with a code' do + it 'redirects back to the client app with a code if client-side redirect is disabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(false) action expect(response).to redirect_to(/^#{params[:redirect_uri]}/) @@ -470,13 +559,29 @@ expect(redirect_params[:code]).to be_present expect(redirect_params[:state]).to eq(params[:state]) end + + it 'renders a client-side redirect back to the client app with a code if it is enabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(true) + + action + + expect(controller).to render_template('openid_connect/shared/redirect') + expect(assigns(:oidc_redirect_uri)).to start_with(params[:redirect_uri]) + + redirect_params = UriService.params(assigns(:oidc_redirect_uri)) + expect(redirect_params[:code]).to be_present + expect(redirect_params[:state]).to eq(params[:state]) + end end end context 'with invalid params that do not interfere with the redirect_uri' do before { params[:prompt] = '' } - it 'redirects to the redirect_uri immediately with an invalid_request' do + it 'redirects the user with an invalid request if client-side redirect is disabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(false) action expect(response).to redirect_to(/^#{params[:redirect_uri]}/) @@ -488,6 +593,21 @@ expect(redirect_params[:state]).to eq(params[:state]) end + it 'renders client-side redirect with an invalid request if client-side redirect is enabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(true) + action + + expect(controller).to render_template('openid_connect/shared/redirect') + expect(assigns(:oidc_redirect_uri)).to start_with(params[:redirect_uri]) + + redirect_params = UriService.params(assigns(:oidc_redirect_uri)) + + expect(redirect_params[:error]).to eq('invalid_request') + expect(redirect_params[:error_description]).to be_present + expect(redirect_params[:state]).to eq(params[:state]) + end + it 'tracks the event with errors' do stub_analytics expect(@analytics).to receive(:track_event). @@ -551,11 +671,22 @@ context 'without valid acr_values' do before { params.delete(:acr_values) } - it 'handles the error and does not blow up' do + it 'handles the error and does not blow up when client-side redirect is disabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(false) action expect(response).to redirect_to(/^#{params[:redirect_uri]}/) end + + it 'handles the error and does not blow up when client-side redirect is enabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(true) + action + + expect(controller).to render_template('openid_connect/shared/redirect') + expect(assigns(:oidc_redirect_uri)).to start_with(params[:redirect_uri]) + end end context 'with a bad redirect_uri' do @@ -572,7 +703,9 @@ params[:acr_values] = Saml::Idp::Constants::IALMAX_AUTHN_CONTEXT_CLASSREF end - it 'redirect the user' do + it 'redirects the user if client-side redirect is disabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(false) action expect(response).to redirect_to(/^#{params[:redirect_uri]}/) @@ -583,6 +716,21 @@ expect(redirect_params[:error_description]).to be_present expect(redirect_params[:state]).to eq(params[:state]) end + + it 'renders a client-side redirect if client-side redirect is enabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(true) + action + + expect(controller).to render_template('openid_connect/shared/redirect') + expect(assigns(:oidc_redirect_uri)).to start_with(params[:redirect_uri]) + + redirect_params = UriService.params(assigns(:oidc_redirect_uri)) + + expect(redirect_params[:error]).to eq('invalid_request') + expect(redirect_params[:error_description]).to be_present + expect(redirect_params[:state]).to eq(params[:state]) + end end it 'redirects to SP landing page with the request_id in the params' do @@ -629,3 +777,4 @@ end end end +# rubocop:enable Layout/LineLength diff --git a/spec/controllers/openid_connect/logout_controller_spec.rb b/spec/controllers/openid_connect/logout_controller_spec.rb index 9bc34dc04e4..c3f0901ed9a 100644 --- a/spec/controllers/openid_connect/logout_controller_spec.rb +++ b/spec/controllers/openid_connect/logout_controller_spec.rb @@ -62,12 +62,23 @@ action end - it 'redirects back to the client' do + it 'redirects back to the client if client-side redirect is disabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(false) action expect(response).to redirect_to(/^#{post_logout_redirect_uri}/) end + it 'renders client-side redirect if client-side redirect is enabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(true) + action + + expect(controller).to render_template('openid_connect/shared/redirect') + expect(assigns(:oidc_redirect_uri)).to start_with(post_logout_redirect_uri) + end + it 'tracks events' do stub_analytics expect(@analytics).to receive(:track_event). @@ -173,11 +184,22 @@ end context 'user is not signed in' do - it 'redirects back with an error' do + it 'renders client-side redirect if client-side redirect is enabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(false) action expect(response).to redirect_to(/^#{post_logout_redirect_uri}/) end + + it 'redirects back to the client if client-side redirect is disabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(true) + action + + expect(controller).to render_template('openid_connect/shared/redirect') + expect(assigns(:oidc_redirect_uri)).to start_with(post_logout_redirect_uri) + end end end @@ -276,11 +298,22 @@ end context 'user is not signed in' do - it 'redirects back to client' do + it 'redirects back to the client if client-side redirect is disabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(false) action expect(response).to redirect_to(/^#{post_logout_redirect_uri}/) end + + it 'renders client-side redirect if client-side redirect is enabled' do + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(true) + action + + expect(controller).to render_template('openid_connect/shared/redirect') + expect(assigns(:oidc_redirect_uri)).to start_with(post_logout_redirect_uri) + end end end end @@ -297,16 +330,32 @@ end context 'user is signed in' do + let(:user) { create(:user) } before { stub_sign_in(user) } - it 'destroys the session' do + + it 'destroys the session and redirects to client if client-side redirect is disabled' do + expect(controller).to receive(:sign_out) + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(false) action expect(response).to redirect_to(/^#{post_logout_redirect_uri}/) end + + it 'destroys session and renders client-side redirect if enabled' do + expect(controller).to receive(:sign_out) + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(true) + action + + expect(controller).to render_template('openid_connect/shared/redirect') + expect(assigns(:oidc_redirect_uri)).to start_with(post_logout_redirect_uri) + end end context 'user is not signed in' do - it 'destroys the session' do + it 'redirects to new session path' do + expect(controller).to_not receive(:sign_out) action expect(response).to redirect_to(new_user_session_path) @@ -327,15 +376,29 @@ context 'user is signed in' do before { stub_sign_in(user) } - it 'destroys the session' do + it 'destroys the session and redirects if client-side redirect is disabled' do + expect(controller).to receive(:sign_out) + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(false) action expect(response).to redirect_to(/^#{post_logout_redirect_uri}/) end + + it 'destroys the session and renders client-side redirect if enabled' do + expect(controller).to receive(:sign_out) + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(true) + action + + expect(controller).to render_template('openid_connect/shared/redirect') + expect(assigns(:oidc_redirect_uri)).to start_with(post_logout_redirect_uri) + end end context 'user is not signed in' do - it 'destroys the session' do + it 'redirects to new session path' do + expect(controller).to_not receive(:sign_out) action expect(response).to redirect_to(new_user_session_path) @@ -489,11 +552,24 @@ end context 'user is not signed in' do - it 'redirects back to client' do + it 'redirects back to the client if client-side redirect is disabled' do + expect(controller).to receive(:sign_out) + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(false) action expect(response).to redirect_to(/^#{post_logout_redirect_uri}/) end + + it 'renders client-side redirect if client-side redirect is enabled' do + expect(controller).to receive(:sign_out) + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(true) + action + + expect(controller).to render_template('openid_connect/shared/redirect') + expect(assigns(:oidc_redirect_uri)).to start_with(post_logout_redirect_uri) + end end end @@ -510,12 +586,25 @@ context 'user is signed in' do before { stub_sign_in(user) } - it 'destroys the session' do + it 'destroys session and redirects to client if client-side redirect is disabled' do + expect(controller).to receive(:sign_out) + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(false) action expect(response).to redirect_to(/^#{post_logout_redirect_uri}/) end + it 'destroys the session and renders client-side redirect if enabled' do + expect(controller).to receive(:sign_out) + allow(IdentityConfig.store).to receive(:openid_connect_redirect_interstitial_enabled). + and_return(true) + action + + expect(controller).to render_template('openid_connect/shared/redirect') + expect(assigns(:oidc_redirect_uri)).to start_with(post_logout_redirect_uri) + end + it 'tracks events' do stub_analytics diff --git a/spec/controllers/users/rules_of_use_controller_spec.rb b/spec/controllers/users/rules_of_use_controller_spec.rb index 6769a87944f..012635be0b7 100644 --- a/spec/controllers/users/rules_of_use_controller_spec.rb +++ b/spec/controllers/users/rules_of_use_controller_spec.rb @@ -122,7 +122,10 @@ action end - it 'includes service provider URIs in form action content security policy header' do + it 'includes service provider URIs in form-action CSP header when enabled' do + allow(IdentityConfig.store).to( + receive(:openid_connect_content_security_form_action_enabled).and_return(true), + ) sp = create(:service_provider, issuer: 'example-issuer', redirect_uris: ['https://example.com']) params = { client_id: sp.issuer, @@ -138,9 +141,31 @@ request_url: "http://test.com?#{URI.encode_www_form(params)}", } action - form_action = response.request.content_security_policy.form_action - csp_array = ["'self'", 'https://example.com'] - expect(form_action).to match_array(csp_array) + expect(response.request.content_security_policy.form_action). + to match_array(["'self'", 'https://example.com']) + end + + it 'does not include service provider URIs in form-action CSP header when disabled' do + allow(IdentityConfig.store).to( + receive(:openid_connect_content_security_form_action_enabled).and_return(false), + ) + sp = create(:service_provider, issuer: 'example-issuer', redirect_uris: ['https://example.com']) + params = { + client_id: sp.issuer, + response_type: 'code', + acr_values: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, + scope: 'openid email', + redirect_uri: sp.redirect_uris.first, + state: '1234567890123456789012', + nonce: '1234567890123456789012', + } + session[:sp] = { + issuer: sp.issuer, + request_url: "http://test.com?#{URI.encode_www_form(params)}", + } + action + expect(response.request.content_security_policy.form_action). + to match_array(["'self'"]) end end diff --git a/spec/features/account_creation/multiple_browsers_spec.rb b/spec/features/account_creation/multiple_browsers_spec.rb index 7995c447d0f..73080c84394 100644 --- a/spec/features/account_creation/multiple_browsers_spec.rb +++ b/spec/features/account_creation/multiple_browsers_spec.rb @@ -3,6 +3,7 @@ RSpec.feature 'account creation across multiple browsers' do include SpAuthHelper include SamlAuthHelper + include OidcAuthHelper it_behaves_like 'creating two accounts during the same session', :oidc it_behaves_like 'creating two accounts during the same session', :saml diff --git a/spec/features/multiple_emails/sp_sign_in_spec.rb b/spec/features/multiple_emails/sp_sign_in_spec.rb index 5179fe24c74..dbae13eb376 100644 --- a/spec/features/multiple_emails/sp_sign_in_spec.rb +++ b/spec/features/multiple_emails/sp_sign_in_spec.rb @@ -2,6 +2,7 @@ RSpec.feature 'signing into an SP with multiple emails enabled' do include SamlAuthHelper + include OidcAuthHelper context 'with the email scope' do scenario 'signing in with OIDC sends the email address used to sign in' do @@ -109,7 +110,7 @@ def visit_idp_from_oidc_sp(scope:) end def fetch_oidc_id_token_info - redirect_uri = URI(current_url) + redirect_uri = URI(oidc_redirect_url) redirect_params = Rack::Utils.parse_query(redirect_uri.query).with_indifferent_access code = redirect_params[:code] diff --git a/spec/features/openid_connect/authorization_confirmation_spec.rb b/spec/features/openid_connect/authorization_confirmation_spec.rb index 86e3ec3123f..25bafb60992 100644 --- a/spec/features/openid_connect/authorization_confirmation_spec.rb +++ b/spec/features/openid_connect/authorization_confirmation_spec.rb @@ -39,7 +39,7 @@ def create_user_and_remember_device expect(page).to have_content second_email.email continue_as(second_email.email) - expect(current_url).to match('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to match('http://localhost:7654/auth/result') end it 'it allows the user to switch accounts prior to continuing to the SP' do @@ -53,7 +53,7 @@ def create_user_and_remember_device fill_in_code_with_last_phone_otp click_submit_default - expect(current_url).to match('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to match('http://localhost:7654/auth/result') end it 'does not render the confirmation screen on a return visit to the SP by default' do @@ -66,7 +66,7 @@ def create_user_and_remember_device # second visit visit_idp_from_ial1_oidc_sp - expect(current_url).to match('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to match('http://localhost:7654/auth/result') end it 'does not render an error if a user goes back after opting to switch accounts' do @@ -100,7 +100,7 @@ def create_user_and_remember_device click_agree_and_continue - expect(current_url).to match('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to match('http://localhost:7654/auth/result') expect(page.get_rack_session.keys).to include('sp') end end diff --git a/spec/features/openid_connect/openid_connect_spec.rb b/spec/features/openid_connect/openid_connect_spec.rb index ee196f1b695..ec1e52df2e3 100644 --- a/spec/features/openid_connect/openid_connect_spec.rb +++ b/spec/features/openid_connect/openid_connect_spec.rb @@ -65,7 +65,7 @@ it 'succeeds in returning back to sp with prompt select_account and prior session' do user = oidc_end_client_secret_jwt(prompt: 'select_account') oidc_end_client_secret_jwt(prompt: 'select_account', user: user, redirs_to: '/auth/result') - expect(current_url).to include('?code=') + expect(oidc_redirect_url).to include('?code=') end it 'succeeds with no prompt and no prior session like select_account' do @@ -75,7 +75,7 @@ it 'succeeds in returning back to sp with no prompt and prior session like select_account' do user = oidc_end_client_secret_jwt oidc_end_client_secret_jwt(user: user, redirs_to: '/auth/result') - expect(current_url).to include('?code=') + expect(oidc_redirect_url).to include('?code=') end it 'succeeds with prompt login and no prior session' do @@ -111,15 +111,17 @@ it 'returns invalid request with bad prompt parameter' do oidc_end_client_secret_jwt(prompt: 'aaa', redirs_to: '/auth/result') - expect(current_url).to include('?error=invalid_request') + expect(oidc_redirect_url).to include('?error=invalid_request') end it 'returns invalid request with a blank prompt parameter' do oidc_end_client_secret_jwt(prompt: '', redirs_to: '/auth/result') - expect(current_url).to include('?error=invalid_request') + expect(oidc_redirect_url).to include('?error=invalid_request') end - it 'auto-allows with a second authorization and includes redirect_uris in CSP headers' do + it 'auto-allows with a second authorization and redirect_uris in CSP headers if enabled' do + allow(IdentityConfig.store).to receive(:openid_connect_content_security_form_action_enabled). + and_return(true) client_id = 'urn:gov:gsa:openidconnect:sp:server' service_provider = build(:service_provider, issuer: client_id) user = user_with_2fa @@ -136,11 +138,37 @@ fill_in_code_with_last_phone_otp click_submit_default - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') expect(page.get_rack_session.keys).to include('sp') end - it 'auto-allows and includes redirect_uris in CSP headers after an incorrect OTP' do + it 'auto-allows with a second authorization and blank CSP headers if not enabled' do + allow(IdentityConfig.store).to receive(:openid_connect_content_security_form_action_enabled). + and_return(false) + client_id = 'urn:gov:gsa:openidconnect:sp:server' + service_provider = build(:service_provider, issuer: client_id) + user = user_with_2fa + + IdentityLinker.new(user, service_provider).link_identity + user.identities.last.update!(verified_attributes: ['email']) + + visit_idp_from_ial1_oidc_sp(client_id: client_id, prompt: 'select_account') + sign_in_user(user) + + expect(page.response_headers['Content-Security-Policy']). + to(include('form-action \'self\'')) + + fill_in_code_with_last_phone_otp + click_submit_default + + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') + expect(page.get_rack_session.keys).to include('sp') + end + + it 'auto-allows and includes redirect_uris in CSP headers if enabled after an incorrect OTP' do + allow(IdentityConfig.store).to receive(:openid_connect_content_security_form_action_enabled). + and_return(true) + client_id = 'urn:gov:gsa:openidconnect:sp:server' service_provider = build(:service_provider, issuer: client_id) user = user_with_2fa @@ -164,7 +192,38 @@ fill_in_code_with_last_phone_otp click_submit_default - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') + expect(page.get_rack_session.keys).to include('sp') + end + + it 'auto-allows and blank CSP headers if disabled after an incorrect OTP' do + allow(IdentityConfig.store).to receive(:openid_connect_content_security_form_action_enabled). + and_return(true) + + client_id = 'urn:gov:gsa:openidconnect:sp:server' + service_provider = build(:service_provider, issuer: client_id) + user = user_with_2fa + + IdentityLinker.new(user, service_provider).link_identity + user.identities.last.update!(verified_attributes: ['email']) + + visit_idp_from_ial1_oidc_sp(client_id: client_id, prompt: 'select_account') + sign_in_user(user) + + expect(page.response_headers['Content-Security-Policy']). + to(include('form-action \'self\'')) + + fill_in :code, with: 'wrong otp' + click_submit_default + + expect(page).to have_content(t('two_factor_authentication.invalid_otp')) + expect(page.response_headers['Content-Security-Policy']). + to(include('form-action \'self\'')) + + fill_in_code_with_last_phone_otp + click_submit_default + + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') expect(page.get_rack_session.keys).to include('sp') end @@ -280,6 +339,9 @@ context 'when sending client_id' do it 'logout destroys the session when confirming logout' do + allow(IdentityConfig.store).to( + receive(:openid_connect_content_security_form_action_enabled).and_return(true), + ) service_provider = ServiceProvider.find_by(issuer: 'urn:gov:gsa:openidconnect:test') sign_in_get_id_token(client_id: service_provider.issuer) @@ -527,7 +589,7 @@ verified_within: '1w', ) - redirect_params = UriService.params(current_url) + redirect_params = UriService.params(oidc_redirect_url) expect(redirect_params[:error]).to eq('invalid_request') expect(redirect_params[:error_description]). @@ -655,7 +717,7 @@ _user = sign_in_live_with_2fa(user) expect(page.html).to_not include(code_challenge) - redirect_uri = URI(current_url) + redirect_uri = URI(oidc_redirect_url) redirect_params = Rack::Utils.parse_query(redirect_uri.query).with_indifferent_access expect(redirect_uri.to_s).to start_with('gov.gsa.openidconnect.test://result') @@ -721,7 +783,7 @@ sign_in_live_with_2fa(user) continue_as(user.email) - redirect_uri2 = URI(current_url) + redirect_uri2 = URI(oidc_redirect_url) expect(redirect_uri2.to_s).to start_with('gov.gsa.openidconnect.test://result') redirect_params2 = Rack::Utils.parse_query(redirect_uri2.query).with_indifferent_access @@ -793,7 +855,7 @@ click_agree_and_continue continue_as(email) - redirect_uri = URI(current_url) + redirect_uri = URI(oidc_redirect_url) expect(redirect_uri.to_s).to start_with('gov.gsa.openidconnect.test://result') expect(page.get_rack_session.keys).to include('sp') end @@ -831,12 +893,12 @@ ) click_agree_and_continue - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') visit sign_out_url visit oidc_path sign_in_live_with_2fa(user) - redirect_uri = URI(current_url) + redirect_uri = URI(oidc_redirect_url) expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') end @@ -983,7 +1045,11 @@ def sign_in_get_token_response( proofing_steps&.call handoff_page_steps&.call - redirect_uri = URI(current_url) + redirect_uri = if current_path.start_with?('/openid_connect/') + URI(oidc_redirect_url) + else + URI(current_url) + end redirect_params = Rack::Utils.parse_query(redirect_uri.query).with_indifferent_access code = redirect_params[:code] expect(code).to be_present @@ -1026,7 +1092,7 @@ def oidc_end_client_secret_jwt(prompt: nil, user: nil, redirs_to: nil) visit_idp_from_ial2_oidc_sp(prompt: prompt, state: state, nonce: nonce, client_id: client_id) continue_as(user.email) if user if redirs_to - expect(current_path).to eq(redirs_to) + expect(URI(oidc_redirect_url).path).to eq(redirs_to) return end expect(current_path).to eq('/') @@ -1038,7 +1104,7 @@ def oidc_end_client_secret_jwt(prompt: nil, user: nil, redirs_to: nil) sign_in_live_with_2fa(user) click_agree_and_continue_optional - redirect_uri = URI(current_url) + redirect_uri = URI(oidc_redirect_url) redirect_params = Rack::Utils.parse_query(redirect_uri.query).with_indifferent_access expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') diff --git a/spec/features/openid_connect/redirect_uri_validation_spec.rb b/spec/features/openid_connect/redirect_uri_validation_spec.rb index 660c4d41217..9815881d9d1 100644 --- a/spec/features/openid_connect/redirect_uri_validation_spec.rb +++ b/spec/features/openid_connect/redirect_uri_validation_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' RSpec.describe 'redirect_uri validation' do + include OidcAuthHelper + context 'when the redirect_uri in the request does not match one that is registered' do it 'displays error instead of branded landing page' do visit_idp_from_sp_with_ial1_with_disallowed_redirect_uri @@ -148,8 +150,9 @@ click_submit_default click_agree_and_continue - redirect_host = URI.parse(current_url).host - redirect_scheme = URI.parse(current_url).scheme + redirect_uri = URI.parse(oidc_redirect_url) + redirect_host = redirect_uri.host + redirect_scheme = redirect_uri.scheme expect(redirect_host).to eq('example.com') expect(redirect_scheme).to eq('https') diff --git a/spec/features/remember_device/session_expiration_spec.rb b/spec/features/remember_device/session_expiration_spec.rb index 1fa3c1b6551..1fea732eb4d 100644 --- a/spec/features/remember_device/session_expiration_spec.rb +++ b/spec/features/remember_device/session_expiration_spec.rb @@ -2,6 +2,7 @@ RSpec.describe 'signing in with remember device and idling on the sign in page' do include SamlAuthHelper + include OidcAuthHelper it 'redirects to the OIDC SP even though session is deleted' do allow(IdentityConfig.store).to receive(:otp_delivery_blocklist_maxretry).and_return(1000) @@ -37,14 +38,11 @@ # Simulate refreshing the page with JS to avoid a CSRF error visit new_user_session_url(request_id: request_id) - expect(page.response_headers['Content-Security-Policy']). - to(include('form-action \'self\' http://localhost:7654')) - fill_in_credentials_and_submit(user.email, user.password) continue_as(user.email, user.password) - expect(current_url).to start_with('http://localhost:7654') + expect(oidc_redirect_url).to start_with('http://localhost:7654') end end end diff --git a/spec/features/reports/sp_active_users_report_spec.rb b/spec/features/reports/sp_active_users_report_spec.rb index f5586511427..caae13a6856 100644 --- a/spec/features/reports/sp_active_users_report_spec.rb +++ b/spec/features/reports/sp_active_users_report_spec.rb @@ -2,6 +2,7 @@ RSpec.feature 'sp active users report' do include SamlAuthHelper + include OidcAuthHelper include IdvHelper it 'reports a user as ial1 active for an ial1 sign in' do @@ -11,7 +12,7 @@ fill_in_code_with_last_phone_otp click_submit_default click_agree_and_continue - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') results = [{ issuer: OidcAuthHelper::OIDC_IAL1_ISSUER, app_id: nil, @@ -30,7 +31,7 @@ fill_in_code_with_last_phone_otp click_submit_default click_agree_and_continue - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') results = [{ issuer: 'urn:gov:gsa:openidconnect:sp:server', app_id: nil, diff --git a/spec/features/sign_in/banned_users_spec.rb b/spec/features/sign_in/banned_users_spec.rb index 4e333e815ea..aecb624ae3a 100644 --- a/spec/features/sign_in/banned_users_spec.rb +++ b/spec/features/sign_in/banned_users_spec.rb @@ -38,7 +38,7 @@ visit_idp_from_sp_with_ial1(:oidc) sign_in_live_with_2fa(user) click_agree_and_continue - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') end end diff --git a/spec/features/users/password_recovery_via_recovery_code_spec.rb b/spec/features/users/password_recovery_via_recovery_code_spec.rb index 76c78856de4..f4ac1cf8fa8 100644 --- a/spec/features/users/password_recovery_via_recovery_code_spec.rb +++ b/spec/features/users/password_recovery_via_recovery_code_spec.rb @@ -4,6 +4,7 @@ include PersonalKeyHelper include IdvStepHelper include SamlAuthHelper + include OidcAuthHelper include SpAuthHelper let(:user) { create(:user, :fully_registered) } @@ -107,7 +108,7 @@ click_agree_and_continue - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') end end diff --git a/spec/features/users/sign_in_spec.rb b/spec/features/users/sign_in_spec.rb index 86280070532..461a8303e3c 100644 --- a/spec/features/users/sign_in_spec.rb +++ b/spec/features/users/sign_in_spec.rb @@ -5,6 +5,7 @@ include ActionView::Helpers::DateHelper include PersonalKeyHelper include SamlAuthHelper + include OidcAuthHelper include SpAuthHelper include IdvHelper include DocAuthHelper @@ -94,7 +95,7 @@ expect(current_url).to eq rules_of_use_url accept_rules_of_use_and_continue_if_displayed - expect(current_url).to start_with service_provider.redirect_uris.first + expect(oidc_redirect_url).to start_with service_provider.redirect_uris.first end scenario 'user with old terms of use can accept and continue to IAL2 SP' do @@ -128,7 +129,7 @@ expect(current_url).to eq rules_of_use_url accept_rules_of_use_and_continue_if_displayed - expect(current_url).to start_with service_provider.redirect_uris.first + expect(oidc_redirect_url).to start_with service_provider.redirect_uris.first end scenario 'user opts to add piv/cac card but gets an error' do @@ -190,7 +191,7 @@ click_submit_default skip_second_mfa_prompt click_agree_and_continue - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') end scenario 'user cannot sign in with certificate none error' do @@ -721,7 +722,7 @@ click_submit_default click_agree_and_continue - redirect_uri = URI(current_url) + redirect_uri = URI(oidc_redirect_url) expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') end @@ -784,7 +785,7 @@ click_agree_and_continue - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') end it 'returns ial2 info for a verified user' do @@ -802,7 +803,7 @@ click_agree_and_continue - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') end end @@ -811,7 +812,7 @@ create(:user, :fully_registered) visit_idp_from_oidc_sp_with_ialmax - expect(page).to have_content 'The page you were looking for doesn’t exist' + expect(oidc_redirect_url).to include('error=invalid_request') end end end @@ -955,7 +956,7 @@ action_url = agree_and_continue_button.ancestor('form')[:action] agree_and_continue_button.click - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') response = page.driver.post(action_url) expect(response).to be_redirect diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index 26d0b4dcb41..8ac814fe613 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -2,6 +2,7 @@ RSpec.feature 'Sign Up' do include SamlAuthHelper + include OidcAuthHelper include DocAuthHelper include ActionView::Helpers::DateHelper @@ -315,7 +316,7 @@ def clipboard_text skip_second_mfa_prompt click_agree_and_continue - redirect_uri = URI(current_url) + redirect_uri = URI(oidc_redirect_url) expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') end diff --git a/spec/features/visitors/password_recovery_spec.rb b/spec/features/visitors/password_recovery_spec.rb index ccda0376641..80ce19fb844 100644 --- a/spec/features/visitors/password_recovery_spec.rb +++ b/spec/features/visitors/password_recovery_spec.rb @@ -4,6 +4,7 @@ include IdvHelper include PersonalKeyHelper include SamlAuthHelper + include OidcAuthHelper context 'user enters valid email in forgot password form', email: true do it 'redirects to forgot_password path and sends an email to the user' do @@ -203,7 +204,7 @@ click_submit_default click_agree_and_continue - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') end end diff --git a/spec/requests/csp_spec.rb b/spec/requests/csp_spec.rb index 5cfd4a927e9..476717e53d1 100644 --- a/spec/requests/csp_spec.rb +++ b/spec/requests/csp_spec.rb @@ -2,40 +2,92 @@ RSpec.describe 'content security policy' do context 'on endpoints that will redirect to an SP' do - it 'includes a CSP with a form action that will allow a redirect to the CSP' do - visit_password_form_with_sp - follow_redirect! + context 'when openid_connect_content_security_form_action_enabled is enabled' do + before do + allow(IdentityConfig.store).to( + receive(:openid_connect_content_security_form_action_enabled), + ).and_return(true) + end - content_security_policy = parse_content_security_policy + it 'includes a CSP with a form action that will allow a redirect to the CSP' do + visit_password_form_with_sp + follow_redirect! - expect(content_security_policy['default-src']).to eq("'self'") - expect(content_security_policy['base-uri']).to eq("'self'") - expect(content_security_policy['child-src']).to eq("'self'") - expect(content_security_policy['connect-src']).to eq("'self'") - expect(content_security_policy['font-src']).to eq("'self' data:") - expect(content_security_policy['form-action']).to eq( - "'self' http://localhost:7654 https://example.com http://www.example.com", - ) - expect(content_security_policy['img-src']).to eq( - "'self' data: login.gov https://s3.us-west-2.amazonaws.com", - ) - expect(content_security_policy['media-src']).to eq("'self'") - expect(content_security_policy['object-src']).to eq("'none'") - expect(content_security_policy['script-src']).to match( - /'self' 'unsafe-eval' 'nonce-[\w\d=\/+]+'/, - ) - expect(content_security_policy['style-src']).to eq("'self'") + content_security_policy = parse_content_security_policy + + expect(content_security_policy['default-src']).to eq("'self'") + expect(content_security_policy['base-uri']).to eq("'self'") + expect(content_security_policy['child-src']).to eq("'self'") + expect(content_security_policy['connect-src']).to eq("'self'") + expect(content_security_policy['font-src']).to eq("'self' data:") + expect(content_security_policy['form-action']).to eq( + "'self' http://localhost:7654 https://example.com http://www.example.com", + ) + expect(content_security_policy['img-src']).to eq( + "'self' data: login.gov https://s3.us-west-2.amazonaws.com", + ) + expect(content_security_policy['media-src']).to eq("'self'") + expect(content_security_policy['object-src']).to eq("'none'") + expect(content_security_policy['script-src']).to match( + /'self' 'unsafe-eval' 'nonce-[\w\d=\/+]+'/, + ) + expect(content_security_policy['style-src']).to eq("'self'") + end + + it 'uses logout SP to override CSP form action that will allow a redirect to the CSP' do + visit_password_form_with_sp + visit_logout_form_with_sp + + content_security_policy = parse_content_security_policy + + expect(content_security_policy['form-action']).to eq( + "'self' gov.gsa.openidconnect.test:", + ) + end end - it 'uses logout SP to override CSP form action that will allow a redirect to the CSP' do - visit_password_form_with_sp - visit_logout_form_with_sp + context 'when openid_connect_content_security_form_action_enabled is disabled' do + before do + allow(IdentityConfig.store).to( + receive(:openid_connect_content_security_form_action_enabled), + ).and_return(false) + end - content_security_policy = parse_content_security_policy + it 'includes a CSP without SP hosts in form-action' do + visit_password_form_with_sp + follow_redirect! - expect(content_security_policy['form-action']).to eq( - "'self' gov.gsa.openidconnect.test:", - ) + content_security_policy = parse_content_security_policy + + expect(content_security_policy['default-src']).to eq("'self'") + expect(content_security_policy['base-uri']).to eq("'self'") + expect(content_security_policy['child-src']).to eq("'self'") + expect(content_security_policy['connect-src']).to eq("'self'") + expect(content_security_policy['font-src']).to eq("'self' data:") + expect(content_security_policy['form-action']).to eq( + "'self'", + ) + expect(content_security_policy['img-src']).to eq( + "'self' data: login.gov https://s3.us-west-2.amazonaws.com", + ) + expect(content_security_policy['media-src']).to eq("'self'") + expect(content_security_policy['object-src']).to eq("'none'") + expect(content_security_policy['script-src']).to match( + /'self' 'unsafe-eval' 'nonce-[\w\d=\/+]+'/, + ) + expect(content_security_policy['style-src']).to eq("'self'") + end + + it 'uses logout SP to override CSP form action that will allow a redirect to the CSP' do + visit_password_form_with_sp + visit_logout_form_with_sp + + content_security_policy = parse_content_security_policy + + expect(content_security_policy['form-action']).to eq( + "'self'", + ) + end end end diff --git a/spec/support/oidc_auth_helper.rb b/spec/support/oidc_auth_helper.rb index 36181e7812c..2f9fce8446a 100644 --- a/spec/support/oidc_auth_helper.rb +++ b/spec/support/oidc_auth_helper.rb @@ -103,4 +103,23 @@ def include_aal3(params) params[:acr_values] = "#{params[:acr_values]} " + Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF end + + # We rely on client-side redirects in some cases using: + # + # This method checks that the url contains the right url + def extract_meta_refresh_url + content = page.find("meta[http-equiv='refresh']", visible: false)['content'] + timeout, url_value = content.split(';') + expect(timeout).to eq '0' + _, url = url_value.split('url=') + url + end + + def oidc_redirect_url + if IdentityConfig.store.openid_connect_redirect_interstitial_enabled + extract_meta_refresh_url + else + current_url + end + end end diff --git a/spec/support/shared_examples/account_creation.rb b/spec/support/shared_examples/account_creation.rb index 5ad098ed863..38625da6863 100644 --- a/spec/support/shared_examples/account_creation.rb +++ b/spec/support/shared_examples/account_creation.rb @@ -1,5 +1,7 @@ RSpec.shared_examples 'creating an account with the site in Spanish' do |sp| - it 'redirects to the SP', email: true do + it 'redirects to the SP with SP URIs in form-action CSP if enabled', email: true do + allow(IdentityConfig.store).to receive(:openid_connect_content_security_form_action_enabled). + and_return(true) Capybara.current_session.driver.header('Accept-Language', 'es') visit_idp_from_sp_with_ial1(sp) register_user @@ -13,7 +15,29 @@ if :sp == :saml expect(current_url).to eq UriService.add_params(@saml_authn_request, locale: :es) elsif sp == :oidc - redirect_uri = URI(current_url) + redirect_uri = URI(oidc_redirect_url) + + expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') + end + end + + it 'redirects to the SP without SP URIs in form-action CSP if disabled', email: true do + allow(IdentityConfig.store).to receive(:openid_connect_content_security_form_action_enabled). + and_return(false) + Capybara.current_session.driver.header('Accept-Language', 'es') + visit_idp_from_sp_with_ial1(sp) + register_user + + if sp == :oidc + expect(page.response_headers['Content-Security-Policy']). + to(include('form-action \'self\'')) + end + + click_agree_and_continue + if :sp == :saml + expect(current_url).to eq UriService.add_params(@saml_authn_request, locale: :es) + elsif sp == :oidc + redirect_uri = URI(oidc_redirect_url) expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') end @@ -21,7 +45,9 @@ end RSpec.shared_examples 'creating an account using authenticator app for 2FA' do |sp| - it 'redirects to the SP', email: true do + it 'redirects to the SP with SP URIs in form-action CSP if enabled', email: true do + allow(IdentityConfig.store).to receive(:openid_connect_content_security_form_action_enabled). + and_return(true) visit_idp_from_sp_with_ial1(sp) register_user_with_authenticator_app @@ -34,7 +60,28 @@ expect(current_url).to eq complete_saml_url if sp == :saml if sp == :oidc - redirect_uri = URI(current_url) + redirect_uri = URI(oidc_redirect_url) + + expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') + end + end + + it 'redirects to the SP without SP URIs in form-action CSP if disabled', email: true do + allow(IdentityConfig.store).to receive(:openid_connect_content_security_form_action_enabled). + and_return(false) + visit_idp_from_sp_with_ial1(sp) + register_user_with_authenticator_app + + if sp == :oidc + expect(page.response_headers['Content-Security-Policy']). + to(include('form-action \'self\'')) + end + + click_agree_and_continue + expect(current_url).to eq complete_saml_url if sp == :saml + + if sp == :oidc + redirect_uri = URI(oidc_redirect_url) expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') end @@ -67,7 +114,9 @@ end RSpec.shared_examples 'creating an account using PIV/CAC for 2FA' do |sp| - it 'redirects to the SP', email: true do + it 'redirects to the SP with SP URIs in form-action CSP if enabled', email: true do + allow(IdentityConfig.store).to receive(:openid_connect_content_security_form_action_enabled). + and_return(true) visit_idp_from_sp_with_ial1(sp) register_user_with_piv_cac @@ -80,7 +129,28 @@ expect(current_url).to eq complete_saml_url if sp == :saml if sp == :oidc - redirect_uri = URI(current_url) + redirect_uri = URI(oidc_redirect_url) + + expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') + end + end + + it 'redirects to the SP without SP URIs in form-action CSP if disabled', email: true do + allow(IdentityConfig.store).to receive(:openid_connect_content_security_form_action_enabled). + and_return(false) + visit_idp_from_sp_with_ial1(sp) + register_user_with_piv_cac + + if sp == :oidc + expect(page.response_headers['Content-Security-Policy']). + to(include('form-action \'self\'')) + end + + click_agree_and_continue + expect(current_url).to eq complete_saml_url if sp == :saml + + if sp == :oidc + redirect_uri = URI(oidc_redirect_url) expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') end @@ -135,7 +205,7 @@ expect(current_url).to eq complete_saml_url if sp == :saml if sp == :oidc - redirect_uri = URI(current_url) + redirect_uri = URI(oidc_redirect_url) expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') end @@ -156,7 +226,7 @@ expect(current_url).to eq complete_saml_url if sp == :saml if sp == :oidc - redirect_uri = URI(current_url) + redirect_uri = URI(oidc_redirect_url) expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') end diff --git a/spec/support/shared_examples/remember_device.rb b/spec/support/shared_examples/remember_device.rb index 6a4948935a8..79719d93e92 100644 --- a/spec/support/shared_examples/remember_device.rb +++ b/spec/support/shared_examples/remember_device.rb @@ -1,4 +1,6 @@ RSpec.shared_examples 'remember device' do + include OidcAuthHelper + it 'does not require 2FA on sign in' do user = remember_device_and_sign_out_user sign_in_user(user) @@ -56,12 +58,9 @@ visit oidc_url - expect(page.response_headers['Content-Security-Policy']). - to(include('form-action \'self\' http://localhost:7654')) - sign_in_user(user) click_continue - expect(current_url).to start_with('http://localhost:7654/auth/result') + expect(oidc_redirect_url).to start_with('http://localhost:7654/auth/result') end def expect_mfa_to_be_required_for_user(user) diff --git a/spec/support/shared_examples/sign_in.rb b/spec/support/shared_examples/sign_in.rb index 4dfddbf1964..fd875aae6ae 100644 --- a/spec/support/shared_examples/sign_in.rb +++ b/spec/support/shared_examples/sign_in.rb @@ -7,11 +7,6 @@ fill_in_credentials_and_submit(user.email, user.password) continue_as(user.email) - if sp == :oidc - expect(page.response_headers['Content-Security-Policy']). - to(include('form-action \'self\' http://localhost:7654')) - end - fill_in_code_with_last_phone_otp sp == :saml ? click_submit_default_twice : click_submit_default @@ -22,7 +17,7 @@ if sp == :saml expect(current_url).to eq UriService.add_params(complete_saml_url, locale: 'es') elsif sp == :oidc - redirect_uri = URI(current_url) + redirect_uri = URI(oidc_redirect_url) expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') end @@ -50,6 +45,8 @@ end RSpec.shared_examples 'visiting 2fa when fully authenticated' do |sp| + include OidcAuthHelper + it 'redirects to SP after visiting a 2fa screen when fully authenticated', email: true do ial1_sign_in_with_personal_key_goes_to_sp(sp) @@ -60,7 +57,7 @@ expect(current_url).to eq complete_saml_url if sp == :saml if sp == :oidc - redirect_uri = URI(current_url) + redirect_uri = URI(oidc_redirect_url) expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') end @@ -120,7 +117,7 @@ expect(current_url).to eq complete_saml_url if sp == :saml if sp == :oidc - redirect_uri = URI(current_url) + redirect_uri = URI(oidc_redirect_url) expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') end @@ -156,7 +153,7 @@ expect(current_url).to eq complete_saml_url if sp == :saml if sp == :oidc - redirect_uri = URI(current_url) + redirect_uri = URI(oidc_redirect_url) expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') end @@ -311,7 +308,7 @@ def ial1_sign_in_with_personal_key_goes_to_sp(sp) return unless sp == :oidc - redirect_uri = URI(current_url) + redirect_uri = URI(oidc_redirect_url) expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') end @@ -326,7 +323,7 @@ def ial1_sign_in_with_piv_cac_goes_to_sp(sp) click_submit_default if sp == :saml click_agree_and_continue return unless sp == :oidc - redirect_uri = URI(current_url) + redirect_uri = URI(oidc_redirect_url) expect(redirect_uri.to_s).to start_with('http://localhost:7654/auth/result') end