Merge pull request #7402 from penpot/niwinz-develop-enhancements-3

✨ Add additional http middlewares

backend/src/app/config.clj

@@ -146,7 +146,6 @@
     [:quotes-team-access-requests-per-team {:optional true} ::sm/int]
     [:quotes-team-access-requests-per-requester {:optional true} ::sm/int]
 
-    [:auth-data-cookie-domain {:optional true} :string]
     [:auth-token-cookie-name {:optional true} :string]
     [:auth-token-cookie-max-age {:optional true} ::ct/duration]
 

backend/src/app/http.clj

@@ -19,6 +19,7 @@
    [app.http.errors :as errors]
    [app.http.management :as mgmt]
    [app.http.middleware :as mw]
+   [app.http.security :as sec]
    [app.http.session :as session]
    [app.http.websocket :as-alias ws]
    [app.main :as-alias main]
@@ -167,6 +168,7 @@
   [_ cfg]
   (rr/router
    [["" {:middleware [[mw/server-timing]
+                      [sec/sec-fetch-metadata]
                       [mw/params]
                       [mw/format-response]
                       [session/soft-auth cfg]
@@ -187,7 +189,8 @@
 
      (::ws/routes cfg)
 
-     ["/api" {:middleware [[mw/cors]]}
+     ["/api" {:middleware [[mw/cors]
+                           [sec/client-header-check]]}
       (::oidc/routes cfg)
       (::rpc.doc/routes cfg)
       (::rpc/routes cfg)]]]))

backend/src/app/http/security.clj

@@ -0,0 +1,55 @@
+;; This Source Code Form is subject to the terms of the Mozilla Public
+;; License, v. 2.0. If a copy of the MPL was not distributed with this
+;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
+;;
+;; Copyright (c) KALEIDOS INC
+
+(ns app.http.security
+  "Additional security layer middlewares"
+  (:require
+   [app.config :as cf]
+   [yetti.request :as yreq]
+   [yetti.response :as yres]))
+
+(def ^:private safe-methods
+  #{:get :head :options})
+
+(defn- wrap-sec-fetch-metadata
+  "Sec-Fetch metadata security layer middleware"
+  [handler]
+  (fn [request]
+    (let [site (yreq/get-header request "sec-fetch-site")]
+      (cond
+        (= site "same-origin")
+        (handler request)
+
+        (or (= site "same-site")
+            (= site "cross-site"))
+        (if (contains? safe-methods (yreq/method request))
+          (handler request)
+          {::yres/status 403})
+
+        :else
+        (handler request)))))
+
+(def sec-fetch-metadata
+  {:name ::sec-fetch-metadata
+   :compile (fn [_ _]
+              (when (contains? cf/flags :sec-fetch-metadata-middleware)
+                wrap-sec-fetch-metadata))})
+
+(defn- wrap-client-header-check
+  "Check for a penpot custom header to be present as additional CSRF
+  protection"
+  [handler]
+  (fn [request]
+    (let [client (yreq/get-header request "x-client")]
+      (if (some? client)
+        (handler request)
+        {::yres/status 403}))))
+
+(def client-header-check
+  {:name ::client-header-check
+   :compile (fn [_ _]
+              (when (contains? cf/flags :client-header-check-middleware)
+                wrap-client-header-check))})

backend/src/app/http/session.clj

@@ -11,7 +11,6 @@
    [app.common.logging :as l]
    [app.common.schema :as sm]
    [app.common.time :as ct]
-   [app.common.uri :as u]
    [app.config :as cf]
    [app.db :as db]
    [app.db.sql :as sql]
@@ -148,9 +147,7 @@
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
 (declare ^:private assign-auth-token-cookie)
-(declare ^:private assign-auth-data-cookie)
 (declare ^:private clear-auth-token-cookie)
-(declare ^:private clear-auth-data-cookie)
 (declare ^:private gen-token)
 
 (defn create-fn
@@ -164,10 +161,9 @@
                    :user-agent uagent}
           token   (gen-token cfg params)
           session (write! manager token params)]
-      (l/trace :hint "create" :profile-id (str profile-id))
+      (l/trc :hint "create" :profile-id (str profile-id))
       (-> response
-          (assign-auth-token-cookie session)
+          (assign-auth-token-cookie session)))))
-          (assign-auth-data-cookie session)))))
 
 (defn delete-fn
   [{:keys [::manager]}]
@@ -175,13 +171,12 @@
   (fn [request response]
     (let [cname   (cf/get :auth-token-cookie-name default-auth-token-cookie-name)
           cookie  (yreq/get-cookie request cname)]
-      (l/trace :hint "delete" :profile-id (:profile-id request))
+      (l/trc :hint "delete" :profile-id (:profile-id request))
       (some->> (:value cookie) (delete! manager))
       (-> response
           (assoc :status 204)
           (assoc :body nil)
-          (clear-auth-token-cookie)
+          (clear-auth-token-cookie)))))
-          (clear-auth-data-cookie)))))
 
 (defn- gen-token
   [cfg {:keys [profile-id created-at]}]
@@ -222,7 +217,7 @@
                   (-> (assoc ::token-claims claims)
                       (assoc ::token token))))
               (catch Throwable cause
-                (l/trace :hint "exception on decoding malformed token" :cause cause)
+                (l/trc :hint "exception on decoding malformed token" :cause cause)
                 request)))]
 
     (fn [request]
@@ -242,8 +237,7 @@
       (if (renew-session? session)
         (let [session (update! manager session)]
           (-> response
-              (assign-auth-token-cookie session)
+              (assign-auth-token-cookie session)))
-              (assign-auth-data-cookie session)))
         response))))
 
 (def soft-auth
@@ -276,46 +270,11 @@
                     :secure secure?}]
     (update response :cookies assoc name cookie)))
 
-(defn- assign-auth-data-cookie
-  [response {profile-id :profile-id updated-at :updated-at}]
-  (let [max-age    (cf/get :auth-token-cookie-max-age default-cookie-max-age)
-        domain     (cf/get :auth-data-cookie-domain)
-        cname      default-auth-data-cookie-name
-
-        created-at updated-at
-        renewal    (ct/plus created-at default-renewal-max-age)
-        expires    (ct/plus created-at max-age)
-
-        comment    (str "Renewal at: " (ct/format-inst renewal :rfc1123))
-        secure?    (contains? cf/flags :secure-session-cookies)
-        strict?    (contains? cf/flags :strict-session-cookies)
-        cors?      (contains? cf/flags :cors)
-
-        cookie     {:domain domain
-                    :expires expires
-                    :path "/"
-                    :comment comment
-                    :value (u/map->query-string {:profile-id profile-id})
-                    :same-site (if cors? :none (if strict? :strict :lax))
-                    :secure secure?}]
-
-    (cond-> response
-      (string? domain)
-      (update :cookies assoc cname cookie))))
-
 (defn- clear-auth-token-cookie
   [response]
   (let [cname (cf/get :auth-token-cookie-name default-auth-token-cookie-name)]
     (update response :cookies assoc cname {:path "/" :value "" :max-age 0})))
 
-(defn- clear-auth-data-cookie
-  [response]
-  (let [cname  default-auth-data-cookie-name
-        domain (cf/get :auth-data-cookie-domain)]
-    (cond-> response
-      (string? domain)
-      (update :cookies assoc cname {:domain domain :path "/" :value "" :max-age 0}))))
-
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; TASK: SESSION GC
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
@@ -342,13 +301,13 @@
   (let [threshold (ct/minus (ct/now) max-age)
         result    (-> (db/exec-one! conn [sql:delete-expired threshold threshold])
                       (db/get-update-count))]
-    (l/debug :task "gc"
+    (l/dbg :task "gc"
-             :hint "clean http sessions"
+           :hint "clean http sessions"
-             :deleted result)
+           :deleted result)
     result))
 
 (defmethod ig/init-key ::tasks/gc
   [_ {:keys [::tasks/max-age] :as cfg}]
-  (l/debug :hint "initializing session gc task" :max-age max-age)
+  (l/dbg :hint "initializing session gc task" :max-age max-age)
   (fn [_]
     (db/tx-run! cfg collect-expired-tasks)))

backend/test/backend_tests/http_middleware_security.clj

@@ -0,0 +1,59 @@
+;; This Source Code Form is subject to the terms of the Mozilla Public
+;; License, v. 2.0. If a copy of the MPL was not distributed with this
+;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
+;;
+;; Copyright (c) KALEIDOS INC
+
+(ns backend-tests.http-middleware-security
+  (:require
+   [app.http.security :as sec]
+   [clojure.test :as t]
+   [yetti.request :as yreq]
+   [yetti.response :as yres]))
+
+(defn- mock-request
+  [method value]
+  (reify yreq/IRequest
+    (method [_]
+      method)
+    (get-header [_ _]
+      value)))
+
+(t/deftest sec-fetch-metadata
+  (let [request1 (mock-request :get "same-origin")
+        request2 (mock-request :post "same-origin")
+        request3 (mock-request :get "same-site")
+        request4 (mock-request :post "same-site")
+        request5 (mock-request :get "cross-site")
+        request6 (mock-request :post "cross-site")
+
+        handler  (fn [request]
+                   {::yres/status 200})
+        handler  (#'sec/wrap-sec-fetch-metadata handler)
+        resp1    (handler request1)
+        resp2    (handler request2)
+        resp3    (handler request3)
+        resp4    (handler request4)
+        resp5    (handler request5)
+        resp6    (handler request6)]
+
+    (t/is (= 200 (::yres/status resp1)))
+    (t/is (= 200 (::yres/status resp2)))
+    (t/is (= 200 (::yres/status resp3)))
+    (t/is (= 403 (::yres/status resp4)))
+    (t/is (= 200 (::yres/status resp5)))
+    (t/is (= 403 (::yres/status resp6)))))
+
+(t/deftest client-header-check
+  (let [request1 (mock-request :get "some")
+        request2 (mock-request :post nil)
+
+        handler  (fn [request]
+                   {::yres/status 200})
+        handler  (#'sec/wrap-client-header-check handler)
+        resp1    (handler request1)
+        resp2    (handler request2)]
+
+    (t/is (= 200 (::yres/status resp1)))
+    (t/is (= 403 (::yres/status resp2)))))
+

common/src/app/common/flags.cljc

@@ -135,7 +135,15 @@
     :subscriptions
     :subscriptions-old
     :frontend-binary-fills
-    :inspect-styles})
+    :inspect-styles
+
+    ;; Security layer middleware that filters request by fetch
+    ;; metadata headers
+    :sec-fetch-metadata-middleware
+
+    ;; Security layer middleware that check the precense of x-client
+    ;; http headers and enables an addtional csrf protection
+    :client-header-check-middleware})
 
 (def all-flags
   (set/union email login varia))

frontend/src/app/util/http.cljs

@@ -51,7 +51,8 @@
 
 (defn default-headers
   []
-  {"x-frontend-version" (:full cfg/version)})
+  {"x-frontend-version" (:full cfg/version)
+   "x-client" (str "penpot-frontend/" (:full cfg/version))})
 
 (defn fetch
   [{:keys [method uri query headers body mode omit-default-headers credentials]