From 3d176d539015bb3ad607e69829254cb4292cf8f3 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 5 Aug 2026 17:42:49 +0200 Subject: [PATCH] :bug: Restrict webhook creation/edit/delete to team members only (#11029) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * :bug: 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 * :bug: 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 --- backend/src/app/rpc/commands/webhooks.clj | 12 +- .../test/backend_tests/rpc_webhooks_test.clj | 160 +++++++++++++----- 2 files changed, 120 insertions(+), 52 deletions(-) diff --git a/backend/src/app/rpc/commands/webhooks.clj b/backend/src/app/rpc/commands/webhooks.clj index 33341bb34e..85051e8ad7 100644 --- a/backend/src/app/rpc/commands/webhooks.clj +++ b/backend/src/app/rpc/commands/webhooks.clj @@ -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)) diff --git a/backend/test/backend_tests/rpc_webhooks_test.clj b/backend/test/backend_tests/rpc_webhooks_test.clj index 3b39c8b52d..df4ae3a622 100644 --- a/backend/test/backend_tests/rpc_webhooks_test.clj +++ b/backend/test/backend_tests/rpc_webhooks_test.clj @@ -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)))))))))))