github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-08-06 12:58:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Restrict webhook creation/edit/delete to team members only (#11029)

Browse Source 
* 🐛 Restrict webhook edit/delete to team members only

Remove the creator-id fallback from get-webhooks-permissions.
Previously, the webhook creator could always edit/delete their
webhook even after being removed from the team. Now can-edit
comes from team role only — removed users get :not-found.

Webhooks are NOT deleted on member removal; the team owns them
and team admins/owners manage them.

AI-assisted-by: mimo-v2.5-pro

* 🐛 Restrict webhook creation to team editors

Use team role check (check-edition-permissions!) for create-webhook
instead of the custom check that allowed any team member to create
webhooks via creator-id self-match override.

AI-assisted-by: mimo-v2.5-pro
This commit is contained in:
Andrey Antukh 2026-08-05 17:42:49 +02:00 committed by GitHub
parent 0481408531
commit 3d176d5390
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 120 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
backend/src/app/rpc/commands/webhooks.clj
View File

@ -23,11 +23,9 @@
[cuerdas.core :as str]))
(defn get-webhooks-permissions
[conn profile-id team-id creator-id]
[conn profile-id team-id]
(let [permissions (t/get-permissions conn profile-id team-id)
can-edit (boolean (or (:can-edit permissions)
(= profile-id creator-id)))]
can-edit (boolean (:can-edit permissions))]
(assoc permissions :can-edit can-edit)))
(def has-webhook-edit-permissions?
@ -120,7 +118,7 @@
{::doc/added "1.17"
::sm/params schema:create-webhook}
[{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id team-id] :as params}]
(check-webhook-edition-permissions! pool profile-id team-id profile-id)
(t/check-edition-permissions! pool profile-id team-id)
(validate-quotes! cfg params)
(validate-webhook! cfg nil params)
(insert-webhook! cfg params))
@ -137,7 +135,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-webhook-edition-permissions! pool profile-id (:team-id whook) (:profile-id whook))
(check-webhook-edition-permissions! pool profile-id (:team-id whook))
(validate-webhook! cfg whook params)
(update-webhook! cfg whook params)))
@ -151,7 +149,7 @@
::db/transaction true}
[{:keys [::db/conn]} {:keys [::rpc/profile-id id]}]
(let [whook (-> (db/get conn :webhook {:id id}) decode-row)]
(check-webhook-edition-permissions! conn profile-id (:team-id whook) (:profile-id whook))
(check-webhook-edition-permissions! conn profile-id (:team-id whook))
(db/delete! conn :webhook {:id id})
nil))

160
backend/test/backend_tests/rpc_webhooks_test.clj
View File

@ -155,8 +155,7 @@
: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)]
team (th/create-team* 1 {:profile-id (:id owner)})]
(th/create-team-role* {:team-id (:id team)
:profile-id (:id viewer)
:role :viewer})
@ -164,52 +163,15 @@
(let [roles (th/db-query :team-profile-rel {:team-id (:id team)})]
(t/is (= 2 (count roles))))
(t/testing "viewer creates a webhook"
(t/testing "viewer cannot create a webhook (requires editor role)"
(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))))))
(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))))
@ -268,6 +230,26 @@
(t/is (= (:type error-data) :not-found))
(t/is (= (:code error-data) :object-not-found)))))))
(t/deftest webhooks-viewer-cannot-create
(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)})]
(th/create-team-role* {:team-id (:id team)
:profile-id (:id viewer)
:role :viewer})
(t/testing "viewer cannot create a webhook on the team"
(let [params (create-webhook-params (:id viewer) (:id team))
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))))))))
(t/deftest webhooks-quotes
(with-mocks [http-mock {:target 'app.http.client/req
:return {:status 200}}]
@ -304,3 +286,91 @@
(t/is (th/ex-info? error))
(t/is (= (:type error-data) :restriction))
(t/is (= (:code error-data) :webhooks-quote-reached))))))
(t/deftest removed-user-cannot-edit-webhook
(with-mocks [http-mock {:target 'app.http.client/req
:return {:status 200}}]
(let [owner (th/create-profile* 1 {:is-active true})
editor (th/create-profile* 2 {:is-active true})
team (th/create-team* 1 {:profile-id (:id owner)})]
(th/create-team-role* {:team-id (:id team)
:profile-id (:id editor)
:role :editor})
(let [params {::th/type :create-webhook
::rpc/profile-id (:id editor)
:team-id (:id team)
:uri (u/uri "http://example.com")
:mtype "application/json"}
out (th/command! params)]
(t/is (nil? (:error out)))
(let [whook (:result out)]
(th/reset-mock! http-mock)
(t/testing "owner can edit editor's webhook (team owns it)"
(let [params {::th/type :update-webhook
::rpc/profile-id (:id owner)
:id (:id whook)
:uri (u/uri "http://example.com/updated")
:mtype "application/transit+json"
:is-active true}
out (th/command! params)]
(t/is (nil? (:error out)))
(t/is (= 1 (:call-count @http-mock)))))
(th/reset-mock! http-mock)
(t/testing "remove editor from team"
(let [params {::th/type :delete-team-member
::rpc/profile-id (:id owner)
:team-id (:id team)
:member-id (:id editor)}
out (th/command! params)]
(t/is (nil? (:error out)))))
(th/reset-mock! http-mock)
(t/testing "removed editor cannot update webhook"
(let [params {::th/type :update-webhook
::rpc/profile-id (:id editor)
:id (:id whook)
:uri (u/uri "http://example.com/evil")
:mtype "application/transit+json"
:is-active true}
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 "removed editor cannot delete webhook"
(let [params {::th/type :delete-webhook
::rpc/profile-id (:id editor)
:id (:id whook)}
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 "owner can still delete editor's webhook"
(let [params {::th/type :delete-webhook
::rpc/profile-id (:id owner)
:id (:id whook)}
out (th/command! params)]
(t/is (nil? (:error out)))
(t/is (nil? (:result out)))
(let [rows (th/db-exec! ["select * from webhook"])]
(t/is (= 0 (count rows)))))))))))