update SameSite cookie configuration

stve · stve · commit 72ae1f0977db · 2016-04-07T21:49:53.000-04:00
global boolean config is not permitted, `lax` and `strict` can accept
booleans to enable for all cookies
diff --git a/lib/secure_headers/headers/cookie.rb b/lib/secure_headers/headers/cookie.rb
@@ -20,23 +20,59 @@ def validate_config!(config)
         return if config.nil? || config == OPT_OUT
         raise CookiesConfigError.new("config must be a hash.") unless config.is_a? Hash
 
-        # validate only boolean or Hash configuration
-        [:secure, :httponly, :samesite].each do |attribute|
+        # secure and httponly - validate only boolean or Hash configuration
+        [:secure, :httponly].each do |attribute|
           if config[attribute] && !(config[attribute].is_a?(Hash) || config[attribute].is_a?(TrueClass) || config[attribute].is_a?(FalseClass))
             raise CookiesConfigError.new("#{attribute} cookie config must be a hash or boolean")
           end
         end
 
+        # secure and httponly - validate exclusive use of only or except but not both at the same time
         [:secure, :httponly].each do |attribute|
-          if config[attribute].is_a?(Hash) && config[attribute].key?(:only) && config[attribute].key?(:except)
-            raise CookiesConfigError.new("#{attribute} cookie config is invalid, simultaneous use of conditional arguments `only` and `except` is not permitted.")
+          if config[attribute].is_a?(Hash)
+            if config[attribute].key?(:only) && config[attribute].key?(:except)
+              raise CookiesConfigError.new("#{attribute} cookie config is invalid, simultaneous use of conditional arguments `only` and `except` is not permitted.")
+            end
+
+            if (intersection = (config[attribute].fetch(:only, []) & config[attribute].fetch(:only, []))).any?
+              raise CookiesConfigError.new("#{attribute} cookie config is invalid, cookies #{intersection.join(', ')} cannot be enforced as lax and strict")
+            end
           end
         end
 
-        if config[:samesite] && config[:samesite].is_a?(Hash)
-          [:lax, :strict].each do |samesite_attribute|
-            if config[:samesite][samesite_attribute].is_a?(Hash) && config[:samesite][samesite_attribute].key?(:only) && config[:samesite][samesite_attribute].key?(:except)
-              raise CookiesConfigError.new("samesite #{samesite_attribute} cookie config is invalid, simultaneous use of conditional arguments `only` and `except` is not permitted.")
+        if config[:samesite]
+          raise CookiesConfigError.new("samesite cookie config must be a hash") unless config[:samesite].is_a?(Hash)
+
+          # when configuring with booleans, only one enforcement is permitted
+          if config[:samesite].key?(:lax) && config[:samesite][:lax].is_a?(TrueClass) && config[:samesite].key?(:strict)
+            raise CookiesConfigError.new("samesite cookie config is invalid, combination use of booleans and Hash to configure lax and strict enforcement is not permitted.")
+          elsif config[:samesite].key?(:strict) && config[:samesite][:strict].is_a?(TrueClass) && config[:samesite].key?(:lax)
+            raise CookiesConfigError.new("samesite cookie config is invalid, combination use of booleans and Hash to configure lax and strict enforcement is not permitted.")
+          end
+
+          # validate Hash-based samesite configuration
+          if config[:samesite].key?(:lax) && config[:samesite][:lax].is_a?(Hash)
+            # validate exclusive use of only or except but not both at the same time
+            if config[:samesite][:lax].key?(:only) && config[:samesite][:lax].key?(:except)
+              raise CookiesConfigError.new("samesite lax cookie config is invalid, simultaneous use of conditional arguments `only` and `except` is not permitted.")
+            end
+
+            if config[:samesite].key?(:strict)
+              # validate exclusivity of only and except members
+              if (intersection = (config[:samesite][:lax].fetch(:only, []) & config[:samesite][:strict].fetch(:only, []))).any?
+                raise CookiesConfigError.new("samesite cookie config is invalid, cookie(s) #{intersection.join(', ')} cannot be enforced as lax and strict")
+              end
+
+              if (intersection = (config[:samesite][:lax].fetch(:except, []) & config[:samesite][:strict].fetch(:except, []))).any?
+                raise CookiesConfigError.new("samesite cookie config is invalid, cookie(s) #{intersection.join(', ')} cannot be enforced as lax and strict")
+              end
+            end
+          end
+
+          if config[:samesite].key?(:strict) && config[:samesite][:strict].is_a?(Hash)
+            # validate exclusive use of only or except but not both at the same time
+            if config[:samesite][:strict].key?(:only) && config[:samesite][:strict].key?(:except)
+              raise CookiesConfigError.new("samesite strict cookie config is invalid, simultaneous use of conditional arguments `only` and `except` is not permitted.")
             end
           end
         end
@@ -102,35 +138,36 @@ def conditionally_flag?(configuration)
     end
 
     def samesite_cookie
-      case config[:samesite]
-      when TrueClass
-        "SameSite"
-      when Hash
-        if flag_samesite_lax?
-          "SameSite=Lax"
-        elsif flag_samesite_strict?
-          "SameSite=Strict"
-        end
+      if flag_samesite_lax?
+        "SameSite=Lax"
+      elsif flag_samesite_strict?
+        "SameSite=Strict"
       end
     end
 
     def flag_samesite?
-      case config[:samesite]
-      when TrueClass
-        true
-      when Hash
-        flag_samesite_lax? || flag_samesite_strict?
-      else
-        false
-      end
+      flag_samesite_lax? || flag_samesite_strict?
     end
 
     def flag_samesite_lax?
-      config[:samesite].key?(:lax) && conditionally_flag?(config[:samesite][:lax])
+      flag_samesite_enforcement?(:lax)
     end
 
     def flag_samesite_strict?
-      config[:samesite].key?(:strict) && conditionally_flag?(config[:samesite][:strict])
+      flag_samesite_enforcement?(:strict)
+    end
+
+    def flag_samesite_enforcement?(mode)
+      return unless config[:samesite]
+
+      case config[:samesite][mode]
+      when Hash
+        conditionally_flag?(config[:samesite][mode])
+      when TrueClass
+        true
+      else
+        false
+      end
     end
   end
 end
diff --git a/spec/lib/secure_headers/cookie_spec.rb b/spec/lib/secure_headers/cookie_spec.rb
@@ -62,38 +62,40 @@ module SecureHeaders
     end
 
     context "SameSite cookies" do
-      context "when configured with a boolean" do
-        it "flags cookies as SameSite" do
-          cookie = Cookie.new(raw_cookie, samesite: true)
-          expect(cookie.to_s).to match(Cookie::SAMESITE_REGEXP)
-        end
+      it "flags SameSite=Lax" do
+        cookie = Cookie.new(raw_cookie, samesite: { lax: { only: ["_session"] } })
+        expect(cookie.to_s).to match(Cookie::SAMESITE_LAX_REGEXP)
       end
 
-      context "when configured with a Hash" do
-        it "flags SameSite=Lax" do
-          cookie = Cookie.new(raw_cookie, samesite: { lax: { only: ["_session"] } })
-          expect(cookie.to_s).to match(Cookie::SAMESITE_LAX_REGEXP)
-        end
+      it "flags SameSite=Lax when configured with a boolean" do
+        cookie = Cookie.new(raw_cookie, samesite: { lax: true})
+        expect(cookie.to_s).to match(Cookie::SAMESITE_LAX_REGEXP)
+      end
 
-        it "does not flag cookies as SameSite=Lax when excluded" do
-          cookie = Cookie.new(raw_cookie, samesite: { lax: { except: ["_session"] } })
-          expect(cookie.to_s).not_to match(Cookie::SAMESITE_LAX_REGEXP)
-        end
+      it "does not flag cookies as SameSite=Lax when excluded" do
+        cookie = Cookie.new(raw_cookie, samesite: { lax: { except: ["_session"] } })
+        expect(cookie.to_s).not_to match(Cookie::SAMESITE_LAX_REGEXP)
+      end
 
-        it "flags SameSite=Strict" do
-          cookie = Cookie.new(raw_cookie, samesite: { strict: { only: ["_session"] } })
-          expect(cookie.to_s).to match(Cookie::SAMESITE_STRICT_REGEXP)
-        end
+      it "flags SameSite=Strict" do
+        cookie = Cookie.new(raw_cookie, samesite: { strict: { only: ["_session"] } })
+        expect(cookie.to_s).to match(Cookie::SAMESITE_STRICT_REGEXP)
+      end
 
-        it "does not flag cookies as SameSite=Strict when excluded" do
-          cookie = Cookie.new(raw_cookie, samesite: { strict: { except: ["_session"] } })
-          expect(cookie.to_s).not_to match(Cookie::SAMESITE_STRICT_REGEXP)
-        end
+      it "does not flag cookies as SameSite=Strict when excluded" do
+        cookie = Cookie.new(raw_cookie, samesite: { strict: { except: ["_session"] } })
+        expect(cookie.to_s).not_to match(Cookie::SAMESITE_STRICT_REGEXP)
+      end
 
-        it "flags properly when both lax and strict are configured" do
-          cookie = Cookie.new(raw_cookie, samesite: { strict: { only: ["_session"] }, lax: { only: ["_additional_session"] } })
-          expect(cookie.to_s).to match(Cookie::SAMESITE_STRICT_REGEXP)
-        end
+      it "flags SameSite=Strict when configured with a boolean" do
+        cookie = Cookie.new(raw_cookie, samesite: { strict: true})
+        expect(cookie.to_s).to match(Cookie::SAMESITE_STRICT_REGEXP)
+      end
+
+      it "flags properly when both lax and strict are configured" do
+        raw_cookie = "_session=thisisatest"
+        cookie = Cookie.new(raw_cookie, samesite: { strict: { only: ["_session"] }, lax: { only: ["_additional_session"] } })
+        expect(cookie.to_s).to match(Cookie::SAMESITE_STRICT_REGEXP)
       end
     end
   end
@@ -117,9 +119,39 @@ module SecureHeaders
       end.to raise_error(CookiesConfigError)
     end
 
+    it "raises an exception when SameSite is not configured with a Hash" do
+      expect do
+        Cookie.validate_config!(samesite: true)
+      end.to raise_error(CookiesConfigError)
+    end
+
+    it "raises an exception when SameSite lax and strict enforcement modes are configured with booleans" do
+      expect do
+        Cookie.validate_config!(samesite: { lax: true, strict: true})
+      end.to raise_error(CookiesConfigError)
+    end
+
+    it "raises an exception when SameSite lax and strict enforcement modes are configured with booleans" do
+      expect do
+        Cookie.validate_config!(samesite: { lax: true, strict: { only: ["_anything"] } })
+      end.to raise_error(CookiesConfigError)
+    end
+
     it "raises an exception when both only and except filters are provided to SameSite configurations" do
       expect do
-        Cookie.validate_config!(samesite: { lax: { only: [], except: [] } })
+        Cookie.validate_config!(samesite: { lax: { only: ["_anything"], except: ["_anythingelse"] } })
+      end.to raise_error(CookiesConfigError)
+    end
+
+    it "raises an exception when both lax and strict only filters are provided to SameSite configurations" do
+      expect do
+        Cookie.validate_config!(samesite: { lax: { only: ["_anything"] }, strict: { only: ["_anything"] } })
+      end.to raise_error(CookiesConfigError)
+    end
+
+    it "raises an exception when both lax and strict only filters are provided to SameSite configurations" do
+      expect do
+        Cookie.validate_config!(samesite: { lax: { except: ["_anything"] }, strict: { except: ["_anything"] } })
       end.to raise_error(CookiesConfigError)
     end
   end
diff --git a/spec/lib/secure_headers/middleware_spec.rb b/spec/lib/secure_headers/middleware_spec.rb
@@ -64,13 +64,12 @@ module SecureHeaders
 
     context "cookies" do
       it "flags cookies from configuration" do
-        Configuration.default { |config| config.cookies = { secure: true, httponly: true, samesite: true } }
+        Configuration.default { |config| config.cookies = { secure: true, httponly: true } }
         request = Rack::Request.new("HTTPS" => "on")
         _, env = cookie_middleware.call request.env
 
         expect(env['Set-Cookie']).to match(SecureHeaders::Cookie::SECURE_REGEXP)
         expect(env['Set-Cookie']).to match(SecureHeaders::Cookie::HTTPONLY_REGEXP)
-        expect(env['Set-Cookie']).to match(SecureHeaders::Cookie::SAMESITE_REGEXP)
       end
 
       it "flags cookies with a combination of SameSite configurations" do
