Simplify Whitelisted Headers

Refactor simple-headers-wo-content-type to access-control-request-headers-whitelist.

The whitelist behaves differently by `set/difference`-ing
`access-control-request-headers` instead of `set/union` the
`access-control-allowed-headers`. This way, we do not put unnecessary
headers in the response `access-control-allowed-headers`.

From testing, the only required header in the whitelist is
"Origin". Safari always sends this in "Access-Control-Request-Headers"
during pre-flight, whereas the Firefox and Chrome do not.

src/com/unbounce/encors/core.clj

@@ -80,15 +80,15 @@
 ;; Request -> CorsPolicy -> [:left [ErrorMsg]] | [:right Headers]
 (defn cors-preflight-check-request-headers
   [{:keys [headers] :as req} cors-policy]
-  (let [supported-headers (set/union
-                            (:request-headers cors-policy)
-                            types/simple-headers-wo-content-type)
+  (let [supported-headers (:request-headers cors-policy)
         supported-headers* (into
                              (sorted-set)
                              (map str/lower-case supported-headers))
         supported-headers-str (str/join ", " supported-headers)
         control-req-headers-str (get headers "access-control-request-headers")
-        control-req-headers (header-string-to-set control-req-headers-str)]
+        control-req-headers (set/difference
+                              (header-string-to-set control-req-headers-str)
+                              types/access-control-request-headers-whitelist)]
 
     (if (or (empty? control-req-headers)
             (set/subset? control-req-headers supported-headers*))

src/com/unbounce/encors/types.clj

@@ -21,6 +21,8 @@
 
 (def simple-methods #{:get :head :post})
 
-(def simple-headers #{"Accept" "Accept-Language" "Content-Language" "Content-Type" "Origin"})
-
-(def simple-headers-wo-content-type #{"Accept" "Accept-Language" "Content-Language" "Origin"})
+(def access-control-request-headers-whitelist
+  ;; Only Safari sends these access-control-request-headers on the preflight
+  ;; even though they are considered "simple headers".
+  ;; Note: these *must* be lowercase
+  #{"origin" "accept"})

test/com/unbounce/encors/core_test.clj

@@ -93,17 +93,13 @@
              [:left [(str "HTTP headers requested in Access-Control-Request-Headers of "
                           "CORS request is not supported; requested: "
                           "'X-Not-Safe-To-Expose, X-Blah-Bleh'; "
-                          "supported are 'X-Safe-To-Expose, Origin, Accept-Language, "
-                          "Content-Language, Accept'.")]])))
+                          "supported are 'X-Safe-To-Expose'.")]])))
     (testing "Access-Control-Request-Headers match policy request headers"
       (is (= (cors-preflight-check-request-headers
               {:headers {"access-control-request-headers"
                          "X-Safe-To-Expose"}}
               policy)
-             [:right {"Access-Control-Allow-Headers"
-                      (str/join ", "
-                        (set/union #{"X-Safe-To-Expose"}
-                                   types/simple-headers-wo-content-type))}])))))
+             [:right {"Access-Control-Allow-Headers" "X-Safe-To-Expose"}])))))
 
 (deftest cors-preflight-headers-test
   (testing "With a request that complies with policy"
@@ -117,13 +113,26 @@
                                                 "access-control-request-method"
                                                 "GET"}}
                                      policy)
-             [:right {"Access-Control-Allow-Headers"
-                      (str/join ", "
-                        (set/union #{"X-Cool-Header"}
-                                   types/simple-headers-wo-content-type))
+             [:right {"Access-Control-Allow-Headers" "X-Cool-Header"
                       "Access-Control-Allow-Methods" "GET, HEAD, POST"
                       "Access-Control-Max-Age" "365"}])))))
 
+(deftest cors-whitelisted-headers-test
+  (testing "With a request that contains whitelisted headers."
+    (let [policy (merge default-cors-options
+                   {:max-age 365
+                    :allow-credentials? true
+                    :allowed-methods #{:get}})]
+      (is (= (cors-preflight-headers {:headers {"access-control-request-headers"
+                                                "Origin, Accept"
+                                                "access-control-request-method"
+                                                "GET"}}
+               policy)
+             [:right {"Access-Control-Allow-Headers" ""
+                      "Access-Control-Allow-Methods" "GET, HEAD, POST"
+                      "Access-Control-Max-Age" "365"}])))))
+
+
 (deftest apply-cors-policy-test
   (testing ":allowed-origins has a :star-origin value"
     (let [policy (merge default-cors-options

test/com/unbounce/encors/integration_test.clj

@@ -49,7 +49,7 @@
 (def ^:const allowed-methods "GET, HEAD, PATCH, POST")
 (def ^:const allowed-headers "Content-Type, X-Allowed")
 (def ^:const active-allowed-headers
-  "Origin, Accept-Language, Content-Language, Content-Type, Accept, X-Allowed")
+  "Content-Type, X-Allowed")
 (def ^:const expose-headers  "X-Safe-To-Expose, X-Safe-To-Expose-Too")
 (def ^:const unallowed-origin "not.cool.io")