github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-11-10 20:10:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Apply the share-link page scope to the comment queries and writes (#12888)

Browse Source 
* 🐛 Apply the share-link page scope to the comment queries and writes

A share link can be restricted to a subset of the pages of a file, and
the comment queries enforced that scope. But the RPC that resolves the
profiles of the commenters authorized the request with
check-comment-permissions! and dropped the returned perms, so its query
filtered only by file id. The holder of a page scoped link got the id,
email, fullname and photo of every profile that commented on any page
of the file, including the pages the owner left out of the link.

The query now takes the pages of the share link and only selects the
authors of the threads on them. The profile of the caller is still
added, so the viewer keeps its own avatar.

The scope was missing as well on the comment writes that take a page id
or a thread id: create-comment-thread accepted a page outside the link,
create-comment accepted a thread of an unshared page, and
update-comment-thread, update-comment-thread-position and
update-comment-thread-frame accepted a thread of an unshared page. They
now raise not-found, the same way the comment queries already did.
Replying to a thread of an unshared page also sent the emails to the
thread participants.

The page scope rule now lives in one place: share-link-pages returns
the pages granted by a share link and check-page-scope raises not-found
when a page is outside of it, so get-comment-threads reuses the helper
instead of checking the pages inline.

The docstring of get-profiles-for-file-comments said that it returns
the participants of all the comment threads of the file, which is not
true anymore for a share link. The docs are public, so the text and
the changes list are updated.

The tests of the share link page scope are hardened: the setup asserts
that the comment threads are created, the membership case has a second
commenter so it can no longer pass for the wrong reason, and the link
without pages, the allowed paths of the thread updates and the
who-comment=team denial are covered.

Relates to GHSA-89cq-3wv7-987r

AI-assisted-by: minimax-m3.1-flash

* 🐛 Enforce the share-link page scope on the remaining comment writes

Check the page scope of the share link before mutating in the four
comment writes that still skipped it: update-comment-thread-status,
update-comment, delete-comment-thread and delete-comment. The check
runs before the ownership checks, so an out-of-scope write fails with
not-found and the response does not reveal that the comment or thread
exists. Load the file in create-comment only after the permissions and
page scope checks, matching create-comment-thread, so a rejected reply
does not load the file data.

Cover the denial and the in-scope paths of the four commands, and the
fail-closed case of a link without pages on the comment reads. Move
the outsider thread creation after the profiles assertion so the
caller-union assertion fails when the union is removed. Document the
page scope invariant in the backend permissions memory.

AI-assisted-by: deepseek-v4.1-flash
This commit is contained in:
Andrey Antukh 2026-11-10 15:53:07 +01:00 committed by GitHub
parent 4874da450f
commit 0420db1ca9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 501 additions and 114 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
.serena/memories/backend/auth-permissions-product-domains.md
View File

@ -21,7 +21,7 @@
- Team role flags are normalized as owner > admin > editor > viewer. Owner/admin imply edit; any membership row implies read.
- File/project role rows only count with a live team membership: the file/project permission queries require a matching `team_profile_rel` row, and leaving or removing a member deletes their team-scoped `file`/`project`/`team-project-profile-rel` rows (GHSA-v9r9-h77c-55m2).
- File/project/comment checks are implemented in the owning command namespaces, often via helpers imported from `files`, `teams`, or `projects`; do not bypass those helpers with direct DB lookups unless preserving their not-found semantics.
- Comment permission includes both logged-in state and the file/team comment policy. Shared viewer paths may pass `share-id`; preserve that path when changing comment queries.
- Comment permission includes both logged-in state and the file/team comment policy. Shared viewer paths may pass `share-id`; preserve that path when changing comment queries. RPCs that accept `share-id` enforce the link's page scope before mutating (`share-link-pages` + `check-page-scope`): a thread or page outside the link raises `:not-found`, and that check runs before the ownership checks. `get-profiles-for-file-comments` filters its query by the same pages. `mark-all-threads-as-read` does not take `share-id`.
## Teams, projects, and invitations

258
backend/src/app/rpc/commands/comments.clj
View File

@ -41,6 +41,23 @@
(def comment-max-length 750)
(defn- share-link-pages
"The set of pages granted by a share-link, or nil when the access to the
file is not restricted to a page scope."
[perms]
(when (= :share-link (:type perms))
(or (:pages perms) #{})))
(defn- check-page-scope
"Raises a :not-found error when `page-id` is outside the page scope granted
by a share-link."
[perms page-id]
(when-let [pages (share-link-pages perms)]
(when-not (contains? pages page-id)
(ex/raise :type :not-found
:code :object-not-found
:hint "not found"))))
(defn- format-comment
[{:keys [content]}]
(->> (d/interleave-all
@ -233,8 +250,8 @@
(db/run! cfg (fn [{:keys [::db/conn] :as cfg}]
(let [perms (files/check-comment-permissions! cfg profile-id file-id share-id)
threads (get-comment-threads conn profile-id file-id)]
(if (= :share-link (:type perms))
(filterv #(contains? (:pages perms) (:page-id %)) threads)
(if-let [pages (share-link-pages perms)]
(filterv #(contains? pages (:page-id %)) threads)
threads)))))
(defn- get-comment-threads-sql
@ -335,11 +352,8 @@
(let [perms (files/check-comment-permissions! cfg profile-id file-id share-id)
thread (some-> (db/exec-one! conn [sql:get-comment-thread profile-id file-id id])
(decode-row))]
(when (and thread (= :share-link (:type perms)))
(when-not (contains? (:pages perms) (:page-id thread))
(ex/raise :type :not-found
:code :object-not-found
:hint "not found")))
(when thread
(check-page-scope perms (:page-id thread)))
thread))))
;; --- COMMAND: Retrieve Comments
@ -359,11 +373,7 @@
(db/run! cfg (fn [{:keys [::db/conn] :as cfg}]
(let [{:keys [file-id page-id]} (get-comment-thread conn thread-id)
perms (files/check-comment-permissions! cfg profile-id file-id share-id)]
(when (and (= :share-link (:type perms))
(not (contains? (:pages perms) page-id)))
(ex/raise :type :not-found
:code :object-not-found
:hint "not found"))
(check-page-scope perms page-id)
(get-comments conn thread-id)))))
(def sql:get-comments
@ -388,32 +398,49 @@
;; All the profiles that had comment the file, plus the current
;; profile.
(defn- file-comment-users-sql
[where]
(str/ffmt
"WITH available_profiles AS (
SELECT DISTINCT c.owner_id AS id
FROM comment c
JOIN comment_thread ct
ON ct.id = c.thread_id
WHERE ct.file_id = ?::uuid
%1
),
profile_ids AS (
SELECT id FROM available_profiles
UNION
SELECT ?::uuid
)
SELECT p.id,
p.email,
p.fullname AS name,
p.fullname,
p.photo_id,
p.is_active
FROM profile p
JOIN profile_ids AS x
ON x.id = p.id;"
where))
(def ^:private sql:file-comment-users
"WITH available_profiles AS (
SELECT DISTINCT c.owner_id AS id
FROM comment c
JOIN comment_thread ct
ON ct.id = c.thread_id
WHERE ct.file_id = ?::uuid
),
profile_ids AS (
SELECT id FROM available_profiles
UNION
SELECT ?::uuid
)
SELECT p.id,
p.email,
p.fullname AS name,
p.fullname,
p.photo_id,
p.is_active
FROM profile p
JOIN profile_ids AS x
ON x.id = p.id;")
(file-comment-users-sql ""))
(def ^:private sql:file-comment-users-by-page
(file-comment-users-sql "AND ct.page_id = ANY(?::uuid[])"))
(defn get-file-comments-users
[conn file-id profile-id]
(db/exec! conn [sql:file-comment-users file-id profile-id]))
"Returns the profiles of the users that commented the file. When `pages` is
not nil, only the commenters of the threads on those pages are returned."
[conn file-id profile-id pages]
(if (some? pages)
(db/exec! conn [sql:file-comment-users-by-page
file-id
(db/create-array conn "uuid" pages)
profile-id])
(db/exec! conn [sql:file-comment-users file-id profile-id])))
(def ^:private
schema:get-profiles-for-file-comments
@ -423,14 +450,17 @@
(sv/defmethod ::get-profiles-for-file-comments
"Retrieves a list of profiles with limited set of properties of all
participants on comment threads of the file."
participants on comment threads of the file. When the request is done with
a share link, only the participants on the pages granted by the link are
returned."
{::doc/added "1.15"
::doc/changes ["1.15" "Imported from queries and renamed."]
::doc/changes ["1.15" "Imported from queries and renamed."
"2.19" "With a share link, only the participants on the pages granted by the link are returned."]
::sm/params schema:get-profiles-for-file-comments}
[cfg {:keys [::rpc/profile-id file-id share-id]}]
(db/run! cfg (fn [{:keys [::db/conn] :as cfg}]
(files/check-comment-permissions! cfg profile-id file-id share-id)
(get-file-comments-users conn file-id profile-id))))
(let [perms (files/check-comment-permissions! cfg profile-id file-id share-id)]
(get-file-comments-users conn file-id profile-id (share-link-pages perms))))))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; MUTATION COMMANDS
@ -472,21 +502,22 @@
::rtry/when rtry/conflict-exception?
::sm/params schema:create-comment-thread}
[cfg {:keys [::rpc/profile-id file-id page-id share-id] :as params}]
(files/check-comment-permissions! cfg profile-id file-id share-id)
(let [perms (files/check-comment-permissions! cfg profile-id file-id share-id)]
(check-page-scope perms page-id)
(let [{:keys [team-id project-id] :as file} (get-file cfg file-id page-id)]
(-> cfg
(assoc ::quotes/profile-id profile-id)
(assoc ::quotes/team-id team-id)
(assoc ::quotes/project-id project-id)
(assoc ::quotes/file-id file-id)
(quotes/check! {::quotes/id ::quotes/comment-threads-per-file}
{::quotes/id ::quotes/comments-per-file}))
(let [{:keys [team-id project-id] :as file} (get-file cfg file-id page-id)]
(-> cfg
(assoc ::quotes/profile-id profile-id)
(assoc ::quotes/team-id team-id)
(assoc ::quotes/project-id project-id)
(assoc ::quotes/file-id file-id)
(quotes/check! {::quotes/id ::quotes/comment-threads-per-file}
{::quotes/id ::quotes/comments-per-file}))
(let [params (assoc params ::file file)
thread (db/tx-run! cfg create-comment-thread params)]
(let [params (assoc params ::file file)
thread (db/tx-run! cfg create-comment-thread params)]
(vary-meta thread assoc ::audit/props (assoc thread :team-id team-id)))))
(vary-meta thread assoc ::audit/props (assoc thread :team-id team-id))))))
(defn- create-comment-thread
[{:keys [::db/conn] :as cfg}
@ -557,8 +588,9 @@
::sm/params schema:update-comment-thread-status
::db/transaction true}
[{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id id share-id]}]
(let [{:keys [file-id]} (get-comment-thread conn id ::sql/for-update true)]
(files/check-comment-permissions! cfg profile-id file-id share-id)
(let [thread (get-comment-thread conn id ::sql/for-update true)
perms (files/check-comment-permissions! cfg profile-id (:file-id thread) share-id)]
(check-page-scope perms (:page-id thread))
(upsert-comment-thread-status! conn profile-id id)))
;; --- COMMAND: Update Comment Thread
@ -575,8 +607,9 @@
::sm/params schema:update-comment-thread
::db/transaction true}
[{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id id is-resolved share-id]}]
(let [{:keys [file-id]} (get-comment-thread conn id ::sql/for-update true)]
(files/check-comment-permissions! cfg profile-id file-id share-id)
(let [thread (get-comment-thread conn id ::sql/for-update true)
perms (files/check-comment-permissions! cfg profile-id (:file-id thread) share-id)]
(check-page-scope perms (:page-id thread))
(db/update! conn :comment-thread
{:is-resolved is-resolved}
{:id id})
@ -601,56 +634,58 @@
(let [{:keys [file-id page-id] :as thread}
(get-comment-thread conn thread-id ::sql/for-update true)
{:keys [team-id project-id] :as file}
(get-file cfg file-id page-id)]
perms (files/check-comment-permissions! cfg profile-id file-id share-id)]
(files/check-comment-permissions! cfg profile-id file-id share-id)
(check-page-scope perms page-id)
(quotes/check! cfg {::quotes/id ::quotes/comments-per-file
::quotes/profile-id profile-id
::quotes/team-id team-id
::quotes/project-id project-id
::quotes/file-id file-id})
(let [{:keys [team-id project-id] :as file}
(get-file cfg file-id page-id)]
(let [profile (profile/get-profile conn profile-id)
mentions (into #{} mentions)
params {:id (uuid/next)
:created-at request-at
:modified-at request-at
:thread-id thread-id
:owner-id profile-id
:content content
:mentions (db/encode-pgarray mentions conn "uuid")}
(quotes/check! cfg {::quotes/id ::quotes/comments-per-file
::quotes/profile-id profile-id
::quotes/team-id team-id
::quotes/project-id project-id
::quotes/file-id file-id})
comment (-> (db/insert! conn :comment params)
(decode-row)
(assoc :file-id file-id)
(add-owner profile))]
(let [profile (profile/get-profile conn profile-id)
mentions (into #{} mentions)
params {:id (uuid/next)
:created-at request-at
:modified-at request-at
:thread-id thread-id
:owner-id profile-id
:content content
:mentions (db/encode-pgarray mentions conn "uuid")}
;; Update thread modified-at attribute and assoc the current
;; profile to the participant set.
(let [mentions (into (:mentions thread) mentions)
participants (-> (:participants thread #{})
(conj profile-id))
params {:modified-at request-at
:participants (db/tjson participants)
:mentions (db/encode-pgarray mentions conn "uuid")}
comment (-> (db/insert! conn :comment params)
(decode-row)
(assoc :file-id file-id)
(add-owner profile))]
;; Update the page-name cached attribute on comment thread table.
params (cond-> params
(not= (:page-name file) (:page-name thread))
(assoc :page-name (:page-name file)))]
;; Update thread modified-at attribute and assoc the current
;; profile to the participant set.
(let [mentions (into (:mentions thread) mentions)
participants (-> (:participants thread #{})
(conj profile-id))
params {:modified-at request-at
:participants (db/tjson participants)
:mentions (db/encode-pgarray mentions conn "uuid")}
(db/update! conn :comment-thread params
{:id thread-id}
{::db/return-keys false}))
;; Update the page-name cached attribute on comment thread table.
params (cond-> params
(not= (:page-name file) (:page-name thread))
(assoc :page-name (:page-name file)))]
;; Update the current profile status in relation to the current thread
(upsert-comment-thread-status! conn profile-id thread-id)
(db/update! conn :comment-thread params
{:id thread-id}
{::db/return-keys false}))
(send-comment-emails! conn profile comment thread file)
;; Update the current profile status in relation to the current thread
(upsert-comment-thread-status! conn profile-id thread-id)
(vary-meta comment assoc ::audit/props (assoc comment :team-id team-id)))))
(send-comment-emails! conn profile comment thread file)
(vary-meta comment assoc ::audit/props (assoc comment :team-id team-id))))))
;; --- COMMAND: Update Comment
@ -673,9 +708,11 @@
(get-comment conn id ::sql/for-update true)
{:keys [file-id page-id] :as thread}
(get-comment-thread conn thread-id ::sql/for-update true)]
(get-comment-thread conn thread-id ::sql/for-update true)
(files/check-comment-permissions! cfg profile-id file-id share-id)
perms (files/check-comment-permissions! cfg profile-id file-id share-id)]
(check-page-scope perms page-id)
;; Don't allow edit comments to not owners
(when-not (= owner-id profile-id)
@ -713,8 +750,13 @@
::sm/params schema:delete-comment-thread
::db/transaction true}
[{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id id share-id]}]
(let [{:keys [owner-id file-id] :as thread} (get-comment-thread conn id ::sql/for-update true)]
(files/check-comment-permissions! cfg profile-id file-id share-id)
(let [{:keys [owner-id file-id page-id] :as thread}
(get-comment-thread conn id ::sql/for-update true)
perms (files/check-comment-permissions! cfg profile-id file-id share-id)]
(check-page-scope perms page-id)
(when-not (= owner-id profile-id)
(ex/raise :type :validation
:code :not-allowed))
@ -739,10 +781,12 @@
(let [{:keys [owner-id thread-id] :as comment}
(get-comment conn id ::sql/for-update true)
{:keys [file-id]}
(get-comment-thread conn thread-id)]
{:keys [file-id page-id]}
(get-comment-thread conn thread-id)
(files/check-comment-permissions! cfg profile-id file-id share-id)
perms (files/check-comment-permissions! cfg profile-id file-id share-id)]
(check-page-scope perms page-id)
(when-not (= owner-id profile-id)
(ex/raise :type :validation
:code :not-allowed))
@ -766,8 +810,9 @@
::sm/params schema:update-comment-thread-position
::db/transaction true}
[{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id ::rpc/request-at id position frame-id share-id]}]
(let [{:keys [file-id]} (get-comment-thread conn id ::sql/for-update true)]
(files/check-comment-permissions! cfg profile-id file-id share-id)
(let [thread (get-comment-thread conn id ::sql/for-update true)
perms (files/check-comment-permissions! cfg profile-id (:file-id thread) share-id)]
(check-page-scope perms (:page-id thread))
(db/update! conn :comment-thread
{:modified-at request-at
:position (db/pgpoint position)
@ -790,8 +835,9 @@
::sm/params schema:update-comment-thread-frame
::db/transaction true}
[{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id ::rpc/request-at id frame-id share-id]}]
(let [{:keys [file-id]} (get-comment-thread conn id ::sql/for-update true)]
(files/check-comment-permissions! cfg profile-id file-id share-id)
(let [thread (get-comment-thread conn id ::sql/for-update true)
perms (files/check-comment-permissions! cfg profile-id (:file-id thread) share-id)]
(check-page-scope perms (:page-id thread))
(db/update! conn :comment-thread
{:modified-at request-at
:frame-id frame-id}

355
backend/test/backend_tests/rpc_comment_test.clj
View File

@ -323,6 +323,14 @@
:frame-id uuid/zero
:share-id share-id})]
(t/is (not (th/success? out)))
(t/is (= :not-found (th/ex-type (:error out))))))
(t/testing "outsider with who-comment=team share-link cannot get-profiles-for-file-comments"
(let [out (th/command! {::th/type :get-profiles-for-file-comments
::rpc/profile-id (:id outsider)
:file-id (:id file)
:share-id share-id})]
(t/is (not (th/success? out)))
(t/is (= :not-found (th/ex-type (:error out))))))))
(t/deftest share-link-who-comment-all-can-comment
@ -449,6 +457,292 @@
(t/is (not (th/success? out)))
(t/is (= :not-found (th/ex-type (:error out))))))))
(t/deftest share-link-page-scope-enforced-for-comment-profiles
(let [owner (th/create-profile* 1 {:is-active true})
outsider (th/create-profile* 2 {:is-active true})
member (th/create-profile* 3 {:is-active true})
team (th/create-team* 1 {:profile-id (:id owner)})
_ (th/create-team-role* {:team-id (:id team)
:profile-id (:id member)
:role :editor})
project (th/create-project* 1 {:team-id (:id team)
:profile-id (:id owner)})
file (th/create-file* 1 {:profile-id (:id owner)
:project-id (:id project)})
page-a (get-in file [:data :pages 0])
page-b (uuid/random)
_ (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 {}}}]})
thread-a (th/command! {::th/type :create-comment-thread
::rpc/profile-id (:id owner)
:file-id (:id file)
:page-id page-a
:position (gpt/point 0)
:content "comment on page A"
:frame-id uuid/zero})
thread-b (th/command! {::th/type :create-comment-thread
::rpc/profile-id (:id member)
:file-id (:id file)
:page-id page-b
:position (gpt/point 0)
:content "comment on page B"
:frame-id uuid/zero})
thread-a-id (get-in thread-a [:result :id])
thread-b-id (get-in thread-b [:result :id])
thread-b-comment-id (get-in thread-b [:result :comment-id])
share (th/command! {::th/type :create-share-link
::rpc/profile-id (:id owner)
:file-id (:id file)
:pages #{page-a}
:who-comment "all"
:who-inspect "all"})
share-id (get-in share [:result :id])
;; A link without pages grants access to nothing.
empty-share (th/command! {::th/type :create-share-link
::rpc/profile-id (:id owner)
:file-id (:id file)
:pages #{}
:who-comment "all"
:who-inspect "all"})
empty-share-id (get-in empty-share [:result :id])]
(t/testing "the comment threads are created"
(t/is (th/success? thread-a))
(t/is (th/success? thread-b)))
(t/testing "share-link holder only gets the profiles of the shared page commenters"
(let [out (th/command! {::th/type :get-profiles-for-file-comments
::rpc/profile-id (:id outsider)
:file-id (:id file)
:share-id share-id})
emails (set (map :email (:result out)))]
(t/is (th/success? out))
(t/is (contains? emails (:email owner)))
(t/is (contains? emails (:email outsider)))
(t/is (not (contains? emails (:email member))))))
(t/testing "share-link holder cannot update the status of a thread of an unshared page"
(let [out (th/command! {::th/type :update-comment-thread-status
::rpc/profile-id (:id outsider)
:id thread-b-id
:share-id share-id})]
(t/is (not (th/success? out)))
(t/is (= :not-found (th/ex-type (:error out))))))
(t/testing "share-link holder cannot update a comment of an unshared page"
(let [out (th/command! {::th/type :update-comment
::rpc/profile-id (:id outsider)
:id thread-b-comment-id
:content "outsider edit on page B"
:share-id share-id})]
(t/is (not (th/success? out)))
(t/is (= :not-found (th/ex-type (:error out))))))
(t/testing "share-link holder cannot delete a comment of an unshared page"
(let [out (th/command! {::th/type :delete-comment
::rpc/profile-id (:id outsider)
:id thread-b-comment-id
:share-id share-id})]
(t/is (not (th/success? out)))
(t/is (= :not-found (th/ex-type (:error out))))))
(t/testing "share-link holder cannot delete a thread of an unshared page"
(let [out (th/command! {::th/type :delete-comment-thread
::rpc/profile-id (:id outsider)
:id thread-b-id
:share-id share-id})]
(t/is (not (th/success? out)))
(t/is (= :not-found (th/ex-type (:error out))))))
(t/testing "share-link holder cannot create a thread on an unshared page"
(let [out (th/command! {::th/type :create-comment-thread
::rpc/profile-id (:id outsider)
:file-id (:id file)
:page-id page-b
:position (gpt/point 0)
:content "comment on page B"
:frame-id uuid/zero
:share-id share-id})]
(t/is (not (th/success? out)))
(t/is (= :not-found (th/ex-type (:error out))))))
(t/testing "share-link holder cannot move a thread of an unshared page"
(let [out (th/command! {::th/type :update-comment-thread-position
::rpc/profile-id (:id outsider)
:id thread-b-id
:position (gpt/point 10 10)
:frame-id uuid/zero
:share-id share-id})]
(t/is (not (th/success? out)))
(t/is (= :not-found (th/ex-type (:error out))))))
(t/testing "share-link holder cannot resize a thread of an unshared page"
(let [out (th/command! {::th/type :update-comment-thread-frame
::rpc/profile-id (:id outsider)
:id thread-b-id
:frame-id uuid/zero
:share-id share-id})]
(t/is (not (th/success? out)))
(t/is (= :not-found (th/ex-type (:error out))))))
(t/testing "share-link holder can move a thread of the shared page"
(let [out (th/command! {::th/type :update-comment-thread-position
::rpc/profile-id (:id outsider)
:id thread-a-id
:position (gpt/point 10 10)
:frame-id uuid/zero
:share-id share-id})]
(t/is (th/success? out))))
(t/testing "share-link holder can resize a thread of the shared page"
(let [out (th/command! {::th/type :update-comment-thread-frame
::rpc/profile-id (:id outsider)
:id thread-a-id
:frame-id uuid/zero
:share-id share-id})]
(t/is (th/success? out))))
(t/testing "share-link holder cannot reply to a thread of an unshared page"
(let [out (th/command! {::th/type :create-comment
::rpc/profile-id (:id outsider)
:thread-id thread-b-id
:content "outsider reply on page B"
:share-id share-id})]
(t/is (not (th/success? out)))
(t/is (= :not-found (th/ex-type (:error out))))))
(t/testing "share-link holder can reply to a thread of the shared page"
(let [out (th/command! {::th/type :create-comment
::rpc/profile-id (:id outsider)
:thread-id thread-a-id
:content "outsider reply on page A"
:share-id share-id})]
(t/is (th/success? out))))
(t/testing "share-link holder cannot resolve a thread of an unshared page"
(let [out (th/command! {::th/type :update-comment-thread
::rpc/profile-id (:id outsider)
:id thread-b-id
:is-resolved true
:share-id share-id})]
(t/is (not (th/success? out)))
(t/is (= :not-found (th/ex-type (:error out))))))
(t/testing "share-link holder can resolve a thread of the shared page"
(let [out (th/command! {::th/type :update-comment-thread
::rpc/profile-id (:id outsider)
:id thread-a-id
:is-resolved true
:share-id share-id})]
(t/is (th/success? out))))
(t/testing "share-link holder can write on the shared page"
(let [thread (th/command! {::th/type :create-comment-thread
::rpc/profile-id (:id outsider)
:file-id (:id file)
:page-id page-a
:position (gpt/point 0)
:content "outsider comment on page A"
:frame-id uuid/zero
:share-id share-id})
thread-id (get-in thread [:result :id])
comment-id (get-in thread [:result :comment-id])]
(t/is (th/success? thread))
(t/testing "can mark a thread as read"
(let [out (th/command! {::th/type :update-comment-thread-status
::rpc/profile-id (:id outsider)
:id thread-id
:share-id share-id})]
(t/is (th/success? out))))
(t/testing "can edit its comment"
(let [out (th/command! {::th/type :update-comment
::rpc/profile-id (:id outsider)
:id comment-id
:content "outsider edit on page A"
:share-id share-id})]
(t/is (th/success? out))))
(t/testing "can delete its comment"
(let [out (th/command! {::th/type :delete-comment
::rpc/profile-id (:id outsider)
:id comment-id
:share-id share-id})]
(t/is (th/success? out))))
(t/testing "can delete its thread"
(let [out (th/command! {::th/type :delete-comment-thread
::rpc/profile-id (:id outsider)
:id thread-id
:share-id share-id})]
(t/is (th/success? out))))))
(t/testing "share-link without pages does not leak the commenter profiles"
(let [out (th/command! {::th/type :get-profiles-for-file-comments
::rpc/profile-id (:id outsider)
:file-id (:id file)
:share-id empty-share-id})
emails (set (map :email (:result out)))]
(t/is (th/success? out))
(t/is (= #{(:email outsider)} emails))))
(t/testing "share-link without pages cannot create a thread"
(let [out (th/command! {::th/type :create-comment-thread
::rpc/profile-id (:id outsider)
:file-id (:id file)
:page-id page-a
:position (gpt/point 0)
:content "outsider comment on page A"
:frame-id uuid/zero
:share-id empty-share-id})]
(t/is (not (th/success? out)))
(t/is (= :not-found (th/ex-type (:error out))))))
(t/testing "share-link without pages lists no comment threads"
(let [out (th/command! {::th/type :get-comment-threads
::rpc/profile-id (:id outsider)
:file-id (:id file)
:share-id empty-share-id})]
(t/is (th/success? out))
(t/is (= [] (:result out)))))
(t/testing "share-link without pages cannot get a comment thread"
(let [out (th/command! {::th/type :get-comment-thread
::rpc/profile-id (:id outsider)
:file-id (:id file)
:id thread-a-id
:share-id empty-share-id})]
(t/is (not (th/success? out)))
(t/is (= :not-found (th/ex-type (:error out))))))
(t/testing "share-link without pages cannot get the comments of a thread"
(let [out (th/command! {::th/type :get-comments
::rpc/profile-id (:id outsider)
:thread-id thread-a-id
:share-id empty-share-id})]
(t/is (not (th/success? out)))
(t/is (= :not-found (th/ex-type (:error out))))))))
(t/deftest membership-can-still-comment
(let [owner (th/create-profile* 1 {:is-active true})
member (th/create-profile* 2 {:is-active true})
@ -461,7 +755,23 @@
:profile-id (:id owner)})
file (th/create-file* 1 {:profile-id (:id owner)
:project-id (:id project)})
page-id (get-in file [:data :pages 0])]
page-id (get-in file [:data :pages 0])
_ (th/command! {::th/type :create-comment-thread
::rpc/profile-id (:id owner)
:file-id (:id file)
:page-id page-id
:position (gpt/point 0)
:content "owner comment"
:frame-id uuid/zero})
thread (th/command! {::th/type :create-comment-thread
::rpc/profile-id (:id member)
:file-id (:id file)
:page-id page-id
:position (gpt/point 0)
:content "member comment"
:frame-id uuid/zero})]
(t/testing "team member can get-comment-threads without share-id"
(let [out (th/command! {::th/type :get-comment-threads
@ -470,14 +780,45 @@
(t/is (th/success? out))))
(t/testing "team member can create-comment-thread without share-id"
(let [out (th/command! {::th/type :create-comment-thread
(t/is (th/success? thread)))
(t/testing "team member can move a thread of the file"
(let [out (th/command! {::th/type :update-comment-thread-position
::rpc/profile-id (:id member)
:file-id (:id file)
:page-id page-id
:position (gpt/point 0)
:content "member comment"
:id (get-in thread [:result :id])
:position (gpt/point 10 10)
:frame-id uuid/zero})]
(t/is (th/success? out))))))
(t/is (th/success? out))))
(t/testing "team member can resize a thread of the file"
(let [out (th/command! {::th/type :update-comment-thread-frame
::rpc/profile-id (:id member)
:id (get-in thread [:result :id])
:frame-id uuid/zero})]
(t/is (th/success? out))))
(t/testing "team member can reply to a thread of the file"
(let [out (th/command! {::th/type :create-comment
::rpc/profile-id (:id member)
:thread-id (get-in thread [:result :id])
:content "member reply"})]
(t/is (th/success? out))))
(t/testing "team member can resolve a thread of the file"
(let [out (th/command! {::th/type :update-comment-thread
::rpc/profile-id (:id member)
:id (get-in thread [:result :id])
:is-resolved true})]
(t/is (th/success? out))))
(t/testing "team member gets the profiles of all the file commenters"
(let [out (th/command! {::th/type :get-profiles-for-file-comments
::rpc/profile-id (:id member)
:file-id (:id file)})
emails (set (map :email (:result out)))]
(t/is (th/success? out))
(t/is (contains? emails (:email member)))
(t/is (contains? emails (:email owner)))))))
(t/deftest comment-audit-events-carry-team-id
;; Comment events carry the file id, so the resolver could find the team; the