From 49fe910f1c057d49bfb7f3d2f2ba72b8c871053a Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Aug 01 2020 15:33:12 +0000 Subject: [PATCH 1/2] Do not fall back to the default branch when viewing raw files Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 06e38ac..d6596e8 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -693,10 +693,7 @@ def view_raw_file( try: commit = repo_obj.get(identifier) except ValueError: - if "master" not in repo_obj.listall_branches(): - flask.abort(404, description="Branch not found") - # If it's not a commit id then it's part of the filename - commit = repo_obj[repo_obj.head.target] + flask.abort(404, description="Branch not found") if not commit: flask.abort(404, description="Commit %s not found" % (identifier)) diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index a100f73..27bf6e6 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -2818,13 +2818,13 @@ class PagureFlaskRepotests(tests.Modeltests): self.assertTrue(output_text.startswith(b"\x00\x00\x01\x00")) # View by image name -- somehow we support this - output = self.app.get("/test/raw/sources/f/test.jpg") + output = self.app.get("/test/raw/master/f/test.jpg") self.assertEqual(output.status_code, 200) output_text = output.get_data() self.assertTrue(output_text.startswith(b"\x00\x00\x01\x00")) # View binary file - output = self.app.get("/test/raw/sources/f/test_binary") + output = self.app.get("/test/raw/master/f/test_binary") self.assertEqual(output.status_code, 200) output_text = output.get_data() self.assertEqual( @@ -2837,7 +2837,7 @@ class PagureFlaskRepotests(tests.Modeltests): self.assertEqual(output.status_code, 404) # View by image name -- with a non-existant file - output = self.app.get("/test/raw/sources/f/testfoo.jpg") + output = self.app.get("/test/raw/master/f/testfoo.jpg") self.assertEqual(output.status_code, 404) output = self.app.get("/test/raw/master/f/folder1/testfoo.jpg") self.assertEqual(output.status_code, 404) @@ -4822,6 +4822,18 @@ index 0000000..fb7093d output = self.app.get("/foo/edit/foo/f/sources") self.assertEqual(output.status_code, 404) + # Add some content to the git repo + tests.add_content_git_repo( + os.path.join(self.path, "repos", "test.git") + ) + tests.add_readme_git_repo(os.path.join(self.path, "repos", "test.git")) + tests.add_binary_git_repo( + os.path.join(self.path, "repos", "test.git"), "test.jpg" + ) + tests.add_binary_git_repo( + os.path.join(self.path, "repos", "test.git"), "test_binary" + ) + user = tests.FakeUser() with tests.user_set(self.app.application, user): # No project registered in the DB @@ -4834,7 +4846,7 @@ index 0000000..fb7093d ) # No a repo admin - output = self.app.get("/test/edit/foo/f/sources") + output = self.app.get("/test/edit/master/f/sources") self.assertEqual(output.status_code, 403) # User not logged in @@ -4844,24 +4856,6 @@ index 0000000..fb7093d user.username = "pingou" with tests.user_set(self.app.application, user): - # No such file - output = self.app.get("/test/edit/foo/f/sources") - self.assertEqual(output.status_code, 404) - - # Add some content to the git repo - tests.add_content_git_repo( - os.path.join(self.path, "repos", "test.git") - ) - tests.add_readme_git_repo( - os.path.join(self.path, "repos", "test.git") - ) - tests.add_binary_git_repo( - os.path.join(self.path, "repos", "test.git"), "test.jpg" - ) - tests.add_binary_git_repo( - os.path.join(self.path, "repos", "test.git"), "test_binary" - ) - output = self.app.get("/test/edit/master/foofile") self.assertEqual(output.status_code, 404) From d52d131dc80927e0bdf5db16c70f7ef7d8900b64 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Aug 01 2020 15:33:12 +0000 Subject: [PATCH 2/2] Allow collaborators to edit files in the branch that they have access to Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index d6596e8..b6aacc8 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -2556,22 +2556,15 @@ def revoke_api_token(repo, token_id, username=None, namespace=None): methods=("GET", "POST"), ) @login_required -@is_repo_admin def edit_file(repo, branchname, filename, username=None, namespace=None): """ Edit a file online. """ repo = flask.g.repo repo_obj = flask.g.repo_obj - user = pagure.lib.query.search_user( - flask.g.session, username=flask.g.fas_user.username - ) - if repo_obj.is_empty: flask.abort(404, description="Empty repo cannot have a file") - form = pagure.forms.EditFileForm(emails=user.emails) - branch = None if branchname in repo_obj.listall_branches(): branch = repo_obj.lookup_branch(branchname) @@ -2579,6 +2572,28 @@ def edit_file(repo, branchname, filename, username=None, namespace=None): else: flask.abort(400, description="Invalid branch specified") + user = pagure.lib.query.search_user( + flask.g.session, username=flask.g.fas_user.username + ) + if not user: + flask.abort( + 403, + description="You are not allowed to edit files in this project", + ) + + form = pagure.forms.EditFileForm(emails=user.emails) + + if not pagure.utils.is_repo_collaborator( + repo, + refname="refs/heads/%s" % form.branch.data, + username=None, + session=flask.g.session, + ): + flask.abort( + 403, + description="You are not allowed to edit files in this project", + ) + if form.validate_on_submit(): try: task = pagure.lib.tasks.update_file_in_git.delay( diff --git a/tests/test_pagure_lib_git_auth.py b/tests/test_pagure_lib_git_auth.py index e71e5e0..4a22754 100644 --- a/tests/test_pagure_lib_git_auth.py +++ b/tests/test_pagure_lib_git_auth.py @@ -230,3 +230,423 @@ class PagureLibGitAuthtests(tests.Modeltests): self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) self.assertEqual(output_text, "foo\n bar\n baz") + + +class PagureLibGitAuthPagureBackendtests(tests.Modeltests): + """ Tests for pagure.lib.git_auth """ + + config_values = {"authbackend": "pagure"} + + def setUp(self): + super(PagureLibGitAuthPagureBackendtests, self).setUp() + + tests.create_projects(self.session) + tests.create_tokens(self.session) + tests.create_tokens_acl(self.session) + self.create_project_full("hooktest") + + def test_edit_no_commit(self): + + user = tests.FakeUser() + user.username = "foo" + with tests.user_set(self.app.application, user): + # Add some content to the git repo + tests.add_content_git_repo( + os.path.join(self.path, "repos", "hooktest.git") + ) + + data = { + "content": "foo\n bar\n baz", + "commit_title": "test commit", + "commit_message": "Online commits from the gure.lib.get", + "email": "bar@pingou.com", + "branch": "master", + "csrf_token": self.get_csrf(), + } + + output = self.app.post( + "/hooktest/edit/master/f/sources", + data=data, + follow_redirects=True, + ) + self.assertEqual(output.status_code, 403) + output_text = output.get_data(as_text=True) + self.assertIn( + "You are not allowed to edit files in this project", + output_text, + ) + + # Check file after the commit: + output = self.app.get("/hooktest/raw/master/f/sources") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertEqual(output_text, "foo\n bar") + + def test_edit_ticket_rejected(self): + project = pagure.lib.query._get_project(self.session, "hooktest") + + # Add user foo to project test + msg = pagure.lib.query.add_user_to_project( + self.session, + project=project, + new_user="foo", + user="pingou", + access="ticket", + branches="epel*", + ) + self.session.commit() + + user = tests.FakeUser() + user.username = "foo" + with tests.user_set(self.app.application, user): + # Add some content to the git repo + tests.add_content_git_repo( + os.path.join(self.path, "repos", "hooktest.git") + ) + + data = { + "content": "foo\n bar\n baz", + "commit_title": "test commit", + "commit_message": "Online commits from the gure.lib.get", + "email": "foo@bar.com", + "branch": "master", + "csrf_token": self.get_csrf(), + } + + output = self.app.post( + "/hooktest/edit/master/f/sources", + data=data, + follow_redirects=True, + ) + self.assertEqual(output.status_code, 403) + output_text = output.get_data(as_text=True) + self.assertIn( + "You are not allowed to edit files in this project", + output_text, + ) + + # Check file after the commit: + output = self.app.get("/hooktest/raw/master/f/sources") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertEqual(output_text, "foo\n bar") + + def test_edit_contributor_rejected(self): + project = pagure.lib.query._get_project(self.session, "hooktest") + + # Add user foo to project test + msg = pagure.lib.query.add_user_to_project( + self.session, + project=project, + new_user="foo", + user="pingou", + access="collaborator", + branches="epel*", + ) + self.session.commit() + + user = tests.FakeUser() + user.username = "foo" + with tests.user_set(self.app.application, user): + # Add some content to the git repo + tests.add_content_git_repo( + os.path.join(self.path, "repos", "hooktest.git") + ) + + data = { + "content": "foo\n bar\n baz", + "commit_title": "test commit", + "commit_message": "Online commits from the gure.lib.get", + "email": "foo@bar.com", + "branch": "master", + "csrf_token": self.get_csrf(), + } + + output = self.app.post( + "/hooktest/edit/master/f/sources", + data=data, + follow_redirects=True, + ) + self.assertEqual(output.status_code, 403) + output_text = output.get_data(as_text=True) + self.assertIn( + "You are not allowed to edit files in this project", + output_text, + ) + + # Check file after the commit: + output = self.app.get("/hooktest/raw/master/f/sources") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertEqual(output_text, "foo\n bar") + + def test_edit_contributor_passed_epel8(self): + project = pagure.lib.query._get_project(self.session, "hooktest") + + # Add user foo to project test + msg = pagure.lib.query.add_user_to_project( + self.session, + project=project, + new_user="foo", + user="pingou", + access="collaborator", + branches="epel*", + ) + self.session.commit() + + user = tests.FakeUser() + user.username = "foo" + with tests.user_set(self.app.application, user): + # Add some content to the git repo + tests.add_content_git_repo( + os.path.join(self.path, "repos", "hooktest.git") + ) + + data = { + "content": "foo\n bar\n baz", + "commit_title": "test commit", + "commit_message": "Online commits from the gure.lib.get", + "email": "foo@bar.com", + "branch": "epel8", + "csrf_token": self.get_csrf(), + } + + output = self.app.post( + "/hooktest/edit/master/f/sources", + data=data, + follow_redirects=True, + ) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + "Commits - hooktest - Pagure", output_text + ) + + # Check file after the commit: + # master did not change + output = self.app.get("/hooktest/raw/master/f/sources") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertEqual(output_text, "foo\n bar") + + # epel8 did change + output = self.app.get("/hooktest/raw/epel8/f/sources") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertEqual(output_text, "foo\n bar\n baz") + + def test_edit_commit_passed_epel8(self): + project = pagure.lib.query._get_project(self.session, "hooktest") + + # Add user foo to project test + msg = pagure.lib.query.add_user_to_project( + self.session, + project=project, + new_user="foo", + user="pingou", + access="commit", + branches="epel*", + ) + self.session.commit() + + user = tests.FakeUser() + user.username = "foo" + with tests.user_set(self.app.application, user): + # Add some content to the git repo + tests.add_content_git_repo( + os.path.join(self.path, "repos", "hooktest.git") + ) + + data = { + "content": "foo\n bar\n baz", + "commit_title": "test commit", + "commit_message": "Online commits from the gure.lib.get", + "email": "foo@bar.com", + "branch": "epel8", + "csrf_token": self.get_csrf(), + } + + output = self.app.post( + "/hooktest/edit/master/f/sources", + data=data, + follow_redirects=True, + ) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + "Commits - hooktest - Pagure", output_text + ) + + # Check file after the commit: + # master did not change + output = self.app.get("/hooktest/raw/master/f/sources") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertEqual(output_text, "foo\n bar") + + # epel8 did change + output = self.app.get("/hooktest/raw/epel8/f/sources") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertEqual(output_text, "foo\n bar\n baz") + + def test_edit_contributor_passed_epel(self): + # Same test as above but the target branch change + project = pagure.lib.query._get_project(self.session, "hooktest") + + # Add user foo to project test + msg = pagure.lib.query.add_user_to_project( + self.session, + project=project, + new_user="foo", + user="pingou", + access="collaborator", + branches="epel*", + ) + self.session.commit() + + user = tests.FakeUser() + user.username = "foo" + with tests.user_set(self.app.application, user): + # Add some content to the git repo + tests.add_content_git_repo( + os.path.join(self.path, "repos", "hooktest.git") + ) + + data = { + "content": "foo\n bar\n baz", + "commit_title": "test commit", + "commit_message": "Online commits from the gure.lib.get", + "email": "foo@bar.com", + "branch": "epel", + "csrf_token": self.get_csrf(), + } + + output = self.app.post( + "/hooktest/edit/master/f/sources", + data=data, + follow_redirects=True, + ) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + "Commits - hooktest - Pagure", output_text + ) + + # Check file after the commit: + # master did not change + output = self.app.get("/hooktest/raw/master/f/sources") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertEqual(output_text, "foo\n bar") + + # epel did change + output = self.app.get("/hooktest/raw/epel/f/sources") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertEqual(output_text, "foo\n bar\n baz") + + def test_edit_contributor_passed_epel_no_regex(self): + # Same test as above but the allowed branch has no regex + project = pagure.lib.query._get_project(self.session, "hooktest") + + # Add user foo to project test + msg = pagure.lib.query.add_user_to_project( + self.session, + project=project, + new_user="foo", + user="pingou", + access="collaborator", + branches="epel", + ) + self.session.commit() + + user = tests.FakeUser() + user.username = "foo" + with tests.user_set(self.app.application, user): + # Add some content to the git repo + tests.add_content_git_repo( + os.path.join(self.path, "repos", "hooktest.git") + ) + + data = { + "content": "foo\n bar\n baz", + "commit_title": "test commit", + "commit_message": "Online commits from the gure.lib.get", + "email": "foo@bar.com", + "branch": "epel", + "csrf_token": self.get_csrf(), + } + + output = self.app.post( + "/hooktest/edit/master/f/sources", + data=data, + follow_redirects=True, + ) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + "Commits - hooktest - Pagure", output_text + ) + + # Check file after the commit: + # master did not change + output = self.app.get("/hooktest/raw/master/f/sources") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertEqual(output_text, "foo\n bar") + + # epel did change + output = self.app.get("/hooktest/raw/epel/f/sources") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertEqual(output_text, "foo\n bar\n baz") + + def test_edit_contributor_denied_epel8_no_regex(self): + # Same test as above but the allowed branch has no regex + project = pagure.lib.query._get_project(self.session, "hooktest") + + # Add user foo to project test + msg = pagure.lib.query.add_user_to_project( + self.session, + project=project, + new_user="foo", + user="pingou", + access="collaborator", + branches="epel", + ) + self.session.commit() + + user = tests.FakeUser() + user.username = "foo" + with tests.user_set(self.app.application, user): + # Add some content to the git repo + tests.add_content_git_repo( + os.path.join(self.path, "repos", "hooktest.git") + ) + + data = { + "content": "foo\n bar\n baz", + "commit_title": "test commit", + "commit_message": "Online commits from the gure.lib.get", + "email": "foo@bar.com", + "branch": "epel8", + "csrf_token": self.get_csrf(), + } + + output = self.app.post( + "/hooktest/edit/master/f/sources", + data=data, + follow_redirects=True, + ) + self.assertEqual(output.status_code, 403) + output_text = output.get_data(as_text=True) + self.assertIn( + "You are not allowed to edit files in this project", + output_text, + ) + + # Check file after the commit: + # epel not found + output = self.app.get("/hooktest/raw/epel8/f/sources") + self.assertEqual(output.status_code, 404)