Lg 2441 redirect URIs not included in CSP for oidc

app/controllers/concerns/secure_headers_concern.rb

@@ -4,19 +4,32 @@ module SecureHeadersConcern
   def apply_secure_headers_override
     return if stored_url_for_user.blank?
 
-    authorize_params = URIService.params(stored_url_for_user)
     authorize_form = OpenidConnectAuthorizeForm.new(authorize_params)
 
     return unless authorize_form.valid?
 
-    redirect_uri = authorize_params[:redirect_uri]
+    override_csp_with_uris
+  end
 
+  def override_csp_with_uris
     override_content_security_policy_directives(
-      form_action: ["'self'", redirect_uri].compact,
+      form_action: csp_uris,
       preserve_schemes: true,
     )
   end
 
+  def csp_uris
+    # Returns fully formed CSP array w/"'self'" and redirect_uris
+    SecureHeadersWhitelister.csp_with_sp_redirect_uris(
+      authorize_params[:redirect_uri],
+      decorated_session.sp_redirect_uris,
+    )
+  end
+
+  def authorize_params
+    URIService.params(stored_url_for_user)
+  end
+
   private
 
   def stored_url_for_user

app/controllers/openid_connect/authorization_controller.rb

@@ -1,15 +1,15 @@
 module OpenidConnect
-  # rubocop:disable Metrics/ClassLength
   class AuthorizationController < ApplicationController
     include FullyAuthenticatable
     include RememberDeviceConcern
     include VerifyProfileConcern
+    include SecureHeadersConcern
 
     before_action :build_authorize_form_from_params, only: [:index]
     before_action :validate_authorize_form, only: [:index]
     before_action :sign_out_if_prompt_param_is_login_and_user_is_signed_in, only: [:index]
     before_action :store_request, only: [:index]
-    before_action :apply_secure_headers_override, only: [:index]
+    before_action :override_csp_with_uris, only: [:index]
     before_action :confirm_user_is_authenticated_with_fresh_mfa, only: :index
     before_action :prompt_for_password_if_ial2_request_and_pii_locked, only: [:index]
 
@@ -68,13 +68,6 @@ def track_authorize_analytics(result)
       )
     end
 
-    def apply_secure_headers_override
-      override_content_security_policy_directives(
-        form_action: ["'self'", authorization_params[:redirect_uri]].compact,
-        preserve_schemes: true,
-      )
-    end
-
     def identity_needs_verification?
       @authorize_form.ial2_requested? && current_user.decorate.identity_not_verified?
     end
@@ -127,5 +120,4 @@ def pii_requested_but_locked?
         user_session[:decrypted_pii].blank?
     end
   end
-  # rubocop:enable Metrics/ClassLength
 end

app/controllers/openid_connect/logout_controller.rb

@@ -1,5 +1,9 @@
 module OpenidConnect
   class LogoutController < ApplicationController
+    include SecureHeadersConcern
+
+    before_action :apply_secure_headers_override, only: [:index]
+
     def index
       @logout_form = OpenidConnectLogoutForm.new(params)
 
@@ -9,7 +13,7 @@ def index
 
       if (redirect_uri = result.extra[:redirect_uri])
         sign_out
-        redirect_to redirect_uri
+        redirect_to redirect_uri unless params['prevent_logout_redirect'] == 'true'
       else
         render :error
       end

spec/features/openid_connect/openid_connect_spec.rb

@@ -56,7 +56,7 @@
       expect(current_url).to include('?error=invalid_request')
     end
 
-    it 'auto-allows with a second authorization and sets the correct CSP headers' do
+    it 'auto-allows with a second authorization and includes redirect_uris in CSP headers' do
       client_id = 'urn:gov:gsa:openidconnect:sp:server'
       user = user_with_2fa
 
@@ -77,7 +77,7 @@
       sign_in_user(user)
 
       expect(page.response_headers['Content-Security-Policy']).
-        to(include('form-action \'self\' http://localhost:7654'))
+        to(include('form-action \'self\' http://localhost:7654/auth/result https://example.com'))
 
       fill_in_code_with_last_phone_otp
       click_submit_default
@@ -86,7 +86,7 @@
       expect(page.get_rack_session.keys).to include('sp')
     end
 
-    it 'auto-allows and sets the correct CSP headers after an incorrect OTP' do
+    it 'auto-allows and includes redirect_uris in CSP headers after an incorrect OTP' do
       client_id = 'urn:gov:gsa:openidconnect:sp:server'
       user = user_with_2fa
 
@@ -107,14 +107,14 @@
       sign_in_user(user)
 
       expect(page.response_headers['Content-Security-Policy']).
-        to(include('form-action \'self\' http://localhost:7654'))
+        to(include('form-action \'self\' http://localhost:7654/auth/result https://example.com'))
 
       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\' http://localhost:7654'))
+        to(include('form-action \'self\' http://localhost:7654/auth/result https://example.com'))
 
       fill_in_code_with_last_phone_otp
       click_submit_default
@@ -124,6 +124,33 @@
     end
   end
 
+  it 'logout includes redirect_uris in CSP headers and destroys the session' do
+    id_token = sign_in_get_id_token
+
+    state = SecureRandom.hex
+
+    visit openid_connect_logout_path(
+      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://result '\
+      'gov.gsa.openidconnect.test://result/signout',
+    )
+
+    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'))
+  end
+
   context 'with PCKE', driver: :mobile_rack_test do
     it 'succeeds with client authentication via PKCE' do
       client_id = 'urn:gov:gsa:openidconnect:test'

spec/features/saml/saml_logout_spec.rb

@@ -27,12 +27,11 @@
         request = OneLogin::RubySaml::Logoutrequest.new
         visit request.create(settings)
 
-        csp_uris = page.all('input[name="csp_uris"]', visible: false).first.value
-
         # contains all redirect_uris in content security policy
-        expect(csp_uris).to have_content('http://example.com/')
-        expect(csp_uris).to have_content('http://example.com/auth/result')
-        expect(csp_uris).to have_content('http://example.com/logout')
+        expect(page.response_headers['Content-Security-Policy']).to include(
+          'form-action \'self\' example.com example.com/ example.com/auth/result '\
+          'example.com/logout',
+        )
       end
 
       it 'contains all redirect_uris in CSP when user is logged in to the IDP' do
@@ -48,12 +47,11 @@
         request = OneLogin::RubySaml::Logoutrequest.new
         visit request.create(settings)
 
-        csp_uris = page.all('input[name="csp_uris"]', visible: false).first.value
-
         # contains all redirect_uris in content security policy
-        expect(csp_uris).to have_content('http://example.com/')
-        expect(csp_uris).to have_content('http://example.com/auth/result')
-        expect(csp_uris).to have_content('http://example.com/logout')
+        expect(page.response_headers['Content-Security-Policy']).to include(
+          'form-action \'self\' example.com example.com/ example.com/auth/result '\
+          'example.com/logout',
+        )
       end
     end