From a53481c3b7cc37a02850097007e184affac170a6 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 15 Oct 2026 13:01:18 +0200 Subject: [PATCH] :sparkles: Enrich backend audit events with team and entity ids (#12233) * :sparkles: Enrich backend audit events with team and entity ids Backend audit events now carry the data a consumer needs to rebuild state without joining production tables: - prepare-rpc-event adds :team-id when the event carries a project or file id; commands that already loaded the team set it themselves. - add-profile-to-team! emits add-team-member on every real insert. - create-project and create-file emit the new row id. - clone-template, import-binfile and duplicate-project emit one event per created entity. - both binfile versions return {:file-ids [...]} from import-files!. Closes #12219 AI-assisted-by: deepseek-v4.1-flash * :sparkles: Emit delete and hard-delete audit events with entity data The event stream must let Nexus rebuild local state without joining production tables. This completes the enrichment with the delete paths: - delete-team keeps returning nil but now carries the deleted row (id, name, timestamps) in the audit props, like delete-project. - create-team adds created-at to its audit props. - delete-profile carries id, email, fullname and deleted-at; its props used to be empty because every param is namespaced. - objects-gc emits hard-delete-team/project/file per purged batch, with the deleted ids, only when the audit log is on. - delete-team-member needs no change: the delete is physical and the params already carry team-id and member-id. The nitrate push-audit-events tests now drop the membership events emitted by their own profile setup before counting. Closes #12219 AI-assisted-by: muse-spark-1.3-free * :sparkles: Announce cascade deletes and missing ids in audit events The :delete-object worker task marked teams, projects and files without leaving any event, so an event-sourced projection kept them alive for weeks. It now emits one cascade-deleted event per execution with every affected id, in the same transaction, only with :audit-log on. Rows purged by objects-gc and snapshot cascades stay silent on purpose. Also: create-team-with-invitations carries the new team id, and update-team-member-role names the demoted member on owner transfers. Closes #12219 AI-assisted-by: muse-spark-1.3-free * :bug: Identify the SREPL profile delete in audit events The SREPL delete-profile! emitted no profile-id and empty props, so the row could not be attributed by any consumer. It now carries the profile id in both fields. delete-team! named the wrong cause in its context. Closes #12219 AI-assisted-by: muse-spark-1.3-free * :bug: Identify the SREPL profile restore in audit events The SREPL restore-profile! emitted no profile-id, so the consumer keyed on the zero profile instead of the restored one. It now carries the profile id like a normal profile event, and no consumer special case is needed. Closes #12219 AI-assisted-by: muse-spark-1.3-free * :bug: Stop attributing SREPL events to the subject profile profile-id names the actor, and SREPL calls have none: setting the subject id blamed the user for an admin action and would have leaked those rows into PostHog as user activity. The subject id stays in props, where the consumer resolves it, and the event profile stays zero. Closes #12219 AI-assisted-by: muse-spark-1.3-free * :sparkles: Expose file analysis data for the nexus pipeline The nexus file-security pipeline needs file content and scheduling state from the backend. This adds a read-only management method, `get-file-for-analysis`, returning the semantic projection of one file (pages, frames, texts, links, interactions; ids as strings; capped with a truncated flag), plus identity, revn, timestamps and share state. No prior projection code existed, so the method ships its own pure `file->projection`. Audit events now carry what the `:local` consumer schedules from: revn and modified-at on update-file, name and timestamps on every create-file emitter, deleted-at on delete-file, and new create/delete-share-link events with file and link ids. AI-assisted-by: muse-spark-1.3-contributor * :recycle: Rename deletion audit events to action names Event names are actions, not past tense: `cascade-deleted` becomes `delete-object-with-cascade` with the same shape and transaction. The three `hard-delete-*` events merge into `hard-delete-objects`: one event per purged chunk carrying only that chunk's entity key (`:team-ids`, `:project-ids` or `:file-ids`), because each GC section purges in its own chunk transactions and a single event could never share them all. Every submit stays inside its delete transaction, `triggered-by` stays annotated, and silent paths stay silent. AI-assisted-by: muse-spark-1.3-contributor * :bug: Read worker parallelism knobs with plain config keys The runner wiring looked up `:app.main/worker-*-parallelism` (namespaced) while the schema and the environment provide plain keys, so `PENPOT_WORKER_DEFAULT_PARALLELISM` and `PENPOT_WORKER_WEBHOOK_PARALLELISM` never applied and workers always ran with the fallback. Read the keys the config holds. AI-assisted-by: muse-spark-1.3-contributor * :bug: Fix binfile audit test with a real file row `submit-file-created-event` reads the light columns back from the row, so the mocked importer must return an existing file id: a phantom id emits no event and the test saw zero `create-file` events. Create the file first and use its id. The audit-log memory now tells both bulk-event helpers apart. AI-assisted-by: muse-spark-1.3-contributor * :bug: Fix audit event fidelity from review findings The review of the audit enrichment found data-fidelity gaps; fix them while the branch is fresh. Membership events now belong to the actor: the flows where the owner adds somebody else pass an explicit actor id, the role is derived from the flags (the table has no role column), and the event carries the request context like its siblings. Also: team-id resolution skips read events, duplicate-project emits the project name and timestamps, delete-profile is idempotent again, the GC tests assert physical purge, and get-file-for-analysis leaves an audit trace. AI-assisted-by: muse-spark-1.3-contributor * :recycle: Nest cascade ids under :cascade in delete event The cascade event carried every affected id flat next to the object identity; nest them under `:cascade` so the payload reads as header plus affected set. The event is an action like its siblings, not a command. Drop the `:audit-log` flag re-check: `submit` already fans out per flag. AI-assisted-by: muse-spark-1.3-contributor * :paperclip: Update copyright name on file header The company name changed; a single header in this branch still carried the old one. Align it with the current form. AI-assisted-by: muse-spark-1.3-contributor * :bug: Fix nil event-id on webhook trigger audit rows process-event generated the row id inside append-audit-entry, so the :process-webhook-event task never received it and the trigger row stored a nil :event-id. Assign the id up-front and share it between the stored row and the worker params; the telemetry shadow row keeps its own id to avoid a PK collision. Add regression tests covering the handoff and the end-to-end trigger row. AI-assisted-by: muse-spark-1.3-contributor --- .serena/memories/audit-log.md | 21 +- backend/src/app/binfile/v1.clj | 4 +- backend/src/app/loggers/audit.clj | 205 +++++++--- backend/src/app/main.clj | 4 +- backend/src/app/rpc.clj | 3 +- backend/src/app/rpc/commands/binfile.clj | 12 +- backend/src/app/rpc/commands/comments.clj | 4 +- backend/src/app/rpc/commands/files.clj | 4 +- backend/src/app/rpc/commands/files_create.clj | 10 +- backend/src/app/rpc/commands/files_share.clj | 14 +- backend/src/app/rpc/commands/files_update.clj | 17 +- backend/src/app/rpc/commands/management.clj | 37 +- backend/src/app/rpc/commands/media.clj | 37 +- backend/src/app/rpc/commands/profile.clj | 9 +- backend/src/app/rpc/commands/projects.clj | 5 +- backend/src/app/rpc/commands/teams.clj | 67 +++- .../app/rpc/commands/teams_invitations.clj | 20 +- backend/src/app/rpc/commands/webhooks.clj | 8 +- backend/src/app/rpc/management/files.clj | 238 +++++++++++ backend/src/app/srepl/main.clj | 5 +- backend/src/app/tasks/delete_object.clj | 53 ++- backend/src/app/tasks/objects_gc.clj | 109 ++++-- .../test/backend_tests/binfile_svg_test.clj | 15 +- .../test/backend_tests/delete_object_test.clj | 107 +++++ backend/test/backend_tests/helpers.clj | 14 + .../backend_tests/loggers_webhooks_test.clj | 40 ++ .../test/backend_tests/objects_gc_test.clj | 121 +++++- backend/test/backend_tests/rpc_audit_test.clj | 150 ++++++- .../test/backend_tests/rpc_binfile_test.clj | 41 +- .../test/backend_tests/rpc_comment_test.clj | 29 ++ .../backend_tests/rpc_file_security_test.clj | 368 ++++++++++++++++++ backend/test/backend_tests/rpc_file_test.clj | 33 ++ .../rpc_management_nitrate_test.clj | 15 + .../backend_tests/rpc_management_test.clj | 56 ++- backend/test/backend_tests/rpc_media_test.clj | 24 ++ .../test/backend_tests/rpc_profile_test.clj | 27 ++ .../test/backend_tests/rpc_project_test.clj | 13 + backend/test/backend_tests/rpc_team_test.clj | 195 ++++++++++ .../test/backend_tests/rpc_webhooks_test.clj | 26 ++ .../test/backend_tests/srepl_main_test.clj | 57 ++- 40 files changed, 2024 insertions(+), 193 deletions(-) create mode 100644 backend/src/app/rpc/management/files.clj create mode 100644 backend/test/backend_tests/delete_object_test.clj create mode 100644 backend/test/backend_tests/rpc_file_security_test.clj 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])))))))))