From cd4df4e668359469ec29e055624ae929fa1d14ed Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 18 2019 08:43:21 +0000 Subject: Allow turning on issue tracking for only some namespaces This commit allows turning on issue tracking for only some namespaces specified in the configuration file. This way, on src.fp.o we can enable issue tracking in pagure for the tests namespace without enabling it for the rpms namespace (for example) since the issues for the rpms namespace are meant to be tracked in bugzilla.redhat.com Fixes https://pagure.io/pagure/issue/4244 Signed-off-by: Pierre-Yves Chibon --- diff --git a/doc/configuration.rst b/doc/configuration.rst index c8009c6..9961ac0 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -999,6 +999,20 @@ for all the projects hosted on this pagure instance. Defaults to: ``True`` +ENABLE_TICKETS_NAMESPACE +~~~~~~~~~~~~~~~~~~~~~~~~ + +This configuration key allows to restrict the namespace in which the ticketing +system is enabled. +So if your pagure instance has ``ENABLE_TICKETS`` as ``True`` and sets +``ENABLE_TICKETS_NAMESPACE`` to ``['tests', 'infra']`` only the projects opened +in these two namespaces will have the ticketing system enabled. All the other +namespaces will not. + + +Defaults to: ``[]`` + + ENABLE_DOCS ~~~~~~~~~~~ diff --git a/pagure/api/issue.py b/pagure/api/issue.py index e9d5200..7835460 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -67,7 +67,12 @@ def _check_issue_tracker(repo): :param repo: repository :raises pagure.exceptions.APIError: when issue tracker is disabled """ - if not repo.settings.get("issue_tracker", True): + ticket_namespaces = pagure_config.get("ENABLE_TICKETS_NAMESPACE") + if ( + ticket_namespaces + and repo.namespace + and repo.namespace not in ticket_namespaces + ) or not repo.settings.get("issue_tracker", True): raise pagure.exceptions.APIError( 404, error_code=APIERROR.ETRACKERDISABLED ) diff --git a/pagure/decorators.py b/pagure/decorators.py index bca63bf..3e6e329 100644 --- a/pagure/decorators.py +++ b/pagure/decorators.py @@ -1,10 +1,11 @@ # -*- coding: utf-8 -*- """ - (c) 2018 - Copyright Red Hat Inc + (c) 2018-2019 - Copyright Red Hat Inc Authors: Clement Verna + Pierre-Yves Chibon """ @@ -25,7 +26,9 @@ def has_issue_tracker(function): @wraps(function) def check_issue_tracker(*args, **kwargs): repo = flask.g.repo - if not repo.settings.get("issue_tracker", True): + if not flask.g.issues_enabled or not repo.settings.get( + "issue_tracker", True + ): flask.abort(404, "No issue tracker found for this project") # forbid all POST requests if the issue tracker is made read-only if flask.request.method == "POST" and repo.settings.get( diff --git a/pagure/flask_app.py b/pagure/flask_app.py index 3f24e46..df9d687 100644 --- a/pagure/flask_app.py +++ b/pagure/flask_app.py @@ -244,6 +244,8 @@ def set_request(): flask.g.version = pagure.__version__ flask.g.confirmationform = pagure.forms.ConfirmationForm() + flask.g.issues_enabled = pagure_config.get("ENABLE_TICKETS", True) + # The API namespace has its own way of getting repo and username and # of handling errors if flask.request.blueprint == "api_ns": @@ -322,6 +324,28 @@ def set_request(): if flask.g.repo is None: flask.abort(404, "Project not found") + # If issues are not globally enabled, there is no point in continuing + if flask.g.issues_enabled: + + ticket_namespaces = pagure_config.get("ENABLE_TICKETS_NAMESPACE") + + if ticket_namespaces and flask.g.repo.namespace: + if flask.g.repo.namespace in (ticket_namespaces or []): + # If the namespace is in the allowed list + # issues are enabled + flask.g.issues_enabled = True + else: + # If the namespace isn't in the list of namespaces + # issues are disabled + flask.g.issues_enabled = False + + flask.g.issues_project_disabled = False + if not flask.g.repo.settings.get("issue_tracker", True): + # If the project specifically disabled its issue tracker, + # disable issues + flask.g.issues_project_disabled = True + flask.g.issues_enabled = False + flask.g.reponame = get_repo_path(flask.g.repo) flask.g.repo_obj = pygit2.Repository(flask.g.reponame) flask.g.repo_admin = pagure.utils.is_repo_admin(flask.g.repo) diff --git a/pagure/templates/comment_update.html b/pagure/templates/comment_update.html index e5ac679..cd15c51 100644 --- a/pagure/templates/comment_update.html +++ b/pagure/templates/comment_update.html @@ -89,7 +89,7 @@ $("#update_comment").atwho(userConfig); }); $.when( - {% if config.get('ENABLE_TICKETS', True) %} + {% if g.issues_enabled %} $.get("{{ url_for('api_ns.api_view_issues', namespace=repo.namespace, repo=repo.name, username=username, status='all') }}"), {% endif %} $.get("{{ url_for('api_ns.api_pull_request_views', namespace=repo.namespace, repo=repo.name, username=username, status='all') }}") diff --git a/pagure/templates/repo_master.html b/pagure/templates/repo_master.html index 294beaa..8b1db30 100644 --- a/pagure/templates/repo_master.html +++ b/pagure/templates/repo_master.html @@ -82,8 +82,7 @@
{% if g.authenticated %} - {% if repo.settings.get('issue_tracker', True) - and config.get('ENABLE_TICKETS', True) + {% if g.issues_enabled and not repo.settings.get('issue_tracker_read_only', False) %} {% endif %} - {% if config.get('ENABLE_TICKETS', True) and repo - and repo.settings.get('issue_tracker', True) %} + {% if g.issues_enabled %}
  • {% endif %} - {% if config.get('ENABLE_TICKETS', True) and repo.milestones - and repo.settings.get('issue_tracker', True)%} + {% if g.issues_enabled and repo.milestones %}
  • Hooks {% endif %} - {% if config.get('ENABLE_TICKETS', True) - and repo.settings.get('issue_tracker', True) %} + {% if g.issues_enabled %} Priorities Reports {% endif %} - {% if (config.get('ENABLE_TICKETS', True) - and repo.settings.get('issue_tracker', True)) - or repo.settings.get('pull_requests', True) %} + {% if g.issues_enabled or repo.settings.get('pull_requests', True) %} Tags

    The email addresses entered below will receive all the notifications - related to {% if config.get('ENABLE_TICKETS', True) %} + related to {% if g.issues_enabled %} (public) issues and {% endif %}pull-requests, this includes - notifications about {% if config.get('ENABLE_TICKETS', True) %} + notifications about {% if g.issues_enabled %} new issue or {% endif %} new pull-request, new comment and status change.

    @@ -260,7 +257,7 @@ {{ tag_form.csrf_token }}
    - {% if config.get('ENABLE_TICKETS', True) %} + {% if g.issues_enabled %}
    Issues notifications @@ -480,7 +477,7 @@
    {% for plugin in plugins %} - {% if not config.get('ENABLE_TICKETS', True) and plugin in ['Pagure tickets'] %} + {% if not g.issues_enabled and plugin in ['Pagure tickets'] %} {% else %}
    {% endif %} - {% if config.get('ENABLE_TICKETS', True) - and repo.settings.get('issue_tracker', True) %} + {% if g.issues_enabled %}

    Priorities @@ -854,9 +850,7 @@ {% endif %} - {% if (config.get('ENABLE_TICKETS', True) - and repo.settings.get('issue_tracker', True)) - or repo.settings.get('pull_requests', True) %} + {% if g.issues_enabled or repo.settings.get('pull_requests', True) %}

    Tags @@ -869,7 +863,7 @@

    - {% if config.get('ENABLE_TICKETS', True) %} + {% if g.issues_enabled %}
    {% for key in repo.settings | sort %} - {% if not config.get('ENABLE_TICKETS', True) and key in ['issue_tracker', 'issues_default_to_private'] %} + {% if not g.issues_enabled and not g.issues_project_disabled and key in ['issue_tracker', 'issues_default_to_private'] %} {% elif not config.get('DOC_APP_URL') and key in ['project_documentation'] %} {% elif not config.get('WEBHOOK') and key in ['Web-hooks'] %} {% else %} diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index bc54cf2..b3f0d8a 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -327,6 +327,28 @@ class PagureFlaskApiIssuetests(tests.SimplePagureTest): self.assertEqual( pagure.api.APIERROR.EINVALIDTOK.name, data['error_code']) + @patch.dict('pagure.config.config', {'ENABLE_TICKETS_NAMESPACE': ['foobar']}) + def test_api_new_issue_wrong_namespace(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) + tests.create_tokens_acl(self.session) + + headers = {'Authorization': 'token aaabbbcccddd'} + + # Valid token, wrong project + output = self.app.post( + '/api/0/somenamespace/test3/new_issue', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual(sorted(data.keys()), ['error', 'error_code']) + self.assertEqual( + pagure.api.APIERROR.ETRACKERDISABLED.value, data['error']) + self.assertEqual( + pagure.api.APIERROR.ETRACKERDISABLED.name, data['error_code']) + def test_api_new_issue_no_input(self): """ Test the api_new_issue method of the flask api. """ tests.create_projects(self.session) @@ -2596,6 +2618,47 @@ class PagureFlaskApiIssuetests(tests.SimplePagureTest): } ) + @patch.dict('pagure.config.config', {'ENABLE_TICKETS_NAMESPACE': ['foobar']}) + def test_api_change_milestone_issue_wrong_namespace(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')) + tests.create_tokens(self.session) + tests.create_tokens_acl(self.session) + + # Set some milestones to the project + repo = pagure.lib.query.get_authorized_project( + self.session, 'test3', namespace='somenamespace') + repo.milestones = {'v1.0': None, 'v2.0': 'Soon'} + self.session.add(repo) + self.session.commit() + + # Create normal issue + repo = pagure.lib.query.get_authorized_project(self.session, 'test') + msg = pagure.lib.query.new_issue( + session=self.session, + repo=repo, + title='Test issue #1', + content='We should work on this', + user='pingou', + private=False, + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue #1') + + headers = {'Authorization': 'token aaabbbcccddd'} + + # Valid token, wrong project + output = self.app.post( + '/api/0/somenamespace/test3/issue/1/milestone', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual(sorted(data.keys()), ['error', 'error_code']) + self.assertEqual( + pagure.api.APIERROR.ETRACKERDISABLED.value, data['error']) + self.assertEqual( + pagure.api.APIERROR.ETRACKERDISABLED.name, data['error_code']) + def test_api_change_milestone_issue_wrong_token(self): """ Test the api_change_milestone_issue method of the flask api. """ tests.create_projects(self.session) @@ -3245,6 +3308,48 @@ class PagureFlaskApiIssuetests(tests.SimplePagureTest): } ) + @patch.dict('pagure.config.config', {'ENABLE_TICKETS_NAMESPACE': ['foobar']}) + def test_api_assign_issue_wrong_namespace(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')) + tests.create_tokens(self.session) + tests.create_tokens_acl(self.session) + + # Set some milestones to the project + repo = pagure.lib.query.get_authorized_project( + self.session, 'test3', namespace='somenamespace') + repo.milestones = {'v1.0': None, 'v2.0': 'Soon'} + self.session.add(repo) + self.session.commit() + + # Create normal issue + repo = pagure.lib.query.get_authorized_project( + self.session, 'test3', namespace='somenamespace') + msg = pagure.lib.query.new_issue( + session=self.session, + repo=repo, + title='Test issue #1', + content='We should work on this', + user='pingou', + private=False, + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue #1') + + headers = {'Authorization': 'token aaabbbcccddd'} + + # Valid token, wrong project + output = self.app.post( + '/api/0/somenamespace/test3/issue/1/assign', headers=headers) + self.assertEqual(output.status_code, 404) + data = json.loads(output.get_data(as_text=True)) + self.assertEqual(sorted(data.keys()), ['error', 'error_code']) + self.assertEqual( + pagure.api.APIERROR.ETRACKERDISABLED.value, data['error']) + self.assertEqual( + pagure.api.APIERROR.ETRACKERDISABLED.name, data['error_code']) + @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') def test_api_assign_issue(self, p_send_email, p_ugt): diff --git a/tests/test_pagure_flask_ui_issues.py b/tests/test_pagure_flask_ui_issues.py index 207e372..bb86064 100644 --- a/tests/test_pagure_flask_ui_issues.py +++ b/tests/test_pagure_flask_ui_issues.py @@ -40,6 +40,52 @@ import tests class PagureFlaskIssuestests(tests.Modeltests): """ Tests for flask issues controller of pagure """ + @patch.dict('pagure.config.config', {'ENABLE_TICKETS_NAMESPACE': ['foobar']}) + @patch('pagure.lib.git.update_git', MagicMock(return_value=True)) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_new_issue_wrong_namespace(self): + """ Test the new_issue endpoint. """ + + tests.create_projects(self.session) + tests.create_projects_git( + os.path.join(self.path, 'repos'), bare=True) + + user = tests.FakeUser(username='pingou') + with tests.user_set(self.app.application, user): + csrf_token = self.get_csrf() + data = { + 'title': 'Test issue', + 'issue_content': 'We really should improve on this issue', + 'status': 'Open', + 'csrf_token': csrf_token, + } + + # Things work fine when the project has no namespace + + output = self.app.post( + '/test/new_issue', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Issue #1: Test issue - test - Pagure', + output_text) + self.assertIn( + '', + output_text) + + # Things do not work when the project has a namespace not allowed + + output = self.app.post( + '/somenamespace/test3/new_issue', data=data, + follow_redirects=True) + self.assertEqual(output.status_code, 404) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Page not found :\'( - Pagure', output_text) + self.assertIn( + '

    No issue tracker found for this project

    ', output_text) + @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') def test_new_issue(self, p_send_email, p_ugt): @@ -619,6 +665,36 @@ class PagureFlaskIssuestests(tests.Modeltests): '\n     ' '\n v2.0\n', output_text) + @patch.dict('pagure.config.config', {'ENABLE_TICKETS_NAMESPACE': ['foobar']}) + @patch('pagure.lib.git.update_git', MagicMock(return_value=True)) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_view_issues_wrong_namespace(self): + """ Test the view_issues endpoint. """ + + tests.create_projects(self.session) + tests.create_projects_git( + os.path.join(self.path, 'repos'), bare=True) + + user = tests.FakeUser(username='pingou') + with tests.user_set(self.app.application, user): + # Things work fine when the project has no namespace + + output = self.app.get('/test/issues') + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Issues - test - Pagure', output_text) + + # Things do not work when the project has a namespace not allowed + + output = self.app.get('/somenamespace/test3/issues') + self.assertEqual(output.status_code, 404) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Page not found :\'( - Pagure', output_text) + self.assertIn( + '

    No issue tracker found for this project

    ', output_text) + @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') def test_view_issues(self, p_send_email, p_ugt): @@ -1670,6 +1746,74 @@ class PagureFlaskIssuestests(tests.Modeltests): output = self.app.get('/test/issue/1') self.assertEqual(output.status_code, 200) + @patch.dict('pagure.config.config', {'ENABLE_TICKETS_NAMESPACE': ['foobar']}) + @patch('pagure.lib.git.update_git', MagicMock(return_value=True)) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_update_issue_wrong_namespace(self): + """ Test the update_issue endpoint. """ + + tests.create_projects(self.session) + tests.create_projects_git( + os.path.join(self.path, 'repos'), bare=True) + + # Create normal issue on test + repo = pagure.lib.query.get_authorized_project(self.session, 'test') + msg = pagure.lib.query.new_issue( + session=self.session, + repo=repo, + title='Test issue #1', + content='We should work on this', + user='pingou', + private=False, + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue #1') + + # Create normal issue on test3 + repo = pagure.lib.query.get_authorized_project( + self.session, 'test3', namespace='somenamespace') + msg = pagure.lib.query.new_issue( + session=self.session, + repo=repo, + title='Test issue #1', + content='We should work on this', + user='pingou', + private=False, + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue #1') + + user = tests.FakeUser(username='pingou') + with tests.user_set(self.app.application, user): + # Add new comment + data = { + 'csrf_token': self.get_csrf(), + 'status': 'Closed', + 'close_status': 'Fixed', + 'comment': 'Woohoo a second comment!', + } + + # Things work fine when the project has no namespace + output = self.app.post( + '/test/issue/1/update', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Issue #1: Test issue #1 - test - Pagure', + output_text) + + # Things do not work when the project has a namespace not allowed + + output = self.app.post( + '/somenamespace/test3/issue/1/update', data=data, + follow_redirects=True) + self.assertEqual(output.status_code, 404) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Page not found :\'( - Pagure', output_text) + self.assertIn( + '

    No issue tracker found for this project

    ', output_text) + @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') def test_update_issue(self, p_send_email, p_ugt):