Skip to content

Commit 0cce2ec

Browse files
committed
Hide default config behind a private class method
We don't expect public callers to actually access the default config directly. So, we hide it behind a private class method. There is only one caller in the entire codebase that is not a test. It does make tests a bit uglier, but is probably worth it for the benefits of hiding it from the public API.
1 parent b405ba3 commit 0cce2ec

File tree

6 files changed

+52
-17
lines changed

6 files changed

+52
-17
lines changed

lib/secure_headers.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ def content_security_policy_style_nonce(request)
195195
# Falls back to the global config
196196
def config_for(request, prevent_dup = false)
197197
config = request.env[SECURE_HEADERS_CONFIG] ||
198-
Configuration.default
198+
Configuration.send(:default_config)
199199

200200

201201
# Global configs are frozen, per-request configs are not. When we're not

lib/secure_headers/configuration.rb

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ class Configuration
66
DEFAULT_CONFIG = :default
77
NOOP_OVERRIDE = "secure_headers_noop_override"
88
class AlreadyConfiguredError < StandardError; end
9+
class NotYetConfiguredError < StandardError; end
910
class IllegalPolicyModificationError < StandardError; end
1011
class << self
1112
# Public: Set the global default configuration.
@@ -14,14 +15,6 @@ class << self
1415
#
1516
# Returns the newly created config.
1617
def default(&block)
17-
if defined?(@default_config) && !block_given?
18-
return @default_config
19-
else
20-
configure(&block)
21-
end
22-
end
23-
24-
def configure(&block)
2518
if defined?(@default_config)
2619
raise AlreadyConfiguredError, "Policy already configured"
2720
end
@@ -38,6 +31,7 @@ def configure(&block)
3831
new_config.validate_config!
3932
@default_config = new_config
4033
end
34+
alias_method :configure, :default
4135

4236
# Public: create a named configuration that overrides the default config.
4337
#
@@ -83,6 +77,17 @@ def deep_copy(config)
8377
end
8478
end
8579

80+
# Private: Returns the internal default configuration. This should only
81+
# ever be called by internal callers (or tests) that know the semantics
82+
# of ensuring that the default config is never mutated and is dup(ed)
83+
# before it is used in a request.
84+
def default_config
85+
unless defined?(@default_config)
86+
raise NotYetConfiguredError, "Default policy not yet configured"
87+
end
88+
@default_config
89+
end
90+
8691
# Private: convenience method purely DRY things up. The value may not be a
8792
# hash (e.g. OPT_OUT, nil)
8893
def deep_copy_if_hash(value)

spec/lib/secure_headers/configuration_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ module SecureHeaders
4141
config.cookies = OPT_OUT
4242
end
4343

44-
config = Configuration.default
44+
config = Configuration.send(:default_config)
4545
expect(config.cookies).to eq(OPT_OUT)
4646
end
4747

@@ -50,7 +50,7 @@ module SecureHeaders
5050
config.cookies = {httponly: true, secure: true, samesite: {lax: false}}
5151
end
5252

53-
config = Configuration.default
53+
config = Configuration.send(:default_config)
5454
expect(config.cookies).to eq({httponly: true, secure: true, samesite: {lax: false}})
5555
end
5656
end

spec/lib/secure_headers/headers/policy_management_spec.rb

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ module SecureHeaders
55
describe PolicyManagement do
66
before(:each) do
77
reset_config
8+
Configuration.default
89
end
910

1011
let (:default_opts) do
@@ -161,14 +162,18 @@ module SecureHeaders
161162
end
162163

163164
describe "#combine_policies" do
165+
before(:each) do
166+
reset_config
167+
end
164168
it "combines the default-src value with the override if the directive was unconfigured" do
165169
Configuration.default do |config|
166170
config.csp = {
167171
default_src: %w(https:),
168172
script_src: %w('self'),
169173
}
170174
end
171-
combined_config = ContentSecurityPolicy.combine_policies(Configuration.default.csp.to_h, style_src: %w(anothercdn.com))
175+
default_policy = Configuration.send(:default_config)
176+
combined_config = ContentSecurityPolicy.combine_policies(default_policy.csp.to_h, style_src: %w(anothercdn.com))
172177
csp = ContentSecurityPolicy.new(combined_config)
173178
expect(csp.name).to eq(ContentSecurityPolicyConfig::HEADER_NAME)
174179
expect(csp.value).to eq("default-src https:; script-src 'self'; style-src https: anothercdn.com")
@@ -183,7 +188,8 @@ module SecureHeaders
183188
}.freeze
184189
end
185190
report_uri = "https://report-uri.io/asdf"
186-
combined_config = ContentSecurityPolicy.combine_policies(Configuration.default.csp.to_h, report_uri: [report_uri])
191+
default_policy = Configuration.send(:default_config)
192+
combined_config = ContentSecurityPolicy.combine_policies(default_policy.csp.to_h, report_uri: [report_uri])
187193
csp = ContentSecurityPolicy.new(combined_config, USER_AGENTS[:firefox])
188194
expect(csp.value).to include("report-uri #{report_uri}")
189195
end
@@ -199,7 +205,8 @@ module SecureHeaders
199205
non_default_source_additions = ContentSecurityPolicy::NON_FETCH_SOURCES.each_with_object({}) do |directive, hash|
200206
hash[directive] = %w("http://example.org)
201207
end
202-
combined_config = ContentSecurityPolicy.combine_policies(Configuration.default.csp.to_h, non_default_source_additions)
208+
default_policy = Configuration.send(:default_config)
209+
combined_config = ContentSecurityPolicy.combine_policies(default_policy.csp.to_h, non_default_source_additions)
203210

204211
ContentSecurityPolicy::NON_FETCH_SOURCES.each do |directive|
205212
expect(combined_config[directive]).to eq(%w("http://example.org))
@@ -214,7 +221,8 @@ module SecureHeaders
214221
report_only: false
215222
}
216223
end
217-
combined_config = ContentSecurityPolicy.combine_policies(Configuration.default.csp.to_h, report_only: true)
224+
default_policy = Configuration.send(:default_config)
225+
combined_config = ContentSecurityPolicy.combine_policies(default_policy.csp.to_h, report_only: true)
218226
csp = ContentSecurityPolicy.new(combined_config, USER_AGENTS[:firefox])
219227
expect(csp.name).to eq(ContentSecurityPolicyReportOnlyConfig::HEADER_NAME)
220228
end
@@ -227,7 +235,8 @@ module SecureHeaders
227235
block_all_mixed_content: false
228236
}
229237
end
230-
combined_config = ContentSecurityPolicy.combine_policies(Configuration.default.csp.to_h, block_all_mixed_content: true)
238+
default_policy = Configuration.send(:default_config)
239+
combined_config = ContentSecurityPolicy.combine_policies(default_policy.csp.to_h, block_all_mixed_content: true)
231240
csp = ContentSecurityPolicy.new(combined_config)
232241
expect(csp.value).to eq("default-src https:; block-all-mixed-content; script-src 'self'")
233242
end
@@ -236,8 +245,9 @@ module SecureHeaders
236245
Configuration.default do |config|
237246
config.csp = OPT_OUT
238247
end
248+
default_policy = Configuration.send(:default_config)
239249
expect do
240-
ContentSecurityPolicy.combine_policies(Configuration.default.csp.to_h, script_src: %w(anothercdn.com))
250+
ContentSecurityPolicy.combine_policies(default_policy.csp.to_h, script_src: %w(anothercdn.com))
241251
end.to raise_error(ContentSecurityPolicyConfigError)
242252
end
243253
end

spec/lib/secure_headers/middleware_spec.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@ module SecureHeaders
1111

1212
before(:each) do
1313
reset_config
14+
Configuration.default
1415
end
1516

1617
it "warns if the hpkp report-uri host is the same as the current host" do
1718
report_host = "report-uri.io"
19+
reset_config
1820
Configuration.default do |config|
1921
config.hpkp = {
2022
max_age: 10000000,
@@ -54,6 +56,9 @@ module SecureHeaders
5456
end
5557

5658
context "cookies" do
59+
before(:each) do
60+
reset_config
61+
end
5762
context "cookies should be flagged" do
5863
it "flags cookies as secure" do
5964
Configuration.default { |config| config.cookies = {secure: true, httponly: OPT_OUT, samesite: OPT_OUT} }
@@ -85,6 +90,9 @@ module SecureHeaders
8590
end
8691

8792
context "cookies" do
93+
before(:each) do
94+
reset_config
95+
end
8896
it "flags cookies from configuration" do
8997
Configuration.default { |config| config.cookies = { secure: true, httponly: true, samesite: { lax: true} } }
9098
request = Rack::Request.new("HTTPS" => "on")

spec/lib/secure_headers_spec.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,18 @@ module SecureHeaders
99

1010
let(:request) { Rack::Request.new("HTTP_X_FORWARDED_SSL" => "on") }
1111

12+
it "raises a NotYetConfiguredError if default has not been set" do
13+
expect do
14+
SecureHeaders.header_hash_for(request)
15+
end.to raise_error(Configuration::NotYetConfiguredError)
16+
end
17+
18+
it "raises a NotYetConfiguredError if trying to opt-out of unconfigured headers" do
19+
expect do
20+
SecureHeaders.opt_out_of_header(request, :csp)
21+
end.to raise_error(Configuration::NotYetConfiguredError)
22+
end
23+
1224
it "raises and ArgumentError when referencing an override that has not been set" do
1325
expect do
1426
Configuration.default

0 commit comments

Comments
 (0)