🐛 Emit the accept-organization-invitation audit event once

The event was written twice per accepted organization invitation. The
backend submitted it, and the browser then re-submitted a copy of the
props that the backend had already put in the response under
`:organization-invitation-audit` (`handle-token :team-invitation` in
`verify-token.cljs`). Both rows carried the same name with different prop
vocabularies, and the browser copy only existed when the browser finished
the flow.

Emit the event from the backend only. It now also carries the three props
that lived in the browser copy: the organization member count before the
add, the add source, and whether the invitee also joined a team. The
origin moves to the event context as `:event-origin`. The response no
longer includes `:organization-invitation-audit`, so the browser stops
emitting the event and `verify-token.cljs` drops its
`app.main.data.event` require.

The `accept-*` events of this command now share one prop vocabulary:
`:profile-id` for the accepting profile, `:invited-by` for the inviter
and `:profile-email` for the email, replacing the mix of
`:user-id`/`:user-who-send-invitation` and `:email`.

Audit consumers of `accept-organization-invitation` now see one row per
acceptance instead of two, and must read the new prop names.

AI-assisted-by: space-bunny-free
This commit is contained in:
Andrey Antukh 2026-09-29 15:53:24 +00:00
parent 0de865748d
commit 3b886995ba
6 changed files with 107 additions and 132 deletions

View File

@ -19,6 +19,7 @@ Penpot records what users do as events in the Postgres `audit_log` table. There
- INVARIANT: the event's `profile-id` is the caller, resolved as `::audit/profile-id` metadata -> `::rpc/profile-id` -> `uuid/zero`. It is NEVER taken from the response. Results carry `:profile-id` keys that belong to someone else (`get-error-report` returns the report with its content merged, so the profile that owned the report; `verify-token` on a team invitation returns the inviter), and honoring them misattributes the action. Commands with `::rpc/auth false` (`login-*`, `register-profile`, `create-demo-profile`, `verify-token`, `prepare-register-profile`) have no caller to fall back on, so they MUST set `::audit/profile-id` in the result metadata or the event lands on `uuid/zero`.
- The `::audit/profile-id` override goes through `coerce-profile-id`: a string is parsed, anything that cannot become a uuid is discarded (with a warning) and the event falls back to the caller. Do not pass the value straight through: `schema:event` requires `::sm/uuid` and `submit*` swallows the validation error, so a non-uuid override silently loses the row instead of failing loudly. Hand-built events built with `event-from-rpc-params` + `submit` (clone-template, accept-team-invitation, `management/nitrate` push-audit-events) do NOT go through that coercion: they must supply a uuid.
- `submit` is the normal entry point (fills defaults, validates `schema:event`, runs inside `tx-run!`, logs failures without failing the RPC). `insert` is the low-level one for CLI/helpers and the webhook subsystem: direct write, no webhook/telemetry fan-out, silent unless `:audit-log` is on. Boot emits `trigger/instance-start` from `setup/props` so every restart is visible in the log.
- `accept-organization-invitation` distinguishes its two flows with the props, not with an origin: `:organization-member-add-source` is `"direct-organization-invitation"` for a direct organization invitation and `"team-invitation"` for a team invitation that also adds to the organization, and `:belongs-to-team-on-add` repeats the same distinction as a boolean. Do not add a third prop for it. The event `context` has an `:event-origin`, but it belongs to the browser: `make-data-event` in `app.main.data.event` puts the UI origin there and it is on the frontend allowlist, while `safe-backend-context-keys` lists no `:event-origin`, so a backend event has nowhere to put one. Text props are dropped from the telemetry shadow rows by `filter-telemetry-props`; the full `audit_log` row and the Nexus archive still carry them.
## Consumers I: webhooks (`app.loggers.webhooks`)
@ -50,4 +51,4 @@ Penpot records what users do as events in the Postgres `audit_log` table. There
## Tests
- `backend_tests/rpc-audit-test.clj` exercises the whole backend path (full-row insert, telemetry-only and dual-row modes, `submit*`, no-op without flags, `insert` gating, `prepare-rpc-event` resolution) with `with-redefs [cf/flags #{...}]` against real `audit_log` rows.
- Other RPC suites mock `app.loggers.audit/submit` (nil return; `helpers.clj` stubs it globally) and assert on `:call-args-list`; any new command that must (or must not) emit an event needs the same treatment.
- Other RPC suites mock `app.loggers.audit/submit` (nil return; `helpers.clj` stubs it globally) and assert on `:call-args-list`; any new command that must (or must not) emit an event needs the same treatment. `rpc-team-test/accept-organization-invitation-audit-event` is the reference for invitation events: three scenarios (direct org invitation, team invitation that adds to the org, already-a-member) asserting the emitted props, the add source and the team flag that tell the two flows apart, and the exact number of rows per event name. The count matters: asserting props against the first match with `first` passes even when a command emits the same name twice. Props have no schema on either ingest path, so these assertions are the only guard on the event contract.

View File

@ -254,12 +254,6 @@
"direct-organization-invitation"
"team-invitation"))
organization-event-origin
(when organization-id-on-add
(if organization-id
"organization-invitation-acceptance"
"team-invitation-acceptance"))
organization-member-count-before
(when organization-id-on-add
(count
@ -300,46 +294,45 @@
(-> (audit/event-from-rpc-params params)
(assoc :profile-id created-by)
(assoc :name "accept-team-invitation-from")
(assoc :props (assoc props
:profile-id (:id profile)
:email (:email profile)))))))
(assoc :props (-> props
(assoc :invited-by (:created-by invitation))
(assoc :profile-id (:id profile))
(assoc :profile-email (:email profile))
(audit/clean-props)))))))
(let [accepted-team-id (accept-invitation cfg claims invitation profile)]
;; NOTE: the browser used to re-submit a copy of this event from
;; the `:organization-invitation-audit` payload of this response,
;; which wrote two rows per acceptance with two prop vocabularies
;; 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
:user-id (:id profile)
:user-who-send-invitation (:created-by invitation))
(audit/clean-props))))))
(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))))))
(-> (cond-> (assoc claims :state :created)
;; when the invitation is to an organization, instead of a team, add the
;; accepted-team-id as :organization-team-id
(:organization-id claims)
(assoc :organization-team-id accepted-team-id)
organization-id-on-add
(assoc :organization-invitation-audit
{:origin organization-event-origin
:props
(-> props
(assoc :organization-id organization-id-on-add
:organization-member-add-source organization-add-source
:belongs-to-team-on-add (boolean team-id)
:user-id (:id profile)
:user-who-send-invitation (:created-by invitation)
:organization-member-count-before
organization-member-count-before)
(audit/clean-props))}))
;; The response carries the inviter's profile-id, so the
;; audit event has to name the accepting profile explicitly
;; or the invitation gets logged against the wrong user.
(rph/with-meta {::audit/profile-id (:id profile)}))))))
(cond-> (assoc claims :state :created)
;; when the invitation is to an organization, instead of a team, add the
;; accepted-team-id as :organization-team-id
(:organization-id claims)
(assoc :organization-team-id accepted-team-id)
;; The response carries the inviter's profile-id, so the
;; audit event has to name the accepting profile explicitly
;; or the invitation gets logged against the wrong user.
:always
(rph/with-meta {::audit/profile-id (:id profile)}))))))
(do
;; If the user is not logged-in and the invitation has been canceled

View File

@ -492,13 +492,14 @@
(th/command! {::th/type :verify-token
::rpc/profile-id (:id invitee)
:token token}))
organization-event
(fn []
emitted-events
(fn [event-name]
(->> (:call-args-list @audit-mock)
(map second)
(filter #(= "accept-organization-invitation" (:name %)))
first))
frontend-event (atom nil)]
(filter #(= event-name (:name %)))))
organization-event
(fn []
(first (emitted-events "accept-organization-invitation")))]
(db/insert! (:app.db/pool th/*system*)
:team-invitation
@ -520,32 +521,22 @@
(fn [& _] default-team-id)]
(let [out (verify! direct-token)]
(t/is (th/success? out))
(reset! frontend-event
(get-in out [:result :organization-invitation-audit]))))
(t/is (not (contains? (:result out) :organization-invitation-audit)))))
(let [event (organization-event)]
(t/is (= organization-id (get-in event [:props :organization-id])))
(t/is (= (:id invitee) (get-in event [:props :user-id])))
(t/is (= (:id inviter)
(get-in event [:props :user-who-send-invitation])))
(t/is (not (contains? (:props event) :organization-member-add-source)))
(t/is (not (contains? (:props event) :belongs-to-team-on-add)))
(t/is (not (contains? (:props event) :organization-member-count-before)))
(t/is (= (:id invitee) (get-in event [:props :profile-id])))
(t/is (= (:id inviter) (get-in event [:props :invited-by])))
(t/is (= (:email invitee) (get-in event [:props :profile-email])))
(t/is (= :editor (get-in event [:props :role])))
(t/is (uuid? (get-in event [:props :invitation-id])))
(t/is (= "organization-invitation-acceptance"
(:origin @frontend-event)))
(t/is (= organization-id
(get-in @frontend-event [:props :organization-id])))
(t/is (= (:id invitee)
(get-in @frontend-event [:props :user-id])))
(t/is (= (:id inviter)
(get-in @frontend-event [:props :user-who-send-invitation])))
(t/is (= "direct-organization-invitation"
(get-in @frontend-event [:props :organization-member-add-source])))
(t/is (false? (get-in @frontend-event [:props :belongs-to-team-on-add])))
(t/is (= 3
(get-in @frontend-event [:props :organization-member-count-before])))
(get-in event [:props :organization-member-add-source])))
(t/is (false? (get-in event [:props :belongs-to-team-on-add])))
(t/is (= 3 (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 (not-any? #(contains? #{"accept-team-invitation"
"accept-team-invitation-from"}
(:name (second %)))
@ -572,8 +563,7 @@
teams/add-profile-to-team! (fn [& _] nil)]
(let [out (verify! team-token)]
(t/is (th/success? out))
(reset! frontend-event
(get-in out [:result :organization-invitation-audit]))))
(t/is (not (contains? (:result out) :organization-invitation-audit)))))
(let [events (mapv second (:call-args-list @audit-mock))
event (organization-event)]
@ -581,26 +571,27 @@
(t/is (some #(= "accept-team-invitation-from" (:name %)) events))
(t/is (= (:id team) (get-in event [:props :team-id])))
(t/is (= organization-id (get-in event [:props :organization-id])))
(t/is (= (:id invitee) (get-in event [:props :user-id])))
(t/is (= (:id inviter)
(get-in event [:props :user-who-send-invitation])))
(t/is (not (contains? (:props event) :organization-member-add-source)))
(t/is (not (contains? (:props event) :belongs-to-team-on-add)))
(t/is (not (contains? (:props event) :organization-member-count-before)))
(t/is (= "team-invitation-acceptance"
(:origin @frontend-event)))
(t/is (= (:id team) (get-in @frontend-event [:props :team-id])))
(t/is (= organization-id
(get-in @frontend-event [:props :organization-id])))
(t/is (= (:id invitee)
(get-in @frontend-event [:props :user-id])))
(t/is (= (:id inviter)
(get-in @frontend-event [:props :user-who-send-invitation])))
(t/is (= (:id invitee) (get-in event [:props :profile-id])))
(t/is (= (:id inviter) (get-in event [:props :invited-by])))
(t/is (= "team-invitation"
(get-in @frontend-event [:props :organization-member-add-source])))
(t/is (true? (get-in @frontend-event [:props :belongs-to-team-on-add])))
(t/is (= 5
(get-in @frontend-event [:props :organization-member-count-before]))))
(get-in event [:props :organization-member-add-source])))
(t/is (true? (get-in event [:props :belongs-to-team-on-add])))
(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")))))
(let [from-event (first (emitted-events "accept-team-invitation-from"))]
(t/is (= (:id team) (get-in from-event [:props :team-id])))
(t/is (= :editor (get-in from-event [:props :role])))
(t/is (uuid? (get-in from-event [:props :invitation-id])))
(t/is (= (:id inviter) (get-in from-event [:props :invited-by])))
(t/is (= (:id invitee) (get-in from-event [:props :profile-id])))
(t/is (= (:email invitee) (get-in from-event [:props :profile-email])))
(t/is (not (contains? (get-in from-event [:props]) :email)))
(t/is (not (contains? (get-in from-event [:props]) :user-id)))
(t/is (not (contains? (get-in from-event [:props])
:user-who-send-invitation))))
(th/reset-mock! audit-mock)
(db/insert! (:app.db/pool th/*system*)
@ -621,13 +612,11 @@
teams/add-profile-to-team! (fn [& _] nil)]
(let [out (verify! team-token)]
(t/is (th/success? out))
(reset! frontend-event
(get-in out [:result :organization-invitation-audit]))))
(t/is (not (contains? (:result out) :organization-invitation-audit)))))
(let [events (mapv second (:call-args-list @audit-mock))]
(t/is (some #(= "accept-team-invitation" (:name %)) events))
(t/is (not-any? #(= "accept-organization-invitation" (:name %)) events))
(t/is (nil? @frontend-event))))))
(t/is (not-any? #(= "accept-organization-invitation" (:name %)) events))))))
(t/deftest create-team-invitations-with-email-verification-disabled
(with-mocks [mock {:target 'app.email/send! :return nil}]

View File

@ -754,6 +754,34 @@
(t/is (not (contains? (:props result) :route)))
(t/is (not (contains? (:props result) :label)))))
(t/deftest test-filter-telemetry-props-accept-organization-invitation
;; The shadow row of accept-organization-invitation keeps the ids, the boolean
;; and the count, and drops every text prop, the raw email among them. The
;; full row and the Nexus archive still carry all of them, and there is no
;; context key to fall back on: `safe-backend-context-keys` has no
;; `:event-origin`, that one belongs to the browser.
(let [ftp (ns-resolve 'app.loggers.audit 'filter-telemetry-props)
profile-id (uuid/next)
organization-id (uuid/next)
result (ftp {:source "backend"
:name "accept-organization-invitation"
:type "action"
:props {:profile-id profile-id
:invited-by profile-id
:organization-id organization-id
:team-id (uuid/next)
:belongs-to-team-on-add true
:organization-member-count-before 3
:organization-member-add-source "team-invitation"
:profile-email "invitee@example.com"}})]
(t/is (= profile-id (get-in result [:props :profile-id])))
(t/is (= profile-id (get-in result [:props :invited-by])))
(t/is (= organization-id (get-in result [:props :organization-id])))
(t/is (true? (get-in result [:props :belongs-to-team-on-add])))
(t/is (= 3 (get-in result [:props :organization-member-count-before])))
(t/is (not (contains? (:props result) :profile-email)))
(t/is (not (contains? (:props result) :organization-member-add-source)))))
(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)

View File

@ -9,7 +9,6 @@
[app.config :as cf]
[app.main.data.auth :as da]
[app.main.data.common :as dcm]
[app.main.data.event :as ev]
[app.main.data.notifications :as ntf]
[app.main.data.profile :as du]
[app.main.repo :as rp]
@ -44,14 +43,8 @@
(st/emit! (da/login-from-token tdata)))
(defmethod handle-token :team-invitation
[{:keys [state team-id organization-team-id organization-name invitation-token] :as tdata}]
(when-let [{:keys [origin props]} (:organization-invitation-audit tdata)]
(st/emit!
(ev/event
(assoc props
::ev/name "accept-organization-invitation"
::ev/origin origin))))
[{:keys [state team-id organization-team-id organization-name invitation-token
redirect-to]}]
(case state
:created
(if organization-team-id
@ -65,7 +58,7 @@
(ntf/success (tr "auth.notifications.team-invitation-accepted"))))
:pending
(let [route-id (:redirect-to tdata :auth-register)]
(let [route-id (or redirect-to :auth-register)]
(st/emit! (rt/nav route-id {:invitation-token invitation-token})))))
(defmethod handle-token :default

View File

@ -11,8 +11,6 @@
[app.main.data.event :as ev]
[app.main.data.nitrate :as dnt]
[app.main.data.nitrate-audit :as nitrate-audit]
[app.main.store :as st]
[app.main.ui.auth.verify-token :as verify-token]
[cljs.test :as t :include-macros true]))
(t/deftest account-age-days-test
@ -115,33 +113,6 @@
(t/is (contains? event :days-since-member-added))
(t/is (nil? (:days-since-member-added event)))))))
(t/deftest accept-organization-invitation-audit-event-test
(let [emitted (atom [])
props {:team-id "team-1"
:organization-id "organization-1"
:role :editor
:invitation-id "invitation-1"
:organization-member-add-source "team-invitation"
:belongs-to-team-on-add true
:organization-member-count-before 4}]
(with-redefs [st/emit! (fn
([event]
(swap! emitted conj event))
([event & events]
(swap! emitted into (cons event events))))]
(verify-token/handle-token
{:iss :team-invitation
:state :created
:team-id "team-1"
:organization-invitation-audit
{:origin "team-invitation-acceptance"
:props props}}))
(let [event @(first @emitted)]
(t/is (= "accept-organization-invitation" (::ev/name event)))
(t/is (= "team-invitation-acceptance" (::ev/origin event)))
(t/is (= props (dissoc event ::ev/name ::ev/origin))))))
(t/deftest build-admin-console-url-preserves-public-uri-subpath
(t/testing "builds admin console routes below the configured Penpot subpath"
(let [public-uri (u/uri "https://example.com/penpot/")]