mirror of
https://github.com/penpot/penpot.git
synced 2026-08-07 13:29:07 +00:00
🐛 Require file read permissions for asset endpoints
Add authorization check to generic-handler in assets.clj so that /assets/by-file-media-id/:id and its /thumbnail variant verify the requesting profile has read access to the parent file. Return 404 (not 403) when access is denied to avoid confirming existence. Also switch get-file-media-object from db/get to db/get* so that non-existent media objects return nil instead of raising. AI-assisted-by: qwen3.7-plus
This commit is contained in:
Andrey Antukh
parent
c329defa5e
commit
aac8a69951
@ -7,6 +7,7 @@
|
||||
(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]
|
||||
@ -42,7 +43,7 @@
|
||||
|
||||
(defn- get-file-media-object
|
||||
[pool id]
|
||||
(db/get pool :file-media-object {:id id} {::db/remove-deleted false}))
|
||||
(db/get* pool :file-media-object {:id id} {::db/remove-deleted false}))
|
||||
|
||||
(defn- serve-object-from-s3
|
||||
[{:keys [::sto/storage ::signature-max-age ::cache-max-age] :as cfg} obj]
|
||||
@ -116,13 +117,21 @@
|
||||
(defn- generic-handler
|
||||
"A generic handler helper/common code for file-media based handlers."
|
||||
[{:keys [::sto/storage] :as cfg} request kf]
|
||||
(let [pool (::db/pool storage)
|
||||
id (get-id request)
|
||||
mobj (get-file-media-object pool id)
|
||||
sobj (sto/get-object storage (kf mobj))]
|
||||
(if sobj
|
||||
(serve-object cfg sobj)
|
||||
{::yres/status 404})))
|
||||
(let [pool (::db/pool storage)
|
||||
id (get-id request)
|
||||
mobj (get-file-media-object pool id)]
|
||||
(if (nil? mobj)
|
||||
{::yres/status 404}
|
||||
(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)]
|
||||
(if-not (:can-read perms)
|
||||
{::yres/status 404}
|
||||
(let [sobj (sto/get-object storage (kf mobj))]
|
||||
(if sobj
|
||||
(serve-object cfg sobj)
|
||||
{::yres/status 404})))))))
|
||||
|
||||
(defn file-objects-handler
|
||||
"Handler that serves storage objects by file media id."
|
||||
|
||||
@ -459,6 +459,135 @@
|
||||
;; Tests: objects-handler — expired objects
|
||||
;; ----------------------------------------------------------------
|
||||
|
||||
;; ----------------------------------------------------------------
|
||||
;; Tests: file-objects-handler — authz required (T2-N1-01)
|
||||
;; ----------------------------------------------------------------
|
||||
|
||||
(t/deftest file-objects-handler-unauthenticated-returns-404
|
||||
;; Unauthenticated requests to file-media assets must return 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))}}
|
||||
response (assets/file-objects-handler cfg request)]
|
||||
(t/is (= 404 (::yres/status response)))))
|
||||
|
||||
(t/deftest file-objects-handler-no-file-perms-returns-404
|
||||
;; Authenticated user without file read permissions must get 404
|
||||
(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)})
|
||||
stranger (th/create-profile* 2)
|
||||
request {:path-params {:id (str (:id media-obj))}
|
||||
::session/profile-id (:id stranger)}
|
||||
response (assets/file-objects-handler cfg request)]
|
||||
(t/is (= 404 (::yres/status response)))))
|
||||
|
||||
(t/deftest file-objects-handler-with-file-perms-succeeds
|
||||
;; Authenticated user with file read permissions must get the object
|
||||
(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)})
|
||||
request {:path-params {:id (str (:id media-obj))}
|
||||
::session/profile-id (:id owner)}
|
||||
response (assets/file-objects-handler cfg request)]
|
||||
(t/is (= 204 (::yres/status response)))))
|
||||
|
||||
(t/deftest file-thumbnails-handler-unauthenticated-returns-404
|
||||
;; Unauthenticated requests to file-thumbnail assets must return 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))}}
|
||||
response (assets/file-thumbnails-handler cfg request)]
|
||||
(t/is (= 404 (::yres/status response)))))
|
||||
|
||||
(t/deftest file-thumbnails-handler-with-file-perms-succeeds
|
||||
;; Authenticated user with file read permissions must get the thumbnail
|
||||
(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)})
|
||||
request {:path-params {:id (str (:id media-obj))}
|
||||
::session/profile-id (:id owner)}
|
||||
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 file-objects-handler-non-existent-media-returns-404
|
||||
;; Request for non-existent file-media-object returns 404
|
||||
(let [storage (-> (:app.storage/storage th/*system*)
|
||||
(configure-storage-backend))
|
||||
cfg (make-handler-cfg storage)
|
||||
profile (th/create-profile* 1)
|
||||
request {:path-params {:id (str (uuid/next))}
|
||||
::session/profile-id (:id profile)}
|
||||
response (assets/file-objects-handler cfg request)]
|
||||
(t/is (= 404 (::yres/status response)))))
|
||||
|
||||
(t/deftest file-objects-handler-nil-profile-id-returns-404
|
||||
;; When profile-id is nil (invalid session), must return 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))}
|
||||
::session/profile-id nil}
|
||||
response (assets/file-objects-handler cfg request)]
|
||||
(t/is (= 404 (::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*)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user