From 3f19a59c18996b083d3392d0d4dca3a60ccb1c81 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Aug 23 2017 17:06:12 +0000 Subject: [PATCH 1/4] Add a configuration key to allow deleting forks but not projects Fixes https://pagure.io/pagure/issue/2460 Fixes https://pagure.io/fedora-infrastructure/issue/6271 Signed-off-by: Pierre-Yves Chibon --- diff --git a/doc/configuration.rst b/doc/configuration.rst index 629f7f5..671b734 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -639,6 +639,15 @@ the user interface of this pagure instance. Defaults to: ``True`` +ENABLE_DEL_FORKS +~~~~~~~~~~~~~~~~ + +This configuration key permits or forbids deletion of forks via +the user interface of this pagure instance. + +Defaults to: ``ENABLE_DEL_PROJECTS`` + + EMAIL_SEND ~~~~~~~~~~ diff --git a/pagure/templates/settings.html b/pagure/templates/settings.html index 72843ca..5aa325b 100644 --- a/pagure/templates/settings.html +++ b/pagure/templates/settings.html @@ -1090,7 +1090,10 @@ {% endif %} - {% if config.get('ENABLE_DEL_PROJECTS', True) %} + {% if (not repo.is_fork and config.get('ENABLE_DEL_PROJECTS', True)) + or + (repo.is_fork and not config.get('ENABLE_DEL_FORKS', + config.get('ENABLE_DEL_PROJECTS', True))) %}
diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 8e4a451..e9e8038 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -1394,7 +1394,12 @@ def change_ref_head(repo, username=None, namespace=None): def delete_repo(repo, username=None, namespace=None): """ Delete the present project. """ - if not pagure.APP.config.get('ENABLE_DEL_PROJECTS', True): + repo = flask.g.repo + + del_project = pagure.APP.config.get('ENABLE_DEL_PROJECTS', True) + del_fork = pagure.APP.config.get('ENABLE_DEL_FORKS', del_project) + if (not repo.is_fork and not del_project) \ + or (repo.is_fork and not del_fork): flask.abort(404) if admin_session_timedout(): @@ -1405,8 +1410,6 @@ def delete_repo(repo, username=None, namespace=None): return flask.redirect( flask.url_for('auth_login', next=url)) - repo = flask.g.repo - if not flask.g.repo_admin: flask.abort( 403, diff --git a/tests/test_pagure_flask_ui_repo_delete_project.py b/tests/test_pagure_flask_ui_repo_delete_project.py new file mode 100644 index 0000000..b26e508 --- /dev/null +++ b/tests/test_pagure_flask_ui_repo_delete_project.py @@ -0,0 +1,127 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2017 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + +""" + +__requires__ = ['SQLAlchemy >= 0.8'] +import pkg_resources + +import sys +import os + +from mock import patch, MagicMock + +sys.path.insert(0, os.path.join(os.path.dirname( + os.path.abspath(__file__)), '..')) + +import pagure +import pagure.lib +import tests + + +class PagureFlaskDeleteRepotests(tests.Modeltests): + """ Tests for deleting a project in pagure """ + + def setUp(self): + """ Set up the environnment, ran before every tests. """ + super(PagureFlaskDeleteRepotests, self).setUp() + + pagure.APP.config['TESTING'] = True + pagure.SESSION = self.session + pagure.ui.SESSION = self.session + pagure.ui.app.SESSION = self.session + pagure.ui.filters.SESSION = self.session + pagure.ui.repo.SESSION = self.session + + # Create some projects + tests.create_projects(self.session) + tests.create_projects_git(os.path.join(self.path, 'repos')) + self.session.commit() + + # Create all the git repos + tests.create_projects_git(os.path.join(self.path, 'repos')) + tests.create_projects_git(os.path.join(self.path, 'docs')) + tests.create_projects_git( + os.path.join(self.path, 'tickets'), bare=True) + tests.create_projects_git( + os.path.join(self.path, 'requests'), bare=True) + + project = pagure.get_authorized_project( + self.session, project_name='test') + self.assertIsNotNone(project) + + # Create a fork + task_id = pagure.lib.fork_project( + session=self.session, + user='pingou', + repo=project, + gitfolder=os.path.join(self.path, 'repos'), + docfolder=os.path.join(self.path, 'docs'), + ticketfolder=os.path.join(self.path, 'tickets'), + requestfolder=os.path.join(self.path, 'requests'), + ) + pagure.lib.tasks.get_result(task_id).get() + + # Ensure everything was correctly created + projects = pagure.lib.search_projects(self.session) + self.assertEqual(len(projects), 4) + + @patch.dict('pagure.APP.config', {'ENABLE_DEL_PROJECTS': False}) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + @patch('pagure.ui.repo.admin_session_timedout', + MagicMock(return_value=False)) + def test_delete_repo_when_turned_off(self): + """ Test the delete_repo endpoint for a fork when only deleting main + project is forbidden. + """ + + user = tests.FakeUser(username='pingou') + with tests.user_set(pagure.APP, user): + output = self.app.post('/test/delete', follow_redirects=True) + self.assertEqual(output.status_code, 404) + + projects = pagure.lib.search_projects(self.session) + self.assertEqual(len(projects), 4) + + @patch.dict('pagure.APP.config', {'ENABLE_DEL_PROJECTS': False}) + @patch.dict('pagure.APP.config', {'ENABLE_DEL_FORKS': True}) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + @patch('pagure.ui.repo.admin_session_timedout', + MagicMock(return_value=False)) + def test_delete_fork_when_project_off(self): + """ Test the delete_repo endpoint for a fork when only deleting main + project is forbidden. + """ + + user = tests.FakeUser(username='pingou') + with tests.user_set(pagure.APP, user): + output = self.app.post( + '/fork/pingou/test/delete', follow_redirects=True) + self.assertEqual(output.status_code, 200) + + projects = pagure.lib.search_projects(self.session) + self.assertEqual(len(projects), 3) + + @patch.dict('pagure.APP.config', {'ENABLE_DEL_PROJECTS': False}) + @patch.dict('pagure.APP.config', {'ENABLE_DEL_FORKS': False}) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + @patch('pagure.ui.repo.admin_session_timedout', + MagicMock(return_value=False)) + def test_delete_fork_when_fork_and_project_off(self): + """ Test the delete_repo endpoint for a fork when deleting fork and + project is forbidden. + """ + + user = tests.FakeUser(username='pingou') + with tests.user_set(pagure.APP, user): + output = self.app.post( + '/fork/pingou/test/delete', follow_redirects=True) + self.assertEqual(output.status_code, 404) + + projects = pagure.lib.search_projects(self.session) + self.assertEqual(len(projects), 4) From e79c7c381b3fb5746d10ab33a0f900fa45359d1d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Aug 23 2017 17:06:12 +0000 Subject: [PATCH 2/4] Show the entire project name in the UI This helps figuring out if we're deleting a project or a fork Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/templates/settings.html b/pagure/templates/settings.html index 5aa325b..0c30864 100644 --- a/pagure/templates/settings.html +++ b/pagure/templates/settings.html @@ -1109,7 +1109,8 @@
From 47ccacf0ef335a67d959ecd2dbe50120c3cb1d10 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Aug 23 2017 17:06:12 +0000 Subject: [PATCH 3/4] Small clean up in a test to use patch() to adjust the configuration Signed-off-by: Pierre-Yves Chibon --- diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index 92843ba..691fdef 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -2600,7 +2600,7 @@ index 0000000..fb7093d '' in output.data) - + @patch.dict('pagure.APP.config', {'ENABLE_DEL_PROJECTS': False}) @patch('pagure.lib.notify.send_email') @patch('pagure.ui.repo.admin_session_timedout') def test_delete_repo_when_turned_off(self, ast, send_email): @@ -2608,7 +2608,6 @@ index 0000000..fb7093d turned off in the pagure instance """ ast.return_value = False send_email.return_value = True - pagure.APP.config['ENABLE_DEL_PROJECTS'] = False # No Git repo output = self.app.post('/foo/delete') @@ -2814,8 +2813,6 @@ index 0000000..fb7093d '/fork/pingou/test3/delete', follow_redirects=True) self.assertEqual(output.status_code, 404) - pagure.APP.config['ENABLE_DEL_PROJECTS'] = True - @patch('pagure.lib.notify.send_email') @patch('pagure.ui.repo.admin_session_timedout') def test_delete_repo(self, ast, send_email): From d4f94ddebb53b7196b409ab73bade055380c71fc Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Aug 23 2017 17:06:12 +0000 Subject: [PATCH 4/4] Fix running the tests and a variable name Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/api/project.py b/pagure/api/project.py index 6a53383..904673d 100644 --- a/pagure/api/project.py +++ b/pagure/api/project.py @@ -1117,7 +1117,7 @@ def api_generate_acls(repo, username=None, namespace=None): wait = json.get('wait', False) try: - task = pagure.lib.git.generate_gitolite_acls( + taskid = pagure.lib.git.generate_gitolite_acls( project=project, ).id diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index 3a7b76d..9509e37 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -2130,7 +2130,7 @@ class PagureFlaskApiProjecttests(tests.Modeltests): } self.assertEqual(data, expected_output) mock_gen_acls.assert_called_once_with( - name='test', namespace=None, user=None) + name='test', namespace=None, user=None, group=None) @patch('pagure.lib.tasks.get_result') @patch('pagure.lib.tasks.generate_gitolite_acls.delay') @@ -2162,7 +2162,7 @@ class PagureFlaskApiProjecttests(tests.Modeltests): } self.assertEqual(data, expected_output) mock_gen_acls.assert_called_once_with( - name='test', namespace=None, user=None) + name='test', namespace=None, user=None, group=None) mock_get_result.assert_called_once_with('abc-1234') def test_api_generate_acls_no_project(self):