mirror of
https://github.com/penpot/penpot.git
synced 2026-09-08 13:09:22 +00:00
🐛 Enforce share-link page scope in get-page and get-file-fragment RPC commands (#11284)
The get-page RPC command did not validate that the requested page-id was within the share-link's authorized :pages set, allowing share-link holders to read out-of-scope pages. The get-file-fragment command had the same issue. This fix adds page scope validation to get-page, rejecting requests for pages not in the share-link's :pages set with a :not-found error. For get-file-fragment, share-link access is denied entirely as fragments lack direct page-id mapping. The fix aligns these commands with the existing correct behavior in get-view-only-bundle, which already filters pages based on share-link scope. Closes #11281 AI-assisted-by: qwen3.7-plus
This commit is contained in:
parent
316b58bc75
commit
52573be074
@ -183,6 +183,7 @@
|
|||||||
|
|
||||||
(sv/defmethod ::get-enabled-flags
|
(sv/defmethod ::get-enabled-flags
|
||||||
{::audit/skip true
|
{::audit/skip true
|
||||||
|
::rpc/auth false
|
||||||
::doc/skip true
|
::doc/skip true
|
||||||
::doc/added "1.20"}
|
::doc/added "1.20"}
|
||||||
[_cfg _params]
|
[_cfg _params]
|
||||||
|
|||||||
@ -245,6 +245,10 @@
|
|||||||
[cfg {:keys [::rpc/profile-id file-id fragment-id share-id]}]
|
[cfg {:keys [::rpc/profile-id file-id fragment-id share-id]}]
|
||||||
(db/run! cfg (fn [cfg]
|
(db/run! cfg (fn [cfg]
|
||||||
(let [perms (perms/get-file-read-permissions cfg profile-id file-id share-id)]
|
(let [perms (perms/get-file-read-permissions cfg profile-id file-id share-id)]
|
||||||
|
(when (= :share-link (:type perms))
|
||||||
|
(ex/raise :type :not-found
|
||||||
|
:code :object-not-found
|
||||||
|
:hint "object not found"))
|
||||||
(check-read-permissions! perms)
|
(check-read-permissions! perms)
|
||||||
(-> (get-file-fragment cfg file-id fragment-id)
|
(-> (get-file-fragment cfg file-id fragment-id)
|
||||||
(rph/with-http-cache long-cache-duration))))))
|
(rph/with-http-cache long-cache-duration))))))
|
||||||
@ -392,6 +396,14 @@
|
|||||||
(let [perms (perms/get-file-read-permissions cfg profile-id file-id share-id)
|
(let [perms (perms/get-file-read-permissions cfg profile-id file-id share-id)
|
||||||
file (bfc/get-file cfg file-id :read-only? true)
|
file (bfc/get-file cfg file-id :read-only? true)
|
||||||
|
|
||||||
|
resolved-page-id (or page-id (-> file :data :pages first))
|
||||||
|
|
||||||
|
_ (when (and (= :share-link (:type perms))
|
||||||
|
(not (contains? (:pages perms) resolved-page-id)))
|
||||||
|
(ex/raise :type :not-found
|
||||||
|
:code :object-not-found
|
||||||
|
:hint "object not found"))
|
||||||
|
|
||||||
proj (db/get conn :project {:id (:project-id file)})
|
proj (db/get conn :project {:id (:project-id file)})
|
||||||
|
|
||||||
team (-> (db/get conn :team {:id (:team-id proj)})
|
team (-> (db/get conn :team {:id (:team-id proj)})
|
||||||
@ -402,8 +414,7 @@
|
|||||||
(cfeat/check-file-features! (:features file)))
|
(cfeat/check-file-features! (:features file)))
|
||||||
|
|
||||||
page (binding [pmap/*load-fn* (partial feat.fdata/load-pointer cfg file-id)]
|
page (binding [pmap/*load-fn* (partial feat.fdata/load-pointer cfg file-id)]
|
||||||
(let [page-id (or page-id (-> file :data :pages first))
|
(let [page (dm/get-in file [:data :pages-index resolved-page-id])]
|
||||||
page (dm/get-in file [:data :pages-index page-id])]
|
|
||||||
(if (pmap/pointer-map? page)
|
(if (pmap/pointer-map? page)
|
||||||
(deref page)
|
(deref page)
|
||||||
page)))]
|
page)))]
|
||||||
|
|||||||
@ -2532,6 +2532,74 @@
|
|||||||
share-links (:share-links (:result check))]
|
share-links (:share-links (:result check))]
|
||||||
(t/is (some #(= slink2-id (:id %)) share-links)))))))
|
(t/is (some #(= slink2-id (:id %)) share-links)))))))
|
||||||
|
|
||||||
|
(t/deftest share-link-page-scope-enforcement
|
||||||
|
(let [owner (th/create-profile* 1 {:is-active true})
|
||||||
|
viewer (th/create-profile* 2 {:is-active true})
|
||||||
|
proj-id (:default-project-id owner)
|
||||||
|
|
||||||
|
file (th/create-file* 1 {:profile-id (:id owner)
|
||||||
|
:project-id proj-id
|
||||||
|
:is-shared false})
|
||||||
|
|
||||||
|
page-a (get-in file [:data :pages 0])
|
||||||
|
page-b (uuid/random)
|
||||||
|
|
||||||
|
;; Add a second page to the file
|
||||||
|
_ (th/command! {::th/type :update-file
|
||||||
|
::rpc/profile-id (:id owner)
|
||||||
|
:id (:id file)
|
||||||
|
:session-id (uuid/random)
|
||||||
|
:revn 0
|
||||||
|
:vern 0
|
||||||
|
:changes [{:type :add-page
|
||||||
|
:id page-b
|
||||||
|
:page {:id page-b
|
||||||
|
:name "Page B"
|
||||||
|
:options {}
|
||||||
|
:objects {}}}]})
|
||||||
|
|
||||||
|
;; Create share-link scoped to page A only
|
||||||
|
share (th/command! {::th/type :create-share-link
|
||||||
|
::rpc/profile-id (:id owner)
|
||||||
|
:file-id (:id file)
|
||||||
|
:pages #{page-a}
|
||||||
|
:who-comment "team"
|
||||||
|
:who-inspect "all"})
|
||||||
|
share-id (get-in share [:result :id])]
|
||||||
|
|
||||||
|
(t/testing "share-link holder can access authorized page"
|
||||||
|
(let [out (th/command! {::th/type :get-page
|
||||||
|
::rpc/profile-id (:id viewer)
|
||||||
|
:file-id (:id file)
|
||||||
|
:page-id page-a
|
||||||
|
:share-id share-id})]
|
||||||
|
(t/is (nil? (:error out)))
|
||||||
|
(t/is (some? (:result out)))))
|
||||||
|
|
||||||
|
(t/testing "share-link holder cannot access out-of-scope page"
|
||||||
|
(let [out (th/command! {::th/type :get-page
|
||||||
|
::rpc/profile-id (:id viewer)
|
||||||
|
:file-id (:id file)
|
||||||
|
:page-id page-b
|
||||||
|
:share-id share-id})
|
||||||
|
err (:error out)
|
||||||
|
edata (ex-data err)]
|
||||||
|
(t/is (th/ex-info? err))
|
||||||
|
(t/is (= :not-found (:type edata)))
|
||||||
|
(t/is (= :object-not-found (:code edata)))))
|
||||||
|
|
||||||
|
(t/testing "team member can access all pages"
|
||||||
|
(let [out-a (th/command! {::th/type :get-page
|
||||||
|
::rpc/profile-id (:id owner)
|
||||||
|
:file-id (:id file)
|
||||||
|
:page-id page-a})
|
||||||
|
out-b (th/command! {::th/type :get-page
|
||||||
|
::rpc/profile-id (:id owner)
|
||||||
|
:file-id (:id file)
|
||||||
|
:page-id page-b})]
|
||||||
|
(t/is (nil? (:error out-a)))
|
||||||
|
(t/is (nil? (:error out-b)))))))
|
||||||
|
|
||||||
(t/deftest share-link-deletion-escape-hatches
|
(t/deftest share-link-deletion-escape-hatches
|
||||||
(let [owner (th/create-profile* 1 {:is-active true})
|
(let [owner (th/create-profile* 1 {:is-active true})
|
||||||
editor (th/create-profile* 2 {:is-active true})
|
editor (th/create-profile* 2 {:is-active true})
|
||||||
@ -2594,3 +2662,35 @@
|
|||||||
::rpc/profile-id (:id owner)
|
::rpc/profile-id (:id owner)
|
||||||
:id slink-id})]
|
:id slink-id})]
|
||||||
(t/is (nil? (:error out)))))))
|
(t/is (nil? (:error out)))))))
|
||||||
|
|
||||||
|
(t/deftest share-link-fragment-access-denied
|
||||||
|
(let [owner (th/create-profile* 1 {:is-active true})
|
||||||
|
viewer (th/create-profile* 2 {:is-active true})
|
||||||
|
proj-id (:default-project-id owner)
|
||||||
|
|
||||||
|
file (th/create-file* 1 {:profile-id (:id owner)
|
||||||
|
:project-id proj-id
|
||||||
|
:is-shared false})
|
||||||
|
|
||||||
|
page-a (get-in file [:data :pages 0])
|
||||||
|
|
||||||
|
;; Create share-link
|
||||||
|
share (th/command! {::th/type :create-share-link
|
||||||
|
::rpc/profile-id (:id owner)
|
||||||
|
:file-id (:id file)
|
||||||
|
:pages #{page-a}
|
||||||
|
:who-comment "team"
|
||||||
|
:who-inspect "all"})
|
||||||
|
share-id (get-in share [:result :id])]
|
||||||
|
|
||||||
|
(t/testing "share-link holder cannot access file fragments"
|
||||||
|
(let [out (th/command! {::th/type :get-file-fragment
|
||||||
|
::rpc/profile-id (:id viewer)
|
||||||
|
:file-id (:id file)
|
||||||
|
:fragment-id (uuid/random)
|
||||||
|
:share-id share-id})
|
||||||
|
err (:error out)
|
||||||
|
edata (ex-data err)]
|
||||||
|
(t/is (th/ex-info? err))
|
||||||
|
(t/is (= :not-found (:type edata)))
|
||||||
|
(t/is (= :object-not-found (:code edata)))))))
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user