From ddc98bdd47157a9803efead01888f8a845e41544 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marina=20L=C3=B3pez?= Date: Wed, 19 Aug 2026 07:58:44 +0200 Subject: [PATCH] :sparkles: Add sso events (#11265) --- backend/src/app/auth/oidc.clj | 85 ++++++++++++++++++- backend/src/app/loggers/audit.clj | 18 ++++ backend/src/app/rpc/commands/nitrate.clj | 17 +++- backend/test/backend_tests/auth_oidc_test.clj | 56 ++++++++++++ .../test/backend_tests/rpc_nitrate_test.clj | 44 +++++++++- .../backend_tests/tasks_telemetry_test.clj | 13 +++ 6 files changed, 226 insertions(+), 7 deletions(-) diff --git a/backend/src/app/auth/oidc.clj b/backend/src/app/auth/oidc.clj index c636aeba24..6cb23fb9de 100644 --- a/backend/src/app/auth/oidc.clj +++ b/backend/src/app/auth/oidc.clj @@ -771,6 +771,82 @@ ;; ORG SSO HELPERS ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +(defn- organization-sso-oauth-failure-reason + [error] + (case (d/name error) + "access_denied" "access-denied" + ("temporarily_unavailable" "server_error") "provider-unavailable" + ("invalid_request" "unauthorized_client" "invalid_scope") "invalid-configuration" + "provider-error")) + +(defn- organization-sso-exception-failure-reason + [cause] + (let [data (ex-data cause) + status (or (:response-status data) + (:response-status-code data) + (:http-status data)) + network-error? + (loop [current cause] + (cond + (nil? current) + false + + (or (instance? java.net.ConnectException current) + (instance? java.net.UnknownHostException current) + (instance? java.net.http.HttpTimeoutException current) + (instance? javax.net.ssl.SSLException current)) + true + + (identical? current (ex-cause current)) + false + + :else + (recur (ex-cause current))))] + (if (or network-error? + (and (number? status) (<= 500 status 599))) + "provider-unavailable" + (case (:code data) + :unable-to-fetch-access-token "token-exchange-failed" + :unable-to-retrieve-user-info "user-info-failed" + :incomplete-user-info "incomplete-user-info" + :invalid-sso-config "invalid-configuration" + :unable-to-fetch-sso-jwks "provider-unavailable" + :unable-to-auth "access-denied" + "unexpected-error")))) + +(defn- submit-organization-sso-auth-event + [cfg request profile-id organization-id name & {:keys [failure-reason]}] + (audit/submit cfg {:type "action" + :name name + :profile-id profile-id + :ip-addr (inet/parse-request request) + :props (d/without-nils + {:organization-id organization-id + :failure-reason failure-reason}) + :context (audit/prepare-context-from-request request)})) + +(defn submit-organization-sso-auth-started-event + [cfg request profile-id organization-id] + (submit-organization-sso-auth-event + cfg request profile-id organization-id "organization-sso-auth-started")) + +(defn submit-organization-sso-auth-failed-event + [cfg request profile-id organization-id cause] + (submit-organization-sso-auth-event + cfg request profile-id organization-id "organization-sso-auth-failed" + :failure-reason (organization-sso-exception-failure-reason cause))) + +(defn- submit-organization-sso-oauth-failed-event + [cfg request state-token error] + (try + (let [state (tokens/verify cfg {:token state-token :iss "oidc"})] + (when (:dest-url state) + (submit-organization-sso-auth-event + cfg request (some-> (session/get-session request) :profile-id) + (:organization-id state) "organization-sso-auth-failed" + :failure-reason (organization-sso-oauth-failure-reason error)))) + (catch Throwable _ nil))) + (defn- non-blank-uri [value] (when-not (str/blank? value) value)) @@ -910,6 +986,8 @@ (let [props (-> (or (:props session) {}) (update :sso assoc organization-id exp))] (session/update-session (::session/manager cfg) (assoc session :props props)))) + (submit-organization-sso-auth-event + cfg request (:profile-id session) organization-id "organization-sso-auth-succeeded") (redirect-response dest-url)) (catch Throwable cause (let [{:keys [code]} (ex-data cause)] @@ -922,6 +1000,9 @@ (l/err :hint "unexpected error on organization sso callback" :organization-id (:organization-id state) :cause cause)))) + (submit-organization-sso-auth-failed-event + cfg request (some-> (session/get-session request) :profile-id) + (:organization-id state) cause) (let [organization-id (:organization-id state) organization-name (:name (nitrate/call cfg :get-organization-summary {:organization-id organization-id}))] (redirect-with-organization-sso-error @@ -932,7 +1013,9 @@ (defn- callback-handler [cfg {:keys [params] :as request}] (if-let [error (get params :error)] - (redirect-with-error "unable-to-auth" error) + (do + (submit-organization-sso-oauth-failed-event cfg request (:state params) error) + (redirect-with-error "unable-to-auth" error)) (try (let [code (get params :code) state (get params :state) diff --git a/backend/src/app/loggers/audit.clj b/backend/src/app/loggers/audit.clj index f68209255b..6ded7befaf 100644 --- a/backend/src/app/loggers/audit.clj +++ b/backend/src/app/loggers/audit.clj @@ -36,6 +36,16 @@ (def ^:private filter-auth-events #{"login-with-oidc" "login-with-password" "register-profile" "update-profile"}) +(def ^:private organization-sso-failure-reasons + #{"access-denied" + "provider-unavailable" + "invalid-configuration" + "provider-error" + "token-exchange-failed" + "user-info-failed" + "incomplete-user-info" + "unexpected-error"}) + (def ^:private safe-backend-context-keys #{:version :initiator @@ -297,6 +307,14 @@ (defn filter-telemetry-props [{:keys [source name props type] :as params}] (cond + (and (= source "backend") + (= name "organization-sso-auth-failed")) + (let [props' (into {} xf:filter-telemetry-props props) + props' (cond-> props' + (contains? organization-sso-failure-reasons (:failure-reason props)) + (assoc :failure-reason (:failure-reason props)))] + (assoc params :props props')) + (or (and (= source "frontend") (= type "identify")) (and (= source "backend") diff --git a/backend/src/app/rpc/commands/nitrate.clj b/backend/src/app/rpc/commands/nitrate.clj index c48834662e..a476ce0dbf 100644 --- a/backend/src/app/rpc/commands/nitrate.clj +++ b/backend/src/app/rpc/commands/nitrate.clj @@ -698,10 +698,19 @@ (if authorized {:authorized true :reason :sso-satisfied} (if (oidc/organization-sso-discovery-uri sso) - {:authorized false - :redirect-uri (oidc/build-organization-sso-auth-redirect-uri cfg sso - :dest-url url - :organization-id organization-id)} + (try + (let [redirect-uri (oidc/build-organization-sso-auth-redirect-uri + cfg sso + :dest-url url + :organization-id organization-id) + organization-id (or organization-id (:organization-id sso))] + (oidc/submit-organization-sso-auth-started-event + cfg request profile-id organization-id) + {:authorized false :redirect-uri redirect-uri}) + (catch Throwable cause + (oidc/submit-organization-sso-auth-failed-event + cfg request profile-id (or organization-id (:organization-id sso)) cause) + (throw cause))) {:authorized false :redirect-uri nil})))) {:authorized true :reason :sso-satisfied})) diff --git a/backend/test/backend_tests/auth_oidc_test.clj b/backend/test/backend_tests/auth_oidc_test.clj index 62f04fd546..b99de502c4 100644 --- a/backend/test/backend_tests/auth_oidc_test.clj +++ b/backend/test/backend_tests/auth_oidc_test.clj @@ -385,6 +385,9 @@ (def ^:private test-profile-id #uuid "11111111-1111-1111-1111-111111111111") +(def ^:private test-organization-id + #uuid "22222222-2222-2222-2222-222222222222") + (def ^:private test-profile {:id test-profile-id :is-active true @@ -519,6 +522,59 @@ (t/is (= 302 (::yres/status result))) (t/is (.contains loc "error=unable-to-auth"))))))) +(t/deftest organization-sso-callback-success-emits-succeeded + (let [cfg (dissoc base-cfg :app.email/blacklist :app.email/whitelist) + state (make-state-token cfg {:dest-url "https://penpot.example.com/#/workspace" + :organization-id test-organization-id}) + request (default-request cfg :state state) + events (atom [])] + (with-redefs [app.nitrate/call (constantly {:active true}) + app.auth.oidc/prepare-organization-sso-provider (constantly {:type "oidc"}) + app.auth.oidc/get-info (constantly {}) + app.loggers.audit/submit (fn [_cfg event] (swap! events conj event))] + (let [result (#'oidc/callback-handler cfg request)] + (t/is (= "https://penpot.example.com/#/workspace" (redirect-location result))) + (t/is (= ["organization-sso-auth-succeeded"] (mapv :name @events))) + (t/is (= test-organization-id (get-in (first @events) [:props :organization-id]))))))) + +(t/deftest organization-sso-callback-error-emits-failed + (let [cfg (dissoc base-cfg :app.email/blacklist :app.email/whitelist) + state (make-state-token cfg {:dest-url "https://penpot.example.com/#/workspace" + :organization-id test-organization-id}) + request (default-request cfg :state state) + events (atom [])] + (with-redefs [app.nitrate/call (fn [_cfg method _params] + (case method + :get-organization-sso {:active true} + :get-organization-summary {:name "Organization"})) + app.auth.oidc/prepare-organization-sso-provider (constantly {:type "oidc"}) + app.auth.oidc/get-info (fn [& _] + (ex/raise :type :internal + :code :unable-to-retrieve-user-info)) + app.loggers.audit/submit (fn [_cfg event] (swap! events conj event))] + (#'oidc/callback-handler cfg request) + (t/is (= ["organization-sso-auth-failed"] (mapv :name @events))) + (t/is (= {:organization-id test-organization-id + :failure-reason "user-info-failed"} + (:props (first @events))))))) + +(t/deftest organization-sso-oauth-error-emits-failed-without-changing-redirect + (let [cfg (dissoc base-cfg :app.email/blacklist :app.email/whitelist) + state (make-state-token cfg {:dest-url "https://penpot.example.com/#/workspace" + :organization-id test-organization-id}) + request (assoc-in (default-request cfg :state state) [:params :error] "access_denied") + events (atom [])] + (binding [cf/config {:public-uri "http://localhost:3449"}] + (with-redefs [app.loggers.audit/submit (fn [_cfg event] (swap! events conj event))] + (let [result (#'oidc/callback-handler cfg request) + loc (redirect-location result)] + (t/is (.contains loc "error=unable-to-auth")) + (t/is (.contains loc "hint=access_denied")) + (t/is (= ["organization-sso-auth-failed"] (mapv :name @events))) + (t/is (= {:organization-id test-organization-id + :failure-reason "access-denied"} + (:props (first @events))))))))) + (t/deftest prepare-organization-sso-provider-does-not-skip-ssrf-check (t/testing "organization SSO provider must use SSRF protection" (let [captured-params (atom nil)] diff --git a/backend/test/backend_tests/rpc_nitrate_test.clj b/backend/test/backend_tests/rpc_nitrate_test.clj index d2ce0043bf..90b746e2a1 100644 --- a/backend/test/backend_tests/rpc_nitrate_test.clj +++ b/backend/test/backend_tests/rpc_nitrate_test.clj @@ -148,6 +148,8 @@ team (th/create-team* 1 {:profile-id (:id team-owner)}) organization-id (uuid/random) redirect-uri "https://idp.example.com/authorize" + redirect-options (atom nil) + started-event (atom nil) params (with-meta {::th/type :check-nitrate-sso ::rpc/profile-id (:id team-owner) @@ -161,12 +163,50 @@ organization-id (:id team-owner)) oidc/build-organization-sso-auth-redirect-uri - (constantly redirect-uri)] + (fn [_cfg _sso & options] + (reset! redirect-options (apply hash-map options)) + redirect-uri) + oidc/submit-organization-sso-auth-started-event + (fn [_cfg _request profile-id received-organization-id] + (reset! started-event {:profile-id profile-id + :organization-id received-organization-id}))] (let [out (th/command! params)] (t/is (th/success? out)) (t/is (= {:authorized false :redirect-uri redirect-uri} - (:result out)))))))) + (:result out))) + (t/is (= #{:dest-url :organization-id} (set (keys @redirect-options)))) + (t/is (= "https://penpot.example.com/#/workspace" (str (:dest-url @redirect-options)))) + (t/is (nil? (:organization-id @redirect-options))) + (t/is (= {:profile-id (:id team-owner) + :organization-id organization-id} + @started-event))))))) + +(t/deftest check-nitrate-sso-reports-redirect-failure + (let [profile (th/create-profile* 1 {:is-active true}) + organization-id (uuid/random) + cause (ex-info "provider unavailable" {:response-status-code 503}) + reported (atom nil) + params (with-meta + {::th/type :check-nitrate-sso + ::rpc/profile-id (:id profile) + :organization-id organization-id + :url "https://penpot.example.com/#/workspace"} + {::http/request {}})] + (binding [cf/flags (conj cf/flags :admin-console)] + (with-redefs [nitrate/sso-session-authorized? (unauthorized-sso-mock organization-id) + oidc/build-organization-sso-auth-redirect-uri (fn [& _] (throw cause)) + oidc/submit-organization-sso-auth-failed-event + (fn [_cfg _request profile-id received-organization-id received-cause] + (reset! reported {:profile-id profile-id + :organization-id received-organization-id + :cause received-cause}))] + (let [out (th/command! params)] + (t/is (not (th/success? out))) + (t/is (= {:profile-id (:id profile) + :organization-id organization-id + :cause cause} + @reported))))))) (t/deftest check-nitrate-sso-keeps-gate-for-non-member-organization-owner (let [team-owner (th/create-profile* 1 {:is-active true}) diff --git a/backend/test/backend_tests/tasks_telemetry_test.clj b/backend/test/backend_tests/tasks_telemetry_test.clj index 07b8f7c7f6..e3f57647df 100644 --- a/backend/test/backend_tests/tasks_telemetry_test.clj +++ b/backend/test/backend_tests/tasks_telemetry_test.clj @@ -710,6 +710,19 @@ (t/is (not (contains? (:props result) :route))) (t/is (not (contains? (:props result) :label))))) +(t/deftest test-filter-telemetry-props-organization-sso-failure-keeps-reason + (let [ftp (ns-resolve 'app.loggers.audit 'filter-telemetry-props) + organization-id (uuid/next) + result (ftp {:source "backend" + :name "organization-sso-auth-failed" + :type "action" + :props {:organization-id organization-id + :failure-reason "access-denied" + :unsafe-label "should-be-stripped"}})] + (t/is (= {:organization-id organization-id + :failure-reason "access-denied"} + (:props result))))) + (t/deftest test-filter-telemetry-props-navigate-keeps-route-and-ids ;; Frontend navigate events keep specific routing keys: :route, ;; :file-id, :team-id, :page-id. These ids are strings because