From 0ede451e740f4f5c2a4da6f887ec71abad1b0f6e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 03 2018 13:56:32 +0000 Subject: [PATCH 1/3] Add a new API endpoint to retrieve the options of a project This API endpoint allows people with the proper API token to access the current settings/options set for a project. Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/api/__init__.py b/pagure/api/__init__.py index 2453d97..7705d52 100644 --- a/pagure/api/__init__.py +++ b/pagure/api/__init__.py @@ -525,6 +525,7 @@ def api(): api_update_project_watchers_doc = load_doc( project.api_update_project_watchers ) + api_get_project_options_doc = load_doc(project.api_get_project_options) issues = [] if pagure_config.get("ENABLE_TICKETS", True): @@ -609,6 +610,7 @@ def api(): api_commit_flags_doc, api_commit_add_flag_doc, api_update_project_watchers_doc, + api_get_project_options_doc, ], issues=issues, requests=[ diff --git a/pagure/api/project.py b/pagure/api/project.py index 1a8de5d..9ea05af 100644 --- a/pagure/api/project.py +++ b/pagure/api/project.py @@ -1898,3 +1898,66 @@ def api_modify_acls(repo, namespace=None, username=None): jsonout = flask.jsonify(output) return jsonout + + +@API.route("//options", methods=["GET"]) +@API.route("///options", methods=["GET"]) +@API.route("/fork///options", methods=["GET"]) +@API.route("/fork////options", methods=["GET"]) +@api_login_required(acls=["modify_project"]) +@api_method +def api_get_project_options(repo, username=None, namespace=None): + """ + Get project options + ---------------------- + Allow project admins to retrieve the current options of a project. + + :: + + GET /api/0//options + GET /api/0///options + + :: + + GET /api/0/fork///options + GET /api/0/fork////options + + Sample response + ^^^^^^^^^^^^^^^ + + :: + + { + "settings": { + "Enforce_signed-off_commits_in_pull-request": false, + "Minimum_score_to_merge_pull-request": -1, + "Only_assignee_can_merge_pull-request": false, + "Web-hooks": null, + "always_merge": false, + "disable_non_fast-forward_merges": false, + "fedmsg_notifications": true, + "issue_tracker": true, + "issue_tracker_read_only": false, + "issues_default_to_private": false, + "notify_on_commit_flag": false, + "notify_on_pull-request_flag": false, + "open_metadata_access_to_all": false, + "project_documentation": false, + "pull_request_access_only": false, + "pull_requests": true, + "stomp_notifications": true + }, + "status": "ok" + } + + """ + project = get_authorized_api_project( + flask.g.session, repo, namespace=namespace + ) + if not project: + raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) + + if flask.g.token.project and project != flask.g.token.project: + raise pagure.exceptions.APIError(401, error_code=APIERROR.EINVALIDTOK) + + return flask.jsonify({"settings": project.settings, "status": "ok"}) diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index 16b5862..ecf758a 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -3789,5 +3789,88 @@ class PagureFlaskApiProjectModifyAclTests(tests.Modeltests): {u'admin': [], u'commit': [], u'ticket': []} ) + +class PagureFlaskApiProjectOptionsTests(tests.Modeltests): + """ Tests for the flask API of pagure for modifying options ofs a project + """ + + maxDiff = None + + def setUp(self): + """ Set up the environnment, ran before every tests. """ + super(PagureFlaskApiProjectOptionsTests, self).setUp() + tests.create_projects(self.session) + tests.create_tokens(self.session, project_id=None) + tests.create_tokens_acl( + self.session, 'aaabbbcccddd', 'modify_project') + + project = pagure.lib.query._get_project(self.session, 'test') + self.assertEquals( + project.access_users, + {u'admin': [], u'commit': [], u'ticket': []} + ) + + def test_api_get_project_options_wrong_project(self): + """ Test accessing api_get_project_options w/o auth header. """ + + headers = {'Authorization': 'token aaabbbcccddd'} + output = self.app.get('/api/0/unknown/options', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + {u'error': u'Project not found', u'error_code': u'ENOPROJECT'} + ) + + def test_api_get_project_options_wo_header(self): + """ Test accessing api_get_project_options w/o auth header. """ + + output = self.app.get('/api/0/test/options') + self.assertEqual(output.status_code, 401) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + { + u'error': u'Invalid or expired token. Please visit ' + 'http://localhost.localdomain/settings#api-keys to get ' + 'or renew your API token.', + u'error_code': u'EINVALIDTOK' + } + ) + + def test_api_get_project_options_w_header(self): + """ Test accessing api_get_project_options w/ auth header. """ + + headers = {'Authorization': 'token aaabbbcccddd'} + output = self.app.get('/api/0/test/options', headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + { + "settings": { + "Enforce_signed-off_commits_in_pull-request": False, + "Minimum_score_to_merge_pull-request": -1, + "Only_assignee_can_merge_pull-request": False, + "Web-hooks": None, + "always_merge": False, + "disable_non_fast-forward_merges": False, + "fedmsg_notifications": True, + "issue_tracker": True, + "issue_tracker_read_only": False, + "issues_default_to_private": False, + "notify_on_commit_flag": False, + "notify_on_pull-request_flag": False, + "open_metadata_access_to_all": False, + "project_documentation": False, + "pull_request_access_only": False, + "pull_requests": True, + "stomp_notifications": True + }, + "status": "ok" + } + ) + + if __name__ == '__main__': unittest.main(verbosity=2) From 473ccbe9349216165ea530cf3c957bac438fbe47 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 03 2018 13:56:32 +0000 Subject: [PATCH 2/3] Add a new API endpoint to update the options set for a project The idea is to use the new endpoint to retrieve the current options, adjust the options of interest and then use this endpoint to update them in pagure. Fixes https://pagure.io/pagure/issue/3892 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/api/__init__.py b/pagure/api/__init__.py index 7705d52..c079ec1 100644 --- a/pagure/api/__init__.py +++ b/pagure/api/__init__.py @@ -526,6 +526,9 @@ def api(): project.api_update_project_watchers ) api_get_project_options_doc = load_doc(project.api_get_project_options) + api_modify_project_options_doc = load_doc( + project.api_modify_project_options + ) issues = [] if pagure_config.get("ENABLE_TICKETS", True): @@ -611,6 +614,7 @@ def api(): api_commit_add_flag_doc, api_update_project_watchers_doc, api_get_project_options_doc, + api_modify_project_options_doc, ], issues=issues, requests=[ diff --git a/pagure/api/project.py b/pagure/api/project.py index 9ea05af..9babd65 100644 --- a/pagure/api/project.py +++ b/pagure/api/project.py @@ -1961,3 +1961,92 @@ def api_get_project_options(repo, username=None, namespace=None): raise pagure.exceptions.APIError(401, error_code=APIERROR.EINVALIDTOK) return flask.jsonify({"settings": project.settings, "status": "ok"}) + + +def _check_value(value): + """ Convert the provided value into a boolean, an int or leave it as it. + """ + if str(value).lower() in ["true"]: + value = True + elif str(value).lower() in ["false"]: + value = True + elif str(value).isnumeric(): + value = int(value) + return value + + +@API.route("//options/update", methods=["POST"]) +@API.route("///options/update", methods=["POST"]) +@API.route("/fork///options/update", methods=["POST"]) +@API.route( + "/fork////options/update", methods=["POST"] +) +@api_login_required(acls=["modify_project"]) +@api_method +def api_modify_project_options(repo, username=None, namespace=None): + """ + Update project options + ---------------------- + Allow project admins to modify the options of a project. + + :: + + POST /api/0//options/update + POST /api/0///options/update + + :: + + POST /api/0/fork///options/update + POST /api/0/fork////options/update + + Input + ^^^^^ + + Simply specify the key/values you would like to set. Beware that if you + do not specify in the request values that have been changed before they + will go back to their default value. + + Sample response + ^^^^^^^^^^^^^^^ + + :: + + { + 'message': 'Edited successfully settings of repo: test', + 'status': 'ok' + } + + """ + project = get_authorized_api_project( + flask.g.session, repo, namespace=namespace + ) + if not project: + raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) + + if flask.g.token.project and project != flask.g.token.project: + raise pagure.exceptions.APIError(401, error_code=APIERROR.EINVALIDTOK) + + settings = {} + for key in flask.request.form: + + settings[key] = _check_value(flask.request.form[key]) + + try: + message = pagure.lib.query.update_project_settings( + flask.g.session, + repo=project, + settings=settings, + user=flask.g.fas_user.username, + from_api=True, + ) + flask.g.session.commit() + except pagure.exceptions.PagureException as err: + raise pagure.exceptions.APIError( + 400, error_code=APIERROR.ENOCODE, error=str(err) + ) + except SQLAlchemyError as err: # pragma: no cover + flask.g.session.rollback() + _log.exception(err) + raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR) + + return flask.jsonify({"message": message, "status": "ok"}) diff --git a/pagure/lib/query.py b/pagure/lib/query.py index 03baa60..87b90b5 100644 --- a/pagure/lib/query.py +++ b/pagure/lib/query.py @@ -2119,8 +2119,15 @@ def edit_issue( return messages -def update_project_settings(session, repo, settings, user): - """ Update the settings of a project. """ +def update_project_settings(session, repo, settings, user, from_api=False): + """ Update the settings of a project. + + If from_api is true, all values that are not specified will be changed + back to their default value. + Otherwise, if from_api is False, all non-specified values are assumed + to be set to ``False`` or ``None``. + + """ user_obj = get_user(session, user) update = [] @@ -2146,9 +2153,12 @@ def update_project_settings(session, repo, settings, user): update.append(key) new_settings[key] = settings[key] else: - val = False - if key == "Web-hooks": - val = None + if from_api: + val = new_settings[key] + else: + val = False + if key == "Web-hooks": + val = None # Ensure the default value is different from what is stored. if new_settings[key] != val: diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index ecf758a..00c97d2 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -3871,6 +3871,147 @@ class PagureFlaskApiProjectOptionsTests(tests.Modeltests): } ) + def test_api_modify_project_options_wrong_project(self): + """ Test accessing api_modify_project_options w/ an invalid project. + """ + + headers = {'Authorization': 'token aaabbbcccddd'} + output = self.app.post('/api/0/unknown/options/update', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + {u'error': u'Project not found', u'error_code': u'ENOPROJECT'} + ) + + def test_api_modify_project_options_wo_header(self): + """ Test accessing api_modify_project_options w/o auth header. """ + + output = self.app.post('/api/0/test/options/update') + self.assertEqual(output.status_code, 401) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + { + u'error': u'Invalid or expired token. Please visit ' + 'http://localhost.localdomain/settings#api-keys to get ' + 'or renew your API token.', + u'error_code': u'EINVALIDTOK' + } + ) + + def test_api_modify_project_options_no_data(self): + """ Test accessing api_modify_project_options w/ auth header. """ + + # check before + headers = {'Authorization': 'token aaabbbcccddd'} + output = self.app.get('/api/0/test/options', headers=headers) + self.assertEqual(output.status_code, 200) + before = json.loads(output.get_data(as_text=True)) + self.assertEqual( + before, + { + "settings": { + "Enforce_signed-off_commits_in_pull-request": False, + "Minimum_score_to_merge_pull-request": -1, + "Only_assignee_can_merge_pull-request": False, + "Web-hooks": None, + "always_merge": False, + "disable_non_fast-forward_merges": False, + "fedmsg_notifications": True, + "issue_tracker": True, + "issue_tracker_read_only": False, + "issues_default_to_private": False, + "notify_on_commit_flag": False, + "notify_on_pull-request_flag": False, + "open_metadata_access_to_all": False, + "project_documentation": False, + "pull_request_access_only": False, + "pull_requests": True, + "stomp_notifications": True + }, + "status": "ok" + } + ) + + # Do not update anything + data = {} + output = self.app.post( + '/api/0/test/options/update', headers=headers, data=data) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + { + u'message': u'No settings to change', + u'status': u'ok' + } + ) + + # check after + headers = {'Authorization': 'token aaabbbcccddd'} + output = self.app.get('/api/0/test/options', headers=headers) + self.assertEqual(output.status_code, 200) + after = json.loads(output.get_data(as_text=True)) + self.assertEqual(after, before) + + def test_api_modify_project_options(self): + """ Test accessing api_modify_project_options w/ auth header. """ + + # check before + headers = {'Authorization': 'token aaabbbcccddd'} + output = self.app.get('/api/0/test/options', headers=headers) + self.assertEqual(output.status_code, 200) + before = json.loads(output.get_data(as_text=True)) + self.assertEqual( + before, + { + "settings": { + "Enforce_signed-off_commits_in_pull-request": False, + "Minimum_score_to_merge_pull-request": -1, + "Only_assignee_can_merge_pull-request": False, + "Web-hooks": None, + "always_merge": False, + "disable_non_fast-forward_merges": False, + "fedmsg_notifications": True, + "issue_tracker": True, + "issue_tracker_read_only": False, + "issues_default_to_private": False, + "notify_on_commit_flag": False, + "notify_on_pull-request_flag": False, + "open_metadata_access_to_all": False, + "project_documentation": False, + "pull_request_access_only": False, + "pull_requests": True, + "stomp_notifications": True + }, + "status": "ok" + } + ) + + # Update: `issues_default_to_private`. + data = {"issues_default_to_private": True} + output = self.app.post( + '/api/0/test/options/update', headers=headers, data=data) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual( + data, + { + u'message': u'Edited successfully settings of repo: test', + u'status': u'ok' + } + ) + + # check after + headers = {'Authorization': 'token aaabbbcccddd'} + output = self.app.get('/api/0/test/options', headers=headers) + self.assertEqual(output.status_code, 200) + after = json.loads(output.get_data(as_text=True)) + self.assertNotEqual(before, after) + before["settings"]["issues_default_to_private"] = True + self.assertEqual(after, before) + if __name__ == '__main__': unittest.main(verbosity=2) From 52bd6c1681bc820ad3f4c3cd8df0aa96694ffaf3 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 03 2018 13:56:32 +0000 Subject: [PATCH 3/3] Small black fix Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/api/user.py b/pagure/api/user.py index 181c606..c8c1f4b 100644 --- a/pagure/api/user.py +++ b/pagure/api/user.py @@ -942,7 +942,7 @@ def api_view_user_requests_filed(username): username=username, status=status, filed=username, - count=True + count=True, ) pagination = pagure.lib.query.get_pagination_metadata( flask.request, page, per_page, pullrequests_cnt @@ -1184,7 +1184,7 @@ def api_view_user_requests_actionable(username): username=username, status=status, actionable=username, - count=True + count=True, ) pagination = pagure.lib.query.get_pagination_metadata( flask.request, page, per_page, pullrequests_cnt