From 661557f3ecab5630e083ae498a54398ca3b5f007 Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Nov 01 2021 12:38:21 +0000 Subject: [PATCH 1/4] Don't assume project token for PR close PR close API method called the _check_token without specifying the project_token variable. This caused rejection of tokens that were created on user level. This will fix the behavior. Signed-off-by: Michal Konečný --- diff --git a/pagure/api/fork.py b/pagure/api/fork.py index abe458e..d9d9894 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -770,7 +770,7 @@ def api_pull_request_close(repo, requestid, username=None, namespace=None): repo = _get_repo(repo, username, namespace) _check_pull_request(repo) - _check_token(repo) + _check_token(repo, project_token=False) request = _get_request(repo, requestid) if not is_repo_committer(repo): From a11142c63e3e7cc6807c76953b7c0ecc49136310 Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Nov 01 2021 12:38:21 +0000 Subject: [PATCH 2/4] Allow PR author to close PR using API The validation of user on close PR call in API was only checking if the user is committer to target repo. Now it will also check if the user is author of the PR. Signed-off-by: Michal Konečný --- diff --git a/pagure/api/fork.py b/pagure/api/fork.py index d9d9894..95e7de8 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -773,7 +773,10 @@ def api_pull_request_close(repo, requestid, username=None, namespace=None): _check_token(repo, project_token=False) request = _get_request(repo, requestid) - if not is_repo_committer(repo): + if ( + not is_repo_committer(repo) + and not flask.g.fas_user.username == request.user.username + ): raise pagure.exceptions.APIError(403, error_code=APIERROR.ENOPRCLOSE) try: From bc4b361fa0cb81a2f545109327cd62aa9c87813e Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Nov 01 2021 12:38:21 +0000 Subject: [PATCH 3/4] Add test for cross-project API token Add test for closing PR using cross-project API token. Signed-off-by: Michal Konečný --- diff --git a/tests/test_pagure_flask_api_fork.py b/tests/test_pagure_flask_api_fork.py index f8ab7dd..969a079 100644 --- a/tests/test_pagure_flask_api_fork.py +++ b/tests/test_pagure_flask_api_fork.py @@ -1115,6 +1115,66 @@ class PagureFlaskApiForktests(tests.Modeltests): ) @patch("pagure.lib.notify.send_email") + def test_api_pull_request_close_cross_project_token(self, send_email): + """ Test the api_pull_request_close method of the flask api for cross-project API token. """ + send_email.return_value = True + + tests.create_projects(self.session) + tests.create_tokens(self.session) + tests.create_tokens_acl(self.session) + + # Create the pull-request to close + repo = pagure.lib.query.get_authorized_project(self.session, "test") + forked_repo = pagure.lib.query.get_authorized_project( + self.session, "test" + ) + req = pagure.lib.query.new_pull_request( + session=self.session, + repo_from=forked_repo, + branch_from="master", + repo_to=repo, + branch_to="master", + title="test pull-request", + user="foo", + ) + self.session.commit() + self.assertEqual(req.id, 1) + self.assertEqual(req.title, "test pull-request") + self.assertEqual(req.user.id, 2) + + # Create a token for foo + item = pagure.lib.model.Token( + id="foobar_token", + user_id=2, + project_id=None, + expiration=datetime.datetime.utcnow() + + datetime.timedelta(days=30), + ) + self.session.add(item) + self.session.commit() + + # Allow the token to close PR + acls = pagure.lib.query.get_acls(self.session) + for acl in acls: + if acl.name == "pull_request_close": + break + item = pagure.lib.model.TokenAcl( + token_id="foobar_token", acl_id=acl.id + ) + self.session.add(item) + self.session.commit() + + headers = {"Authorization": "token foobar_token"} + + # User is the same that created this PR + output = self.app.post( + "/api/0/test/pull-request/1/close", headers=headers + ) + self.assertEqual(output.status_code, 403) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual(data, {"message": "Pull-request closed!"}) + + @patch("pagure.lib.notify.send_email") def test_api_pull_request_close(self, send_email): """ Test the api_pull_request_close method of the flask api. """ send_email.return_value = True From 0b46e90824f8e369c7c45e8817ebd51478b1a5b2 Mon Sep 17 00:00:00 2001 From: Michal Konečný Date: Nov 01 2021 12:38:21 +0000 Subject: [PATCH 4/4] Use the correct status_code in test Signed-off-by: Michal Konečný --- diff --git a/tests/test_pagure_flask_api_fork.py b/tests/test_pagure_flask_api_fork.py index 969a079..ddb3cef 100644 --- a/tests/test_pagure_flask_api_fork.py +++ b/tests/test_pagure_flask_api_fork.py @@ -1170,7 +1170,7 @@ class PagureFlaskApiForktests(tests.Modeltests): output = self.app.post( "/api/0/test/pull-request/1/close", headers=headers ) - self.assertEqual(output.status_code, 403) + self.assertEqual(output.status_code, 200) data = json.loads(output.get_data(as_text=True)) self.assertDictEqual(data, {"message": "Pull-request closed!"})