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

🐛 Add authorization checks to WebSocket subscriptions

Browse Source 
Subscribe-file and subscribe-team handlers now verify the requesting
profile has read permissions on the target resource before creating
a subscription. Pointer-update handler now validates that the message
file-id matches the subscribed file-id before publishing.

Closes #11067

AI-assisted-by: mimo-v2.5-pro
This commit is contained in:
Andrey Antukh 2026-08-04 13:28:20 +00:00
parent 43e05c38bf
commit 2e8cdac124
2 changed files with 121 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -17,6 +17,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,14 +133,15 @@
(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}]
[cfg {: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! cfg 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)))]
(sp/pipe channel output-ch false)
(mbus/sub! msgbus :topic team-id :chan channel)
(mbus/sub! (::mbus/msgbus cfg) :topic team-id :chan channel)
(let [subs {:team-id team-id :channel channel :topic team-id}]
(swap! state assoc ::team-subscription subs))
@ -146,12 +149,13 @@
;; Close previous subscription if exists
(when-let [ch (:channel prev-subs)]
(sp/close! ch)
(mbus/purge! msgbus [ch]))))
(mbus/purge! (::mbus/msgbus cfg) [ch]))))
(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}]
[cfg {: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)
(files/check-read-permissions! cfg profile-id file-id)
(let [psub (::file-subscription @state)
fch (sp/chan :buf (sp/dropping-buffer 64)
:xf (remove #(= (:session-id %) session-id)))]
@ -162,7 +166,7 @@
;; Close previous subscription if exists
(when-let [ch (:channel psub)]
(sp/close! ch)
(mbus/purge! msgbus [ch]))
(mbus/purge! (::mbus/msgbus cfg) [ch]))
(sp/go-loop []
(when-let [{:keys [type] :as message} (sp/take! fch)]
@ -174,20 +178,20 @@
:file-id file-id
:session-id session-id
:profile-id profile-id}]
(mbus/pub! msgbus
(mbus/pub! (::mbus/msgbus cfg)
:topic file-id
:message message)))
(recur)))
;; Subscribe to file topic
(mbus/sub! msgbus :topic file-id :chan fch)
(mbus/sub! (::mbus/msgbus cfg) :topic file-id :chan fch)
;; Notifify the rest of participants of the new connection.
(let [message {:type :join-file
:file-id file-id
:session-id session-id
:profile-id profile-id}]
(mbus/pub! msgbus :topic file-id :message message))))
(mbus/pub! (::mbus/msgbus cfg) :topic file-id :message message))))
(defmethod handle-message :unsubscribe-file
[{:keys [::mbus/msgbus]} {:keys [::ws/id ::ws/state ::session-id ::profile-id]} {:keys [file-id] :as params}]
@ -220,12 +224,13 @@
(defmethod handle-message :pointer-update
[{:keys [::mbus/msgbus]} {:keys [::ws/state ::session-id ::profile-id]} {:keys [file-id] :as message}]
(when (::file-subscription @state)
(let [message (-> message
(assoc :subs-id file-id)
(assoc :profile-id profile-id)
(assoc :session-id session-id))]
(mbus/pub! msgbus :topic file-id :message message))))
(when-let [subs (::file-subscription @state)]
(when (= file-id (:file-id subs))
(let [message (-> message
(assoc :subs-id file-id)
(assoc :profile-id profile-id)
(assoc :session-id session-id))]
(mbus/pub! msgbus :topic file-id :message message)))))
(defmethod handle-message :default
[_ {:keys [::ws/id]} message]

102
backend/test/backend_tests/http_websocket_test.clj Normal file
View File

@ -0,0 +1,102 @@
;; This Source Code Form is subject to the terms of the Mozilla Public
;; License, v. 2.0. If a copy of the MPL was not distributed with this
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
;;
;; Copyright (c) KALEIDOS INC Sucursal en España SL
(ns backend-tests.http-websocket-test
(:require
[app.common.uuid :as uuid]
[app.db :as db]
[app.http.websocket :as ws]
[app.msgbus :as mbus]
[app.rpc :as-alias rpc]
[app.rpc.commands.files :as files]
[app.rpc.commands.teams :as teams]
[app.util.websocket :as util-ws]
[backend-tests.helpers :as th]
[clojure.test :as t]
[promesa.exec.csp :as sp]))
(t/use-fixtures :once th/state-init)
(t/use-fixtures :each th/database-reset)
(defn make-wsp
[profile-id state output-ch]
{::util-ws/id (uuid/next)
::util-ws/state state
::util-ws/output-ch output-ch
::ws/profile-id profile-id
::ws/session-id (uuid/next)})
(t/deftest subscribe-file-permission-check
(let [profile1 (th/create-profile* 1 {:is-active true})
profile2 (th/create-profile* 2 {:is-active true})
file (th/create-file* 1 {:profile-id (:id profile1)
:project-id (:default-project-id profile1)})
cfg th/*system*
state (atom {})
output-ch (sp/chan :buf (sp/dropping-buffer 64))]
(t/testing "rejects unauthorized user"
(let [wsp (make-wsp (:id profile2) state output-ch)]
(t/is (thrown-with-msg?
clojure.lang.ExceptionInfo
#"not found"
((get-method ws/handle-message :subscribe-file)
cfg wsp {:file-id (:id file)})))))
(t/testing "permission check passes for authorized user"
(t/is (nil? (files/check-read-permissions! cfg (:id profile1) (:id file)))))))
(t/deftest subscribe-team-permission-check
(let [profile1 (th/create-profile* 1 {:is-active true})
profile2 (th/create-profile* 2 {:is-active true})
team (th/create-team* 1 {:profile-id (:id profile1)})
cfg th/*system*
state (atom {})
output-ch (sp/chan :buf (sp/dropping-buffer 64))]
(t/testing "rejects unauthorized user"
(let [wsp (make-wsp (:id profile2) state output-ch)]
(t/is (thrown-with-msg?
clojure.lang.ExceptionInfo
#"not found"
((get-method ws/handle-message :subscribe-team)
cfg wsp {:team-id (:id team)})))))
(t/testing "permission check passes for authorized user"
(t/is (nil? (teams/check-read-permissions! cfg (:id profile1) (:id team)))))))
(t/deftest pointer-update-validates-file-id
(let [profile (th/create-profile* 1 {:is-active true})
file (th/create-file* 1 {:profile-id (:id profile)
:project-id (:default-project-id profile)})
cfg th/*system*
file-id (:id file)
sub-ch (sp/chan :buf (sp/dropping-buffer 64))
state (atom {::ws/file-subscription {:file-id file-id
:channel sub-ch
:topic file-id}})
output-ch (sp/chan :buf (sp/dropping-buffer 64))
wsp (make-wsp (:id profile) state output-ch)]
(t/testing "skips publish when file-id does not match subscription"
(let [wrong-msg {:type :pointer-update
:file-id (uuid/next)
:position {:x 10 :y 20}
:zoom 1.0}]
(t/is (nil?
((get-method ws/handle-message :pointer-update)
cfg wsp wrong-msg)))))
(t/testing "does nothing when no file subscription exists"
(let [empty-state (atom {})
empty-wsp (make-wsp (:id profile) empty-state output-ch)
msg {:type :pointer-update
:file-id file-id
:position {:x 10 :y 20}
:zoom 1.0}]
(t/is (nil?
((get-method ws/handle-message :pointer-update)
cfg empty-wsp msg)))))))