From 78d1ae2310a54e47927737529e584aa1ce90d59f Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 02 2019 09:52:22 +0000 Subject: Fix checking if the user is a committer of the repo the PR originates from There are a few cases where if the PR originates from the same repo that it targets the ``project_from`` attribute of the ``PullRequest`` object is ``None``. Therefore, we need to check if the user is a committer of either the ``project_from`` if there is one and otherwise the target repo. Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index d1590ab..46388f7 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -341,11 +341,13 @@ def request_pull(repo, requestid, username=None, namespace=None): continue trigger_ci[comment] = meta - can_rebase_branch = ( - not request.remote_git - and request.project_from - and pagure.utils.is_repo_committer(request.project_from) - ) + committer = False + if request.project_from: + committer = pagure.utils.is_repo_committer(request.project_from) + else: + committer = pagure.utils.is_repo_committer(request.project) + + can_rebase_branch = not request.remote_git and committer can_delete_branch = ( pagure_config.get("ALLOW_DELETE_BRANCH", True) and can_rebase_branch @@ -1177,7 +1179,14 @@ def merge_request_pull(repo, requestid, username=None, namespace=None): requestid=requestid, ) ) - if not pagure.utils.is_repo_committer(request.project_from): + + committer = False + if request.project_from: + committer = pagure.utils.is_repo_committer(request.project_from) + else: + committer = pagure.utils.is_repo_committer(request.project) + + if not committer: flask.flash( "You do not have permissions to delete the branch in the " "source repo", diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index 1723dc1..fc53674 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -301,6 +301,170 @@ class PagureFlaskForktests(tests.Modeltests): ) @patch("pagure.lib.notify.send_email") + def test_request_pull_delete_branch_button_no_auth(self, send_email): + """ Test the request_pull endpoint. """ + send_email.return_value = True + + tests.create_projects(self.session) + tests.create_projects_git( + os.path.join(self.path, "requests"), bare=True + ) + + set_up_git_repo( + self.session, self.path, new_project=None, branch_from="feature" + ) + + project = pagure.lib.query.get_authorized_project(self.session, "test") + self.assertEqual(len(project.requests), 1) + + # View the pull-request + output = self.app.get("/test/pull-request/1") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + "PR#1: PR from the feature branch - test\n - " + "Pagure", + output_text, + ) + self.assertIn( + 'title="View file as of 2a552b">sources', output_text + ) + # Un-authenticated user cannot see this checkbox + self.assertNotIn( + ' ", + output_text, + ) + + @patch("pagure.lib.notify.send_email") + def test_request_pull_delete_branch_button(self, send_email): + """ Test the request_pull endpoint. """ + send_email.return_value = True + + tests.create_projects(self.session) + tests.create_projects_git( + os.path.join(self.path, "requests"), bare=True + ) + + set_up_git_repo( + self.session, self.path, new_project=None, branch_from="feature" + ) + + project = pagure.lib.query.get_authorized_project(self.session, "test") + self.assertEqual(len(project.requests), 1) + + # View the pull-request + user = tests.FakeUser() + user.username = "pingou" + with tests.user_set(self.app.application, user): + output = self.app.get("/test/pull-request/1") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + "PR#1: PR from the feature branch - test\n - " + "Pagure", + output_text, + ) + self.assertIn( + 'title="View file as of 2a552b">sources', output_text + ) + self.assertIn( + ' ", + output_text, + ) + + @patch("pagure.lib.notify.send_email") + def test_request_pull_delete_branch_button_no_project_from( + self, send_email + ): + """ Test the request_pull endpoint. """ + send_email.return_value = True + + tests.create_projects(self.session) + tests.create_projects_git( + os.path.join(self.path, "requests"), bare=True + ) + + set_up_git_repo( + self.session, self.path, new_project=None, branch_from="feature" + ) + + project = pagure.lib.query.get_authorized_project(self.session, "test") + self.assertEqual(len(project.requests), 1) + project.requests[0].project_from = None + self.session.add(project.requests[0]) + self.session.commit() + + # View the pull-request + user = tests.FakeUser() + user.username = "pingou" + with tests.user_set(self.app.application, user): + output = self.app.get("/test/pull-request/1") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + "PR#1: PR from the feature branch - test\n - " + "Pagure", + output_text, + ) + self.assertIn( + 'title="View file as of 2a552b">sources', output_text + ) + self.assertIn( + ' ", + output_text, + ) + + @patch("pagure.lib.notify.send_email") + def test_request_pull_delete_branch_button_no_project_from_no_acl( + self, send_email + ): + """ Test the request_pull endpoint. """ + send_email.return_value = True + + tests.create_projects(self.session) + tests.create_projects_git( + os.path.join(self.path, "requests"), bare=True + ) + + set_up_git_repo( + self.session, self.path, new_project=None, branch_from="feature" + ) + + project = pagure.lib.query.get_authorized_project(self.session, "test") + self.assertEqual(len(project.requests), 1) + project.requests[0].project_from = None + self.session.add(project.requests[0]) + self.session.commit() + + # View the pull-request + user = tests.FakeUser() + user.username = "foo" + with tests.user_set(self.app.application, user): + output = self.app.get("/test/pull-request/1") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + "PR#1: PR from the feature branch - test\n - " + "Pagure", + output_text, + ) + self.assertIn( + 'title="View file as of 2a552b">sources', output_text + ) + self.assertNotIn( + ' ", + output_text, + ) + + @patch("pagure.lib.notify.send_email") def test_task_update_request_pull(self, send_email): """ Test the task update_pull_request endpoint. """ send_email.return_value = True