Merge pull request #5135 from penpot/palba-eva-viewer-role

  Add viewer role
This commit is contained in:
Andrey Antukh 2024-10-18 10:31:40 +02:00 committed by GitHub
commit 1aa2c0f9de
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
62 changed files with 1207 additions and 448 deletions

View file

@ -412,7 +412,10 @@
:fn (mg/resource "app/migrations/sql/0129-mod-file-change-table.sql")}
{:name "0130-mod-file-change-table"
:fn (mg/resource "app/migrations/sql/0130-mod-file-change-table.sql")}])
:fn (mg/resource "app/migrations/sql/0130-mod-file-change-table.sql")}
{:name "0131-mod-webhook-table"
:fn (mg/resource "app/migrations/sql/0131-mod-webhook-table.sql")}])
(defn apply-migrations!
[pool name migrations]

View file

@ -0,0 +1,6 @@
ALTER TABLE webhook
ADD COLUMN profile_id uuid NULL REFERENCES profile (id) ON DELETE SET NULL;
CREATE INDEX webhook__profile_id__idx
ON webhook (profile_id)
WHERE profile_id IS NOT NULL;

View file

@ -89,7 +89,7 @@
::sse/stream? true
::sm/params schema:import-binfile}
[{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id name project-id file] :as params}]
(projects/check-read-permissions! pool profile-id project-id)
(projects/check-edition-permissions! pool profile-id project-id)
(let [cfg (-> cfg
(assoc ::bf.v1/project-id project-id)
(assoc ::bf.v1/profile-id profile-id)

View file

@ -222,7 +222,7 @@
::webhooks/event? true}
[{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id id team-id is-pinned] :as params}]
(db/with-atomic [conn pool]
(check-edition-permissions! conn profile-id id)
(check-read-permissions! conn profile-id id)
(db/exec-one! conn [sql:update-project-pin team-id id profile-id is-pinned is-pinned])
nil))

View file

@ -12,6 +12,7 @@
[app.common.features :as cfeat]
[app.common.logging :as l]
[app.common.schema :as sm]
[app.common.types.team :as tt]
[app.common.uuid :as uuid]
[app.config :as cf]
[app.db :as db]
@ -20,6 +21,7 @@
[app.loggers.audit :as audit]
[app.main :as-alias main]
[app.media :as media]
[app.msgbus :as mbus]
[app.rpc :as-alias rpc]
[app.rpc.commands.profile :as profile]
[app.rpc.doc :as-alias doc]
@ -605,14 +607,8 @@
nil)))
;; --- Mutation: Team Update Role
;; Temporarily disabled viewer role
;; https://tree.taiga.io/project/penpot/issue/1083
(def valid-roles
#{:owner :admin :editor #_:viewer})
(def schema:role
[::sm/one-of valid-roles])
[::sm/one-of tt/valid-roles])
(defn role->params
[role]
@ -623,7 +619,7 @@
:viewer {:is-owner false :is-admin false :can-edit false}))
(defn update-team-member-role
[conn {:keys [profile-id team-id member-id role] :as params}]
[{:keys [::db/conn ::mbus/msgbus]} {:keys [profile-id team-id member-id role] :as params}]
;; We retrieve all team members instead of query the
;; database for a single member. This is just for
;; convenience, if this becomes a bottleneck or problematic,
@ -631,7 +627,6 @@
(let [perms (get-permissions conn profile-id team-id)
members (get-team-members conn team-id)
member (d/seek #(= member-id (:id %)) members)
is-owner? (:is-owner perms)
is-admin? (:is-admin perms)]
@ -655,6 +650,13 @@
(ex/raise :type :validation
:code :cant-promote-to-owner))
(mbus/pub! msgbus
:topic member-id
:message {:type :team-role-change
:subs-id member-id
:team-id team-id
:role role})
(let [params (role->params role)]
;; Only allow single owner on team
(when (= role :owner)
@ -678,9 +680,8 @@
(sv/defmethod ::update-team-member-role
{::doc/added "1.17"
::sm/params schema:update-team-member-role}
[{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id] :as params}]
(db/with-atomic [conn pool]
(update-team-member-role conn (assoc params :profile-id profile-id))))
[cfg {:keys [::rpc/profile-id] :as params}]
(db/tx-run! cfg update-team-member-role (assoc params :profile-id profile-id)))
;; --- Mutation: Delete Team Member
@ -692,9 +693,10 @@
(sv/defmethod ::delete-team-member
{::doc/added "1.17"
::sm/params schema:delete-team-member}
[{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id team-id member-id] :as params}]
[{:keys [::db/pool ::mbus/msgbus] :as cfg} {:keys [::rpc/profile-id team-id member-id] :as params}]
(db/with-atomic [conn pool]
(let [perms (get-permissions conn profile-id team-id)]
(let [team (get-team pool :profile-id profile-id :team-id team-id)
perms (get-permissions conn profile-id team-id)]
(when-not (or (:is-owner perms)
(:is-admin perms))
(ex/raise :type :validation
@ -707,6 +709,14 @@
(db/delete! conn :team-profile-rel {:profile-id member-id
:team-id team-id})
(mbus/pub! msgbus
:topic member-id
:message {:type :team-membership-change
:change :removed
:subs-id member-id
:team-id team-id
:team-name (:name team)})
nil)))
;; --- Mutation: Update Team Photo
@ -724,6 +734,7 @@
::sm/params schema:update-team-photo}
[cfg {:keys [::rpc/profile-id file] :as params}]
;; Validate incoming mime type
(media/validate-media-type! file #{"image/jpeg" "image/png" "image/webp"})
(update-team-photo cfg (assoc params :profile-id profile-id)))
@ -789,7 +800,7 @@
[:map
[:id ::sm/uuid]
[:fullname :string]]]
[:role [::sm/one-of valid-roles]]
[:role [::sm/one-of tt/valid-roles]]
[:email ::sm/email]])
(def ^:private check-create-invitation-params!
@ -1115,7 +1126,7 @@
::sm/params schema:update-team-invitation-role}
[{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id team-id email role] :as params}]
(db/with-atomic [conn pool]
(let [perms (get-permissions conn profile-id team-id)]
(let [perms (get-permissions conn profile-id team-id)]
(when-not (:is-admin perms)
(ex/raise :type :validation
@ -1124,6 +1135,7 @@
(db/update! conn :team-invitation
{:role (name role) :updated-at (dt/now)}
{:team-id team-id :email-to (profile/clean-email email)})
nil)))
;; --- Mutation: Delete invitation

View file

@ -15,12 +15,27 @@
[app.http.client :as http]
[app.loggers.webhooks :as webhooks]
[app.rpc :as-alias rpc]
[app.rpc.commands.teams :refer [check-edition-permissions! check-read-permissions!]]
[app.rpc.commands.teams :refer [check-read-permissions!] :as t]
[app.rpc.doc :as-alias doc]
[app.rpc.permissions :as perms]
[app.util.services :as sv]
[app.util.time :as dt]
[cuerdas.core :as str]))
(defn get-webhooks-permissions
[conn profile-id team-id creator-id]
(let [permissions (t/get-permissions conn profile-id team-id)
can-edit (boolean (or (:can-edit permissions)
(= profile-id creator-id)))]
(assoc permissions :can-edit can-edit)))
(def has-webhook-edit-permissions?
(perms/make-edition-predicate-fn get-webhooks-permissions))
(def check-webhook-edition-permissions!
(perms/make-check-fn has-webhook-edit-permissions?))
(defn decode-row
[{:keys [uri] :as row}]
(cond-> row
@ -65,11 +80,12 @@
max-hooks-for-team)))))
(defn- insert-webhook!
[{:keys [::db/pool]} {:keys [team-id uri mtype is-active] :as params}]
[{:keys [::db/pool]} {:keys [team-id uri mtype is-active ::rpc/profile-id] :as params}]
(-> (db/insert! pool :webhook
{:id (uuid/next)
:team-id team-id
:uri (str uri)
:profile-id profile-id
:is-active is-active
:mtype mtype})
(decode-row)))
@ -101,7 +117,7 @@
{::doc/added "1.17"
::sm/params schema:create-webhook}
[{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id team-id] :as params}]
(check-edition-permissions! pool profile-id team-id)
(check-webhook-edition-permissions! pool profile-id team-id profile-id)
(validate-quotes! cfg params)
(validate-webhook! cfg nil params)
(insert-webhook! cfg params))
@ -118,7 +134,7 @@
::sm/params schema:update-webhook}
[{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id id] :as params}]
(let [whook (-> (db/get pool :webhook {:id id}) (decode-row))]
(check-edition-permissions! pool profile-id (:team-id whook))
(check-webhook-edition-permissions! pool profile-id (:team-id whook) (:profile-id whook))
(validate-webhook! cfg whook params)
(update-webhook! cfg whook params)))
@ -132,15 +148,17 @@
[{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id id]}]
(db/with-atomic [conn pool]
(let [whook (-> (db/get conn :webhook {:id id}) decode-row)]
(check-edition-permissions! conn profile-id (:team-id whook))
(check-webhook-edition-permissions! conn profile-id (:team-id whook) (:profile-id whook))
(db/delete! conn :webhook {:id id})
nil)))
;; --- Query: Webhooks
(def sql:get-webhooks
"select id, uri, mtype, is_active, error_code, error_count
from webhook where team_id = ? order by uri")
"SELECT id, uri, mtype, is_active, error_code, error_count, profile_id
FROM webhook
WHERE team_id = ?
ORDER BY uri")
(def ^:private schema:get-webhooks
[:map {:title "get-webhooks"}

View file

@ -332,6 +332,7 @@
(t/is (nil? (:error out)))
(:result out)))
(defn create-webhook*
([params] (create-webhook* *system* params))
([system {:keys [team-id id uri mtype is-active]

View file

@ -152,7 +152,7 @@
(t/is (th/ex-info? error))
(t/is (th/ex-of-type? error :not-found))))
(t/deftest permissions-checks-delete-project
(t/deftest permissions-checks-pin-project
(let [profile1 (th/create-profile* 1)
profile2 (th/create-profile* 2)
project (th/create-project* 1 {:team-id (:default-team-id profile1)

View file

@ -19,6 +19,23 @@
(t/use-fixtures :once th/state-init)
(t/use-fixtures :each th/database-reset)
(defn create-webhook-params [id team]
{::th/type :create-webhook
::rpc/profile-id id
:team-id team
:uri (u/uri "http://example.com")
:mtype "application/json"})
(defn check-webhook-format
([result]
(t/is (contains? result :id))
(t/is (contains? result :team-id))
(t/is (contains? result :created-at))
(t/is (contains? result :profile-id))
(t/is (contains? result :updated-at))
(t/is (contains? result :uri))
(t/is (contains? result :mtype))))
(t/deftest webhook-crud
(with-mocks [http-mock {:target 'app.http.client/req!
:return {:status 200}}]
@ -39,15 +56,8 @@
(t/is (nil? (:error out)))
(t/is (= 1 (:call-count @http-mock)))
;; (th/print-result! out)
(let [result (:result out)]
(t/is (contains? result :id))
(t/is (contains? result :team-id))
(t/is (contains? result :created-at))
(t/is (contains? result :updated-at))
(t/is (contains? result :uri))
(t/is (contains? result :mtype))
(check-webhook-format result)
(t/is (= (:uri params) (:uri result)))
(t/is (= (:team-id params) (:team-id result)))
@ -69,12 +79,7 @@
(t/is (= 0 (:call-count @http-mock)))
(let [result (:result out)]
(t/is (contains? result :id))
(t/is (contains? result :team-id))
(t/is (contains? result :created-at))
(t/is (contains? result :updated-at))
(t/is (contains? result :uri))
(t/is (contains? result :mtype))
(check-webhook-format result)
(t/is (= (:id params) (:id result)))
(t/is (= (:id @whook) (:id result)))
@ -130,13 +135,14 @@
(let [rows (th/db-exec! ["select * from webhook"])]
(t/is (= 0 (count rows))))))
(t/testing "delete webhook (unauthorozed)"
(th/reset-mock! http-mock)
(t/testing "delete webhook (unauthorized)"
(let [params {::th/type :delete-webhook
::rpc/profile-id uuid/zero
:id (:id @whook)}
out (th/command! params)]
;; (th/print-result! out)
(t/is (= 0 (:call-count @http-mock)))
(let [error (:error out)
error-data (ex-data error)]
@ -144,6 +150,124 @@
(t/is (= (:type error-data) :not-found))
(t/is (= (:code error-data) :object-not-found))))))))
(t/deftest webhooks-permissions-crud-viewer-only
(with-mocks [http-mock {:target 'app.http.client/req!
:return {:status 200}}]
(let [owner (th/create-profile* 1 {:is-active true})
viewer (th/create-profile* 2 {:is-active true})
team (th/create-team* 1 {:profile-id (:id owner)})
whook (volatile! nil)]
(th/create-team-role* {:team-id (:id team)
:profile-id (:id viewer)
:role :viewer})
;; Assert all roles for team
(let [roles (th/db-query :team-profile-rel {:team-id (:id team)})]
(t/is (= 2 (count roles))))
(t/testing "viewer creates a webhook"
(let [viewers-webhook (create-webhook-params (:id viewer) (:id team))
out (th/command! viewers-webhook)]
(t/is (nil? (:error out)))
(t/is (= 1 (:call-count @http-mock)))
(let [result (:result out)]
(check-webhook-format result)
(t/is (= (:uri viewers-webhook) (:uri result)))
(t/is (= (:team-id viewers-webhook) (:team-id result)))
(t/is (= (::rpc/profile-id viewers-webhook) (:profile-id result)))
(t/is (= (:mtype viewers-webhook) (:mtype result)))
(vreset! whook result))))
(th/reset-mock! http-mock)
(t/testing "viewer updates it's own webhook (success)"
(let [params {::th/type :update-webhook
::rpc/profile-id (:id viewer)
:id (:id @whook)
:uri (:uri @whook)
:mtype "application/transit+json"
:is-active false}
out (th/command! params)
result (:result out)]
(t/is (nil? (:error out)))
(t/is (= 0 (:call-count @http-mock)))
(check-webhook-format result)
(t/is (= (:is-active params) (:is-active result)))
(t/is (= (:team-id @whook) (:team-id result)))
(t/is (= (:mtype params) (:mtype result)))
(vreset! whook result)))
(th/reset-mock! http-mock)
(t/testing "viewer deletes it's own webhook (success)"
(let [params {::th/type :delete-webhook
::rpc/profile-id (:id viewer)
:id (:id @whook)}
out (th/command! params)]
(t/is (= 0 (:call-count @http-mock)))
(t/is (nil? (:error out)))
(t/is (nil? (:result out)))
(let [rows (th/db-exec! ["select * from webhook"])]
(t/is (= 0 (count rows))))))
(th/reset-mock! http-mock))))
(t/deftest webhooks-permissions-crud-viewer-owner
(with-mocks [http-mock {:target 'app.http.client/req!
:return {:status 200}}]
(let [owner (th/create-profile* 1 {:is-active true})
viewer (th/create-profile* 2 {:is-active true})
team (th/create-team* 1 {:profile-id (:id owner)})
whook (volatile! nil)]
(th/create-team-role* {:team-id (:id team)
:profile-id (:id viewer)
:role :viewer})
(t/testing "owner creates a wehbook"
(let [owners-webhook (create-webhook-params (:id owner) (:id team))
out (th/command! owners-webhook)
result (:result out)]
(t/is (nil? (:error out)))
(t/is (= 1 (:call-count @http-mock)))
(check-webhook-format result)
(t/is (= (:uri owners-webhook) (:uri result)))
(t/is (= (:team-id owners-webhook) (:team-id result)))
(t/is (= (:mtype owners-webhook) (:mtype result)))
(vreset! whook result)))
(th/reset-mock! http-mock)
(t/testing "viewer updates owner's webhook (unauthorized)"
(let [params {::th/type :update-webhook
::rpc/profile-id (:id viewer)
:id (:id @whook)
:uri (str (:uri @whook) "/test")
:mtype "application/transit+json"
:is-active false}
out (th/command! params)]
(t/is (= 0 (:call-count @http-mock)))
(let [error (:error out)
error-data (ex-data error)]
(t/is (th/ex-info? error))
(t/is (= (:type error-data) :not-found))
(t/is (= (:code error-data) :object-not-found)))))
(th/reset-mock! http-mock)
(t/testing "viewer deletes owner's webhook (unauthorized)"
(let [params {::th/type :delete-webhook
::rpc/profile-id (:id viewer)
:id (:id @whook)}
out (th/command! params)
error (:error out)
error-data (ex-data error)]
(t/is (= 0 (:call-count @http-mock)))
(t/is (th/ex-info? error))
(t/is (= (:type error-data) :not-found))
(t/is (= (:code error-data) :object-not-found)))))))
(t/deftest webhooks-quotes
(with-mocks [http-mock {:target 'app.http.client/req!
:return {:status 200}}]