From b79680eeb70f8fe6776d3df29a8feca2a7b7c1ab Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Tue, 25 Aug 2026 13:10:55 +0200 Subject: [PATCH] :bug: Fix asset 404 for unauthenticated share-link viewers (#11342) PR #11036 added a per-request permission check to the file-media asset endpoints (/assets/by-file-media-id/:id and the /thumbnail variant) using bfc/get-file-permissions. Anonymous share-link viewers were then rejected because they have neither a session nor an access token, the asset URL carries no share context, and the 2-arg get-file-permissions short-circuits to nil when profile-id is nil. Make the asset endpoints share-link aware, mirroring how get-view-only-bundle already authorizes the same scenario: read the share-id from the query string, validate it as a UUID, and call the 3-arg perms/get-file-read-permissions (which chains the existing 2-arg bfc lookup, the bfc share-link fallback, and the Nitrate org-owner fallback). On the frontend, extend cf/resolve-file-media with an optional share-id arg and pass it from the WASM viewer render path using the share-id already present in [:viewer-local :share-id]. Non-viewer call sites (workspace, clipboard, code-gen) keep the original URL shape because the new arg defaults to nil. Closes #11338 AI-assisted-by: minimax-m3 --- backend/src/app/http/assets.clj | 11 +- .../test/backend_tests/http_assets_test.clj | 106 ++++++++++++++++++ frontend/src/app/config.cljs | 20 +++- frontend/src/app/main/data/viewer.cljs | 11 +- 4 files changed, 143 insertions(+), 5 deletions(-) diff --git a/backend/src/app/http/assets.clj b/backend/src/app/http/assets.clj index 6258760548..22783be1e2 100644 --- a/backend/src/app/http/assets.clj +++ b/backend/src/app/http/assets.clj @@ -7,7 +7,6 @@ (ns app.http.assets "Assets related handlers." (:require - [app.binfile.common :as bfc] [app.common.data :as d] [app.common.exceptions :as ex] [app.common.time :as ct] @@ -15,6 +14,7 @@ [app.db :as db] [app.http.access-token :as actoken] [app.http.session :as session] + [app.rpc.permissions :as perms] [app.storage :as sto] [integrant.core :as ig] [yetti.response :as-alias yres])) @@ -41,6 +41,12 @@ (ex/raise :type :not-found :hint "object not found"))) +(defn- get-share-id + "Extract and validate the optional `share-id` query param. Returns a UUID + or `nil` for missing/malformed values." + [{:keys [query-params]}] + (some-> query-params :share-id d/parse-uuid)) + (defn- get-file-media-object [pool id] (db/get* pool :file-media-object {:id id} {::db/remove-deleted false})) @@ -125,7 +131,8 @@ (let [file-id (:file-id mobj) profile-id (or (::session/profile-id request) (::actoken/profile-id request)) - perms (bfc/get-file-permissions pool profile-id file-id)] + share-id (get-share-id request) + perms (perms/get-file-read-permissions pool profile-id file-id share-id)] (if-not (:can-read perms) {::yres/status 404} (let [sobj (sto/get-object storage (kf mobj))] diff --git a/backend/test/backend_tests/http_assets_test.clj b/backend/test/backend_tests/http_assets_test.clj index 94510d73d6..e4f5ebff43 100644 --- a/backend/test/backend_tests/http_assets_test.clj +++ b/backend/test/backend_tests/http_assets_test.clj @@ -13,6 +13,7 @@ [app.http.access-token :as actoken] [app.http.assets :as assets] [app.http.session :as session] + [app.rpc :as-alias rpc] [app.rpc.commands.access-token :as access-token] [app.storage :as sto] [backend-tests.helpers :as th] @@ -588,6 +589,111 @@ response (assets/file-objects-handler cfg request)] (t/is (= 404 (::yres/status response))))) +;; ---------------------------------------------------------------- +;; Tests: file-objects-handler — share-link authz (issue #11338) +;; ---------------------------------------------------------------- + +(t/deftest file-objects-handler-anonymous-with-valid-share-id-succeeds + ;; Anonymous request with a valid share-id matching the file must + ;; succeed (share-link viewers are unauthenticated by definition). + (let [storage (-> (:app.storage/storage th/*system*) + (configure-storage-backend)) + cfg (make-handler-cfg storage) + owner (th/create-profile* 1) + team (th/create-team* 1 {:profile-id (:id owner)}) + project (th/create-project* 1 {:profile-id (:id owner) + :team-id (:id team)}) + file (th/create-file* 1 {:profile-id (:id owner) + :project-id (:id project)}) + media-storage (create-storage-object! storage "file-media-object" "image data") + media-obj (th/create-file-media-object* {:file-id (:id file) + :media-id (:id media-storage)}) + slink (:result (th/command! {::th/type :create-share-link + ::rpc/profile-id (:id owner) + :file-id (:id file) + :pages #{} + :who-comment "team" + :who-inspect "all"})) + request {:path-params {:id (str (:id media-obj))} + :query-params {:share-id (str (:id slink))}} + response (assets/file-objects-handler cfg request)] + (t/is (= 204 (::yres/status response))))) + +(t/deftest file-objects-handler-anonymous-with-share-id-for-other-file-returns-404 + ;; A share-id from file A must not grant access to assets of file B. + (let [storage (-> (:app.storage/storage th/*system*) + (configure-storage-backend)) + cfg (make-handler-cfg storage) + owner (th/create-profile* 1) + team (th/create-team* 1 {:profile-id (:id owner)}) + project (th/create-project* 1 {:profile-id (:id owner) + :team-id (:id team)}) + file-a (th/create-file* 1 {:profile-id (:id owner) + :project-id (:id project)}) + file-b (th/create-file* 2 {:profile-id (:id owner) + :project-id (:id project)}) + media-a (create-storage-object! storage "file-media-object" "image A") + media-obj-a (th/create-file-media-object* {:file-id (:id file-a) + :media-id (:id media-a)}) + media-b (create-storage-object! storage "file-media-object" "image B") + media-obj-b (th/create-file-media-object* {:file-id (:id file-b) + :media-id (:id media-b)}) + slink (:result (th/command! {::th/type :create-share-link + ::rpc/profile-id (:id owner) + :file-id (:id file-a) + :pages #{} + :who-comment "team" + :who-inspect "all"})) + request {:path-params {:id (str (:id media-obj-b))} + :query-params {:share-id (str (:id slink))}} + response (assets/file-objects-handler cfg request)] + (t/is (= 404 (::yres/status response))))) + +(t/deftest file-objects-handler-anonymous-with-malformed-share-id-returns-404 + ;; Malformed share-id must not raise; it must short-circuit to 404. + (let [storage (-> (:app.storage/storage th/*system*) + (configure-storage-backend)) + cfg (make-handler-cfg storage) + profile (th/create-profile* 1) + team (th/create-team* 1 {:profile-id (:id profile)}) + project (th/create-project* 1 {:profile-id (:id profile) + :team-id (:id team)}) + file (th/create-file* 1 {:profile-id (:id profile) + :project-id (:id project)}) + media-storage (create-storage-object! storage "file-media-object" "image data") + media-obj (th/create-file-media-object* {:file-id (:id file) + :media-id (:id media-storage)}) + request {:path-params {:id (str (:id media-obj))} + :query-params {:share-id "not-a-uuid"}} + response (assets/file-objects-handler cfg request)] + (t/is (= 404 (::yres/status response))))) + +(t/deftest file-thumbnails-handler-anonymous-with-valid-share-id-succeeds + ;; Thumbnail endpoint must also honor the share-id query param. + (let [storage (-> (:app.storage/storage th/*system*) + (configure-storage-backend)) + cfg (make-handler-cfg storage) + owner (th/create-profile* 1) + team (th/create-team* 1 {:profile-id (:id owner)}) + project (th/create-project* 1 {:profile-id (:id owner) + :team-id (:id team)}) + file (th/create-file* 1 {:profile-id (:id owner) + :project-id (:id project)}) + thumb-storage (create-storage-object! storage "file-object-thumbnail" "thumb data") + media-obj (th/create-file-media-object* {:file-id (:id file) + :media-id (:id thumb-storage)}) + slink (:result (th/command! {::th/type :create-share-link + ::rpc/profile-id (:id owner) + :file-id (:id file) + :pages #{} + :who-comment "team" + :who-inspect "all"})) + request {:path-params {:id (str (:id media-obj))} + :query-params {:share-id (str (:id slink))}} + response (assets/file-thumbnails-handler cfg request)] + ;; Falls back to media-id since no thumbnail-id, but still serves + (t/is (= 204 (::yres/status response))))) + (t/deftest objects-handler-expired-object ;; Expired objects should return 404 (get-object filters them out). (let [storage (-> (:app.storage/storage th/*system*) diff --git a/frontend/src/app/config.cljs b/frontend/src/app/config.cljs index dc2c5a237a..97bb2eb0f6 100644 --- a/frontend/src/app/config.cljs +++ b/frontend/src/app/config.cljs @@ -258,6 +258,23 @@ [id] (dm/str (u/join public-uri "assets/by-id/" (str id)))) +;; Current share-id for asset URL building. The share-link viewer sets +;; this in `app.main.data.viewer/initialize` so every caller of +;; `resolve-file-media` (inspector, code panel, image previews, +;; code generators, etc.) automatically receives a share-id without +;; having to thread it through every call site. Workspace callers +;; leave it nil and continue to get the original URL shape. +(defonce ^:private ^{:doc "Active share-id used by `resolve-file-media`." + :dynamic true} + current-share-id + nil) + +(defn set-current-share-id! + "Set the share-id used by `resolve-file-media`. Pass `nil` to clear it + (e.g. when leaving the viewer)." + [share-id] + (set! current-share-id share-id)) + (defn resolve-file-media ([media] (resolve-file-media media false)) @@ -266,7 +283,8 @@ (dm/str (cond-> (u/join public-uri "assets/by-file-media-id/") (true? thumbnail?) (u/join (dm/str id "/thumbnail")) - (false? thumbnail?) (u/join (dm/str id))))))) + (false? thumbnail?) (u/join (dm/str id)) + (some? current-share-id) (u/join (dm/str "?share-id=" current-share-id))))))) (defn resolve-href [resource] diff --git a/frontend/src/app/main/data/viewer.cljs b/frontend/src/app/main/data/viewer.cljs index 1d9c49f9d3..f847cd862d 100644 --- a/frontend/src/app/main/data/viewer.cljs +++ b/frontend/src/app/main/data/viewer.cljs @@ -95,14 +95,21 @@ ;; browser just focus the opened tab instead of creating new ;; tab. (let [name (str "viewer-" file-id)] - (unchecked-set ug/global "name" name))))) + (unchecked-set ug/global "name" name)) + ;; Make every `cf/resolve-file-media` call (inspector, code panel, + ;; image previews, ...) share-link aware for the lifetime of this + ;; viewer. Cleared by `finalize` below. + (cf/set-current-share-id! share-id)))) (defn finalize [_] (ptk/reify ::finalize ptk/UpdateEvent (update [_ state] - (dissoc state :viewer)))) + (dissoc state :viewer)) + ptk/EffectEvent + (effect [_ _ _] + (cf/set-current-share-id! nil)))) ;; --- Data Fetching