From 2f679eaa0e8f07e1ac5908b88fbb3b85aa83041f Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 22 Sep 2026 15:51:50 +0200 Subject: [PATCH] :boom: Remove client-provided id from creation RPC commands (#11784) The seven creation commands no longer accept an optional client id: create-file, create-project, create-team, create-team-with-invitations, upload-file-media-object, create-file-media-object-from-url and assemble-file-media-object. The server always generates the identifier; a sent id is ignored. Malli maps are open and the RPC layer never strips unknown params, so the handlers that would still honor an id (create-file, create-project) now drop it explicitly. Internal callers that pass remapped ids (project duplicate, binfile import) keep working. Closes #11783 AI-assisted-by: muse-spark-1.3-contributor --- .serena/memories/backend/subtleties.md | 1 + backend/src/app/rpc/commands/files_create.clj | 4 +- backend/src/app/rpc/commands/media.clj | 17 +- backend/src/app/rpc/commands/projects.clj | 6 +- backend/src/app/rpc/commands/teams.clj | 9 +- .../app/rpc/commands/teams_invitations.clj | 4 +- backend/test/backend_tests/rpc_file_test.clj | 259 +++++++----------- backend/test/backend_tests/rpc_media_test.clj | 251 ++++------------- .../test/backend_tests/rpc_project_test.clj | 63 ++--- backend/test/backend_tests/rpc_team_test.clj | 80 ++---- .../app/main/data/workspace/svg_upload.cljs | 5 +- .../frontend_tests/data/svg_upload_test.cljs | 63 +++++ frontend/test/frontend_tests/runner.cljs | 2 + 13 files changed, 299 insertions(+), 465 deletions(-) create mode 100644 frontend/test/frontend_tests/data/svg_upload_test.cljs diff --git a/.serena/memories/backend/subtleties.md b/.serena/memories/backend/subtleties.md index c00117666a..dfe0a490af 100644 --- a/.serena/memories/backend/subtleties.md +++ b/.serena/memories/backend/subtleties.md @@ -7,6 +7,7 @@ - RPC auth defaults to enabled. Public endpoints must set `::auth false` metadata explicitly. - The wrapper stack does auth before params validation, then auditing/rate/concurrency/metrics/retry/condition handling, with DB transaction handling inside that stack. `::db/transaction` metadata controls transaction wrapping. - Params with `::sm/params` are decoded/conformed through the JSON transformer and successful IObj results get `:encode/json` metadata. Legacy spec conforming only applies when no Malli params schema exists. Client params are stripped of qualified keys (`d/without-qualified`) before merging with the server auth context, so request bodies cannot override `::profile-id`, `::auth-type`, or `::token-perms`. +- Params schemas are open by default, so undeclared client keys reach the handler unless the map is `:closed true`. Creation commands (`create-file`, `create-project`, `create-team`, `create-team-with-invitations`, `upload-file-media-object`, `create-file-media-object-from-url`, `assemble-file-media-object`) use closed schemas: a client-provided `:id` fails with `:params-validation`. Their internal creation functions still accept an optional explicit `:id` for imports, duplicates and deterministic test fixtures. - Nil RPC bodies become HTTP 204 unless explicit status metadata is present. Stream bodies default to `application/octet-stream` when no content type is set. ## DB helpers diff --git a/backend/src/app/rpc/commands/files_create.clj b/backend/src/app/rpc/commands/files_create.clj index b7e2373909..d1b14283a5 100644 --- a/backend/src/app/rpc/commands/files_create.clj +++ b/backend/src/app/rpc/commands/files_create.clj @@ -72,15 +72,15 @@ (bfc/get-file cfg (:id file))))) (def ^:private schema:create-file - [:map {:title "create-file"} + [:map {:title "create-file" :closed true} [:name [:string {:max 250}]] [:project-id ::sm/uuid] - [:id {:optional true} ::sm/user-provided-uuid] [:is-shared {:optional true} ::sm/boolean] [:features {:optional true} ::cfeat/features]]) (sv/defmethod ::create-file {::doc/added "1.17" + ::doc/changes [["2.19" "The optional :id param is rejected with a params-validation error; the server always generates the identifier"]] ::doc/module :files ::webhooks/event? true ::sm/params schema:create-file diff --git a/backend/src/app/rpc/commands/media.clj b/backend/src/app/rpc/commands/media.clj index c86e9ee457..e914ff3302 100644 --- a/backend/src/app/rpc/commands/media.clj +++ b/backend/src/app/rpc/commands/media.clj @@ -47,8 +47,7 @@ WHERE f.id = ?") (def ^:private schema:upload-file-media-object - [:map {:title "upload-file-media-object"} - [:id {:optional true} ::sm/user-provided-uuid] + [:map {:title "upload-file-media-object" :closed true} [:file-id ::sm/uuid] [:is-local ::sm/boolean] [:name [:string {:max 250}]] @@ -56,6 +55,7 @@ (sv/defmethod ::upload-file-media-object {::doc/added "1.17" + ::doc/changes [["2.19" "The optional :id param is rejected with a params-validation error; the server always generates the identifier"]] ::sm/params schema:upload-file-media-object ::climit/id [[:process-image/by-profile ::rpc/profile-id] [:process-image/global]]} @@ -221,15 +221,15 @@ (declare ^:private create-file-media-object-from-url) (def ^:private schema:create-file-media-object-from-url - [:map {:title "create-file-media-object-from-url"} + [:map {:title "create-file-media-object-from-url" :closed true} [:file-id ::sm/uuid] [:is-local ::sm/boolean] [:url ::sm/uri] - [:id {:optional true} ::sm/user-provided-uuid] [:name {:optional true} [:string {:max 250}]]]) (sv/defmethod ::create-file-media-object-from-url {::doc/added "1.17" + ::doc/changes [["2.19" "The optional :id param is rejected with a params-validation error; the server always generates the identifier"]] ::sm/params schema:create-file-media-object-from-url} [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id file-id] :as params}] (files/check-edition-permissions! pool profile-id file-id) @@ -550,21 +550,21 @@ ;; --- Chunked Upload: Assemble all chunks into a final media object (def ^:private schema:assemble-file-media-object - [:map {:title "assemble-file-media-object"} + [:map {:title "assemble-file-media-object" :closed true} [:session-id ::sm/uuid] [:file-id ::sm/uuid] [:is-local ::sm/boolean] [:name [:string {:max 250}]] - [:mtype :string] - [:id {:optional true} ::sm/user-provided-uuid]]) + [:mtype :string]]) (sv/defmethod ::assemble-file-media-object {::doc/added "2.17" + ::doc/changes [["2.19" "The optional :id param is rejected with a params-validation error; the server always generates the identifier"]] ::sm/params schema:assemble-file-media-object ::climit/id [[:process-image/by-profile ::rpc/profile-id] [:process-image/global]]} [{:keys [::db/pool] :as cfg} - {:keys [::rpc/profile-id session-id file-id is-local name mtype id] :as params}] + {:keys [::rpc/profile-id session-id file-id is-local name mtype] :as params}] (files/check-edition-permissions! pool profile-id file-id) (db/tx-run! cfg @@ -576,7 +576,6 @@ (media.v/validate-media-type!) (media.v/validate-media-size!)) mobj (create-file-media-object cfg (assoc params - :id id :from-chunks? true :content content))] diff --git a/backend/src/app/rpc/commands/projects.clj b/backend/src/app/rpc/commands/projects.clj index 6d997dd896..e69c704270 100644 --- a/backend/src/app/rpc/commands/projects.clj +++ b/backend/src/app/rpc/commands/projects.clj @@ -196,13 +196,13 @@ (assoc project :is-pinned false))) (def ^:private schema:create-project - [:map {:title "create-project"} + [:map {:title "create-project" :closed true} [:team-id ::sm/uuid] - [:name [:string {:max 250 :min 1}]] - [:id {:optional true} ::sm/user-provided-uuid]]) + [:name [:string {:max 250 :min 1}]]]) (sv/defmethod ::create-project {::doc/added "1.18" + ::doc/changes [["2.19" "The optional :id param is rejected with a params-validation error; the server always generates the identifier"]] ::webhooks/event? true ::sm/params schema:create-project} [cfg {:keys [::rpc/profile-id team-id] :as params}] diff --git a/backend/src/app/rpc/commands/teams.clj b/backend/src/app/rpc/commands/teams.clj index 55b0f87fae..405933a463 100644 --- a/backend/src/app/rpc/commands/teams.clj +++ b/backend/src/app/rpc/commands/teams.clj @@ -520,15 +520,15 @@ (declare ^:private create-team-default-project) (def ^:private schema:create-team - [:map {:title "create-team"} + [:map {:title "create-team" :closed true} [:name types.team/schema:team-name] [:features {:optional true} ::cfeat/features] - [:id {:optional true} ::sm/user-provided-uuid] [:organization-id {:optional true} ::sm/uuid] [:is-default {:optional true} :boolean]]) (sv/defmethod ::create-team {::doc/added "1.17" + ::doc/changes [["2.19" "The optional :id param is rejected with a params-validation error; the server always generates the identifier"]] ::sm/params schema:create-team} [cfg {:keys [::rpc/profile-id organization-id] :as params}] @@ -651,7 +651,7 @@ (assoc team :default-project-id (:id project)))) (defn- create-team* - [conn {:keys [id name is-default features] :as params}] + [conn {:keys [id name is-default features]}] (let [id (or id (uuid/next)) is-default (if (boolean? is-default) is-default false) features (db/create-array conn "text" features) @@ -690,6 +690,9 @@ (defn create-project [conn {:keys [id team-id name is-default created-at modified-at]}] + ;; NOTE: the explicit id is kept for internal callers that duplicate or + ;; import projects with a remapped id (see management.clj); the RPC + ;; commands no longer accept a client-provided id. (let [id (or id (uuid/next)) is-default (if (boolean? is-default) is-default false) name (d/normalize-string name) diff --git a/backend/src/app/rpc/commands/teams_invitations.clj b/backend/src/app/rpc/commands/teams_invitations.clj index 4965b0ef3f..77fc8246e8 100644 --- a/backend/src/app/rpc/commands/teams_invitations.clj +++ b/backend/src/app/rpc/commands/teams_invitations.clj @@ -535,15 +535,15 @@ ;; --- Mutation: Create Team & Invite Members (def ^:private schema:create-team-with-invitations - [:map {:title "create-team-with-invitations"} + [:map {:title "create-team-with-invitations" :closed true} [:name [:string {:max 250}]] [:features {:optional true} ::cfeat/features] - [:id {:optional true} ::sm/user-provided-uuid] [:emails [::sm/set ::sm/email]] [:role types.team/schema:role]]) (sv/defmethod ::create-team-with-invitations {::doc/added "1.17" + ::doc/changes [["2.19" "The optional :id param is rejected with a params-validation error; the server always generates the identifier"]] ::doc/module :teams ::sm/params schema:create-team-with-invitations ::db/transaction true} diff --git a/backend/test/backend_tests/rpc_file_test.clj b/backend/test/backend_tests/rpc_file_test.clj index dab6c773a1..3ebcc4b80f 100644 --- a/backend/test/backend_tests/rpc_file_test.clj +++ b/backend/test/backend_tests/rpc_file_test.clj @@ -46,25 +46,23 @@ (let [prof (th/create-profile* 1 {:is-active true}) team-id (:default-team-id prof) proj-id (:default-project-id prof) - file-id (uuid/next) - page-id (uuid/next)] + page-id (uuid/next) + data {::th/type :create-file + ::rpc/profile-id (:id prof) + :project-id proj-id + :name "foobar" + :is-shared false} + out (th/command! data) + file-id (:id (:result out))] (t/testing "create file" - (let [data {::th/type :create-file - ::rpc/profile-id (:id prof) - :project-id proj-id - :id file-id - :name "foobar" - :is-shared false - :components-v2 true} - out (th/command! data)] + ;; (th/print-result! out) + (t/is (nil? (:error out))) - ;; (th/print-result! out) - (t/is (nil? (:error out))) - - (let [result (:result out)] - (t/is (= (:name data) (:name result))) - (t/is (= proj-id (:project-id result)))))) + (let [result (:result out)] + (t/is (uuid? file-id)) + (t/is (= (:name data) (:name result))) + (t/is (= proj-id (:project-id result))))) (t/testing "rename file" (let [data {::th/type :rename-file @@ -141,58 +139,6 @@ (let [result (:result out)] (t/is (= 0 (count result)))))))) -(t/deftest create-file-with-duplicate-id - (let [prof (th/create-profile* 1 {:is-active true}) - proj-id (:default-project-id prof) - file-id (uuid/next)] - - (t/testing "create file with specific id" - (let [data {::th/type :create-file - ::rpc/profile-id (:id prof) - :project-id proj-id - :id file-id - :name "first-file"} - out (th/command! data)] - (t/is (nil? (:error out))))) - - (t/testing "create file with duplicate id returns normalized error" - (let [data {::th/type :create-file - ::rpc/profile-id (:id prof) - :project-id proj-id - :id file-id - :name "duplicate-file"} - out (th/command! data) - err (:error out)] - (t/is (th/ex-info? err)) - (t/is (th/ex-of-type? err :not-found)))))) - -(t/deftest create-file-id-version - (let [prof (th/create-profile* 1 {:is-active true}) - proj-id (:default-project-id prof) - v3-id "6fa459ea-ee8a-3ca4-894e-db77e160355e" - v4-id "550e8400-e29b-41d4-a716-446655440000"] - - ;; reserved version (v3) must be rejected at the RPC boundary - (let [data {::th/type :create-file - ::rpc/profile-id (:id prof) - :project-id proj-id - :id v3-id - :name "file with v3 id"} - out (th/command! data)] - (t/is (not (th/success? out))) - (t/is (th/ex-of-type? (:error out) :validation)) - (t/is (th/ex-of-code? (:error out) :params-validation))) - - ;; v4 id is accepted - (let [data {::th/type :create-file - ::rpc/profile-id (:id prof) - :project-id proj-id - :id v4-id - :name "file with v4 id"} - out (th/command! data)] - (t/is (th/success? out)) - (t/is (= v4-id (str (:id (:result out)))))))) - (t/deftest file-gc-with-fragments (let [profile (th/create-profile* 1) file (th/create-file* 1 {:profile-id (:id profile) @@ -824,8 +770,7 @@ ::rpc/profile-id (:id profile2) :project-id (:default-project-id profile1) :name "foobar" - :is-shared false - :components-v2 true} + :is-shared false} out (th/command! data) error (:error out)] @@ -2118,127 +2063,121 @@ (let [prof (th/create-profile* 1 {:is-active true}) team-id (:default-team-id prof) proj-id (:default-project-id prof) - file-id (uuid/next) now (ct/inst "2025-10-31T00:00:00Z")] (binding [ct/*clock* (ct/fixed-clock now)] (let [data {::th/type :create-file ::rpc/profile-id (:id prof) :project-id proj-id - :id file-id :name "foobar" - :is-shared false - :components-v2 true} - out (th/command! data)] + :is-shared false} + out (th/command! data) + _ (t/is (nil? (:error out))) + file-id (:id (:result out))] ;; (th/print-result! out) - (t/is (nil? (:error out))) - (let [result (:result out)] (t/is (= (:name data) (:name result))) - (t/is (= proj-id (:project-id result))))) + (t/is (= proj-id (:project-id result)))) - (let [data {::th/type :delete-file - :id file-id - ::rpc/profile-id (:id prof)} - out (th/command! data)] - ;; (th/print-result! out) - (t/is (nil? (:error out))) - (t/is (nil? (:result out)))) + (let [data {::th/type :delete-file + :id file-id + ::rpc/profile-id (:id prof)} + out (th/command! data)] + ;; (th/print-result! out) + (t/is (nil? (:error out))) + (t/is (nil? (:result out)))) - ;; get deleted files - (let [data {::th/type :get-team-deleted-files - ::rpc/profile-id (:id prof) - :team-id team-id} - out (th/command! data)] - ;; (th/print-result! out) - (t/is (nil? (:error out))) - (let [[row1 :as result] (:result out)] - (t/is (= 1 (count result))) - (t/is (= (:will-be-deleted-at row1) #penpot/inst "2025-11-07T00:00:00Z")) - (t/is (= (:created-at row1) #penpot/inst "2025-10-31T00:00:00Z")) - (t/is (= (:modified-at row1) #penpot/inst "2025-10-31T00:00:00Z")))) + ;; get deleted files + (let [data {::th/type :get-team-deleted-files + ::rpc/profile-id (:id prof) + :team-id team-id} + out (th/command! data)] + ;; (th/print-result! out) + (t/is (nil? (:error out))) + (let [[row1 :as result] (:result out)] + (t/is (= 1 (count result))) + (t/is (= (:will-be-deleted-at row1) #penpot/inst "2025-11-07T00:00:00Z")) + (t/is (= (:created-at row1) #penpot/inst "2025-10-31T00:00:00Z")) + (t/is (= (:modified-at row1) #penpot/inst "2025-10-31T00:00:00Z")))) - (let [data {::th/type :permanently-delete-team-files - ::rpc/profile-id (:id prof) - :team-id team-id - :ids #{file-id}} - out (th/command! data)] - ;; (th/print-result! out) - (t/is (nil? (:error out))) - (let [result (:result out)] - (t/is (fn? result)) + (let [data {::th/type :permanently-delete-team-files + ::rpc/profile-id (:id prof) + :team-id team-id + :ids #{file-id}} + out (th/command! data)] + ;; (th/print-result! out) + (t/is (nil? (:error out))) + (let [result (:result out)] + (t/is (fn? result)) - (let [[ev1 ev2 :as events] (th/consume-sse result)] - (t/is (= 2 (count events))) - (t/is (= (:ids data) (val ev2))))) + (let [[ev1 ev2 :as events] (th/consume-sse result)] + (t/is (= 2 (count events))) + (t/is (= (:ids data) (val ev2))))) - (let [row (th/db-exec-one! ["select * from file where id = ?" file-id])] - (t/is (= (:deleted-at row) now))))))) + (let [row (th/db-exec-one! ["select * from file where id = ?" file-id])] + (t/is (= (:deleted-at row) now)))))))) (t/deftest restore-deleted-files (let [prof (th/create-profile* 1 {:is-active true}) team-id (:default-team-id prof) proj-id (:default-project-id prof) - file-id (uuid/next) now (ct/inst "2025-10-31T00:00:00Z")] (binding [ct/*clock* (ct/fixed-clock now)] (let [data {::th/type :create-file ::rpc/profile-id (:id prof) :project-id proj-id - :id file-id :name "foobar" - :is-shared false - :components-v2 true} - out (th/command! data)] + :is-shared false} + out (th/command! data) + _ (t/is (nil? (:error out))) + file-id (:id (:result out))] ;; (th/print-result! out) - (t/is (nil? (:error out))) - (let [result (:result out)] (t/is (= (:name data) (:name result))) - (t/is (= proj-id (:project-id result))))) + (t/is (= proj-id (:project-id result)))) - (let [data {::th/type :delete-file - :id file-id - ::rpc/profile-id (:id prof)} - out (th/command! data)] - ;; (th/print-result! out) - (t/is (nil? (:error out))) - (t/is (nil? (:result out)))) + (let [data {::th/type :delete-file + :id file-id + ::rpc/profile-id (:id prof)} + out (th/command! data)] + ;; (th/print-result! out) + (t/is (nil? (:error out))) + (t/is (nil? (:result out)))) - ;; get deleted files - (let [data {::th/type :get-team-deleted-files - ::rpc/profile-id (:id prof) - :team-id team-id} - out (th/command! data)] - ;; (th/print-result! out) - (t/is (nil? (:error out))) - (let [[row1 :as result] (:result out)] - (t/is (= 1 (count result))) - (t/is (= (:will-be-deleted-at row1) #penpot/inst "2025-11-07T00:00:00Z")) - (t/is (= (:created-at row1) #penpot/inst "2025-10-31T00:00:00Z")) - (t/is (= (:modified-at row1) #penpot/inst "2025-10-31T00:00:00Z")))) + ;; get deleted files + (let [data {::th/type :get-team-deleted-files + ::rpc/profile-id (:id prof) + :team-id team-id} + out (th/command! data)] + ;; (th/print-result! out) + (t/is (nil? (:error out))) + (let [[row1 :as result] (:result out)] + (t/is (= 1 (count result))) + (t/is (= (:will-be-deleted-at row1) #penpot/inst "2025-11-07T00:00:00Z")) + (t/is (= (:created-at row1) #penpot/inst "2025-10-31T00:00:00Z")) + (t/is (= (:modified-at row1) #penpot/inst "2025-10-31T00:00:00Z")))) - (let [data {::th/type :restore-deleted-team-files - ::rpc/profile-id (:id prof) - :team-id team-id - :ids #{file-id}} - out (th/command! data)] - ;; (th/print-result! out) - (t/is (nil? (:error out))) - (let [result (:result out)] - (t/is (fn? result)) + (let [data {::th/type :restore-deleted-team-files + ::rpc/profile-id (:id prof) + :team-id team-id + :ids #{file-id}} + out (th/command! data)] + ;; (th/print-result! out) + (t/is (nil? (:error out))) + (let [result (:result out)] + (t/is (fn? result)) - (let [events (th/consume-sse result)] - ;; (pp/pprint events) - (t/is (= 2 (count events))) - (t/is (= :end (first (last events)))) - (t/is (= (:ids data) (last (last events))))))) + (let [events (th/consume-sse result)] + ;; (pp/pprint events) + (t/is (= 2 (count events))) + (t/is (= :end (first (last events)))) + (t/is (= (:ids data) (last (last events))))))) - (let [row (th/db-exec-one! ["select * from file where id = ?" file-id])] - (t/is (nil? (:deleted-at row))))))) + (let [row (th/db-exec-one! ["select * from file where id = ?" file-id])] + (t/is (nil? (:deleted-at row)))))))) (t/deftest restore-deleted-files-and-projets @@ -2721,3 +2660,15 @@ (t/is (th/ex-info? err)) (t/is (= :not-found (:type edata))) (t/is (= :object-not-found (:code edata))))))) + +(t/deftest create-file-rejects-client-id + (let [prof (th/create-profile* 1 {:is-active true}) + sent-id (uuid/next) + out (th/command! {::th/type :create-file + ::rpc/profile-id (:id prof) + :project-id (:default-project-id prof) + :name "file with client id" + :id sent-id})] + (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)))) diff --git a/backend/test/backend_tests/rpc_media_test.clj b/backend/test/backend_tests/rpc_media_test.clj index 8972f26c5a..fb34396a09 100644 --- a/backend/test/backend_tests/rpc_media_test.clj +++ b/backend/test/backend_tests/rpc_media_test.clj @@ -79,6 +79,7 @@ (t/is (nil? (:error out))) (let [{:keys [media-id thumbnail-id] :as result} (:result out)] (t/is (= (:id file) (:file-id result))) + (t/is (uuid? (:id result))) (t/is (= 800 (:width result))) (t/is (= 800 (:height result))) (t/is (= "image/jpeg" (:mtype result))) @@ -93,111 +94,6 @@ (t/is (= 3890 (:size mobj2))))))) -(t/deftest media-object-upload-idempotency - (let [prof (th/create-profile* 1) - proj (th/create-project* 1 {:profile-id (:id prof) - :team-id (:default-team-id prof)}) - file (th/create-file* 1 {:profile-id (:id prof) - :project-id (:default-project-id prof) - :is-shared false}) - mfile {:filename "sample.jpg" - :path (th/tempfile "backend_tests/test_files/sample.jpg") - :mtype "image/jpeg" - :size 312043} - - params {::th/type :upload-file-media-object - ::rpc/profile-id (:id prof) - :file-id (:id file) - :is-local true - :name "testfile" - :content mfile - :id (uuid/next)}] - - ;; First try - (let [{:keys [result error] :as out} (th/command! params)] - ;; (th/print-result! out) - (t/is (nil? error)) - (t/is (= (:id params) (:id result))) - (t/is (= (:file-id params) (:file-id result))) - (t/is (= 800 (:width result))) - (t/is (= 800 (:height result))) - (t/is (= "image/jpeg" (:mtype result))) - (t/is (uuid? (:media-id result))) - (t/is (uuid? (:thumbnail-id result)))) - - ;; Second try - (let [{:keys [result error] :as out} (th/command! params)] - ;; (th/print-result! out) - (t/is (nil? error)) - (t/is (= (:id params) (:id result))) - (t/is (= (:file-id params) (:file-id result))) - (t/is (= 800 (:width result))) - (t/is (= 800 (:height result))) - (t/is (= "image/jpeg" (:mtype result))) - (t/is (uuid? (:media-id result))) - (t/is (uuid? (:thumbnail-id result)))))) - - -(t/deftest upload-file-media-object-id-version - (let [prof (th/create-profile* 1) - _ (th/create-project* 1 {:profile-id (:id prof) - :team-id (:default-team-id prof)}) - file (th/create-file* 1 {:profile-id (:id prof) - :project-id (:default-project-id prof) - :is-shared false}) - mfile {:filename "sample.jpg" - :path (th/tempfile "backend_tests/test_files/sample.jpg") - :mtype "image/jpeg" - :size 312043} - v3-id "6fa459ea-ee8a-3ca4-894e-db77e160355e" - v4-id "550e8400-e29b-41d4-a716-446655440000"] - - ;; reserved version (v3) must be rejected at the RPC boundary - (let [params {::th/type :upload-file-media-object - ::rpc/profile-id (:id prof) - :file-id (:id file) - :is-local true - :name "testfile" - :content mfile - :id v3-id} - out (th/command! params)] - (t/is (not (th/success? out))) - (t/is (th/ex-of-type? (:error out) :validation)) - (t/is (th/ex-of-code? (:error out) :params-validation))) - - ;; v4 id is accepted - (let [params {::th/type :upload-file-media-object - ::rpc/profile-id (:id prof) - :file-id (:id file) - :is-local true - :name "testfile" - :content mfile - :id v4-id} - out (th/command! params)] - (t/is (th/success? out)) - (t/is (= v4-id (str (:id (:result out)))))))) - -(t/deftest create-file-media-object-from-url-id-version - (let [prof (th/create-profile* 1) - _ (th/create-project* 1 {:profile-id (:id prof) - :team-id (:default-team-id prof)}) - file (th/create-file* 1 {:profile-id (:id prof) - :project-id (:default-project-id prof) - :is-shared false}) - v3-id "6fa459ea-ee8a-3ca4-894e-db77e160355e"] - - ;; reserved version (v3) must be rejected before any download happens - (let [params {::th/type :create-file-media-object-from-url - ::rpc/profile-id (:id prof) - :file-id (:id file) - :is-local true - :url "https://example.com/sample.jpg" - :id v3-id} - out (th/command! params)] - (t/is (not (th/success? out))) - (t/is (th/ex-of-type? (:error out) :validation)) - (t/is (th/ex-of-code? (:error out) :params-validation))))) - (t/deftest media-object-from-url-command (let [prof (th/create-profile* 1) proj (th/create-project* 1 {:profile-id (:id prof) @@ -254,6 +150,7 @@ (t/is (nil? (:error out))) (let [{:keys [media-id thumbnail-id] :as result} (:result out)] (t/is (= (:id file) (:file-id result))) + (t/is (uuid? (:id result))) (t/is (= 800 (:width result))) (t/is (= 800 (:height result))) (t/is (= "image/jpeg" (:mtype result))) @@ -268,51 +165,6 @@ (t/is (= 3890 (:size mobj2))))))) -(t/deftest media-object-upload-idempotency-command - (let [prof (th/create-profile* 1) - proj (th/create-project* 1 {:profile-id (:id prof) - :team-id (:default-team-id prof)}) - file (th/create-file* 1 {:profile-id (:id prof) - :project-id (:default-project-id prof) - :is-shared false}) - mfile {:filename "sample.jpg" - :path (th/tempfile "backend_tests/test_files/sample.jpg") - :mtype "image/jpeg" - :size 312043} - - params {::th/type :upload-file-media-object - ::rpc/profile-id (:id prof) - :file-id (:id file) - :is-local true - :name "testfile" - :content mfile - :id (uuid/next)}] - - ;; First try - (let [{:keys [result error] :as out} (th/command! params)] - ;; (th/print-result! out) - (t/is (nil? error)) - (t/is (= (:id params) (:id result))) - (t/is (= (:file-id params) (:file-id result))) - (t/is (= 800 (:width result))) - (t/is (= 800 (:height result))) - (t/is (= "image/jpeg" (:mtype result))) - (t/is (uuid? (:media-id result))) - (t/is (uuid? (:thumbnail-id result)))) - - ;; Second try - (let [{:keys [result error] :as out} (th/command! params)] - ;; (th/print-result! out) - (t/is (nil? error)) - (t/is (= (:id params) (:id result))) - (t/is (= (:file-id params) (:file-id result))) - (t/is (= 800 (:width result))) - (t/is (= 800 (:height result))) - (t/is (= "image/jpeg" (:mtype result))) - (t/is (uuid? (:media-id result))) - (t/is (uuid? (:thumbnail-id result)))))) - - (t/deftest media-object-upload-command-when-file-is-deleted (let [prof (th/create-profile* 1) proj (th/create-project* 1 {:profile-id (:id prof) @@ -580,7 +432,6 @@ file (th/create-file* 1 {:profile-id (:id prof) :project-id (:default-project-id prof) :is-shared false}) - media-id (uuid/next) source-path (th/tempfile "backend_tests/test_files/sample.jpg") chunks (split-file-into-chunks source-path 312043) ; single chunk = whole file mtype "image/jpeg" @@ -600,10 +451,9 @@ :file-id (:id file) :is-local true :name "sample" - :mtype mtype - :id media-id})] + :mtype mtype})] (t/is (nil? (:error out1))) - (t/is (= media-id (:id (:result out1))))) + (t/is (uuid? (:id (:result out1))))) ;; Second assemble with the same session-id must fail because the ;; session row has been marked as consumed after the first assembly @@ -613,8 +463,7 @@ :file-id (:id file) :is-local true :name "sample" - :mtype mtype - :id media-id})] + :mtype mtype})] (t/is (some? (:error out2))) (t/is (= :not-found (-> out2 :error ex-data :type))) (t/is (= :object-not-found (-> out2 :error ex-data :code)))))) @@ -1323,47 +1172,57 @@ (t/is (= :not-found (:type error-data))) (t/is (= :object-not-found (:code error-data)))))) -(t/deftest assemble-file-media-object-id-version +(t/deftest upload-file-media-object-rejects-client-id + (let [prof (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id prof) + :project-id (:default-project-id prof) + :is-shared false}) + mfile {:filename "sample.jpg" + :path (th/tempfile "backend_tests/test_files/sample.jpg") + :mtype "image/jpeg" + :size 312043} + sent-id (uuid/next) + 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 + :id sent-id})] + (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-file-media-object-from-url-rejects-client-id + (let [prof (th/create-profile* 1) + file (th/create-file* 1 {:profile-id (:id prof) + :project-id (:default-project-id prof) + :is-shared false}) + sent-id (uuid/next) + out (th/command! {::th/type :create-file-media-object-from-url + ::rpc/profile-id (:id prof) + :file-id (:id file) + :is-local true + :url "https://example.com/sample.jpg" + :id sent-id})] + (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 assemble-file-media-object-rejects-client-id (let [prof (th/create-profile* 1) - _ (th/create-project* 1 {:profile-id (:id prof) - :team-id (:default-team-id prof)}) file (th/create-file* 1 {:profile-id (:id prof) :project-id (:default-project-id prof) :is-shared false}) - v3-id "6fa459ea-ee8a-3ca4-894e-db77e160355e" - v4-id "550e8400-e29b-41d4-a716-446655440000" - mtype "image/jpeg"] - - ;; reserved version (v3) must be rejected without touching any session - (let [out (th/command! {::th/type :assemble-file-media-object - ::rpc/profile-id (:id prof) - :session-id (uuid/next) - :file-id (:id file) - :is-local true - :name "assembled-image" - :mtype mtype - :id v3-id})] - (t/is (not (th/success? out))) - (t/is (th/ex-of-type? (:error out) :validation)) - (t/is (th/ex-of-code? (:error out) :params-validation))) - - ;; v4 id is accepted through the full chunked flow - (let [source-path (th/tempfile "backend_tests/test_files/sample.jpg") - chunks (split-file-into-chunks source-path 312043) - session-id (create-session! prof 1) - mfile (make-chunk-mfile (first chunks) mtype) - _ (th/command! {::th/type :upload-chunk - ::rpc/profile-id (:id prof) - :session-id session-id - :index 0 - :content mfile}) - out (th/command! {::th/type :assemble-file-media-object - ::rpc/profile-id (:id prof) - :session-id session-id - :file-id (:id file) - :is-local true - :name "assembled-image" - :mtype mtype - :id v4-id})] - (t/is (th/success? out)) - (t/is (= v4-id (str (:id (:result out)))))))) + sent-id (uuid/next) + out (th/command! {::th/type :assemble-file-media-object + ::rpc/profile-id (:id prof) + :session-id (uuid/next) + :file-id (:id file) + :is-local true + :name "assembled-image" + :mtype "image/jpeg" + :id sent-id})] + (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)))) diff --git a/backend/test/backend_tests/rpc_project_test.clj b/backend/test/backend_tests/rpc_project_test.clj index 5501cf6273..f5855a43e8 100644 --- a/backend/test/backend_tests/rpc_project_test.clj +++ b/backend/test/backend_tests/rpc_project_test.clj @@ -21,20 +21,19 @@ (t/deftest projects-simple-crud (let [profile (th/create-profile* 1) team (th/create-team* 1 {:profile-id (:id profile)}) - project-id (uuid/next)] + data {::th/type :create-project + ::rpc/profile-id (:id profile) + :team-id (:id team) + :name "test project"} + out (th/command! data) + _ (t/is (nil? (:error out))) + project-id (:id (:result out))] ;; create project - (let [data {::th/type :create-project - ::rpc/profile-id (:id profile) - :id project-id - :team-id (:id team) - :name "test project"} - out (th/command! data)] - ;; (th/print-result! out) - - (t/is (nil? (:error out))) - (let [result (:result out)] - (t/is (= (:name data) (:name result))))) + ;; (th/print-result! out) + (t/is (uuid? project-id)) + (let [result (:result out)] + (t/is (= (:name data) (:name result)))) ;; query the list of projects of a team (let [data {::th/type :get-projects @@ -123,33 +122,6 @@ (t/is (th/ex-info? error)) (t/is (th/ex-of-type? error :not-found)))) -(t/deftest create-project-id-version - (let [profile (th/create-profile* 1) - team (th/create-team* 1 {:profile-id (:id profile)}) - v3-id "6fa459ea-ee8a-3ca4-894e-db77e160355e" - v4-id "550e8400-e29b-41d4-a716-446655440000"] - - ;; reserved version (v3) must be rejected at the RPC boundary - (let [data {::th/type :create-project - ::rpc/profile-id (:id profile) - :team-id (:id team) - :id v3-id - :name "project with v3 id"} - out (th/command! data)] - (t/is (not (th/success? out))) - (t/is (th/ex-of-type? (:error out) :validation)) - (t/is (th/ex-of-code? (:error out) :params-validation))) - - ;; v4 id is accepted - (let [data {::th/type :create-project - ::rpc/profile-id (:id profile) - :team-id (:id team) - :id v4-id - :name "project with v4 id"} - out (th/command! data)] - (t/is (th/success? out)) - (t/is (= v4-id (str (:id (:result out)))))))) - (t/deftest permissions-checks-rename-project (let [profile1 (th/create-profile* 1) profile2 (th/create-profile* 2) @@ -289,3 +261,16 @@ err (:error out)] (t/is (th/ex-info? err)) (t/is (th/ex-of-type? err :not-found)))) + +(t/deftest create-project-rejects-client-id + (let [profile (th/create-profile* 1 {:is-active true}) + team (th/create-team* 1 {:profile-id (:id profile)}) + sent-id (uuid/next) + out (th/command! {::th/type :create-project + ::rpc/profile-id (:id profile) + :team-id (:id team) + :name "project with client id" + :id sent-id})] + (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)))) diff --git a/backend/test/backend_tests/rpc_team_test.clj b/backend/test/backend_tests/rpc_team_test.clj index cd7d1f3dbe..b15013eb8f 100644 --- a/backend/test/backend_tests/rpc_team_test.clj +++ b/backend/test/backend_tests/rpc_team_test.clj @@ -1116,62 +1116,6 @@ out (th/command! data)] (t/is (th/success? out))))) -(t/deftest create-team-id-version - (let [profile (th/create-profile* 1 {:is-active true}) - v3-id "6fa459ea-ee8a-3ca4-894e-db77e160355e" - v4-id "550e8400-e29b-41d4-a716-446655440000" - v7-id "0191062e-3f50-7a5e-9f5a-1a2b3c4d5e6f"] - - ;; reserved version (v3) must be rejected at the RPC boundary - (let [data {::th/type :create-team - ::rpc/profile-id (:id profile) - :name "team with v3 id" - :id v3-id} - out (th/command! data)] - (t/is (not (th/success? out))) - (t/is (th/ex-of-type? (:error out) :validation)) - (t/is (th/ex-of-code? (:error out) :params-validation))) - - ;; v4, v7 and v8 ids are accepted - (doseq [id [v4-id v7-id (str (uuid/next))]] - (let [data {::th/type :create-team - ::rpc/profile-id (:id profile) - :name (str "team with id " id) - :id id} - out (th/command! data)] - (t/is (th/success? out)) - (t/is (= id (str (:id (:result out))))))))) - -(t/deftest create-team-with-invitations-id-version - (with-mocks [mock {:target 'app.email/send! :return nil}] - (let [profile (th/create-profile* 1 {:is-active true}) - v3-id "6fa459ea-ee8a-3ca4-894e-db77e160355e" - v4-id "550e8400-e29b-41d4-a716-446655440000"] - - ;; reserved version (v3) must be rejected before any invitation is sent - (let [data {::th/type :create-team-with-invitations - ::rpc/profile-id (:id profile) - :name "team with v3 id" - :id v3-id - :emails #{"invitee@example.com"} - :role :editor} - out (th/command! data)] - (t/is (not (th/success? out))) - (t/is (th/ex-of-type? (:error out) :validation)) - (t/is (th/ex-of-code? (:error out) :params-validation)) - (t/is (= 0 (:call-count @mock)))) - - ;; v4 id is accepted - (let [data {::th/type :create-team-with-invitations - ::rpc/profile-id (:id profile) - :name "team with v4 id" - :id v4-id - :emails #{"invitee@example.com"} - :role :editor} - out (th/command! data)] - (t/is (th/success? out)) - (t/is (= v4-id (str (:id (:result out))))))))) - (t/deftest create-team-invitations-email-cooldown (with-mocks [mock {:target 'app.email/send! :return nil}] (let [profile1 (th/create-profile* 1 {:is-active true}) @@ -1482,3 +1426,27 @@ (t/is (not (th/success? out))) (t/is (th/ex-of-type? (:error out) :not-found)) (t/is (th/ex-of-code? (:error out) :member-does-not-exist))))) + +(t/deftest create-team-rejects-client-id + (let [profile (th/create-profile* 1 {:is-active true}) + sent-id (uuid/next) + out (th/command! {::th/type :create-team + ::rpc/profile-id (:id profile) + :name "team with client id" + :id sent-id})] + (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-team-with-invitations-rejects-client-id + (let [profile (th/create-profile* 1 {:is-active true}) + sent-id (uuid/next) + out (th/command! {::th/type :create-team-with-invitations + ::rpc/profile-id (:id profile) + :name "team with client id" + :emails ["invitee@example.com"] + :role :editor + :id sent-id})] + (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)))) diff --git a/frontend/src/app/main/data/workspace/svg_upload.cljs b/frontend/src/app/main/data/workspace/svg_upload.cljs index a65b232363..35ff91c272 100644 --- a/frontend/src/app/main/data/workspace/svg_upload.cljs +++ b/frontend/src/app/main/data/workspace/svg_upload.cljs @@ -53,10 +53,13 @@ (or (str/starts-with? url "http://") (str/starts-with? url "https://")))))) (rx/mapcat (fn [item] + ;; The upload commands use closed params schemas, so + ;; drop the svg-only attrs (:href :width :height) that + ;; `svg/collect-images` attaches. (->> (rp/cmd! (if (contains? item :content) :upload-file-media-object :create-file-media-object-from-url) - (dissoc item :href)) + (dissoc item :href :width :height)) ;; When the image uploaded fail we skip the shape ;; returning `nil` will afterward not create the shape. (rx/catch #(rx/of nil)) diff --git a/frontend/test/frontend_tests/data/svg_upload_test.cljs b/frontend/test/frontend_tests/data/svg_upload_test.cljs new file mode 100644 index 0000000000..bb768d7137 --- /dev/null +++ b/frontend/test/frontend_tests/data/svg_upload_test.cljs @@ -0,0 +1,63 @@ +;; 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 frontend-tests.data.svg-upload-test + "Tests for the media upload triggered from an SVG import." + (:require + [app.common.transit :as tr] + [app.common.uuid :as uuid] + [app.main.data.workspace.svg-upload :as svg] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.http :as http])) + +(defn- image-node + [href] + {:tag :image + :attrs {:href href :width "10" :height "20"} + :content []}) + +(t/deftest upload-images-drops-svg-only-attrs + (t/testing "media upload params do not carry svg-only attrs" + (t/async done + (let [file-id (uuid/next) + bodies (atom []) + + fetch-mock + (fn [url opts] + (swap! bodies conj {:cmd (http/url->cmd url) + :body (.-body opts)}) + (js/Promise.resolve + (http/make-transit-response + {:id (uuid/next) + :file-id file-id + :name "pic" + :mtype "image/png" + :width 10 + :height 20}))) + + orig (http/install-fetch-mock! fetch-mock) + svg-data {:tag :svg + :attrs {} + :content [(image-node "https://example.com/pic.png")]}] + + (->> (svg/upload-images svg-data file-id) + (rx/subs! + (fn [_] nil) + (fn [err] + (http/restore-fetch! orig) + (t/is false (str "unexpected error: " (ex-message err))) + (done)) + (fn [] + (http/restore-fetch! orig) + (let [{:keys [cmd body]} (first @bodies) + params (tr/decode-str body)] + (t/is (= :create-file-media-object-from-url cmd)) + (t/is (= "https://example.com/pic.png" (:url params))) + (t/is (not (contains? params :href))) + (t/is (not (contains? params :width))) + (t/is (not (contains? params :height)))) + (done)))))))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index 8cf3b8162b..51690bbb32 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -16,6 +16,7 @@ [frontend-tests.data.profile-test] [frontend-tests.data.repo-test] [frontend-tests.data.store-test] + [frontend-tests.data.svg-upload-test] [frontend-tests.data.uploads-test] [frontend-tests.data.viewer-test] [frontend-tests.data.workspace-colors-test] @@ -134,6 +135,7 @@ 'frontend-tests.data.repo-test 'frontend-tests.data.store-test 'frontend-tests.data.exports-assets-test + 'frontend-tests.data.svg-upload-test 'frontend-tests.data.uploads-test 'frontend-tests.data.viewer-test 'frontend-tests.data.workspace-colors-test