Merge remote-tracking branch 'origin/main' into staging

.serena/memories/backend/audit-log.md

@@ -16,7 +16,10 @@ Penpot records what users do as events in the Postgres `audit_log` table. There
 
 - Most backend events need no manual code: `wrap-audit` in `app.rpc` runs after every RPC handler when `:webhooks`, `:audit-log` or `:telemetry` is on (unless the command sets `::audit/skip`) and builds the event via `prepare-rpc-event`. The event name defaults to the command name (prefixed with `<module>-` outside `main`), props default to the request params, and timestamps come from the server request time.
 - Commands customize through result metadata (`rph/with-meta`): `::audit/replace-props` swaps the props wholesale (auth commands use `profile->props` so a register event carries the profile, not the password), `::audit/props` merges extras, `::audit/context`/`profile-id`/`name`/`type` override the defaults. `clean-props` always strips nils, qualified keys and `:session-id/:password/:old-password/:token/:client-secret` as a last line of defense.
+- 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`)
 
@@ -48,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.

.serena/memories/common/data-model-change-checklist.md

@@ -6,6 +6,7 @@
 - Do not treat nil as a distinct persisted state from absence. Import/export and cleanup paths may filter nil attrs away.
 - Avoid Clojure-special naming in exported object attrs, especially boolean names ending in `?`; exported/imported data must survive JSON/SVG/Transit and external tooling.
 - Any new shape attr that participates in component sync must be listed in `app.common.types.component/sync-attrs` with the correct touched group. Attrs absent from `sync-attrs` are ignored by component synchronization.
+- Shape `:svg-attrs` is the one map whose keys are camelCase React prop names (`fillRule`, not `fill-rule`), because the SVG import path runs them through `app.common.svg/attrs->props`. The `.penpot` reader (`app.common.json/read-kebab-key`) rewrites every json key of every entry to kebab-case, nested maps included, so any code that decodes a binary file must run `:svg-attrs` back through `attrs->props` or the names are lost. `:svg-defs` and the `:content` tree of `svg-raw` shapes keep their source names and round-trip on their own.
 
 ## Cross-module update checklist
 

backend/src/app/binfile/cleaner.clj

@@ -9,6 +9,7 @@
   for recently imported shapes."
   (:require
    [app.common.data :as d]
+   [app.common.svg :as csvg]
    [app.common.types.shape :as cts]
    [app.common.uuid :as uuid]))
 
@@ -105,6 +106,15 @@
                      :reverse-column :column-reverse
                      dir))))
 
+(defn- fix-svg-attrs
+  "The json reader of the binfile rewrites every key of every entry to
+  kebab-case, but `:svg-attrs` keys are react prop names and are stored
+  in camelCase. `attrs->props` is the transform the svg import path
+  already applies to them, so running it again restores the names; it
+  is idempotent, so shapes that come in correct are left untouched."
+  [shape]
+  (d/update-when shape :svg-attrs csvg/attrs->props))
+
 (defn clean-shape-post-decode
   "A shape procesor that expected to be executed after schema decoding
   process but before validation."
@@ -112,7 +122,8 @@
   (-> shape
       (fix-shape-shadow-color)
       (fix-root-shape)
-      (fix-legacy-flex-dir)))
+      (fix-legacy-flex-dir)
+      (fix-svg-attrs)))
 
 (defn- fix-container
   [container]

backend/src/app/loggers/audit.clj

@@ -350,14 +350,44 @@
                   {})]
     (assoc params :context context)))
 
+(defn- coerce-profile-id
+  "Normalize a hand-written `::audit/profile-id` override to a uuid.
+
+  `schema:event` requires a uuid and `submit*` swallows the validation
+  error, so a value that is not a uuid loses the event instead of
+  failing loudly. Commands read the override from places that are not
+  typed by us (token claims, stringly-typed drivers), so a string has
+  to be accepted. Anything that cannot become a uuid is discarded, and
+  the event falls back to the caller, which is always a valid uuid."
+  [v]
+  (let [coerced (cond
+                  ;; Fast path: the override comes straight from a `profile`
+                  ;; row in almost every command, so it is already a uuid.
+                  (uuid? v)
+                  v
+
+                  (string? v)
+                  (uuid/parse* v)
+
+                  :else
+                  nil)]
+    (when (and (nil? coerced) (some? v))
+      (l/error :hint "ignoring unusable ::audit/profile-id"
+               :profile-id v))
+
+    coerced))
+
 (defn prepare-rpc-event
   [cfg mdata params result]
   (let [resultm      (meta result)
         request      (-> params meta ::http/request)
-        profile-id   (or (::profile-id resultm)
+        ;; SECURITY: the event belongs to whoever made the request. The only
-                         (some-> (:profile-id result)
+        ;; sanctioned override is the `::audit/profile-id` metadata, set
-                                 (cond-> (string? (:profile-id result))
+        ;; explicitly by the command. Never derive it from the response:
-                                   uuid/parse*))
+        ;; results can carry a `:profile-id` that belongs to somebody else
+        ;; (the owner of an error report, the inviter of an invitation, ...)
+        ;; and that silently misattributes the action.
+        profile-id   (or (coerce-profile-id (::profile-id resultm))
                          (::rpc/profile-id params)
                          uuid/zero)
 

backend/src/app/rpc/commands/verify_token.clj

@@ -6,7 +6,6 @@
 
 (ns app.rpc.commands.verify-token
   (:require
-   [app.common.data :as d]
    [app.common.exceptions :as ex]
    [app.common.schema :as sm]
    [app.common.time :as ct]
@@ -99,7 +98,11 @@
                     (profile/strip-private-attrs)
                     (update :props profile/filter-props)
                     (with-nitrate-licence cfg))]
-    (assoc claims :profile profile)))
+    ;; The command is anonymous, so the audit event has no caller to fall
+    ;; back on and the profile must be declared here. The claims also carry
+    ;; a `:profile-id`, but the audit layer ignores the response on purpose.
+    (-> (assoc claims :profile profile)
+        (rph/with-meta {::audit/profile-id (:id profile)}))))
 
 ;; --- Team Invitation
 
@@ -240,6 +243,12 @@
                          (not (:is-member membership)))
                 (:organization-id membership))
 
+              organization-add-source
+              (when organization-id-on-add
+                (if organization-id
+                  "direct-organization-invitation"
+                  "team-invitation"))
+
               organization-member-count-before
               (when organization-id-on-add
                 (count
@@ -280,22 +289,34 @@
                               (-> (audit/event-from-rpc-params params)
                                   (assoc :profile-id created-by)
                                   (assoc :name "accept-team-invitation-from")
-                                  (assoc :props (assoc props
+                                  (assoc :props (-> props
-                                                       :profile-id (:id profile)
+                                                    (assoc :invited-by (:created-by invitation))
-                                                       :email (:email profile)))))))
+                                                    (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
+                (audit/submit cfg (-> (audit/event-from-rpc-params params)
-                 cfg
+                                      (assoc :name "accept-organization-invitation")
-                 (-> (audit/event-from-rpc-params params)
+                                      (assoc :props
-                     (assoc :name "accept-organization-invitation")
+                                             (-> props
-                     (assoc :props
+                                                 (assoc :organization-id organization-id-on-add)
-                            (-> props
+                                                 (assoc :invited-by (:created-by invitation))
-                                (assoc :organization-id organization-id-on-add
+                                                 (assoc :profile-id (:id profile))
-                                       :user-id (:id profile)
+                                                 (assoc :profile-email (:email profile))
-                                       :user-who-send-invitation (:created-by invitation))
+                                                 (assoc :organization-member-add-source
-                                (audit/clean-props))))))
+                                                        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
@@ -316,15 +337,11 @@
                 ;; 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
-                organization-id-on-add
+                ;; audit event has to name the accepting profile explicitly
-                (merge (d/without-nils
+                ;; or the invitation gets logged against the wrong user.
-                        {:invitation-id (:id invitation)
+                :always
-                         :organization-member-count-before
+                (rph/with-meta {::audit/profile-id (:id profile)}))))))
-                         organization-member-count-before}))
-
-                (and organization-id-on-add team-id)
-                (assoc :organization-id organization-id-on-add))))))
 
       (do
         ;; If the user is not logged-in and the invitation has been canceled

backend/test/backend_tests/binfile_test.clj

@@ -29,6 +29,7 @@
    [app.storage.tmp :as tmp]
    [backend-tests.helpers :as th]
    [backend-tests.storage-test :as stt]
+   [clojure.java.io :as jio]
    [clojure.test :as t]
    [cuerdas.core :as str]
    [datoteka.fs :as fs]
@@ -201,6 +202,57 @@
       ;; of failing with :child-not-found on the next update-file.
       (t/is (nil? (cfv/validate-file imported []))))))
 
+(defn- import-svg-attrs-asset
+  "Imports the `svg-attrs-camel-case.penpot` asset, a real penpot export
+  whose shapes carry `:svg-attrs` keys in camelCase (the format the
+  binary export writes), and returns the imported file."
+  [profile]
+  (let [input  (-> "backend_tests/test_files/svg-attrs-camel-case.penpot"
+                   io/resource
+                   jio/file)
+        result (-> th/*system*
+                   (assoc ::bfc/project-id (:default-project-id profile))
+                   (assoc ::bfc/profile-id (:id profile))
+                   (assoc ::bfc/input input)
+                   (v3/import-files!))]
+    (bfc/get-file th/*system* (first result))))
+
+(t/deftest import-binfile-v3-preserves-camel-case-svg-attrs
+  ;; The json reader used by the v3 import rewrites every key of every
+  ;; zip entry to kebab-case, and `:svg-attrs` is the one shape map
+  ;; whose keys are camelCase react prop names. A shape exported with
+  ;; `fillRule: "evenodd"` must not come back as `:fill-rule`, or the
+  ;; renderer falls back to the default fill rule and the shape is
+  ;; painted without its hole.
+  (let [profile (th/create-profile* 1)
+        file    (import-svg-attrs-asset profile)
+        shape   (get-in file [:data :pages-index
+                              (uuid/uuid "fc80ab5f-1bf2-817c-8008-b5408f039100")
+                              :objects
+                              (uuid/uuid "ce3641bd-48c8-804c-8008-b54d35cc6f80")])]
+
+    (t/is (= {:fillRule "evenodd"}
+             (:svg-attrs shape)))))
+
+(t/deftest import-binfile-v3-preserves-camel-case-svg-attrs-on-components
+  ;; Same guarantee for shapes stored inside a component: the v3 import
+  ;; cleans those in a different code path than page shapes.
+  (let [profile (th/create-profile* 1)
+        file    (import-svg-attrs-asset profile)
+        shape   (fn [component-id shape-id]
+                  (-> file
+                      (get-in [:data :components (uuid/uuid component-id) :objects])
+                      (get (uuid/uuid shape-id))
+                      :svg-attrs))]
+
+    (t/is (= {:fillRule "evenodd"}
+             (shape "fae4bc76-0cc2-8057-8008-b540912cdf78"
+                    "fae4bc76-0cc2-8057-8008-b540912774cb")))
+
+    (t/is (= {:fillRule "nonzero"}
+             (shape "fae4bc76-0cc2-8057-8008-b540912c917b"
+                    "fae4bc76-0cc2-8057-8008-b540912774c8")))))
+
 (t/deftest export-binfile-v3
   (let [profile (th/create-profile* 1)
         file    (prepare-simple-file profile)

backend/test/backend_tests/rpc_audit_test.clj

@@ -550,46 +550,83 @@
       (t/is (= {} (:props row)))
       (t/is (= {} (:context row))))))
 
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-;; PREPARE-RPC-EVENT PROFILE-ID CONVERSION
+;; PREPARE-RPC-EVENT PROFILE-ID RESOLUTION
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
-(t/deftest prepare-rpc-event-converts-string-profile-id-to-uuid
+(defn- prepare-event
-  ;; When result contains a string :profile-id (e.g. from error reports),
+  "Call prepare-rpc-event with a bare request, returning the built event."
-  ;; prepare-rpc-event must convert it to a UUID for audit schema compliance.
+  [caller result]
-  (let [prof       (th/create-profile* 1 {:is-active true})
+  (let [mdata    {::sv/name "test-cmd"}
-        string-pid "33601240-a00b-11ea-ba1b-c554cc60e361"
+        params   {::rpc/profile-id caller
-        expected   #uuid "33601240-a00b-11ea-ba1b-c554cc60e361"
+                  ::rpc/request-id (uuid/next)
-        mdata      {::sv/name "test-cmd"}
+                  ::rpc/request-at (ct/now)}
-        params     {::rpc/profile-id (:id prof)
-                    ::rpc/request-id (uuid/next)
-                    ::rpc/request-at (ct/now)}
-        mock-req   (reify
-                     yetti.request/IRequest
-                     (get-header [_ _] nil)
-                     (remote-addr [_] "127.0.0.1"))
-        params     (with-meta params {:app.http/request mock-req})
-        result     {:profile-id string-pid :some-data "value"}
-        event      (audit/prepare-rpc-event th/*system* mdata params result)]
-    ;; profile-id must be a UUID, not a string
-    (t/is (uuid? (:profile-id event)))
-    (t/is (= expected (:profile-id event)))))
-
-(t/deftest prepare-rpc-event-handles-invalid-string-profile-id
-  ;; When result contains an invalid string :profile-id, it should fall back
-  ;; to the RPC params profile-id (which is always a valid UUID).
-  (let [prof   (th/create-profile* 1 {:is-active true})
-        mdata  {::sv/name "test-cmd"}
-        params {::rpc/profile-id (:id prof)
-                ::rpc/request-id (uuid/next)
-                ::rpc/request-at (ct/now)}
         mock-req (reify
                    yetti.request/IRequest
                    (get-header [_ _] nil)
                    (remote-addr [_] "127.0.0.1"))
-        params (with-meta params {:app.http/request mock-req})
+        params   (with-meta params {:app.http/request mock-req})]
-        result {:profile-id "not-a-valid-uuid"}
+    (audit/prepare-rpc-event th/*system* mdata params result)))
-        event  (audit/prepare-rpc-event th/*system* mdata params result)]
+
-    ;; profile-id must fall back to the RPC params profile-id
+(t/deftest prepare-rpc-event-ignores-profile-id-from-result
-    (t/is (uuid? (:profile-id event)))
+  ;; An audit event belongs to the caller, never to whatever `:profile-id`
-    (t/is (= (:id prof) (:profile-id event)))))
+  ;; the response carries. `get-error-report` returns the report with its
+  ;; decoded content merged in, and that content can hold the profile that
+  ;; owned the report, so honoring it attributed the call to somebody who
+  ;; never made it.
+  (let [caller (th/create-profile* 1 {:is-active true})]
+    (t/is (= (:id caller)
+             (:profile-id (prepare-event (:id caller)
+                                         {:profile-id "33601240-a00b-11ea-ba1b-c554cc60e361"
+                                          :some-data "value"}))))
+    ;; an unparseable one must not break the event either
+    (t/is (= (:id caller)
+             (:profile-id (prepare-event (:id caller)
+                                         {:profile-id "not-a-valid-uuid"}))))))
+
+(t/deftest prepare-rpc-event-uses-metadata-profile-id
+  ;; `::audit/profile-id` metadata is the only sanctioned override: it is
+  ;; how commands that authenticate somebody else (login, verify-token)
+  ;; attribute the event to the right profile.
+  (let [caller (th/create-profile* 1 {:is-active true})
+        target (th/create-profile* 2 {:is-active true})
+        result (with-meta {:some-data "value"}
+                 {::audit/profile-id (:id target)})]
+    (t/is (= (:id target)
+             (:profile-id (prepare-event (:id caller) result))))))
+
+(t/deftest prepare-rpc-event-falls-back-to-zero-for-anonymous-callers
+  ;; With no metadata and no authenticated caller there is nobody to
+  ;; attribute the event to, so it lands on the zero uuid.
+  (t/is (= uuid/zero
+           (:profile-id (prepare-event nil {:some-data "value"})))))
+
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;; PREPARE-RPC-EVENT PROFILE-ID COERCION
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+
+(t/deftest prepare-rpc-event-coerces-string-metadata-profile-id
+  ;; `::audit/profile-id` is set by hand in a dozen commands, and some of
+  ;; them read the value from token claims or other stringly-typed sources.
+  ;; The audit schema demands a uuid, and `submit*` swallows the validation
+  ;; error, so an unconverted string would drop the event on the floor.
+  (let [caller (th/create-profile* 1 {:is-active true})
+        target "33601240-a00b-11ea-ba1b-c554cc60e361"
+        result (with-meta {:some-data "value"}
+                 {::audit/profile-id target})]
+    (t/is (= #uuid "33601240-a00b-11ea-ba1b-c554cc60e361"
+             (:profile-id (prepare-event (:id caller) result))))
+    (t/is (uuid? (:profile-id (prepare-event (:id caller) result))))))
+
+(t/deftest prepare-rpc-event-discards-unusable-metadata-profile-id
+  ;; An override that cannot be turned into a uuid is dropped, not honoured
+  ;; and not propagated: the event falls back to the caller, which is always
+  ;; a valid uuid, instead of failing the schema check and losing the row.
+  (let [caller (th/create-profile* 1 {:is-active true})]
+    (doseq [bad ["not-a-valid-uuid" "" "  " 42 {} [] :whatever nil false]]
+      (let [result (with-meta {:some-data "value"}
+                     (cond-> {::audit/profile-id bad}
+                       (nil? bad) (dissoc ::audit/profile-id)))]
+        (t/is (= (:id caller)
+                 (:profile-id (prepare-event (:id caller) result)))
+              (str "override " (pr-str bad) " must fall back to the caller"))))))

backend/test/backend_tests/rpc_commands_error_reports_test.clj

@@ -256,31 +256,41 @@
 
 ;; --- Audit event tests
 
-(t/deftest get-error-report-audit-event-has-uuid-profile-id
+(t/deftest get-error-report-audit-event-attributes-the-caller
-  ;; When get-error-report returns a report with string profile-id in content,
+  ;; The report content carries the profile that owned the report and the
-  ;; the audit event must have a proper UUID profile-id (not a string).
+  ;; handler merges that content into the response, so the response holds a
-  ;; This tests the prepare-rpc-event function directly since the test RPC
+  ;; `:profile-id` that does not belong to the caller. The audit event must
-  ;; flow doesn't include the audit middleware wrapper.
+  ;; still belong to the caller: deriving it from the response attributed
-  (let [profile  (th/create-profile* 1 {:is-active true})
+  ;; privileged reads to the users whose crashes were being inspected.
-        id       (uuid/next)
+  (let [caller (th/create-profile* 1 {:is-active true})
-        orig-pid "33601240-a00b-11ea-ba1b-c554cc60e361"
+        owner  "33601240-a00b-11ea-ba1b-c554cc60e361"
-        ;; Simulate the result from get-error-report with string profile-id
+        id     (uuid/next)]
-        result   {:id id
+    (insert-report! th/*system*
-                  :source "logging"
+                    {:id id
-                  :hint "test error"
+                     :source 4
-                  :profile-id orig-pid}
+                     :content {:profile-id owner
-        mdata    {::sv/name "get-error-report"}
+                               :hint "test error"}})
-        params   {::rpc/profile-id (:id profile)
+
-                  ::rpc/request-id (uuid/next)
+    (let [out (token-cmd caller {::th/type :get-error-report :id id})]
-                  ::rpc/request-at (ct/now)}
+      (t/is (th/success? out))
-        mock-req (reify yetti.request/IRequest
+
-                   (get-header [_ _] nil)
+      (let [result (:result out)
-                   (remote-addr [_] "127.0.0.1"))
+            event  (audit/prepare-rpc-event
-        params   (with-meta params {:app.http/request mock-req})
+                    th/*system*
-        event    (audit/prepare-rpc-event th/*system* mdata params result)]
+                    {::sv/name "get-error-report"}
-    ;; profile-id must be a UUID, not a string
+                    (with-meta {::rpc/profile-id (:id caller)
-    (t/is (uuid? (:profile-id event)))
+                                ::rpc/request-id (uuid/next)
-    (t/is (= #uuid "33601240-a00b-11ea-ba1b-c554cc60e361" (:profile-id event)))))
+                                ::rpc/request-at (ct/now)
+                                :id id}
+                      {:app.http/request (reify
+                                           yetti.request/IRequest
+                                           (get-header [_ _] nil)
+                                           (remote-addr [_] "127.0.0.1"))})
+                    result)]
+        ;; the response does expose the report owner's profile...
+        (t/is (= owner (get result :profile-id)))
+        ;; ...but the audit event belongs to whoever called the command
+        (t/is (= (:id caller) (:profile-id event)))))))
 
 ;; Note: The integration of access token middleware with audit context is tested
 ;; via unit tests in rpc_audit_test.clj and http_middleware_test.clj.

backend/test/backend_tests/rpc_profile_test.clj

@@ -1594,7 +1594,6 @@
     (t/is (th/ex-of-type? (:error out) :validation))
     (t/is (th/ex-of-code? (:error out) :weak-password))))
 
-
 (t/deftest update-profile-password-sends-notification
   (with-mocks [mock {:target 'app.email/send! :return nil}]
     (let [profile (th/create-profile* 1)
@@ -1645,3 +1644,56 @@
         (t/is (= eml/password-changed factory))
         (t/is (= (:email profile) to))
         (t/is (= (:fullname profile) name))))))
+
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;; VERIFY-TOKEN AUDIT ATTRIBUTION
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+
+(t/deftest verify-token-auth-audit-event-attributes-the-authenticated-profile
+  ;; `verify-token` is anonymous, so the audit event cannot infer the profile
+  ;; from the caller: the handler must declare it in the result metadata.
+  (let [profile (th/create-profile* 1 {:is-active true})
+        token   (tokens/generate th/*system*
+                                 {:iss :auth
+                                  :exp (ct/in-future "1h")
+                                  :profile-id (:id profile)})
+        out     (th/command! {::th/type :verify-token
+                              :token token})]
+    (t/is (th/success? out))
+    (t/is (= (:id profile)
+             (get-in (meta (:result out)) [:app.loggers.audit/profile-id])))))
+
+(t/deftest verify-token-invitation-audit-event-attributes-the-accepting-profile
+  ;; Both the invitation claims and the response carry the inviter's
+  ;; profile-id, so the event must not end up on the inviter: the member who
+  ;; clicked the link is the one who accepted the invitation.
+  (with-redefs [app.config/flags #{:login-with-password}]
+    (let [owner  (th/create-profile* 1 {:is-active true})
+          team   (th/create-team* 1 {:profile-id (:id owner)})
+          member (th/create-profile* 2 {:is-active true
+                                        :email "invited@example.com"})
+          email  (:email member)
+          token  (tokens/generate th/*system*
+                                  {:iss :team-invitation
+                                   :exp (ct/in-future "48h")
+                                   :role :editor
+                                   :profile-id (:id owner)
+                                   :team-id (:id team)
+                                   :member-email email
+                                   :member-id (:id member)})]
+      (th/db-insert! :team-invitation
+                     {:id (uuid/random)
+                      :team-id (:id team)
+                      :email-to email
+                      :created-by (:id owner)
+                      :role "editor"
+                      :valid-until (ct/in-future "48h")})
+
+      (let [out     (th/command! {::th/type :verify-token
+                                  :token token
+                                  ::rpc/profile-id (:id member)
+                                  ::rpc/auth-type :session})
+            event-pid (get-in (meta (:result out)) [:app.loggers.audit/profile-id])]
+        (t/is (th/success? out))
+        (t/is (= (:id member) event-pid))
+        (t/is (not= (:id owner) event-pid))))))

backend/test/backend_tests/rpc_team_test.clj

@@ -492,13 +492,14 @@
                              (th/command! {::th/type :verify-token
                                            ::rpc/profile-id (:id invitee)
                                            :token token}))
-          organization-event
+          emitted-events
-          (fn []
+          (fn [event-name]
             (->> (:call-args-list @audit-mock)
                  (map second)
-                 (filter #(= "accept-organization-invitation" (:name %)))
+                 (filter #(= event-name (:name %)))))
-                 first))
+          organization-event
-          token-result     (atom nil)]
+          (fn []
+            (first (emitted-events "accept-organization-invitation")))]
 
       (db/insert! (:app.db/pool th/*system*)
                   :team-invitation
@@ -520,29 +521,22 @@
                     (fn [& _] default-team-id)]
         (let [out (verify! direct-token)]
           (t/is (th/success? out))
-          (reset! token-result (:result out))))
+          (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 invitee) (get-in event [:props :profile-id])))
-        (t/is (= (:id inviter)
+        (t/is (= (:id inviter) (get-in event [:props :invited-by])))
-                 (get-in event [:props :user-who-send-invitation])))
+        (t/is (= (:email invitee) (get-in event [:props :profile-email])))
-        (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 (= :editor (get-in event [:props :role])))
         (t/is (uuid? (get-in event [:props :invitation-id])))
-        (t/is (= organization-id (:organization-id @token-result)))
+        (t/is (= "direct-organization-invitation"
-        (t/is (= :editor (:role @token-result)))
+                 (get-in event [:props :organization-member-add-source])))
-        (t/is (not (contains? @token-result :organization-invitation-audit)))
+        (t/is (false? (get-in event [:props :belongs-to-team-on-add])))
-        (t/is (= (get-in event [:props :invitation-id])
+        (t/is (= 3 (get-in event [:props :organization-member-count-before])))
-                 (:invitation-id @token-result)))
+        (t/is (not (contains? (:props event) :invitation-origin)))
-        (t/is (= (:id invitee)
+        (t/is (not (contains? (:context event) :event-origin)))
-                 (:member-id @token-result)))
+        (t/is (= 1 (count (emitted-events "accept-organization-invitation"))))
-        (t/is (= (:id inviter)
-                 (:profile-id @token-result)))
-        (t/is (= 3
-                 (:organization-member-count-before @token-result)))
         (t/is (not-any? #(contains? #{"accept-team-invitation"
                                       "accept-team-invitation-from"}
                                     (:name (second %)))
@@ -569,7 +563,7 @@
                     teams/add-profile-to-team! (fn [& _] nil)]
         (let [out (verify! team-token)]
           (t/is (th/success? out))
-          (reset! token-result (:result out))))
+          (t/is (not (contains? (:result out) :organization-invitation-audit)))))
 
       (let [events (mapv second (:call-args-list @audit-mock))
             event  (organization-event)]
@@ -577,25 +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 invitee) (get-in event [:props :profile-id])))
-        (t/is (= (:id inviter)
+        (t/is (= (:id inviter) (get-in event [:props :invited-by])))
-                 (get-in event [:props :user-who-send-invitation])))
+        (t/is (= "team-invitation"
-        (t/is (not (contains? (:props event) :organization-member-add-source)))
+                 (get-in event [:props :organization-member-add-source])))
-        (t/is (not (contains? (:props event) :belongs-to-team-on-add)))
+        (t/is (true? (get-in event [:props :belongs-to-team-on-add])))
-        (t/is (not (contains? (:props event) :organization-member-count-before)))
+        (t/is (= 5 (get-in event [:props :organization-member-count-before])))
-        (t/is (= organization-id
+        (t/is (not (contains? (:props event) :invitation-origin)))
-                 (:organization-id @token-result)))
+        (t/is (not (contains? (:context event) :event-origin)))
-        (t/is (= (:id team) (:team-id @token-result)))
+        (t/is (= 1 (count (emitted-events "accept-organization-invitation")))))
-        (t/is (= :editor (:role @token-result)))
+
-        (t/is (not (contains? @token-result :organization-invitation-audit)))
+      (let [from-event (first (emitted-events "accept-team-invitation-from"))]
-        (t/is (= (get-in event [:props :invitation-id])
+        (t/is (= (:id team) (get-in from-event [:props :team-id])))
-                 (:invitation-id @token-result)))
+        (t/is (= :editor (get-in from-event [:props :role])))
-        (t/is (= (:id invitee)
+        (t/is (uuid? (get-in from-event [:props :invitation-id])))
-                 (:member-id @token-result)))
+        (t/is (= (:id inviter) (get-in from-event [:props :invited-by])))
-        (t/is (= (:id inviter)
+        (t/is (= (:id invitee) (get-in from-event [:props :profile-id])))
-                 (:profile-id @token-result)))
+        (t/is (= (:email invitee) (get-in from-event [:props :profile-email])))
-        (t/is (= 5
+        (t/is (not (contains? (get-in from-event [:props]) :email)))
-                 (:organization-member-count-before @token-result))))
+        (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*)
@@ -616,57 +612,11 @@
                     teams/add-profile-to-team! (fn [& _] nil)]
         (let [out (verify! team-token)]
           (t/is (th/success? out))
-          (reset! token-result (:result out))))
+          (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 (not-any? #(= "accept-organization-invitation" (:name %)) events))))))
-        (t/is (not (contains? @token-result :organization-invitation-audit)))
-        (t/is (not (contains? @token-result :organization-member-count-before)))))))
-
-(t/deftest accept-organization-invitation-response-ids-match-database
-  (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}]
-    (let [inviter         (th/create-profile* 211 {:is-active true})
-          invitee         (th/create-profile* 212 {:is-active true})
-          organization-id (uuid/random)
-          default-team-id (uuid/random)
-          ;; Token minted with a stale :profile-id (e.g. a re-sent link
-          ;; requested by someone else) and no :member-id (invitee was
-          ;; unregistered when invited); the invitation row says inviter.
-          stale-token     (tokens/generate
-                           th/*system*
-                           {:iss :team-invitation
-                            :exp (ct/in-future "1h")
-                            :profile-id (uuid/random)
-                            :role :editor
-                            :organization-id organization-id
-                            :member-email (:email invitee)})]
-      (db/insert! (:app.db/pool th/*system*)
-                  :team-invitation
-                  {:org-id organization-id
-                   :email-to (:email invitee)
-                   :created-by (:id inviter)
-                   :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)]
-                        nil))
-                    teams/initialize-user-in-organization
-                    (fn [& _] default-team-id)]
-        (let [out (th/command! {::th/type :verify-token
-                                ::rpc/profile-id (:id invitee)
-                                :token stale-token})]
-          (t/is (th/success? out))
-          (t/is (= (:id inviter) (:profile-id (:result out))))
-          (t/is (= (:id invitee) (:member-id (:result out))))
-          (t/is (not (contains? (:result out) :user-who-send-invitation)))
-          (t/is (not (contains? (:result out) :user-id))))))))
 
 (t/deftest create-team-invitations-with-email-verification-disabled
   (with-mocks [mock {:target 'app.email/send! :return nil}]

backend/test/backend_tests/tasks_telemetry_test.clj

@@ -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)

frontend/src/app/main/ui/auth/verify_token.cljs

@@ -6,11 +6,9 @@
 
 (ns app.main.ui.auth.verify-token
   (:require
-   [app.common.data :as d]
    [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]
@@ -45,33 +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}]
+  [{:keys [state team-id organization-team-id organization-name invitation-token
-  (when (and (= state :created)
+           redirect-to]}]
-             (contains? tdata :organization-member-count-before))
-    (let [direct-invitation? (some? organization-team-id)]
-      (st/emit!
-       (ev/event
-        (-> (select-keys tdata [:team-id :organization-id :role
-                                :invitation-id :member-id :profile-id
-                                :organization-member-count-before])
-            ;; The audit event keeps its own names: :user-id is the
-            ;; invitee (:member-id, backfilled by the backend with the
-            ;; accepting profile) and :user-who-send-invitation is the
-            ;; inviter (:profile-id, backfilled from `created-by`).
-            (assoc :user-id (:member-id tdata)
-                   :user-who-send-invitation (:profile-id tdata))
-            (dissoc :member-id :profile-id)
-            (d/without-nils)
-            (assoc :organization-member-add-source
-                   (if direct-invitation?
-                     "direct-organization-invitation"
-                     "team-invitation")
-                   :belongs-to-team-on-add (boolean team-id)
-                   ::ev/name "accept-organization-invitation"
-                   ::ev/origin (if direct-invitation?
-                                 "organization-invitation-acceptance"
-                                 "team-invitation-acceptance")))))))
-
   (case state
     :created
     (if organization-team-id
@@ -85,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

frontend/test/frontend_tests/data/nitrate_test.cljs

@@ -140,74 +140,6 @@
         (t/is (= :auth-register (:id event)))
         (t/is (= invitation-token (get-in event [:params :invitation-token])))))))
 
-(t/deftest accept-organization-invitation-audit-event-test
-  (let [emitted (atom [])]
-    (with-redefs [st/emit! (fn
-                             ([event]
-                              (swap! emitted conj event))
-                             ([event & events]
-                              (swap! emitted into (cons event events))))]
-      (t/testing "accepting a team invitation that adds an organization member"
-        (verify-token/handle-token
-         {:iss :team-invitation
-          :state :created
-          :team-id "team-1"
-          :organization-id "organization-1"
-          :role :editor
-          :invitation-id "invitation-1"
-          :member-id "invitee-1"
-          :profile-id "inviter-1"
-          :organization-member-count-before 4})
-
-        (t/is (= {::ev/name "accept-organization-invitation"
-                  ::ev/origin "team-invitation-acceptance"
-                  :team-id "team-1"
-                  :organization-id "organization-1"
-                  :role :editor
-                  :invitation-id "invitation-1"
-                  :user-id "invitee-1"
-                  :user-who-send-invitation "inviter-1"
-                  :organization-member-add-source "team-invitation"
-                  :belongs-to-team-on-add true
-                  :organization-member-count-before 4}
-                 @(first @emitted))))
-
-      (reset! emitted [])
-      (t/testing "accepting an invitation directly to the organization"
-        (verify-token/handle-token
-         {:iss :team-invitation
-          :state :created
-          :organization-id "organization-2"
-          :organization-team-id "team-default"
-          :role :viewer
-          :invitation-id "invitation-2"
-          :member-id "invitee-2"
-          :profile-id "inviter-2"
-          :organization-member-count-before 0})
-
-        (t/is (= {::ev/name "accept-organization-invitation"
-                  ::ev/origin "organization-invitation-acceptance"
-                  :organization-id "organization-2"
-                  :role :viewer
-                  :invitation-id "invitation-2"
-                  :user-id "invitee-2"
-                  :user-who-send-invitation "inviter-2"
-                  :organization-member-add-source "direct-organization-invitation"
-                  :belongs-to-team-on-add false
-                  :organization-member-count-before 0}
-                 @(first @emitted))))
-
-      (reset! emitted [])
-      (t/testing "does not audit a team invitation for an existing organization member"
-        (verify-token/handle-token
-         {:iss :team-invitation
-          :state :created
-          :team-id "team-3"
-          :organization-id "organization-3"
-          :role :editor})
-
-        (t/is (= 3 (count @emitted)))))))
-
 (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/")]