🐛 Fix issues on files and project rpc methods.

backend/src/app/rpc/mutations/files.clj

@@ -155,6 +155,7 @@
               :hint "A file cannot be linked to itself"))
   (db/with-atomic [conn pool]
     (files/check-edition-permissions! conn profile-id file-id)
+    (files/check-edition-permissions! conn profile-id library-id)
     (link-file-to-library conn params)))
 
 (def sql:link-file-to-library

backend/src/app/rpc/mutations/projects.clj

@@ -14,6 +14,7 @@
    [app.config :as cfg]
    [app.db :as db]
    [app.rpc.queries.projects :as proj]
+   [app.rpc.queries.teams :as teams]
    [app.tasks :as tasks]
    [app.util.services :as sv]
    [app.util.time :as dt]
@@ -38,13 +39,14 @@
           :opt-un [::id]))
 
 (sv/defmethod ::create-project
-  [{:keys [pool] :as cfg} params]
+  [{:keys [pool] :as cfg} {:keys [profile-id team-id] :as params}]
   (db/with-atomic [conn pool]
-    (let [proj   (create-project conn params)
+    (teams/check-edition-permissions! conn profile-id team-id)
-          params (assoc params :project-id (:id proj))]
+    (let [project (create-project conn params)
+          params  (assoc params :project-id (:id project))]
       (create-project-profile conn params)
       (create-team-project-profile conn params)
-      (assoc proj :is-pinned true))))
+      (assoc project :is-pinned true))))
 
 (defn create-project
   [conn {:keys [id team-id name default?] :as params}]
@@ -92,6 +94,7 @@
 (sv/defmethod ::update-project-pin
   [{:keys [pool] :as cfg} {:keys [id profile-id team-id is-pinned] :as params}]
   (db/with-atomic [conn pool]
+    (proj/check-edition-permissions! conn profile-id id)
     (db/exec-one! conn [sql:update-project-pin team-id id profile-id is-pinned is-pinned])
     nil))
 

backend/tests/app/tests/helpers.clj

@@ -39,12 +39,11 @@
 (def ^:dynamic *pool* nil)
 
 (def config
-  (merge {:redis-uri "redis://redis/1"
+  (merge cfg/config
+         {:redis-uri "redis://redis/1"
           :database-uri "postgresql://postgres/penpot_test"
           :storage-fs-directory "/tmp/app/storage"
-          :migrations-verbose false}
+          :migrations-verbose false}))
-         cfg/config))
-
 
 (defn state-init
   [next]
@@ -108,48 +107,7 @@
   [prefix & args]
   (uuid/namespaced uuid/zero (apply str prefix args)))
 
-
+;; --- FACTORIES
-(defn create-profile
-  [conn i]
-  (let [params {:id (mk-uuid "profile" i)
-                :fullname (str "Profile " i)
-                :email (str "profile" i ".test@nodomain.com")
-                :password "123123"
-                :is-demo true}]
-    (->> (#'profile/create-profile conn params)
-         (#'profile/create-profile-relations conn))))
-
-(defn create-team
-  [conn profile-id i]
-  (let [id (mk-uuid "team" i)
-        team (#'teams/create-team conn {:id id
-                                        :profile-id profile-id
-                                        :name (str "team" i)})]
-    (#'teams/create-team-profile conn
-                                 {:team-id id
-                                  :profile-id profile-id
-                                  :is-owner true
-                                  :is-admin true
-                                  :can-edit true})
-    team))
-
-(defn create-project
-  [conn profile-id team-id i]
-  (#'projects/create-project conn {:id (mk-uuid "project" i)
-                                   :profile-id profile-id
-                                   :team-id team-id
-                                   :name (str "project" i)}))
-
-(defn create-file
-  [conn profile-id project-id is-shared i]
-  (#'files/create-file conn {:id (mk-uuid "file" i)
-                             :profile-id profile-id
-                             :project-id project-id
-                             :is-shared is-shared
-                             :name (str "file" i)}))
-
-
-;; --- NEW HELPERS
 
 (defn create-profile*
   ([i] (create-profile* *pool* i {}))
@@ -202,6 +160,60 @@
                                    :can-edit true})
      team)))
 
+(defn link-file-to-library*
+  ([params] (link-file-to-library* *pool* params))
+  ([conn {:keys [file-id library-id] :as params}]
+   (#'files/link-file-to-library conn {:file-id file-id :library-id library-id})))
+
+(defn create-complaint-for
+  [conn {:keys [id created-at type]}]
+  (db/insert! conn :profile-complaint-report
+              {:profile-id id
+               :created-at (or created-at (dt/now))
+               :type (name type)
+               :content (db/tjson {})}))
+
+(defn create-global-complaint-for
+  [conn {:keys [email type created-at]}]
+  (db/insert! conn :global-complaint-report
+              {:email email
+               :type (name type)
+               :created-at (or created-at (dt/now))
+               :content (db/tjson {})}))
+
+
+(defn create-team-permission*
+  ([params] (create-team-permission* *pool* params))
+  ([conn {:keys [team-id profile-id is-owner is-admin can-edit]
+          :or {is-owner true is-admin true can-edit true}}]
+   (db/insert! conn :team-profile-rel {:team-id team-id
+                                       :profile-id profile-id
+                                       :is-owner is-owner
+                                       :is-admin is-admin
+                                       :can-edit can-edit})))
+
+(defn create-project-permission*
+  ([params] (create-project-permission* *pool* params))
+  ([conn {:keys [project-id profile-id is-owner is-admin can-edit]
+          :or {is-owner true is-admin true can-edit true}}]
+   (db/insert! conn :project-profile-rel {:project-id project-id
+                                          :profile-id profile-id
+                                          :is-owner is-owner
+                                          :is-admin is-admin
+                                          :can-edit can-edit})))
+
+(defn create-file-permission*
+  ([params] (create-file-permission* *pool* params))
+  ([conn {:keys [file-id profile-id is-owner is-admin can-edit]
+          :or {is-owner true is-admin true can-edit true}}]
+   (db/insert! conn :project-profile-rel {:file-id file-id
+                                          :profile-id profile-id
+                                          :is-owner is-owner
+                                          :is-admin is-admin
+                                          :can-edit can-edit})))
+
+
+;; --- RPC HELPERS
 
 (defn handle-error
   [^Throwable err]
@@ -209,14 +221,6 @@
     (handle-error (.getCause err))
     err))
 
-(defmacro try-on
-  [expr]
-  `(try
-     (let [result# (deref ~expr)]
-       [nil result#])
-     (catch Exception e#
-       [(handle-error e#) nil])))
-
 (defmacro try-on!
   [expr]
   `(try
@@ -226,16 +230,6 @@
        {:error (handle-error e#)
         :result nil})))
 
-(defmacro try!
-  [expr]
-  `(try
-     {:error nil
-      :result ~expr}
-     (catch Exception e#
-       {:error (handle-error e#)
-        :result nil})))
-
-
 (defn mutation!
   [{:keys [::type] :as data}]
   (let [method-fn (get-in *system* [:app.rpc/rpc :methods :mutation type])]
@@ -248,7 +242,7 @@
     (try-on!
      (method-fn (dissoc data ::type)))))
 
-;; --- Utils
+;; --- UTILS
 
 (defn print-error!
   [error]
@@ -317,23 +311,6 @@
     ([key] (get (merge config data) key))
     ([key default] (get (merge config data) key default))))
 
-(defn create-complaint-for
-  [conn {:keys [id created-at type]}]
-  (db/insert! conn :profile-complaint-report
-              {:profile-id id
-               :created-at (or created-at (dt/now))
-               :type (name type)
-               :content (db/tjson {})}))
-
-(defn create-global-complaint-for
-  [conn {:keys [email type created-at]}]
-  (db/insert! conn :global-complaint-report
-              {:email email
-               :type (name type)
-               :created-at (or created-at (dt/now))
-               :content (db/tjson {})}))
-
-
 (defn reset-mock!
   [m]
   (reset! m @(mk/make-mock {})))

backend/tests/app/tests/test_services_files.clj

@@ -120,102 +120,209 @@
           (t/is (= 0 (count result))))))
     ))
 
-(defn- create-file-media-object
-  [{:keys [profile-id file-id]}]
-  (let [mfile  {:filename "sample.jpg"
-                :tempfile (th/tempfile "app/tests/_files/sample.jpg")
-                :content-type "image/jpeg"
-                :size 312043}
-        params {::th/type :upload-file-media-object
-                :profile-id profile-id
-                :file-id file-id
-                :is-local true
-                :name "testfile"
-                :content mfile}
-        out    (th/mutation! params)]
-    (t/is (nil? (:error out)))
-    (:result out)))
-
-(defn- update-file
-  [{:keys [profile-id file-id changes revn] :or {revn 0}}]
-  (let [params {::th/type :update-file
-                :id file-id
-                :session-id (uuid/random)
-                :profile-id profile-id
-                :revn revn
-                :changes changes}
-        out    (th/mutation! params)]
-    (t/is (nil? (:error out)))
-    (:result out)))
-
 (t/deftest file-media-gc-task
-  (let [task    (:app.tasks.file-media-gc/handler th/*system*)
+  (letfn [(create-file-media-object [{:keys [profile-id file-id]}]
-        storage (:app.storage/storage th/*system*)
+            (let [mfile  {:filename "sample.jpg"
+                          :tempfile (th/tempfile "app/tests/_files/sample.jpg")
+                          :content-type "image/jpeg"
+                          :size 312043}
+                  params {::th/type :upload-file-media-object
+                          :profile-id profile-id
+                          :file-id file-id
+                          :is-local true
+                          :name "testfile"
+                          :content mfile}
+                  out    (th/mutation! params)]
+              (t/is (nil? (:error out)))
+              (:result out)))
 
-        prof    (th/create-profile* 1)
+          (update-file [{:keys [profile-id file-id changes revn] :or {revn 0}}]
-        proj    (th/create-project* 1 {:profile-id (:id prof)
+            (let [params {::th/type :update-file
-                                       :team-id (:default-team-id prof)})
+                          :id file-id
-        file    (th/create-file* 1 {:profile-id (:id prof)
+                          :session-id (uuid/random)
-                                    :project-id (:default-project-id prof)
+                          :profile-id profile-id
-                                    :is-shared false})
+                          :revn revn
+                          :changes changes}
+                  out    (th/mutation! params)]
+              (t/is (nil? (:error out)))
+              (:result out)))]
 
-        fmo1    (create-file-media-object {:profile-id (:id prof)
+    (let [storage (:app.storage/storage th/*system*)
-                                           :file-id (:id file)})
-        fmo2    (create-file-media-object {:profile-id (:id prof)
-                                           :file-id (:id file)})
-        shid    (uuid/random)
 
-        ures    (update-file
+          profile (th/create-profile* 1)
-                 {:file-id (:id file)
+          file    (th/create-file* 1 {:profile-id (:id profile)
-                  :profile-id (:id prof)
+                                      :project-id (:default-project-id profile)
-                  :revn 0
+                                      :is-shared false})
-                  :changes
-                  [{:type :add-obj
-                    :page-id (first (get-in file [:data :pages]))
-                    :id shid
-                    :parent-id uuid/zero
-                    :frame-id uuid/zero
-                    :obj {:id shid
-                          :name "image"
-                          :frame-id uuid/zero
-                          :parent-id uuid/zero
-                          :type :image
-                          :metadata {:id (:id fmo1)}}}]})]
 
-    ;; run the task inmediatelly
+          fmo1    (create-file-media-object {:profile-id (:id profile)
-    (let [res (task {})]
+                                             :file-id (:id file)})
-      (t/is (= 0 (:processed res))))
+          fmo2    (create-file-media-object {:profile-id (:id profile)
+                                             :file-id (:id file)})
+          shid    (uuid/random)
 
-    ;; make the file ellegible for GC waiting 300ms
+          ures    (update-file
-    (th/sleep 300)
+                   {:file-id (:id file)
+                    :profile-id (:id profile)
+                    :revn 0
+                    :changes
+                    [{:type :add-obj
+                      :page-id (first (get-in file [:data :pages]))
+                      :id shid
+                      :parent-id uuid/zero
+                      :frame-id uuid/zero
+                      :obj {:id shid
+                            :name "image"
+                            :frame-id uuid/zero
+                            :parent-id uuid/zero
+                            :type :image
+                            :metadata {:id (:id fmo1)}}}]})]
 
-    ;; run the task again
+      ;; run the task inmediatelly
-    (let [res (task {})]
+      (let [task  (:app.tasks.file-media-gc/handler th/*system*)
-      (t/is (= 1 (:processed res))))
+            res   (task {})]
+        (t/is (= 0 (:processed res))))
 
-    ;; Retrieve file and check trimmed attribute
+      ;; make the file ellegible for GC waiting 300ms (configured
-    (let [row (db/exec-one! th/*pool* ["select * from file where id = ?" (:id file)])]
+      ;; timeout for testing)
-      (t/is (:has-media-trimmed row)))
+      (th/sleep 300)
 
-    ;; check file media objects
+      ;; run the task again
-    (let [fmos (db/exec! th/*pool* ["select * from file_media_object where file_id = ?" (:id file)])]
+      (let [task  (:app.tasks.file-media-gc/handler th/*system*)
-      (t/is (= 1 (count fmos))))
+            res   (task {})]
+        (t/is (= 1 (:processed res))))
 
-    ;; The underlying storage objects are still available.
+      ;; retrieve file and check trimmed attribute
-    (t/is (some? (sto/get-object storage (:media-id fmo2))))
+      (let [row (db/exec-one! th/*pool* ["select * from file where id = ?" (:id file)])]
-    (t/is (some? (sto/get-object storage (:thumbnail-id fmo2))))
+        (t/is (true? (:has-media-trimmed row))))
-    (t/is (some? (sto/get-object storage (:media-id fmo1))))
-    (t/is (some? (sto/get-object storage (:thumbnail-id fmo1))))
 
-    ;; but if we pass the touched gc task two of them should disappear
+      ;; check file media objects
-    (let [task (:app.storage/gc-touched-task th/*system*)
+      (let [rows (db/exec! th/*pool* ["select * from file_media_object where file_id = ?" (:id file)])]
-          res  (task {})]
+        (t/is (= 1 (count rows))))
-      (t/is (= 0 (:freeze res)))
-      (t/is (= 2 (:delete res)))
 
-      (t/is (nil? (sto/get-object storage (:media-id fmo2))))
+      ;; The underlying storage objects are still available.
-      (t/is (nil? (sto/get-object storage (:thumbnail-id fmo2))))
+      (t/is (some? (sto/get-object storage (:media-id fmo2))))
+      (t/is (some? (sto/get-object storage (:thumbnail-id fmo2))))
       (t/is (some? (sto/get-object storage (:media-id fmo1))))
-      (t/is (some? (sto/get-object storage (:thumbnail-id fmo1)))))
+      (t/is (some? (sto/get-object storage (:thumbnail-id fmo1))))
+
+      ;; but if we pass the touched gc task two of them should disappear
+      (let [task (:app.storage/gc-touched-task th/*system*)
+            res  (task {})]
+        (t/is (= 0 (:freeze res)))
+        (t/is (= 2 (:delete res)))
+
+        (t/is (nil? (sto/get-object storage (:media-id fmo2))))
+        (t/is (nil? (sto/get-object storage (:thumbnail-id fmo2))))
+        (t/is (some? (sto/get-object storage (:media-id fmo1))))
+        (t/is (some? (sto/get-object storage (:thumbnail-id fmo1)))))
+
+      )))
+
+(t/deftest permissions-checks-creating-file
+  (let [profile1 (th/create-profile* 1)
+        profile2 (th/create-profile* 2)
+
+        data     {::th/type :create-file
+                  :profile-id (:id profile2)
+                  :project-id (:default-project-id profile1)
+                  :name "foobar"
+                  :is-shared false}
+        out      (th/mutation! data)
+        error    (:error out)]
+
+    ;; (th/print-result! out)
+    (t/is (th/ex-info? error))
+    (t/is (th/ex-of-type? error :not-found))))
+
+(t/deftest permissions-checks-rename-file
+  (let [profile1 (th/create-profile* 1)
+        profile2 (th/create-profile* 2)
+
+        file     (th/create-file* 1 {:project-id (:default-project-id profile1)
+                                     :profile-id (:id profile1)})
+        data     {::th/type :rename-file
+                  :id (:id file)
+                  :profile-id (:id profile2)
+                  :name "foobar"}
+        out      (th/mutation! data)
+        error    (:error out)]
+
+    ;; (th/print-result! out)
+    (t/is (th/ex-info? error))
+    (t/is (th/ex-of-type? error :not-found))))
+
+(t/deftest permissions-checks-delete-file
+  (let [profile1 (th/create-profile* 1)
+        profile2 (th/create-profile* 2)
+
+        file     (th/create-file* 1 {:project-id (:default-project-id profile1)
+                                     :profile-id (:id profile1)})
+        data     {::th/type :delete-file
+                  :profile-id (:id profile2)
+                  :id (:id file)}
+        out      (th/mutation! data)
+        error    (:error out)]
+
+    ;; (th/print-result! out)
+    (t/is (th/ex-info? error))
+    (t/is (th/ex-of-type? error :not-found))))
+
+(t/deftest permissions-checks-set-file-shared
+  (let [profile1 (th/create-profile* 1)
+        profile2 (th/create-profile* 2)
+        file     (th/create-file* 1 {:project-id (:default-project-id profile1)
+                                     :profile-id (:id profile1)})
+        data     {::th/type :set-file-shared
+                  :profile-id (:id profile2)
+                  :id (:id file)
+                  :is-shared true}
+        out      (th/mutation! data)
+        error    (:error out)]
+
+    ;; (th/print-result! out)
+    (t/is (th/ex-info? error))
+    (t/is (th/ex-of-type? error :not-found))))
+
+(t/deftest permissions-checks-link-to-library-1
+  (let [profile1 (th/create-profile* 1)
+        profile2 (th/create-profile* 2)
+        file1    (th/create-file* 1 {:project-id (:default-project-id profile1)
+                                     :profile-id (:id profile1)
+                                     :is-shared true})
+        file2    (th/create-file* 2 {:project-id (:default-project-id profile1)
+                                     :profile-id (:id profile1)})
+
+        data     {::th/type :link-file-to-library
+                  :profile-id (:id profile2)
+                  :file-id (:id file2)
+                  :library-id (:id file1)}
+
+        out      (th/mutation! data)
+        error    (:error out)]
+
+      ;; (th/print-result! out)
+      (t/is (th/ex-info? error))
+      (t/is (th/ex-of-type? error :not-found))))
+
+(t/deftest permissions-checks-link-to-library-2
+  (let [profile1 (th/create-profile* 1)
+        profile2 (th/create-profile* 2)
+        file1    (th/create-file* 1 {:project-id (:default-project-id profile1)
+                                     :profile-id (:id profile1)
+                                     :is-shared true})
+
+        file2    (th/create-file* 2 {:project-id (:default-project-id profile2)
+                                     :profile-id (:id profile2)})
+
+        data     {::th/type :link-file-to-library
+                  :profile-id (:id profile2)
+                  :file-id (:id file2)
+                  :library-id (:id file1)}
+
+        out      (th/mutation! data)
+        error    (:error out)]
+
+      ;; (th/print-result! out)
+      (t/is (th/ex-info? error))
+      (t/is (th/ex-of-type? error :not-found))))
 
-    ))

backend/tests/app/tests/test_services_projects.clj

@@ -19,15 +19,15 @@
 (t/use-fixtures :once th/state-init)
 (t/use-fixtures :each th/database-reset)
 
-(t/deftest projects-crud
+(t/deftest projects-simple-crud
-  (let [prof       (th/create-profile* 1)
+  (let [profile    (th/create-profile* 1)
-        team       (th/create-team* 1 {:profile-id (:id prof)})
+        team       (th/create-team* 1 {:profile-id (:id profile)})
         project-id (uuid/next)]
 
     ;; crate project
     (let [data {::th/type :create-project
                 :id project-id
-                :profile-id (:id prof)
+                :profile-id (:id profile)
                 :team-id (:id team)
                 :name "test project"}
           out  (th/mutation! data)]
@@ -40,7 +40,7 @@
     ;; query a list of projects
     (let [data {::th/type :projects
                 :team-id (:id team)
-                :profile-id (:id prof)}
+                :profile-id (:id profile)}
           out  (th/query! data)]
       ;; (th/print-result! out)
 
@@ -54,7 +54,7 @@
     (let [data {::th/type :rename-project
                 :id project-id
                 :name "renamed project"
-                :profile-id (:id prof)}
+                :profile-id (:id profile)}
           out  (th/mutation! data)]
       ;; (th/print-result! out)
       (t/is (nil? (:error out)))
@@ -63,7 +63,7 @@
     ;; retrieve project
     (let [data {::th/type :project
                 :id project-id
-                :profile-id (:id prof)}
+                :profile-id (:id profile)}
           out  (th/query! data)]
       ;; (th/print-result! out)
       (t/is (nil? (:error out)))
@@ -73,7 +73,7 @@
     ;; delete project
     (let [data {::th/type :delete-project
                 :id project-id
-                :profile-id (:id prof)}
+                :profile-id (:id profile)}
           out  (th/mutation! data)]
 
         ;; (th/print-result! out)
@@ -83,10 +83,75 @@
     ;; query a list of projects after delete"
     (let [data {::th/type :projects
                 :team-id (:id team)
-                :profile-id (:id prof)}
+                :profile-id (:id profile)}
           out (th/query! data)]
       ;; (th/print-result! out)
       (t/is (nil? (:error out)))
       (let [result (:result out)]
         (t/is (= 0 (count result)))))
   ))
+
+(t/deftest permissions-checks-create-project
+  (let [profile1 (th/create-profile* 1)
+        profile2 (th/create-profile* 2)
+
+        data     {::th/type :create-project
+                  :profile-id (:id profile2)
+                  :team-id (:default-team-id profile1)
+                  :name "test project"}
+        out      (th/mutation! data)
+        error    (:error out)]
+
+    ;; (th/print-result! out)
+    (t/is (th/ex-info? error))
+    (t/is (th/ex-of-type? error :not-found))))
+
+(t/deftest permissions-checks-rename-project
+  (let [profile1 (th/create-profile* 1)
+        profile2 (th/create-profile* 2)
+        project  (th/create-project* 1 {:team-id (:default-team-id profile1)
+                                        :profile-id (:id profile1)})
+        data     {::th/type :rename-project
+                  :id (:id project)
+                  :profile-id (:id profile2)
+                  :name "foobar"}
+        out      (th/mutation! data)
+        error    (:error out)]
+
+    ;; (th/print-result! out)
+    (t/is (th/ex-info? error))
+    (t/is (th/ex-of-type? error :not-found))))
+
+(t/deftest permissions-checks-delete-project
+  (let [profile1 (th/create-profile* 1)
+        profile2 (th/create-profile* 2)
+        project  (th/create-project* 1 {:team-id (:default-team-id profile1)
+                                        :profile-id (:id profile1)})
+        data     {::th/type :delete-project
+                  :id (:id project)
+                  :profile-id (:id profile2)}
+        out      (th/mutation! data)
+        error    (:error out)]
+
+    ;; (th/print-result! out)
+    (t/is (th/ex-info? error))
+    (t/is (th/ex-of-type? error :not-found))))
+
+(t/deftest permissions-checks-delete-project
+  (let [profile1 (th/create-profile* 1)
+        profile2 (th/create-profile* 2)
+        project  (th/create-project* 1 {:team-id (:default-team-id profile1)
+                                        :profile-id (:id profile1)})
+        data     {::th/type :update-project-pin
+                  :id (:id project)
+                  :team-id (:default-team-id profile1)
+                  :profile-id (:id profile2)
+                  :is-pinned true}
+
+        out      (th/mutation! data)
+        error    (:error out)]
+
+    ;; (th/print-result! out)
+    (t/is (th/ex-info? error))
+    (t/is (th/ex-of-type? error :not-found))))
+