launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging lp:~twom/launchpad/branch-permissions-for-gitapi into lp:launchpad with lp:~cjwatson/launchpad/git-permissions-model as a prerequisite.

Commit message:
Add GitRuleGrant api to xmlrpc API

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~twom/launchpad/branch-permissions-for-gitapi/+merge/355716
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~twom/launchpad/branch-permissions-for-gitapi into lp:launchpad.

=== added file 'database/schema/patch-2209-85-0.sql'
--- database/schema/patch-2209-85-0.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2209-85-0.sql	2018-09-26 15:45:21 +0000
@@ -0,0 +1,68 @@
+-- Copyright 2018 Canonical Ltd.  This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+SET client_min_messages=ERROR;
+
+CREATE TABLE GitRule (
+    id serial PRIMARY KEY,
+    repository integer NOT NULL REFERENCES gitrepository,
+    position integer NOT NULL,
+    ref_pattern text NOT NULL,
+    creator integer NOT NULL REFERENCES person,
+    date_created timestamp without time zone DEFAULT (CURRENT_TIMESTAMP AT TIME ZONE 'UTC') NOT NULL,
+    date_last_modified timestamp without time zone DEFAULT (CURRENT_TIMESTAMP AT TIME ZONE 'UTC') NOT NULL,
+    CONSTRAINT gitrule__repository__position__key UNIQUE (repository, position) DEFERRABLE INITIALLY DEFERRED,
+    CONSTRAINT gitrule__repository__ref_pattern__key UNIQUE (repository, ref_pattern),
+    -- Used by repository_matches_rule constraint on GitRuleGrant.
+    CONSTRAINT gitrule__repository__id__key UNIQUE (repository, id)
+);
+
+COMMENT ON TABLE GitRule IS 'An access rule for a Git repository.';
+COMMENT ON COLUMN GitRule.repository IS 'The repository that this rule is for.';
+COMMENT ON COLUMN GitRule.position IS 'The position of this rule in its repository''s rule order.';
+COMMENT ON COLUMN GitRule.ref_pattern IS 'The pattern of references matched by this rule.';
+COMMENT ON COLUMN GitRule.creator IS 'The user who created this rule.';
+COMMENT ON COLUMN GitRule.date_created IS 'The time when this rule was created.';
+COMMENT ON COLUMN GitRule.date_last_modified IS 'The time when this rule was last modified.';
+
+CREATE TABLE GitRuleGrant (
+    id serial PRIMARY KEY,
+    repository integer NOT NULL REFERENCES gitrepository,
+    rule integer NOT NULL REFERENCES gitrule,
+    grantee_type integer NOT NULL,
+    grantee integer REFERENCES person,
+    can_create boolean DEFAULT false NOT NULL,
+    can_push boolean DEFAULT false NOT NULL,
+    can_force_push boolean DEFAULT false NOT NULL,
+    grantor integer NOT NULL REFERENCES person,
+    date_created timestamp without time zone DEFAULT (CURRENT_TIMESTAMP AT TIME ZONE 'UTC') NOT NULL,
+    date_last_modified timestamp without time zone DEFAULT (CURRENT_TIMESTAMP AT TIME ZONE 'UTC') NOT NULL,
+    CONSTRAINT repository_matches_rule FOREIGN KEY (repository, rule) REFERENCES gitrule (repository, id),
+    -- 2 == PERSON
+    CONSTRAINT has_grantee CHECK ((grantee_type = 2) = (grantee IS NOT NULL))
+);
+
+CREATE INDEX gitrulegrant__repository__idx
+    ON GitRuleGrant(repository);
+CREATE UNIQUE INDEX gitrulegrant__rule__grantee_type__key
+    ON GitRuleGrant(rule, grantee_type)
+    -- 2 == PERSON
+    WHERE grantee_type != 2;
+CREATE UNIQUE INDEX gitrulegrant__rule__grantee_type__grantee_key
+    ON GitRuleGrant(rule, grantee_type, grantee)
+    -- 2 == PERSON
+    WHERE grantee_type = 2;
+
+COMMENT ON TABLE GitRuleGrant IS 'An access grant for a Git repository rule.';
+COMMENT ON COLUMN GitRuleGrant.repository IS 'The repository that this grant is for.';
+COMMENT ON COLUMN GitRuleGrant.rule IS 'The rule that this grant is for.';
+COMMENT ON COLUMN GitRuleGrant.grantee_type IS 'The type of entity being granted access.';
+COMMENT ON COLUMN GitRuleGrant.grantee IS 'The person or team being granted access.';
+COMMENT ON COLUMN GitRuleGrant.can_create IS 'Whether creating references is allowed.';
+COMMENT ON COLUMN GitRuleGrant.can_push IS 'Whether pushing references is allowed.';
+COMMENT ON COLUMN GitRuleGrant.can_force_push IS 'Whether force-pushing references is allowed.';
+COMMENT ON COLUMN GitRuleGrant.grantor IS 'The user who created this grant.';
+COMMENT ON COLUMN GitRuleGrant.date_created IS 'The time when this grant was created.';
+COMMENT ON COLUMN GitRuleGrant.date_last_modified IS 'The time when this grant was last modified.';
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2209, 85, 0);

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2018-09-26 15:45:20 +0000
+++ lib/lp/code/configure.zcml	2018-09-26 15:45:21 +0000
@@ -1004,6 +1004,9 @@
   <adapter factory="lp.code.model.defaultgit.OwnerProjectDefaultGitRepository" />
   <adapter factory="lp.code.model.defaultgit.OwnerPackageDefaultGitRepository" />
 
+  <class class="lp.code.model.gitlookup.GitRuleGrantLookup">
+    <allow interface="lp.code.interfaces.gitlookup.IGitRuleGrantLookup" />
+  </class>
   <class class="lp.code.model.gitlookup.GitLookup">
     <allow interface="lp.code.interfaces.gitlookup.IGitLookup" />
   </class>
@@ -1013,6 +1016,11 @@
     <allow interface="lp.code.interfaces.gitlookup.IGitLookup" />
   </securedutility>
   <securedutility
+      class="lp.code.model.gitlookup.GitRuleGrantLookup"
+      provides="lp.code.interfaces.gitlookup.IGitRuleGrantLookup">
+    <allow interface="lp.code.interfaces.gitlookup.IGitRuleGrantLookup" />
+  </securedutility>
+  <securedutility
       class="lp.code.model.gitlookup.GitTraverser"
       provides="lp.code.interfaces.gitlookup.IGitTraverser">
     <allow interface="lp.code.interfaces.gitlookup.IGitTraverser" />

=== modified file 'lib/lp/code/interfaces/gitapi.py'
--- lib/lp/code/interfaces/gitapi.py	2015-03-31 04:18:22 +0000
+++ lib/lp/code/interfaces/gitapi.py	2018-09-26 15:45:21 +0000
@@ -67,3 +67,9 @@
         :returns: An `Unauthorized` fault, as password authentication is
             not yet supported.
         """
+
+    def listRefRules(self, repository, user):
+        """Return the list of RefRules for `user` in `repository`
+
+        :returns: A List of rules for the user in the specified repository
+        """

=== modified file 'lib/lp/code/interfaces/gitlookup.py'
--- lib/lp/code/interfaces/gitlookup.py	2015-03-30 14:47:22 +0000
+++ lib/lp/code/interfaces/gitlookup.py	2018-09-26 15:45:21 +0000
@@ -6,6 +6,7 @@
 __metaclass__ = type
 __all__ = [
     'IGitLookup',
+    'IGitRuleGrantLookup',
     'IGitTraversable',
     'IGitTraverser',
     ]
@@ -145,3 +146,14 @@
             leading part of a path as a repository, such as external code
             browsers.
         """
+
+
+class IGitRuleGrantLookup(Interface):
+    """Utility for looking up a GitRuleGrant by properties"""
+
+    def getByRulesAffectingPerson(repository, grantee_id):
+        """Find all the rules for a repository that affect a Person.
+
+        :param repository: An instance of a GitRepository
+        :param granteed_id: An integer of the id of the Person
+        """

=== modified file 'lib/lp/code/model/gitlookup.py'
--- lib/lp/code/model/gitlookup.py	2018-07-23 10:28:33 +0000
+++ lib/lp/code/model/gitlookup.py	2018-09-26 15:45:21 +0000
@@ -27,6 +27,7 @@
     )
 from lp.code.interfaces.gitlookup import (
     IGitLookup,
+    IGitRuleGrantLookup,
     IGitTraversable,
     IGitTraverser,
     )
@@ -34,6 +35,7 @@
 from lp.code.interfaces.gitrepository import IGitRepositorySet
 from lp.code.interfaces.hasgitrepositories import IHasGitRepositories
 from lp.code.model.gitrepository import GitRepository
+from lp.code.model.gitrule import GitRuleGrant
 from lp.registry.errors import NoSuchSourcePackageName
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.distributionsourcepackage import (
@@ -372,3 +374,14 @@
         if trailing:
             trailing_segments.insert(0, trailing)
         return repository, "/".join(trailing_segments)
+
+
+@implementer(IGitRuleGrantLookup)
+class GitRuleGrantLookup:
+
+    def getByRulesAffectingPerson(self, repository, grantee):
+        grants = IStore(GitRuleGrant).find(
+            GitRuleGrant,
+            GitRuleGrant.repository == repository)
+        grants = [grant for grant in grants if grantee.inTeam(grant.grantee)]
+        return grants

=== modified file 'lib/lp/code/xmlrpc/git.py'
--- lib/lp/code/xmlrpc/git.py	2018-08-28 13:58:37 +0000
+++ lib/lp/code/xmlrpc/git.py	2018-09-26 15:45:21 +0000
@@ -40,6 +40,7 @@
 from lp.code.interfaces.gitjob import IGitRefScanJobSource
 from lp.code.interfaces.gitlookup import (
     IGitLookup,
+    IGitRuleGrantLookup,
     IGitTraverser,
     )
 from lp.code.interfaces.gitnamespace import (
@@ -325,3 +326,45 @@
         else:
             # Only macaroons are supported for password authentication.
             return faults.Unauthorized()
+
+    def _isRepositoryOwner(self, requester, repository):
+        try:
+            return requester.inTeam(repository.owner)
+        except Unauthorized:
+            return False
+
+    def _listRefRules(self, requester, translated_path):
+        repository = getUtility(IGitLookup).getByHostingPath(translated_path)
+        grants = getUtility(IGitRuleGrantLookup).getByRulesAffectingPerson(
+            repository, requester)
+
+        lines = []
+        for grant in grants:
+            permissions = []
+            if grant.can_create:
+                permissions.append("create")
+            if grant.can_push:
+                permissions.append("push")
+            if grant.can_force_push:
+                permissions.append("force-push")
+            lines.append(
+                {'ref_pattern': grant.rule.ref_pattern,
+                'permissions': permissions})
+
+        if self._isRepositoryOwner(requester, repository):
+            lines.append({
+                'ref_pattern': '*',
+                'permissions': ['create', 'push', 'force-push']})
+        return lines
+
+    def listRefRules(self, translated_path, auth_params):
+        """See `IGitAPI`"""
+        requester_id = auth_params.get("uid")
+        if requester_id is None:
+            requester_id = LAUNCHPAD_ANONYMOUS
+
+        return run_with_login(
+            requester_id,
+            self._listRefRules,
+            translated_path,
+        )

=== modified file 'lib/lp/code/xmlrpc/tests/test_git.py'
--- lib/lp/code/xmlrpc/tests/test_git.py	2018-08-28 14:07:38 +0000
+++ lib/lp/code/xmlrpc/tests/test_git.py	2018-09-26 15:45:21 +0000
@@ -260,6 +260,125 @@
         self.assertEqual(
             initial_count, getUtility(IAllGitRepositories).count())
 
+    def test_listRefRules(self):
+        # Test that GitGrantRule (ref rule) can be retrieved for a user
+        requester = self.factory.makePerson()
+        repository = removeSecurityProxy(
+            self.factory.makeGitRepository(
+                owner=requester, information_type=InformationType.USERDATA))
+
+        rule = self.factory.makeGitRule(repository)
+        self.factory.makeGitRuleGrant(
+            rule=rule, grantee=requester, can_push=True, can_create=True)
+
+        results = self.git_api.listRefRules(
+            repository.getInternalPath(),
+            {'uid': requester.id})
+        self.assertEqual(len(results), 2)
+        self.assertEqual(results[0]['ref_pattern'], 'refs/heads/*')
+        self.assertEqual(results[0]['permissions'], ['create', 'push'])
+
+    def test_listRefRules_no_grants(self):
+        # User that has no grants and is not the owner
+        requester = self.factory.makePerson()
+        owner = self.factory.makePerson()
+        repository = removeSecurityProxy(
+            self.factory.makeGitRepository(
+                owner=owner, information_type=InformationType.USERDATA))
+
+        rule = self.factory.makeGitRule(repository)
+        self.factory.makeGitRuleGrant(
+            rule=rule, grantee=owner, can_push=True, can_create=True)
+
+        results = self.git_api.listRefRules(
+            repository.getInternalPath(),
+            {'uid': requester.id})
+        self.assertEqual(len(results), 0)
+
+    def test_listRefRules_owner_has_default(self):
+        owner = self.factory.makePerson()
+        repository = removeSecurityProxy(
+            self.factory.makeGitRepository(
+                owner=owner, information_type=InformationType.USERDATA))
+
+        rule = self.factory.makeGitRule(
+            repository=repository, ref_pattern=u'refs/heads/master')
+        self.factory.makeGitRuleGrant(
+            rule=rule, grantee=owner, can_push=True, can_create=True)
+
+        results = self.git_api.listRefRules(
+            repository.getInternalPath(),
+            {'uid': owner.id})
+        self.assertEqual(len(results), 2)
+        # Default grant should be last in pattern
+        self.assertEqual(results[-1]['ref_pattern'], '*')
+
+    def test_listRefRules_owner_is_team(self):
+        member = self.factory.makePerson()
+        owner = self.factory.makeTeam(members=[member])
+        repository = removeSecurityProxy(
+            self.factory.makeGitRepository(
+                owner=owner, information_type=InformationType.USERDATA))
+
+        results = self.git_api.listRefRules(
+            repository.getInternalPath(),
+            {'uid': member.id})
+
+        # Should have default grant as member of owning team
+        self.assertEqual(len(results), 1)
+        self.assertEqual(results[-1]['ref_pattern'], '*')
+
+    def test_listRefRules_owner_is_team_with_grants(self):
+        member = self.factory.makePerson()
+        owner = self.factory.makeTeam(members=[member])
+        repository = removeSecurityProxy(
+            self.factory.makeGitRepository(
+                owner=owner, information_type=InformationType.USERDATA))
+
+        rule = self.factory.makeGitRule(
+            repository=repository, ref_pattern=u'refs/heads/master')
+        self.factory.makeGitRuleGrant(
+            rule=rule, grantee=owner, can_push=True, can_create=True)
+
+        results = self.git_api.listRefRules(
+            repository.getInternalPath(),
+            {'uid': member.id})
+
+        # Should have default grant as member of owning team
+        self.assertEqual(len(results), 2)
+        self.assertEqual(results[-1]['ref_pattern'], '*')
+
+    def test_listRefRules_owner_is_team_with_grants_to_person(self):
+        member = self.factory.makePerson()
+        other_member = self.factory.makePerson()
+        owner = self.factory.makeTeam(members=[member, other_member])
+        repository = removeSecurityProxy(
+            self.factory.makeGitRepository(
+                owner=owner, information_type=InformationType.USERDATA))
+
+        rule = self.factory.makeGitRule(
+            repository=repository, ref_pattern=u'refs/heads/master')
+        self.factory.makeGitRuleGrant(
+            rule=rule, grantee=owner, can_push=True, can_create=True)
+
+        rule = self.factory.makeGitRule(
+            repository=repository, ref_pattern=u'refs/heads/tags')
+        self.factory.makeGitRuleGrant(
+            rule=rule, grantee=member, can_create=True)
+
+        # This should not appear
+        self.factory.makeGitRuleGrant(
+            rule=rule, grantee=other_member, can_push=True)
+
+        results = self.git_api.listRefRules(
+            repository.getInternalPath(),
+            {'uid': member.id})
+
+        # Should have default grant as member of owning team
+        self.assertEqual(len(results), 3)
+        self.assertEqual(results[-1]['ref_pattern'], '*')
+        tags_rule = results[1]
+        self.assertEqual(tags_rule['permissions'], ['create'])
 
 class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
     """Tests for the implementation of `IGitAPI`."""