From dfc1848ffc29008a9e118cf86099a2196799e484 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marina=20L=C3=B3pez?= Date: Thu, 24 Sep 2026 14:13:52 +0200 Subject: [PATCH] :recycle: Build organization invitation audit event in frontend (#11825) * :recycle: Build organization invitation audit event in frontend * :recycle: Align invitation token profile-id with created-by The invitation token carried the minter in :profile-id while the invitation row tracks the creator in :created-by. Both mean the inviter, so re-sends or re-requested links made them disagree and forced a second response key, :user-who-send-invitation. Mint :profile-id from :created-by in both token creators, backfill it from the row on accept (covers stale in-flight tokens), and drop the duplicate response key. The frontend maps :profile-id to the unchanged :user-who-send-invitation audit prop. AI-assisted-by: Muse Spark 1.3 Free * :recycle: Reuse token ids in invitation accept response Backfill :member-id with the accepting profile and drop the :user-id duplicate from the verify-token response, mirroring the :profile-id/:user-who-send-invitation cleanup. The frontend maps :member-id to the unchanged :user-id audit prop. AI-assisted-by: Muse Spark 1.3 Free --------- Co-authored-by: Andrey Antukh --- .../app/rpc/commands/teams_invitations.clj | 11 ++- backend/src/app/rpc/commands/verify_token.clj | 49 +++++----- backend/test/backend_tests/rpc_team_test.clj | 95 +++++++++++++------ .../src/app/main/ui/auth/verify_token.cljs | 32 +++++-- .../frontend_tests/data/nitrate_test.cljs | 79 +++++++++++---- 5 files changed, 186 insertions(+), 80 deletions(-) diff --git a/backend/src/app/rpc/commands/teams_invitations.clj b/backend/src/app/rpc/commands/teams_invitations.clj index 77fc8246e8..f78ef08c70 100644 --- a/backend/src/app/rpc/commands/teams_invitations.clj +++ b/backend/src/app/rpc/commands/teams_invitations.clj @@ -75,6 +75,9 @@ (tokens/generate cfg {:iss :team-invitation :exp valid-until + ;; NOTE: :profile-id is the inviter (the `created-by` of + ;; the invitation row). Callers must pass the row value + ;; so the token stays consistent with the database. :profile-id profile-id :role role :team-id team-id @@ -218,7 +221,7 @@ updated? (not= id (:id invitation)) profile-id (:id profile) team-organization-id (get-in team [:organization :id]) - tprops {:profile-id profile-id + tprops {:profile-id (or (:created-by invitation) profile-id) :invitation-id (:id invitation) :valid-until expire :team-id (:id team) @@ -611,7 +614,11 @@ member (profile/get-profile-by-email pool (:email-to invit)) token (create-invitation-token cfg {:team-id (:team-id invit) - :profile-id profile-id + ;; The inviter is the creator of + ;; the invitation row, not the + ;; profile requesting the token. + :profile-id (or (:created-by invit) + profile-id) :valid-until (:valid-until invit) :role (:role invit) :member-id (:id member) diff --git a/backend/src/app/rpc/commands/verify_token.clj b/backend/src/app/rpc/commands/verify_token.clj index aafcb47096..4a7d8a8439 100644 --- a/backend/src/app/rpc/commands/verify_token.clj +++ b/backend/src/app/rpc/commands/verify_token.clj @@ -6,6 +6,7 @@ (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] @@ -174,6 +175,7 @@ [:map {:title "TeamInvitationClaims"} [:iss :keyword] [:exp ::ct/inst] + ;; The inviter: always the `created-by` of the invitation row. [:profile-id ::sm/uuid] [:role types.team/schema:role] [:team-id {:optional true} ::sm/uuid] @@ -244,18 +246,6 @@ (not (:is-member membership))) (:organization-id membership)) - organization-add-source - (when organization-id-on-add - (if organization-id - "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 @@ -313,25 +303,34 @@ :user-who-send-invitation (:created-by invitation)) (audit/clean-props)))))) - (cond-> (assoc claims :state :created) + (cond-> (assoc claims + :state :created + ;; The invitation row is authoritative for the + ;; inviter: backfill :profile-id so the response + ;; stays consistent even with tokens minted + ;; before :profile-id was aligned with + ;; :created-by (or re-requested by someone else). + :profile-id (or (:created-by invitation) + (:profile-id claims)) + ;; Likewise, the accepting profile is + ;; authoritative for the invitee: backfill + ;; :member-id (nil for invitations sent to an + ;; unregistered email, possibly stale + ;; otherwise). + :member-id (:id profile)) ;; 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))})))))) + (merge (d/without-nils + {:invitation-id (:id invitation) + :organization-member-count-before + 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 diff --git a/backend/test/backend_tests/rpc_team_test.clj b/backend/test/backend_tests/rpc_team_test.clj index b15013eb8f..b8f782080a 100644 --- a/backend/test/backend_tests/rpc_team_test.clj +++ b/backend/test/backend_tests/rpc_team_test.clj @@ -498,7 +498,7 @@ (map second) (filter #(= "accept-organization-invitation" (:name %))) first)) - frontend-event (atom nil)] + token-result (atom nil)] (db/insert! (:app.db/pool th/*system*) :team-invitation @@ -520,8 +520,7 @@ (fn [& _] default-team-id)] (let [out (verify! direct-token)] (t/is (th/success? out)) - (reset! frontend-event - (get-in out [:result :organization-invitation-audit])))) + (reset! token-result (:result out)))) (let [event (organization-event)] (t/is (= organization-id (get-in event [:props :organization-id]))) @@ -533,19 +532,17 @@ (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-invitation-acceptance" - (:origin @frontend-event))) - (t/is (= organization-id - (get-in @frontend-event [:props :organization-id]))) + (t/is (= organization-id (:organization-id @token-result))) + (t/is (= :editor (:role @token-result))) + (t/is (not (contains? @token-result :organization-invitation-audit))) + (t/is (= (get-in event [:props :invitation-id]) + (:invitation-id @token-result))) (t/is (= (:id invitee) - (get-in @frontend-event [:props :user-id]))) + (:member-id @token-result))) (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]))) + (:profile-id @token-result))) (t/is (= 3 - (get-in @frontend-event [:props :organization-member-count-before]))) + (:organization-member-count-before @token-result))) (t/is (not-any? #(contains? #{"accept-team-invitation" "accept-team-invitation-from"} (:name (second %))) @@ -572,8 +569,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])))) + (reset! token-result (:result out)))) (let [events (mapv second (:call-args-list @audit-mock)) event (organization-event)] @@ -587,20 +583,19 @@ (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]))) + (:organization-id @token-result))) + (t/is (= (:id team) (:team-id @token-result))) + (t/is (= :editor (:role @token-result))) + (t/is (not (contains? @token-result :organization-invitation-audit))) + (t/is (= (get-in event [:props :invitation-id]) + (:invitation-id @token-result))) (t/is (= (:id invitee) - (get-in @frontend-event [:props :user-id]))) + (:member-id @token-result))) (t/is (= (:id inviter) - (get-in @frontend-event [:props :user-who-send-invitation]))) - (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]))) + (:profile-id @token-result))) (t/is (= 5 - (get-in @frontend-event [:props :organization-member-count-before])))) + (:organization-member-count-before @token-result)))) (th/reset-mock! audit-mock) (db/insert! (:app.db/pool th/*system*) @@ -621,13 +616,57 @@ 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])))) + (reset! token-result (:result out)))) (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 (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}] diff --git a/frontend/src/app/main/ui/auth/verify_token.cljs b/frontend/src/app/main/ui/auth/verify_token.cljs index ff08a3679f..14a3a5625f 100644 --- a/frontend/src/app/main/ui/auth/verify_token.cljs +++ b/frontend/src/app/main/ui/auth/verify_token.cljs @@ -6,6 +6,7 @@ (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] @@ -45,12 +46,31 @@ (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)))) + (when (and (= state :created) + (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 diff --git a/frontend/test/frontend_tests/data/nitrate_test.cljs b/frontend/test/frontend_tests/data/nitrate_test.cljs index 2b08740f6a..eb50f0b0c9 100644 --- a/frontend/test/frontend_tests/data/nitrate_test.cljs +++ b/frontend/test/frontend_tests/data/nitrate_test.cljs @@ -121,31 +121,72 @@ (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}] + (let [emitted (atom [])] (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}})) + (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}) - (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/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"