From 2e8cdac1241814d2d1f222776f82a766198255c5 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 4 Aug 2026 13:28:20 +0000 Subject: [PATCH] :bug: Add authorization checks to WebSocket subscriptions 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 --- backend/src/app/http/websocket.clj | 33 +++--- .../backend_tests/http_websocket_test.clj | 102 ++++++++++++++++++ 2 files changed, 121 insertions(+), 14 deletions(-) create mode 100644 backend/test/backend_tests/http_websocket_test.clj diff --git a/backend/src/app/http/websocket.clj b/backend/src/app/http/websocket.clj index 4dd74dfc11..9a3972a26a 100644 --- a/backend/src/app/http/websocket.clj +++ b/backend/src/app/http/websocket.clj @@ -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] diff --git a/backend/test/backend_tests/http_websocket_test.clj b/backend/test/backend_tests/http_websocket_test.clj new file mode 100644 index 0000000000..22683e93c2 --- /dev/null +++ b/backend/test/backend_tests/http_websocket_test.clj @@ -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)))))))