aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAndrew White <pixeltrix@users.noreply.github.com>2018-10-23 07:35:51 +0100
committerGitHub <noreply@github.com>2018-10-23 07:35:51 +0100
commit759b3af0c6fc9fb502031a05b281736602ff8e5f (patch)
tree97962a0b7a59310e2299da20dd94c5d39a1b74d1
parentc2f8df67f34e233ff3f7f058d492217c5ad3eff1 (diff)
parenta150a026591b7b9dcaba5a2ef5fce02f7d990aba (diff)
downloadrails-759b3af0c6fc9fb502031a05b281736602ff8e5f.tar.gz
rails-759b3af0c6fc9fb502031a05b281736602ff8e5f.tar.bz2
rails-759b3af0c6fc9fb502031a05b281736602ff8e5f.zip
Merge pull request #34286 from rails/fix-csp-dynamic-sources
Fix CSP dynamic sources
-rw-r--r--actionpack/CHANGELOG.md27
-rw-r--r--actionpack/lib/action_dispatch/http/content_security_policy.rb6
-rw-r--r--actionpack/test/dispatch/content_security_policy_test.rb14
3 files changed, 41 insertions, 6 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 3858c211ea..5554d4e6b8 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,30 @@
+* Use request object for context if there's no controller
+
+ There is no controller instance when using a redirect route or a
+ mounted rack application so pass the request object as the context
+ when resolving dynamic CSP sources in this scenario.
+
+ Fixes #34200.
+
+ *Andrew White*
+
+* Apply mapping to symbols returned from dynamic CSP sources
+
+ Previously if a dynamic source returned a symbol such as :self it
+ would be converted to a string implicity, e.g:
+
+ policy.default_src -> { :self }
+
+ would generate the header:
+
+ Content-Security-Policy: default-src self
+
+ and now it generates:
+
+ Content-Security-Policy: default-src 'self'
+
+ *Andrew White*
+
* Add `ActionController::Parameters#each_value`.
*Lukáš Zapletal*
diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb
index 50953e32b5..b1e5a28be5 100644
--- a/actionpack/lib/action_dispatch/http/content_security_policy.rb
+++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb
@@ -22,7 +22,8 @@ module ActionDispatch #:nodoc:
if policy = request.content_security_policy
nonce = request.content_security_policy_nonce
- headers[header_name(request)] = policy.build(request.controller_instance, nonce)
+ context = request.controller_instance || request
+ headers[header_name(request)] = policy.build(context, nonce)
end
response
@@ -257,7 +258,8 @@ module ActionDispatch #:nodoc:
if context.nil?
raise RuntimeError, "Missing context for the dynamic content security policy source: #{source.inspect}"
else
- context.instance_exec(&source)
+ resolved = context.instance_exec(&source)
+ resolved.is_a?(Symbol) ? apply_mapping(resolved) : resolved
end
else
raise RuntimeError, "Unexpected content security policy source: #{source.inspect}"
diff --git a/actionpack/test/dispatch/content_security_policy_test.rb b/actionpack/test/dispatch/content_security_policy_test.rb
index 13ad22b5c5..c8c885f35c 100644
--- a/actionpack/test/dispatch/content_security_policy_test.rb
+++ b/actionpack/test/dispatch/content_security_policy_test.rb
@@ -260,12 +260,13 @@ class DefaultContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationT
ROUTES.draw do
scope module: "default_content_security_policy_integration_test" do
get "/", to: "policy#index"
+ get "/redirect", to: redirect("/")
end
end
POLICY = ActionDispatch::ContentSecurityPolicy.new do |p|
- p.default_src :self
- p.script_src :https
+ p.default_src -> { :self }
+ p.script_src -> { :https }
end
class PolicyConfigMiddleware
@@ -295,14 +296,19 @@ class DefaultContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationT
def test_adds_nonce_to_script_src_content_security_policy_only_once
get "/"
get "/"
+ assert_response :success
+ assert_policy "default-src 'self'; script-src https: 'nonce-iyhD0Yc0W+c='"
+ end
+
+ def test_redirect_works_with_dynamic_sources
+ get "/redirect"
+ assert_response :redirect
assert_policy "default-src 'self'; script-src https: 'nonce-iyhD0Yc0W+c='"
end
private
def assert_policy(expected, report_only: false)
- assert_response :success
-
if report_only
expected_header = "Content-Security-Policy-Report-Only"
unexpected_header = "Content-Security-Policy"