From a5dac030735d21da63a9c0dd54de7e5702c98248 Mon Sep 17 00:00:00 2001 From: Pablo Alba Date: Tue, 20 Oct 2026 14:08:40 +0200 Subject: [PATCH] :sparkles: Add organization SSO and invitation audit events for nitrate (#12461) --- backend/src/app/auth/oidc.clj | 4 +- .../src/app/loggers/audit/consumer_task.clj | 7 ++++ backend/src/app/rpc/commands/verify_token.clj | 35 +++++++++------- backend/src/app/rpc/management/nitrate.clj | 14 ++++++- backend/test/backend_tests/auth_oidc_test.clj | 32 +++++++++++++++ .../loggers_audit_consumer_task_test.clj | 3 +- .../rpc_management_nitrate_test.clj | 28 +++++++++++++ backend/test/backend_tests/rpc_team_test.clj | 41 +++++++++++++++++-- 8 files changed, 141 insertions(+), 23 deletions(-) diff --git a/backend/src/app/auth/oidc.clj b/backend/src/app/auth/oidc.clj index 05121e7479..be8c9078eb 100644 --- a/backend/src/app/auth/oidc.clj +++ b/backend/src/app/auth/oidc.clj @@ -1037,7 +1037,9 @@ (when (and session organization-id) (let [props (-> (or (:props session) {}) (update :sso assoc organization-id exp))] - (session/update-session (::session/manager cfg) (assoc session :props props)))) + (session/update-session (::session/manager cfg) (assoc session :props props)) + (submit-organization-sso-auth-event + cfg request (:profile-id session) organization-id "organization-sso-token-issued"))) (submit-organization-sso-auth-event cfg request (:profile-id session) organization-id "organization-sso-auth-succeeded") (redirect-response dest-url)) diff --git a/backend/src/app/loggers/audit/consumer_task.clj b/backend/src/app/loggers/audit/consumer_task.clj index 6c4986eed5..9fa05c5e70 100644 --- a/backend/src/app/loggers/audit/consumer_task.clj +++ b/backend/src/app/loggers/audit/consumer_task.clj @@ -208,10 +208,13 @@ (def ^:private event-names-for-admin-console #{"accept-organization-invitation" + "accept-organization-invitation-from" "accept-team-invitation" "accept-team-invitation-from" "add-member-to-organization" "add-team-to-organization" + "auditlog-exported" + "auditlog-guide-downloaded" "cancel-organization-invitation" "create-export-binfile-job" "create-file" @@ -238,12 +241,16 @@ "organization-sso-auth-failed" "organization-sso-auth-started" "organization-sso-auth-succeeded" + "organization-sso-token-issued" + "organization-sso-token-revoked" "permanently-delete-team-files" "remove-organization-team" "remove-team-from-organization" "rename-organization" "rename-project" "restore-deleted-team-files" + "sso-config-deleted" + "sso-config-tested" "success-activate-organization-sso" "success-apply-organization-sso-changes" "update-organization-invitation" diff --git a/backend/src/app/rpc/commands/verify_token.clj b/backend/src/app/rpc/commands/verify_token.clj index 26746382c8..4a0b84981f 100644 --- a/backend/src/app/rpc/commands/verify_token.clj +++ b/backend/src/app/rpc/commands/verify_token.clj @@ -302,21 +302,26 @@ ;; for the same name, and tied the record to the browser finishing ;; the flow. Everything the copy carried is computed above. (when organization-id-on-add - (audit/submit cfg (-> (audit/event-from-rpc-params params) - (assoc :name "accept-organization-invitation") - (assoc :props - (-> props - (assoc :organization-id organization-id-on-add) - (assoc :invited-by (:created-by invitation)) - (assoc :profile-id (:id profile)) - (assoc :profile-email (:email profile)) - (assoc :organization-member-add-source - organization-add-source) - (assoc :belongs-to-team-on-add - (boolean team-id)) - (assoc :organization-member-count-before - organization-member-count-before) - (audit/clean-props)))))) + (let [org-props (-> props + (assoc :organization-id organization-id-on-add + :invited-by (:created-by invitation) + :profile-id (:id profile) + :profile-email (:email profile) + :organization-member-add-source + organization-add-source + :belongs-to-team-on-add + (boolean team-id) + :organization-member-count-before + organization-member-count-before) + (audit/clean-props))] + (audit/submit cfg (-> (audit/event-from-rpc-params params) + (assoc :name "accept-organization-invitation" + :props org-props))) + (when-let [created-by (:created-by invitation)] + (audit/submit cfg (-> (audit/event-from-rpc-params params) + (assoc :profile-id created-by + :name "accept-organization-invitation-from" + :props org-props)))))) (cond-> (assoc claims :state :created diff --git a/backend/src/app/rpc/management/nitrate.clj b/backend/src/app/rpc/management/nitrate.clj index ac18a6c10d..c8c4f83e8c 100644 --- a/backend/src/app/rpc/management/nitrate.clj +++ b/backend/src/app/rpc/management/nitrate.clj @@ -1060,10 +1060,20 @@ RETURNING id, deleted_at;") [:updated-props ::sm/boolean] [:announce-activation ::sm/boolean]] ::rpc/auth false} - [{:keys [::db/pool] :as cfg} {:keys [organization-id updated-props announce-activation]}] + [{:keys [::db/pool] :as cfg} + {:keys [organization-id updated-props announce-activation] :as params}] (when updated-props (rpc/invalidate-organization-sso-cache-by-organization! organization-id) - (session/clear-organization-sso-sessions! pool organization-id)) + (session/clear-organization-sso-sessions! pool organization-id) + (let [request (-> params meta ::http/request) + context (-> (audit/prepare-context-from-request request) + (assoc :request-id (::rpc/request-id params)))] + (audit/submit cfg {:type "action" + :name "organization-sso-token-revoked" + :profile-id uuid/zero + :ip-addr (::rpc/ip-addr params) + :props {:organization-id organization-id} + :context (d/without-nils context)}))) (notifications/notify-organization-change-sso cfg organization-id) (when announce-activation (neh/send-organization-setup-sso-emails! cfg organization-id)) diff --git a/backend/test/backend_tests/auth_oidc_test.clj b/backend/test/backend_tests/auth_oidc_test.clj index 90e1aa18f4..55bab1a134 100644 --- a/backend/test/backend_tests/auth_oidc_test.clj +++ b/backend/test/backend_tests/auth_oidc_test.clj @@ -10,6 +10,7 @@ [app.common.data :as d] [app.common.exceptions :as ex] [app.common.time :as ct] + [app.common.uuid :as uuid] [app.config :as cf] [app.http.session :as session] [app.setup :as-alias setup] @@ -524,6 +525,7 @@ (t/is (.contains loc "error=unable-to-auth"))))))) (t/deftest organization-sso-callback-success-emits-succeeded + ;; No session on the request → auth-succeeded only (no token-issued). (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}) @@ -538,6 +540,36 @@ (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-success-with-session-emits-token-issued + (let [manager (session/inmemory-manager) + cfg (assoc (dissoc base-cfg :app.email/blacklist :app.email/whitelist) + ::session/manager manager) + 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 []) + sess {:id (uuid/next) + :profile-id test-profile-id + :props {}}] + (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.http.session/get-session (constantly sess) + app.loggers.audit/submit (fn [_cfg event] (swap! events conj event))] + (let [result (#'oidc/callback-handler cfg request) + names (mapv :name @events) + stored (session/read-session manager (:id sess))] + (t/is (= "https://penpot.example.com/#/workspace" (redirect-location result))) + (t/is (= ["organization-sso-token-issued" + "organization-sso-auth-succeeded"] + names)) + (t/is (= test-organization-id + (get-in (first @events) [:props :organization-id]))) + (t/is (= test-profile-id (:profile-id (first @events)))) + (t/is (= #{:organization-id} + (set (keys (:props (first @events)))))) + (t/is (contains? (get-in stored [:props :sso]) test-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" diff --git a/backend/test/backend_tests/loggers_audit_consumer_task_test.clj b/backend/test/backend_tests/loggers_audit_consumer_task_test.clj index 146f0da10b..1fb3efd9e5 100644 --- a/backend/test/backend_tests/loggers_audit_consumer_task_test.clj +++ b/backend/test/backend_tests/loggers_audit_consumer_task_test.clj @@ -113,12 +113,13 @@ http-mock {:target 'app.http.client/req :return {:status 204}}] (insert-audit-row! (:id prof) "create-project") (insert-audit-row! (:id prof) "create-organization-attribute") + (insert-audit-row! (:id prof) "auditlog-exported") (insert-audit-row! (:id prof) "unrelated-event") (run-both! {:uri archive-uri}) (t/is (:called? @admin-console-mock)) (t/is (:called? @http-mock)) (let [[_ _ params] (:call-args @admin-console-mock)] - (t/is (= #{"create-project" "create-organization-attribute"} + (t/is (= #{"create-project" "create-organization-attribute" "auditlog-exported"} (into #{} (map :name) (:events params))))) (let [rows (th/db-exec! ["select * from audit_log"])] (t/is (= 0 (count rows)))))))) diff --git a/backend/test/backend_tests/rpc_management_nitrate_test.clj b/backend/test/backend_tests/rpc_management_nitrate_test.clj index be36f84680..500b241279 100644 --- a/backend/test/backend_tests/rpc_management_nitrate_test.clj +++ b/backend/test/backend_tests/rpc_management_nitrate_test.clj @@ -576,6 +576,34 @@ (t/is (= :organization-change-sso (:type msg))) (t/is (= organization-id (:organization-id msg)))))) +(t/deftest notify-organization-sso-change-emits-token-revoked-when-updated-props + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (let [organization-id (uuid/random) + out (with-redefs [mbus/pub! (constantly nil)] + (th/management-command! {::th/type :notify-organization-sso-change + :organization-id organization-id + :updated-props true + :announce-activation false})) + revoked (->> (:call-args-list @audit-mock) + (map second) + (filter #(= "organization-sso-token-revoked" (:name %))))] + (t/is (th/success? out)) + (t/is (= 1 (count revoked))) + (t/is (= uuid/zero (:profile-id (first revoked)))) + (t/is (= {:organization-id organization-id} (:props (first revoked)))))) + + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (let [out (with-redefs [mbus/pub! (constantly nil)] + (th/management-command! {::th/type :notify-organization-sso-change + :organization-id (uuid/random) + :updated-props false + :announce-activation false})) + revoked (->> (:call-args-list @audit-mock) + (map second) + (filter #(= "organization-sso-token-revoked" (:name %))))] + (t/is (th/success? out)) + (t/is (empty? revoked))))) + (t/deftest notify-user-organizations-deletion-renames-or-deletes-teams-and-publishes-per-organization-events ;; --- Deferred owned-organizations: nil during setup, filled before RPC --- (let [owned-organizations-ref (atom nil)] diff --git a/backend/test/backend_tests/rpc_team_test.clj b/backend/test/backend_tests/rpc_team_test.clj index ed0a12fb5b..b1bab1acb6 100644 --- a/backend/test/backend_tests/rpc_team_test.clj +++ b/backend/test/backend_tests/rpc_team_test.clj @@ -527,7 +527,8 @@ (t/is (th/success? out)) (t/is (not (contains? (:result out) :organization-invitation-audit))))) - (let [event (organization-event)] + (let [event (organization-event) + from-event (first (emitted-events "accept-organization-invitation-from"))] (t/is (= organization-id (get-in event [:props :organization-id]))) (t/is (= (:id invitee) (get-in event [:props :profile-id]))) (t/is (= (:id inviter) (get-in event [:props :invited-by]))) @@ -541,11 +542,39 @@ (t/is (not (contains? (:props event) :invitation-origin))) (t/is (not (contains? (:context event) :event-origin))) (t/is (= 1 (count (emitted-events "accept-organization-invitation")))) + (t/is (= 1 (count (emitted-events "accept-organization-invitation-from")))) + (t/is (= (:id inviter) (:profile-id from-event))) + (t/is (= (:props event) (:props from-event))) (t/is (not-any? #(contains? #{"accept-team-invitation" "accept-team-invitation-from"} (:name (second %))) (:call-args-list @audit-mock)))) + ;; Legacy org invitation with no :created-by → invitee event only, no -from. + (th/reset-mock! audit-mock) + (db/insert! (:app.db/pool th/*system*) + :team-invitation + {:org-id organization-id + :email-to (:email invitee) + :role "editor" + :valid-until (ct/in-future "48h")}) + + (with-redefs [cf/flags (conj cf/flags :admin-console) + nitrate/call + (fn [_cfg method _params] + (case method + :get-organization-membership {:organization-id organization-id + :is-member false} + :get-organization-members [(:id inviter) (uuid/random) (uuid/random)] + nil)) + teams/initialize-user-in-organization + (fn [& _] default-team-id)] + (let [out (verify! direct-token)] + (t/is (th/success? out)))) + + (t/is (= 1 (count (emitted-events "accept-organization-invitation")))) + (t/is (empty? (emitted-events "accept-organization-invitation-from"))) + (th/reset-mock! audit-mock) (db/insert! (:app.db/pool th/*system*) :team-invitation @@ -569,8 +598,9 @@ (t/is (th/success? out)) (t/is (not (contains? (:result out) :organization-invitation-audit))))) - (let [events (mapv second (:call-args-list @audit-mock)) - event (organization-event)] + (let [events (mapv second (:call-args-list @audit-mock)) + event (organization-event) + org-from-event (first (emitted-events "accept-organization-invitation-from"))] (t/is (some #(= "accept-team-invitation" (:name %)) events)) (t/is (some #(= "accept-team-invitation-from" (:name %)) events)) (t/is (= (:id team) (get-in event [:props :team-id]))) @@ -583,7 +613,10 @@ (t/is (= 5 (get-in event [:props :organization-member-count-before]))) (t/is (not (contains? (:props event) :invitation-origin))) (t/is (not (contains? (:context event) :event-origin))) - (t/is (= 1 (count (emitted-events "accept-organization-invitation"))))) + (t/is (= 1 (count (emitted-events "accept-organization-invitation")))) + (t/is (= 1 (count (emitted-events "accept-organization-invitation-from")))) + (t/is (= (:id inviter) (:profile-id org-from-event))) + (t/is (= (:props event) (:props org-from-event)))) (let [from-event (first (emitted-events "accept-team-invitation-from"))] (t/is (= (:id team) (get-in from-event [:props :team-id])))