Refactor the CSP allow list tooling to better reduce CSP and preserve schemes

app/controllers/saml_idp_controller.rb

@@ -113,11 +113,9 @@ def handle_successful_handoff
   end
 
   def render_template_for(message, action_url, type)
-    domain = SecureHeadersAllowList.extract_domain(action_url)
-
     # Returns fully formed CSP array w/"'self'", domain, and ServiceProvider#redirect_uris
     csp_uris = SecureHeadersAllowList.csp_with_sp_redirect_uris(
-      domain, decorated_session.sp_redirect_uris
+      action_url, decorated_session.sp_redirect_uris
     )
     override_content_security_policy_directives(form_action: csp_uris)
 

app/services/secure_headers_allow_list.rb

@@ -1,13 +1,53 @@
 class SecureHeadersAllowList
-  def self.extract_domain(url)
-    url.split('//')[1].split('/')[0]
+  def self.csp_with_sp_redirect_uris(action_url_domain, sp_redirect_uris)
+    ["'self'"] + reduce_sp_redirect_uris_for_csp([action_url_domain, *sp_redirect_uris].compact)
   end
 
-  def self.csp_with_sp_redirect_uris(action_url_domain, sp_redirect_uris)
-    csp_uris = ["'self'", action_url_domain]
+  ##
+  # This method reduces a list of URIs into a reasonable list for the form
+  # action CSP directive on a page that may redirect to a service provider.
+  #
+  # It is necessary to include a list of destinations because some browsers
+  # enforce the form action directive on redirects. For example, an SP may have
+  # a redirect uri of `https://auth.example.com/result` that then redirects to
+  # `https://app.example.com/`. The redirect will violate the IDP's CSP if
+  # `https://app.example.com` is not included as a form action destination.
+  #
+  # Validations on the service provider ensure there will be 2 types of URLs
+  # this method must handle:
+  # - Web URLs
+  # - Native app URLs
+  #
+  # Web URLs need to appear in the form action directive with a scheme, host,
+  # and port. As such, `https://example.com/auth/result` and
+  # `https://example.com/other/path` can be reduced to `https://example.com`.
+  #
+  # Native app URLs are for deep links to mobile applications. They will have
+  # custom schemes and simple hostnames, such as `mymobileapp://result`. These
+  # URLs need to be reduced to allow the scheme, but the host information will
+  # need to be removed or some browsers will reject them. In the
+  # `mymobileapp://result` example the URI must be reduced to `mymobileapp://`.
+  #
+  def self.reduce_sp_redirect_uris_for_csp(uris)
+    csp_uri_set = uris.each_with_object(Set.new) do |uri, uri_set|
+      parsed_uri = URI.parse(uri)
+      reduced_uri = if parsed_uri.scheme.match?(/\Ahttps?\z/)
+                         reduce_web_sp_uri(parsed_uri)
+                       else
+                         reduce_native_app_sp_uri(parsed_uri)
+                       end
+      uri_set.add(reduced_uri)
+    end
+    csp_uri_set.to_a
+  end
 
-    csp_uris |= sp_redirect_uris.compact if sp_redirect_uris.present?
+  def self.reduce_web_sp_uri(uri)
+    uri.path = ''
+    uri.query = nil
+    uri.to_s
+  end
 
-    csp_uris
+  def self.reduce_native_app_sp_uri(uri)
+    "#{uri.scheme}://"
   end
 end

config/initializers/secure_headers.rb

@@ -35,6 +35,7 @@
     ],
     style_src: ["'self'", IdentityConfig.store.asset_host.presence],
     base_uri: ["'self'"],
+    preserve_schemes: true,
     disable_nonce_backwards_compatibility: IdentityConfig.store.disable_csp_unsafe_inline,
   }
 

spec/controllers/saml_idp_controller_spec.rb

@@ -1214,7 +1214,7 @@ def name_id_version(format_urn)
 
       it 'sets correct CSP config that includes any custom app scheme uri from SP redirect_uris' do
         form_action = response.request.headers.env['secure_headers_request_config'].csp.form_action
-        csp_array = ["'self'", 'localhost:3000', 'x-example-app://idp_return']
+        csp_array = ["'self'", 'http://localhost:3000', 'x-example-app://']
         expect(form_action).to match_array(csp_array)
       end
 

spec/features/openid_connect/openid_connect_spec.rb

@@ -96,7 +96,7 @@
       sign_in_user(user)
 
       expect(page.response_headers['Content-Security-Policy']).
-        to(include('form-action \'self\' http://localhost:7654/auth/result https://example.com'))
+        to(include('form-action \'self\' http://localhost:7654 https://example.com'))
 
       fill_in_code_with_last_phone_otp
       click_submit_default
@@ -116,14 +116,14 @@
       sign_in_user(user)
 
       expect(page.response_headers['Content-Security-Policy']).
-        to(include('form-action \'self\' http://localhost:7654/auth/result https://example.com'))
+        to(include('form-action \'self\' http://localhost:7654 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/auth/result https://example.com'))
+        to(include('form-action \'self\' http://localhost:7654 https://example.com'))
 
       fill_in_code_with_last_phone_otp
       click_submit_default
@@ -151,8 +151,7 @@
     )
 
     expect(page.response_headers['Content-Security-Policy']).to include(
-      'form-action \'self\' gov.gsa.openidconnect.test://result '\
-      'gov.gsa.openidconnect.test://result/signout',
+      'form-action \'self\' gov.gsa.openidconnect.test://',
     )
 
     visit account_path

spec/features/remember_device/session_expiration_spec.rb

@@ -33,13 +33,13 @@
       visit new_user_session_url(request_id: request_id)
 
       expect(page.response_headers['Content-Security-Policy']).
-        to(include('form-action \'self\' http://localhost:7654/auth/result'))
+        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/auth/result')
+      expect(current_url).to start_with('http://localhost:7654')
     end
   end
 end

spec/features/saml/saml_logout_spec.rb

@@ -30,8 +30,7 @@
 
         # contains all redirect_uris in content security policy
         expect(page.response_headers['Content-Security-Policy']).to include(
-          'form-action \'self\' example.com example.com/ example.com/auth/result '\
-          'example.com/logout',
+          "form-action 'self' http://example.com",
         )
       end
 
@@ -53,8 +52,7 @@
 
         # contains all redirect_uris in content security policy
         expect(page.response_headers['Content-Security-Policy']).to include(
-          'form-action \'self\' example.com example.com/ example.com/auth/result '\
-          'example.com/logout',
+          "form-action 'self' http://example.com",
         )
       end
     end

spec/features/users/sign_in_spec.rb

@@ -101,8 +101,7 @@
 
     expected_form_action = <<-STR.squish
       form-action https://*.pivcac.test.example.com 'self'
-      http://localhost:7654/auth/result https://example.com
-      http://www.example.com/test/oidc;
+      http://localhost:7654 https://example.com
     STR
 
     expect(page.response_headers['Content-Security-Policy']).

spec/requests/csp_spec.rb

@@ -16,7 +16,7 @@
       )
       expect(content_security_policy['font-src']).to eq("'self' data:")
       expect(content_security_policy['form-action']).to eq(
-        "'self' http://localhost:7654/auth/result https://example.com http://www.example.com/test/oidc",
+        "'self' http://localhost:7654 https://example.com http://www.example.com",
       )
       expect(content_security_policy['img-src']).to eq(
         "'self' data: login.gov idscangoweb.acuant.com https://s3.us-west-2.amazonaws.com",
@@ -45,7 +45,7 @@
       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 idscangoweb.acuant.com s3.us-west-2.amazonaws.com",
+        "'self' data: login.gov idscangoweb.acuant.com 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'")

spec/services/secure_headers_allow_list_spec.rb

@@ -1,31 +1,16 @@
 require 'rails_helper'
 
 RSpec.describe SecureHeadersAllowList do
-  describe '.extract_domain' do
-    def extract_domain(url)
-      SecureHeadersAllowList.extract_domain(url)
-    end
-
-    it 'extracts the domain and port from a url' do
-      aggregate_failures do
-        expect(extract_domain('http://localhost:1234/foo/bar')).to eq('localhost:1234')
-        expect(extract_domain('https://example.com')).to eq('example.com')
-        expect(extract_domain('https://example.com/test')).to eq('example.com')
-        expect(extract_domain('https://example.com:1234')).to eq('example.com:1234')
-      end
-    end
-  end
-
   describe '.csp_with_sp_redirect_uris' do
     def csp_with_sp_redirect_uris(domain, sp_redirect_uris)
       SecureHeadersAllowList.csp_with_sp_redirect_uris(domain, sp_redirect_uris)
     end
 
     it 'generates the proper CSP array from action_url domain and ServiceProvider#redirect_uris' do
       aggregate_failures do
-        domain = 'example1.com'
+        domain = 'https://example1.com'
         test_sp_uris = ['x-example-app://test', 'https://example2.com']
-        full_return = ["'self'", 'example1.com', 'x-example-app://test', 'https://example2.com']
+        full_return = ["'self'", 'https://example1.com', 'x-example-app://', 'https://example2.com']
 
         expect(csp_with_sp_redirect_uris(domain, test_sp_uris)).to eq(full_return)
 
@@ -35,5 +20,62 @@ def csp_with_sp_redirect_uris(domain, sp_redirect_uris)
         expect(csp_with_sp_redirect_uris(domain, nil)).to eq(full_return[0..1])
       end
     end
+
+    it 'properly reduces web uris' do
+      redirect_uri = 'https://example1.com/auth/result'
+      allowed_redirect_uris = [
+        'https://example1.com/auth/result',
+        'https://example1.com/',
+        'http://example2.com/',
+        'https://example3.com:3000/',
+      ]
+
+      result = csp_with_sp_redirect_uris(redirect_uri, allowed_redirect_uris)
+
+      expect(result).to match_array(
+        ["'self'", 'https://example1.com', 'http://example2.com', 'https://example3.com:3000'],
+      )
+    end
+
+    it 'properly reduces mobile uris' do
+      redirect_uri = 'mymobileapp://result'
+      allowed_redirect_uris = [
+        'mymobileapp://result',
+        'mymobileapp://result2',
+        'myothermobileapp://result',
+        'https://example.com/',
+      ]
+
+      result = csp_with_sp_redirect_uris(redirect_uri, allowed_redirect_uris)
+
+      expect(result).to match_array(
+        ["'self'", 'mymobileapp://', 'myothermobileapp://', 'https://example.com'],
+      )
+    end
+
+    it 'handles nil sp_redirect_uris' do
+      redirect_uri = 'https://example.com/auth/result'
+
+      result = csp_with_sp_redirect_uris(redirect_uri, nil)
+
+      expect(result).to match_array(
+        ["'self'", 'https://example.com'],
+      )
+    end
+
+    it 'handles sp_redirect_uris with nil elements' do
+      redirect_uri = 'https://example1.com/auth/result'
+      allowed_redirect_uris = [
+        'https://example1.com/auth/result',
+        nil,
+        'http://example2.com/',
+      ]
+
+      result = csp_with_sp_redirect_uris(redirect_uri, allowed_redirect_uris)
+
+      expect(result).to match_array(
+        ["'self'", 'https://example1.com', 'http://example2.com'],
+      )
+    end
   end
 end