✨ Make permissions subsystem more flexible.

And fix some bugs related to permissions.

backend/src/app/rpc.clj

@@ -169,7 +169,6 @@
                      'app.rpc.mutations.files
                      'app.rpc.mutations.comments
                      'app.rpc.mutations.projects
-                     'app.rpc.mutations.viewer
                      'app.rpc.mutations.teams
                      'app.rpc.mutations.management
                      'app.rpc.mutations.ldap

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

@@ -132,8 +132,8 @@
 (sv/defmethod ::delete-team
   [{:keys [pool] :as cfg} {:keys [id profile-id] :as params}]
   (db/with-atomic [conn pool]
-    (let [perms (teams/check-edition-permissions! conn profile-id id)]
+    (let [perms (teams/get-permissions conn profile-id id)]
-      (when-not (some :is-owner perms)
+      (when-not (:is-owner perms)
         (ex/raise :type :validation
                   :code :only-owner-can-delete-team))
 
@@ -300,7 +300,7 @@
 (sv/defmethod ::invite-team-member
   [{:keys [pool tokens] :as cfg} {:keys [profile-id team-id email role] :as params}]
   (db/with-atomic [conn pool]
-    (let [perms    (teams/check-edition-permissions! conn profile-id team-id)
+    (let [perms    (teams/get-permissions conn profile-id team-id)
           profile  (db/get-by-id conn :profile profile-id)
           member   (profile/retrieve-profile-data-by-email conn email)
           team     (db/get-by-id conn :team team-id)
@@ -316,7 +316,7 @@
                            {:iss :profile-identity
                             :profile-id (:id profile)})]
 
-      (when-not (some :is-admin perms)
+      (when-not (:is-admin perms)
         (ex/raise :type :validation
                   :code :insufficient-permissions))
 

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

@@ -1,49 +0,0 @@
-;; 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) UXBOX Labs SL
-
-(ns app.rpc.mutations.viewer
-  (:require
-   [app.common.spec :as us]
-   [app.db :as db]
-   [app.rpc.queries.files :as files]
-   [app.util.services :as sv]
-   [buddy.core.codecs :as bc]
-   [buddy.core.nonce :as bn]
-   [clojure.spec.alpha :as s]))
-
-(s/def ::profile-id ::us/uuid)
-(s/def ::file-id ::us/uuid)
-(s/def ::page-id ::us/uuid)
-
-(s/def ::create-file-share-token
-  (s/keys :req-un [::profile-id ::file-id ::page-id]))
-
-(sv/defmethod ::create-file-share-token
-  [{:keys [pool] :as cfg} {:keys [profile-id file-id page-id] :as params}]
-  (db/with-atomic [conn pool]
-    (files/check-edition-permissions! conn profile-id file-id)
-    (let [token (-> (bn/random-bytes 16)
-                    (bc/bytes->b64u)
-                    (bc/bytes->str))]
-      (db/insert! conn :file-share-token
-                  {:file-id file-id
-                   :page-id page-id
-                   :token token})
-      {:token token})))
-
-
-(s/def ::token ::us/not-empty-string)
-(s/def ::delete-file-share-token
-  (s/keys :req-un [::profile-id ::file-id ::token]))
-
-(sv/defmethod ::delete-file-share-token
-  [{:keys [pool] :as cfg} {:keys [profile-id file-id token]}]
-  (db/with-atomic [conn pool]
-    (files/check-edition-permissions! conn profile-id file-id)
-    (db/delete! conn :file-share-token
-                {:file-id file-id
-                 :token token})
-    nil))

backend/src/app/rpc/permissions.clj

@@ -41,59 +41,24 @@
   "A simple factory for edition permission predicate functions."
   [qfn]
   (us/assert fn? qfn)
-  (fn [& args]
+  (fn check
-    (let [rows (apply qfn args)]
+    ([perms] (:can-edit perms))
-      (when-not (or (empty? rows)
+    ([conn & args] (check (apply qfn conn args)))))
-                    (not (or (some :can-edit rows)
-                             (some :is-admin rows)
-                             (some :is-owner rows))))
-        rows))))
 
 (defn make-read-predicate-fn
   "A simple factory for read permission predicate functions."
   [qfn]
   (us/assert fn? qfn)
-  (fn [& args]
+  (fn check
-    (let [rows (apply qfn args)]
+    ([perms] (:can-read perms))
-      (when (seq rows)
+    ([conn & args] (check (apply qfn conn args)))))
-        rows))))
 
 (defn make-check-fn
   "Helper that converts a predicate permission function to a check
   function (function that raises an exception)."
   [pred]
   (fn [& args]
-    (when-not (seq (apply pred args))
+    (when-not (apply pred args)
       (ex/raise :type :not-found
                 :code :object-not-found
                 :hint "not found"))))
-
-
-;; TODO: the following functions are deprecated and replaced with the
-;; new ones. Should not be used.
-
-(defn make-edition-check-fn
-  "A simple factory for edition permission check functions."
-  [qfn]
-  (us/assert fn? qfn)
-  (fn [& args]
-    (let [rows (apply qfn args)]
-      (if (or (empty? rows)
-              (not (or (some :can-edit rows)
-                       (some :is-admin rows)
-                       (some :is-owner rows))))
-        (ex/raise :type :not-found
-                  :code :object-not-found
-                  :hint "not found")
-        rows))))
-
-(defn make-read-check-fn
-  "A simple factory for read permission check functions."
-  [qfn]
-  (us/assert fn? qfn)
-  (fn [& args]
-    (let [rows (apply qfn args)]
-      (if-not (seq rows)
-        (ex/raise :type :not-found
-                  :code :object-not-found)
-        rows))))

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

@@ -12,6 +12,7 @@
    [app.db :as db]
    [app.rpc.permissions :as perms]
    [app.rpc.queries.projects :as projects]
+   [app.rpc.queries.share-link :refer [retrieve-share-link]]
    [app.rpc.queries.teams :as teams]
    [app.storage.impl :as simpl]
    [app.util.blob :as blob]
@@ -59,7 +60,7 @@
     where f.id = ?
       and ppr.profile_id = ?")
 
-(defn- retrieve-file-permissions
+(defn retrieve-file-permissions
   [conn profile-id file-id]
   (when (and profile-id file-id)
     (db/exec! conn [sql:file-permissions
@@ -67,11 +68,37 @@
                     file-id profile-id
                     file-id profile-id])))
 
+(defn get-permissions
+  ([conn profile-id file-id]
+   (let [rows     (retrieve-file-permissions conn profile-id file-id)
+         is-owner (boolean (some :is-owner rows))
+         is-admin (boolean (some :is-admin rows))
+         can-edit (boolean (some :can-edit rows))]
+     (when (seq rows)
+       {:type :membership
+        :is-owner is-owner
+        :is-admin (or is-owner is-admin)
+        :can-edit (or is-owner is-admin can-edit)
+        :can-read true})))
+  ([conn profile-id file-id share-id]
+   (let [perms  (get-permissions conn profile-id file-id)
+         ldata  (retrieve-share-link conn file-id share-id)]
+
+     ;; NOTE: in a future when share-link becomes more powerfull and
+     ;; will allow us specify which parts of the app is availabel, we
+     ;; will probably need to tweak this function in order to expose
+     ;; this flags to the frontend.
+     (cond
+       (some? perms) perms
+       (some? ldata) {:type :share-link
+                      :can-read true
+                      :flags (:flags ldata)}))))
+
 (def has-edit-permissions?
-  (perms/make-edition-predicate-fn retrieve-file-permissions))
+  (perms/make-edition-predicate-fn get-permissions))
 
 (def has-read-permissions?
-  (perms/make-read-predicate-fn retrieve-file-permissions))
+  (perms/make-read-predicate-fn get-permissions))
 
 (def check-edition-permissions!
   (perms/make-check-fn has-edit-permissions?))
@@ -79,7 +106,6 @@
 (def check-read-permissions!
   (perms/make-check-fn has-read-permissions?))
 
-
 ;; --- Query: Files search
 
 ;; TODO: this query need to a good refactor
@@ -180,9 +206,12 @@
 (sv/defmethod ::file
   [{:keys [pool] :as cfg} {:keys [profile-id id] :as params}]
   (db/with-atomic [conn pool]
-    (let [cfg (assoc cfg :conn conn)]
+    (let [cfg   (assoc cfg :conn conn)
-      (check-edition-permissions! conn profile-id id)
+          perms (get-permissions conn profile-id id)]
-      (retrieve-file cfg id))))
+
+      (check-read-permissions! perms)
+      (some-> (retrieve-file cfg id)
+              (assoc :permissions perms)))))
 
 (s/def ::page
   (s/keys :req-un [::profile-id ::file-id]))
@@ -217,7 +246,8 @@
 (sv/defmethod ::page
   [{:keys [pool] :as cfg} {:keys [profile-id file-id strip-thumbnails]}]
   (db/with-atomic [conn pool]
-    (check-edition-permissions! conn profile-id file-id)
+    (check-read-permissions! conn profile-id file-id)
+
     (let [cfg     (assoc cfg :conn conn)
           file    (retrieve-file cfg file-id)
           page-id (get-in file [:data :pages 0])]
@@ -291,7 +321,7 @@
   [{:keys [pool] :as cfg} {:keys [profile-id file-id] :as params}]
   (db/with-atomic [conn pool]
     (let [cfg (assoc cfg :conn conn)]
-      (check-edition-permissions! conn profile-id file-id)
+      (check-read-permissions! conn profile-id file-id)
       (retrieve-file-libraries cfg false file-id))))
 
 ;; --- QUERY: team-recent-files

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

@@ -31,18 +31,31 @@
     where ppr.project_id = ?
       and ppr.profile_id = ?")
 
-(defn- retrieve-project-permissions
+(defn- get-permissions
   [conn profile-id project-id]
-  (db/exec! conn [sql:project-permissions
+  (let [rows     (db/exec! conn [sql:project-permissions
-                  project-id profile-id
+                                 project-id profile-id
-                  project-id profile-id]))
+                                 project-id profile-id])
+        is-owner (boolean (some :is-owner rows))
+        is-admin (boolean (some :is-admin rows))
+        can-edit (boolean (some :can-edit rows))]
+     (when (seq rows)
+       {:is-owner is-owner
+        :is-admin (or is-owner is-admin)
+        :can-edit (or is-owner is-admin can-edit)
+        :can-read true})))
+
+(def has-edit-permissions?
+  (perms/make-edition-predicate-fn get-permissions))
+
+(def has-read-permissions?
+  (perms/make-read-predicate-fn get-permissions))
 
 (def check-edition-permissions!
-  (perms/make-edition-check-fn retrieve-project-permissions))
+  (perms/make-check-fn has-edit-permissions?))
 
 (def check-read-permissions!
-  (perms/make-read-check-fn retrieve-project-permissions))
+  (perms/make-check-fn has-read-permissions?))
-
 
 ;; --- Query: Projects
 

backend/src/app/rpc/queries/share_link.clj

@@ -0,0 +1,23 @@
+;; 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) UXBOX Labs SL
+
+(ns app.rpc.queries.share-link
+  (:require
+   [app.db :as db]))
+
+(defn decode-share-link-row
+  [row]
+  (-> row
+      (update :flags db/decode-pgarray #{})
+      (update :pages db/decode-pgarray #{})))
+
+(defn retrieve-share-link
+  [conn file-id share-id]
+  (some-> (db/get-by-params conn :share-link
+                            {:id share-id :file-id file-id}
+                            {:check-not-found false})
+          (decode-share-link-row)))
+

backend/src/app/rpc/queries/teams.clj

@@ -24,16 +24,29 @@
     where tpr.profile_id = ?
       and tpr.team_id = ?")
 
-(defn- retrieve-team-permissions
+(defn get-permissions
   [conn profile-id team-id]
-  (db/exec! conn [sql:team-permissions profile-id team-id]))
+  (let [rows     (db/exec! conn [sql:team-permissions profile-id team-id])
+        is-owner (boolean (some :is-owner rows))
+        is-admin (boolean (some :is-admin rows))
+        can-edit (boolean (some :can-edit rows))]
+     (when (seq rows)
+       {:is-owner is-owner
+        :is-admin (or is-owner is-admin)
+        :can-edit (or is-owner is-admin can-edit)
+        :can-read true})))
+
+(def has-edit-permissions?
+  (perms/make-edition-predicate-fn get-permissions))
+
+(def has-read-permissions?
+  (perms/make-read-predicate-fn get-permissions))
 
 (def check-edition-permissions!
-  (perms/make-edition-check-fn retrieve-team-permissions))
+  (perms/make-check-fn has-edit-permissions?))
 
 (def check-read-permissions!
-  (perms/make-read-check-fn retrieve-team-permissions))
+  (perms/make-check-fn has-read-permissions?))
-
 
 ;; --- Query: Teams
 
@@ -60,10 +73,24 @@
       and tp.profile_id = ?
     order by tp.created_at asc")
 
+(defn process-permissions
+  [team]
+  (let [is-owner    (:is-owner team)
+        is-admin    (:is-admin team)
+        can-edit    (:can-edit team)
+        permissions {:type :membership
+                     :is-owner is-owner
+                     :is-admin (or is-owner is-admin)
+                     :can-edit (or is-owner is-admin can-edit)}]
+    (-> team
+        (dissoc :is-owner :is-admin :can-edit)
+        (assoc :permissions permissions))))
+
 (defn retrieve-teams
   [conn profile-id]
   (let [defaults (profile/retrieve-additional-data conn profile-id)]
-    (db/exec! conn [sql:teams (:default-team-id defaults) profile-id])))
+    (->> (db/exec! conn [sql:teams (:default-team-id defaults) profile-id])
+         (mapv process-permissions))))
 
 ;; --- Query: Team (by ID)
 
@@ -86,7 +113,7 @@
     (when-not result
       (ex/raise :type :not-found
                 :code :team-does-not-exist))
-    result))
+    (process-permissions result)))
 
 
 ;; --- Query: Team Members

backend/src/app/rpc/queries/viewer.clj

@@ -10,29 +10,17 @@
    [app.common.spec :as us]
    [app.db :as db]
    [app.rpc.queries.files :as files]
+   [app.rpc.queries.share-link :as slnk]
    [app.rpc.queries.teams :as teams]
    [app.util.services :as sv]
    [clojure.spec.alpha :as s]))
 
 ;; --- Query: View Only Bundle
 
-(defn- decode-share-link-row
-  [row]
-  (-> row
-      (update :flags db/decode-pgarray #{})
-      (update :pages db/decode-pgarray #{})))
-
 (defn- retrieve-project
   [conn id]
   (db/get-by-id conn :project id {:columns [:id :name :team-id]}))
 
-(defn- retrieve-share-link
-  [{:keys [conn]} file-id id]
-  (some-> (db/get-by-params conn :share-link
-                            {:id id :file-id file-id}
-                            {:check-not-found false})
-          (decode-share-link-row)))
-
 (defn- retrieve-bundle
   [{:keys [conn] :as cfg} file-id]
   (let [file    (files/retrieve-file cfg file-id)
@@ -41,7 +29,7 @@
         users   (teams/retrieve-users conn (:team-id project))
 
         links   (->> (db/query conn :share-link {:file-id file-id})
-                     (mapv decode-share-link-row))
+                     (mapv slnk/decode-share-link-row))
 
         fonts   (db/query conn :team-font-variant
                           {:team-id (:team-id project)
@@ -64,8 +52,11 @@
   [{:keys [pool] :as cfg} {:keys [profile-id file-id share-id] :as params}]
   (db/with-atomic [conn pool]
     (let [cfg    (assoc cfg :conn conn)
-          bundle (retrieve-bundle cfg file-id)
+          slink  (slnk/retrieve-share-link conn file-id share-id)
-          slink  (retrieve-share-link cfg file-id share-id)]
+          perms  (files/get-permissions conn profile-id file-id share-id)
+
+          bundle (some-> (retrieve-bundle cfg file-id)
+                         (assoc :permissions perms))]
 
       ;; When we have neither profile nor share, we just return a not
       ;; found response to the user.
@@ -80,13 +71,6 @@
         (files/check-read-permissions! conn profile-id file-id))
 
       (cond-> bundle
-        ;; If we have current profile, put
-        (some? profile-id)
-        (as-> $ (let [edit? (boolean (files/has-edit-permissions? conn profile-id file-id))
-                      read? (boolean (files/has-read-permissions? conn profile-id file-id))]
-                  (-> (assoc $ :permissions {:read read? :edit edit?})
-                      (cond-> (not edit?) (dissoc :share-links)))))
-
         (some? slink)
         (assoc :share slink)
 

backend/test/app/services_teams_test.clj

@@ -33,11 +33,13 @@
                     :role :editor
                     :profile-id (:id profile1)}]
 
-      ;; (th/print-result! out)
 
       ;; invite external user without complaints
       (let [data (assoc data :email "foo@bar.com")
             out  (th/mutation! data)]
+
+        ;; (th/print-result! out)
+
         (t/is (nil? (:result out)))
         (t/is (= 1 (:call-count (deref mock)))))
 
@@ -111,6 +113,7 @@
                   :id (:id team)
                   :profile-id (:id profile1)}
           out    (th/mutation! params)]
+      ;; (th/print-result! out)
       (t/is (nil? (:error out))))
 
     ;; query the list of teams after soft deletion
@@ -133,7 +136,6 @@
                 :profile-id (:id profile1)}
           out  (th/query! data)]
       ;; (th/print-result! out)
-
       (t/is (nil? (:error out)))
       (let [result (:result out)]
         (t/is (= 0 (count result)))))

frontend/src/app/main/ui/dashboard/team.cljs

@@ -26,12 +26,13 @@
 
 (mf/defc header
   {::mf/wrap [mf/memo]}
-  [{:keys [section] :as props}]
+  [{:keys [section team] :as props}]
   (let [go-members        (st/emitf (dd/go-to-team-members))
         go-settings       (st/emitf (dd/go-to-team-settings))
-        invite-member     (st/emitf (modal/show {:type ::invite-member}))
+        invite-member     (st/emitf (modal/show {:type ::invite-member :team team}))
         members-section?  (= section :dashboard-team-members)
-        settings-section? (= section :dashboard-team-settings)]
+        settings-section? (= section :dashboard-team-settings)
+        permissions       (:permissions team)]
 
     [:header.dashboard-header
      [:div.dashboard-title
@@ -46,20 +47,21 @@
        [:li {:class (when settings-section? "active")}
         [:a {:on-click go-settings} (tr "labels.settings")]]]]
 
-     (if members-section?
+     (if (and members-section? (:is-admin permissions))
        [:a.btn-secondary.btn-small {:on-click invite-member}
         (tr "dashboard.invite-profile")]
        [:div])]))
 
 (defn get-available-roles
-  []
+  [permissions]
-  [{:value "" :label (tr "labels.role")}
+  (->> [{:value "editor" :label (tr "labels.editor")}
-   {:value "admin" :label (tr "labels.admin")}
+        (when (:is-admin permissions)
-   {:value "editor" :label (tr "labels.editor")}
+          {:value "admin" :label (tr "labels.admin")})
-   ;; Temporarily disabled viewer role
+        ;; Temporarily disabled viewer role
-   ;; https://tree.taiga.io/project/uxboxproject/issue/1083
+        ;; https://tree.taiga.io/project/uxboxproject/issue/1083
-   ;; {:value "viewer" :label (tr "labels.viewer")}
+        ;; {:value "viewer" :label (tr "labels.viewer")}
-   ])
+        ]
+       (filterv identity)))
 
 (s/def ::email ::us/email)
 (s/def ::role  ::us/keyword)
@@ -69,8 +71,9 @@
 (mf/defc invite-member-modal
   {::mf/register modal/components
    ::mf/register-as ::invite-member}
-  []
+  [{:keys [team]}]
-  (let [roles   (mf/use-memo get-available-roles)
+  (let [perms   (:permissions team)
+        roles   (mf/use-memo (mf/deps perms) #(get-available-roles perms))
         initial (mf/use-memo (constantly {:role "editor"}))
         form    (fm/use-form :spec ::invite-member-form
                              :initial initial)
@@ -262,6 +265,7 @@
                                  (tr "dashboard.your-penpot")
                                  (:name team))))))
 
+
     (mf/use-effect
      (st/emitf (dd/fetch-team-members)
                (dd/fetch-team-stats)))

frontend/src/app/main/ui/viewer/header.cljs

@@ -67,10 +67,10 @@
         i/full-screen-off
         i/full-screen)]
 
-     (when (:edit permissions)
+     (when (:is-admin permissions)
        [:span.btn-primary {:on-click open-share-dialog} (tr "labels.share-prototype")])
 
-     (when (:edit permissions)
+     (when (:can-edit permissions)
        [:span.btn-text-dark {:on-click go-to-workspace} (tr "labels.edit-file")])]))
 
 (mf/defc header-sitemap
@@ -151,14 +151,16 @@
         :alt (tr "viewer.header.interactions-section")}
        i/play]
 
-      (when (:edit permissions)
+      (when (:can-edit permissions)
         [:button.mode-zone-button.tooltip.tooltip-bottom
          {:on-click #(navigate :comments)
           :class (dom/classnames :active (= section :comments))
           :alt (tr "viewer.header.comments-section")}
          i/chat])
 
-      (when (:read permissions)
+      (when (or (= (:type permissions) :membership)
+                (and (= (:type permissions) :share-link)
+                     (contains? (:flags permissions) :section-handoff)))
         [:button.mode-zone-button.tooltip.tooltip-bottom
          {:on-click #(navigate :handoff)
           :class (dom/classnames :active (= section :handoff))