Remove all notions of a set of stored configurations

The current code stored named configs in class instance hash. Since the only
config that should ever exist is now the default config (all other cases are
handled by dynamic overrides and not configs), we remove all vestiages of the
configuration hash lookup.

Author: ptoomey3

lib/secure_headers.rb

@@ -148,6 +148,7 @@ def opt_out_of_all_protection(request)
     def header_hash_for(request)
       prevent_dup = true
       config = config_for(request, prevent_dup)
+      config.validate_config!
       user_agent = UserAgent.parse(request.user_agent)
       headers = config.generate_headers(user_agent)
 
@@ -198,7 +199,7 @@ def content_security_policy_style_nonce(request)
     # Falls back to the global config
     def config_for(request, prevent_dup = false)
       config = request.env[SECURE_HEADERS_CONFIG] ||
-        Configuration.get(Configuration::DEFAULT_CONFIG)
+        Configuration.get
 
 
       # Global configs are frozen, per-request configs are not. When we're not

lib/secure_headers/configuration.rb

@@ -14,17 +14,25 @@ class << self
       #
       # Returns the newly created config.
       def default(&block)
-        config = new(&block)
-        add_configuration(DEFAULT_CONFIG, config)
+        # Define a built-in override that clears all configuration options and
+        # results in no security headers being set.
         override(NOOP_OVERRIDE) do |config|
           CONFIG_ATTRIBUTES.each do |attr|
             config.instance_variable_set("@#{attr}", OPT_OUT)
           end
         end
-        config
+
+        new_config = new(&block).freeze
+        new_config.validate_config!
+        @default_config = new_config
       end
       alias_method :configure, :default
 
+      def get
+        raise NotYetConfiguredError, "Default policy not yet supplied" unless @default_config
+        @default_config
+      end
+
       # Public: create a named configuration that overrides the default config.
       #
       # name - use an idenfier for the override config.
@@ -34,6 +42,7 @@ def default(&block)
       # Returns: the newly created config
       def override(name, &block)
         @overrides ||= {}
+        raise "Provide a configuration block" unless block_given?
         @overrides[name] = block
       end
 
@@ -42,46 +51,19 @@ def overrides(name)
         @overrides[name]
       end
 
-      # Public: retrieve a global configuration object
-      #
-      # Returns the configuration with a given name or raises a
-      # NotYetConfiguredError if `default` has not been called.
-      def get(name = DEFAULT_CONFIG)
-        if @configurations.nil?
-          raise NotYetConfiguredError, "Default policy not yet supplied"
-        end
-        @configurations[name]
-      end
-
       def named_appends(name)
         @appends ||= {}
         @appends[name]
       end
 
-      def named_append(name, target = nil, &block)
+      def named_append(name, &block)
         @appends ||= {}
         raise "Provide a configuration block" unless block_given?
         @appends[name] = block
       end
 
       private
 
-      # Private: add a valid configuration to the global set of named configs.
-      #
-      # config - the config to store
-      # name - the lookup value for this config
-      #
-      # Raises errors if the config is invalid or if a config named `name`
-      # already exists.
-      #
-      # Returns the config, if valid
-      def add_configuration(name, config)
-        config.validate_config!
-        @configurations ||= {}
-        config.freeze
-        @configurations[name] = config
-      end
-
       # Public: perform a basic deep dup. The shallow copy provided by dup/clone
       # can lead to modifying parent objects.
       def deep_copy(config)
@@ -106,11 +88,7 @@ def deep_copy_if_hash(value)
       end
     end
 
-    NON_HEADER_ATTRIBUTES = [
-      :cookies, :hpkp_report_host
-    ].freeze
-
-    HEADER_ATTRIBUTES_TO_HEADER_CLASSES = {
+    CONFIG_ATTRIBUTES_TO_HEADER_CLASSES = {
       hsts: StrictTransportSecurity,
       x_frame_options: XFrameOptions,
       x_content_type_options: XContentTypeOptions,
@@ -123,11 +101,18 @@ def deep_copy_if_hash(value)
       csp: ContentSecurityPolicy,
       csp_report_only: ContentSecurityPolicy,
       hpkp: PublicKeyPins,
+      cookies: Cookie,
     }.freeze
 
-    CONFIG_ATTRIBUTES = (HEADER_ATTRIBUTES_TO_HEADER_CLASSES.keys + NON_HEADER_ATTRIBUTES).freeze
+    CONFIG_ATTRIBUTES = CONFIG_ATTRIBUTES_TO_HEADER_CLASSES.keys.freeze
 
-    attr_accessor(*CONFIG_ATTRIBUTES)
+    # The list of attributes that must respond to a `validate_config!` method
+    VALIDATABLE_ATTRIBUTES = CONFIG_ATTRIBUTES
+
+    # The list of attributes that must respond to a `make_header` method
+    HEADERABLE_ATTRIBUTES = (CONFIG_ATTRIBUTES - [:cookies]).freeze
+
+    attr_accessor(*CONFIG_ATTRIBUTES_TO_HEADER_CLASSES.keys)
 
     @script_hashes = nil
     @style_hashes = nil
@@ -144,7 +129,6 @@ def initialize(&block)
       @clear_site_data = nil
       @csp = nil
       @csp_report_only = nil
-      @hpkp_report_host = nil
       @hpkp = nil
       @hsts = nil
       @x_content_type_options = nil
@@ -185,7 +169,8 @@ def dup
 
     def generate_headers(user_agent)
       headers = {}
-      HEADER_ATTRIBUTES_TO_HEADER_CLASSES.each do |attr, klass|
+      HEADERABLE_ATTRIBUTES.each do |attr|
+        klass = CONFIG_ATTRIBUTES_TO_HEADER_CLASSES[attr]
         header_name, value = klass.make_header(instance_variable_get("@#{attr}"), user_agent)
         if header_name && value
           headers[header_name] = value
@@ -208,26 +193,26 @@ def update_x_frame_options(value)
     #
     # Returns nothing
     def validate_config!
-      HEADER_ATTRIBUTES_TO_HEADER_CLASSES.each do |attr, klass|
+      VALIDATABLE_ATTRIBUTES.each do |attr|
+        klass = CONFIG_ATTRIBUTES_TO_HEADER_CLASSES[attr]
         klass.validate_config!(instance_variable_get("@#{attr}"))
       end
-      Cookie.validate_config!(@cookies)
     end
 
     def secure_cookies=(secure_cookies)
       raise ArgumentError, "#{Kernel.caller.first}: `#secure_cookies=` is no longer supported. Please use `#cookies=` to configure secure cookies instead."
     end
 
     def csp=(new_csp)
-      if new_csp.respond_to?(:opt_out?)
-        @csp = new_csp.dup
+      case new_csp
+      when OPT_OUT
+        @csp = new_csp
+      when ContentSecurityPolicyConfig
+        @csp = new_csp
+      when Hash
+        @csp = ContentSecurityPolicyConfig.new(new_csp)
       else
-        if new_csp[:report_only]
-          # invalid configuration implies that CSPRO should be set, CSP should not - so opt out
-          raise ArgumentError, "#{Kernel.caller.first}: `#csp=` was supplied a config with report_only: true. Use #csp_report_only="
-        else
-          @csp = ContentSecurityPolicyConfig.new(new_csp)
-        end
+        raise ArgumentError, "Must provide either an existing CSP config or a CSP config hash"
       end
     end
 
@@ -238,34 +223,23 @@ def csp=(new_csp)
     # configuring csp_report_only, the code will assume you mean to only use
     # report-only mode and you will be opted-out of enforce mode.
     def csp_report_only=(new_csp)
-      @csp_report_only = begin
-        if new_csp.is_a?(ContentSecurityPolicyConfig)
-          new_csp.make_report_only
-        elsif new_csp.respond_to?(:opt_out?)
-          new_csp.dup
-        else
-          if new_csp[:report_only] == false # nil is a valid value on which we do not want to raise
-            raise ContentSecurityPolicyConfigError, "`#csp_report_only=` was supplied a config with report_only: false. Use #csp="
-          else
-            ContentSecurityPolicyReportOnlyConfig.new(new_csp)
-          end
-        end
+      case new_csp
+      when OPT_OUT
+        @csp_report_only = new_csp
+      when ContentSecurityPolicyReportOnlyConfig
+        @csp_report_only = new_csp.dup
+      when ContentSecurityPolicyConfig
+        @csp_report_only = new_csp.make_report_only
+      when Hash
+        @csp_report_only = ContentSecurityPolicyReportOnlyConfig.new(new_csp)
+      else
+        raise ArgumentError, "Must provide either an existing CSP config or a CSP config hash"
       end
     end
 
     def hpkp_report_host
       return nil unless @hpkp && hpkp != OPT_OUT && @hpkp[:report_uri]
       URI.parse(@hpkp[:report_uri]).host
     end
-
-    protected
-
-    def cookies=(cookies)
-      @cookies = cookies
-    end
-
-    def hpkp=(hpkp)
-      @hpkp = self.class.send(:deep_copy_if_hash, hpkp)
-    end
   end
 end

lib/secure_headers/headers/content_security_policy_config.rb

@@ -55,7 +55,6 @@ def directive_value(directive)
 
     def merge(new_hash)
       new_config = self.dup
-      puts new_config.inspect
       new_config.send(:from_hash, new_hash)
       new_config
     end
@@ -141,7 +140,6 @@ def make_report_only
   end
 
   class ContentSecurityPolicyReportOnlyConfig < ContentSecurityPolicyConfig
-    CONFIG_KEY = :csp_report_only
     HEADER_NAME = "Content-Security-Policy-Report-Only".freeze
 
     def report_only?

lib/secure_headers/headers/policy_management.rb

@@ -216,12 +216,22 @@ def validate_config!(config)
         if config.directive_value(:script_src).nil?
           raise ContentSecurityPolicyConfigError.new(":script_src is required, falling back to default-src is too dangerous. Use `script_src: OPT_OUT` to override")
         end
+        if !config.report_only? && config.directive_value(:report_only)
+          raise ContentSecurityPolicyConfigError.new("Only the csp_report_only config should  sset :report_only to true")
+        end
+
+        if config.report_only? && config.directive_value(:report_only) == false
+          raise ContentSecurityPolicyConfigError.new("csp_report_only config must have :report_only set to true")
+        end
 
         ContentSecurityPolicyConfig.attrs.each do |key|
           value = config.directive_value(key)
           next unless value
+
           if META_CONFIGS.include?(key)
             raise ContentSecurityPolicyConfigError.new("#{key} must be a boolean value") unless boolean?(value) || value.nil?
+          elsif NONCES.include?(key)
+            raise ContentSecurityPolicyConfigError.new("#{key} must be a non-nil value") if value.nil?
           else
             validate_directive!(key, value)
           end

spec/lib/secure_headers/configuration_spec.rb

@@ -9,7 +9,7 @@ module SecureHeaders
     end
 
     it "has a default config" do
-      expect(Configuration.get(Configuration::DEFAULT_CONFIG)).to_not be_nil
+      expect(Configuration.default).to_not be_nil
     end
 
     it "has an 'noop' override" do

spec/lib/secure_headers/headers/policy_management_spec.rb

@@ -18,7 +18,7 @@ module SecureHeaders
         # (pulled from README)
         config = {
           # "meta" values. these will shape the header, but the values are not included in the header.
-          report_only:  true,     # default: false
+          report_only:  false,
           preserve_schemes: true, # default: false. Schemes are removed from host sources to save bytes and discourage mixed content.
 
           # directive values: these values will directly translate into source directives
@@ -142,6 +142,18 @@ module SecureHeaders
           ContentSecurityPolicy.validate_config!(ContentSecurityPolicyConfig.new(default_opts.merge(plugin_types: ["application/pdf"])))
         end.to_not raise_error
       end
+
+      it "doesn't allow report_only to be set in a non-report-only config" do
+        expect do
+          ContentSecurityPolicy.validate_config!(ContentSecurityPolicyConfig.new(default_opts.merge(report_only: true)))
+        end.to raise_error(ContentSecurityPolicyConfigError)
+      end
+
+      it "allows report_only to be set in a report-only config" do
+        expect do
+          ContentSecurityPolicy.validate_config!(ContentSecurityPolicyReportOnlyConfig.new(default_opts.merge(report_only: true)))
+        end.to_not raise_error
+      end
     end
 
     describe "#combine_policies" do

spec/lib/secure_headers_spec.rb

@@ -131,7 +131,7 @@ module SecureHeaders
         firefox_request = Rack::Request.new(request.env.merge("HTTP_USER_AGENT" => USER_AGENTS[:firefox]))
 
         # append an unsupported directive
-        SecureHeaders.override_content_security_policy_directives(firefox_request, {plugin_types: %w(flash)})
+        SecureHeaders.override_content_security_policy_directives(firefox_request, {plugin_types: %w(application/pdf)})
         # append a supported directive
         SecureHeaders.override_content_security_policy_directives(firefox_request, {script_src: %w('self')})
 
@@ -337,7 +337,7 @@ module SecureHeaders
                 report_only: true
               }
             end
-          }.to raise_error(ArgumentError)
+          }.to raise_error(ContentSecurityPolicyConfigError)
         end
 
         it "Raises an error if csp_report_only is used with `report_only: false`" do

spec/spec_helper.rb

@@ -43,13 +43,13 @@ def expect_default_values(hash)
 module SecureHeaders
   class Configuration
     class << self
-      def clear_configurations
-        @configurations = nil
+      def clear_default_config
+        @default_config = nil
       end
     end
   end
 end
 
 def reset_config
-  SecureHeaders::Configuration.clear_configurations
+  SecureHeaders::Configuration.clear_default_config
 end