fix(security): do not expose draft comments via API to other users than caller

rhodecode/api/tests/test_get_pull_request_comments.py

@@ -20,6 +20,8 @@
 import pytest
 
 from rhodecode.api.tests.utils import build_data, api_call, assert_error, assert_ok
+from rhodecode.model.db import ChangesetComment
+from rhodecode.model.meta import Session
 
 pytestmark = pytest.mark.backends("git", "hg")
 
@@ -77,3 +79,57 @@ class TestGetPullRequestComments(object):
 
         expected = "pull request `666` does not exist"
         assert_error(id_, expected, given=response.body)
+
+    def test_api_get_pull_request_comments_drafts_permissions(self, pr_util, http_host_only_stub):
+        from rhodecode.model.pull_request import PullRequestModel
+
+        pull_request = pr_util.create_pull_request(mergeable=True)
+
+        # REGULAR USER CALLING
+        id_, params = build_data(
+            self.apikey_regular, "get_pull_request_comments", pullrequestid=pull_request.pull_request_id
+        )
+
+        response = api_call(self.app, params)
+        authors = [x["comment_author"]["username"] for x in response.json["result"]]
+        assert authors == ["test_admin"]
+
+        # ADMIN CALLING
+        id_, params = build_data(self.apikey, "get_pull_request_comments", pullrequestid=pull_request.pull_request_id)
+
+        response = api_call(self.app, params)
+        authors = [x["comment_author"]["username"] for x in response.json["result"]]
+        assert authors == ["test_admin"]
+
+        # NOW REGULAR USER MAKES DRAFT COMMENTS....
+        id_, params = build_data(
+            self.apikey_regular,
+            "comment_pull_request",
+            repoid=pull_request.target_repo.repo_name,
+            pullrequestid=pull_request.pull_request_id,
+            message="test message",
+        )
+        response = api_call(self.app, params)
+        # turn the comment into inline draft... (drafts are only inline comments...)
+        comment = ChangesetComment().get(response.json["result"]["comment_id"])
+        comment.draft = True
+        comment.f_path = "hello.pl"
+        comment.line_no = "1"
+        Session().add(comment)
+        Session().commit()
+
+        # REGULAR USER SHOULD SEE IT
+        id_, params = build_data(
+            self.apikey_regular, "get_pull_request_comments", pullrequestid=pull_request.pull_request_id
+        )
+
+        response = api_call(self.app, params)
+        authors = sorted([x["comment_author"]["username"] for x in response.json["result"]])
+        assert authors == ["test-api", "test_admin"]
+
+        # ADMIN SHOULD NOT SEE IT
+        id_, params = build_data(self.apikey, "get_pull_request_comments", pullrequestid=pull_request.pull_request_id)
+
+        response = api_call(self.app, params)
+        authors = [x["comment_author"]["username"] for x in response.json["result"]]
+        assert authors == ["test_admin"]

rhodecode/api/views/pull_request_api.py

@@ -39,7 +39,15 @@ from rhodecode.lib.utils2 import str2bool
 from rhodecode.lib.vcs.backends.base import unicode_to_reference
 from rhodecode.model.changeset_status import ChangesetStatusModel
 from rhodecode.model.comment import CommentsModel
-from rhodecode.model.db import Session, ChangesetStatus, ChangesetComment, PullRequest, PullRequestReviewers
+from rhodecode.model.db import (
+    Session,
+    ChangesetStatus,
+    ChangesetComment,
+    PullRequest,
+    PullRequestReviewers,
+    false,
+    true,
+)
 from rhodecode.model.pull_request import PullRequestModel, MergeCheck
 from rhodecode.model.settings import SettingsModel
 from rhodecode.model.validation_schema import Invalid
@@ -398,7 +406,7 @@ def get_pull_request_comments(request, apiuser, pullrequestid, repoid=Optional(N
         repo = pull_request.target_repo
 
     if not PullRequestModel().check_user_read(pull_request, apiuser, api=True):
-        raise JSONRPCError("repository `%s` or pull request `%s` does not exist" % (repoid, pullrequestid))
+        raise JSONRPCError(f"repository `{repoid}` or pull request `{pullrequestid}` does not exist")
 
     (pull_request_latest, pull_request_at_ver, pull_request_display_obj, at_version) = (
         PullRequestModel().get_pr_version(pull_request.pull_request_id, version=None)
@@ -410,19 +418,29 @@ def get_pull_request_comments(request, apiuser, pullrequestid, repoid=Optional(N
     # GENERAL COMMENTS with versions #
     q = CommentsModel()._all_general_comments_of_pull_request(pull_request)
     q = q.order_by(ChangesetComment.comment_id.asc())
+    q = q.filter(ChangesetComment.draft == false())
     general_comments = q.all()
 
     # INLINE COMMENTS with versions  #
     q = CommentsModel()._all_inline_comments_of_pull_request(pull_request)
     q = q.order_by(ChangesetComment.comment_id.asc())
+    q = q.filter(ChangesetComment.draft == false())
     inline_comments = q.all()
 
+    # INLINE DRAFT COMMENTS with versions  #
+    q = CommentsModel()._all_inline_comments_of_pull_request(pull_request)
+    q = q.order_by(ChangesetComment.comment_id.asc())
+    q = q.filter(ChangesetComment.user_id == apiuser.user_id)
+    q = q.filter(ChangesetComment.draft == true())
+    inline_draft_comments = q.all()
+    log.error(q)
+
     data = []
-    for comment in inline_comments + general_comments:
+    for comment in inline_draft_comments + inline_comments + general_comments:
         full_data = comment.get_api_data()
         pr_version_id = None
         if comment.pull_request_version_id:
-            pr_version_id = "v{}".format(ver_map[comment.pull_request_version_id])
+            pr_version_id = f"v{ver_map[comment.pull_request_version_id]}"
 
         # sanitize some entries
 
@@ -566,7 +584,7 @@ def comment_pull_request(
     text = message
     status_label = ChangesetStatus.get_status_lbl(status)
     if status and allowed_to_change_status:
-        st_message = "Status change %(transition_icon)s %(status)s" % {"transition_icon": ">", "status": status_label}
+        st_message = f"Status change > {status_label}"
         text = message or st_message
 
     rc_config = SettingsModel().get_all_settings()