Raise on override defined config (#436)

* Raise exception when trying to override an existing config

* Raise exception when trying to set a named append more than once

* Update documentation

* Raise an exception when a named append or override with the same name exists

Co-authored-by: Neil Matatall <448516+oreoshake@users.noreply.github.com>

Author: jobertabma

docs/named_overrides_and_appends.md

@@ -1,6 +1,6 @@
 ## Named Appends
 
-Named Appends are blocks of code that can be reused and composed during requests. e.g. If a certain partial is rendered conditionally, and the csp needs to be adjusted for that partial, you can create a named append for that situation. The value returned by the block will be passed into `append_content_security_policy_directives`. The current request object is passed as an argument to the block for even more flexibility.
+Named Appends are blocks of code that can be reused and composed during requests. e.g. If a certain partial is rendered conditionally, and the csp needs to be adjusted for that partial, you can create a named append for that situation. The value returned by the block will be passed into `append_content_security_policy_directives`. The current request object is passed as an argument to the block for even more flexibility. Reusing a configuration name is not allowed and will throw an exception.
 
 ```ruby
 def show
@@ -91,6 +91,8 @@ class MyController < ApplicationController
 end
 ```
 
+Reusing a configuration name is not allowed and will throw an exception.
+
 By default, a no-op configuration is provided. No headers will be set when this default override is used.
 
 ```ruby

lib/secure_headers/configuration.rb

@@ -43,6 +43,9 @@ def default(&block)
       def override(name, &block)
         @overrides ||= {}
         raise "Provide a configuration block" unless block_given?
+        if named_append_or_override_exists?(name)
+          raise AlreadyConfiguredError, "Configuration already exists"
+        end
         @overrides[name] = block
       end
 
@@ -59,6 +62,9 @@ def named_appends(name)
       def named_append(name, &block)
         @appends ||= {}
         raise "Provide a configuration block" unless block_given?
+        if named_append_or_override_exists?(name)
+          raise AlreadyConfiguredError, "Configuration already exists"
+        end
         @appends[name] = block
       end
 
@@ -68,6 +74,11 @@ def dup
 
       private
 
+      def named_append_or_override_exists?(name)
+        (defined?(@appends) && @appends.key?(name)) ||
+          (defined?(@overrides) && @overrides.key?(name))
+      end
+
       # Public: perform a basic deep dup. The shallow copy provided by dup/clone
       # can lead to modifying parent objects.
       def deep_copy(config)

spec/lib/secure_headers/configuration_spec.rb

@@ -34,6 +34,60 @@ module SecureHeaders
       expect(Configuration.overrides(:test_override)).to_not be_nil
     end
 
+    describe "#override" do
+      it "raises on configuring an existing override" do
+        set_override = Proc.new {
+          Configuration.override(:test_override) do |config|
+            config.x_frame_options = "DENY"
+          end
+        }
+
+        set_override.call
+
+        expect { set_override.call }
+          .to raise_error(Configuration::AlreadyConfiguredError, "Configuration already exists")
+      end
+
+      it "raises when a named append with the given name exists" do
+        Configuration.named_append(:test_override) do |config|
+          config.x_frame_options = "DENY"
+        end
+
+        expect do
+          Configuration.override(:test_override) do |config|
+            config.x_frame_options = "SAMEORIGIN"
+          end
+        end.to raise_error(Configuration::AlreadyConfiguredError, "Configuration already exists")
+      end
+    end
+
+    describe "#named_append" do
+      it "raises on configuring an existing append" do
+        set_override = Proc.new {
+          Configuration.named_append(:test_override) do |config|
+            config.x_frame_options = "DENY"
+          end
+        }
+
+        set_override.call
+
+        expect { set_override.call }
+          .to raise_error(Configuration::AlreadyConfiguredError, "Configuration already exists")
+      end
+
+      it "raises when an override with the given name exists" do
+        Configuration.override(:test_override) do |config|
+          config.x_frame_options = "DENY"
+        end
+
+        expect do
+          Configuration.named_append(:test_override) do |config|
+            config.x_frame_options = "SAMEORIGIN"
+          end
+        end.to raise_error(Configuration::AlreadyConfiguredError, "Configuration already exists")
+      end
+    end
+
     it "deprecates the secure_cookies configuration" do
       expect {
         Configuration.default do |config|

spec/spec_helper.rb

@@ -45,10 +45,20 @@ class << self
       def clear_default_config
         remove_instance_variable(:@default_config) if defined?(@default_config)
       end
+
+      def clear_overrides
+        remove_instance_variable(:@overrides) if defined?(@overrides)
+      end
+
+      def clear_appends
+        remove_instance_variable(:@appends) if defined?(@appends)
+      end
     end
   end
 end
 
 def reset_config
   SecureHeaders::Configuration.clear_default_config
+  SecureHeaders::Configuration.clear_overrides
+  SecureHeaders::Configuration.clear_appends
 end