launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-grant-limitedview into lp:launchpad with lp:~cjwatson/launchpad/git-permissions-notifications as a prerequisite.

Commit message:
Grant LimitedView on teams to people who own Git repositories with grants for them.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-grant-limitedview/+merge/355607

This means that we don't get stuck in situations where somebody can't modify the grants on a repository they own (e.g. using the proposed first-pass webservice interface where they need to supply a full replacement permission structure); and in general we want people to be able to see who can write to repositories they own, even if it was configured by another member of the owning team.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-grant-limitedview into lp:launchpad.

=== modified file 'lib/lp/code/interfaces/gitcollection.py'
--- lib/lp/code/interfaces/gitcollection.py	2016-05-05 07:22:32 +0000
+++ lib/lp/code/interfaces/gitcollection.py	2018-09-25 15:43:49 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """A collection of Git repositories.
@@ -116,6 +116,12 @@
             states are returned.
         """
 
+    def getGrantsForGrantee(grantee):
+        """Return a result set of access grants to the given grantee.
+
+        :param grantee: An `IPerson`.
+        """
+
     def getTeamsWithRepositories(person):
         """Return the teams that person is a member of that have
         repositories."""

=== modified file 'lib/lp/code/model/gitcollection.py'
--- lib/lp/code/model/gitcollection.py	2016-10-12 23:24:24 +0000
+++ lib/lp/code/model/gitcollection.py	2018-09-25 15:43:49 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Implementations of `IGitCollection`."""
@@ -49,6 +49,7 @@
     get_git_repository_privacy_filter,
     GitRepository,
     )
+from lp.code.model.gitrule import GitRuleGrant
 from lp.code.model.gitsubscription import GitSubscription
 from lp.registry.enums import EXCLUSIVE_TEAM_POLICY
 from lp.registry.model.distribution import Distribution
@@ -408,6 +409,19 @@
         proposals.order_by(Desc(CodeReviewComment.vote))
         return proposals
 
+    def getGrantsForGrantee(self, grantee):
+        """See `IGitCollection`."""
+        expressions = [
+            GitRuleGrant.grantee == grantee,
+            GitRuleGrant.repository_id.is_in(self._getRepositorySelect()),
+            ]
+        visibility = self._getRepositoryVisibilityExpression()
+        if visibility:
+            expressions.append(
+                GitRuleGrant.repository_id.is_in(
+                    Select(GitRepository.id, visibility)))
+        return self.store.find(GitRuleGrant, *expressions)
+
     def getTeamsWithRepositories(self, person):
         """See `IGitCollection`."""
         # This method doesn't entirely fit with the intent of the

=== modified file 'lib/lp/registry/tests/test_private_team_visibility.py'
--- lib/lp/registry/tests/test_private_team_visibility.py	2016-01-26 15:47:37 +0000
+++ lib/lp/registry/tests/test_private_team_visibility.py	2018-09-25 15:43:49 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for visibility of private teams.
@@ -275,6 +275,33 @@
     def test_teams_with_private_branch_review_requests(self, private=True):
         self._test_teams_with_branch_review_requests()
 
+    def _test_git_repository_grantee(self, private):
+        # Users who own Git repositories can see private teams that have
+        # been granted some kind of write access to those repositories.
+        some_person = self.factory.makePerson()
+        self._check_permission(some_person, False)
+        login_person(some_person)
+        project = self.factory.makeProduct()
+        if private:
+            information_type = InformationType.USERDATA
+        else:
+            information_type = InformationType.PUBLIC
+        repository = self.factory.makeGitRepository(
+            owner=some_person, target=project,
+            information_type=information_type)
+        self.factory.makeGitRuleGrant(
+            repository=repository, grantee=self.priv_team)
+        # The team is now visible to the repository owner.
+        self._check_permission(some_person, True)
+        # The team is still invisible to other users.
+        self._check_permission(self.factory.makePerson(), False)
+
+    def test_public_git_repository_grantee(self):
+        self._test_git_repository_grantee(private=False)
+
+    def test_private_git_repository_grantee(self):
+        self._test_git_repository_grantee(private=True)
+
     def test_private_ppa_subscriber(self):
         # Subscribers to the team's private PPA have limited view permission.
         login_person(self.priv_owner)

=== modified file 'lib/lp/security.py'
--- lib/lp/security.py	2018-09-25 15:43:49 +0000
+++ lib/lp/security.py	2018-09-25 15:43:49 +0000
@@ -1054,6 +1054,13 @@
             if not team_repositories.visibleByUser(user.person).is_empty():
                 return True
 
+            # Grant visibility to people who own Git repositories that grant
+            # some kind of write access to the private team.
+            owned_repositories = IGitCollection(user.person)
+            grants = owned_repositories.getGrantsForGrantee(self.obj)
+            if not grants.is_empty():
+                return True
+
             # Grant visibility to users in a team that has the private team as
             # a member, so that they can see the team properly in member
             # listings.