mirror of
https://github.com/penpot/penpot.git
synced 2026-09-10 22:19:19 +00:00
🐛 Add ssrf check for nitrate sso and add timeouts to http client (#11576)
This commit is contained in:
parent
8a3540336b
commit
d263c23a58
@ -15,6 +15,7 @@
|
||||
(:require
|
||||
[app.common.schema :as sm]
|
||||
[app.util.ssrf :as ssrf]
|
||||
[app.worker :as-alias wrk]
|
||||
[cuerdas.core :as str]
|
||||
[integrant.core :as ig]
|
||||
[java-http-clj.core :as http])
|
||||
@ -23,6 +24,8 @@
|
||||
java.net.URI))
|
||||
|
||||
(def default-max-redirects 5)
|
||||
(def default-connect-timeout 30000)
|
||||
(def default-request-timeout 30000)
|
||||
|
||||
(defn client?
|
||||
[o]
|
||||
@ -33,15 +36,17 @@
|
||||
:pred client?})
|
||||
|
||||
(defmethod ig/init-key ::client
|
||||
[_ _]
|
||||
(http/build-client {:connect-timeout 30000
|
||||
[_ {:keys [::wrk/executor]}]
|
||||
(http/build-client {:connect-timeout default-connect-timeout
|
||||
:executor executor
|
||||
:follow-redirects :never}))
|
||||
|
||||
(defn send!
|
||||
([client req] (send! client req {}))
|
||||
([client req {:keys [response-type] :or {response-type :string}}]
|
||||
(assert (client? client) "expected valid http client")
|
||||
(http/send req {:client client :as response-type})))
|
||||
(http/send (merge {:timeout default-request-timeout} req)
|
||||
{:client client :as response-type})))
|
||||
|
||||
(defn- resolve-client
|
||||
[params]
|
||||
|
||||
@ -200,7 +200,7 @@
|
||||
{::db/pool (ig/ref ::db/pool)}
|
||||
|
||||
::http.client/client
|
||||
{}
|
||||
{::wrk/executor (ig/ref ::wrk/executor)}
|
||||
|
||||
::session/manager
|
||||
{::db/pool (ig/ref ::db/pool)}
|
||||
|
||||
@ -75,10 +75,10 @@
|
||||
{:method method
|
||||
:uri uri
|
||||
:body body
|
||||
:headers headers}
|
||||
:headers headers
|
||||
:timeout timeout}
|
||||
{:response-type :input-stream
|
||||
:skip-ssrf-check? true
|
||||
:timeout timeout})
|
||||
:skip-ssrf-check? true})
|
||||
status (:status resp)]
|
||||
(when (not (<= 200 status 299))
|
||||
(let [body (:body resp)]
|
||||
|
||||
@ -41,6 +41,7 @@
|
||||
[app.rpc.notifications :as notifications]
|
||||
[app.storage :as sto]
|
||||
[app.util.services :as sv]
|
||||
[app.util.ssrf :as ssrf]
|
||||
[app.worker :as wrk]
|
||||
[cuerdas.core :as str]))
|
||||
|
||||
@ -960,13 +961,18 @@ RETURNING id, deleted_at;")
|
||||
(sv/defmethod ::check-organization-sso
|
||||
"Validate an organization SSO configuration by generating a login redirect URL.
|
||||
Nitrate calls this while configuring SSO to verify client credentials and OIDC
|
||||
discovery before saving the settings."
|
||||
discovery before saving the settings. The issuer URL is nitrate-supplied
|
||||
(customer-configured), so it is checked against the SSRF blocklist before
|
||||
any outbound request is attempted."
|
||||
{::doc/added "2.18"
|
||||
::sm/params cto/schema:nitrate-sso
|
||||
::sm/result schema:check-organization-sso-result
|
||||
::rpc/auth false}
|
||||
[cfg params]
|
||||
{:valid (oidc/is-organization-sso-config-valid? cfg params)})
|
||||
(let [issuer (oidc/organization-sso-discovery-uri params)]
|
||||
{:valid (boolean (and issuer
|
||||
(ssrf/safe-url? issuer)
|
||||
(oidc/is-organization-sso-config-valid? cfg params)))}))
|
||||
|
||||
;; ---- API: notify-organization-sso-change
|
||||
(sv/defmethod ::notify-organization-sso-change
|
||||
|
||||
30
backend/test/backend_tests/http_client_test.clj
Normal file
30
backend/test/backend_tests/http_client_test.clj
Normal file
@ -0,0 +1,30 @@
|
||||
;; 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 Sucursal en España SL
|
||||
|
||||
(ns backend-tests.http-client-test
|
||||
(:require
|
||||
[app.http.client :as http]
|
||||
[clojure.test :as t]
|
||||
[java-http-clj.core :as jhttp]
|
||||
[mockery.core :refer [with-mocks]]))
|
||||
|
||||
(t/deftest send-injects-default-timeout-when-absent
|
||||
(with-mocks [mock {:target 'java-http-clj.core/send
|
||||
:return {:status 200 :body ""}}]
|
||||
(let [client (jhttp/build-client {})]
|
||||
(http/send! client {:method :get :uri "https://example.com/"})
|
||||
(let [[req _opts] (:call-args @mock)]
|
||||
(t/is (= http/default-request-timeout (:timeout req)))))))
|
||||
|
||||
(t/deftest send-preserves-caller-supplied-timeout
|
||||
(with-mocks [mock {:target 'java-http-clj.core/send
|
||||
:return {:status 200 :body ""}}]
|
||||
(let [client (jhttp/build-client {})]
|
||||
(http/send! client {:method :get
|
||||
:uri "https://example.com/"
|
||||
:timeout 5000})
|
||||
(let [[req _opts] (:call-args @mock)]
|
||||
(t/is (= 5000 (:timeout req)))))))
|
||||
@ -8,6 +8,7 @@
|
||||
(:require
|
||||
[app.common.exceptions :as ex]
|
||||
[app.config :as cf]
|
||||
[app.http.client :as http]
|
||||
[app.media.remote :as media.remote]
|
||||
[app.setup :as-alias setup]
|
||||
[app.util.json :as json]
|
||||
@ -500,6 +501,22 @@
|
||||
:headers {}})]
|
||||
(t/is (= 200 (:status resp))))))))
|
||||
|
||||
(t/deftest service-request-puts-configured-timeout-in-request
|
||||
(t/testing "service-request puts media-processing-service-timeout on the http request"
|
||||
(let [captured (atom nil)]
|
||||
(with-redefs [cf/get (th/config-get-mock config-mock)
|
||||
http/req (fn [_client request _opts]
|
||||
(reset! captured request)
|
||||
{:status 200
|
||||
:body (json-stream {:width 100 :height 100})})]
|
||||
(media.remote/service-request
|
||||
(mk-system)
|
||||
{:method :post
|
||||
:uri "http://localhost:6065/api/image/info"
|
||||
:body nil
|
||||
:headers {}})
|
||||
(t/is (= 5000 (:timeout @captured)))))))
|
||||
|
||||
;; ---------------------------------------------------------------------------
|
||||
;; Shared key
|
||||
;; ---------------------------------------------------------------------------
|
||||
|
||||
@ -17,6 +17,7 @@
|
||||
[app.msgbus :as mbus]
|
||||
[app.nitrate :as nitrate]
|
||||
[app.rpc :as-alias rpc]
|
||||
[app.util.ssrf :as ssrf]
|
||||
[app.worker :as wrk]
|
||||
[backend-tests.helpers :as th]
|
||||
[clojure.set :as set]
|
||||
@ -1806,13 +1807,14 @@
|
||||
|
||||
(t/deftest check-organization-sso-returns-valid-true
|
||||
(let [organization-id (uuid/random)
|
||||
out (with-redefs [oidc/is-organization-sso-config-valid? (constantly true)]
|
||||
(th/management-command!
|
||||
{::th/type :check-organization-sso
|
||||
:organization-id organization-id
|
||||
:client-id "test-client"
|
||||
:client-secret "test-secret"
|
||||
:issuer "https://idp.example.com"}))]
|
||||
out (with-redefs [ssrf/safe-url? (constantly true)
|
||||
oidc/is-organization-sso-config-valid? (constantly true)]
|
||||
(th/management-command!
|
||||
{::th/type :check-organization-sso
|
||||
:organization-id organization-id
|
||||
:client-id "test-client"
|
||||
:client-secret "test-secret"
|
||||
:issuer "https://idp.example.com"}))]
|
||||
(t/is (th/success? out))
|
||||
(t/is (true? (-> out :result :valid)))))
|
||||
|
||||
@ -1827,19 +1829,36 @@
|
||||
|
||||
(t/deftest check-organization-sso-passes-issuer-to-validation
|
||||
(let [organization-id (uuid/random)
|
||||
out (with-redefs [oidc/is-organization-sso-config-valid?
|
||||
(fn [_cfg sso]
|
||||
(and (= "test-client" (:client-id sso))
|
||||
(= "https://idp.example.com/" (:issuer sso))))]
|
||||
(th/management-command!
|
||||
{::th/type :check-organization-sso
|
||||
:organization-id organization-id
|
||||
:client-id "test-client"
|
||||
:client-secret "test-secret"
|
||||
:issuer "https://idp.example.com/"}))]
|
||||
out (with-redefs [ssrf/safe-url? (constantly true)
|
||||
oidc/is-organization-sso-config-valid?
|
||||
(fn [_cfg sso]
|
||||
(and (= "test-client" (:client-id sso))
|
||||
(= "https://idp.example.com/" (:issuer sso))))]
|
||||
(th/management-command!
|
||||
{::th/type :check-organization-sso
|
||||
:organization-id organization-id
|
||||
:client-id "test-client"
|
||||
:client-secret "test-secret"
|
||||
:issuer "https://idp.example.com/"}))]
|
||||
(t/is (th/success? out))
|
||||
(t/is (true? (-> out :result :valid)))))
|
||||
|
||||
(t/deftest check-organization-sso-returns-valid-false-on-ssrf-blocked-issuer
|
||||
(t/testing "an SSRF-blocked issuer must not reach the OIDC validation flow"
|
||||
(let [called? (atom false)
|
||||
out (with-redefs [oidc/is-organization-sso-config-valid?
|
||||
(fn [_cfg _sso] (reset! called? true) true)]
|
||||
(th/management-command!
|
||||
{::th/type :check-organization-sso
|
||||
:organization-id (uuid/random)
|
||||
:client-id "test-client"
|
||||
:client-secret "test-secret"
|
||||
:issuer "http://127.0.0.1/idp"}))]
|
||||
(t/is (th/success? out))
|
||||
(t/is (false? (-> out :result :valid)))
|
||||
(t/is (false? @called?)
|
||||
"OIDC validation should not run when the issuer is SSRF-blocked"))))
|
||||
|
||||
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
|
||||
;; PUSH AUDIT EVENTS
|
||||
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user