From a3cd8f60bc7f7c8f29f1ce0f0737c2778bb839d2 Mon Sep 17 00:00:00 2001
From: Maja Massarini <mmassari@redhat.com>
Date: Apr 19 2024 13:30:50 +0000
Subject: [PATCH 1/2] Owner of a PR can update is own PR


Packit needs this change to be able to update PRs
it has already created.
We are implementing this feature:
https://github.com/packit/packit/issues/2182

Related with commit 133da07764314a73452487603f6f509b05f96204

---

diff --git a/pagure/api/utils.py b/pagure/api/utils.py
index 2df505a..c2a9e33 100644
--- a/pagure/api/utils.py
+++ b/pagure/api/utils.py
@@ -203,7 +203,9 @@ def _check_pull_request_access(request, assignee=False):
 
     error = False
     # Public tickets require ticket access
-    error = not is_repo_user(request.project)
+    error = not is_repo_user(request.project) and not (
+        request.user.user == flask.g.fas_user.username
+    )
 
     if assignee:
         if (
diff --git a/tests/test_pagure_flask_api_fork_update.py b/tests/test_pagure_flask_api_fork_update.py
index 4ffbbc6..77c57f4 100644
--- a/tests/test_pagure_flask_api_fork_update.py
+++ b/tests/test_pagure_flask_api_fork_update.py
@@ -764,5 +764,239 @@ class PagureFlaskApiForkUpdatetests(tests.SimplePagureTest):
         self.assertEqual(len(project.issues[0].related_prs), 1)
 
 
+class PagureFlaskApiForkUpdateNoCommitterTests(tests.SimplePagureTest):
+    """Tests for the flask API of pagure for updating a PR
+    when the PR owner is not a committer in the target project
+    but he is the owner of the PR.
+    """
+
+    maxDiff = None
+
+    @patch("pagure.lib.git.update_git", MagicMock(return_value=True))
+    @patch("pagure.lib.notify.send_email", MagicMock(return_value=True))
+    def setUp(self):
+        """Set up the environnment, ran before every tests.
+
+        Pingou is the owner of the target project (test).
+        Maja has a fork of the test project and creates
+        a PR against Pingou parent project.
+        She should be able to update her own PR.
+        """
+        super(PagureFlaskApiForkUpdateNoCommitterTests, self).setUp()
+
+        tests.create_projects(self.session)
+        tests.add_content_git_repo(
+            os.path.join(self.path, "repos", "test.git")
+        )
+
+        # Fork
+        tests.create_user(self.session, "maja", "Maja M.", ["mm@f.com"])
+        project = pagure.lib.query.get_authorized_project(self.session, "test")
+        task = pagure.lib.query.fork_project(
+            session=self.session, user="maja", repo=project
+        )
+        self.session.commit()
+        self.assertEqual(
+            task.get(),
+            {
+                "endpoint": "ui_ns.view_repo",
+                "repo": "test",
+                "namespace": None,
+                "username": "maja",
+            },
+        )
+
+        tests.add_readme_git_repo(
+            os.path.join(self.path, "repos", "forks", "maja", "test.git")
+        )
+        project = pagure.lib.query.get_authorized_project(self.session, "test")
+        fork = pagure.lib.query.get_authorized_project(
+            self.session, "test", user="maja"
+        )
+
+        tests.create_tokens(self.session)
+        tests.create_tokens_acl(self.session)
+
+        req = pagure.lib.query.new_pull_request(
+            session=self.session,
+            repo_from=fork,
+            branch_from="master",
+            repo_to=project,
+            branch_to="master",
+            title="test pull-request",
+            user="maja",
+        )
+        self.session.commit()
+        self.assertEqual(req.id, 1)
+        self.assertEqual(req.title, "test pull-request")
+
+        # Assert the PR is open
+        self.session = pagure.lib.query.create_session(self.dbpath)
+        project = pagure.lib.query.get_authorized_project(self.session, "test")
+        self.assertEqual(len(project.requests), 1)
+        self.assertEqual(project.requests[0].status, "Open")
+        # Check how the PR renders in the API and the UI
+        output = self.app.get("/api/0/test/pull-request/1")
+        self.assertEqual(output.status_code, 200)
+        output = self.app.get("/test/pull-request/1")
+        self.assertEqual(output.status_code, 200)
+
+    def test_api_pull_request_updated_by_owner(self):
+        """Owners of PRs can update their own PRs."""
+
+        headers = {"Authorization": "token aaabbbcccddd"}
+
+        data = {
+            "title": "edited test PR",
+            "initial_comment": "Edited initial comment",
+        }
+
+        user = tests.FakeUser()
+        user.username = "maja"
+        with tests.user_set(self.app.application, user):
+            output = self.app.post(
+                "/api/0/test/pull-request/1", data=data, headers=headers
+            )
+
+            self.assertEqual(output.status_code, 200)
+
+    def test_api_pull_request_not_updated_by_other_user(self):
+        """No repo committers or PR owners are allowed to update PR."""
+
+        headers = {"Authorization": "token aaabbbcccddd"}
+
+        data = {
+            "title": "edited test PR",
+            "initial_comment": "Edited initial comment",
+        }
+
+        tests.create_user(self.session, "other", "Another User", ["au@rh.com"])
+        user = tests.FakeUser()
+        user.username = "other"
+        with tests.user_set(self.app.application, user):
+            output = self.app.post(
+                "/api/0/test/pull-request/1", data=data, headers=headers
+            )
+
+            self.assertEqual(output.status_code, 403)
+
+
+class PagureFlaskApiForkUpdateNoCommitterTests(tests.SimplePagureTest):
+    """Tests for the flask API of pagure for updating a PR
+    when the PR owner is not a committer in the target project
+    but he is the owner of the PR.
+    """
+
+    maxDiff = None
+
+    @patch("pagure.lib.git.update_git", MagicMock(return_value=True))
+    @patch("pagure.lib.notify.send_email", MagicMock(return_value=True))
+    def setUp(self):
+        """Set up the environnment, ran before every tests.
+
+        Pingou is the owner of the target project (test).
+        Maja has a fork of the test project and creates
+        a PR against Pingou parent project.
+        She should be able to update her own PR.
+        """
+        super(PagureFlaskApiForkUpdateNoCommitterTests, self).setUp()
+
+        tests.create_projects(self.session)
+        tests.add_content_git_repo(
+            os.path.join(self.path, "repos", "test.git")
+        )
+
+        # Fork
+        tests.create_user(self.session, "maja", "Maja M.", ["mm@f.com"])
+        project = pagure.lib.query.get_authorized_project(self.session, "test")
+        task = pagure.lib.query.fork_project(
+            session=self.session, user="maja", repo=project
+        )
+        self.session.commit()
+        self.assertEqual(
+            task.get(),
+            {
+                "endpoint": "ui_ns.view_repo",
+                "repo": "test",
+                "namespace": None,
+                "username": "maja",
+            },
+        )
+
+        tests.add_readme_git_repo(
+            os.path.join(self.path, "repos", "forks", "maja", "test.git")
+        )
+        project = pagure.lib.query.get_authorized_project(self.session, "test")
+        fork = pagure.lib.query.get_authorized_project(
+            self.session, "test", user="maja"
+        )
+
+        tests.create_tokens(self.session)
+        tests.create_tokens_acl(self.session)
+
+        req = pagure.lib.query.new_pull_request(
+            session=self.session,
+            repo_from=fork,
+            branch_from="master",
+            repo_to=project,
+            branch_to="master",
+            title="test pull-request",
+            user="maja",
+        )
+        self.session.commit()
+        self.assertEqual(req.id, 1)
+        self.assertEqual(req.title, "test pull-request")
+
+        # Assert the PR is open
+        self.session = pagure.lib.query.create_session(self.dbpath)
+        project = pagure.lib.query.get_authorized_project(self.session, "test")
+        self.assertEqual(len(project.requests), 1)
+        self.assertEqual(project.requests[0].status, "Open")
+        # Check how the PR renders in the API and the UI
+        output = self.app.get("/api/0/test/pull-request/1")
+        self.assertEqual(output.status_code, 200)
+        output = self.app.get("/test/pull-request/1")
+        self.assertEqual(output.status_code, 200)
+
+    def test_api_pull_request_updated_by_owner(self):
+        """Owners of PRs can update their own PRs."""
+
+        headers = {"Authorization": "token aaabbbcccddd"}
+
+        data = {
+            "title": "edited test PR",
+            "initial_comment": "Edited initial comment",
+        }
+
+        user = tests.FakeUser()
+        user.username = "maja"
+        with tests.user_set(self.app.application, user):
+            output = self.app.post(
+                "/api/0/test/pull-request/1", data=data, headers=headers
+            )
+
+            self.assertEqual(output.status_code, 200)
+
+    def test_api_pull_request_not_updated_by_other_user(self):
+        """No repo committers or PR owners are allowed to update PR."""
+
+        headers = {"Authorization": "token aaabbbcccddd"}
+
+        data = {
+            "title": "edited test PR",
+            "initial_comment": "Edited initial comment",
+        }
+
+        tests.create_user(self.session, "other", "Another User", ["au@rh.com"])
+        user = tests.FakeUser()
+        user.username = "other"
+        with tests.user_set(self.app.application, user):
+            output = self.app.post(
+                "/api/0/test/pull-request/1", data=data, headers=headers
+            )
+
+            self.assertEqual(output.status_code, 403)
+
+
 if __name__ == "__main__":
     unittest.main(verbosity=2)

From c8ea20215862e8ea8611fec66e1a386aff91c65c Mon Sep 17 00:00:00 2001
From: Maja Massarini <mmassari@redhat.com>
Date: Apr 19 2024 13:30:50 +0000
Subject: [PATCH 2/2] Allow author to update PR but not to change assignee


---

diff --git a/pagure/api/fork.py b/pagure/api/fork.py
index cfffecc..f8bd223 100644
--- a/pagure/api/fork.py
+++ b/pagure/api/fork.py
@@ -502,7 +502,7 @@ def api_pull_request_update(repo, requestid, username=None, namespace=None):
     _check_token(repo, project_token=False)
 
     request = _get_request(repo, requestid)
-    _check_pull_request_access(request, assignee=True)
+    _check_pull_request_access(request, assignee=True, allow_author=True)
 
     form = pagure.forms.RequestPullForm(meta={"csrf": False})
     if not form.validate_on_submit():
diff --git a/pagure/api/utils.py b/pagure/api/utils.py
index c2a9e33..924f03e 100644
--- a/pagure/api/utils.py
+++ b/pagure/api/utils.py
@@ -190,12 +190,14 @@ def _check_private_issue_access(issue):
         )
 
 
-def _check_pull_request_access(request, assignee=False):
+def _check_pull_request_access(request, assignee=False, allow_author=False):
     """Check if user can access Pull-Request. Must be repo committer
-    or author to see private pull-requests.
+    or author (if flag is true) to see private pull-requests.
     :param request: PullRequest object
     :param assignee: a boolean specifying whether to allow the assignee or not
         defaults to False
+    :param allow_author: a boolean specifying whether the PR author should be
+        allowed, defaults to False
     :raises pagure.exceptions.APIError: when access denied
     """
     # Private PRs require commit access
@@ -204,7 +206,7 @@ def _check_pull_request_access(request, assignee=False):
     error = False
     # Public tickets require ticket access
     error = not is_repo_user(request.project) and not (
-        request.user.user == flask.g.fas_user.username
+        allow_author and request.user.user == flask.g.fas_user.username
     )
 
     if assignee:

