From 446bcc607fee8fdc85e2987e25157018ec20d9bd Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: May 14 2018 16:45:32 +0000 Subject: [PATCH 1/5] Make API endpoint for creating new git branch have its own ACL Basically, that API endpoint was relying on the modify_project ACL which is a public ACL so users can update descriptions of their projects. It's also an ACL that can be created with non-project specific API token thus making anyone's API token with this ACL able to create new git branches in any project. This fixes CVE: CVE-2018-1002151 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/api/project.py b/pagure/api/project.py index 73a58c1..da07b9d 100644 --- a/pagure/api/project.py +++ b/pagure/api/project.py @@ -1225,7 +1225,7 @@ def api_generate_acls(repo, username=None, namespace=None): @API.route('/fork///git/branch', methods=['POST']) @API.route('/fork////git/branch', methods=['POST']) -@api_login_required(acls=['modify_project']) +@api_login_required(acls=['create_branch']) @api_method def api_new_branch(repo, username=None, namespace=None): """ @@ -1273,6 +1273,10 @@ def api_new_branch(repo, username=None, namespace=None): 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) + # Check if it's JSON or form data if flask.request.headers.get('Content-Type') == 'application/json': # Set force to True to ignore the mimetype. Set silent so that None is diff --git a/pagure/default_config.py b/pagure/default_config.py index 4233076..e81ffde 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -285,6 +285,7 @@ ACLS = { 'modify_project': 'Modify an existing project', 'generate_acls_project': 'Generate the Gitolite ACLs on a project', 'commit_flag': 'Flag a commit', + 'create_branch': 'Create a git branch on a project', } # List of ACLs which a regular user is allowed to associate to an API token @@ -309,6 +310,7 @@ ADMIN_API_ACLS = [ 'pull_request_merge', 'generate_acls_project', 'commit_flag', + 'create_branch', ] # Bootstrap URLS From e99858c816ae952c4e0eea3c24656244713eb360 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: May 14 2018 16:45:32 +0000 Subject: [PATCH 2/5] Ensure we always check the API token's project if there is one Fix CVE-2018-1002151 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/api/fork.py b/pagure/api/fork.py index 14252cf..9068994 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -24,7 +24,7 @@ import pagure.lib.tasks from pagure.api import (API, api_method, api_login_required, APIERROR, get_authorized_api_project) from pagure.config import config as pagure_config -from pagure.utils import is_repo_committer, api_authenticated, is_true +from pagure.utils import is_repo_committer, is_true _log = logging.getLogger(__name__) @@ -852,10 +852,9 @@ def api_subscribe_pull_request( raise pagure.exceptions.APIError( 404, error_code=APIERROR.EPULLREQUESTSDISABLED) - if api_authenticated(): - if flask.g.token.project and repo != flask.g.token.project: - raise pagure.exceptions.APIError( - 401, error_code=APIERROR.EINVALIDTOK) + if flask.g.token.project and repo != flask.g.token.project: + raise pagure.exceptions.APIError( + 401, error_code=APIERROR.EINVALIDTOK) request = pagure.lib.search_pull_requests( flask.g.session, project_id=repo.id, requestid=requestid) @@ -994,6 +993,10 @@ def api_pull_request_create(repo, username=None, namespace=None): if repo is None: raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) + if flask.g.token.project and repo != flask.g.token.project: + raise pagure.exceptions.APIError( + 401, error_code=APIERROR.EINVALIDTOK) + form = pagure.forms.RequestPullForm(csrf_enabled=False) if not form.validate_on_submit(): raise pagure.exceptions.APIError( diff --git a/pagure/api/project.py b/pagure/api/project.py index da07b9d..c1fcc1e 100644 --- a/pagure/api/project.py +++ b/pagure/api/project.py @@ -964,6 +964,10 @@ def api_modify_project(repo, namespace=None): 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) + is_site_admin = pagure.utils.is_admin() admins = [u.username for u in project.get_project_users('admin')] # Only allow the main admin, the admins of the project, and Pagure site @@ -1192,6 +1196,10 @@ def api_generate_acls(repo, username=None, namespace=None): 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) + # Check if it's JSON or form data if flask.request.headers.get('Content-Type') == 'application/json': # Set force to True to ignore the mimetype. Set silent so that None is From 8d8fb0fc797f616069ec5cde27b4981657abf19f Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: May 14 2018 16:45:32 +0000 Subject: [PATCH 3/5] Call the API specific function checking if the user is authenticated Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/api/project.py b/pagure/api/project.py index c1fcc1e..3f15ee7 100644 --- a/pagure/api/project.py +++ b/pagure/api/project.py @@ -226,7 +226,7 @@ def api_project_git_urls(repo, username=None, namespace=None): git_urls = {} git_url_ssh = pagure_config.get('GIT_URL_SSH') - if pagure.utils.authenticated() and git_url_ssh: + if pagure.utils.api_authenticated() and git_url_ssh: try: git_url_ssh = git_url_ssh.format( username=flask.g.fas_user.username) From ae22c6a477e0c9fc9de4edb9fe5f75b9de752bf6 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: May 14 2018 16:45:32 +0000 Subject: [PATCH 4/5] Fix the unit-tests for the changes in the API ACL checks Signed-off-by: Pierre-Yves Chibon --- diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index 539a18f..c38b0ae 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -2536,7 +2536,7 @@ class PagureFlaskApiIssuetests(tests.SimplePagureTest): # is required item = pagure.lib.model.TokenAcl( token_id='pingou_foo', - acl_id=6, + acl_id=7, ) self.session.add(item) self.session.commit() diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index 875b490..8719c6f 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -1379,59 +1379,56 @@ class PagureFlaskApiProjecttests(tests.Modeltests): tests.create_tokens_acl(self.session, 'aaabbbcccddd', 'modify_project') headers = {'Authorization': 'token aaabbbcccddd'} - user = pagure.lib.get_user(self.session, 'pingou') - user.cla_done = True - with tests.user_set(self.app.application, user): - output = self.app.patch( - '/api/0/test', headers=headers, - data={'main_admin': 'foo'}) - self.assertEqual(output.status_code, 200) - data = json.loads(output.get_data(as_text=True)) - data['date_created'] = '1496338274' - data['date_modified'] = '1496338274' - expected_output = { - "access_groups": { - "admin": [], - "commit": [], - "ticket": [] - }, - "access_users": { - "admin": [], - "commit": [], - "owner": [ - "foo" - ], - "ticket": [] - }, - "close_status": [ - "Invalid", - "Insufficient data", - "Fixed", - "Duplicate" + output = self.app.patch( + '/api/0/test', headers=headers, + data={'main_admin': 'foo'}) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + data['date_created'] = '1496338274' + data['date_modified'] = '1496338274' + expected_output = { + "access_groups": { + "admin": [], + "commit": [], + "ticket": [] + }, + "access_users": { + "admin": [], + "commit": [], + "owner": [ + "foo" ], - "custom_keys": [], - "date_created": "1496338274", - "date_modified": "1496338274", - "description": "test project #1", - "fullname": "test", - "url_path": "test", - "id": 1, - "milestones": {}, - "name": "test", - "namespace": None, - "parent": None, - "priorities": {}, - "tags": [], - "user": { - "default_email": "foo@bar.com", - "emails": [ - "foo@bar.com" - ], - "fullname": "foo bar", - "name": "foo" - } + "ticket": [] + }, + "close_status": [ + "Invalid", + "Insufficient data", + "Fixed", + "Duplicate" + ], + "custom_keys": [], + "date_created": "1496338274", + "date_modified": "1496338274", + "description": "test project #1", + "fullname": "test", + "url_path": "test", + "id": 1, + "milestones": {}, + "name": "test", + "namespace": None, + "parent": None, + "priorities": {}, + "tags": [], + "user": { + "default_email": "foo@bar.com", + "emails": [ + "foo@bar.com" + ], + "fullname": "foo bar", + "name": "foo" } - self.assertEqual(data, expected_output) + } + self.assertEqual(data, expected_output) def test_api_modify_project_main_admin_retain_access(self): """ Test the api_modify_project method of the flask api when the @@ -1442,61 +1439,58 @@ class PagureFlaskApiProjecttests(tests.Modeltests): tests.create_tokens_acl(self.session, 'aaabbbcccddd', 'modify_project') headers = {'Authorization': 'token aaabbbcccddd'} - user = pagure.lib.get_user(self.session, 'pingou') - user.cla_done = True - with tests.user_set(self.app.application, user): - output = self.app.patch( - '/api/0/test', headers=headers, - data={'main_admin': 'foo', 'retain_access': True}) - self.assertEqual(output.status_code, 200) - data = json.loads(output.get_data(as_text=True)) - data['date_created'] = '1496338274' - data['date_modified'] = '1496338274' - expected_output = { - "access_groups": { - "admin": [], - "commit": [], - "ticket": [] - }, - "access_users": { - "admin": [ - "pingou" - ], - "commit": [], - "owner": [ - "foo" - ], - "ticket": [] - }, - "close_status": [ - "Invalid", - "Insufficient data", - "Fixed", - "Duplicate" + output = self.app.patch( + '/api/0/test', headers=headers, + data={'main_admin': 'foo', 'retain_access': True}) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + data['date_created'] = '1496338274' + data['date_modified'] = '1496338274' + expected_output = { + "access_groups": { + "admin": [], + "commit": [], + "ticket": [] + }, + "access_users": { + "admin": [ + "pingou" ], - "custom_keys": [], - "date_created": "1496338274", - "date_modified": "1496338274", - "description": "test project #1", - "fullname": "test", - "url_path": "test", - "id": 1, - "milestones": {}, - "name": "test", - "namespace": None, - "parent": None, - "priorities": {}, - "tags": [], - "user": { - "default_email": "foo@bar.com", - "emails": [ - "foo@bar.com" - ], - "fullname": "foo bar", - "name": "foo" - } + "commit": [], + "owner": [ + "foo" + ], + "ticket": [] + }, + "close_status": [ + "Invalid", + "Insufficient data", + "Fixed", + "Duplicate" + ], + "custom_keys": [], + "date_created": "1496338274", + "date_modified": "1496338274", + "description": "test project #1", + "fullname": "test", + "url_path": "test", + "id": 1, + "milestones": {}, + "name": "test", + "namespace": None, + "parent": None, + "priorities": {}, + "tags": [], + "user": { + "default_email": "foo@bar.com", + "emails": [ + "foo@bar.com" + ], + "fullname": "foo bar", + "name": "foo" } - self.assertEqual(data, expected_output) + } + self.assertEqual(data, expected_output) def test_api_modify_project_main_admin_retain_access_already_user(self): """ Test the api_modify_project method of the flask api when the @@ -1516,61 +1510,58 @@ class PagureFlaskApiProjecttests(tests.Modeltests): ) self.session.commit() - user = pagure.lib.get_user(self.session, 'pingou') - user.cla_done = True - with tests.user_set(self.app.application, user): - output = self.app.patch( - '/api/0/test', headers=headers, - data={'main_admin': 'foo', 'retain_access': True}) - self.assertEqual(output.status_code, 200) - data = json.loads(output.get_data(as_text=True)) - data['date_created'] = '1496338274' - data['date_modified'] = '1496338274' - expected_output = { - "access_groups": { - "admin": [], - "commit": [], - "ticket": [] - }, - "access_users": { - "admin": [ - "pingou" - ], - "commit": [], - "owner": [ - "foo" - ], - "ticket": [] - }, - "close_status": [ - "Invalid", - "Insufficient data", - "Fixed", - "Duplicate" + output = self.app.patch( + '/api/0/test', headers=headers, + data={'main_admin': 'foo', 'retain_access': True}) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + data['date_created'] = '1496338274' + data['date_modified'] = '1496338274' + expected_output = { + "access_groups": { + "admin": [], + "commit": [], + "ticket": [] + }, + "access_users": { + "admin": [ + "pingou" ], - "custom_keys": [], - "date_created": "1496338274", - "date_modified": "1496338274", - "description": "test project #1", - "fullname": "test", - "url_path": "test", - "id": 1, - "milestones": {}, - "name": "test", - "namespace": None, - "parent": None, - "priorities": {}, - "tags": [], - "user": { - "default_email": "foo@bar.com", - "emails": [ - "foo@bar.com" - ], - "fullname": "foo bar", - "name": "foo" - } + "commit": [], + "owner": [ + "foo" + ], + "ticket": [] + }, + "close_status": [ + "Invalid", + "Insufficient data", + "Fixed", + "Duplicate" + ], + "custom_keys": [], + "date_created": "1496338274", + "date_modified": "1496338274", + "description": "test project #1", + "fullname": "test", + "url_path": "test", + "id": 1, + "milestones": {}, + "name": "test", + "namespace": None, + "parent": None, + "priorities": {}, + "tags": [], + "user": { + "default_email": "foo@bar.com", + "emails": [ + "foo@bar.com" + ], + "fullname": "foo bar", + "name": "foo" } - self.assertEqual(data, expected_output) + } + self.assertEqual(data, expected_output) def test_api_modify_project_main_admin_json(self): """ Test the api_modify_project method of the flask api when the @@ -1581,59 +1572,56 @@ class PagureFlaskApiProjecttests(tests.Modeltests): headers = {'Authorization': 'token aaabbbcccddd', 'Content-Type': 'application/json'} - user = pagure.lib.get_user(self.session, 'pingou') - user.cla_done = True - with tests.user_set(self.app.application, user): - output = self.app.patch( - '/api/0/test', headers=headers, - data=json.dumps({'main_admin': 'foo'})) - self.assertEqual(output.status_code, 200) - data = json.loads(output.get_data(as_text=True)) - data['date_created'] = '1496338274' - data['date_modified'] = '1496338274' - expected_output = { - "access_groups": { - "admin": [], - "commit": [], - "ticket": [] - }, - "access_users": { - "admin": [], - "commit": [], - "owner": [ - "foo" - ], - "ticket": [] - }, - "close_status": [ - "Invalid", - "Insufficient data", - "Fixed", - "Duplicate" + output = self.app.patch( + '/api/0/test', headers=headers, + data=json.dumps({'main_admin': 'foo'})) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + data['date_created'] = '1496338274' + data['date_modified'] = '1496338274' + expected_output = { + "access_groups": { + "admin": [], + "commit": [], + "ticket": [] + }, + "access_users": { + "admin": [], + "commit": [], + "owner": [ + "foo" ], - "custom_keys": [], - "date_created": "1496338274", - "date_modified": "1496338274", - "description": "test project #1", - "fullname": "test", - "url_path": "test", - "id": 1, - "milestones": {}, - "name": "test", - "namespace": None, - "parent": None, - "priorities": {}, - "tags": [], - "user": { - "default_email": "foo@bar.com", - "emails": [ - "foo@bar.com" - ], - "fullname": "foo bar", - "name": "foo" - } + "ticket": [] + }, + "close_status": [ + "Invalid", + "Insufficient data", + "Fixed", + "Duplicate" + ], + "custom_keys": [], + "date_created": "1496338274", + "date_modified": "1496338274", + "description": "test project #1", + "fullname": "test", + "url_path": "test", + "id": 1, + "milestones": {}, + "name": "test", + "namespace": None, + "parent": None, + "priorities": {}, + "tags": [], + "user": { + "default_email": "foo@bar.com", + "emails": [ + "foo@bar.com" + ], + "fullname": "foo bar", + "name": "foo" } - self.assertEqual(data, expected_output) + } + self.assertEqual(data, expected_output) @patch.dict('pagure.config.config', {'PAGURE_ADMIN_USERS': 'foo'}) def test_api_modify_project_main_admin_as_site_admin(self): @@ -1645,59 +1633,56 @@ class PagureFlaskApiProjecttests(tests.Modeltests): tests.create_tokens_acl(self.session, 'aaabbbcccddd', 'modify_project') headers = {'Authorization': 'token aaabbbcccddd'} - user = pagure.lib.get_user(self.session, 'foo') - user.cla_done = True - with tests.user_set(self.app.application, user): - output = self.app.patch( - '/api/0/test', headers=headers, - data={'main_admin': 'foo'}) - self.assertEqual(output.status_code, 200) - data = json.loads(output.get_data(as_text=True)) - data['date_created'] = '1496338274' - data['date_modified'] = '1496338274' - expected_output = { - "access_groups": { - "admin": [], - "commit": [], - "ticket": [] - }, - "access_users": { - "admin": [], - "commit": [], - "owner": [ - "foo" - ], - "ticket": [] - }, - "close_status": [ - "Invalid", - "Insufficient data", - "Fixed", - "Duplicate" + output = self.app.patch( + '/api/0/test', headers=headers, + data={'main_admin': 'foo'}) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + data['date_created'] = '1496338274' + data['date_modified'] = '1496338274' + expected_output = { + "access_groups": { + "admin": [], + "commit": [], + "ticket": [] + }, + "access_users": { + "admin": [], + "commit": [], + "owner": [ + "foo" ], - "custom_keys": [], - "date_created": "1496338274", - "date_modified": "1496338274", - "description": "test project #1", - "fullname": "test", - "url_path": "test", - "id": 1, - "milestones": {}, - "name": "test", - "namespace": None, - "parent": None, - "priorities": {}, - "tags": [], - "user": { - "default_email": "foo@bar.com", - "emails": [ - "foo@bar.com" - ], - "fullname": "foo bar", - "name": "foo" - } + "ticket": [] + }, + "close_status": [ + "Invalid", + "Insufficient data", + "Fixed", + "Duplicate" + ], + "custom_keys": [], + "date_created": "1496338274", + "date_modified": "1496338274", + "description": "test project #1", + "fullname": "test", + "url_path": "test", + "id": 1, + "milestones": {}, + "name": "test", + "namespace": None, + "parent": None, + "priorities": {}, + "tags": [], + "user": { + "default_email": "foo@bar.com", + "emails": [ + "foo@bar.com" + ], + "fullname": "foo bar", + "name": "foo" } - self.assertEqual(data, expected_output) + } + self.assertEqual(data, expected_output) def test_api_modify_project_main_admin_not_main_admin(self): """ Test the api_modify_project method of the flask api when the @@ -1716,20 +1701,17 @@ class PagureFlaskApiProjecttests(tests.Modeltests): tests.create_tokens_acl(self.session, 'aaabbbcccddd', 'modify_project') headers = {'Authorization': 'token aaabbbcccddd'} - user = pagure.lib.get_user(self.session, 'foo') - user.cla_done = True - with tests.user_set(self.app.application, user): - output = self.app.patch( - '/api/0/test', headers=headers, - data={'main_admin': 'foo'}) - self.assertEqual(output.status_code, 401) - expected_error = { - 'error': ('Only the main admin can set the main admin of a ' - 'project'), - 'error_code': 'ENOTMAINADMIN' - } - self.assertEqual( - json.loads(output.get_data(as_text=True)), expected_error) + output = self.app.patch( + '/api/0/test', headers=headers, + data={'main_admin': 'foo'}) + self.assertEqual(output.status_code, 401) + expected_error = { + 'error': ('Only the main admin can set the main admin of a ' + 'project'), + 'error_code': 'ENOTMAINADMIN' + } + self.assertEqual( + json.loads(output.get_data(as_text=True)), expected_error) def test_api_modify_project_not_admin(self): """ Test the api_modify_project method of the flask api when the @@ -1740,19 +1722,16 @@ class PagureFlaskApiProjecttests(tests.Modeltests): tests.create_tokens_acl(self.session, 'aaabbbcccddd', 'modify_project') headers = {'Authorization': 'token aaabbbcccddd'} - user = pagure.lib.get_user(self.session, 'foo') - user.cla_done = True - with tests.user_set(self.app.application, user): - output = self.app.patch( - '/api/0/test', headers=headers, - data={'main_admin': 'foo'}) - self.assertEqual(output.status_code, 401) - expected_error = { - 'error': 'You are not allowed to modify this project', - 'error_code': 'EMODIFYPROJECTNOTALLOWED' - } - self.assertEqual( - json.loads(output.get_data(as_text=True)), expected_error) + output = self.app.patch( + '/api/0/test', headers=headers, + data={'main_admin': 'foo'}) + self.assertEqual(output.status_code, 401) + expected_error = { + 'error': 'You are not allowed to modify this project', + 'error_code': 'EMODIFYPROJECTNOTALLOWED' + } + self.assertEqual( + json.loads(output.get_data(as_text=True)), expected_error) def test_api_modify_project_invalid_request(self): """ Test the api_modify_project method of the flask api when the @@ -1763,19 +1742,16 @@ class PagureFlaskApiProjecttests(tests.Modeltests): tests.create_tokens_acl(self.session, 'aaabbbcccddd', 'modify_project') headers = {'Authorization': 'token aaabbbcccddd'} - user = pagure.lib.get_user(self.session, 'pingou') - user.cla_done = True - with tests.user_set(self.app.application, user): - output = self.app.patch( - '/api/0/test', headers=headers, - data='invalid') - self.assertEqual(output.status_code, 400) - expected_error = { - 'error': 'Invalid or incomplete input submitted', - 'error_code': 'EINVALIDREQ' - } - self.assertEqual( - json.loads(output.get_data(as_text=True)), expected_error) + output = self.app.patch( + '/api/0/test', headers=headers, + data='invalid') + self.assertEqual(output.status_code, 400) + expected_error = { + 'error': 'Invalid or incomplete input submitted', + 'error_code': 'EINVALIDREQ' + } + self.assertEqual( + json.loads(output.get_data(as_text=True)), expected_error) def test_api_modify_project_invalid_keys(self): """ Test the api_modify_project method of the flask api when the @@ -1786,19 +1762,16 @@ class PagureFlaskApiProjecttests(tests.Modeltests): tests.create_tokens_acl(self.session, 'aaabbbcccddd', 'modify_project') headers = {'Authorization': 'token aaabbbcccddd'} - user = pagure.lib.get_user(self.session, 'pingou') - user.cla_done = True - with tests.user_set(self.app.application, user): - output = self.app.patch( - '/api/0/test', headers=headers, - data={'invalid': 'invalid'}) - self.assertEqual(output.status_code, 400) - expected_error = { - 'error': 'Invalid or incomplete input submitted', - 'error_code': 'EINVALIDREQ' - } - self.assertEqual( - json.loads(output.get_data(as_text=True)), expected_error) + output = self.app.patch( + '/api/0/test', headers=headers, + data={'invalid': 'invalid'}) + self.assertEqual(output.status_code, 400) + expected_error = { + 'error': 'Invalid or incomplete input submitted', + 'error_code': 'EINVALIDREQ' + } + self.assertEqual( + json.loads(output.get_data(as_text=True)), expected_error) def test_api_modify_project_invalid_new_main_admin(self): """ Test the api_modify_project method of the flask api when the @@ -1810,19 +1783,16 @@ class PagureFlaskApiProjecttests(tests.Modeltests): tests.create_tokens_acl(self.session, 'aaabbbcccddd', 'modify_project') headers = {'Authorization': 'token aaabbbcccddd'} - user = pagure.lib.get_user(self.session, 'pingou') - user.cla_done = True - with tests.user_set(self.app.application, user): - output = self.app.patch( - '/api/0/test', headers=headers, - data={'main_admin': 'tbrady'}) - self.assertEqual(output.status_code, 400) - expected_error = { - 'error': 'No such user found', - 'error_code': 'ENOUSER' - } - self.assertEqual( - json.loads(output.get_data(as_text=True)), expected_error) + output = self.app.patch( + '/api/0/test', headers=headers, + data={'main_admin': 'tbrady'}) + self.assertEqual(output.status_code, 400) + expected_error = { + 'error': 'No such user found', + 'error_code': 'ENOUSER' + } + self.assertEqual( + json.loads(output.get_data(as_text=True)), expected_error) def test_api_project_watchers(self): """ Test the api_project_watchers method of the flask api. """ @@ -2570,19 +2540,18 @@ class PagureFlaskApiProjecttests(tests.Modeltests): headers = {'Authorization': 'token aaabbbcccddd'} user = pagure.lib.get_user(self.session, 'pingou') - with tests.user_set(self.app.application, user): - output = self.app.post( - '/api/0/test/git/generateacls', headers=headers, - data={'wait': False}) - self.assertEqual(output.status_code, 200) - data = json.loads(output.get_data(as_text=True)) - expected_output = { - 'message': 'Project ACL generation queued', - 'taskid': 'abc-1234' - } - self.assertEqual(data, expected_output) - self.mock_gen_acls.assert_called_once_with( - name='test', namespace=None, user=None, group=None) + output = self.app.post( + '/api/0/test/git/generateacls', headers=headers, + data={'wait': False}) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + expected_output = { + 'message': 'Project ACL generation queued', + 'taskid': 'abc-1234' + } + self.assertEqual(data, expected_output) + self.mock_gen_acls.assert_called_once_with( + name='test', namespace=None, user=None, group=None) def test_api_generate_acls_json(self): """ Test the api_generate_acls method of the flask api using JSON """ @@ -2594,19 +2563,19 @@ class PagureFlaskApiProjecttests(tests.Modeltests): 'Content-Type': 'application/json'} user = pagure.lib.get_user(self.session, 'pingou') - with tests.user_set(self.app.application, user): - output = self.app.post( - '/api/0/test/git/generateacls', headers=headers, - data=json.dumps({'wait': False})) - self.assertEqual(output.status_code, 200) - data = json.loads(output.get_data(as_text=True)) - expected_output = { - 'message': 'Project ACL generation queued', - 'taskid': 'abc-1234' - } - self.assertEqual(data, expected_output) - self.mock_gen_acls.assert_called_once_with( - name='test', namespace=None, user=None, group=None) + + output = self.app.post( + '/api/0/test/git/generateacls', headers=headers, + data=json.dumps({'wait': False})) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + expected_output = { + 'message': 'Project ACL generation queued', + 'taskid': 'abc-1234' + } + self.assertEqual(data, expected_output) + self.mock_gen_acls.assert_called_once_with( + name='test', namespace=None, user=None, group=None) def test_api_generate_acls_wait_true(self): """ Test the api_generate_acls method of the flask api when wait is @@ -2622,19 +2591,18 @@ class PagureFlaskApiProjecttests(tests.Modeltests): self.mock_gen_acls.return_value = task_result user = pagure.lib.get_user(self.session, 'pingou') - with tests.user_set(self.app.application, user): - output = self.app.post( - '/api/0/test/git/generateacls', headers=headers, - data={'wait': True}) - self.assertEqual(output.status_code, 200) - data = json.loads(output.get_data(as_text=True)) - expected_output = { - 'message': 'Project ACLs generated', - } - self.assertEqual(data, expected_output) - self.mock_gen_acls.assert_called_once_with( - name='test', namespace=None, user=None, group=None) - self.assertTrue(task_result.get.called) + output = self.app.post( + '/api/0/test/git/generateacls', headers=headers, + data={'wait': True}) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + expected_output = { + 'message': 'Project ACLs generated', + } + self.assertEqual(data, expected_output) + self.mock_gen_acls.assert_called_once_with( + name='test', namespace=None, user=None, group=None) + self.assertTrue(task_result.get.called) def test_api_generate_acls_no_project(self): """ Test the api_generate_acls method of the flask api when the project @@ -2646,17 +2614,16 @@ class PagureFlaskApiProjecttests(tests.Modeltests): headers = {'Authorization': 'token aaabbbcccddd'} user = pagure.lib.get_user(self.session, 'pingou') - with tests.user_set(self.app.application, user): - output = self.app.post( - '/api/0/test12345123/git/generateacls', headers=headers, - data={'wait': False}) - self.assertEqual(output.status_code, 404) - data = json.loads(output.get_data(as_text=True)) - expected_output = { - 'error_code': 'ENOPROJECT', - 'error': 'Project not found' - } - self.assertEqual(data, expected_output) + output = self.app.post( + '/api/0/test12345123/git/generateacls', headers=headers, + data={'wait': False}) + self.assertEqual(output.status_code, 404) + data = json.loads(output.get_data(as_text=True)) + expected_output = { + 'error_code': 'ENOPROJECT', + 'error': 'Project not found' + } + self.assertEqual(data, expected_output) def test_api_new_git_branch(self): """ Test the api_new_branch method of the flask api """ @@ -2666,7 +2633,7 @@ class PagureFlaskApiProjecttests(tests.Modeltests): tests.add_content_git_repo(os.path.join(repo_path, 'test.git')) tests.create_tokens(self.session, project_id=None) tests.create_tokens_acl( - self.session, 'aaabbbcccddd', 'modify_project') + self.session, 'aaabbbcccddd', 'create_branch') headers = {'Authorization': 'token aaabbbcccddd'} args = {'branch': 'test123'} output = self.app.post('/api/0/test/git/branch', headers=headers, @@ -2690,7 +2657,7 @@ class PagureFlaskApiProjecttests(tests.Modeltests): tests.add_content_git_repo(os.path.join(repo_path, 'test.git')) tests.create_tokens(self.session, project_id=None) tests.create_tokens_acl( - self.session, 'aaabbbcccddd', 'modify_project') + self.session, 'aaabbbcccddd', 'create_branch') headers = {'Authorization': 'token aaabbbcccddd', 'Content-Type': 'application/json'} args = {'branch': 'test123'} @@ -2714,7 +2681,7 @@ class PagureFlaskApiProjecttests(tests.Modeltests): tests.add_content_git_repo(os.path.join(repo_path, 'test.git')) tests.create_tokens(self.session, project_id=None) tests.create_tokens_acl( - self.session, 'aaabbbcccddd', 'modify_project') + self.session, 'aaabbbcccddd', 'create_branch') git_path = os.path.join(self.path, 'repos', 'test.git') repo_obj = pygit2.Repository(git_path) parent = pagure.lib.git.get_branch_ref(repo_obj, 'master').get_object() @@ -2740,7 +2707,7 @@ class PagureFlaskApiProjecttests(tests.Modeltests): tests.add_content_git_repo(os.path.join(repo_path, 'test.git')) tests.create_tokens(self.session, project_id=None) tests.create_tokens_acl( - self.session, 'aaabbbcccddd', 'modify_project') + self.session, 'aaabbbcccddd', 'create_branch') headers = {'Authorization': 'token aaabbbcccddd'} args = {'branch': 'master'} output = self.app.post('/api/0/test/git/branch', headers=headers, @@ -2762,7 +2729,7 @@ class PagureFlaskApiProjecttests(tests.Modeltests): tests.add_content_git_repo(git_path) tests.create_tokens(self.session, project_id=None) tests.create_tokens_acl( - self.session, 'aaabbbcccddd', 'modify_project') + self.session, 'aaabbbcccddd', 'create_branch') repo_obj = pygit2.Repository(git_path) from_commit = repo_obj.revparse_single('HEAD').oid.hex headers = {'Authorization': 'token aaabbbcccddd'} diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 71723f2..e92729b 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -5595,6 +5595,7 @@ foo bar sorted([a.name for a in acls]), [ 'commit_flag', + 'create_branch', 'create_project', 'fork_project', 'generate_acls_project', From 3dad6ca27e7da90672b3c489236fff2436d0664e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: May 14 2018 16:45:32 +0000 Subject: [PATCH 5/5] Make the ACL list alphabetically ordered This will make it easier to update the acl_id in the tests since the tests order the ACL shortname alphabetically. Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/default_config.py b/pagure/default_config.py index e81ffde..2450bb0 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -265,27 +265,27 @@ BLACKLISTED_GROUPS = ['forks', 'group'] ACLS = { + 'create_branch': 'Create a git branch on a project', 'create_project': 'Create a new project', + 'commit_flag': 'Flag a commit', 'fork_project': 'Fork a project', + 'generate_acls_project': 'Generate the Gitolite ACLs on a project', 'issue_assign': 'Assign issue to someone', - 'issue_create': 'Create a new ticket', 'issue_change_status': 'Change the status of a ticket', 'issue_comment': 'Comment on a ticket', + 'issue_create': 'Create a new ticket', + 'issue_subscribe': 'Subscribe the user with this token to an issue', + 'issue_update': 'Update an issue, status, comments, custom fields...', + 'issue_update_custom_fields': 'Update the custom fields of an issue', + 'issue_update_milestone': 'Update the milestone of an issue', + 'modify_project': 'Modify an existing project', + 'pull_request_create': 'Open a new pull-request', 'pull_request_close': 'Close a pull-request', 'pull_request_comment': 'Comment on a pull-request', - 'pull_request_create': 'Open a new pull-request', 'pull_request_flag': 'Flag a pull-request', 'pull_request_merge': 'Merge a pull-request', 'pull_request_subscribe': 'Subscribe the user with this token to a pull-request', - 'issue_subscribe': 'Subscribe the user with this token to an issue', - 'issue_update': 'Update an issue, status, comments, custom fields...', - 'issue_update_custom_fields': 'Update the custom fields of an issue', - 'issue_update_milestone': 'Update the milestone of an issue', - 'modify_project': 'Modify an existing project', - 'generate_acls_project': 'Generate the Gitolite ACLs on a project', - 'commit_flag': 'Flag a commit', - 'create_branch': 'Create a git branch on a project', } # List of ACLs which a regular user is allowed to associate to an API token