diff --git a/.serena/memories/backend/auth-permissions-product-domains.md b/.serena/memories/backend/auth-permissions-product-domains.md index 2a58e49dfd..fc9dc51bbf 100644 --- a/.serena/memories/backend/auth-permissions-product-domains.md +++ b/.serena/memories/backend/auth-permissions-product-domains.md @@ -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 diff --git a/backend/src/app/rpc/commands/comments.clj b/backend/src/app/rpc/commands/comments.clj index 8ca0a165bc..6c83247998 100644 --- a/backend/src/app/rpc/commands/comments.clj +++ b/backend/src/app/rpc/commands/comments.clj @@ -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} diff --git a/backend/test/backend_tests/rpc_comment_test.clj b/backend/test/backend_tests/rpc_comment_test.clj index 0c58687751..0d8f6b77e5 100644 --- a/backend/test/backend_tests/rpc_comment_test.clj +++ b/backend/test/backend_tests/rpc_comment_test.clj @@ -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