From bef2320f64aecbbdfd021f3bbef4e074f81112d8 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Aug 03 2020 12:52:42 +0000 Subject: [PATCH 1/4] Increase logging to the pagure_auth logger This way we have all the information in one place instead of spread over two. Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/internal/__init__.py b/pagure/internal/__init__.py index 9a65e1e..25b6d87 100644 --- a/pagure/internal/__init__.py +++ b/pagure/internal/__init__.py @@ -137,6 +137,9 @@ def check_ssh_access(): # Build a fake path so we can use get_repo_info_from_path path = os.path.join(pagure_config["GIT_FOLDER"], gitdir) + _auth_log.info( + "%s asks to access %s (path: %s) via ssh" % (remoteuser, gitdir, path) + ) _log.info( "%s asks to access %s (path: %s) via ssh" % (remoteuser, gitdir, path) ) @@ -146,6 +149,11 @@ def check_ssh_access(): namespace, repo, ) = pagure.lib.git.get_repo_info_from_path(path, hide_notfound=True) + + _auth_log.info( + "%s asks to access the %s repo of %s/%s from user %s" + % (remoteuser, repotype, namespace, repo, project_user) + ) _log.info( "%s asks to access the %s repo of %s/%s from user %s" % (remoteuser, repotype, namespace, repo, project_user) @@ -177,6 +185,8 @@ def check_ssh_access(): ) _log.info("Project not found with this path") return flask.jsonify({"access": False}) + + _auth_log.info("Checking ACLs on project: %s" % project.fullname) _log.info("Checking ACLs on project: %s" % project.fullname) if repotype not in ["main", "docs"] and not pagure.utils.is_repo_user( @@ -186,12 +196,15 @@ def check_ssh_access(): # allowed for main and docs repos. _log.info("%s is not a contributor to this project" % remoteuser) _auth_log.info( - "User tried to access a projec they do not have access to -- " + "User tried to access a project they do not have access to -- " "|user: %s|IP: %s|method: N/A|repo: %s|query: N/A" % (remoteuser, flask.request.remote_addr, gitdir) ) return flask.jsonify({"access": False}) + _auth_log.info( + "Read access granted to %s on: %s" % (remoteuser, project.fullname) + ) _log.info( "Read access granted to %s on: %s" % (remoteuser, project.fullname) ) From 1db4d74e74168b1a3c9f0ce7d09cc61e7348bb56 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Aug 03 2020 12:52:42 +0000 Subject: [PATCH 2/4] If the target is a link and does not exists, remove it before proceeding This allows to quickly fix all the git hooks on a pagure instance that was, for example, migrated from one python version to another and where thus all the symlinks are pointing to the old python version and thus dangling. Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/hooks/__init__.py b/pagure/hooks/__init__.py index 0e12ad5..a848afa 100644 --- a/pagure/hooks/__init__.py +++ b/pagure/hooks/__init__.py @@ -180,6 +180,8 @@ class BaseHook(object): # Install the main hook file target = os.path.join(hookfolder, hooktype) if not os.path.exists(target): + if os.path.islink(target): + os.unlink(target) os.symlink(os.path.join(hook_files, "hookrunner"), target) @classmethod From eabcfc48047a4704a7e09ac173caf43641099461 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Aug 03 2020 12:52:42 +0000 Subject: [PATCH 3/4] Fix sorting collaborators and groups of collaborators Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 9c23143..e1f32f2 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -1024,7 +1024,8 @@ class Project(BASE): key=lambda u: u.user, ), "collaborator": sorted( - self.get_project_users(access="collaborator", combine=False) + self.get_project_users(access="collaborator", combine=False), + key=lambda u: u.user, ), "ticket": sorted( self.get_project_users(access="ticket", combine=False), @@ -1066,7 +1067,8 @@ class Project(BASE): key=lambda x: x.group_name, ), "collaborator": sorted( - self.get_project_groups(access="collaborator", combine=False) + self.get_project_groups(access="collaborator", combine=False), + key=lambda x: x.group_name, ), "ticket": sorted( self.get_project_groups(access="ticket", combine=False), diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index 870aaa0..3c364be 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -984,6 +984,79 @@ class PagureFlaskApiProjecttests(tests.Modeltests): } self.assertDictEqual(data, expected_data) + def test_api_project_collaborators(self): + """ Test the api_project method of the flask api. """ + tests.create_projects(self.session) + tests.create_user(self.session, "ralph", "Ralph B", ["ralph@b.org"]) + tests.create_user(self.session, "nils", "Nils P", ["nils@p.net"]) + + # Add a couple of collaborators + project = pagure.lib.query._get_project(self.session, "test") + pagure.lib.query.add_user_to_project( + self.session, + project, + new_user="ralph", + user="pingou", + access="collaborator", + branches="f*,epel*", + ) + pagure.lib.query.add_user_to_project( + self.session, + project, + new_user="nils", + user="pingou", + access="collaborator", + branches="epel*", + ) + self.session.commit() + + # Existing project + output = self.app.get("/api/0/test") + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + data["date_created"] = "1436527638" + data["date_modified"] = "1436527638" + expected_data = { + "access_groups": { + "admin": [], + "collaborator": [], + "commit": [], + "ticket": [], + }, + "access_users": { + "admin": [], + "collaborator": ["nils", "ralph"], + "commit": [], + "owner": ["pingou"], + "ticket": [], + }, + "close_status": [ + "Invalid", + "Insufficient data", + "Fixed", + "Duplicate", + ], + "custom_keys": [], + "date_created": "1436527638", + "date_modified": "1436527638", + "description": "test project #1", + "fullname": "test", + "url_path": "test", + "id": 1, + "milestones": {}, + "name": "test", + "namespace": None, + "parent": None, + "priorities": {}, + "tags": [], + "user": { + "fullname": "PY C", + "name": "pingou", + "url_path": "user/pingou", + }, + } + self.assertDictEqual(data, expected_data) + def test_api_project_group(self): """ Test the api_project method of the flask api. """ tests.create_projects(self.session) From 0d6901fe1a72b684e048cc57a19ef2ef7dc436ac Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Aug 03 2020 12:52:42 +0000 Subject: [PATCH 4/4] import pagure.lib.query before using it, in the code For a reason I do not quite understand, without this import statement (that is already present earlier in the code), the code crash with the error: File "/usr/lib/python3.6/site-packages/pagure/ui/clone.py", line 71, in _get_remote_user token = pagure.lib.query.get_api_token( UnboundLocalError: local variable 'pagure' referenced before assignment This solves the error, but I really don't see why it occurs in the first place. Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/ui/clone.py b/pagure/ui/clone.py index 7a2c85b..dd769b5 100644 --- a/pagure/ui/clone.py +++ b/pagure/ui/clone.py @@ -68,6 +68,8 @@ def _get_remote_user(project): # approach above didn't work, the user may still be # using an API token, so we want to check that as well. if not remote_user: + import pagure.lib.query + token = pagure.lib.query.get_api_token( flask.g.session, token_str )