diff --git a/.serena/memories/audit-log.md b/.serena/memories/audit-log.md index 223f370815..3739612214 100644 --- a/.serena/memories/audit-log.md +++ b/.serena/memories/audit-log.md @@ -118,6 +118,21 @@ Critical `profile-id` INVARIANT: the event belongs to **whoever made the request The `::audit/profile-id` override goes through `coerce-profile-id`: a string is parsed, anything that cannot become a uuid is dropped (with a warning) and the event falls back to the caller. Do not pass the value raw: `schema:event` needs a uuid and `submit*` swallows the validation error (it only logs, the RPC still succeeds), so a non-uuid override **loses the row silently**. Hand-built events with `event-from-rpc-params` + `submit` (clone template, accept team invitation, `management/nitrate` push-audit-events) do **not** go through that coercion: they must already carry a uuid. +### Team id and entity ids (projection data) + +The event stream is meant to let a consumer (Nexus) rebuild what it needs **without joining production tables**, so events carry the ids a projection keys on: + +- **Team id enrichment.** After `clean-props`, `prepare-rpc-event` calls `enrich-team-id`: when props have no `:team-id` but carry a `:project-id` or `:file-id`, it resolves the team with a local query and adds `:team-id`. It never resolves from `:id` (read commands put the entity id there, and that would add a query to every read), and it skips read events entirely (`get-*` names plus `has-file-libraries`: their events carry no consumer state). Commands that already loaded the team add `:team-id` themselves in `::audit/props` (`delete-file`, comments, media upload, `delete/update-webhook`, `clone-template`, ...), so the resolver does not run. +- **Membership.** `add-profile-to-team!` emits `add-team-member` (props `:team-id`, `:member-id`, `:role`, `:is-owner`, `:is-admin`, `:can-edit`) whenever it actually inserts a row, only with `:audit-log` on, inside the caller's transaction. It covers the flows that add a member with no event of their own (`create-team` owner, invitee added directly when `:email-verification` is off, approved join-request, duplicated team). With `on-conflict-do-nothing?` a conflict returns nil and emits nothing, so an existing membership is reported once. The event belongs to the actor: the two flows where the owner adds somebody else (`create-invitation`, `add-member-to-team`) pass `:actor-id` through a separate `event-opts` arg (never in `params`, which is inserted as-is); everywhere else actor and member coincide. `:role` is derived in the emitter from the flags (inverse of `permissions-for-role`: owner > admin > editor > viewer) because the table has no role column; request context travels in `event-opts` as `:rpc-params`. +- **Created entity ids.** `create-project` and `create-file` add the new row's `:id` (and `:project-id`/`:team-id`) to `::audit/props`, so a projection can key it. +- **Bulk operations.** `clone-template`, `import-binfile` and `duplicate-project` create entities through internal calls that emit nothing on their own; they emit a `create-file` (and `create-project`) per entity (only with `:audit-log`). The import paths go through `audit/submit-file-created-event`, which reads the light columns (`id`, `name`, `project-id`, timestamps, `revn`) back from the row — the file must really exist, a phantom id emits nothing. `duplicate-project` already holds the rows and uses `audit/submit-create-event` directly. Both binfile versions return the same shape from `import-files!` (`{:file-ids [...]}`). +- **Deletes.** `delete-team` still returns nil but carries the deleted row (`:id`, `:name`, `:created-at`, `:modified-at`, `:deleted-at`) in `::audit/props`, like `delete-project`. `delete-profile` carries `:id`, `:email`, `:fullname`, `:created-at`, `:modified-at` and `:deleted-at` (without this its props are empty: every param is namespaced and `clean-props` drops them). `create-team` adds `:created-at` and `:modified-at`. `delete-team-member` needs nothing: the delete is physical and the params already carry `:team-id`/`:member-id`. +- **Hard deletes.** `objects-gc` emits one `hard-delete-objects` event per purged chunk, carrying only that chunk's entity key (`:team-ids`, `:project-ids` or `:file-ids`; one chunk transaction purges a single entity, so one event can never span sections) with context `{:triggered-by "objects-gc"}`, only with `:audit-log` on. No other GC path emits (profiles, fonts, thumbnails); those purges stay silent. +- **SREPL deletes.** `delete-profile!` carries the subject id in props (it used to emit nothing usable); the event profile stays zero because SREPL has no actor. `delete-team!` props already carried the id. Both travel as actions with a `triggered-by: srepl` context. +- **SREPL restores.** `restore-profile!` carries the full row (with id) in props with a zero event profile; `restore-team!` and `restore-project!` already carried their ids. The consumer resolves the subject from props by a uniform rule, with no SREPL special case. +- **Cascades.** The `:delete-object` worker task emits one `delete-object-with-cascade` event per execution, type `action`, with `:object`/`:id`/`:deleted-at` plus the affected ids nested under `:cascade` (`:team-ids`, `:project-ids`, `:file-ids`), in the same transaction. The handler submits unconditionally (`submit` fans out per flag, so no `:audit-log` re-check); the `audit_log` row itself is still only written with `:audit-log` on. Direct calls (objects-gc) and snapshot cascades stay silent on purpose. +- **Ownership.** `create-team-with-invitations` carries the new team `:id` like `create-team`; `update-team-member-role` names the demoted member in `:prev-owner-id` on owner transfers. + Entry points: - `submit`: the normal one (fills defaults, validates, runs inside `tx-run!`, never breaks the RPC). @@ -223,6 +238,8 @@ Webhooks let an external URL get notified when something happens in a team (for One guarantee: sending webhooks never changes the audit event itself. If delivery fails, only the webhook (and its delivery log) is affected; the audit row stays untouched and still gets archived normally. +Every match also writes a `webhook` / `trigger` row to `audit_log` (even with zero matches): its props are `{:name , :event-id , :total-affected }`. `process-event` assigns the origin row id up-front so the trigger can reference it; the telemetry shadow row always gets its own id (reusing it would collide on the PK). + ## 10. Consumers II: error reporters Both `app.loggers.database` and `app.loggers.mattermost` listen to backend `:error` logs **and** frontend crash events (found by the `::audit/event` marker), through a sliding-buffer channel so a flood cannot stall the app. The database one stores them in `server-error-report` (source 3 = backend log, 4 = audit-log origin, 5 = rate-limit); the Mattermost one forwards a short note to `:error-report-webhook` when set. @@ -308,13 +325,15 @@ Config (`backend/src/app/config.clj`): `:audit-log-archive-uri`, `:telemetry-uri - **New backend command that must (or must not) emit**: `wrap-audit` does it alone; to exclude it set `::audit/skip`. To customize props/name/type use result metadata. Add a test mocking `app.loggers.audit/submit` (nil; `helpers.clj` stubs it globally) and assert `:call-args-list` — see `backend_tests/rpc-audit-test.clj` (full-row, telemetry-only, dual, no-op without flags, `insert`, `prepare-rpc-event`) and `rpc-team-test/accept-organization-invitation-audit-event` (three cases, exact props; counts matter: asserting with `first` passes even when a command emits the same name twice). - **New Penpot frontend event**: create the data-event or implement `Event`/`PerformanceEvent` in `app.main.data.event`, with `::name`/`::type`/`::origin`. Honor `skip-audit?` if the action can resume. Remember: with no props schema, **your asserts are the contract**. - **New admin-console event**: server action → add the backend-log call next to the mutation, best-effort so it never breaks the action; browser action → add a new event helper plus its test. +- **Command that creates an entity or already knows its team**: put the new row's `:id` in `::audit/props` (`create-project`/`create-file`) and set `:team-id` when the team is loaded, so the consumer never has to join production tables. For entities created inside an internal call (`clone-template`, `import-binfile`, `duplicate-project`), emit one event per entity: `audit/submit-file-created-event` when only the id is known (it reads the row back), `audit/submit-create-event` when the row is already in hand. - **Never** put secrets in `props`; `clean-props` is a last defense, not an excuse. - **Do not duplicate events**: if the backend already emits an event for an action, do not add a frontend (or admin-console browser) event for the same action — the backend one wins because it is reliable. A second event for the same action is only acceptable with a strong reason (for example the backend cannot see it at all), and then it must be clearly justified. - **Follow the naming policy**: use kebab-case names (`"create-organization"`, not `"createOrganization"`), and before inventing prop keys, look at similar existing events and reuse their key names (`organization-id`, `team-id`, `file-id`, `event-origin`...). Same concept, same key spelling — that is what keeps the data queryable later. ## 15. Code map -- Backend producer: `backend/src/app/loggers/audit.clj` (`submit`, `submit*`, `insert`, `prepare-rpc-event`, `event-from-rpc-params`, telemetry filters), `backend/src/app/rpc.clj` (`wrap-audit`), `backend/src/app/rpc/commands/audit.clj` (`push-audit-events`, `get-environment-data`, deprecated `get-enabled-flags`), `backend/src/app/rpc/management/nitrate.clj` (`push-audit-events` Nitrate→Penpot), `backend/src/app/http/middleware.clj` + `backend/src/app/http/management.clj` (shared-key auth). +- Backend producer: `backend/src/app/loggers/audit.clj` (`submit`, `submit*`, `insert`, `prepare-rpc-event`, `enrich-team-id`, `submit-create-event`, `event-from-rpc-params`, telemetry filters), `backend/src/app/rpc.clj` (`wrap-audit`), `backend/src/app/rpc/commands/audit.clj` (`push-audit-events`, `get-environment-data`, deprecated `get-enabled-flags`), `backend/src/app/rpc/management/nitrate.clj` (`push-audit-events` Nitrate→Penpot), `backend/src/app/http/middleware.clj` + `backend/src/app/http/management.clj` (shared-key auth). +- Membership emitter: `backend/src/app/rpc/commands/teams.clj` (`add-profile-to-team!` → `add-team-member`). - Penpot frontend producer: `frontend/src/app/main/data/event.cljs` (collector, buffer, send), `frontend/src/app/main/repo.cljs` + `frontend/src/app/util/http.cljs` (headers), `frontend/src/app/main/data/nitrate.cljs` (Nitrate membership events). - Admin-console producer (private repo, no paths cited here): server-side organization/subscription events through the management `push-audit-events`, browser-side UI events through the public `push-audit-events`. - Consumers: `backend/src/app/loggers/webhooks.clj`, `backend/src/app/loggers/database.clj`, `backend/src/app/loggers/mattermost.clj`, `backend/src/app/tasks/telemetry.clj`. diff --git a/backend/src/app/binfile/v1.clj b/backend/src/app/binfile/v1.clj index d7468eaa38..83423e771a 100644 --- a/backend/src/app/binfile/v1.clj +++ b/backend/src/app/binfile/v1.clj @@ -482,9 +482,9 @@ (bfm/apply-pending-migrations! cfg) ;; Knowing that the ids of the created files are in index, - ;; just lookup them and return it as a set + ;; just lookup them and return them in the same shape as v3. (let [files (-> bfc/*state* deref :files)] - (into #{} (keep #(get-in @bfc/*state* [:index %])) files)))))) + {:file-ids (into [] (keep #(get-in @bfc/*state* [:index %])) files)}))))) (defmethod read-import :v1 [options] diff --git a/backend/src/app/loggers/audit.clj b/backend/src/app/loggers/audit.clj index 085d402faf..a25e765ba5 100644 --- a/backend/src/app/loggers/audit.clj +++ b/backend/src/app/loggers/audit.clj @@ -234,7 +234,9 @@ (defn- append-audit-entry [cfg params] (let [params (-> params - (assoc :id (uuid/next)) + ;; Keep a pre-assigned id (process-event assigns one so + ;; the webhook task can reference the stored row). + (update :id #(or % (uuid/next))) (update :props db/tjson) (update :context db/tjson) (update :ip-addr db/inet)) @@ -253,62 +255,70 @@ (defn- process-event [cfg event] - (when (contains? cf/flags :audit-log-logger) - (l/log! ::l/logger "app.audit" - ::l/level :info - :profile-id (str (:profile-id event)) - :ip-addr (str (:ip-addr event)) - :type (:type event) - :name (:name event) - :props (json/encode (:props event) :key-fn json/write-camel-key) - :context (json/encode (:context event) :key-fn json/write-camel-key))) + ;; Assign the row id up-front so every sink below shares it: the + ;; stored row and the `:process-webhook-event` task params. The + ;; webhook trigger row links back to its origin event through props + ;; `:event-id`, which used to land nil because the id was generated + ;; inside `append-audit-entry` without reaching the worker params. + (let [event (assoc event :id (or (:id event) (uuid/next)))] + (when (contains? cf/flags :audit-log-logger) + (l/log! ::l/logger "app.audit" + ::l/level :info + :profile-id (str (:profile-id event)) + :ip-addr (str (:ip-addr event)) + :type (:type event) + :name (:name event) + :props (json/encode (:props event) :key-fn json/write-camel-key) + :context (json/encode (:context event) :key-fn json/write-camel-key))) - (when (contains? cf/flags :audit-log) - (append-audit-entry cfg event)) + (when (contains? cf/flags :audit-log) + (append-audit-entry cfg event)) - (when (contains? cf/flags :telemetry) - ;; NOTE: when both audit-log and telemetry are enabled, events are stored - ;; twice: once with full details (above) and once stripped of props and - ;; ip-addr, tagged with source="telemetry" so the telemetry task can - ;; collect and ship them. The profile-id is preserved (UUIDs are already - ;; anonymous random identifiers). Only a safe subset of context fields - ;; is kept: initiator, version, client-version and client-user-agent. - ;; Timestamps are truncated to day precision to avoid leaking exact event - ;; timing. - (let [event (-> event - (filter-telemetry-props) - (filter-telemetry-context) - (update :created-at ct/truncate :days) - (update :tracked-at ct/truncate :days) - (assoc :source "telemetry:backend") - (assoc :ip-addr "0.0.0.0"))] - (append-audit-entry cfg event))) + (when (contains? cf/flags :telemetry) + ;; NOTE: when both audit-log and telemetry are enabled, events are stored + ;; twice: once with full details (above) and once stripped of props and + ;; ip-addr, tagged with source="telemetry" so the telemetry task can + ;; collect and ship them. The profile-id is preserved (UUIDs are already + ;; anonymous random identifiers). Only a safe subset of context fields + ;; is kept: initiator, version, client-version and client-user-agent. + ;; Timestamps are truncated to day precision to avoid leaking exact event + ;; timing. The shadow row gets its own id: reusing the full row id + ;; would collide on the primary key. + (let [event (-> event + (assoc :id (uuid/next)) + (filter-telemetry-props) + (filter-telemetry-context) + (update :created-at ct/truncate :days) + (update :tracked-at ct/truncate :days) + (assoc :source "telemetry:backend") + (assoc :ip-addr "0.0.0.0"))] + (append-audit-entry cfg event))) - (when (and (contains? cf/flags :webhooks) - (::webhooks/event? event)) - (let [batch-key (::webhooks/batch-key event) - batch-timeout (::webhooks/batch-timeout event) - label (dm/str "rpc:" (:name event)) - label (cond - (ifn? batch-key) (dm/str label ":" (batch-key (::rpc/params event))) - (string? batch-key) (dm/str label ":" batch-key) - :else label) - dedupe? (boolean (and batch-key batch-timeout))] + (when (and (contains? cf/flags :webhooks) + (::webhooks/event? event)) + (let [batch-key (::webhooks/batch-key event) + batch-timeout (::webhooks/batch-timeout event) + label (dm/str "rpc:" (:name event)) + label (cond + (ifn? batch-key) (dm/str label ":" (batch-key (::rpc/params event))) + (string? batch-key) (dm/str label ":" batch-key) + :else label) + dedupe? (boolean (and batch-key batch-timeout))] - (wrk/submit! (-> cfg - (assoc ::wrk/task :process-webhook-event) - (assoc ::wrk/queue :webhooks) - (assoc ::wrk/max-retries 0) - (assoc ::wrk/delay (or batch-timeout 0)) - (assoc ::wrk/dedupe dedupe?) - (assoc ::wrk/label label) - (assoc ::wrk/params (-> event - (d/without-qualified) - (dissoc :source) - (dissoc :context) - (dissoc :ip-addr) - (dissoc :type))))))) - event) + (wrk/submit! (-> cfg + (assoc ::wrk/task :process-webhook-event) + (assoc ::wrk/queue :webhooks) + (assoc ::wrk/max-retries 0) + (assoc ::wrk/delay (or batch-timeout 0)) + (assoc ::wrk/dedupe dedupe?) + (assoc ::wrk/label label) + (assoc ::wrk/params (-> event + (d/without-qualified) + (dissoc :source) + (dissoc :context) + (dissoc :ip-addr) + (dissoc :type))))))) + event)) (defn submit* "A public API, lower-level than submit, assumes all required fields are filled" @@ -396,6 +406,47 @@ coerced)) +(def ^:private sql:team-id-by-project + "SELECT id FROM team + WHERE id = (SELECT team_id FROM project WHERE id = ?)") + +(def ^:private sql:team-id-by-file + "SELECT t.id FROM team AS t + JOIN project AS p ON (p.team_id = t.id) + JOIN file AS f ON (f.project_id = p.id) + WHERE f.id = ?") + +(def ^:private read-event-names + "Read commands excluded from team-id resolution on top of the `get-*` + prefix: their events carry no consumer state, so the lookup would only + tax the read path." + #{"has-file-libraries"}) + +(defn- read-event? + [event-name] + (or (str/starts-with? event-name "get-") + (contains? read-event-names event-name))) + +(defn- enrich-team-id + "Adds the team id to the event props when it is missing and can be resolved + locally, so consumers do not have to join production tables. Only events that + already carry a project or file id are considered: read commands put the + entity id under `:id`, and resolving those would add a query to every read." + [cfg event-name props] + (if (or (:team-id props) + (read-event? event-name) + (and (nil? (:project-id props)) + (nil? (:file-id props)))) + props + (let [project-id (:project-id props) + file-id (:file-id props) + team-id (or (when (some? project-id) + (:id (db/exec-one! cfg [sql:team-id-by-project project-id]))) + (when (some? file-id) + (:id (db/exec-one! cfg [sql:team-id-by-file file-id]))))] + (cond-> props + (some? team-id) (assoc :team-id team-id))))) + (defn prepare-rpc-event [cfg mdata params result] (let [resultm (meta result) @@ -410,25 +461,32 @@ (::rpc/profile-id params) uuid/zero) + module (get cfg ::rpc/module) + event-name (or (::name resultm) + (let [sname (::sv/name mdata)] + (if (not= module "main") + (str module "-" sname) + sname))) + props (-> (or (::replace-props resultm) (merge params (::props resultm))) (clean-props)) + ;; Resolve the team locally when the event carries a project or file + ;; id, so the consumers do not need to join production tables. + ;; Reads are excluded: their events carry no consumer state. + props (enrich-team-id cfg event-name props) + context (-> (::context resultm) (merge (prepare-context-from-request request)) (assoc :request-id (::rpc/request-id params)) (d/without-nils)) - ip-addr (inet/parse-request request) - module (get cfg ::rpc/module)] + ip-addr (inet/parse-request request)] {:type (or (::type resultm) (::rpc/type cfg)) - :name (or (::name resultm) - (let [sname (::sv/name mdata)] - (if (not= module "main") - (str module "-" sname) - sname))) + :name event-name :profile-id profile-id :ip-addr ip-addr @@ -485,6 +543,33 @@ (d/without-nils))] (submit* cfg event))) +(defn submit-create-event + "Emits a `create-*` audit event for an entity created as a side effect of a + bulk operation (import, duplicate, clone). Those entities get no event from + their own command, so a local projection cannot key them otherwise. The + event is built by hand and does not pass through `prepare-rpc-event`, so its + props are passed as-is minus nils." + [cfg params event-name props] + (when (contains? cf/flags :audit-log) + (submit cfg + (-> (event-from-rpc-params params) + (assoc :name event-name) + (assoc :props (d/without-nils props)))))) + +(def ^:private sql:get-file-event-columns + "SELECT id, name, project_id, created_at, modified_at, revn + FROM file WHERE id = ?;") + +(defn submit-file-created-event + "Emit a `create-file` event for a file born inside a bulk operation, + with the same prop shape as the `create-file` command event. Only + the file id is needed: the light columns are read back (no data + blob), so projections that key on name, revn or timestamps work. + `extra` carries what the row lacks (the team id)." + [cfg params file-id extra] + (when-let [row (db/exec-one! cfg [sql:get-file-event-columns file-id])] + (submit-create-event cfg params "create-file" (merge row extra)))) + (defn insert "Submit audit event to the collector, intended to be used only from command line helpers because this skips all webhooks and telemetry diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index d1cff7f2ab..c8898f90e6 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -665,7 +665,7 @@ ::wrk/tenant (cf/get :tenant)} [::default ::wrk/runner] - {::wrk/parallelism (cf/get ::worker-default-parallelism 1) + {::wrk/parallelism (cf/get :worker-default-parallelism 1) ::wrk/queue :default ::wrk/tenant (cf/get :tenant) ::rds/client (ig/ref ::rds/client) @@ -674,7 +674,7 @@ ::db/pool (ig/ref ::db/pool)} [::webhook ::wrk/runner] - {::wrk/parallelism (cf/get ::worker-webhook-parallelism 1) + {::wrk/parallelism (cf/get :worker-webhook-parallelism 1) ::wrk/queue :webhooks ::wrk/tenant (cf/get :tenant) ::rds/client (ig/ref ::rds/client) diff --git a/backend/src/app/rpc.clj b/backend/src/app/rpc.clj index a558315376..e28485181f 100644 --- a/backend/src/app/rpc.clj +++ b/backend/src/app/rpc.clj @@ -426,7 +426,8 @@ (defn- resolve-management-methods [cfg] (let [cfg (assoc cfg ::module "management" ::type "command" ::metrics-id :rpc-management-timing) - mods (cond->> (list 'app.rpc.management.exporter) + mods (cond->> (list 'app.rpc.management.exporter + 'app.rpc.management.files) (contains? cf/flags :admin-console) (cons 'app.rpc.management.nitrate))] diff --git a/backend/src/app/rpc/commands/binfile.clj b/backend/src/app/rpc/commands/binfile.clj index de0d3faadd..c4fc37cb99 100644 --- a/backend/src/app/rpc/commands/binfile.clj +++ b/backend/src/app/rpc/commands/binfile.clj @@ -17,7 +17,7 @@ [app.config :as cf] [app.db :as db] [app.http.sse :as sse] - [app.loggers.audit :as-alias audit] + [app.loggers.audit :as audit] [app.loggers.webhooks :as-alias webhooks] [app.media.validation :as media.v] [app.rpc :as-alias rpc] @@ -91,7 +91,7 @@ ;; --- Command: import-binfile (defn- import-binfile - [{:keys [::db/pool] :as cfg} {:keys [profile-id project-id version name file upload-id]}] + [{:keys [::db/pool] :as cfg} {:keys [profile-id project-id version name file upload-id] :as params}] (let [team (teams/get-team pool :profile-id profile-id @@ -133,6 +133,14 @@ {:id project-id} {::db/return-keys false}) + ;; The imported files get no event from their own command, so emit one per + ;; file inside a single transaction. + (db/tx-run! cfg + (fn [cfg] + (doseq [file-id (:file-ids result)] + (audit/submit-file-created-event cfg params file-id + {:team-id (:id team)})))) + result)) (def ^:private schema:import-binfile diff --git a/backend/src/app/rpc/commands/comments.clj b/backend/src/app/rpc/commands/comments.clj index 9de796a72e..8ca0a165bc 100644 --- a/backend/src/app/rpc/commands/comments.clj +++ b/backend/src/app/rpc/commands/comments.clj @@ -486,7 +486,7 @@ (let [params (assoc params ::file file) thread (db/tx-run! cfg create-comment-thread params)] - (vary-meta thread assoc ::audit/props thread)))) + (vary-meta thread assoc ::audit/props (assoc thread :team-id team-id))))) (defn- create-comment-thread [{:keys [::db/conn] :as cfg} @@ -650,7 +650,7 @@ (send-comment-emails! conn profile comment thread file) - (vary-meta comment assoc ::audit/props comment)))) + (vary-meta comment assoc ::audit/props (assoc comment :team-id team-id))))) ;; --- COMMAND: Update Comment diff --git a/backend/src/app/rpc/commands/files.clj b/backend/src/app/rpc/commands/files.clj index fd313f6170..1679e76204 100644 --- a/backend/src/app/rpc/commands/files.clj +++ b/backend/src/app/rpc/commands/files.clj @@ -1098,7 +1098,9 @@ {::audit/props {:project-id (:project-id file) :name (:name file) :created-at (:created-at file) - :modified-at (:modified-at file)}}))) + :modified-at (:modified-at file) + :deleted-at (:deleted-at file) + :team-id (:id team)}}))) (sv/defmethod ::delete-file {::doc/added "1.17" diff --git a/backend/src/app/rpc/commands/files_create.clj b/backend/src/app/rpc/commands/files_create.clj index d1b14283a5..23959989af 100644 --- a/backend/src/app/rpc/commands/files_create.clj +++ b/backend/src/app/rpc/commands/files_create.clj @@ -137,5 +137,11 @@ {:id team-id} {::db/return-keys false})))) - (-> (create-file cfg params) - (vary-meta assoc ::audit/props {:team-id team-id})))) + (let [file (create-file cfg params)] + (vary-meta file assoc ::audit/props {:id (:id file) + :project-id project-id + :team-id team-id + :name (:name file) + :created-at (:created-at file) + :modified-at (:modified-at file) + :revn (:revn file)})))) diff --git a/backend/src/app/rpc/commands/files_share.clj b/backend/src/app/rpc/commands/files_share.clj index 6643554a06..66bb1447ad 100644 --- a/backend/src/app/rpc/commands/files_share.clj +++ b/backend/src/app/rpc/commands/files_share.clj @@ -12,9 +12,11 @@ [app.common.schema :as sm] [app.common.uuid :as uuid] [app.db :as db] + [app.loggers.audit :as-alias audit] [app.rpc :as-alias rpc] [app.rpc.commands.files :as files] [app.rpc.doc :as-alias doc] + [app.rpc.helpers :as rph] [app.util.services :as sv])) ;; --- MUTATION: Create Share Link @@ -39,7 +41,11 @@ ::db/transaction true} [{:keys [::db/conn]} {:keys [::rpc/profile-id file-id] :as params}] (files/check-edition-permissions! conn profile-id file-id) - (create-share-link conn (assoc params :profile-id profile-id))) + (let [slink (create-share-link conn (assoc params :profile-id profile-id))] + ;; The consumers key share scope by file, so the event carries + ;; both ids; `:file-id` is already in params, `:share-id` is new. + (rph/with-meta slink + {::audit/props {:share-id (:id slink)}}))) (defn create-share-link [conn {:keys [profile-id file-id pages who-comment who-inspect]}] @@ -80,4 +86,8 @@ :hint "You can only delete share-links you created"))) (db/delete! conn :share-link {:id id}) - nil)) + ;; The delete params carry only the link id, so resolve the file + ;; from the row fetched above before it is gone. + (rph/with-meta (rph/wrap) + {::audit/props {:file-id (:file-id slink) + :share-id id}}))) diff --git a/backend/src/app/rpc/commands/files_update.clj b/backend/src/app/rpc/commands/files_update.clj index 9d996c6dd4..373a06717f 100644 --- a/backend/src/app/rpc/commands/files_update.clj +++ b/backend/src/app/rpc/commands/files_update.clj @@ -283,11 +283,18 @@ (with-meta {:revn revn :lagged (get-lagged-changes conn params)} {::audit/replace-props - {:id (:id file) - :name (:name file) - :features (:features file) - :project-id (:project-id file) - :team-id (:team-id file)}})))) + {:id (:id file) + :name (:name file) + :features (:features file) + :project-id (:project-id file) + :team-id (:team-id file) + ;; NOTE: `file` carries the post-update revn here (`revn` + ;; above is the pre-update one kept for the response), and + ;; `modified-at` mirrors `persist-file!`: same `::timestamp` + ;; fallback, so both agree to the millisecond when the + ;; request timestamp exists. + :revn (:revn file) + :modified-at (or timestamp (ct/now))}})))) (defn get-file "Get not-decoded file, only decodes the features set." diff --git a/backend/src/app/rpc/commands/management.clj b/backend/src/app/rpc/commands/management.clj index 62d5a88330..b0ac390b57 100644 --- a/backend/src/app/rpc/commands/management.clj +++ b/backend/src/app/rpc/commands/management.clj @@ -142,12 +142,29 @@ (when (uuid? profile-id) (teams/create-project-role conn profile-id (:id project) :owner)) + ;; The new entities are created by internal calls, so they carry no + ;; event of their own; emit one per entity for a local projection. + (audit/submit-create-event cfg params "create-project" + {:id (:id project) + :team-id (:team-id project) + :name (:name project) + :created-at (:created-at project) + :modified-at (:modified-at project)}) + (doseq [file-id files] (let [params (-> params (dissoc :name) (assoc :file-id file-id) - (assoc :reset-shared-flag false))] - (duplicate-file cfg params))) + (assoc :reset-shared-flag false)) + file (duplicate-file cfg params)] + (audit/submit-create-event cfg params "create-file" + {:id (:id file) + :project-id (:project-id file) + :team-id (:team-id project) + :name (:name file) + :created-at (:created-at file) + :modified-at (:modified-at file) + :revn (:revn file)}))) project)))) @@ -211,7 +228,7 @@ (update :team-id bfc/lookup-index) (assoc :created-at timestamp) (assoc :modified-at timestamp))] - (teams/add-profile-to-team! cfg params {::db/return-keys false}))) + (teams/add-profile-to-team! cfg params))) ;; Duplicate team fonts (doseq [font fonts] @@ -444,14 +461,12 @@ {:id project-id} {::db/return-keys false}) - (let [props (audit/clean-props params)] - (doseq [file-id result] - (let [props (assoc props :id file-id) - event (-> (audit/event-from-rpc-params params) - (assoc :profile-id profile-id) - (assoc :name "create-file") - (assoc :props props))] - (audit/submit cfg event)))))) + (doseq [file-id (:file-ids result)] + (audit/submit-file-created-event + cfg + (assoc params :profile-id profile-id) + file-id + {:team-id (:id team)})))) result)) diff --git a/backend/src/app/rpc/commands/media.clj b/backend/src/app/rpc/commands/media.clj index e914ff3302..1c9e07ed01 100644 --- a/backend/src/app/rpc/commands/media.clj +++ b/backend/src/app/rpc/commands/media.clj @@ -68,27 +68,28 @@ (quotes/check! cfg {::quotes/id ::quotes/media-storage-bytes-per-team ::quotes/profile-id profile-id ::quotes/team-id team-id - ::quotes/incr (:size content)})) + ::quotes/incr (:size content)}) - (db/run! cfg (fn [{:keys [::db/conn] :as cfg}] - ;; We get the minimal file for proper checking if - ;; file is not already deleted - (let [_ (files/get-minimal-file conn file-id) - mobj (create-file-media-object cfg params)] + (db/run! cfg (fn [{:keys [::db/conn] :as cfg}] + ;; We get the minimal file for proper checking if + ;; file is not already deleted + (let [_ (files/get-minimal-file conn file-id) + mobj (create-file-media-object cfg params)] - (db/update! conn :file - {:modified-at (ct/now) - :has-media-trimmed false} - {:id file-id} - {::db/return-keys false}) + (db/update! conn :file + {:modified-at (ct/now) + :has-media-trimmed false} + {:id file-id} + {::db/return-keys false}) - (with-meta mobj - {::audit/replace-props - {:name (:name params) - :file-id file-id - :is-local (:is-local params) - :size (:size content) - :mtype (:mtype content)}}))))) + (with-meta mobj + {::audit/replace-props + {:name (:name params) + :file-id file-id + :team-id team-id + :is-local (:is-local params) + :size (:size content) + :mtype (:mtype content)}})))))) (defn- big-enough-for-thumbnail? "Checks if the provided image info is big enough for diff --git a/backend/src/app/rpc/commands/profile.clj b/backend/src/app/rpc/commands/profile.clj index 686884aeed..82d307bb46 100644 --- a/backend/src/app/rpc/commands/profile.clj +++ b/backend/src/app/rpc/commands/profile.clj @@ -540,7 +540,8 @@ {::doc/added "1.0" ::db/transaction true} [{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id] :as params}] - (let [teams (get-owned-teams conn profile-id) + (let [profile (db/get* conn :profile {:id profile-id} {::db/remove-deleted false}) + teams (get-owned-teams conn profile-id) deleted-at (ct/now)] ;; If we found owned teams with participants, we don't allow @@ -581,6 +582,12 @@ (session/invalidate-all cfg profile-id) (-> (rph/wrap nil) + (rph/with-meta {::audit/props {:id (:id profile) + :email (:email profile) + :fullname (:fullname profile) + :created-at (:created-at profile) + :modified-at (:modified-at profile) + :deleted-at deleted-at}}) (rph/with-transform (session/delete-fn cfg))))) (def sql:get-subscription-editors diff --git a/backend/src/app/rpc/commands/projects.clj b/backend/src/app/rpc/commands/projects.clj index 157b44310a..5c809cfd96 100644 --- a/backend/src/app/rpc/commands/projects.clj +++ b/backend/src/app/rpc/commands/projects.clj @@ -222,8 +222,9 @@ ::quotes/profile-id profile-id ::quotes/team-id team-id}) - (let [params (assoc params :profile-id profile-id)] - (db/tx-run! cfg create-project params))) + (let [params (assoc params :profile-id profile-id) + project (db/tx-run! cfg create-project params)] + (rph/with-meta project {::audit/props {:id (:id project)}}))) ;; --- MUTATION: Toggle Project Pin diff --git a/backend/src/app/rpc/commands/teams.clj b/backend/src/app/rpc/commands/teams.clj index a878018511..0cfb0f2baf 100644 --- a/backend/src/app/rpc/commands/teams.clj +++ b/backend/src/app/rpc/commands/teams.clj @@ -28,6 +28,7 @@ [app.rpc :as-alias rpc] [app.rpc.commands.profile :as profile] [app.rpc.doc :as-alias doc] + [app.rpc.helpers :as rph] [app.rpc.permissions :as perms] [app.rpc.quotes :as quotes] [app.setup :as-alias setup] @@ -563,7 +564,9 @@ team (db/tx-run! cfg create-team params)] (with-meta team - {::audit/props {:id (:id team)}}))) + {::audit/props {:id (:id team) + :created-at (:created-at team) + :modified-at (:modified-at team)}}))) (defn create-default-organization-team @@ -618,10 +621,42 @@ :default-team-id default-team-id})) default-team-id)))))))) +(defn- params->role + "Derive the team role from membership flags, inverse of + `types.team/permissions-for-role`. `team_profile_rel` carries no role + column, so no caller can pass it in `params`: derive it here so the event + always carries one." + [{:keys [is-owner is-admin can-edit]}] + (cond is-owner :owner is-admin :admin can-edit :editor :else :viewer)) + +(defn- submit-membership-event + "Emits an audit event for a new team membership, so the event stream carries + every membership add, including the ones that only happen as a side effect of + another command (team creation, invitation accepted without email roundtrip, + duplicated team). `event-opts` carries what the row lacks: `:actor-id` (the + action owner, defaulting to the member) and `:rpc-params` (request context, + which sibling events get from `prepare-rpc-event`)." + [cfg {:keys [profile-id team-id] :as params} {:keys [actor-id rpc-params]}] + (when (contains? cf/flags :audit-log) + (audit/submit cfg + (-> (audit/event-from-rpc-params (or rpc-params {})) + (assoc :name "add-team-member" + :type "action" + :profile-id (or actor-id profile-id)) + (assoc :props (d/without-nils + {:team-id team-id + :member-id profile-id + :role (params->role params) + :is-owner (boolean (:is-owner params)) + :is-admin (boolean (:is-admin params)) + :can-edit (boolean (:can-edit params))})))))) + (defn add-profile-to-team! ([cfg params] - (add-profile-to-team! cfg params nil)) - ([{:keys [::db/conn] :as cfg} {:keys [:profile-id :team-id] :as params} options] + (add-profile-to-team! cfg params nil nil)) + ([cfg params options] + (add-profile-to-team! cfg params options nil)) + ([{:keys [::db/conn] :as cfg} {:keys [:profile-id :team-id] :as params} options event-opts] (assert (db/connection-map? cfg) "expected cfg with valid connection") (when (contains? cf/flags :admin-console) @@ -631,7 +666,13 @@ (some? (:organization-id membership)) ;; the team do belong to an organization (not (:is-member membership))) ;; the user is not a member of the organization yet (initialize-user-in-organization cfg profile-id (:organization-id membership))))) - (db/insert! conn :team-profile-rel (assoc params :id (uuid/next)) options))) + (let [options (merge options {::db/return-keys [:id]}) + result (db/insert! conn :team-profile-rel (assoc params :id (uuid/next)) options)] + ;; Only emit when a row was actually inserted: with `on-conflict-do-nothing` + ;; an existing membership returns nil and must not be reported twice. + (when (some? result) + (submit-membership-event cfg params event-opts)) + result))) (defn create-team "This is a complete team creation process, it creates the team @@ -894,8 +935,14 @@ ::sm/params schema:delete-team ::db/transaction true} [cfg {:keys [::rpc/profile-id id] :as params}] - (delete-team cfg {:team-id id :profile-id profile-id}) - nil) + (let [team (delete-team cfg {:team-id id :profile-id profile-id})] + ;; The client keeps receiving nil; the audit event carries the deleted row. + (rph/with-meta (rph/wrap) + {::audit/props {:id (:id team) + :name (:name team) + :created-at (:created-at team) + :modified-at (:modified-at team) + :deleted-at (:deleted-at team)}}))) ;; --- Mutation: Team Update Role @@ -962,7 +1009,13 @@ {::doc/added "1.17" ::sm/params schema:update-team-member-role} [cfg {:keys [::rpc/profile-id] :as params}] - (db/tx-run! cfg update-team-member-role (assoc params :profile-id profile-id))) + (db/tx-run! cfg update-team-member-role (assoc params :profile-id profile-id)) + ;; The client keeps receiving nil; when ownership moves, the audit event + ;; names the demoted member so a projection can keep a single owner. + (rph/with-meta (rph/wrap) + (if (= :owner (:role params)) + {::audit/props {:prev-owner-id profile-id}} + {}))) ;; --- Mutation: Delete Team Member diff --git a/backend/src/app/rpc/commands/teams_invitations.clj b/backend/src/app/rpc/commands/teams_invitations.clj index fe8abd5b24..66ac06493d 100644 --- a/backend/src/app/rpc/commands/teams_invitations.clj +++ b/backend/src/app/rpc/commands/teams_invitations.clj @@ -202,7 +202,10 @@ (when (contains? cf/flags :admin-console) (teams/initialize-user-in-organization cfg (:id member) (:id organization) email)) ;; Insert the invited member to the team - (teams/add-profile-to-team! cfg params {::db/on-conflict-do-nothing? true})) + (teams/add-profile-to-team! cfg params + {::db/on-conflict-do-nothing? true} + {:actor-id (:id profile) + :rpc-params params})) ;; If profile is not yet verified, mark it as verified because ;; accepting an invitation link serves as verification. @@ -318,7 +321,7 @@ :role :editor)))) (defn- add-member-to-team - [{:keys [::db/conn] :as cfg} profile team role member] + [{:keys [::db/conn] :as cfg} profile team role member event-opts] (assert (db/connection-map? cfg) "expected cfg with valid connection") @@ -340,7 +343,9 @@ ::quotes/team-id team-id}) ;; Insert the member to the team - (teams/add-profile-to-team! cfg params {::db/on-conflict-do-nothing? true}) + (teams/add-profile-to-team! cfg params + {::db/on-conflict-do-nothing? true} + event-opts) ;; Delete any request (db/delete! conn :team-access-request @@ -438,7 +443,9 @@ (filter #(contains? invitation-emails (key %))) (map (fn [[email member]] (let [role (:role (first (filter #(= (:email %) email) invitation-data)))] - (add-member-to-team cfg profile team role member)))) + (add-member-to-team cfg profile team role member + {:actor-id (:id profile) + :rpc-params params})))) (doall)) invitations)) @@ -593,7 +600,7 @@ :hint "the maximum of invitation on single request is reached" :threshold max-invitations-by-request-threshold)) - (let [props {:name name :features features} + (let [props {:id (:id team) :name name :features features} event (-> (audit/event-from-rpc-params params) (assoc :name "create-team") (assoc :props props))] @@ -609,7 +616,8 @@ (map (fn [email] (assoc params :email email))) (map (partial create-invitation cfg)))] - (vary-meta team assoc ::audit/props {:invitations (count invitations)})))) + (vary-meta team assoc ::audit/props {:id (:id team) + :invitations (count invitations)})))) ;; --- Query: get-team-invitation-token diff --git a/backend/src/app/rpc/commands/webhooks.clj b/backend/src/app/rpc/commands/webhooks.clj index 2476128e7c..f22a693fd2 100644 --- a/backend/src/app/rpc/commands/webhooks.clj +++ b/backend/src/app/rpc/commands/webhooks.clj @@ -14,10 +14,12 @@ [app.common.uuid :as uuid] [app.db :as db] [app.http.client :as http] + [app.loggers.audit :as-alias audit] [app.loggers.webhooks :as webhooks] [app.rpc :as-alias rpc] [app.rpc.commands.teams :refer [check-read-permissions!] :as t] [app.rpc.doc :as-alias doc] + [app.rpc.helpers :as rph] [app.rpc.permissions :as perms] [app.util.services :as sv] [cuerdas.core :as str])) @@ -137,7 +139,8 @@ (let [whook (-> (db/get pool :webhook {:id id}) (decode-row))] (check-webhook-edition-permissions! pool profile-id (:team-id whook)) (validate-webhook! cfg whook params) - (update-webhook! cfg whook params))) + (rph/with-meta (update-webhook! cfg whook params) + {::audit/props {:team-id (:team-id whook)}}))) (def ^:private schema:delete-webhook [:map {:title "delete-webhook"} @@ -151,7 +154,8 @@ (let [whook (-> (db/get conn :webhook {:id id}) decode-row)] (check-webhook-edition-permissions! conn profile-id (:team-id whook)) (db/delete! conn :webhook {:id id}) - nil)) + (rph/with-meta (rph/wrap) + {::audit/props {:team-id (:team-id whook)}}))) ;; --- Query: Webhooks diff --git a/backend/src/app/rpc/management/files.clj b/backend/src/app/rpc/management/files.clj new file mode 100644 index 0000000000..d947ec7549 --- /dev/null +++ b/backend/src/app/rpc/management/files.clj @@ -0,0 +1,238 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS SUBSIDIARY SL + +(ns app.rpc.management.files + "Read-only file content for the nexus file-security pipeline. + + The single method `get-file-for-analysis` returns the semantic + projection of one file (pages, frames, text, links, interactions): + small, neutral data the decision model consumes. Full file binaries + never cross this boundary. Every call leaves an audit event like any + other management call, so content reads stay traceable; the volume + scales with analyzed files, and the nexus consumer ignores the event." + (:require + [app.binfile.common :as bfc] + [app.common.data :as d] + [app.common.exceptions :as ex] + [app.common.schema :as sm] + [app.common.time :as ct] + [app.common.types.text :as txt] + [app.db :as db] + [app.rpc :as-alias rpc] + [app.rpc.doc :as doc] + [app.util.services :as sv] + [cuerdas.core :as str])) + +(def projection-version 1) + +;; Bounds for the data that crosses to nexus on every sweep. The +;; projection stays small by construction; anything past the caps sets +;; `:truncated?` instead of growing the payload. +(def ^:const max-text-chars 30000) +(def ^:const max-links 1000) +(def ^:const max-interactions 2000) + +(defn- page-shapes + "All shapes of one page with their page id attached." + [{:keys [id objects]}] + (let [page-id (str id)] + (->> (vals objects) + (filter map?) + (map #(assoc % :page-id page-id))))) + +(defn- shape-text + "Plain text of a text shape, or nil when it has none. Never throws: + corrupt content skips the shape instead of failing the file." + [shape] + (when (and (= :text (:type shape)) + (some? (:content shape))) + (let [text (ex/ignoring (txt/content->text (:content shape)))] + (when-not (str/blank? text) + text)))) + +(defn- open-url? + [{:keys [action-type url]}] + (and (= :open-url action-type) + (not (str/blank? url)))) + +(defn file->projection + "Turn decoded file data into the neutral projection the decision + model consumes. Pure: takes the `:data` map, returns plain data + with all ids as strings." + [{:keys [pages pages-index]}] + (let [shapes (mapcat page-shapes + (keep #(get pages-index %) pages)) + frames (into [] + (comp (filter #(= :frame (:type %))) + (map (fn [{:keys [id name page-id]}] + (d/without-nils + {:id (str id) + :page-id page-id + :name name})))) + shapes) + texts (into [] + (comp (map (juxt :id :page-id shape-text)) + (remove #(nil? (nth % 2))) + (map (fn [[id page-id text]] + {:shape-id (str id) + :page-id page-id + :text text}))) + shapes) + ;; Texts are capped by total characters, the unit the model + ;; bills; links and interactions by count. + texts (loop [acc [] + chars 0 + [item & rest] texts] + (cond + (nil? item) + {:items acc :truncated? false} + + (> (+ chars (count (:text item))) max-text-chars) + {:items acc :truncated? true} + + :else + (recur (conj acc item) + (+ chars (count (:text item))) + rest))) + inter (into [] + (comp (mapcat (fn [shape] + (->> (:interactions shape) + (filter map?) + (filter :action-type) + (filter :event-type) + (map #(assoc % :shape-id (:id shape) + :page-id (:page-id shape)))))) + (map (fn [{:keys [shape-id page-id event-type action-type destination url]}] + (d/without-nils + {:shape-id (str shape-id) + :page-id page-id + :event (d/name event-type) + :action (d/name action-type) + :target (or url (some-> destination str))})))) + shapes) + links (into [] + (comp (mapcat (fn [shape] + (->> (:interactions shape) + (filter map?) + (filter open-url?) + (map #(assoc % :shape-id (:id shape) + :page-id (:page-id shape)))))) + (map (fn [{:keys [shape-id page-id url]}] + {:shape-id (str shape-id) + :page-id page-id + :url url}))) + shapes)] + {:version projection-version + :pages (mapv (fn [page-id] + (let [page (get pages-index page-id)] + (d/without-nils + {:id (str page-id) + :name (:name page)}))) + pages) + :frames frames + :texts texts + :links (if (> (count links) max-links) + {:items (subvec links 0 max-links) :truncated? true} + {:items links :truncated? false}) + :interactions (if (> (count inter) max-interactions) + {:items (subvec inter 0 max-interactions) :truncated? true} + {:items inter :truncated? false})})) + +(def ^:private sql:has-share-link + "SELECT EXISTS (SELECT 1 FROM share_link WHERE file_id = ?) AS shared;") + +(def ^:private sql:get-team-id + "SELECT team_id FROM project WHERE id = ?;") + +(def ^:private + schema:get-file-for-analysis-params + [:map {:title "get-file-for-analysis-params"} + [:file-id ::sm/uuid]]) + +(def ^:private schema:projection-page + [:map + [:id :string] + [:name {:optional true} :string]]) + +(def ^:private schema:projection-frame + [:map + [:id :string] + [:page-id :string] + [:name {:optional true} :string]]) + +(def ^:private schema:projection-text + [:map + [:shape-id :string] + [:page-id :string] + [:text :string]]) + +(def ^:private schema:projection-link + [:map + [:shape-id :string] + [:page-id :string] + [:url :string]]) + +(def ^:private schema:projection-interaction + [:map + [:shape-id :string] + [:page-id :string] + [:event :string] + [:action :string] + [:target {:optional true} :string]]) + +(def ^:private schema:projection + [:map {:title "file-projection"} + [:version ::sm/int] + [:pages [:vector schema:projection-page]] + [:frames [:vector schema:projection-frame]] + [:texts [:map + [:items [:vector schema:projection-text]] + [:truncated? :boolean]]] + [:links [:map + [:items [:vector schema:projection-link]] + [:truncated? :boolean]]] + [:interactions [:map + [:items [:vector schema:projection-interaction]] + [:truncated? :boolean]]]]) + +(def ^:private + schema:get-file-for-analysis-result + [:map {:title "get-file-for-analysis-result"} + [:id ::sm/uuid] + [:revn ::sm/int] + [:modified-at ::ct/inst] + [:project-id ::sm/uuid] + [:team-id ::sm/uuid] + [:has-share-link :boolean] + [:projection-version ::sm/int] + [:projection schema:projection]]) + +(sv/defmethod ::get-file-for-analysis + "Get the semantic projection of one file for security analysis. + + Read-only: returns the file identity, revision, share state and the + neutral projection (pages, frames, text, links, interactions). + Called by nexus with the shared-key identity; takes no profile. + Every call leaves an audit event like any other management call." + {::doc/added "2.20" + ::sm/params schema:get-file-for-analysis-params + ::sm/result schema:get-file-for-analysis-result + ::rpc/auth false} + [cfg {:keys [file-id]}] + ;; `realize?` loads the whole object tree eagerly: the projection + ;; walks every shape, so lazy pointers (no `load-fn` bound here) + ;; would fail on first touch. Same choice as the file exporter. + (let [file (bfc/get-file cfg file-id :realize? true) + shared (:shared (db/exec-one! cfg [sql:has-share-link file-id])) + team-id (:team-id (db/exec-one! cfg [sql:get-team-id (:project-id file)]))] + {:id (:id file) + :revn (:revn file) + :modified-at (:modified-at file) + :project-id (:project-id file) + :team-id team-id + :has-share-link (boolean shared) + :projection-version projection-version + :projection (file->projection (:data file))})) diff --git a/backend/src/app/srepl/main.clj b/backend/src/app/srepl/main.clj index b617934c46..8a475e2cae 100644 --- a/backend/src/app/srepl/main.clj +++ b/backend/src/app/srepl/main.clj @@ -782,7 +782,7 @@ :type "action" :props {:id team-id} :context {:triggered-by "srepl" - :cause "explicit call to delete-profile!"} + :cause "explicit call to delete-team!"} :tracked-at tnow}) (wrk/invoke! (-> sys/system @@ -851,6 +851,7 @@ (audit/insert sys/system {:name "delete-profile" :type "action" + :props {:id profile-id} :context {:triggered-by "srepl" :cause "explicit call to delete-profile!"} :tracked-at tnow}) @@ -1039,4 +1040,4 @@ (let [params (-> rel (assoc :id (uuid/next)) (assoc :team-id (:id team)))] - (teams/add-profile-to-team! cfg params {::db/return-keys false})))))))) + (teams/add-profile-to-team! cfg params)))))))) diff --git a/backend/src/app/tasks/delete_object.clj b/backend/src/app/tasks/delete_object.clj index fad5b54a36..abdc4bc5f3 100644 --- a/backend/src/app/tasks/delete_object.clj +++ b/backend/src/app/tasks/delete_object.clj @@ -11,6 +11,7 @@ [app.common.time :as ct] [app.db :as db] [app.db.sql :as-alias sql] + [app.loggers.audit :as audit] [app.rpc.commands.files :as files] [app.rpc.commands.profile :as profile] [app.storage :as sto] @@ -18,6 +19,29 @@ (def ^:dynamic *team-deletion* false) +(def ^:dynamic *cascade* + "Accumulates the ids marked deleted by the current cascade execution, + grouped by entity. Bound by the task entry point; nil on direct calls + (e.g. objects-gc, whose purges are announced by their own procs)." + nil) + +(defn- record + [kind id] + (when (some? *cascade*) + (swap! *cascade* update kind conj id))) + +(defn- top-exists? + "The cascade marks blindly, so an unknown top id would report phantom + rows. Every queue submission and SREPL call names a real row; only + check, never change behavior: a missing row just skips the run." + [{:keys [::db/conn]} {:keys [object id]}] + (let [object (when (some? object) (keyword (name object)))] + (if (contains? #{:team :project :file :profile} object) + (some? (db/get* conn object {:id id} + {::db/remove-deleted false + ::sql/columns [:id]})) + true))) + (defmulti delete-object (fn [_ props] (:object props))) @@ -88,7 +112,9 @@ (db/update! conn :file-tagged-object-thumbnail {:deleted-at deleted-at} {:file-id id} - {::db/return-keys false}))) + {::db/return-keys false}) + + (record :files id))) (defmethod delete-object :project [{:keys [::db/conn] :as cfg} {:keys [id deleted-at]}] @@ -105,7 +131,9 @@ {::db/columns [:id :deleted-at]})] (delete-object cfg (assoc file :object :file - :deleted-at deleted-at)))) + :deleted-at deleted-at))) + + (record :projects id)) (defmethod delete-object :team [{:keys [::db/conn] :as cfg} {:keys [id deleted-at]}] @@ -127,7 +155,9 @@ {::db/columns [:id :deleted-at]})] (delete-object cfg (assoc project :object :project - :deleted-at deleted-at))))) + :deleted-at deleted-at)))) + + (record :teams id)) (defmethod delete-object :profile [{:keys [::db/conn] :as cfg} {:keys [id deleted-at]}] @@ -156,4 +186,19 @@ (defmethod ig/init-key ::handler [_ cfg] (fn [{:keys [props] :as task}] - (db/tx-run! cfg delete-object props))) + (db/tx-run! cfg + (fn [cfg] + (when (top-exists? cfg props) + (binding [*cascade* (atom {:teams [] :projects [] :files []})] + (delete-object cfg props) + (let [{:keys [teams projects files]} @*cascade*] + (when (or (seq teams) (seq projects) (seq files)) + (audit/submit cfg {:name "delete-object-with-cascade" + :type "action" + :props {:object (:object props) + :id (:id props) + :deleted-at (:deleted-at props) + :cascade {:team-ids teams + :project-ids projects + :file-ids files}} + :context {:triggered-by "delete-object"}}))))))))) diff --git a/backend/src/app/tasks/objects_gc.clj b/backend/src/app/tasks/objects_gc.clj index 7ddc210291..1d6f842e30 100644 --- a/backend/src/app/tasks/objects_gc.clj +++ b/backend/src/app/tasks/objects_gc.clj @@ -10,8 +10,10 @@ (:require [app.common.logging :as l] [app.common.time :as ct] + [app.config :as cf] [app.db :as db] [app.features.fdata :as fdata] + [app.loggers.audit :as audit] [app.storage :as sto] [app.tasks.delete-object :as dobj] [integrant.core :as ig])) @@ -27,6 +29,22 @@ ;; reduction is running, so the key is read inside the reducing function: ;; buffering the row itself yields nil columns. +(defn- submit-hard-delete-event + "Emits one hard-delete audit event for the batch of rows just purged + by this task, so a projection of the audit stream can drop its local + rows. Each GC section purges in its own chunk transactions, so each + chunk carries only its own entity key (`:team-ids`, `:project-ids` + or `:file-ids`); a single event spanning sections could never share + their transactions. Runs inside the caller transaction, next to the + deletes: if the purge aborts, the event aborts with it." + [cfg ids-key ids] + (when (contains? cf/flags :audit-log) + (audit/submit cfg + {:name "hard-delete-objects" + :type "command" + :props {ids-key (vec ids)} + :context {:triggered-by "objects-gc"}}))) + (def ^:private sql:get-upload-sessions "SELECT us.id FROM upload_session AS us @@ -179,23 +197,28 @@ [cfg ids] (try (db/tx-run! cfg - (fn [{:keys [::db/conn ::timestamp ::sto/storage]}] + (fn [{:keys [::db/conn ::timestamp ::sto/storage] :as cfg}] (db/exec-one! conn ["SET LOCAL rules.deletion_protection TO off"]) - (let [ids (db/create-array conn "uuid" ids)] - (->> (db/exec! conn [sql:lock-teams ids timestamp]) - (reduce (fn [total {:keys [id photo-id deleted-at]}] - (l/trc :obj "team" - :id (str id) - :deleted-at (ct/format-inst deleted-at)) + (let [ids (db/create-array conn "uuid" ids) + [total deleted-ids] + (->> (db/exec! conn [sql:lock-teams ids timestamp]) + (reduce (fn [[total deleted-ids] {:keys [id photo-id deleted-at]}] + (l/trc :obj "team" + :id (str id) + :deleted-at (ct/format-inst deleted-at)) - ;; Mark as deleted the storage object - (some->> photo-id (sto/touch-object! storage)) + ;; Mark as deleted the storage object + (some->> photo-id (sto/touch-object! storage)) - ;; And finally, permanently delete the team. - (let [affected (-> (db/delete! conn :team {:id id}) - (db/get-update-count))] - (+ total affected))) - 0))))) + ;; And finally, permanently delete the team. + (let [affected (-> (db/delete! conn :team {:id id}) + (db/get-update-count))] + [(+ total affected) + (cond-> deleted-ids (pos? affected) (conj id))])) + [0 []]))] + (when (seq deleted-ids) + (submit-hard-delete-event cfg :team-ids deleted-ids)) + total))) (catch Throwable cause (l/err :obj "team" :hint "error on chunk deletion, skipping it" @@ -287,20 +310,25 @@ [cfg ids] (try (db/tx-run! cfg - (fn [{:keys [::db/conn ::timestamp]}] + (fn [{:keys [::db/conn ::timestamp] :as cfg}] (db/exec-one! conn ["SET LOCAL rules.deletion_protection TO off"]) - (let [ids (db/create-array conn "uuid" ids)] - (->> (db/exec! conn [sql:lock-projects ids timestamp]) - (reduce (fn [total {:keys [id team-id deleted-at]}] - (l/trc :obj "project" - :id (str id) - :team-id (str team-id) - :deleted-at (ct/format-inst deleted-at)) + (let [ids (db/create-array conn "uuid" ids) + [total deleted-ids] + (->> (db/exec! conn [sql:lock-projects ids timestamp]) + (reduce (fn [[total deleted-ids] {:keys [id team-id deleted-at]}] + (l/trc :obj "project" + :id (str id) + :team-id (str team-id) + :deleted-at (ct/format-inst deleted-at)) - (let [affected (-> (db/delete! conn :project {:id id}) - (db/get-update-count))] - (+ total affected))) - 0))))) + (let [affected (-> (db/delete! conn :project {:id id}) + (db/get-update-count))] + [(+ total affected) + (cond-> deleted-ids (pos? affected) (conj id))])) + [0 []]))] + (when (seq deleted-ids) + (submit-hard-delete-event cfg :project-ids deleted-ids)) + total))) (catch Throwable cause (l/err :obj "project" :hint "error on chunk deletion, skipping it" @@ -336,20 +364,25 @@ [cfg ids] (try (db/tx-run! cfg - (fn [{:keys [::db/conn ::timestamp]}] + (fn [{:keys [::db/conn ::timestamp] :as cfg}] (db/exec-one! conn ["SET LOCAL rules.deletion_protection TO off"]) - (let [ids (db/create-array conn "uuid" ids)] - (->> (db/exec! conn [sql:lock-files ids timestamp]) - (reduce (fn [total {:keys [id deleted-at project-id]}] - (l/trc :obj "file" - :id (str id) - :project-id (str project-id) - :deleted-at (ct/format-inst deleted-at)) + (let [ids (db/create-array conn "uuid" ids) + [total deleted-ids] + (->> (db/exec! conn [sql:lock-files ids timestamp]) + (reduce (fn [[total deleted-ids] {:keys [id deleted-at project-id]}] + (l/trc :obj "file" + :id (str id) + :project-id (str project-id) + :deleted-at (ct/format-inst deleted-at)) - (let [affected (-> (db/delete! conn :file {:id id}) - (db/get-update-count))] - (+ total affected))) - 0))))) + (let [affected (-> (db/delete! conn :file {:id id}) + (db/get-update-count))] + [(+ total affected) + (cond-> deleted-ids (pos? affected) (conj id))])) + [0 []]))] + (when (seq deleted-ids) + (submit-hard-delete-event cfg :file-ids deleted-ids)) + total))) (catch Throwable cause (l/err :obj "file" :hint "error on chunk deletion, skipping it" diff --git a/backend/test/backend_tests/binfile_svg_test.clj b/backend/test/backend_tests/binfile_svg_test.clj index a12d30e895..92fe3f2a0e 100644 --- a/backend/test/backend_tests/binfile_svg_test.clj +++ b/backend/test/backend_tests/binfile_svg_test.clj @@ -374,13 +374,14 @@ of its single media object." [profile file] (let [bundle (export-v1 profile file)] - (let [result (-> th/*system* - (assoc ::bfc/project-id (:default-project-id profile)) - (assoc ::bfc/profile-id (:id profile)) - (assoc ::bfc/input (io/input-stream bundle)) - (v1/import-files!))] - (t/is (= 1 (count result))) - (let [rows (th/db-query :file-media-object {:file-id (first result)}) + (let [result (-> th/*system* + (assoc ::bfc/project-id (:default-project-id profile)) + (assoc ::bfc/profile-id (:id profile)) + (assoc ::bfc/input (io/input-stream bundle)) + (v1/import-files!)) + file-id (first (:file-ids result))] + (t/is (= 1 (count (:file-ids result)))) + (let [rows (th/db-query :file-media-object {:file-id file-id}) storage (:app.storage/storage th/*system*)] (t/is (= 1 (count rows))) (let [object (sto/get-object storage (:media-id (first rows)))] diff --git a/backend/test/backend_tests/delete_object_test.clj b/backend/test/backend_tests/delete_object_test.clj new file mode 100644 index 0000000000..878bea305d --- /dev/null +++ b/backend/test/backend_tests/delete_object_test.clj @@ -0,0 +1,107 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS SUBSIDIARY SL + +(ns backend-tests.delete-object-test + (:require + [app.common.time :as ct] + [app.common.uuid :as uuid] + [app.config :as cf] + [app.db :as db] + [backend-tests.helpers :as th] + [clojure.test :as t] + [mockery.core :refer [with-mocks]])) + +(t/use-fixtures :once th/state-init) +(t/use-fixtures :each th/database-reset) + +(defn- submitted-events + [audit-mock] + (->> (:call-args-list @audit-mock) + (map second))) + +(defn- cascade-events + [audit-mock] + (filter #(= "delete-object-with-cascade" (:name %)) + (submitted-events audit-mock))) + +(t/deftest delete-object-team-cascade-emits-event + ;; The cascade marks the team, its projects and their files; the event + ;; carries every affected id so a projection can follow without reading + ;; production tables. + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (with-redefs [cf/flags (conj cf/flags :audit-log)] + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 91 {:profile-id (:id profile)}) + project (th/create-project* 91 {:profile-id (:id profile) + :team-id (:id team)}) + file (th/create-file* 91 {:profile-id (:id profile) + :project-id (:id project)}) + deleted-at (ct/now)] + (th/run-task! :delete-object {:object :team + :id (:id team) + :deleted-at deleted-at}) + (t/is (some? (:deleted-at (th/db-get :team {:id (:id team)} + {::db/remove-deleted false})))) + (t/is (some? (:deleted-at (th/db-get :project {:id (:id project)} + {::db/remove-deleted false})))) + (t/is (some? (:deleted-at (th/db-get :file {:id (:id file)} + {::db/remove-deleted false})))) + (let [events (cascade-events audit-mock)] + (t/is (= 1 (count events))) + (t/is (= "action" (:type (first events)))) + (let [props (:props (first events))] + (t/is (= "team" (name (:object props)))) + (t/is (= (:id team) (:id props))) + (t/is (= deleted-at (:deleted-at props))) + (t/is (not (contains? props :team-ids))) + (t/is (not (contains? props :project-ids))) + (t/is (not (contains? props :file-ids))) + (t/is (= #{(:id team)} (set (get-in props [:cascade :team-ids])))) + (t/is (= #{(:default-project-id team) (:id project)} + (set (get-in props [:cascade :project-ids])))) + (t/is (= #{(:id file)} (set (get-in props [:cascade :file-ids])))))))))) + +(t/deftest delete-object-file-cascade-emits-event + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (with-redefs [cf/flags (conj cf/flags :audit-log)] + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 92 {:profile-id (:id profile)}) + project (th/create-project* 92 {:profile-id (:id profile) + :team-id (:id team)}) + file (th/create-file* 92 {:profile-id (:id profile) + :project-id (:id project)}) + deleted-at (ct/now)] + (th/run-task! :delete-object {:object :file + :id (:id file) + :deleted-at deleted-at}) + (let [events (cascade-events audit-mock)] + (t/is (= 1 (count events))) + (t/is (= "action" (:type (first events)))) + (let [props (:props (first events))] + (t/is (= "file" (name (:object props)))) + (t/is (= (:id file) (:id props))) + (t/is (= [] (get-in props [:cascade :team-ids]))) + (t/is (= [] (get-in props [:cascade :project-ids]))) + (t/is (= [(:id file)] (get-in props [:cascade :file-ids]))))))))) + +(t/deftest delete-object-missing-row-emits-nothing + ;; A cascade over an unknown id marks nothing, so there is nothing to report. + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (with-redefs [cf/flags (conj cf/flags :audit-log)] + (th/run-task! :delete-object {:object :team + :id (uuid/next) + :deleted-at (ct/now)}) + (t/is (empty? (:call-args-list @audit-mock)))))) + +(t/deftest delete-object-snapshot-emits-nothing + ;; Snapshots have no projection table; their cascade stays silent. + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (with-redefs [cf/flags (conj cf/flags :audit-log)] + (th/run-task! :delete-object {:object :snapshot + :id (uuid/next) + :file-id (uuid/next) + :deleted-at (ct/now)}) + (t/is (empty? (:call-args-list @audit-mock)))))) diff --git a/backend/test/backend_tests/helpers.clj b/backend/test/backend_tests/helpers.clj index 2718e48615..0335369709 100644 --- a/backend/test/backend_tests/helpers.clj +++ b/backend/test/backend_tests/helpers.clj @@ -483,6 +483,20 @@ (let [params (prepare-rpc-params data)] (try-on! (method-fn params))))) +(defn command-raw! + "Like `command!`, but returns the raw result without unwrapping it, so a test + can read the result metadata (for example the audit props a command sets on a + nil result). The HTTP layer unwraps it the same way in production." + [{:keys [::type] :as data}] + (let [[_ method-fn] (get-in *system* [:app.rpc/methods type])] + (when-not method-fn + (ex/raise :type :assertion + :code :rpc-method-not-found + :hint (str/ffmt "rpc method '%' not found" (name type)))) + + (let [params (prepare-rpc-params data)] + (method-fn params)))) + (defn management-command! [{:keys [::type] :as data}] (let [[_ method-fn] (get-in *system* [:app.rpc/management-methods type])] diff --git a/backend/test/backend_tests/loggers_webhooks_test.clj b/backend/test/backend_tests/loggers_webhooks_test.clj index ece038fef7..5bad72051b 100644 --- a/backend/test/backend_tests/loggers_webhooks_test.clj +++ b/backend/test/backend_tests/loggers_webhooks_test.clj @@ -6,9 +6,12 @@ (ns backend-tests.loggers-webhooks-test (:require + [app.common.time :as ct] [app.common.uuid :as uuid] + [app.config :as cf] [app.db :as db] [app.http :as http] + [app.loggers.audit :as audit] [app.storage :as sto] [backend-tests.helpers :as th] [clojure.test :as t] @@ -17,6 +20,12 @@ (t/use-fixtures :once th/state-init) (t/use-fixtures :each th/database-reset) +(defn decode-row + [{:keys [props context] :as row}] + (cond-> row + (db/pgobject? props) (assoc :props (db/decode-transit-pgobject props)) + (db/pgobject? context) (assoc :context (db/decode-transit-pgobject context)))) + (t/deftest process-event-handler-with-no-webhooks (with-mocks [submit-mock {:target 'app.worker/submit! :return nil}] (let [prof (th/create-profile* 1 {:is-active true}) @@ -40,6 +49,37 @@ (t/is (= 1 (:call-count @submit-mock))) (t/is (nil? res))))) +(t/deftest process-event-handler-persists-origin-event-id + ;; End-to-end regression test for the `webhook/trigger` row: its props + ;; `:event-id` must hold the origin event row id. It used to be nil + ;; because `process-event` never forwarded the id to the worker task. + (with-mocks [submit-mock {:target 'app.worker/submit! :return nil}] + (with-redefs [cf/flags #{:audit-log :webhooks}] + (let [prof (th/create-profile* 1 {:is-active true}) + event {:type "action" + :name "delete-project" + :profile-id (:id prof) + :props {:team-id (:default-team-id prof)} + :context {:version "2.0.0" :initiator "app"} + :tracked-at (ct/now) + :created-at (ct/now) + :source "backend" + :app.loggers.webhooks/event? true}] + (audit/submit* th/*system* event) + (let [params (-> @submit-mock :call-args-list ffirst :app.worker/params)] + (t/is (some? (:id params))) + ;; Run the handler the way the worker would. + (th/run-task! :process-webhook-event params) + (let [[origin trigger :as rows] (->> (th/db-exec! ["select * from audit_log order by created_at"]) + (mapv decode-row))] + (t/is (= 2 (count rows))) + (t/is (= "delete-project" (:name origin))) + (t/is (= "webhook" (:name trigger))) + (t/is (= "trigger" (:type trigger))) + (t/is (= "delete-project" (get-in trigger [:props :name]))) + (t/is (= (:id origin) (get-in trigger [:props :event-id]))) + (t/is (= 0 (get-in trigger [:props :total-affected]))))))))) + (t/deftest run-webhook-handler-1 (with-mocks [http-mock {:target 'app.http.client/req :return {:status 200}}] (let [prof (th/create-profile* 1 {:is-active true}) diff --git a/backend/test/backend_tests/objects_gc_test.clj b/backend/test/backend_tests/objects_gc_test.clj index f02a437f4f..5a3052901f 100644 --- a/backend/test/backend_tests/objects_gc_test.clj +++ b/backend/test/backend_tests/objects_gc_test.clj @@ -8,11 +8,14 @@ (:require [app.common.logging :as l] [app.common.time :as ct] + [app.config :as cf] [app.db :as db] + [app.rpc :as-alias rpc] [app.storage :as sto] [app.tasks.objects-gc :as ogc] [backend-tests.helpers :as th] - [clojure.test :as t])) + [clojure.test :as t] + [mockery.core :refer [with-mocks]])) (t/use-fixtures :once th/state-init) (t/use-fixtures :each th/database-reset) @@ -123,3 +126,119 @@ (t/testing "the run finishes" (t/is (pos? (:processed result)))))) + +(defn- submitted-events + [audit-mock] + (->> (:call-args-list @audit-mock) + (map second))) + +(defn- events-named + [audit-mock event-name] + (filter #(= event-name (:name %)) + (submitted-events audit-mock))) + +(t/deftest gc-emits-hard-delete-objects-for-teams + ;; When the GC physically deletes a team, the projection must drop its row, + ;; so the event carries the purged ids. + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (with-redefs [cf/flags (conj cf/flags :audit-log)] + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 71 {:profile-id (:id profile)})] + (th/command! {::th/type :delete-team + ::rpc/profile-id (:id profile) + :id (:id team)}) + ;; Run the queued cascade so the default project is marked deleted too, + ;; then purge everything the way production does. + (th/run-pending-tasks!) + (th/reset-mock! audit-mock) + (th/run-task! :objects-gc {:skip-delay true}) + (t/is (nil? (th/db-get :team {:id (:id team)} {::db/remove-deleted false}))) + (let [events (filter #(contains? (:props %) :team-ids) + (events-named audit-mock "hard-delete-objects"))] + (t/is (= 1 (count events))) + (let [event (first events)] + (t/is (= "command" (:type event))) + (t/is (= [(:id team)] (get-in event [:props :team-ids]))) + (t/is (= "objects-gc" (get-in event [:context :triggered-by]))))))))) + +(t/deftest gc-emits-hard-delete-objects-for-projects + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (with-redefs [cf/flags (conj cf/flags :audit-log)] + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 72 {:profile-id (:id profile)}) + project (th/create-project* 72 {:profile-id (:id profile) + :team-id (:id team)})] + (th/command! {::th/type :delete-project + ::rpc/profile-id (:id profile) + :id (:id project)}) + (th/run-pending-tasks!) + (th/reset-mock! audit-mock) + (th/run-task! :objects-gc {:skip-delay true}) + (t/is (nil? (th/db-get :project {:id (:id project)} {::db/remove-deleted false}))) + (let [events (filter #(contains? (:props %) :project-ids) + (events-named audit-mock "hard-delete-objects"))] + (t/is (= 1 (count events))) + (let [event (first events)] + (t/is (= "command" (:type event))) + (t/is (= [(:id project)] (get-in event [:props :project-ids]))) + (t/is (= "objects-gc" (get-in event [:context :triggered-by]))))))))) + +(t/deftest gc-emits-hard-delete-objects-for-files + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (with-redefs [cf/flags (conj cf/flags :audit-log)] + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 73 {:profile-id (:id profile)}) + project (th/create-project* 73 {:profile-id (:id profile) + :team-id (:id team)}) + file (th/create-file* 73 {:profile-id (:id profile) + :project-id (:id project)})] + (th/command! {::th/type :delete-file + ::rpc/profile-id (:id profile) + :id (:id file)}) + (th/run-pending-tasks!) + (th/reset-mock! audit-mock) + (th/run-task! :objects-gc {:skip-delay true}) + (t/is (nil? (th/db-get :file {:id (:id file)} {::db/remove-deleted false}))) + (let [events (filter #(contains? (:props %) :file-ids) + (events-named audit-mock "hard-delete-objects"))] + (t/is (= 1 (count events))) + (let [event (first events)] + (t/is (= "command" (:type event))) + (t/is (= [(:id file)] (get-in event [:props :file-ids]))) + (t/is (= "objects-gc" (get-in event [:context :triggered-by]))))))))) + +(t/deftest gc-emits-no-event-without-audit-log-flag + ;; The GC must stay silent when the audit log is off, even though the row is + ;; still purged. + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 74 {:profile-id (:id profile)})] + (th/command! {::th/type :delete-team + ::rpc/profile-id (:id profile) + :id (:id team)}) + (th/run-pending-tasks!) + (th/reset-mock! audit-mock) + (th/run-task! :objects-gc {:skip-delay true}) + (t/is (nil? (th/db-get :team {:id (:id team)} {::db/remove-deleted false}))) + (t/is (empty? (:call-args-list @audit-mock)))))) + +(t/deftest gc-emits-events-only-for-committed-chunks + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (with-redefs [cf/flags (conj cf/flags :audit-log)] + (let [profile (th/create-profile* 1) + rolled-back (create-deleted-file! profile 1 (ct/in-past "30m")) + blocked (create-deleted-file! profile 2 (ct/in-past "20m")) + clean (create-deleted-file! profile 3 (ct/in-past "10m"))] + (block-file! blocked) + (th/reset-mock! audit-mock) + (let [result (th/run-task! :objects-gc {:chunk-size 2}) + events (filter #(contains? (:props %) :file-ids) + (events-named audit-mock "hard-delete-objects"))] + (t/testing "the failed chunk keeps both files" + (t/is (some? (find-file (:id rolled-back)))) + (t/is (some? (find-file (:id blocked))))) + (t/testing "the next chunk commits and reports only its deleted file" + (t/is (nil? (find-file (:id clean)))) + (t/is (pos? (:processed result))) + (t/is (= 1 (count events))) + (t/is (= [(:id clean)] (get-in (first events) [:props :file-ids]))))))))) diff --git a/backend/test/backend_tests/rpc_audit_test.clj b/backend/test/backend_tests/rpc_audit_test.clj index 50ee79f2e8..81ca636dbe 100644 --- a/backend/test/backend_tests/rpc_audit_test.clj +++ b/backend/test/backend_tests/rpc_audit_test.clj @@ -17,6 +17,7 @@ [app.util.services :as sv] [backend-tests.helpers :as th] [clojure.test :as t] + [mockery.core :refer [with-mocks]] [yetti.request])) (t/use-fixtures :once th/state-init) @@ -329,6 +330,58 @@ (t/is (= day-now (:created-at row2))) (t/is (= day-now (:tracked-at row2)))))))) +(t/deftest backend-process-event-forwards-id-to-webhook-task + ;; The `webhook/trigger` row links back to its origin event through + ;; props `:event-id`, so `process-event` must forward the stored row + ;; id to the `:process-webhook-event` worker task. Regression test: + ;; the id used to be generated inside `append-audit-entry` without + ;; ever reaching the worker params, so `:event-id` always landed nil. + (with-mocks [submit-mock {:target 'app.worker/submit! :return nil}] + (with-redefs [cf/flags #{:audit-log :webhooks}] + (let [prof (th/create-profile* 1 {:is-active true}) + event {:type "action" + :name "delete-project" + :profile-id (:id prof) + :props {:team-id (:default-team-id prof)} + :context {:version "2.0.0" :initiator "app"} + :tracked-at (ct/now) + :created-at (ct/now) + :source "backend" + :app.loggers.webhooks/event? true}] + (audit/submit* th/*system* event) + (let [[row :as rows] (->> (th/db-exec! ["select * from audit_log"]) + (mapv decode-row))] + (t/is (= 1 (count rows))) + (t/is (= 1 (:call-count @submit-mock))) + (let [params (-> @submit-mock :call-args-list ffirst :app.worker/params)] + (t/is (some? (:id params))) + (t/is (= (:id row) (:id params))))))))) + +(t/deftest backend-process-event-telemetry-row-gets-own-id + ;; When both `:audit-log` and `:telemetry` are active, the shadow + ;; telemetry row must not reuse the full row id (shared PK), while + ;; the webhook task still carries the full row id. + (with-mocks [submit-mock {:target 'app.worker/submit! :return nil}] + (with-redefs [cf/flags #{:audit-log :telemetry :webhooks}] + (let [prof (th/create-profile* 1 {:is-active true}) + event {:type "action" + :name "delete-project" + :profile-id (:id prof) + :props {:team-id (:default-team-id prof)} + :context {:version "2.0.0" :initiator "app"} + :tracked-at (ct/now) + :created-at (ct/now) + :source "backend" + :app.loggers.webhooks/event? true}] + (audit/submit* th/*system* event) + (let [[row1 row2 :as rows] (->> (th/db-exec! ["select * from audit_log order by source"]) + (mapv decode-row))] + (t/is (= 2 (count rows))) + (t/is (not= (:id row1) (:id row2))) + (let [params (-> @submit-mock :call-args-list ffirst :app.worker/params)] + (t/is (= (:id (first (filter #(= "backend" (:source %)) rows))) + (:id params))))))))) + (t/deftest push-events-disabled-when-no-flags-and-no-telemetry ;; When neither audit-log nor telemetry is enabled, no rows should ;; be stored. @@ -651,17 +704,22 @@ (defn- prepare-event "Call prepare-rpc-event with a bare request, returning the built event." - [caller result] - (let [mdata {::sv/name "test-cmd"} - params {::rpc/profile-id caller - ::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})] - (audit/prepare-rpc-event th/*system* mdata params result))) + ([caller result] + (prepare-event caller "test-cmd" result)) + ([caller cmd-name result] + (let [mdata {::sv/name cmd-name} + params {::rpc/profile-id caller + ::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})] + ;; Production always resolves commands with `::module` set ("main" or + ;; "management"): the event name (and the read exclusion) depends on it. + (audit/prepare-rpc-event (assoc th/*system* ::rpc/module "main") + mdata params result)))) (t/deftest prepare-rpc-event-ignores-profile-id-from-result ;; An audit event belongs to the caller, never to whatever `:profile-id` @@ -696,6 +754,76 @@ (t/is (= uuid/zero (:profile-id (prepare-event nil {:some-data "value"}))))) +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; PREPARE-RPC-EVENT TEAM-ID ENRICHMENT +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; + +(t/deftest prepare-rpc-event-enriches-team-id-from-project-id + ;; An event that carries a project id resolves its team locally, so the + ;; downstream consumers do not need to join production tables. + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 10 {:profile-id (:id profile)}) + project (th/create-project* 10 {:profile-id (:id profile) + :team-id (:id team)}) + result (with-meta {} {::audit/props {:project-id (:id project)}}) + event (prepare-event (:id profile) result)] + (t/is (= (:id team) (get-in event [:props :team-id]))))) + +(t/deftest prepare-rpc-event-enriches-team-id-from-file-id + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 11 {:profile-id (:id profile)}) + project (th/create-project* 11 {:profile-id (:id profile) + :team-id (:id team)}) + file (th/create-file* 11 {:profile-id (:id profile) + :project-id (:id project)}) + result (with-meta {} {::audit/props {:file-id (:id file)}}) + event (prepare-event (:id profile) result)] + (t/is (= (:id team) (get-in event [:props :team-id]))))) + +(t/deftest prepare-rpc-event-keeps-existing-team-id + ;; The resolver never overwrites a team id the command already set. + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 12 {:profile-id (:id profile)}) + project (th/create-project* 12 {:profile-id (:id profile) + :team-id (:id team)}) + result (with-meta {} {::audit/props {:team-id (:id team) + :project-id (:id project)}}) + event (prepare-event (:id profile) result)] + (t/is (= (:id team) (get-in event [:props :team-id]))))) + +(t/deftest prepare-rpc-event-does-not-resolve-without-project-or-file + ;; Read commands put the entity id under `:id`; enriching those would add a + ;; query to every read event, so the resolver must leave them untouched. + (let [profile (th/create-profile* 1 {:is-active true}) + result (with-meta {} {::audit/props {:id (uuid/next)}}) + event (prepare-event (:id profile) result)] + (t/is (not (contains? (:props event) :team-id))))) + +(t/deftest prepare-rpc-event-resolves-default-team-id + ;; The resolver adds the team id whenever it can resolve it, including the + ;; personal (default) team. + (let [profile (th/create-profile* 1 {:is-active true}) + result (with-meta {} {::audit/props {:project-id (:default-project-id profile)}}) + event (prepare-event (:id profile) result)] + (t/is (= (:default-team-id profile) (get-in event [:props :team-id]))))) + +(t/deftest prepare-rpc-event-skips-team-id-resolution-for-reads + ;; Read events carry no consumer state: resolving their team would add a + ;; query to every read, so `get-*` names (plus the known non-get reads) + ;; are left untouched even when they carry a project or file id. + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 13 {:profile-id (:id profile)}) + project (th/create-project* 13 {:profile-id (:id profile) + :team-id (:id team)}) + file (th/create-file* 13 {:profile-id (:id profile) + :project-id (:id project)})] + (doseq [cmd-name ["get-file-fragment" "get-comments" "has-file-libraries"] + props [{:file-id (:id file)} {:project-id (:id project)}]] + (let [result (with-meta {} {::audit/props props}) + event (prepare-event (:id profile) cmd-name result)] + (t/is (not (contains? (:props event) :team-id)) + (str cmd-name " with " (keys props) " must not resolve team-id")))))) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; PREPARE-RPC-EVENT PROFILE-ID COERCION ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/backend/test/backend_tests/rpc_binfile_test.clj b/backend/test/backend_tests/rpc_binfile_test.clj index b1a144ab70..ccdcc7fafa 100644 --- a/backend/test/backend_tests/rpc_binfile_test.clj +++ b/backend/test/backend_tests/rpc_binfile_test.clj @@ -6,13 +6,17 @@ (ns backend-tests.rpc-binfile-test (:require + [app.binfile.v3 :as bf.v3] [app.common.schema :as sm] [app.common.uuid :as uuid] + [app.config :as cf] + [app.db :as db] [app.rpc :as-alias rpc] [app.rpc.commands.binfile :as binfile] [backend-tests.helpers :as th] [clojure.test :as t] - [datoteka.fs :as fs])) + [datoteka.fs :as fs] + [mockery.core :refer [with-mocks]])) (t/use-fixtures :once th/state-init) (t/use-fixtures :each th/database-reset) @@ -72,3 +76,38 @@ ;; Version 4 should be rejected (t/is (false? (validator (assoc base-params :version 4))) "version 4 should be rejected"))) + +(t/deftest import-binfile-audit-events-carry-entity-ids + ;; The imported files are created inside the importer, so the command emits a + ;; create-file event per file for a local projection. + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (with-redefs [cf/flags (conj cf/flags :audit-log)] + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 51 {:profile-id (:id profile)}) + project (th/create-project* 51 {:profile-id (:id profile) + :team-id (:id team)}) + ;; A real row: `submit-file-created-event` reads the light + ;; columns back, so a phantom id would emit no event. + file (th/create-file* 99 {:profile-id (:id profile) + :project-id (:id project)}) + file-id (:id file)] + (with-redefs [bf.v3/import-files! (fn [_cfg] {:file-ids [file-id]})] + (#'binfile/import-binfile + th/*system* + {::rpc/profile-id (:id profile) + :profile-id (:id profile) + :project-id (:id project) + :version 3 + :name "imported" + :file {:filename "package.penpot" + :path (th/tempfile "backend_tests/test_files/svg-attrs-camel-case.penpot") + :mtype "application/zip" + :size 1}})) + + (let [events (->> (:call-args-list @audit-mock) + (map second) + (filter #(= "create-file" (:name %))))] + (t/is (= 1 (count events))) + (t/is (= file-id (get-in (first events) [:props :id]))) + (t/is (= (:id project) (get-in (first events) [:props :project-id]))) + (t/is (= (:id team) (get-in (first events) [:props :team-id])))))))) diff --git a/backend/test/backend_tests/rpc_comment_test.clj b/backend/test/backend_tests/rpc_comment_test.clj index ea64723545..0c58687751 100644 --- a/backend/test/backend_tests/rpc_comment_test.clj +++ b/backend/test/backend_tests/rpc_comment_test.clj @@ -478,3 +478,32 @@ :content "member comment" :frame-id uuid/zero})] (t/is (th/success? out)))))) + +(t/deftest comment-audit-events-carry-team-id + ;; Comment events carry the file id, so the resolver could find the team; the + ;; command already has the file with its team id and sets it without a join. + (with-mocks [mock {:target 'app.config/get + :return (th/config-get-mock + {:quotes-teams-per-profile 200})}] + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 1 {:profile-id (:id profile)}) + project (th/create-project* 1 {:team-id (:id team) + :profile-id (:id profile)}) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:id project)}) + page-id (get-in file [:data :pages 0]) + thread (th/command! {::th/type :create-comment-thread + ::rpc/profile-id (:id profile) + :file-id (:id file) + :page-id page-id + :position (gpt/point 0) + :content "hello" + :frame-id uuid/zero}) + comment (th/command! {::th/type :create-comment + ::rpc/profile-id (:id profile) + :thread-id (get-in thread [:result :id]) + :content "comment"})] + (t/is (= (:id team) + (get-in (meta (:result thread)) [:app.loggers.audit/props :team-id]))) + (t/is (= (:id team) + (get-in (meta (:result comment)) [:app.loggers.audit/props :team-id])))))) diff --git a/backend/test/backend_tests/rpc_file_security_test.clj b/backend/test/backend_tests/rpc_file_security_test.clj new file mode 100644 index 0000000000..15ac56c53d --- /dev/null +++ b/backend/test/backend_tests/rpc_file_security_test.clj @@ -0,0 +1,368 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC Sucursal en España SL + +(ns backend-tests.rpc-file-security-test + "Tests for the nexus file-security backend prerequisite: the + `get-file-for-analysis` management method, the semantic projection, + and the audit prop coverage the `:local` consumer schedules from." + (:require + [app.binfile.common :as bfc] + [app.common.features :as cfeat] + [app.common.time :as ct] + [app.common.types.shape :as cts] + [app.common.uuid :as uuid] + [app.loggers.audit :as audit] + [app.rpc :as-alias rpc] + [app.rpc.management.files :as mgmt.files] + [app.util.services :as sv] + [backend-tests.helpers :as th] + [clojure.test :as t] + [yetti.request])) + +(t/use-fixtures :once th/state-init) +(t/use-fixtures :each th/database-reset) + +;; --- Projection unit tests (pure, no database) + +(def ^:private page-id (uuid/random)) +(def ^:private frame-id (uuid/random)) +(def ^:private text-id (uuid/random)) +(def ^:private link-id (uuid/random)) + +(defn- test-data + [objects] + {:pages [page-id] + :pages-index {page-id {:id page-id + :name "Page 1" + :objects objects}}}) + +(t/deftest projection-empty-file + (let [projection (mgmt.files/file->projection (test-data {}))] + (t/is (= 1 (:version projection))) + (t/is (= [{:id (str page-id) :name "Page 1"}] (:pages projection))) + (t/is (= [] (:frames projection))) + (t/is (= {:items [] :truncated? false} (:texts projection))) + (t/is (= {:items [] :truncated? false} (:links projection))) + (t/is (= {:items [] :truncated? false} (:interactions projection))))) + +(t/deftest projection-picks-frames-text-links-and-interactions + (let [data (test-data + {frame-id {:id frame-id :name "Frame" :type :frame} + text-id {:id text-id :name "Text" :type :text + :content {:type "root" + :children [{:type "paragraph" + :children [{:text "hello"} + {:text " world"}]}]}} + link-id {:id link-id :name "Button" :type :rect + :interactions [{:event-type :click + :action-type :open-url + :url "https://phish.example/login"} + {:event-type :click + :action-type :navigate + :destination page-id} + ;; No event, cannot trigger: + ;; collected as link, never + ;; as interaction. + {:action-type :open-url + :url "https://quiet.example"}]}}) + projection (mgmt.files/file->projection data)] + (t/is (= [{:id (str frame-id) :page-id (str page-id) :name "Frame"}] + (:frames projection))) + (t/is (= {:items [{:shape-id (str text-id) + :page-id (str page-id) + :text "hello world"}] + :truncated? false} + (:texts projection))) + (t/is (= {:items [{:shape-id (str link-id) + :page-id (str page-id) + :url "https://phish.example/login"} + {:shape-id (str link-id) + :page-id (str page-id) + :url "https://quiet.example"}] + :truncated? false} + (:links projection))) + (t/is (= {:items [{:shape-id (str link-id) + :page-id (str page-id) + :event "click" + :action "open-url" + :target "https://phish.example/login"} + {:shape-id (str link-id) + :page-id (str page-id) + :event "click" + :action "navigate" + :target (str page-id)}] + :truncated? false} + (:interactions projection))))) + +(t/deftest projection-skips-blank-text-and-corrupt-content + (let [bad-id (uuid/random) + projection (mgmt.files/file->projection + (test-data + {text-id {:id text-id :type :text + :content {:type "root" + :children [{:type "paragraph" + :children [{:text " "}]}]}} + bad-id {:id bad-id :type :text + :content {:type "root" :children 42}}}))] + ;; Blank text contributes nothing; corrupt content skips the shape + ;; instead of failing the file. + (t/is (= {:items [] :truncated? false} (:texts projection))))) + +(t/deftest projection-truncates-links + (let [objects (into {} + (map (fn [i] + (let [id (uuid/random)] + [id {:id id :type :rect + :interactions [{:event-type :click + :action-type :open-url + :url (str "https://example.com/" i)}]}]))) + (range (inc mgmt.files/max-links))) + projection (mgmt.files/file->projection (test-data objects))] + (t/is (= mgmt.files/max-links (count (get-in projection [:links :items])))) + (t/is (true? (get-in projection [:links :truncated?]))) + (t/is (false? (get-in projection [:interactions :truncated?]))))) + +;; --- Management method (through the worker stack) + +(def text-content + {:type "root" + :children [{:type "paragraph-set" + :children [{:type "paragraph" + :children [{:text "cheap meds"}]}]}]}) + +(defn- create-file! + [profile-id project-id] + (let [out (th/command! {::th/type :create-file + ::rpc/profile-id profile-id + :project-id project-id + :name "Shop" + :is-shared false})] + (t/is (nil? (:error out))) + (get-in out [:result :id]))) + +(defn- file-page-id + [file-id] + (get-in (bfc/get-file th/*system* file-id) [:data :pages 0])) + +(defn- update-file! + [profile-id file-id page-id changes revn] + (let [out (th/command! {::th/type :update-file + ::rpc/profile-id profile-id + :id file-id + :session-id (uuid/random) + :revn revn + :vern 0 + :features cfeat/supported-features + :changes changes})] + (t/is (nil? (:error out))) + (:result out))) + +(defn- add-text-shape-change + [page-id shape-id] + [{:type :add-obj + :page-id page-id + :id shape-id + :parent-id uuid/zero + :frame-id uuid/zero + :components-v2 true + :obj (cts/setup-shape + {:id shape-id + :name "Buy now" + :frame-id uuid/zero + :parent-id uuid/zero + :type :text + :content text-content})}]) + +(t/deftest get-file-for-analysis-returns-projection + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 1 {:profile-id (:id profile)}) + project (th/create-project* 1 {:profile-id (:id profile) + :team-id (:id team)}) + file-id (create-file! (:id profile) (:id project)) + page-id (file-page-id file-id)] + ;; One edit so revn moves past zero and the file has content. + (update-file! (:id profile) file-id page-id + (add-text-shape-change page-id (uuid/random)) + 0) + (let [out (th/management-command! {::th/type :get-file-for-analysis + :file-id file-id})] + (t/is (nil? (:error out))) + (let [result (:result out)] + (t/is (= file-id (:id result))) + (t/is (= 1 (:revn result))) + (t/is (some? (:modified-at result))) + (t/is (= (:id project) (:project-id result))) + (t/is (= (:id team) (:team-id result))) + (t/is (false? (:has-share-link result))) + (t/is (= mgmt.files/projection-version (:projection-version result))) + (t/is (= "cheap meds" + (-> result :projection :texts :items first :text))))) + ;; A share link flips the share state, no re-edit needed. + (th/command! {::th/type :create-share-link + ::rpc/profile-id (:id profile) + :file-id file-id + :pages #{page-id} + :who-comment "all" + :who-inspect "all"}) + (let [out (th/management-command! {::th/type :get-file-for-analysis + :file-id file-id})] + (t/is (nil? (:error out))) + (t/is (true? (:has-share-link (:result out))))))) + +(t/deftest get-file-for-analysis-rejects-unknown-file + (let [out (th/management-command! {::th/type :get-file-for-analysis + :file-id (uuid/random)})] + (t/is (some? (:error out))) + (t/is (= :not-found (th/ex-type (:error out)))))) + +;; --- Audit prop coverage (through the commands, into real events) +;; +;; NOTE: the test system neutralizes `audit/submit`, so no audit row +;; is ever written. The observable contract of a command is the +;; metadata it sets on its result, which `prepare-rpc-event` turns +;; into the stored event: feed one into the other with the real +;; method metadata and assert the final props. + +(def mock-request + (reify + yetti.request/IRequest + (get-header [_ _] nil) + (remote-addr [_] "127.0.0.1"))) + +(defn- event-for + "Run a main RPC command and return the audit event it would emit." + [type params] + (let [[mdata _] (get-in th/*system* [:app.rpc/methods type]) + result (th/command-raw! (assoc params ::th/type type)) + params (-> params + (assoc ::rpc/request-id (uuid/next) + ::rpc/request-at (ct/now)) + (with-meta {:app.http/request mock-request})) + ;; In production the caller assocs the module before + ;; preparing the event; without it every name would come out + ;; with a leading dash. + cfg (assoc th/*system* ::rpc/module "main")] + (audit/prepare-rpc-event cfg mdata params result))) + +(t/deftest get-file-for-analysis-emits-audit-event + ;; Content reads stay traceable: with the skip gone, the call flows + ;; through `wrap-audit` like any other management method. `wrap-audit` + ;; itself is fixed at system init (no audit flags there), so assert the + ;; metadata flip plus the event shape through `prepare-rpc-event` with + ;; the real method metadata. + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 1 {:profile-id (:id profile)}) + project (th/create-project* 1 {:profile-id (:id profile) + :team-id (:id team)}) + file-id (create-file! (:id profile) (:id project)) + [mdata _] (get-in th/*system* [:app.rpc/management-methods :get-file-for-analysis])] + (t/is (not (::audit/skip mdata))) + (let [out (th/management-command! {::th/type :get-file-for-analysis + :file-id file-id}) + result (:result out) + params (-> {:file-id file-id} + (assoc ::rpc/request-id (uuid/next) + ::rpc/request-at (ct/now)) + (with-meta {:app.http/request mock-request})) + cfg (assoc th/*system* ::rpc/module "management") + event (audit/prepare-rpc-event cfg mdata params result)] + (t/is (nil? (:error out))) + (t/is (= "management-get-file-for-analysis" (:name event))) + (t/is (= file-id (get-in event [:props :file-id]))) + (t/is (= (:id team) (get-in event [:props :team-id])))))) + +(t/deftest file-events-carry-scheduling-props + (let [profile (th/create-profile* 2 {:is-active true}) + team (th/create-team* 2 {:profile-id (:id profile)}) + project (th/create-project* 2 {:profile-id (:id profile) + :team-id (:id team)}) + base {::rpc/profile-id (:id profile)}] + (t/testing "create-file" + (let [event (event-for :create-file + (assoc base + :project-id (:id project) + :name "Shop" + :is-shared false)) + props (:props event)] + (t/is (= "create-file" (:name event))) + (t/is (some? (:id props))) + (t/is (= "Shop" (:name props))) + (t/is (some? (:created-at props))) + (t/is (some? (:modified-at props))) + (t/is (= 0 (:revn props))))) + (t/testing "update-file" + ;; A file to edit, created through the command so the event + ;; below belongs to a real row. + (let [created (event-for :create-file + (assoc base + :project-id (:id project) + :name "Edit me" + :is-shared false)) + file-id (get-in created [:props :id]) + page-id (file-page-id file-id) + shape-id (uuid/random) + event (event-for :update-file + (assoc base + :id file-id + :session-id (uuid/random) + :revn 0 + :vern 0 + :features cfeat/supported-features + :changes [{:type :add-obj + :page-id page-id + :id shape-id + :parent-id uuid/zero + :frame-id uuid/zero + :components-v2 true + :obj (cts/setup-shape + {:id shape-id + :name "rect" + :frame-id uuid/zero + :parent-id uuid/zero + :type :rect})}])) + props (:props event)] + (t/is (= "update-file" (:name event))) + (t/is (= file-id (:id props))) + (t/is (= 1 (:revn props))) + (t/is (some? (:modified-at props))))) + (t/testing "share-link create and delete" + (let [created (event-for :create-file + (assoc base + :project-id (:id project) + :name "Shared" + :is-shared false)) + file-id (get-in created [:props :id]) + page-id (file-page-id file-id) + event (event-for :create-share-link + (assoc base + :file-id file-id + :pages #{page-id} + :who-comment "all" + :who-inspect "all")) + props (:props event) + slink-id (:share-id props)] + (t/is (= "create-share-link" (:name event))) + (t/is (= file-id (:file-id props))) + (t/is (some? slink-id)) + (let [event (event-for :delete-share-link + (assoc base :id slink-id)) + props (:props event)] + (t/is (= "delete-share-link" (:name event))) + (t/is (= file-id (:file-id props))) + (t/is (= slink-id (:share-id props)))))) + (t/testing "delete-file" + (let [created (event-for :create-file + (assoc base + :project-id (:id project) + :name "Doomed" + :is-shared false)) + file-id (get-in created [:props :id]) + event (event-for :delete-file + (assoc base :id file-id)) + props (:props event)] + (t/is (= "delete-file" (:name event))) + (t/is (= file-id (:id props))) + (t/is (some? (:deleted-at props))))))) diff --git a/backend/test/backend_tests/rpc_file_test.clj b/backend/test/backend_tests/rpc_file_test.clj index 2ac2bf8424..aadd976c2d 100644 --- a/backend/test/backend_tests/rpc_file_test.clj +++ b/backend/test/backend_tests/rpc_file_test.clj @@ -2782,3 +2782,36 @@ (t/is (th/ex-info? (:error out))) (t/is (th/ex-of-type? (:error out) :validation)) (t/is (th/ex-of-code? (:error out) :params-validation)))) + +(t/deftest delete-file-audit-event-carries-team-id + ;; The event already carries the project id, so the resolver could find the + ;; team; the command has the team row and sets the id without a lookup. + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 1 {:profile-id (:id profile)}) + project (th/create-project* 1 {:profile-id (:id profile) + :team-id (:id team)}) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:id project)}) + wrapped (th/command-raw! {::th/type :delete-file + ::rpc/profile-id (:id profile) + :id (:id file)})] + (t/is (= (:id team) + (get-in (meta wrapped) [:app.loggers.audit/props :team-id]))))) + +(t/deftest create-file-audit-event-carries-its-id + ;; The command returns the new file but did not emit its id, so a consumer + ;; could not key a local projection. + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 1 {:profile-id (:id profile)}) + project (th/create-project* 1 {:profile-id (:id profile) + :team-id (:id team)}) + out (th/command! {::th/type :create-file + ::rpc/profile-id (:id profile) + :project-id (:id project) + :name "foobar" + :is-shared false})] + (t/is (th/success? out)) + (t/is (= (get-in out [:result :id]) + (get-in (meta (:result out)) [:app.loggers.audit/props :id]))) + (t/is (= (:id team) + (get-in (meta (:result out)) [:app.loggers.audit/props :team-id]))))) diff --git a/backend/test/backend_tests/rpc_management_nitrate_test.clj b/backend/test/backend_tests/rpc_management_nitrate_test.clj index a754e45b1c..30ff6a3823 100644 --- a/backend/test/backend_tests/rpc_management_nitrate_test.clj +++ b/backend/test/backend_tests/rpc_management_nitrate_test.clj @@ -1916,6 +1916,9 @@ :type "action"}]} params (with-meta params {::http/request http-request}) + ;; The profile setup above emits membership events; drop them so + ;; only the pushed events are counted below. + _ (th/reset-mock! audit-mock) out (th/management-command! params)] (t/is (nil? (:error out))) (t/is (:called? @audit-mock)) @@ -1936,6 +1939,9 @@ :profile-id (:id prof)}]} params (with-meta params {::http/request http-request}) + ;; The profile setup above emits membership events; drop them so + ;; only the pushed events are asserted below. + _ (th/reset-mock! audit-mock) out (th/management-command! params)] (t/is (nil? (:error out))) (let [[_ event] (:call-args @audit-mock)] @@ -1955,6 +1961,9 @@ :type "action"}]} params (with-meta params {::http/request http-request}) + ;; The profile setup above emits membership events; drop them so + ;; only the pushed events are counted below. + _ (th/reset-mock! audit-mock) out (th/management-command! params)] (t/is (nil? (:error out))) (t/is (= 2 (:call-count @audit-mock))) @@ -1974,6 +1983,9 @@ :foo "bar"}}]} params (with-meta params {::http/request http-request}) + ;; The profile setup above emits membership events; drop them so + ;; only the pushed events are asserted below. + _ (th/reset-mock! audit-mock) out (th/management-command! params)] (t/is (nil? (:error out))) (let [[_ event] (:call-args @audit-mock)] @@ -1998,6 +2010,9 @@ params (with-meta params {::http/request (assoc http-request ::http/auth-key-id :admin-console)}) + ;; The profile setup above emits membership events; drop them so + ;; only the pushed events are asserted below. + _ (th/reset-mock! audit-mock) out (th/management-command! params)] (t/is (nil? (:error out))) (let [[_ event] (:call-args @audit-mock)] diff --git a/backend/test/backend_tests/rpc_management_test.clj b/backend/test/backend_tests/rpc_management_test.clj index 4b449af993..e1f6e81950 100644 --- a/backend/test/backend_tests/rpc_management_test.clj +++ b/backend/test/backend_tests/rpc_management_test.clj @@ -9,6 +9,7 @@ [app.binfile.common :as bfc] [app.common.features :as cfeat] [app.common.pprint :as pp] + [app.common.time :as ct] [app.common.types.shape :as cts] [app.common.uuid :as uuid] [app.config :as cf] @@ -22,7 +23,8 @@ [clojure.test :as t] [cuerdas.core :as str] [datoteka.fs :as fs] - [datoteka.io :as io])) + [datoteka.io :as io] + [mockery.core :refer [with-mocks]])) (t/use-fixtures :once th/state-init) (t/use-fixtures :each th/database-reset) @@ -666,3 +668,55 @@ (t/is (vector? result)) (t/is (= 1 (count result))) (t/is (= "test" (:id (first result))))))) + +(t/deftest clone-template-audit-events-carry-team-id + ;; The per-file `create-file` events are built by hand, so they do not pass + ;; through the generic resolver: the command must copy the team id itself. + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (with-redefs [cf/flags (conj cf/flags :audit-log)] + (let [prof (th/create-profile* 1 {:is-active true}) + team (th/create-team* 1 {:profile-id (:id prof)}) + project (th/create-project* 1 {:profile-id (:id prof) + :team-id (:id team)}) + out (th/command! {::th/type :clone-template + ::rpc/profile-id (:id prof) + :project-id (:id project) + :template-id "test"})] + (t/is (th/success? out)) + (th/consume-sse (:result out)) + (let [events (->> (:call-args-list @audit-mock) + (map second) + (filter #(= "create-file" (:name %))))] + (t/is (seq events)) + (t/is (every? #(= (:id team) (get-in % [:props :team-id])) events))))))) + +(t/deftest duplicate-project-audit-events-carry-entity-ids + ;; The duplicated project and files are created by internal calls, so the + ;; command emits one create event per entity for a local projection. + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (with-redefs [cf/flags (conj cf/flags :audit-log)] + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 52 {:profile-id (:id profile)}) + project (th/create-project* 52 {:profile-id (:id profile) + :team-id (:id team)}) + _file (th/create-file* 52 {:profile-id (:id profile) + :project-id (:id project)}) + out (th/command! {::th/type :duplicate-project + ::rpc/profile-id (:id profile) + :project-id (:id project) + :name "copy"}) + events (->> (:call-args-list @audit-mock) + (map second) + (filter #(contains? #{"create-project" "create-file"} (:name %))))] + (t/is (th/success? out)) + (t/is (some #(and (= "create-project" (:name %)) + (= (get-in out [:result :id]) (get-in % [:props :id])) + (= (:id team) (get-in % [:props :team-id])) + (= "copy" (get-in % [:props :name]))) + events)) + (t/is (some #(and (= "create-file" (:name %)) + (string? (get-in % [:props :name])) + (int? (get-in % [:props :revn])) + (ct/inst? (get-in % [:props :modified-at]))) + events)) + (t/is (every? #(= (:id team) (get-in % [:props :team-id])) events)))))) diff --git a/backend/test/backend_tests/rpc_media_test.clj b/backend/test/backend_tests/rpc_media_test.clj index fb34396a09..985fb68fc2 100644 --- a/backend/test/backend_tests/rpc_media_test.clj +++ b/backend/test/backend_tests/rpc_media_test.clj @@ -1226,3 +1226,27 @@ (t/is (th/ex-info? (:error out))) (t/is (th/ex-of-type? (:error out) :validation)) (t/is (th/ex-of-code? (:error out) :params-validation)))) + +(t/deftest media-audit-event-carries-team-id + ;; The media event carries the file id, so the resolver could find the team; + ;; the command already resolved the team for the quota check and sets it. + (let [prof (th/create-profile* 1) + team (th/create-team* 1 {:profile-id (:id prof)}) + project (th/create-project* 1 {:profile-id (:id prof) + :team-id (:id team)}) + file (th/create-file* 1 {:profile-id (:id prof) + :project-id (:id project) + :is-shared false}) + mfile {:filename "sample.jpg" + :path (th/tempfile "backend_tests/test_files/sample.jpg") + :mtype "image/jpeg" + :size 312043} + out (th/command! {::th/type :upload-file-media-object + ::rpc/profile-id (:id prof) + :file-id (:id file) + :is-local true + :name "testfile" + :content mfile})] + (t/is (th/success? out)) + (t/is (= (:id team) + (get-in (meta (:result out)) [:app.loggers.audit/replace-props :team-id]))))) diff --git a/backend/test/backend_tests/rpc_profile_test.clj b/backend/test/backend_tests/rpc_profile_test.clj index 1bfb54aeee..6200261e86 100644 --- a/backend/test/backend_tests/rpc_profile_test.clj +++ b/backend/test/backend_tests/rpc_profile_test.clj @@ -1802,3 +1802,30 @@ (t/is (th/success? out)) (t/is (= (:id member) event-pid)) (t/is (not= (:id owner) event-pid)))))) + +(t/deftest delete-profile-audit-event-carries-profile-data + ;; The event used to carry empty props because every param is namespaced and + ;; clean-props drops them; the projection needs the deleted identity. + (let [profile (th/create-profile* 1 {:is-active true}) + out (th/command-raw! {::th/type :delete-profile + ::rpc/profile-id (:id profile)}) + props (:app.loggers.audit/props (meta out))] + (t/is (= (:id profile) (:id props))) + (t/is (= "profile1.test@nodomain.com" (:email props))) + (t/is (= "Profile 1" (:fullname props))) + (t/is (ct/inst? (:created-at props))) + (t/is (ct/inst? (:modified-at props))) + (t/is (ct/inst? (:deleted-at props))))) + +(t/deftest delete-profile-is-idempotent + ;; Deleting an already-deleted profile (client retry after a lost response) + ;; succeeds: the row is still there, only marked. + (let [profile (th/create-profile* 1 {:is-active true}) + params {::th/type :delete-profile + ::rpc/profile-id (:id profile)} + first-out (th/command-raw! params)] + (t/is (th/success? first-out)) + (let [second-out (th/command-raw! params)] + (t/is (th/success? second-out)) + (t/is (= (:id profile) + (get-in (meta second-out) [:app.loggers.audit/props :id])))))) diff --git a/backend/test/backend_tests/rpc_project_test.clj b/backend/test/backend_tests/rpc_project_test.clj index f5855a43e8..c27bb31868 100644 --- a/backend/test/backend_tests/rpc_project_test.clj +++ b/backend/test/backend_tests/rpc_project_test.clj @@ -274,3 +274,16 @@ (t/is (th/ex-info? (:error out))) (t/is (th/ex-of-type? (:error out) :validation)) (t/is (th/ex-of-code? (:error out) :params-validation)))) + +(t/deftest create-project-audit-event-carries-its-id + ;; The command returns the new project but did not emit its id, so a consumer + ;; could not key a local projection. + (let [profile (th/create-profile* 1) + team (th/create-team* 1 {:profile-id (:id profile)}) + out (th/command! {::th/type :create-project + ::rpc/profile-id (:id profile) + :team-id (:id team) + :name "test project"})] + (t/is (th/success? out)) + (t/is (= (get-in out [:result :id]) + (get-in (meta (:result out)) [:app.loggers.audit/props :id]))))) diff --git a/backend/test/backend_tests/rpc_team_test.clj b/backend/test/backend_tests/rpc_team_test.clj index ed6520b815..991b380548 100644 --- a/backend/test/backend_tests/rpc_team_test.clj +++ b/backend/test/backend_tests/rpc_team_test.clj @@ -6,8 +6,10 @@ (ns backend-tests.rpc-team-test (:require + [app.common.data :as d] [app.common.logging :as l] [app.common.time :as ct] + [app.common.types.team :as types.team] [app.common.uuid :as uuid] [app.config :as cf] [app.db :as db] @@ -16,6 +18,7 @@ [app.nitrate :as nitrate] [app.rpc :as-alias rpc] [app.rpc.commands.teams :as teams] + [app.rpc.helpers :as rph] [app.storage :as sto] [app.tokens :as tokens] [backend-tests.helpers :as th] @@ -1751,3 +1754,195 @@ ::rpc/profile-id (:id stranger)})] (t/is (th/success? out)) (t/is (nil? (some #(= (:id project) (:id %)) (:result out)))))))) + +(t/deftest create-team-emits-add-team-member-audit-event + ;; Team creation adds the owner through add-profile-to-team!, which used to + ;; leave no membership event. + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (with-redefs [cf/flags (conj cf/flags :audit-log)] + (let [owner (th/create-profile* 1 {:is-active true}) + team (th/create-team* 21 {:profile-id (:id owner)}) + events (->> (:call-args-list @audit-mock) + (map second) + (filter #(= "add-team-member" (:name %))) + (filter #(= (:id team) (get-in % [:props :team-id]))))] + (t/is (= 1 (count events))) + (t/is (= (:id owner) (get-in (first events) [:props :member-id]))) + (t/is (true? (get-in (first events) [:props :is-owner]))))))) + +(t/deftest add-team-member-event-skipped-when-membership-exists + ;; Adding an existing member with on-conflict-do-nothing inserts nothing and + ;; must not report a membership that already exists. + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (with-redefs [cf/flags (conj cf/flags :audit-log)] + (let [owner (th/create-profile* 1 {:is-active true}) + team (th/create-team* 22 {:profile-id (:id owner)})] + (th/reset-mock! audit-mock) + (db/tx-run! th/*system* + (fn [cfg] + (teams/add-profile-to-team! cfg + {:team-id (:id team) + :profile-id (:id owner)} + {::db/on-conflict-do-nothing? true}))) + (t/is (empty? (:call-args-list @audit-mock))))))) + +(t/deftest no-add-team-member-event-without-audit-log + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (let [owner (th/create-profile* 1 {:is-active true}) + _team (th/create-team* 23 {:profile-id (:id owner)}) + events (->> (:call-args-list @audit-mock) + (map second) + (filter #(= "add-team-member" (:name %))))] + (t/is (empty? events))))) + +(t/deftest add-team-member-event-carries-derived-role + ;; `team_profile_rel` has no role column, so no caller can pass it: the + ;; emitter derives it from the flags, inverse of permissions-for-role. + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (with-redefs [cf/flags (conj cf/flags :audit-log)] + (let [owner (th/create-profile* 1 {:is-active true}) + team (th/create-team* 26 {:profile-id (:id owner)}) + editor (th/create-profile* 27 {:is-active true}) + admin (th/create-profile* 28 {:is-active true}) + viewer (th/create-profile* 29 {:is-active true})] + (db/tx-run! th/*system* + (fn [cfg] + (doseq [[member role] [[editor :editor] + [admin :admin] + [viewer :viewer]]] + (teams/add-profile-to-team! + cfg + (merge {:team-id (:id team) + :profile-id (:id member)} + (get types.team/permissions-for-role role)))))) + (let [by-member (->> (:call-args-list @audit-mock) + (map second) + (filter #(= "add-team-member" (:name %))) + (d/index-by #(get-in % [:props :member-id])))] + (t/is (= :owner (get-in by-member [(:id owner) :props :role]))) + (t/is (= :editor (get-in by-member [(:id editor) :props :role]))) + (t/is (= :admin (get-in by-member [(:id admin) :props :role]))) + (t/is (= :viewer (get-in by-member [(:id viewer) :props :role])))))))) + +(t/deftest add-team-member-event-carries-request-context + ;; Sibling events built by `prepare-rpc-event` carry the request context; + ;; the hand-built membership event must not lose it when rpc params travel. + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (with-redefs [cf/flags (conj cf/flags :audit-log)] + (let [owner (th/create-profile* 1 {:is-active true}) + member (th/create-profile* 2 {:is-active true}) + team (th/create-team* 30 {:profile-id (:id owner)})] + (th/reset-mock! audit-mock) + (db/tx-run! th/*system* + (fn [cfg] + (teams/add-profile-to-team! + cfg + (merge {:team-id (:id team) + :profile-id (:id member)} + (get types.team/permissions-for-role :editor)) + nil + {:rpc-params {::rpc/profile-id (:id owner) + ::rpc/request-id "req-1"}}))) + (let [events (->> (:call-args-list @audit-mock) + (map second) + (filter #(= "add-team-member" (:name %))))] + (t/is (= 1 (count events))) + (t/is (= "req-1" (get-in (first events) [:context :request-id])))))))) + +(t/deftest invitation-direct-add-attributes-event-to-inviter + ;; With email verification off, inviting an existing profile adds the + ;; member directly; the event belongs to the inviter, not the member. + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}] + (with-redefs [cf/flags #{:audit-log}] + (let [owner (th/create-profile* 1 {:is-active true}) + member (th/create-profile* 2 {:is-active true}) + team (th/create-team* 31 {:profile-id (:id owner)})] + (th/reset-mock! audit-mock) + (let [out (th/command! {::th/type :create-team-invitations + ::rpc/profile-id (:id owner) + :team-id (:id team) + :role :editor + :emails [(:email member)]})] + (t/is (th/success? out)) + (let [events (->> (:call-args-list @audit-mock) + (map second) + (filter #(= "add-team-member" (:name %))) + (filter #(= (:id member) (get-in % [:props :member-id]))))] + (t/is (= 1 (count events))) + (t/is (= (:id owner) (:profile-id (first events)))) + (t/is (= :editor (get-in (first events) [:props :role]))))))))) + +(t/deftest create-team-audit-event-carries-created-at + ;; The projection keys the new team on the event id and needs its creation + ;; time from the same event. + (let [profile (th/create-profile* 1 {:is-active true}) + out (th/command-raw! {::th/type :create-team + ::rpc/profile-id (:id profile) + :name "team"}) + props (:app.loggers.audit/props (meta out))] + (t/is (= (:id out) (:id props))) + (t/is (= (:created-at out) (:created-at props))) + (t/is (ct/inst? (:created-at props))) + (t/is (= (:modified-at out) (:modified-at props))) + (t/is (ct/inst? (:modified-at props))))) + +(t/deftest delete-team-audit-event-carries-entity-data + ;; The projection marks the team deleted from this single event, so it needs + ;; the identity, the name and the timestamps. + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 31 {:profile-id (:id profile)}) + out (th/command-raw! {::th/type :delete-team + ::rpc/profile-id (:id profile) + :id (:id team)}) + props (:app.loggers.audit/props (meta out))] + ;; The API response is unchanged: the command still returns nil. + (t/is (nil? (rph/unwrap out))) + (t/is (= (:id team) (:id props))) + (t/is (= (:name team) (:name props))) + (t/is (= (:created-at team) (:created-at props))) + (t/is (ct/inst? (:modified-at props))) + (t/is (ct/inst? (:deleted-at props))))) + +(t/deftest create-team-with-invitations-audit-event-carries-team-id + ;; The team is created by an internal call, so both the command event and + ;; the manual event must carry its id for a projection to key it. + (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil} + email-mock {:target 'app.email/send! :return nil}] + (with-redefs [cf/flags (conj cf/flags :audit-log)] + (let [profile (th/create-profile* 1 {:is-active true}) + out (th/command-raw! {::th/type :create-team-with-invitations + ::rpc/profile-id (:id profile) + :name "team" + :emails #{"invitee@example.com"} + :role :editor}) + props (:app.loggers.audit/props (meta out)) + events (->> (:call-args-list @audit-mock) + (map second) + (filter #(= "create-team" (:name %))))] + (t/is (= (:id out) (:id props))) + (t/is (= 1 (count events))) + (t/is (= (:id out) (get-in (first events) [:props :id]))))))) + +(t/deftest update-team-member-role-to-owner-names-demoted-member + ;; Promoting a member to owner demotes the actor; the event names them so a + ;; projection can keep a single owner. + (let [owner (th/create-profile* 1 {:is-active true}) + editor (th/create-profile* 2 {:is-active true}) + team (th/create-team* 82 {:profile-id (:id owner)})] + (th/create-team-role* {:team-id (:id team) + :profile-id (:id editor) + :role :editor}) + (let [out (th/command-raw! {::th/type :update-team-member-role + ::rpc/profile-id (:id owner) + :team-id (:id team) + :member-id (:id editor) + :role :editor}) + props (:app.loggers.audit/props (meta out))] + (t/is (nil? (:prev-owner-id props)))) + (let [out (th/command-raw! {::th/type :update-team-member-role + ::rpc/profile-id (:id owner) + :team-id (:id team) + :member-id (:id editor) + :role :owner}) + props (:app.loggers.audit/props (meta out))] + (t/is (= (:id owner) (:prev-owner-id props)))))) diff --git a/backend/test/backend_tests/rpc_webhooks_test.clj b/backend/test/backend_tests/rpc_webhooks_test.clj index 5b15d46111..de5227aeb2 100644 --- a/backend/test/backend_tests/rpc_webhooks_test.clj +++ b/backend/test/backend_tests/rpc_webhooks_test.clj @@ -374,3 +374,29 @@ (t/is (nil? (:result out))) (let [rows (th/db-exec! ["select * from webhook"])] (t/is (= 0 (count rows))))))))))) + +(t/deftest webhook-audit-events-carry-team-id + ;; `update-webhook` and `delete-webhook` have no team id in their params, so + ;; the command copies it from the stored webhook into the audit props. + (with-mocks [http-mock {:target 'app.http.client/req :return {:status 200}}] + (let [prof (th/create-profile* 1 {:is-active true}) + team (th/create-team* 1 {:profile-id (:id prof)}) + whook (-> (th/command! {::th/type :create-webhook + ::rpc/profile-id (:id prof) + :team-id (:id team) + :uri (u/uri "http://example.com") + :mtype "application/json"}) + :result) + upd (th/command! {::th/type :update-webhook + ::rpc/profile-id (:id prof) + :id (:id whook) + :uri (u/uri "http://example.com/updated") + :mtype "application/json" + :is-active true}) + del (th/command-raw! {::th/type :delete-webhook + ::rpc/profile-id (:id prof) + :id (:id whook)})] + (t/is (= (:id team) + (get-in (meta (:result upd)) [:app.loggers.audit/props :team-id]))) + (t/is (= (:id team) + (get-in (meta del) [:app.loggers.audit/props :team-id])))))) diff --git a/backend/test/backend_tests/srepl_main_test.clj b/backend/test/backend_tests/srepl_main_test.clj index 1e2add5361..aebc99c9bf 100644 --- a/backend/test/backend_tests/srepl_main_test.clj +++ b/backend/test/backend_tests/srepl_main_test.clj @@ -11,7 +11,8 @@ [app.system :as sys] [app.worker :as wrk] [backend-tests.helpers :as th] - [clojure.test :as t])) + [clojure.test :as t] + [mockery.core :refer [with-mocks]])) (t/use-fixtures :once th/state-init) (t/use-fixtures :each th/database-reset) @@ -150,3 +151,57 @@ #(srepl/process-deleted-files-cascade :ids [(:id file)]))] (t/is (= [{:object :file :id (:id file)}] (mapv #(select-keys % [:object :id]) (params-of submitted)))))))))) + +(t/deftest delete-profile-emits-identifiable-audit-event + ;; SREPL has no actor, so the event profile stays zero; the subject id in + ;; props is what makes the row attributable. Both insert and cascade are + ;; mocked, so this has no side effects. + (with-mocks [audit-mock {:target 'app.loggers.audit/insert :return nil} + wrk-mock {:target 'app.worker/invoke! :return nil}] + (let [profile (th/create-profile* 1 {:is-active true})] + (srepl/delete-profile! (str (:id profile))) + (let [events (->> (:call-args-list @audit-mock) + (map second))] + (t/is (= 1 (count events))) + (let [event (first events)] + (t/is (= "delete-profile" (:name event))) + (t/is (= "action" (:type event))) + ;; No actor on SREPL calls; insert defaults the missing profile + ;; to the zero one. + (t/is (nil? (:profile-id event))) + (t/is (= (:id profile) (get-in event [:props :id])))))))) + +(t/deftest delete-team-emits-identifiable-audit-event + (with-mocks [audit-mock {:target 'app.loggers.audit/insert :return nil} + wrk-mock {:target 'app.worker/invoke! :return nil}] + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 95 {:profile-id (:id profile)})] + (srepl/delete-team! (str (:id team))) + (let [events (->> (:call-args-list @audit-mock) + (map second))] + (t/is (= 1 (count events))) + (let [event (first events)] + (t/is (= "delete-team" (:name event))) + (t/is (= "action" (:type event))) + (t/is (= (:id team) (get-in event [:props :id]))) + (t/is (= "explicit call to delete-team!" + (get-in event [:context :cause])))))))) + +(t/deftest restore-profile-emits-identifiable-audit-event + ;; SREPL has no actor, so the event profile stays zero; the subject id in + ;; props is what makes the row attributable. + (with-mocks [audit-mock {:target 'app.loggers.audit/insert :return nil} + wrk-mock {:target 'app.worker/invoke! :return nil}] + (with-redefs [sys/system th/*system*] + (let [profile (th/create-profile* 1 {:is-active true})] + (srepl/restore-profile! (str (:id profile))) + (let [events (->> (:call-args-list @audit-mock) + (map second))] + (t/is (= 1 (count events))) + (let [event (first events)] + (t/is (= "restore-profile" (:name event))) + (t/is (= "action" (:type event))) + ;; No actor on SREPL calls; insert defaults the missing profile + ;; to the zero one. + (t/is (nil? (:profile-id event))) + (t/is (= (:id profile) (get-in event [:props :id])))))))))