From f48c7d65079531cc88c48b4ee33ba4d1f9ec063f Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Aug 18 2020 11:58:06 +0000
Subject: [PATCH 1/3] Introduce the collaborator_project_groups mapping


We need two mappings on a regular basis. One linking a project to its
group of collaborator and pointing to the PagureGroup objects directly.
This is used for example to retrieve the list of collaborator groups.
The one mapping is between a project and the corresponding ProjectGroup
for collaborators which is where the "branches" that the group is
restricted to is stored.

So we now have both mappings available and we need to be careful to use
the proper one, but pagure will quickly indicate (ie: crash) if one
uses the wrong one and tries to access either the group name or the
branches attribute.

Fixes https://pagure.io/fedora-infrastructure/issue/9247

Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/pagure/lib/model.py b/pagure/lib/model.py
index 325a545..4eea9f5 100644
--- a/pagure/lib/model.py
+++ b/pagure/lib/model.py
@@ -487,6 +487,16 @@ class Project(BASE):
     )
 
     collaborator_groups = relation(
+        "PagureGroup",
+        secondary="projects_groups",
+        primaryjoin="projects.c.id==projects_groups.c.project_id",
+        secondaryjoin="and_(pagure_group.c.id==projects_groups.c.group_id,\
+                projects_groups.c.access=='collaborator')",
+        order_by="PagureGroup.group_name.asc()",
+        viewonly=True,
+    )
+
+    collaborator_project_groups = relation(
         "ProjectGroup",
         primaryjoin="and_(projects.c.id==projects_groups.c.project_id,\
                     projects_groups.c.access=='collaborator')",
diff --git a/pagure/utils.py b/pagure/utils.py
index 24a0ce4..915ba49 100644
--- a/pagure/utils.py
+++ b/pagure/utils.py
@@ -311,7 +311,7 @@ def is_repo_collaborator(repo_obj, refname, username=None, session=None):
                     return True
 
     # If they are in a group that has commit access -> maybe
-    for project_group in repo_obj.collaborator_groups:
+    for project_group in repo_obj.collaborator_project_groups:
         if project_group.group.group_name in usergroups:
             # if branch is None when the user tries to read,
             # so we'll allow that
diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py
index 3c364be..51dd8e9 100644
--- a/tests/test_pagure_flask_api_project.py
+++ b/tests/test_pagure_flask_api_project.py
@@ -1010,6 +1010,27 @@ class PagureFlaskApiProjecttests(tests.Modeltests):
         )
         self.session.commit()
 
+        # Add a collaborator group
+        msg = pagure.lib.query.add_group(
+            self.session,
+            group_name="some_group",
+            display_name="Some Group",
+            description=None,
+            group_type="bar",
+            user="pingou",
+            is_admin=False,
+            blacklist=[],
+        )
+        pagure.lib.query.add_group_to_project(
+            session=self.session,
+            project=project,
+            new_group="some_group",
+            user="pingou",
+            access="collaborator",
+            branches="features/*",
+        )
+        self.session.commit()
+
         # Existing project
         output = self.app.get("/api/0/test")
         self.assertEqual(output.status_code, 200)
@@ -1019,7 +1040,7 @@ class PagureFlaskApiProjecttests(tests.Modeltests):
         expected_data = {
             "access_groups": {
                 "admin": [],
-                "collaborator": [],
+                "collaborator": ["some_group"],
                 "commit": [],
                 "ticket": [],
             },

From 1671c2d4a56d3ac88d92f6b904ebf3e3584914be Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Aug 18 2020 11:58:06 +0000
Subject: [PATCH 2/3] Small code style change


Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/pagure/lib/model.py b/pagure/lib/model.py
index 4eea9f5..0c935a6 100644
--- a/pagure/lib/model.py
+++ b/pagure/lib/model.py
@@ -1010,9 +1010,8 @@ class Project(BASE):
             elif access == "collaborator":
                 committers = set(self.committer_groups)
                 admins = set(self.admin_groups)
-                return list(
-                    set(self.collaborator_groups) - committers - admins
-                )
+                collaborators = set(self.collaborator_groups)
+                return list(collaborators - committers - admins)
             elif access == "ticket":
                 committers = set(self.committer_groups)
                 admins = set(self.admin_groups)

From 7f8ef43df94c0ec7bf50e1c7cfe34bca13084080 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Aug 18 2020 11:58:06 +0000
Subject: [PATCH 3/3] Adjust a test so the logic matches its description


Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py
index 51dd8e9..7179e9b 100644
--- a/tests/test_pagure_flask_api_project.py
+++ b/tests/test_pagure_flask_api_project.py
@@ -4315,7 +4315,7 @@ class PagureFlaskApiProjectWebhookTokenTests(tests.Modeltests):
             project,
             new_user="foo",
             user="pingou",
-            access="ticket",
+            access="collaborator",
         )
         self.session.commit()