mirror of
https://github.com/penpot/penpot.git
synced 2026-08-05 20:38:38 +00:00
🐛 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
This commit is contained in:
parent
319a2185c9
commit
835cf5c55e
@ -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)))]
|
||||
|
||||
@ -2320,8 +2320,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)
|
||||
@ -2391,3 +2389,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))))
|
||||
|
||||
@ -922,3 +922,64 @@
|
||||
:name "Valid Font Name"}
|
||||
out (th/command! params)]
|
||||
(t/is (th/success? out))))))
|
||||
|
||||
(t/deftest create-font-variant-rejects-foreign-font-id
|
||||
;; N2-07: A user with edit permissions on their own team must not be
|
||||
;; able to create a font variant using a font-id that already belongs
|
||||
;; to another team (BOLA / CWE-639).
|
||||
(with-mocks [mock {:target 'app.rpc.quotes/check! :return nil}]
|
||||
(let [prof1 (th/create-profile* 1 {:is-active true})
|
||||
prof2 (th/create-profile* 2 {:is-active true})
|
||||
team1 (:default-team-id prof1)
|
||||
team2 (:default-team-id prof2)
|
||||
font-id (uuid/custom 10 999)
|
||||
data (-> (io/resource "backend_tests/test_files/font-1.ttf")
|
||||
(io/read*))]
|
||||
|
||||
;; prof1 creates a font variant in team1 with font-id
|
||||
(let [params {::th/type :create-font-variant
|
||||
::rpc/profile-id (:id prof1)
|
||||
:team-id team1
|
||||
:font-id font-id
|
||||
:font-family "SharedFont"
|
||||
:font-weight 400
|
||||
:font-style "normal"
|
||||
:data {"font/ttf" data}}
|
||||
out (th/command! params)]
|
||||
(t/is (nil? (:error out))))
|
||||
|
||||
;; prof2 tries to create a variant using the same font-id but
|
||||
;; in team2 — must be rejected because font-id belongs to team1
|
||||
(let [params {::th/type :create-font-variant
|
||||
::rpc/profile-id (:id prof2)
|
||||
:team-id team2
|
||||
:font-id font-id
|
||||
:font-family "SharedFont"
|
||||
:font-weight 700
|
||||
:font-style "normal"
|
||||
:data {"font/ttf" data}}
|
||||
out (th/command! params)]
|
||||
(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))))
|
||||
|
||||
@ -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))))
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user