From 91fe5e5acb2cb5497e41aa96640f1af91afd42d7 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 06 2017 19:18:42 +0000 Subject: [PATCH 1/12] Add a list of ACL for API token that are allowed to not be linked to a project --- diff --git a/pagure/default_config.py b/pagure/default_config.py index 87ce58d..9ac9d43 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -221,6 +221,17 @@ ACLS = { 'issue_update_custom_fields': 'Update the custom fields of an issue', } +# From the ACLs above lists which ones are tolerated to be associated with +# an API token that isn't linked to a particular project. +CROSS_PROJECT_ACLS = [ + 'create_project', + 'fork_project', + 'issue_comment', + 'issue_create', + 'pull_request_flag', + 'pull_request_comment', +] + # Bootstrap URLS BOOTSTRAP_URLS_CSS = 'https://apps.fedoraproject.org/global/' \ 'fedora-bootstrap-1.0.1/fedora-bootstrap.css' From 1e5d3af95da3b0b67b5adf5db6e792593fb829df Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 06 2017 19:18:42 +0000 Subject: [PATCH 2/12] Make the project field in the tokens table nullable --- diff --git a/alembic/versions/770149d96e24_nullable_project_for_api_token.py b/alembic/versions/770149d96e24_nullable_project_for_api_token.py new file mode 100644 index 0000000..6e1f4f2 --- /dev/null +++ b/alembic/versions/770149d96e24_nullable_project_for_api_token.py @@ -0,0 +1,35 @@ +"""nullable project for api token + +Revision ID: 770149d96e24 +Revises: 987edda096f5 +Create Date: 2017-03-04 18:05:07.956057 + +""" + +# revision identifiers, used by Alembic. +revision = '770149d96e24' +down_revision = '987edda096f5' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + """ Make the field 'project_id' of the table tokens be nullable. """ + op.alter_column( + 'tokens', + 'project_id', + nullable=True, + existing_nullable=False, + ) + + +def downgrade(): + """ Make the field 'project_id' of the table tokens be not nullable. + """ + op.alter_column( + 'tokens', + 'project_id', + nullable=False, + existing_nullable=True, + ) diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 2167167..0414aa7 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -2169,7 +2169,7 @@ class Token(BASE): sa.ForeignKey( 'projects.id', onupdate='CASCADE', ), - nullable=False, + nullable=True, index=True) expiration = sa.Column( sa.DateTime, nullable=False, default=datetime.datetime.utcnow) From e3dee76bbe0dcaad1042b4ca4b7861f5a6e1f82a Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 06 2017 19:18:42 +0000 Subject: [PATCH 3/12] Do not update the expiration date of an expired API token --- diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index a6e0ee5..f2bb386 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -2133,7 +2133,9 @@ def revoke_api_token(repo, token_id, username=None, namespace=None): if form.validate_on_submit(): try: - token.expiration = datetime.datetime.utcnow() + if token.expiration >= datetime.datetime.utcnow(): + token.expiration = datetime.datetime.utcnow() + SESSION.add(token) SESSION.commit() flask.flash('Token revoked') except SQLAlchemyError as err: # pragma: no cover From 170fc9649f3350dd3d83310150bc61aef8c7e691 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 06 2017 19:18:42 +0000 Subject: [PATCH 4/12] Allow creating API tokens not linked to a particular project This is most useful API tokens with ACLs such as 'create_project' which up until now required you had a project to get an API token with the ACL to create a project. The list of ACLs allowed for project-less API token is defined in the configuration file. --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 49407c6..448d570 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -3231,7 +3231,7 @@ def get_api_token(session, token_str): return query.first() -def get_acls(session): +def get_acls(session, restrict=None): """ Returns all the possible ACLs a token can have according to the database. """ @@ -3240,6 +3240,15 @@ def get_acls(session): ).order_by( model.ACL.name ) + if restrict: + if isinstance(restrict, list): + query = query.filter( + model.ACL.name.in_(restrict) + ) + else: + query = query.filter( + model.ACL.name == restrict + ) return query.all() @@ -3259,7 +3268,7 @@ def add_token_to_user(session, project, acls, username): token = pagure.lib.model.Token( id=pagure.lib.login.id_generator(64), user_id=user.id, - project_id=project.id, + project_id=project.id if project else None, expiration=datetime.datetime.utcnow() + datetime.timedelta(days=60) ) session.add(token) diff --git a/pagure/templates/add_token.html b/pagure/templates/add_token.html index 11e9b31..b6aed5c 100644 --- a/pagure/templates/add_token.html +++ b/pagure/templates/add_token.html @@ -1,11 +1,15 @@ +{% if repo %} {% extends "repo_master.html" %} +{% else %} +{% extends "master.html" %} +{% endif %} {% from "_formhelper.html" import render_field_in_row %} {% set tag = "home" %} {% block title %}Create token{% endblock %} - -{% block repo %} +{% macro render_page() %} +
@@ -21,10 +25,14 @@ After that, click 'Create' to generate a token with the selected permissions.

+ {% if repo %}
+ {% else %} + + {% endif %} {% for acl in acls %}
+
+{% endmacro %} + + +{% block content %} + {{ render_page() }} +{% endblock %} + +{% block repo %} + {{ render_page() }} {% endblock %} diff --git a/pagure/templates/settings.html b/pagure/templates/settings.html index 2ff690f..2a90324 100644 --- a/pagure/templates/settings.html +++ b/pagure/templates/settings.html @@ -205,7 +205,6 @@
- diff --git a/pagure/templates/user_settings.html b/pagure/templates/user_settings.html index fe22f90..8cdd62c 100644 --- a/pagure/templates/user_settings.html +++ b/pagure/templates/user_settings.html @@ -148,12 +148,104 @@ -
- {% if config.get('PAGURE_AUTH')=='local' %} - Change password + {% if config.get('PAGURE_AUTH')=='local' %} +
+ Change password +
+ {% endif %} +
+ +
+
+
+ API Keys +
+
+

+ API keys are tokens used to authenticate you on pagure. They can also + be used to grant access to 3rd party application to behave on all + projects in your name. +

+

+ These are your personal tokens; they are not visible to others. +

+

+ These keys are valid for 60 days. +

+

+ These keys are private, make sure to store in a safe place and + do not share it. +

+
+ {% if user.tokens %} +
    + {% for token in user.tokens %} + {% if not token.project %} +
  • +
    +
    +
    + +
    +
    + {% if token.expired %} + Expired since {{ token.expiration.date() }} + {% else %} + Valid until: {{ token.expiration.date() }} +
    + + {{ form.csrf_token }} +
    + {% endif %} + + +
    • {% endif %} + {% endfor %} +
+ {% endif %} +
+ + + +
+ {% endblock %} diff --git a/pagure/ui/app.py b/pagure/ui/app.py index 693ab36..82759de 100644 --- a/pagure/ui/app.py +++ b/pagure/ui/app.py @@ -1,16 +1,17 @@ # -*- coding: utf-8 -*- """ - (c) 2014-2015 - Copyright Red Hat Inc + (c) 2014-2017 - Copyright Red Hat Inc Authors: Pierre-Yves Chibon """ -import flask +import datetime from math import ceil +import flask from sqlalchemy.exc import SQLAlchemyError import pagure.exceptions @@ -756,3 +757,92 @@ def ssh_hostkey(): return flask.render_template( 'doc_ssh_keys.html', ) + + +@APP.route('/settings/token/new/', methods=('GET', 'POST')) +@APP.route('/settings/token/new', methods=('GET', 'POST')) +@login_required +def add_user_token(): + """ Create an user token (not project specific). + """ + if admin_session_timedout(): + if flask.request.method == 'POST': + flask.flash('Action canceled, try it again', 'error') + return flask.redirect( + flask.url_for('auth_login', next=flask.request.url)) + + # Ensure the user is in the DB at least + user = pagure.lib.search_user( + SESSION, username=flask.g.fas_user.username) + if not user: + flask.abort(404, 'User not found') + + acls = pagure.lib.get_acls( + SESSION, restrict=APP.config.get('CROSS_PROJECT_ACLS')) + form = pagure.forms.NewTokenForm(acls=acls) + + if form.validate_on_submit(): + try: + msg = pagure.lib.add_token_to_user( + SESSION, + project=None, + acls=form.acls.data, + username=flask.g.fas_user.username, + ) + SESSION.commit() + flask.flash(msg) + return flask.redirect(flask.url_for('.user_settings')) + except SQLAlchemyError as err: # pragma: no cover + SESSION.rollback() + APP.logger.exception(err) + flask.flash('API key could not be added', 'error') + + # When form is displayed after an empty submission, show an error. + if form.errors.get('acls'): + flask.flash('You must select at least one permission.', 'error') + + return flask.render_template( + 'add_token.html', + select='settings', + form=form, + acls=acls, + ) + + +@APP.route('/settings/token/revoke//', methods=['POST']) +@APP.route('/settings/token/revoke/', methods=['POST']) +@login_required +def revoke_api_user_token(token_id): + """ Revokie an user token (ie: not project specific). + """ + if admin_session_timedout(): + flask.flash('Action canceled, try it again', 'error') + url = flask.url_for( + 'view_settings', username=username, repo=repo, + namespace=namespace) + return flask.redirect( + flask.url_for('auth_login', next=url)) + + token = pagure.lib.get_api_token(SESSION, token_id) + + if not token \ + or token.user.username != flask.g.fas_user.username: + flask.abort(404, 'Token not found') + + form = pagure.forms.ConfirmationForm() + + if form.validate_on_submit(): + try: + if token.expiration >= datetime.datetime.utcnow(): + token.expiration = datetime.datetime.utcnow() + SESSION.add(token) + SESSION.commit() + flask.flash('Token revoked') + except SQLAlchemyError as err: # pragma: no cover + SESSION.rollback() + APP.logger.exception(err) + flask.flash( + 'Token could not be revoked, please contact an admin', + 'error') + + return flask.redirect(flask.url_for('.user_settings')) diff --git a/tests/test_pagure_flask_ui_app.py b/tests/test_pagure_flask_ui_app.py index 3f2ecf2..a9acd72 100644 --- a/tests/test_pagure_flask_ui_app.py +++ b/tests/test_pagure_flask_ui_app.py @@ -11,6 +11,7 @@ __requires__ = ['SQLAlchemy >= 0.8'] import pkg_resources +import datetime import unittest import shutil import sys @@ -1207,6 +1208,174 @@ class PagureFlaskApptests(tests.Modeltests): self.assertEqual(output.status_code, 404) pagure.APP.config['ENABLE_TICKETS'] = True + @patch('pagure.ui.app.admin_session_timedout') + def test_add_user_token(self, ast): + """ Test the add_user_token endpoint. """ + ast.return_value = False + self.test_new_project() + + user = tests.FakeUser() + with tests.user_set(pagure.APP, user): + output = self.app.get('/settings/token/new/') + self.assertEqual(output.status_code, 404) + self.assertTrue('

Page not found (404)

' in output.data) + + user.username = 'foo' + with tests.user_set(pagure.APP, user): + output = self.app.get('/settings/token/new') + self.assertEqual(output.status_code, 200) + self.assertIn( + '
\n ' + 'Create a new token\n', output.data) + self.assertIn( + '', + output.data) + + csrf_token = output.data.split( + 'name="csrf_token" type="hidden" value="')[1].split('">')[0] + + data = { + 'acls': ['create_project', 'fork_project'] + } + + # missing CSRF + output = self.app.post('/settings/token/new', data=data) + self.assertEqual(output.status_code, 200) + self.assertIn( + 'Create token - Pagure', output.data) + self.assertIn( + '
\n ' + 'Create a new token\n', output.data) + self.assertIn( + '', + output.data) + + data = { + 'acls': ['new_project'], + 'csrf_token': csrf_token + } + + # Invalid ACLs + output = self.app.post('/settings/token/new', data=data) + self.assertEqual(output.status_code, 200) + self.assertIn( + 'Create token - Pagure', output.data) + self.assertIn( + '
\n ' + 'Create a new token\n', output.data) + self.assertIn( + '', + output.data) + + data = { + 'acls': ['create_project', 'fork_project'], + 'csrf_token': csrf_token + } + + # All good + output = self.app.post( + '/settings/token/new', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + 'foo\'s settings - Pagure', output.data) + self.assertIn( + '\n Token created\n', + output.data) + self.assertEqual( + output.data.count( + 'Valid' + ' until: '), 1) + + ast.return_value = True + output = self.app.get('/settings/token/new') + self.assertEqual(output.status_code, 302) + + @patch('pagure.ui.app.admin_session_timedout') + def test_revoke_api_user_token(self, ast): + """ Test the revoke_api_user_token endpoint. """ + ast.return_value = False + self.test_new_project() + + user = tests.FakeUser() + with tests.user_set(pagure.APP, user): + # Token doesn't exist + output = self.app.post('/settings/token/revoke/foobar') + self.assertEqual(output.status_code, 404) + self.assertTrue('

Page not found (404)

' in output.data) + + # Create the foobar API token but associated w/ the user 'foo' + item = pagure.lib.model.Token( + id='foobar', + user_id=2, # foo + expiration=datetime.datetime.utcnow() \ + + datetime.timedelta(days=30) + ) + self.session.add(item) + self.session.commit() + + # Token not associated w/ this user + output = self.app.post('/settings/token/revoke/foobar') + self.assertEqual(output.status_code, 404) + self.assertTrue('

Page not found (404)

' in output.data) + + user.username = 'foo' + with tests.user_set(pagure.APP, user): + # Missing CSRF token + output = self.app.post( + '/settings/token/revoke/foobar', follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + "foo's settings - Pagure", output.data) + self.assertEqual( + output.data.count( + 'Valid' + ' until: '), 1) + + csrf_token = output.data.split( + 'name="csrf_token" type="hidden" value="')[1].split('">')[0] + + data = { + 'csrf_token': csrf_token + } + + # All good - token is deleted + output = self.app.post( + '/settings/token/revoke/foobar', data=data, + follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + "foo's settings - Pagure", output.data) + self.assertEqual( + output.data.count( + 'Valid' + ' until: '), 0) + + user = pagure.lib.get_user(self.session, key='foo') + self.assertEqual(len(user.tokens), 1) + expiration_dt = user.tokens[0].expiration + + # Token was already deleted - no changes + output = self.app.post( + '/settings/token/revoke/foobar', data=data, + follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + "foo's settings - Pagure", output.data) + self.assertEqual( + output.data.count( + 'Valid' + ' until: '), 0) + + # Ensure the expiration date did not change + user = pagure.lib.get_user(self.session, key='foo') + self.assertEqual(len(user.tokens), 1) + self.assertEqual( + expiration_dt, user.tokens[0].expiration + ) + + ast.return_value = True + output = self.app.get('/settings/token/new') + self.assertEqual(output.status_code, 302) if __name__ == '__main__': - unittest.main() + unittest.main(verbosity=2) From a1463615b07fca4a6619c8c208af199c13020ab7 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 06 2017 19:18:42 +0000 Subject: [PATCH 5/12] Let tests.create_tokens create as well project-less tokens --- diff --git a/tests/__init__.py b/tests/__init__.py index 3e5dafd..418a01a 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -260,12 +260,12 @@ def create_projects_git(folder, bare=False): return repos -def create_tokens(session, user_id=1): +def create_tokens(session, user_id=1, project_id=1): """ Create some tokens for the project in the database. """ item = pagure.lib.model.Token( id='aaabbbcccddd', user_id=user_id, - project_id=1, + project_id=project_id, expiration=datetime.utcnow() + timedelta(days=30) ) session.add(item) @@ -273,7 +273,7 @@ def create_tokens(session, user_id=1): item = pagure.lib.model.Token( id='foo_token', user_id=user_id, - project_id=1, + project_id=project_id, expiration=datetime.utcnow() + timedelta(days=30) ) session.add(item) @@ -281,7 +281,7 @@ def create_tokens(session, user_id=1): item = pagure.lib.model.Token( id='expired_token', user_id=user_id, - project_id=1, + project_id=project_id, expiration=datetime.utcnow() - timedelta(days=1) ) session.add(item) From 1dfd9499711e7b7b083fa92ee29444a72c0ae60e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 06 2017 19:18:42 +0000 Subject: [PATCH 6/12] Support user tokens for some of the API endpoints These API endpoints are to: - Create a comment on a ticket or a PR - Flag a pull-request - Create a project - Fork a project The configuration file can then be used to turn on or off which of these actions are supported by this pagure instance. --- diff --git a/pagure/api/fork.py b/pagure/api/fork.py index 043833c..314fa27 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -509,7 +509,7 @@ def api_pull_request_add_comment( raise pagure.exceptions.APIError( 404, error_code=APIERROR.EPULLREQUESTSDISABLED) - if repo.fullname != flask.g.token.project.fullname: + 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( @@ -646,14 +646,16 @@ def api_pull_request_add_flag(repo, requestid, username=None, namespace=None): output = {} if repo is None: - raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) + raise pagure.exceptions.APIError( + 404, error_code=APIERROR.ENOPROJECT) if not repo.settings.get('pull_requests', True): raise pagure.exceptions.APIError( 404, error_code=APIERROR.EPULLREQUESTSDISABLED) - if repo.fullname != flask.g.token.project.fullname: - 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( SESSION, project_id=repo.id, requestid=requestid) diff --git a/pagure/api/issue.py b/pagure/api/issue.py index 63ee624..601869e 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -100,14 +100,16 @@ def api_new_issue(repo, username=None, namespace=None): output = {} if repo is None: - raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT) + raise pagure.exceptions.APIError( + 404, error_code=APIERROR.ENOPROJECT) if not repo.settings.get('issue_tracker', True): raise pagure.exceptions.APIError( 404, error_code=APIERROR.ETRACKERDISABLED) - if 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) user_obj = pagure.lib.get_user( SESSION, flask.g.fas_user.username) @@ -723,7 +725,7 @@ def api_comment_issue(repo, issueid, username=None, namespace=None): 404, error_code=APIERROR.ETRACKERDISABLED) if api_authenticated(): - if repo != flask.g.token.project: + if flask.g.token.project and repo != flask.g.token.project: raise pagure.exceptions.APIError( 401, error_code=APIERROR.EINVALIDTOK) diff --git a/tests/test_pagure_flask_api_fork.py b/tests/test_pagure_flask_api_fork.py index be8a3d7..c4c7a8c 100644 --- a/tests/test_pagure_flask_api_fork.py +++ b/tests/test_pagure_flask_api_fork.py @@ -670,6 +670,120 @@ class PagureFlaskApiForktests(tests.Modeltests): self.assertEqual(len(request.comments), 1) @patch('pagure.lib.notify.send_email') + def test_api_pull_request_add_comment_user_token(self, mockemail): + """ Test the api_pull_request_add_comment method of the flask api. """ + mockemail.return_value = True + + tests.create_projects(self.session) + tests.create_tokens(self.session, project_id=None) + tests.create_tokens_acl(self.session) + + headers = {'Authorization': 'token aaabbbcccddd'} + + # Invalid project + output = self.app.post( + '/api/0/foo/pull-request/1/comment', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Project not found", + "error_code": "ENOPROJECT", + } + ) + + # Valid token, invalid request + output = self.app.post( + '/api/0/test2/pull-request/1/comment', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Pull-Request not found", + "error_code": "ENOREQ", + } + ) + + # Valid token, invalid request in another project + output = self.app.post( + '/api/0/test/pull-request/1/comment', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Pull-Request not found", + "error_code": "ENOREQ", + } + ) + + # Create a pull-request + repo = pagure.lib.get_project(self.session, 'test') + forked_repo = pagure.lib.get_project(self.session, 'test') + req = pagure.lib.new_pull_request( + session=self.session, + repo_from=forked_repo, + branch_from='master', + repo_to=repo, + branch_to='master', + title='test pull-request', + user='pingou', + requestfolder=None, + ) + self.session.commit() + self.assertEqual(req.id, 1) + self.assertEqual(req.title, 'test pull-request') + + # Check comments before + request = pagure.lib.search_pull_requests( + self.session, project_id=1, requestid=1) + self.assertEqual(len(request.comments), 0) + + data = { + 'title': 'test issue', + } + + # Incomplete request + output = self.app.post( + '/api/0/test/pull-request/1/comment', data=data, headers=headers) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Invalid or incomplete input submited", + "error_code": "EINVALIDREQ", + "errors": {"comment": ["This field is required."]} + } + ) + + # No change + request = pagure.lib.search_pull_requests( + self.session, project_id=1, requestid=1) + self.assertEqual(len(request.comments), 0) + + data = { + 'comment': 'This is a very interesting question', + } + + # Valid request + output = self.app.post( + '/api/0/test/pull-request/1/comment', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertDictEqual( + data, + {'message': 'Comment added'} + ) + + # One comment added + request = pagure.lib.search_pull_requests( + self.session, project_id=1, requestid=1) + self.assertEqual(len(request.comments), 1) + + @patch('pagure.lib.notify.send_email') def test_api_pull_request_add_flag(self, mockemail): """ Test the api_pull_request_add_flag method of the flask api. """ mockemail.return_value = True @@ -813,6 +927,154 @@ class PagureFlaskApiForktests(tests.Modeltests): self.assertEqual(request.flags[0].comment, 'Tests passed') self.assertEqual(request.flags[0].percent, 100) + @patch('pagure.lib.notify.send_email') + def test_api_pull_request_add_flag_user_token(self, mockemail): + """ Test the api_pull_request_add_flag method of the flask api. """ + mockemail.return_value = True + + tests.create_projects(self.session) + tests.create_tokens(self.session, project_id=None) + tests.create_tokens_acl(self.session) + + headers = {'Authorization': 'token aaabbbcccddd'} + + # Invalid project + output = self.app.post( + '/api/0/foo/pull-request/1/flag', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Project not found", + "error_code": "ENOPROJECT", + } + ) + + # Valid token, wrong project + output = self.app.post( + '/api/0/test2/pull-request/1/flag', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Pull-Request not found", + "error_code": "ENOREQ", + } + ) + + # No input + output = self.app.post( + '/api/0/test/pull-request/1/flag', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Pull-Request not found", + "error_code": "ENOREQ", + } + ) + + # Create a pull-request + repo = pagure.lib.get_project(self.session, 'test') + forked_repo = pagure.lib.get_project(self.session, 'test') + req = pagure.lib.new_pull_request( + session=self.session, + repo_from=forked_repo, + branch_from='master', + repo_to=repo, + branch_to='master', + title='test pull-request', + user='pingou', + requestfolder=None, + ) + self.session.commit() + self.assertEqual(req.id, 1) + self.assertEqual(req.title, 'test pull-request') + + # Check comments before + request = pagure.lib.search_pull_requests( + self.session, project_id=1, requestid=1) + self.assertEqual(len(request.flags), 0) + + data = { + 'username': 'Jenkins', + 'percent': 100, + 'url': 'http://jenkins.cloud.fedoraproject.org/', + 'uid': 'jenkins_build_pagure_100+seed', + } + + # Incomplete request + output = self.app.post( + '/api/0/test/pull-request/1/flag', data=data, headers=headers) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Invalid or incomplete input submited", + "error_code": "EINVALIDREQ", + "errors": {"comment": ["This field is required."]} + } + ) + + # No change + request = pagure.lib.search_pull_requests( + self.session, project_id=1, requestid=1) + self.assertEqual(len(request.flags), 0) + + data = { + 'username': 'Jenkins', + 'percent': 0, + 'comment': 'Tests failed', + 'url': 'http://jenkins.cloud.fedoraproject.org/', + 'uid': 'jenkins_build_pagure_100+seed', + } + + # Valid request + output = self.app.post( + '/api/0/test/pull-request/1/flag', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertDictEqual( + data, + {'message': 'Flag added'} + ) + + # One flag added + request = pagure.lib.search_pull_requests( + self.session, project_id=1, requestid=1) + self.assertEqual(len(request.flags), 1) + self.assertEqual(request.flags[0].comment, 'Tests failed') + self.assertEqual(request.flags[0].percent, 0) + + # Update flag + data = { + 'username': 'Jenkins', + 'percent': 100, + 'comment': 'Tests passed', + 'url': 'http://jenkins.cloud.fedoraproject.org/', + 'uid': 'jenkins_build_pagure_100+seed', + } + + output = self.app.post( + '/api/0/test/pull-request/1/flag', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertDictEqual( + data, + {'message': 'Flag updated'} + ) + + # One flag added + request = pagure.lib.search_pull_requests( + self.session, project_id=1, requestid=1) + self.assertEqual(len(request.flags), 1) + self.assertEqual(request.flags[0].comment, 'Tests passed') + self.assertEqual(request.flags[0].percent, 100) + if __name__ == '__main__': SUITE = unittest.TestLoader().loadTestsFromTestCase( diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index 9ac5544..18a6e66 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -486,6 +486,247 @@ class PagureFlaskApiIssuetests(tests.Modeltests): } ) + def test_api_new_issue_user_token(self): + """ Test the api_new_issue method of the flask api. """ + tests.create_projects(self.session) + tests.create_projects_git( + os.path.join(self.path, 'tickets'), bare=True) + tests.create_tokens(self.session, project_id=None) + tests.create_tokens_acl(self.session) + + headers = {'Authorization': 'token aaabbbcccddd'} + + # Valid token, invalid request - No input + output = self.app.post('/api/0/test2/new_issue', headers=headers) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Invalid or incomplete input submited", + "error_code": "EINVALIDREQ", + "errors": { + "issue_content": ["This field is required."], + "title": ["This field is required."], + } + } + ) + + # Another project, still an invalid request - No input + output = self.app.post('/api/0/test/new_issue', headers=headers) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Invalid or incomplete input submited", + "error_code": "EINVALIDREQ", + "errors": { + "issue_content": ["This field is required."], + "title": ["This field is required."], + } + } + ) + + data = { + 'title': 'test issue' + } + + # Invalid repo + output = self.app.post( + '/api/0/foo/new_issue', data=data, headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Project not found", + "error_code": "ENOPROJECT", + } + ) + + # Incomplete request + output = self.app.post( + '/api/0/test/new_issue', data=data, headers=headers) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Invalid or incomplete input submited", + "error_code": "EINVALIDREQ", + "errors": { + "issue_content": ["This field is required."], + "title": ["This field is required."] + } + } + ) + + data = { + 'title': 'test issue', + 'issue_content': 'This issue needs attention', + } + + # Valid request + output = self.app.post( + '/api/0/test/new_issue', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['issue']['date_created'] = '1431414800' + data['issue']['last_updated'] = '1431414800' + self.assertDictEqual( + data, + { + "issue": FULL_ISSUE_LIST[8], + "message": "Issue created" + } + ) + + # Valid request with milestone + data = { + 'title': 'test issue', + 'issue_content': 'This issue needs attention', + 'milestone': ['milestone-1.0'], + } + output = self.app.post( + '/api/0/test/new_issue', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['issue']['date_created'] = '1431414800' + data['issue']['last_updated'] = '1431414800' + + self.assertDictEqual( + data, + { + "issue": FULL_ISSUE_LIST[7], + "message": "Issue created" + } + ) + + # Valid request, with private='false' + data = { + 'title': 'test issue', + 'issue_content': 'This issue needs attention', + 'private': 'false', + } + + output = self.app.post( + '/api/0/test/new_issue', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['issue']['date_created'] = '1431414800' + data['issue']['last_updated'] = '1431414800' + self.assertDictEqual( + data, + { + "issue": FULL_ISSUE_LIST[6], + "message": "Issue created" + } + ) + + # Valid request, with private=False + data = { + 'title': 'test issue', + 'issue_content': 'This issue needs attention', + 'private': False + } + + output = self.app.post( + '/api/0/test/new_issue', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['issue']['date_created'] = '1431414800' + data['issue']['last_updated'] = '1431414800' + self.assertDictEqual( + data, + { + "issue": FULL_ISSUE_LIST[5], + "message": "Issue created" + } + ) + + # Valid request, with private='False' + data = { + 'title': 'test issue', + 'issue_content': 'This issue needs attention', + 'private': 'False' + } + + output = self.app.post( + '/api/0/test/new_issue', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['issue']['date_created'] = '1431414800' + data['issue']['last_updated'] = '1431414800' + self.assertDictEqual( + data, + { + "issue": FULL_ISSUE_LIST[4], + "message": "Issue created" + } + ) + + # Valid request, with private=0 + data = { + 'title': 'test issue', + 'issue_content': 'This issue needs attention', + 'private': 0 + } + + output = self.app.post( + '/api/0/test/new_issue', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['issue']['date_created'] = '1431414800' + data['issue']['last_updated'] = '1431414800' + self.assertDictEqual( + data, + { + "issue": FULL_ISSUE_LIST[3], + "message": "Issue created" + } + ) + + # Private issue: True + data = { + 'title': 'test issue', + 'issue_content': 'This issue needs attention', + 'private': True, + } + output = self.app.post( + '/api/0/test/new_issue', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['issue']['date_created'] = '1431414800' + data['issue']['last_updated'] = '1431414800' + self.assertDictEqual( + data, + { + "issue": FULL_ISSUE_LIST[2], + "message": "Issue created" + } + ) + + # Private issue: 1 + data = { + 'title': 'test issue1', + 'issue_content': 'This issue needs attention', + 'private': 1, + } + output = self.app.post( + '/api/0/test/new_issue', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['issue']['date_created'] = '1431414800' + data['issue']['last_updated'] = '1431414800' + self.assertDictEqual( + data, + { + "issue": FULL_ISSUE_LIST[1], + "message": "Issue created" + } + ) + def test_api_view_issues(self): """ Test the api_view_issues method of the flask api. """ self.test_api_new_issue() @@ -1315,6 +1556,212 @@ class PagureFlaskApiIssuetests(tests.Modeltests): @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') + def test_api_comment_issue_user_token(self, p_send_email, p_ugt): + """ Test the api_comment_issue method of the flask api. """ + p_send_email.return_value = True + p_ugt.return_value = True + + tests.create_projects(self.session) + tests.create_tokens(self.session, project_id=None) + tests.create_tokens_acl(self.session) + + headers = {'Authorization': 'token aaabbbcccddd'} + + # Invalid project + output = self.app.post('/api/0/foo/issue/1/comment', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Project not found", + "error_code": "ENOPROJECT", + } + ) + + # Valid token, no issue on the project + output = self.app.post('/api/0/test2/issue/1/comment', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Issue not found", + "error_code": "ENOISSUE", + } + ) + + # Valid token, still no issue on this other project + output = self.app.post('/api/0/test/issue/1/comment', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Issue not found", + "error_code": "ENOISSUE", + } + ) + + # Create normal issue + repo = pagure.lib.get_project(self.session, 'test') + msg = pagure.lib.new_issue( + session=self.session, + repo=repo, + title='Test issue #1', + content='We should work on this', + user='pingou', + ticketfolder=None, + private=False, + issue_uid='aaabbbccc#1', + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue #1') + + # Check comments before + repo = pagure.lib.get_project(self.session, 'test') + issue = pagure.lib.search_issues(self.session, repo, issueid=1) + self.assertEqual(len(issue.comments), 0) + + data = { + 'title': 'test issue', + } + + # Incomplete request + output = self.app.post( + '/api/0/test/issue/1/comment', data=data, headers=headers) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Invalid or incomplete input submited", + "error_code": "EINVALIDREQ", + "errors": {"comment": ["This field is required."]} + } + ) + + # No change + repo = pagure.lib.get_project(self.session, 'test') + issue = pagure.lib.search_issues(self.session, repo, issueid=1) + self.assertEqual(issue.status, 'Open') + + data = { + 'comment': 'This is a very interesting question', + } + + # Valid request + output = self.app.post( + '/api/0/test/issue/1/comment', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertDictEqual( + data, + {'message': 'Comment added'} + ) + + # One comment added + repo = pagure.lib.get_project(self.session, 'test') + issue = pagure.lib.search_issues(self.session, repo, issueid=1) + self.assertEqual(len(issue.comments), 1) + + # Create another project + item = pagure.lib.model.Project( + user_id=2, # foo + name='foo', + description='test project #3', + hook_token='aaabbbdddeee', + ) + self.session.add(item) + self.session.commit() + + # Create a token for pingou for this project + item = pagure.lib.model.Token( + id='pingou_foo', + user_id=1, + project_id=4, + expiration=datetime.datetime.utcnow() + datetime.timedelta( + days=30) + ) + self.session.add(item) + self.session.commit() + + # Give `issue_change_status` to this token when `issue_comment` + # is required + item = pagure.lib.model.TokenAcl( + token_id='pingou_foo', + acl_id=2, + ) + self.session.add(item) + self.session.commit() + + repo = pagure.lib.get_project(self.session, 'foo') + # Create private issue + msg = pagure.lib.new_issue( + session=self.session, + repo=repo, + title='Test issue', + content='We should work on this', + user='foo', + ticketfolder=None, + private=True, + issue_uid='aaabbbccc#2', + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue') + + # Check before + repo = pagure.lib.get_project(self.session, 'foo') + issue = pagure.lib.search_issues(self.session, repo, issueid=1) + self.assertEqual(len(issue.comments), 0) + + data = { + 'comment': 'This is a very interesting question', + } + headers = {'Authorization': 'token pingou_foo'} + + # Valid request but un-authorized + output = self.app.post( + '/api/0/foo/issue/1/comment', data=data, headers=headers) + self.assertEqual(output.status_code, 401) + data = json.loads(output.data) + self.assertEqual(pagure.api.APIERROR.EINVALIDTOK.name, + data['error_code']) + self.assertEqual(pagure.api.APIERROR.EINVALIDTOK.value, data['error']) + + # No comment added + repo = pagure.lib.get_project(self.session, 'foo') + issue = pagure.lib.search_issues(self.session, repo, issueid=1) + self.assertEqual(len(issue.comments), 0) + + # Create token for user foo + item = pagure.lib.model.Token( + id='foo_token2', + user_id=2, + project_id=4, + expiration=datetime.datetime.utcnow() + datetime.timedelta(days=30) + ) + self.session.add(item) + self.session.commit() + tests.create_tokens_acl(self.session, token_id='foo_token2') + + data = { + 'comment': 'This is a very interesting question', + } + headers = {'Authorization': 'token foo_token2'} + + # Valid request and authorized + output = self.app.post( + '/api/0/foo/issue/1/comment', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertDictEqual( + data, + {'message': 'Comment added'} + ) + + @patch('pagure.lib.git.update_git') + @patch('pagure.lib.notify.send_email') def test_api_view_issue_comment(self, p_send_email, p_ugt): """ Test the api_view_issue_comment endpoint. """ p_send_email.return_value = True diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index 9e510a3..ea8fe6b 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -460,6 +460,96 @@ class PagureFlaskApiProjecttests(tests.Modeltests): ) @patch('pagure.lib.git.generate_gitolite_acls') + def test_api_new_project_user_token(self, p_gga): + """ Test the api_new_project method of the flask api. """ + p_gga.return_value = True + + tests.create_projects(self.session) + tests.create_projects_git(os.path.join(self.path, 'tickets')) + tests.create_tokens(self.session, project_id=None) + tests.create_tokens_acl(self.session) + + headers = {'Authorization': 'token foo_token'} + + # Invalid token + output = self.app.post('/api/0/new', headers=headers) + self.assertEqual(output.status_code, 401) + data = json.loads(output.data) + self.assertEqual(pagure.api.APIERROR.EINVALIDTOK.name, + data['error_code']) + self.assertEqual(pagure.api.APIERROR.EINVALIDTOK.value, data['error']) + + headers = {'Authorization': 'token aaabbbcccddd'} + + # No input + output = self.app.post('/api/0/new', headers=headers) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Invalid or incomplete input submited", + "error_code": "EINVALIDREQ", + "errors": { + "name": ["This field is required."], + "description": ["This field is required."] + } + } + ) + + data = { + 'name': 'test', + } + + # Incomplete request + output = self.app.post( + '/api/0/new', data=data, headers=headers) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Invalid or incomplete input submited", + "error_code": "EINVALIDREQ", + "errors": {"description": ["This field is required."]} + } + ) + + data = { + 'name': 'test', + 'description': 'Just a small test project', + } + + # Valid request but repo already exists + output = self.app.post( + '/api/0/new/', data=data, headers=headers) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "The project repo \"test\" already exists " + "in the database", + "error_code": "ENOCODE" + } + ) + + data = { + 'name': 'test_42', + 'description': 'Just another small test project', + } + + # Valid request + output = self.app.post( + '/api/0/new/', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertDictEqual( + data, + {'message': 'Project "test_42" created'} + ) + + @patch('pagure.lib.git.generate_gitolite_acls') def test_api_new_project_user_ns(self, p_gga): """ Test the api_new_project method of the flask api. """ pagure.APP.config['USER_NAMESPACE'] = True @@ -632,6 +722,130 @@ class PagureFlaskApiProjecttests(tests.Modeltests): } ) + @patch('pagure.lib.git.generate_gitolite_acls') + def test_api_fork_project_user_token(self, p_gga): + """ Test the api_fork_project method of the flask api. """ + p_gga.return_value = True + + tests.create_projects(self.session) + for folder in ['docs', 'tickets', 'requests', 'repos']: + tests.create_projects_git( + os.path.join(self.path, folder), bare=True) + tests.create_tokens(self.session, project_id=None) + tests.create_tokens_acl(self.session) + + headers = {'Authorization': 'token foo_token'} + + # Invalid token + output = self.app.post('/api/0/fork', headers=headers) + self.assertEqual(output.status_code, 401) + data = json.loads(output.data) + self.assertEqual(pagure.api.APIERROR.EINVALIDTOK.name, + data['error_code']) + self.assertEqual(pagure.api.APIERROR.EINVALIDTOK.value, data['error']) + + headers = {'Authorization': 'token aaabbbcccddd'} + + # No input + output = self.app.post('/api/0/fork', headers=headers) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Invalid or incomplete input submited", + "error_code": "EINVALIDREQ", + "errors": {"repo": ["This field is required."]} + } + ) + + data = { + 'name': 'test', + } + + # Incomplete request + output = self.app.post( + '/api/0/fork', data=data, headers=headers) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Invalid or incomplete input submited", + "error_code": "EINVALIDREQ", + "errors": {"repo": ["This field is required."]} + } + ) + + data = { + 'repo': 'test', + } + + # Valid request + output = self.app.post( + '/api/0/fork/', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "message": "Repo \"test\" cloned to \"pingou/test\"" + } + ) + + data = { + 'repo': 'test', + } + + # project already forked + output = self.app.post( + '/api/0/fork/', data=data, headers=headers) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Repo \"forks/pingou/test\" already exists", + "error_code": "ENOCODE" + } + ) + + data = { + 'repo': 'test', + 'username': 'pingou', + } + + # Fork already exists + output = self.app.post( + '/api/0/fork/', data=data, headers=headers) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Repo \"forks/pingou/test\" already exists", + "error_code": "ENOCODE" + } + ) + + data = { + 'repo': 'test', + 'namespace': 'pingou', + } + + # Repo does not exists + output = self.app.post( + '/api/0/fork/', data=data, headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Project not found", + "error_code": "ENOPROJECT" + } + ) + if __name__ == '__main__': SUITE = unittest.TestLoader().loadTestsFromTestCase( PagureFlaskApiProjecttests) From 4499ab310f49ac5eaea60991623200f95b6feb89 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 06 2017 19:18:42 +0000 Subject: [PATCH 7/12] Regular users should only be allowed to have limited project-less API token Basically, we do not want people to generate API tokens that could flag any PR on any project, potentially leading to invalid flag or abuse of the system. --- diff --git a/pagure/default_config.py b/pagure/default_config.py index 9ac9d43..62f639f 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -226,10 +226,15 @@ ACLS = { CROSS_PROJECT_ACLS = [ 'create_project', 'fork_project', +] + +# ACLs with which admins are allowed to create project-less API tokens +ADMIN_API_ACLS = [ 'issue_comment', 'issue_create', 'pull_request_flag', 'pull_request_comment', + 'pull_request_merge', ] # Bootstrap URLS From dea6ca971e2411b39ddf2500b34524ac75d6fadf Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 06 2017 19:18:42 +0000 Subject: [PATCH 8/12] Support project-less API token to merge PR --- diff --git a/pagure/api/fork.py b/pagure/api/fork.py index 314fa27..319d569 100644 --- a/pagure/api/fork.py +++ b/pagure/api/fork.py @@ -318,7 +318,7 @@ def api_pull_request_merge(repo, requestid, username=None, namespace=None): raise pagure.exceptions.APIError( 404, error_code=APIERROR.EPULLREQUESTSDISABLED) - if repo != flask.g.token.project: + 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( diff --git a/tests/test_pagure_flask_api_fork.py b/tests/test_pagure_flask_api_fork.py index c4c7a8c..984ab64 100644 --- a/tests/test_pagure_flask_api_fork.py +++ b/tests/test_pagure_flask_api_fork.py @@ -560,6 +560,121 @@ class PagureFlaskApiForktests(tests.Modeltests): ) @patch('pagure.lib.notify.send_email') + @patch('pagure.lib.git.merge_pull_request') + def test_api_pull_request_merge_user_token(self, mpr, send_email): + """ Test the api_pull_request_merge method of the flask api. """ + mpr.return_value = 'Changes merged!' + send_email.return_value = True + + tests.create_projects(self.session) + tests.create_tokens(self.session, project_id=None) + tests.create_tokens_acl(self.session) + + # Create the pull-request to close + repo = pagure.lib.get_project(self.session, 'test') + forked_repo = pagure.lib.get_project(self.session, 'test') + req = pagure.lib.new_pull_request( + session=self.session, + repo_from=forked_repo, + branch_from='master', + repo_to=repo, + branch_to='master', + title='test pull-request', + user='pingou', + requestfolder=None, + ) + self.session.commit() + self.assertEqual(req.id, 1) + self.assertEqual(req.title, 'test pull-request') + + headers = {'Authorization': 'token aaabbbcccddd'} + + # Invalid project + output = self.app.post( + '/api/0/foo/pull-request/1/merge', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Project not found", + "error_code": "ENOPROJECT", + } + ) + + # Valid token, invalid PR + output = self.app.post( + '/api/0/test2/pull-request/1/merge', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + {'error': 'Pull-Request not found', 'error_code': "ENOREQ"} + ) + + # Valid token, invalid PR - other project + output = self.app.post( + '/api/0/test/pull-request/2/merge', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.data) + self.assertDictEqual( + data, + {'error': 'Pull-Request not found', 'error_code': "ENOREQ"} + ) + + # Create a token for foo for this project + item = pagure.lib.model.Token( + id='foobar_token', + user_id=2, + project_id=1, + expiration=datetime.datetime.utcnow() + datetime.timedelta( + days=30) + ) + self.session.add(item) + self.session.commit() + + # Allow the token to merge PR + acls = pagure.lib.get_acls(self.session) + acl = None + for acl in acls: + if acl.name == 'pull_request_merge': + 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 not admin + output = self.app.post( + '/api/0/test/pull-request/1/merge', headers=headers) + self.assertEqual(output.status_code, 403) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + 'error': 'You are not allowed to merge/close pull-request ' + 'for this project', + 'error_code': "ENOPRCLOSE", + } + ) + + headers = {'Authorization': 'token aaabbbcccddd'} + + # Merge PR + output = self.app.post( + '/api/0/test/pull-request/1/merge', headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertDictEqual( + data, + {"message": "Changes merged!"} + ) + + @patch('pagure.lib.notify.send_email') def test_api_pull_request_add_comment(self, mockemail): """ Test the api_pull_request_add_comment method of the flask api. """ mockemail.return_value = True From 2aa56fdafc70ecca6babcfff369c624871651087 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 06 2017 19:22:15 +0000 Subject: [PATCH 9/12] Add unit-tests testing creating a project with a namespace in the API --- diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index ea8fe6b..f59ea08 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -549,6 +549,48 @@ class PagureFlaskApiProjecttests(tests.Modeltests): {'message': 'Project "test_42" created'} ) + # Project with a namespace + pagure.APP.config['ALLOWED_PREFIX'] = ['rpms'] + data = { + 'name': 'test_42', + 'namespace': 'pingou', + 'description': 'Just another small test project', + } + + # Invalid namespace + output = self.app.post( + '/api/0/new/', data=data, headers=headers) + self.assertEqual(output.status_code, 400) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + "error": "Invalid or incomplete input submited", + "error_code": "EINVALIDREQ", + "errors": { + "namespace": [ + "Not a valid choice" + ] + } + } + ) + + data = { + 'name': 'test_42', + 'namespace': 'rpms', + 'description': 'Just another small test project', + } + + # All good + output = self.app.post( + '/api/0/new/', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertDictEqual( + data, + {'message': 'Project "rpms/test_42" created'} + ) + @patch('pagure.lib.git.generate_gitolite_acls') def test_api_new_project_user_ns(self, p_gga): """ Test the api_new_project method of the flask api. """ From 0c6a50813ff14eb97ed31d0c38aeef3534161dd7 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 06 2017 19:28:46 +0000 Subject: [PATCH 10/12] Add a test checking creating a private ticket using private = 'true' --- diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index 18a6e66..94eabd6 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -478,10 +478,12 @@ class PagureFlaskApiIssuetests(tests.Modeltests): data = json.loads(output.data) data['issue']['date_created'] = '1431414800' data['issue']['last_updated'] = '1431414800' + exp = FULL_ISSUE_LIST[1] + exp['id'] = 8 self.assertDictEqual( data, { - "issue": FULL_ISSUE_LIST[1], + "issue": exp, "message": "Issue created" } ) @@ -727,6 +729,28 @@ class PagureFlaskApiIssuetests(tests.Modeltests): } ) + # Private issue: 'true' + data = { + 'title': 'test issue1', + 'issue_content': 'This issue needs attention', + 'private': 'true', + } + output = self.app.post( + '/api/0/test/new_issue', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + data['issue']['date_created'] = '1431414800' + data['issue']['last_updated'] = '1431414800' + exp = FULL_ISSUE_LIST[1] + exp['id'] = 9 + self.assertDictEqual( + data, + { + "issue": exp, + "message": "Issue created" + } + ) + def test_api_view_issues(self): """ Test the api_view_issues method of the flask api. """ self.test_api_new_issue() From 10018d093e13f58eae157c3a2e78bd111221b730 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 06 2017 19:29:48 +0000 Subject: [PATCH 11/12] Drop un-necessary lines of code --- diff --git a/tests/test_pagure_flask_api_fork.py b/tests/test_pagure_flask_api_fork.py index 984ab64..ed6cee4 100644 --- a/tests/test_pagure_flask_api_fork.py +++ b/tests/test_pagure_flask_api_fork.py @@ -406,7 +406,6 @@ class PagureFlaskApiForktests(tests.Modeltests): # Allow the token to close PR acls = pagure.lib.get_acls(self.session) - acl = None for acl in acls: if acl.name == 'pull_request_close': break @@ -520,7 +519,6 @@ class PagureFlaskApiForktests(tests.Modeltests): # Allow the token to merge PR acls = pagure.lib.get_acls(self.session) - acl = None for acl in acls: if acl.name == 'pull_request_merge': break From 2c757271024159dd7d8584b83e03501128ffc795 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 06 2017 19:33:58 +0000 Subject: [PATCH 12/12] Rename add_user_token to add_api_user_token to be consistent --- diff --git a/pagure/templates/add_token.html b/pagure/templates/add_token.html index b6aed5c..a9f6b53 100644 --- a/pagure/templates/add_token.html +++ b/pagure/templates/add_token.html @@ -31,7 +31,7 @@ repo=repo.name, namespace=repo.namespace) }}" method="post"> {% else %} -
+ {% endif %} {% for acl in acls %}
diff --git a/pagure/templates/user_settings.html b/pagure/templates/user_settings.html index 8cdd62c..d572e9d 100644 --- a/pagure/templates/user_settings.html +++ b/pagure/templates/user_settings.html @@ -83,7 +83,8 @@
Email Addresses - + Add Email
diff --git a/pagure/ui/app.py b/pagure/ui/app.py index 82759de..319e4cd 100644 --- a/pagure/ui/app.py +++ b/pagure/ui/app.py @@ -762,7 +762,7 @@ def ssh_hostkey(): @APP.route('/settings/token/new/', methods=('GET', 'POST')) @APP.route('/settings/token/new', methods=('GET', 'POST')) @login_required -def add_user_token(): +def add_api_user_token(): """ Create an user token (not project specific). """ if admin_session_timedout(): diff --git a/tests/test_pagure_flask_ui_app.py b/tests/test_pagure_flask_ui_app.py index a9acd72..e04d64a 100644 --- a/tests/test_pagure_flask_ui_app.py +++ b/tests/test_pagure_flask_ui_app.py @@ -703,8 +703,8 @@ class PagureFlaskApptests(tests.Modeltests): @patch('pagure.lib.notify.send_email') @patch('pagure.ui.app.admin_session_timedout') - def test_add_user_email(self, ast, send_email): - """ Test the add_user_email endpoint. """ + def test_add_api_user_email(self, ast, send_email): + """ Test the add_api_user_email endpoint. """ send_email.return_value = True ast.return_value = False self.test_new_project()