mirror of
https://github.com/penpot/penpot.git
synced 2026-10-11 21:26:16 +00:00
✨ Add and endpoint for nitrate to check the SSO configuration for an organization (#10432)
This commit is contained in:
parent
d328cb4a9e
commit
6e61e3304b
@ -761,6 +761,16 @@
|
|||||||
;; ORG SSO HELPERS
|
;; ORG SSO HELPERS
|
||||||
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
|
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
|
||||||
|
|
||||||
|
(defn- non-blank-uri
|
||||||
|
[value]
|
||||||
|
(when-not (str/blank? value) value))
|
||||||
|
|
||||||
|
(defn org-sso-discovery-uri
|
||||||
|
"Return the OIDC discovery URI from an org SSO config, preferring :issuer."
|
||||||
|
[sso]
|
||||||
|
(or (non-blank-uri (:issuer sso))
|
||||||
|
(non-blank-uri (:base-url sso))))
|
||||||
|
|
||||||
(defn prepare-org-sso-provider
|
(defn prepare-org-sso-provider
|
||||||
"Build an OIDC provider map dynamically from the Nitrate org SSO config.
|
"Build an OIDC provider map dynamically from the Nitrate org SSO config.
|
||||||
Uses OIDC discovery via :base-url (or :issuer as fallback) when
|
Uses OIDC discovery via :base-url (or :issuer as fallback) when
|
||||||
@ -770,12 +780,96 @@
|
|||||||
{:type "oidc"
|
{:type "oidc"
|
||||||
:client-id client-id
|
:client-id client-id
|
||||||
:client-secret client-secret
|
:client-secret client-secret
|
||||||
:base-uri (some-> (or base-url issuer)
|
:base-uri (some-> (or (non-blank-uri base-url)
|
||||||
|
(non-blank-uri issuer))
|
||||||
(str/rtrim "/")
|
(str/rtrim "/")
|
||||||
(str "/"))
|
(str "/"))
|
||||||
:scopes (into default-oidc-scopes (or scopes #{}))
|
:scopes (into default-oidc-scopes (or scopes #{}))
|
||||||
:skip-ssrf-check? true}))
|
:skip-ssrf-check? true}))
|
||||||
|
|
||||||
|
(defn build-org-sso-auth-redirect-uri
|
||||||
|
"Build the OIDC authorization redirect URI for an organization SSO config.
|
||||||
|
Raises if the config is incomplete or OIDC discovery fails."
|
||||||
|
[cfg sso & {:keys [dest-url organization-id provider]}]
|
||||||
|
(let [organization-id (or organization-id (:organization-id sso))
|
||||||
|
issuer (org-sso-discovery-uri sso)
|
||||||
|
dest-url (or dest-url (str (cf/get :public-uri)))]
|
||||||
|
(when-not issuer
|
||||||
|
(ex/raise :type :validation
|
||||||
|
:code :invalid-sso-config
|
||||||
|
:hint "missing issuer or base-url"))
|
||||||
|
(let [oidc-provider (or provider (prepare-org-sso-provider cfg sso))
|
||||||
|
state-token (tokens/generate cfg {:iss "oidc"
|
||||||
|
:dest-url dest-url
|
||||||
|
:organization-id organization-id
|
||||||
|
:issuer issuer
|
||||||
|
:exp (ct/in-future "4h")})]
|
||||||
|
(build-auth-redirect-uri oidc-provider state-token))))
|
||||||
|
|
||||||
|
(def ^:private probe-auth-code "penpot-sso-config-probe")
|
||||||
|
|
||||||
|
(defn- decode-token-error-response
|
||||||
|
[body]
|
||||||
|
(when (and (string? body) (pos? (count body)))
|
||||||
|
(try
|
||||||
|
(json/decode body)
|
||||||
|
(catch Throwable _ nil))))
|
||||||
|
|
||||||
|
(defn- token-endpoint-error
|
||||||
|
[response]
|
||||||
|
(some-> response :body decode-token-error-response :error d/name))
|
||||||
|
|
||||||
|
(defn- token-endpoint-error-description
|
||||||
|
[response]
|
||||||
|
(some-> response :body decode-token-error-response :error-description))
|
||||||
|
|
||||||
|
(defn- token-endpoint-valid-client-error?
|
||||||
|
"Token endpoint rejected the dummy auth code but accepted the client credentials."
|
||||||
|
[response]
|
||||||
|
(= "invalid_grant" (token-endpoint-error response)))
|
||||||
|
|
||||||
|
(defn- token-endpoint-invalid-client-error?
|
||||||
|
"Token endpoint rejected the client credentials."
|
||||||
|
[{:keys [status] :as response}]
|
||||||
|
(let [error (token-endpoint-error response)
|
||||||
|
description (str/lower (or (token-endpoint-error-description response) ""))]
|
||||||
|
(or (= status 401)
|
||||||
|
(#{"invalid_client" "unauthorized_client"} error)
|
||||||
|
(and (= error "access_denied")
|
||||||
|
(str/includes? description "unauthorized")))))
|
||||||
|
|
||||||
|
(defn- probe-org-sso-client-credentials
|
||||||
|
"Probe the token endpoint with a dummy authorization code.
|
||||||
|
Valid client credentials are expected to answer with `invalid_grant`."
|
||||||
|
[cfg provider]
|
||||||
|
(let [params {:client_id (:client-id provider)
|
||||||
|
:client_secret (:client-secret provider)
|
||||||
|
:code probe-auth-code
|
||||||
|
:grant_type "authorization_code"
|
||||||
|
:redirect_uri (build-redirect-uri)}
|
||||||
|
req {:method :post
|
||||||
|
:headers {"content-type" "application/x-www-form-urlencoded"
|
||||||
|
"accept" "application/json"}
|
||||||
|
:uri (:token-uri provider)
|
||||||
|
:body (u/map->query-string params)}
|
||||||
|
response (http/req cfg req {:skip-ssrf-check? (:skip-ssrf-check? provider)})]
|
||||||
|
(cond
|
||||||
|
(token-endpoint-valid-client-error? response) true
|
||||||
|
(token-endpoint-invalid-client-error? response) false
|
||||||
|
:else false)))
|
||||||
|
|
||||||
|
(defn is-organization-sso-config-valid?
|
||||||
|
"Return true when the SSO config can be discovered, can build a login URL,
|
||||||
|
and the client credentials are accepted by the token endpoint."
|
||||||
|
[cfg sso]
|
||||||
|
(try
|
||||||
|
(if (org-sso-discovery-uri sso)
|
||||||
|
(let [provider (prepare-org-sso-provider cfg sso)]
|
||||||
|
(and (build-org-sso-auth-redirect-uri cfg sso :provider provider)
|
||||||
|
(probe-org-sso-client-credentials cfg provider)))
|
||||||
|
false)
|
||||||
|
(catch Throwable _ false)))
|
||||||
|
|
||||||
(defn- auth-handler
|
(defn- auth-handler
|
||||||
[cfg {:keys [params] :as request}]
|
[cfg {:keys [params] :as request}]
|
||||||
(let [provider (resolve-provider cfg params)
|
(let [provider (resolve-provider cfg params)
|
||||||
|
|||||||
@ -88,7 +88,8 @@
|
|||||||
#{:session-id
|
#{:session-id
|
||||||
:password
|
:password
|
||||||
:old-password
|
:old-password
|
||||||
:token})
|
:token
|
||||||
|
:client-secret})
|
||||||
|
|
||||||
(defn extract-utm-params
|
(defn extract-utm-params
|
||||||
"Extracts additional data from params and namespace them under
|
"Extracts additional data from params and namespace them under
|
||||||
|
|||||||
@ -14,7 +14,8 @@
|
|||||||
[app.common.schema :as sm]
|
[app.common.schema :as sm]
|
||||||
[app.common.schema.generators :as sg]
|
[app.common.schema.generators :as sg]
|
||||||
[app.common.time :as ct]
|
[app.common.time :as ct]
|
||||||
[app.common.types.organization :as cto]
|
[app.common.types.organization :as cto
|
||||||
|
:refer [schema:nitrate-sso]]
|
||||||
[app.common.uri :as u]
|
[app.common.uri :as u]
|
||||||
[app.config :as cf]
|
[app.config :as cf]
|
||||||
[app.http.client :as http]
|
[app.http.client :as http]
|
||||||
@ -430,17 +431,6 @@
|
|||||||
[:permissions [:map-of :keyword :string]]]
|
[:permissions [:map-of :keyword :string]]]
|
||||||
params))
|
params))
|
||||||
|
|
||||||
(def ^:private schema:nitrate-sso
|
|
||||||
[:map
|
|
||||||
[:organization-id ::sm/uuid]
|
|
||||||
[:active [:maybe :boolean]]
|
|
||||||
[:provider [:maybe :string]]
|
|
||||||
[:client-id [:maybe :string]]
|
|
||||||
[:base-url [:maybe :string]]
|
|
||||||
[:client-secret [:maybe :string]]
|
|
||||||
[:issuer [:maybe :string]]
|
|
||||||
[:scopes [:maybe [::sm/set ::sm/text]]]])
|
|
||||||
|
|
||||||
(defn- get-org-sso-api
|
(defn- get-org-sso-api
|
||||||
"Fetches the SSO configuration for an organization from Nitrate."
|
"Fetches the SSO configuration for an organization from Nitrate."
|
||||||
[cfg {:keys [organization-id] :as params}]
|
[cfg {:keys [organization-id] :as params}]
|
||||||
|
|||||||
@ -24,7 +24,6 @@
|
|||||||
[app.rpc.nitrate.emails-helper :as neh]
|
[app.rpc.nitrate.emails-helper :as neh]
|
||||||
[app.rpc.nitrate.organization-helper :as noh]
|
[app.rpc.nitrate.organization-helper :as noh]
|
||||||
[app.rpc.notifications :as notifications]
|
[app.rpc.notifications :as notifications]
|
||||||
[app.tokens :as tokens]
|
|
||||||
[app.util.services :as sv]))
|
[app.util.services :as sv]))
|
||||||
|
|
||||||
|
|
||||||
@ -647,17 +646,11 @@
|
|||||||
{:keys [authorized sso]} (nitrate/sso-session-authorized? cfg organization-id team-id request)]
|
{:keys [authorized sso]} (nitrate/sso-session-authorized? cfg organization-id team-id request)]
|
||||||
(if authorized
|
(if authorized
|
||||||
{:authorized true}
|
{:authorized true}
|
||||||
(if-let [issuer (or (:issuer sso) (:base-url sso))]
|
(if (oidc/org-sso-discovery-uri sso)
|
||||||
(let [oidc-provider (oidc/prepare-org-sso-provider cfg sso)
|
{:authorized false
|
||||||
org-id (or organization-id (:organization-id sso))
|
:redirect-uri (oidc/build-org-sso-auth-redirect-uri cfg sso
|
||||||
state-token (tokens/generate cfg {:iss "oidc"
|
:dest-url url
|
||||||
:dest-url url
|
:organization-id organization-id)}
|
||||||
:organization-id org-id
|
|
||||||
:issuer issuer
|
|
||||||
:exp (ct/in-future "4h")})
|
|
||||||
redirect-uri (oidc/build-auth-redirect-uri oidc-provider state-token)]
|
|
||||||
{:authorized false
|
|
||||||
:redirect-uri redirect-uri})
|
|
||||||
{:authorized false
|
{:authorized false
|
||||||
:redirect-uri nil})))
|
:redirect-uri nil})))
|
||||||
{:authorized true}))
|
{:authorized true}))
|
||||||
|
|||||||
@ -8,11 +8,12 @@
|
|||||||
"Internal Nitrate HTTP RPC API. Provides authenticated access to
|
"Internal Nitrate HTTP RPC API. Provides authenticated access to
|
||||||
organization management and token validation endpoints."
|
organization management and token validation endpoints."
|
||||||
(:require
|
(:require
|
||||||
|
[app.auth.oidc :as oidc]
|
||||||
[app.common.data :as d]
|
[app.common.data :as d]
|
||||||
[app.common.exceptions :as ex]
|
[app.common.exceptions :as ex]
|
||||||
[app.common.schema :as sm]
|
[app.common.schema :as sm]
|
||||||
[app.common.time :as ct]
|
[app.common.time :as ct]
|
||||||
[app.common.types.organization :refer [schema:team-with-organization schema:organization-with-avatar]]
|
[app.common.types.organization :refer [schema:team-with-organization schema:organization-with-avatar schema:nitrate-sso]]
|
||||||
[app.common.types.profile :refer [schema:profile, schema:basic-profile]]
|
[app.common.types.profile :refer [schema:profile, schema:basic-profile]]
|
||||||
[app.common.types.team :refer [schema:team]]
|
[app.common.types.team :refer [schema:team]]
|
||||||
[app.config :as cf]
|
[app.config :as cf]
|
||||||
@ -878,6 +879,23 @@ RETURNING id, deleted_at;")
|
|||||||
photo-id (assoc :photo-url (files/resolve-public-uri photo-id))
|
photo-id (assoc :photo-url (files/resolve-public-uri photo-id))
|
||||||
owner-photo-id (assoc :owner-photo-url (files/resolve-public-uri owner-photo-id))))))))))))
|
owner-photo-id (assoc :owner-photo-url (files/resolve-public-uri owner-photo-id))))))))))))
|
||||||
|
|
||||||
|
;; ---- API: check-organization-sso
|
||||||
|
|
||||||
|
(def ^:private schema:check-organization-sso-result
|
||||||
|
[:map
|
||||||
|
[:valid ::sm/boolean]])
|
||||||
|
|
||||||
|
(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."
|
||||||
|
{::doc/added "2.20"
|
||||||
|
::sm/params schema:nitrate-sso
|
||||||
|
::sm/result schema:check-organization-sso-result
|
||||||
|
::rpc/auth false}
|
||||||
|
[cfg params]
|
||||||
|
{:valid (oidc/is-organization-sso-config-valid? cfg params)})
|
||||||
|
|
||||||
;; ---- API: notify-org-sso-change
|
;; ---- API: notify-org-sso-change
|
||||||
(sv/defmethod ::notify-org-sso-change
|
(sv/defmethod ::notify-org-sso-change
|
||||||
"Nitrate notifies that an organization sso values have changed"
|
"Nitrate notifies that an organization sso values have changed"
|
||||||
@ -895,3 +913,4 @@ RETURNING id, deleted_at;")
|
|||||||
(when became-active
|
(when became-active
|
||||||
(neh/send-organization-setup-sso-emails! cfg organization-id))
|
(neh/send-organization-setup-sso-emails! cfg organization-id))
|
||||||
nil)
|
nil)
|
||||||
|
|
||||||
|
|||||||
@ -53,3 +53,20 @@
|
|||||||
;; not silently slip through as if it were the matching string.
|
;; not silently slip through as if it were the matching string.
|
||||||
(t/is (= :auto (#'oidc/select-user-info-source :token)))
|
(t/is (= :auto (#'oidc/select-user-info-source :token)))
|
||||||
(t/is (= :auto (#'oidc/select-user-info-source :userinfo)))))
|
(t/is (= :auto (#'oidc/select-user-info-source :userinfo)))))
|
||||||
|
|
||||||
|
(t/deftest token-endpoint-errors-detect-valid-client-credentials
|
||||||
|
(let [response {:status 403
|
||||||
|
:body "{\"error\":\"invalid_grant\",\"error_description\":\"Invalid authorization code\"}"}]
|
||||||
|
(t/is (#'oidc/token-endpoint-valid-client-error? response))
|
||||||
|
(t/is (not (#'oidc/token-endpoint-invalid-client-error? response)))))
|
||||||
|
|
||||||
|
(t/deftest token-endpoint-errors-detect-invalid-client-credentials
|
||||||
|
(t/is (#'oidc/token-endpoint-invalid-client-error?
|
||||||
|
{:status 401
|
||||||
|
:body "{\"error\":\"access_denied\",\"error_description\":\"Unauthorized\"}"}))
|
||||||
|
(t/is (#'oidc/token-endpoint-invalid-client-error?
|
||||||
|
{:status 400
|
||||||
|
:body "{\"error\":\"invalid_client\"}"}))
|
||||||
|
(t/is (not (#'oidc/token-endpoint-valid-client-error?
|
||||||
|
{:status 400
|
||||||
|
:body "{\"error\":\"invalid_client\"}"}))))
|
||||||
|
|||||||
@ -6,6 +6,7 @@
|
|||||||
|
|
||||||
(ns backend-tests.rpc-management-nitrate-test
|
(ns backend-tests.rpc-management-nitrate-test
|
||||||
(:require
|
(:require
|
||||||
|
[app.auth.oidc :as oidc]
|
||||||
[app.common.data :as d]
|
[app.common.data :as d]
|
||||||
[app.common.time :as ct]
|
[app.common.time :as ct]
|
||||||
[app.common.uuid :as uuid]
|
[app.common.uuid :as uuid]
|
||||||
@ -1277,3 +1278,40 @@
|
|||||||
(with-redefs [eml/send! (fn [params] (swap! sent conj params))]
|
(with-redefs [eml/send! (fn [params] (swap! sent conj params))]
|
||||||
(management-command-with-nitrate! params))
|
(management-command-with-nitrate! params))
|
||||||
(t/is (empty? @sent))))
|
(t/is (empty? @sent))))
|
||||||
|
|
||||||
|
(t/deftest check-organization-sso-returns-valid-true
|
||||||
|
(let [org-id (uuid/random)
|
||||||
|
out (with-redefs [oidc/is-organization-sso-config-valid? (constantly true)]
|
||||||
|
(management-command-with-nitrate!
|
||||||
|
{::th/type :check-organization-sso
|
||||||
|
:organization-id org-id
|
||||||
|
:client-id "test-client"
|
||||||
|
:client-secret "test-secret"
|
||||||
|
:base-url "https://idp.example.com"}))]
|
||||||
|
(t/is (th/success? out))
|
||||||
|
(t/is (true? (-> out :result :valid)))))
|
||||||
|
|
||||||
|
(t/deftest check-organization-sso-returns-valid-false-on-invalid-config
|
||||||
|
(let [out (management-command-with-nitrate!
|
||||||
|
{::th/type :check-organization-sso
|
||||||
|
:organization-id (uuid/random)
|
||||||
|
:client-id "test-client"
|
||||||
|
:client-secret "test-secret"})]
|
||||||
|
(t/is (th/success? out))
|
||||||
|
(t/is (false? (-> out :result :valid)))))
|
||||||
|
|
||||||
|
(t/deftest check-organization-sso-uses-issuer-when-base-url-is-blank
|
||||||
|
(let [org-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))))]
|
||||||
|
(management-command-with-nitrate!
|
||||||
|
{::th/type :check-organization-sso
|
||||||
|
:organization-id org-id
|
||||||
|
:client-id "test-client"
|
||||||
|
:client-secret "test-secret"
|
||||||
|
:base-url ""
|
||||||
|
:issuer "https://idp.example.com/"}))]
|
||||||
|
(t/is (th/success? out))
|
||||||
|
(t/is (true? (-> out :result :valid)))))
|
||||||
|
|||||||
@ -63,3 +63,14 @@
|
|||||||
[:logo [:maybe ::sm/uri]]
|
[:logo [:maybe ::sm/uri]]
|
||||||
[:avatar-bg-url [:maybe ::sm/uri]]
|
[:avatar-bg-url [:maybe ::sm/uri]]
|
||||||
[:sso-active {:optional true} [:maybe :boolean]]])
|
[:sso-active {:optional true} [:maybe :boolean]]])
|
||||||
|
|
||||||
|
(def schema:nitrate-sso
|
||||||
|
[:map {:title "NitrateOrganizationSso"}
|
||||||
|
[:organization-id ::sm/uuid]
|
||||||
|
[:active {:optional true} [:maybe :boolean]]
|
||||||
|
[:provider {:optional true} [:maybe :string]]
|
||||||
|
[:client-id {:optional true} [:maybe :string]]
|
||||||
|
[:base-url {:optional true} [:maybe :string]]
|
||||||
|
[:client-secret {:optional true} [:maybe :string]]
|
||||||
|
[:issuer {:optional true} [:maybe :string]]
|
||||||
|
[:scopes {:optional true} [:maybe [::sm/set ::sm/text]]]])
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user