🐛 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

@@ -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))]

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*)

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]

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