github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-08-08 05:48:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Add permission checks to WebSocket subscription handlers (#11054)

Browse Source 
* 🐛 Add permission checks to WebSocket subscription handlers

Check file and team read permissions before allowing WebSocket
subscriptions to prevent resource enumeration via presence
notifications.

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

* 🐛 Fix random backend test failure
This commit is contained in:
Andrey Antukh 2026-08-07 11:24:24 +02:00 committed by GitHub
parent bc9319eac5
commit 399b00b86d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 72 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
backend/src/app/http/websocket.clj
View File

@ -7,6 +7,7 @@
(ns app.http.websocket
"A penpot notification service for file cooperative edition."
(:require
[app.binfile.common :as bfc]
[app.common.exceptions :as ex]
[app.common.logging :as l]
[app.common.pprint :as pp]
@ -17,6 +18,8 @@
[app.http.session :as session]
[app.metrics :as mtx]
[app.msgbus :as mbus]
[app.rpc.commands.files :as files]
[app.rpc.commands.teams :as teams]
[app.util.websocket :as ws]
[integrant.core :as ig]
[promesa.exec.csp :as sp]
@ -131,8 +134,9 @@
(mbus/pub! msgbus :topic topic :message msg))))
(defmethod handle-message :subscribe-team
[{:keys [::mbus/msgbus]} {:keys [::ws/id ::ws/state ::ws/output-ch ::session-id]} {:keys [team-id] :as params}]
[{:keys [::mbus/msgbus ::db/pool]} {:keys [::ws/id ::ws/state ::ws/output-ch ::session-id ::profile-id]} {:keys [team-id] :as params}]
(l/trace :fn "handle-message" :event "subscribe-team" :team-id team-id :conn-id id)
(teams/check-read-permissions! pool profile-id team-id)
(let [prev-subs (get @state ::team-subscription)
channel (sp/chan :buf (sp/dropping-buffer 64)
:xf (remove #(= (:session-id %) session-id)))]
@ -150,8 +154,10 @@
(defmethod handle-message :subscribe-file
[{:keys [::mbus/msgbus]} {:keys [::ws/id ::ws/state ::ws/output-ch ::session-id ::profile-id]} {:keys [file-id] :as params}]
[{:keys [::mbus/msgbus ::db/pool]} {:keys [::ws/id ::ws/state ::ws/output-ch ::session-id ::profile-id]} {:keys [file-id] :as params}]
(l/trace :fn "handle-message" :event "subscribe-file" :file-id file-id :conn-id id)
(bfc/check-file-exists pool file-id)
(files/check-read-permissions! pool profile-id file-id)
(let [psub (::file-subscription @state)
fch (sp/chan :buf (sp/dropping-buffer 64)
:xf (remove #(= (:session-id %) session-id)))]

25
backend/test/backend_tests/rpc_file_test.clj
View File

@ -733,7 +733,7 @@
(t/is (= 2 (count rows)))
(t/is (= 1 (count (remove (comp some? :deleted-at) rows))))
(t/is (= (thc/fmt-object-id file-id page-id frame-id-1 "frame")
(-> rows first :object-id))))
(->> rows (remove (comp some? :deleted-at)) first :object-id))))
;; Now that file-gc have marked for deletion the object
;; thumbnail lets execute the objects-gc task which remove
@ -2377,8 +2377,6 @@
(let [edata (-> out :error ex-data)]
(t/is (= :not-found (:type edata))))))
;; --- Security Fix Tests ---
(t/deftest link-file-to-library-circular-reference
(let [profile (th/create-profile* 1)
file1 (th/create-file* 1 {:profile-id (:id profile)
@ -2448,3 +2446,24 @@
(t/is (th/ex-info? (:error out)))
(let [edata (-> out :error ex-data)]
(t/is (= :validation (:type edata))))))
(t/deftest get-file-libraries-nonexistent-file
(let [prof (th/create-profile* 1 {:is-active true})
out (th/command! {::th/type :get-file-libraries
::rpc/profile-id (:id prof)
:file-id (uuid/random)})
err (:error out)]
(t/is (th/ex-info? err))
(t/is (th/ex-of-type? err :not-found))))
(t/deftest get-file-libraries-no-permission
(let [owner (th/create-profile* 1 {:is-active true})
other (th/create-profile* 2 {:is-active true})
file (th/create-file* 1 {:profile-id (:id owner)
:project-id (:default-project-id owner)})
out (th/command! {::th/type :get-file-libraries
::rpc/profile-id (:id other)
:file-id (:id file)})
err (:error out)]
(t/is (th/ex-info? err))
(t/is (th/ex-of-type? err :not-found))))

21
backend/test/backend_tests/rpc_font_test.clj
View File

@ -650,3 +650,24 @@
(t/is (some? (:error out)))
(t/is (= :not-found (-> out :error ex-data :type)))
(t/is (= :object-not-found (-> out :error ex-data :code)))))))
(t/deftest get-font-variants-nonexistent-file
(let [prof (th/create-profile* 1 {:is-active true})
out (th/command! {::th/type :get-font-variants
::rpc/profile-id (:id prof)
:file-id (uuid/random)})
err (:error out)]
(t/is (th/ex-info? err))
(t/is (th/ex-of-type? err :not-found))))
(t/deftest get-font-variants-no-permission
(let [owner (th/create-profile* 1 {:is-active true})
other (th/create-profile* 2 {:is-active true})
file (th/create-file* 1 {:profile-id (:id owner)
:project-id (:default-project-id owner)})
out (th/command! {::th/type :get-font-variants
::rpc/profile-id (:id other)
:file-id (:id file)})
err (:error out)]
(t/is (th/ex-info? err))
(t/is (th/ex-of-type? err :not-found))))

21
backend/test/backend_tests/rpc_project_test.clj
View File

@ -241,3 +241,24 @@
error-data (ex-data error)]
(t/is (th/ex-info? error))
(t/is (= (:type error-data) :not-found))))))
(t/deftest get-project-nonexistent
(let [prof (th/create-profile* 1 {:is-active true})
out (th/command! {::th/type :get-project
::rpc/profile-id (:id prof)
:id (uuid/random)})
err (:error out)]
(t/is (th/ex-info? err))
(t/is (th/ex-of-type? err :not-found))))
(t/deftest get-project-no-permission
(let [owner (th/create-profile* 1 {:is-active true})
other (th/create-profile* 2 {:is-active true})
proj (th/create-project* 1 {:profile-id (:id owner)
:team-id (:default-team-id owner)})
out (th/command! {::th/type :get-project
::rpc/profile-id (:id other)
:id (:id proj)})
err (:error out)]
(t/is (th/ex-info? err))
(t/is (th/ex-of-type? err :not-found))))