mirror of
https://github.com/penpot/penpot.git
synced 2026-06-09 08:52:05 +00:00
🐛 Fix CORS middleware reflecting arbitrary origins (#9675)
* ⏪ Align profile and dashboard files with penpot develop * 🐛 Fix CORS origin allowlist for issue #9659 --------- Signed-off-by: Chan <101856681+enjoyandlove@users.noreply.github.com> Co-authored-by: Andrey Antukh <niwi@niwi.nz>
This commit is contained in:
parent
b08ceca81d
commit
ac3950e36c
@ -109,6 +109,11 @@
|
||||
[:http-server-io-threads {:optional true} ::sm/int]
|
||||
[:http-server-max-worker-threads {:optional true} ::sm/int]
|
||||
|
||||
;; Explicit CORS allowlist used when the :cors flag is enabled.
|
||||
;; Configured via PENPOT_ALLOWED_ORIGINS as a comma/whitespace
|
||||
;; separated list of origins (e.g. "https://plugins.example.com").
|
||||
[:allowed-origins {:optional true} [::sm/set :string]]
|
||||
|
||||
[:exporter-shared-key {:optional true} :string]
|
||||
[:nitrate-shared-key {:optional true} :string]
|
||||
[:nexus-shared-key {:optional true} :string]
|
||||
|
||||
@ -208,28 +208,40 @@
|
||||
:compile (constantly wrap-errors)})
|
||||
|
||||
(defn- with-cors-headers
|
||||
[headers origin]
|
||||
(-> headers
|
||||
(assoc "access-control-allow-origin" origin)
|
||||
(assoc "access-control-allow-methods" "GET,POST,DELETE,OPTIONS,PUT,HEAD,PATCH")
|
||||
(assoc "access-control-allow-credentials" "true")
|
||||
(assoc "access-control-expose-headers" "content-type, set-cookie")
|
||||
(assoc "access-control-allow-headers" "x-frontend-version, x-client, x-requested-width, content-type, accept, cookie")))
|
||||
"Build CORS response headers. Only emits permissive headers when the
|
||||
request `origin` is present on the configured `allowed` allowlist;
|
||||
otherwise returns the headers unchanged except for `Vary: Origin` so
|
||||
shared caches don't leak per-origin responses."
|
||||
[headers origin allowed]
|
||||
(cond-> (assoc headers "vary" "Origin")
|
||||
(and (some? origin) (contains? allowed origin))
|
||||
(-> (assoc "access-control-allow-origin" origin)
|
||||
(assoc "access-control-allow-credentials" "true")
|
||||
(assoc "access-control-allow-methods" "GET,POST,DELETE,OPTIONS,PUT,HEAD,PATCH")
|
||||
(assoc "access-control-expose-headers" "content-type")
|
||||
(assoc "access-control-allow-headers" "x-frontend-version, x-client, content-type, accept"))))
|
||||
|
||||
(defn wrap-cors
|
||||
[handler]
|
||||
[handler allowed]
|
||||
(fn [request]
|
||||
(let [response (if (= (yreq/method request) :options)
|
||||
{::yres/status 204}
|
||||
(handler request))
|
||||
origin (yreq/get-header request "origin")]
|
||||
(update response ::yres/headers with-cors-headers origin))))
|
||||
(update response ::yres/headers with-cors-headers origin allowed))))
|
||||
|
||||
(def cors
|
||||
{:name ::cors
|
||||
:compile (fn [& _]
|
||||
(when (contains? cf/flags :cors)
|
||||
wrap-cors))})
|
||||
(let [allowed (not-empty (cf/get :allowed-origins))]
|
||||
(if allowed
|
||||
(fn [handler] (wrap-cors handler allowed))
|
||||
(do
|
||||
(l/wrn :hint (str "cors flag is enabled but :allowed-origins is empty; "
|
||||
"CORS middleware disabled (fail-closed). "
|
||||
"Configure PENPOT_ALLOWED_ORIGINS with a comma-separated list of trusted origins."))
|
||||
nil)))))})
|
||||
|
||||
(def restrict-methods
|
||||
{:name ::restrict-methods
|
||||
|
||||
@ -17,6 +17,7 @@
|
||||
[app.rpc.commands.access-token]
|
||||
[app.tokens :as tokens]
|
||||
[backend-tests.helpers :as th]
|
||||
[clojure.string :as str]
|
||||
[clojure.test :as t]
|
||||
[mockery.core :refer [with-mocks]]
|
||||
[yetti.request :as yreq]
|
||||
@ -112,6 +113,74 @@
|
||||
(t/is (= #{} (:app.http.access-token/perms response)))
|
||||
(t/is (= (:id profile) (:app.http.access-token/profile-id response))))))
|
||||
|
||||
(defrecord MethodAwareDummyRequest [req-method headers]
|
||||
yreq/IRequest
|
||||
(method [_] req-method)
|
||||
(get-header [_ name] (get headers name)))
|
||||
|
||||
(t/deftest cors-middleware-allowlisted-origin
|
||||
(let [handler (#'app.http.middleware/wrap-cors
|
||||
(fn [_] {::yres/status 200 ::yres/headers {}})
|
||||
#{"https://trusted.example"})
|
||||
resp (handler (->MethodAwareDummyRequest :get {"origin" "https://trusted.example"}))
|
||||
headers (::yres/headers resp)]
|
||||
|
||||
(t/is (= 200 (::yres/status resp)))
|
||||
(t/is (= "https://trusted.example" (get headers "access-control-allow-origin")))
|
||||
(t/is (= "true" (get headers "access-control-allow-credentials")))
|
||||
(t/is (= "Origin" (get headers "vary")))
|
||||
(t/is (= "content-type" (get headers "access-control-expose-headers")))
|
||||
(t/is (not (str/includes?
|
||||
(get headers "access-control-allow-headers" "")
|
||||
"cookie")))))
|
||||
|
||||
(t/deftest cors-middleware-non-allowlisted-origin
|
||||
(let [handler (#'app.http.middleware/wrap-cors
|
||||
(fn [_] {::yres/status 200 ::yres/headers {}})
|
||||
#{"https://trusted.example"})
|
||||
resp (handler (->MethodAwareDummyRequest :get {"origin" "https://attacker.example"}))
|
||||
headers (::yres/headers resp)]
|
||||
|
||||
(t/is (= 200 (::yres/status resp)))
|
||||
(t/is (nil? (get headers "access-control-allow-origin")))
|
||||
(t/is (nil? (get headers "access-control-allow-credentials")))
|
||||
(t/is (nil? (get headers "access-control-allow-headers")))
|
||||
(t/is (nil? (get headers "access-control-expose-headers")))
|
||||
(t/is (= "Origin" (get headers "vary")))))
|
||||
|
||||
(t/deftest cors-middleware-preflight-allowlisted
|
||||
(let [handler (#'app.http.middleware/wrap-cors
|
||||
(fn [_] {::yres/status 200 ::yres/headers {}})
|
||||
#{"https://trusted.example"})
|
||||
resp (handler (->MethodAwareDummyRequest :options {"origin" "https://trusted.example"}))
|
||||
headers (::yres/headers resp)]
|
||||
|
||||
(t/is (= 204 (::yres/status resp)))
|
||||
(t/is (= "https://trusted.example" (get headers "access-control-allow-origin")))
|
||||
(t/is (= "true" (get headers "access-control-allow-credentials")))))
|
||||
|
||||
(t/deftest cors-middleware-preflight-non-allowlisted
|
||||
(let [handler (#'app.http.middleware/wrap-cors
|
||||
(fn [_] {::yres/status 200 ::yres/headers {}})
|
||||
#{"https://trusted.example"})
|
||||
resp (handler (->MethodAwareDummyRequest :options {"origin" "https://attacker.example"}))
|
||||
headers (::yres/headers resp)]
|
||||
|
||||
(t/is (= 204 (::yres/status resp)))
|
||||
(t/is (nil? (get headers "access-control-allow-origin")))
|
||||
(t/is (nil? (get headers "access-control-allow-credentials")))))
|
||||
|
||||
(t/deftest cors-middleware-missing-origin
|
||||
(let [handler (#'app.http.middleware/wrap-cors
|
||||
(fn [_] {::yres/status 200 ::yres/headers {}})
|
||||
#{"https://trusted.example"})
|
||||
resp (handler (->MethodAwareDummyRequest :get {}))
|
||||
headers (::yres/headers resp)]
|
||||
|
||||
(t/is (= 200 (::yres/status resp)))
|
||||
(t/is (nil? (get headers "access-control-allow-origin")))
|
||||
(t/is (nil? (get headers "access-control-allow-credentials")))))
|
||||
|
||||
(t/deftest session-authz
|
||||
(let [cfg th/*system*
|
||||
manager (session/inmemory-manager)
|
||||
|
||||
@ -72,7 +72,11 @@
|
||||
:backend-worker
|
||||
;; Only for development
|
||||
:component-thumbnails
|
||||
;; enables the default cors configuration that allows all domains (currently this configuration is only used for development).
|
||||
;; Enables CORS support for the RPC API. Requires an explicit
|
||||
;; allowlist of origins via PENPOT_ALLOWED_ORIGINS; if no allowlist
|
||||
;; is configured the middleware fails closed (a warning is logged
|
||||
;; and CORS headers are not emitted) to avoid CSRF / data
|
||||
;; exfiltration via origin reflection.
|
||||
:cors
|
||||
;; Enables the templates dialog on Penpot dashboard.
|
||||
:dashboard-templates-section
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user