Merge lp:~cjwatson/launchpad/git-permissions-model into lp:launchpad

[Colin Watson]

Add basic GitRule and GitGrant models.

See https://docs.google.com/document/d/1JW_D_Tgo4X2-vPMZtShSbi3cm1iOsGcNIzeOpa5E_wA for the design and https://code.launchpad.net/~cjwatson/launchpad/db-git-permissions/+merge/354200 for schema changes.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'database/schema/security.cfg'
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2018-05-16 17:33:18 +0000
+++ lib/lp/code/configure.zcml	2018-10-05 14:49:26 +0000
@@ -919,6 +919,35 @@
     <allow interface="lp.code.interfaces.gitref.IGitRefRemoteSet" />
   </securedutility>
 
+  <!-- Git repository access rules -->
+
+  <class class="lp.code.model.gitrule.GitRule">
+    <require
+        permission="launchpad.View"
+        interface="lp.code.interfaces.gitrule.IGitRuleView
+                   lp.code.interfaces.gitrule.IGitRuleEditableAttributes" />
+    <require
+        permission="launchpad.Edit"
+        interface="lp.code.interfaces.gitrule.IGitRuleEdit"
+        set_schema="lp.code.interfaces.gitrule.IGitRuleEditableAttributes" />
+  </class>
+  <subscriber
+      for="lp.code.interfaces.gitrule.IGitRule zope.lifecycleevent.interfaces.IObjectModifiedEvent"
+      handler="lp.code.model.gitrule.git_rule_modified"/>
+  <class class="lp.code.model.gitrule.GitRuleGrant">
+    <require
+        permission="launchpad.View"
+        interface="lp.code.interfaces.gitrule.IGitRuleGrantView
+                   lp.code.interfaces.gitrule.IGitRuleGrantEditableAttributes" />
+    <require
+        permission="launchpad.Edit"
+        interface="lp.code.interfaces.gitrule.IGitRuleGrantEdit"
+        set_schema="lp.code.interfaces.gitrule.IGitRuleGrantEditableAttributes" />
+  </class>
+  <subscriber
+      for="lp.code.interfaces.gitrule.IGitRuleGrant zope.lifecycleevent.interfaces.IObjectModifiedEvent"
+      handler="lp.code.model.gitrule.git_rule_grant_modified"/>
+
   <!-- GitCollection -->
 
   <class class="lp.code.model.gitcollection.GenericGitCollection">

=== modified file 'lib/lp/code/enums.py'
--- lib/lp/code/enums.py	2017-06-15 01:02:11 +0000
+++ lib/lp/code/enums.py	2018-10-05 14:49:26 +0000
@@ -21,6 +21,7 @@
     'CodeImportReviewStatus',
     'CodeReviewNotificationLevel',
     'CodeReviewVote',
+    'GitGranteeType',
     'GitObjectType',
     'GitRepositoryType',
     'NON_CVS_RCS_TYPES',
@@ -175,6 +176,26 @@
         """)
 
 
+class GitGranteeType(DBEnumeratedType):
+    """Git Grantee Type
+
+    Access grants for Git repositories can be made to various kinds of
+    grantees.
+    """
+
+    REPOSITORY_OWNER = DBItem(1, """
+        Repository owner
+
+        A grant to the owner of the associated repository.
+        """)
+
+    PERSON = DBItem(2, """
+        Person
+
+        A grant to a particular person or team.
+        """)
+
+
 class BranchLifecycleStatusFilter(EnumeratedType):
     """Branch Lifecycle Status Filter
 

=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py	2018-08-23 17:03:05 +0000
+++ lib/lp/code/interfaces/gitrepository.py	2018-10-05 14:49:26 +0000
@@ -266,6 +266,10 @@
         # Really ICodeImport, patched in _schema_circular_imports.py.
         schema=Interface))
 
+    rules = Attribute("The access rules for this repository.")
+
+    grants = Attribute("The access grants for this repository.")
+
     @operation_parameters(
         path=TextLine(title=_("A string to look up as a path.")))
     # Really IGitRef, patched in _schema_circular_imports.py.
@@ -715,6 +719,26 @@
         This may be helpful in cases where a previous scan crashed.
         """
 
+    def addRule(ref_pattern, creator, position=None):
+        """Add an access rule to this repository.
+
+        :param ref_pattern: The reference pattern that the new rule should
+            match.
+        :param creator: The `IPerson` who is adding the rule.
+        :param position: The list position at which to insert the rule, or
+            None to append it.
+        """
+
+    def moveRule(rule, position):
+        """Move a rule to a new position in its repository's rule order.
+
+        :param rule: The `IGitRule` to move.
+        :param position: The new position.  For example, 0 puts the rule at
+            the start, while `len(repository.rules)` puts the rule at the
+            end.  If the new position is before the end of the list, then
+            other rules are shifted to later positions to make room.
+        """
+
     @export_read_operation()
     @operation_for_version("devel")
     def canBeDeleted():

=== added file 'lib/lp/code/interfaces/gitrule.py'
--- lib/lp/code/interfaces/gitrule.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/interfaces/gitrule.py	2018-10-05 14:49:26 +0000
@@ -0,0 +1,178 @@
+# Copyright 2018 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Git repository access rules."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'IGitRule',
+    'IGitRuleGrant',
+    ]
+
+from lazr.restful.fields import Reference
+from zope.interface import (
+    Attribute,
+    Interface,
+    )
+from zope.schema import (
+    Bool,
+    Choice,
+    Datetime,
+    Int,
+    TextLine,
+    )
+
+from lp import _
+from lp.code.enums import GitGranteeType
+from lp.code.interfaces.gitrepository import IGitRepository
+from lp.services.fields import (
+    PersonChoice,
+    PublicPersonChoice,
+    )
+
+
+class IGitRuleView(Interface):
+    """`IGitRule` attributes that require launchpad.View."""
+
+    id = Int(title=_("ID"), readonly=True, required=True)
+
+    repository = Reference(
+        title=_("Repository"), required=True, readonly=True,
+        schema=IGitRepository,
+        description=_("The repository that this rule is for."))
+
+    position = Int(
+        title=_("Position"), required=True, readonly=True,
+        description=_(
+            "The position of this rule in its repository's rule order."))
+
+    creator = PublicPersonChoice(
+        title=_("Creator"), required=True, readonly=True,
+        vocabulary="ValidPerson",
+        description=_("The user who created this rule."))
+
+    date_created = Datetime(
+        title=_("Date created"), required=True, readonly=True,
+        description=_("The time when this rule was created."))
+
+    date_last_modified = Datetime(
+        title=_("Date last modified"), required=True, readonly=True,
+        description=_("The time when this rule was last modified."))
+
+    is_exact = Bool(
+        title=_("Is this an exact-match rule?"), required=True, readonly=True,
+        description=_(
+            "True if this rule is for an exact reference name, or False if "
+            "it is for a wildcard."))
+
+    grants = Attribute("The access grants for this rule.")
+
+
+class IGitRuleEditableAttributes(Interface):
+    """`IGitRule` attributes that can be edited.
+
+    These attributes need launchpad.View to see, and launchpad.Edit to change.
+    """
+
+    ref_pattern = TextLine(
+        title=_("Pattern"), required=True, readonly=False,
+        description=_("The pattern of references matched by this rule."))
+
+
+class IGitRuleEdit(Interface):
+    """`IGitRule` attributes that require launchpad.Edit."""
+
+    def addGrant(grantee, grantor, can_create=False, can_push=False,
+                 can_force_push=False):
+        """Add an access grant to this rule.
+
+        :param grantee: The `IPerson` who is being granted permission, or an
+            item of `GitGranteeType` other than `GitGranteeType.PERSON` to
+            grant permission to some other kind of entity.
+        :param grantor: The `IPerson` who is granting permission.
+        :param can_create: Whether the grantee can create references
+            matching this rule.
+        :param can_push: Whether the grantee can push references matching
+            this rule.
+        :param can_force_push: Whether the grantee can force-push references
+            matching this rule.
+        """
+
+    def destroySelf():
+        """Delete this rule."""
+
+
+class IGitRule(IGitRuleView, IGitRuleEditableAttributes, IGitRuleEdit):
+    """An access rule for a Git repository."""
+
+
+class IGitRuleGrantView(Interface):
+    """`IGitRuleGrant` attributes that require launchpad.View."""
+
+    id = Int(title=_("ID"), readonly=True, required=True)
+
+    repository = Reference(
+        title=_("Repository"), required=True, readonly=True,
+        schema=IGitRepository,
+        description=_("The repository that this grant is for."))
+
+    rule = Reference(
+        title=_("Rule"), required=True, readonly=True,
+        schema=IGitRule,
+        description=_("The rule that this grant is for."))
+
+    grantor = PublicPersonChoice(
+        title=_("Grantor"), required=True, readonly=True,
+        vocabulary="ValidPerson",
+        description=_("The user who created this grant."))
+
+    grantee_type = Choice(
+        title=_("Grantee type"), required=True, readonly=True,
+        vocabulary=GitGranteeType,
+        description=_("The type of grantee for this grant."))
+
+    grantee = PersonChoice(
+        title=_("Grantee"), required=False, readonly=True,
+        vocabulary="ValidPersonOrTeam",
+        description=_("The person being granted access."))
+
+    date_created = Datetime(
+        title=_("Date created"), required=True, readonly=True,
+        description=_("The time when this grant was created."))
+
+    date_last_modified = Datetime(
+        title=_("Date last modified"), required=True, readonly=True,
+        description=_("The time when this grant was last modified."))
+
+
+class IGitRuleGrantEditableAttributes(Interface):
+    """`IGitRuleGrant` attributes that can be edited.
+
+    These attributes need launchpad.View to see, and launchpad.Edit to change.
+    """
+
+    can_create = Bool(
+        title=_("Can create"), required=True, readonly=False,
+        description=_("Whether creating references is allowed."))
+
+    can_push = Bool(
+        title=_("Can push"), required=True, readonly=False,
+        description=_("Whether pushing references is allowed."))
+
+    can_force_push = Bool(
+        title=_("Can force-push"), required=True, readonly=False,
+        description=_("Whether force-pushing references is allowed."))
+
+
+class IGitRuleGrantEdit(Interface):
+    """`IGitRuleGrant` attributes that require launchpad.Edit."""
+
+    def destroySelf():
+        """Delete this access grant."""
+
+
+class IGitRuleGrant(IGitRuleGrantView, IGitRuleGrantEditableAttributes,
+                    IGitRuleGrantEdit):
+    """An access grant for a Git repository rule."""

=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py	2018-09-06 14:25:46 +0000
+++ lib/lp/code/model/gitrepository.py	2018-10-05 14:49:26 +0000
@@ -112,6 +112,10 @@
     GitRef,
     GitRefDefault,
     )
+from lp.code.model.gitrule import (
+    GitRule,
+    GitRuleGrant,
+    )
 from lp.code.model.gitsubscription import GitSubscription
 from lp.registry.enums import PersonVisibility
 from lp.registry.errors import CannotChangeInformationType
@@ -1121,6 +1125,67 @@
     def code_import(self):
         return getUtility(ICodeImportSet).getByGitRepository(self)
 
+    @property
+    def rules(self):
+        """See `IGitRepository`."""
+        return Store.of(self).find(
+            GitRule, GitRule.repository == self).order_by(GitRule.position)
+
+    def _syncRulePositions(self, rules):
+        """Synchronise rule positions with their order in a provided list.
+
+        :param rules: A sequence of `IGitRule`s in the desired order.
+        """
+        # Canonicalise rule ordering: exact-match rules come first in
+        # lexicographical order, followed by wildcard rules in the requested
+        # order.  (Note that `sorted` is guaranteed to be stable.)
+        rules = sorted(
+            rules,
+            key=lambda rule: (0, rule.ref_pattern) if rule.is_exact else (1,))
+        # Ensure the correct position of all rules, which may involve more
+        # work than necessary, but is simple and tends to be
+        # self-correcting.  This works because the unique constraint on
+        # GitRule(repository, position) is deferred.
+        for position, rule in enumerate(rules):
+            if rule.repository != self:
+                raise AssertionError("%r does not belong to %r" % (rule, self))
+            if rule.position != position:
+                removeSecurityProxy(rule).position = position
+
+    def addRule(self, ref_pattern, creator, position=None):
+        """See `IGitRepository`."""
+        rules = list(self.rules)
+        rule = GitRule(
+            repository=self,
+            # -1 isn't a valid position, but _syncRulePositions will correct
+            # it in a moment.
+            position=position if position is not None else -1,
+            ref_pattern=ref_pattern, creator=creator, date_created=DEFAULT)
+        if position is None:
+            rules.append(rule)
+        else:
+            rules.insert(position, rule)
+        self._syncRulePositions(rules)
+        return rule
+
+    def moveRule(self, rule, position):
+        """See `IGitRepository`."""
+        if rule.repository != self:
+            raise ValueError("%r does not belong to %r" % (rule, self))
+        if position < 0:
+            raise ValueError("Negative positions are not supported")
+        if position != rule.position:
+            rules = list(self.rules)
+            rules.remove(rule)
+            rules.insert(position, rule)
+            self._syncRulePositions(rules)
+
+    @property
+    def grants(self):
+        """See `IGitRepository`."""
+        return Store.of(self).find(
+            GitRuleGrant, GitRuleGrant.repository_id == self.id)
+
     def canBeDeleted(self):
         """See `IGitRepository`."""
         # Can't delete if the repository is associated with anything.
@@ -1252,6 +1317,11 @@
         self._deleteRepositorySubscriptions()
         self._deleteJobs()
         getUtility(IWebhookSet).delete(self.webhooks)
+        # We intentionally skip the usual destructors; the only other useful
+        # thing they do is to log the removal activity, and we remove the
+        # activity logs for removed repositories anyway.
+        self.grants.remove()
+        self.rules.remove()
 
         # Now destroy the repository.
         repository_name = self.unique_name

=== added file 'lib/lp/code/model/gitrule.py'
--- lib/lp/code/model/gitrule.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/model/gitrule.py	2018-10-05 14:49:26 +0000
@@ -0,0 +1,208 @@
+# Copyright 2018 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Git repository access rules."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'GitRule',
+    'GitRuleGrant',
+    ]
+
+from lazr.enum import DBItem
+import pytz
+from storm.locals import (
+    Bool,
+    DateTime,
+    Int,
+    Reference,
+    Store,
+    Unicode,
+    )
+from zope.interface import implementer
+from zope.security.proxy import removeSecurityProxy
+
+from lp.code.enums import GitGranteeType
+from lp.code.interfaces.gitrule import (
+    IGitRule,
+    IGitRuleGrant,
+    )
+from lp.registry.interfaces.person import (
+    validate_person,
+    validate_public_person,
+    )
+from lp.services.database.constants import (
+    DEFAULT,
+    UTC_NOW,
+    )
+from lp.services.database.enumcol import DBEnum
+from lp.services.database.stormbase import StormBase
+
+
+def git_rule_modified(rule, event):
+    """Update date_last_modified when a GitRule is modified.
+
+    This method is registered as a subscriber to `IObjectModifiedEvent`
+    events on Git repository rules.
+    """
+    if event.edited_fields:
+        removeSecurityProxy(rule).date_last_modified = UTC_NOW
+
+
+@implementer(IGitRule)
+class GitRule(StormBase):
+    """See `IGitRule`."""
+
+    __storm_table__ = 'GitRule'
+
+    id = Int(primary=True)
+
+    repository_id = Int(name='repository', allow_none=False)
+    repository = Reference(repository_id, 'GitRepository.id')
+
+    position = Int(name='position', allow_none=False)
+
+    ref_pattern = Unicode(name='ref_pattern', allow_none=False)
+
+    creator_id = Int(
+        name='creator', allow_none=False, validator=validate_public_person)
+    creator = Reference(creator_id, 'Person.id')
+
+    date_created = DateTime(
+        name='date_created', tzinfo=pytz.UTC, allow_none=False)
+    date_last_modified = DateTime(
+        name='date_last_modified', tzinfo=pytz.UTC, allow_none=False)
+
+    def __init__(self, repository, position, ref_pattern, creator,
+                 date_created):
+        super(GitRule, self).__init__()
+        self.repository = repository
+        self.position = position
+        self.ref_pattern = ref_pattern
+        self.creator = creator
+        self.date_created = date_created
+        self.date_last_modified = date_created
+
+    def __repr__(self):
+        return "<GitRule '%s' for %s>" % (
+            self.ref_pattern, self.repository.unique_name)
+
+    @property
+    def is_exact(self):
+        """See `IGitRule`."""
+        # turnip's glob_to_re only treats * as special, so any rule whose
+        # pattern does not contain * must be an exact-match rule.
+        return "*" not in self.ref_pattern
+
+    @property
+    def grants(self):
+        """See `IGitRule`."""
+        return Store.of(self).find(
+            GitRuleGrant, GitRuleGrant.rule_id == self.id)
+
+    def addGrant(self, grantee, grantor, can_create=False, can_push=False,
+                 can_force_push=False):
+        """See `IGitRule`."""
+        return GitRuleGrant(
+            rule=self, grantee=grantee, can_create=can_create,
+            can_push=can_push, can_force_push=can_force_push, grantor=grantor,
+            date_created=DEFAULT)
+
+    def destroySelf(self):
+        """See `IGitRule`."""
+        for grant in self.grants:
+            grant.destroySelf()
+        rules = list(self.repository.rules)
+        Store.of(self).remove(self)
+        rules.remove(self)
+        removeSecurityProxy(self.repository)._syncRulePositions(rules)
+
+
+def git_rule_grant_modified(grant, event):
+    """Update date_last_modified when a GitRuleGrant is modified.
+
+    This method is registered as a subscriber to `IObjectModifiedEvent`
+    events on Git repository grants.
+    """
+    if event.edited_fields:
+        removeSecurityProxy(grant).date_last_modified = UTC_NOW
+
+
+@implementer(IGitRuleGrant)
+class GitRuleGrant(StormBase):
+    """See `IGitRuleGrant`."""
+
+    __storm_table__ = 'GitRuleGrant'
+
+    id = Int(primary=True)
+
+    repository_id = Int(name='repository', allow_none=False)
+    repository = Reference(repository_id, 'GitRepository.id')
+
+    rule_id = Int(name='rule', allow_none=False)
+    rule = Reference(rule_id, 'GitRule.id')
+
+    grantee_type = DBEnum(
+        name='grantee_type', enum=GitGranteeType, allow_none=False)
+
+    grantee_id = Int(
+        name='grantee', allow_none=True, validator=validate_person)
+    grantee = Reference(grantee_id, 'Person.id')
+
+    can_create = Bool(name='can_create', allow_none=False)
+    can_push = Bool(name='can_push', allow_none=False)
+    can_force_push = Bool(name='can_force_push', allow_none=False)
+
+    grantor_id = Int(
+        name='grantor', allow_none=False, validator=validate_public_person)
+    grantor = Reference(grantor_id, 'Person.id')
+
+    date_created = DateTime(
+        name='date_created', tzinfo=pytz.UTC, allow_none=False)
+    date_last_modified = DateTime(
+        name='date_last_modified', tzinfo=pytz.UTC, allow_none=False)
+
+    def __init__(self, rule, grantee, can_create, can_push, can_force_push,
+                 grantor, date_created):
+        if isinstance(grantee, DBItem) and grantee.enum == GitGranteeType:
+            if grantee == GitGranteeType.PERSON:
+                raise ValueError(
+                    "grantee may not be GitGranteeType.PERSON; pass a person "
+                    "object instead")
+            grantee_type = grantee
+            grantee = None
+        else:
+            grantee_type = GitGranteeType.PERSON
+
+        self.repository = rule.repository
+        self.rule = rule
+        self.grantee_type = grantee_type
+        self.grantee = grantee
+        self.can_create = can_create
+        self.can_push = can_push
+        self.can_force_push = can_force_push
+        self.grantor = grantor
+        self.date_created = date_created
+        self.date_last_modified = date_created
+
+    def __repr__(self):
+        permissions = []
+        if self.can_create:
+            permissions.append("create")
+        if self.can_push:
+            permissions.append("push")
+        if self.can_force_push:
+            permissions.append("force-push")
+        if self.grantee_type == GitGranteeType.PERSON:
+            grantee_name = "~%s" % self.grantee.name
+        else:
+            grantee_name = self.grantee_type.title.lower()
+        return "<GitRuleGrant [%s] to %s for %s:%s>" % (
+            ", ".join(permissions), grantee_name, self.repository.unique_name,
+            self.rule.ref_pattern)
+
+    def destroySelf(self):
+        """See `IGitRuleGrant`."""
+        Store.of(self).remove(self)

=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py	2018-08-31 14:25:40 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py	2018-10-05 14:49:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 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).
 
 """Tests for Git repositories."""
@@ -23,6 +23,7 @@
 from testtools.matchers import (
     EndsWith,
     LessThan,
+    MatchesListwise,
     MatchesSetwise,
     MatchesStructure,
     )
@@ -537,6 +538,14 @@
         transaction.commit()
         self.assertRaises(LostObjectError, getattr, webhook, 'target')
 
+    def test_related_rules_and_grants_deleted(self):
+        rule = self.factory.makeGitRule(repository=self.repository)
+        grant = self.factory.makeGitRuleGrant(rule=rule)
+        self.repository.destroySelf()
+        transaction.commit()
+        self.assertRaises(LostObjectError, getattr, grant, 'rule')
+        self.assertRaises(LostObjectError, getattr, rule, 'repository')
+
 
 class TestGitRepositoryDeletionConsequences(TestCaseWithFactory):
     """Test determination and application of repository deletion
@@ -2179,6 +2188,120 @@
         self.assertEqual('Some text', ret)
 
 
+class TestGitRepositoryRules(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_rules(self):
+        repository = self.factory.makeGitRepository()
+        other_repository = self.factory.makeGitRepository()
+        self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/*")
+        self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/stable/*")
+        self.factory.makeGitRule(
+            repository=other_repository, ref_pattern="refs/heads/*")
+        self.assertThat(list(repository.rules), MatchesListwise([
+            MatchesStructure.byEquality(
+                repository=repository,
+                ref_pattern="refs/heads/*"),
+            MatchesStructure.byEquality(
+                repository=repository,
+                ref_pattern="refs/heads/stable/*"),
+            ]))
+
+    def test_addRule_append(self):
+        repository = self.factory.makeGitRepository()
+        initial_rule = self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/*")
+        self.assertEqual(0, initial_rule.position)
+        with person_logged_in(repository.owner):
+            new_rule = repository.addRule(
+                "refs/heads/stable/*", repository.owner)
+        self.assertEqual(1, new_rule.position)
+        self.assertEqual([initial_rule, new_rule], list(repository.rules))
+
+    def test_addRule_insert(self):
+        repository = self.factory.makeGitRepository()
+        initial_rules = [
+            self.factory.makeGitRule(
+                repository=repository, ref_pattern="refs/heads/*"),
+            self.factory.makeGitRule(
+                repository=repository, ref_pattern="refs/heads/protected/*"),
+            self.factory.makeGitRule(
+                repository=repository, ref_pattern="refs/heads/another/*"),
+            ]
+        self.assertEqual([0, 1, 2], [rule.position for rule in initial_rules])
+        with person_logged_in(repository.owner):
+            new_rule = repository.addRule(
+                "refs/heads/stable/*", repository.owner, position=1)
+        self.assertEqual(1, new_rule.position)
+        self.assertEqual([0, 2, 3], [rule.position for rule in initial_rules])
+        self.assertEqual(
+            [initial_rules[0], new_rule, initial_rules[1], initial_rules[2]],
+            list(repository.rules))
+
+    def test_addRule_exact_first(self):
+        repository = self.factory.makeGitRepository()
+        initial_rules = [
+            self.factory.makeGitRule(
+                repository=repository, ref_pattern="refs/heads/exact"),
+            self.factory.makeGitRule(
+                repository=repository, ref_pattern="refs/heads/*"),
+            ]
+        self.assertEqual([0, 1], [rule.position for rule in initial_rules])
+        with person_logged_in(repository.owner):
+            exact_rule = repository.addRule(
+                "refs/heads/exact-2", repository.owner)
+        self.assertEqual(
+            [initial_rules[0], exact_rule, initial_rules[1]],
+            list(repository.rules))
+        with person_logged_in(repository.owner):
+            wildcard_rule = repository.addRule(
+                "refs/heads/wildcard/*", repository.owner, position=0)
+        self.assertEqual(
+            [initial_rules[0], exact_rule, wildcard_rule, initial_rules[1]],
+            list(repository.rules))
+
+    def test_moveRule(self):
+        repository = self.factory.makeGitRepository()
+        rules = [
+            self.factory.makeGitRule(
+                repository=repository,
+                ref_pattern=self.factory.getUniqueUnicode(
+                    prefix="refs/heads/*/"))
+            for _ in range(5)]
+        with person_logged_in(repository.owner):
+            self.assertEqual(rules, list(repository.rules))
+            repository.moveRule(rules[0], 4)
+            self.assertEqual(rules[1:] + [rules[0]], list(repository.rules))
+            repository.moveRule(rules[0], 0)
+            self.assertEqual(rules, list(repository.rules))
+            repository.moveRule(rules[2], 1)
+            self.assertEqual(
+                [rules[0], rules[2], rules[1], rules[3], rules[4]],
+                list(repository.rules))
+
+    def test_moveRule_non_negative(self):
+        rule = self.factory.makeGitRule()
+        with person_logged_in(rule.repository.owner):
+            self.assertRaises(ValueError, rule.repository.moveRule, rule, -1)
+
+    def test_grants(self):
+        repository = self.factory.makeGitRepository()
+        other_repository = self.factory.makeGitRepository()
+        rule = self.factory.makeGitRule(repository=repository)
+        other_rule = self.factory.makeGitRule(repository=other_repository)
+        grants = [
+            self.factory.makeGitRuleGrant(
+                rule=rule, grantee=self.factory.makePerson())
+            for _ in range(2)
+            ]
+        self.factory.makeGitRuleGrant(
+            rule=other_rule, grantee=self.factory.makePerson())
+        self.assertContentEqual(grants, repository.grants)
+
+
 class TestGitRepositorySet(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer

=== added file 'lib/lp/code/model/tests/test_gitrule.py'
--- lib/lp/code/model/tests/test_gitrule.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/model/tests/test_gitrule.py	2018-10-05 14:49:26 +0000
@@ -0,0 +1,221 @@
+# Copyright 2018 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for Git repository access rules."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+
+from storm.store import Store
+from testtools.matchers import (
+    Equals,
+    Is,
+    MatchesSetwise,
+    MatchesStructure,
+    )
+
+from lp.code.enums import GitGranteeType
+from lp.code.interfaces.gitrule import (
+    IGitRule,
+    IGitRuleGrant,
+    )
+from lp.services.database.sqlbase import get_transaction_timestamp
+from lp.testing import (
+    person_logged_in,
+    TestCaseWithFactory,
+    verifyObject,
+    )
+from lp.testing.layers import DatabaseFunctionalLayer
+
+
+class TestGitRule(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_implements_IGitRule(self):
+        rule = self.factory.makeGitRule()
+        verifyObject(IGitRule, rule)
+
+    def test_properties(self):
+        owner = self.factory.makeTeam()
+        member = self.factory.makePerson(member_of=[owner])
+        repository = self.factory.makeGitRepository(owner=owner)
+        rule = self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/stable/*",
+            creator=member)
+        now = get_transaction_timestamp(Store.of(rule))
+        self.assertThat(rule, MatchesStructure.byEquality(
+            repository=repository,
+            ref_pattern="refs/heads/stable/*",
+            creator=member,
+            date_created=now,
+            date_last_modified=now))
+
+    def test_repr(self):
+        repository = self.factory.makeGitRepository()
+        rule = self.factory.makeGitRule(repository=repository)
+        self.assertEqual(
+            "<GitRule 'refs/heads/*' for %s>" % repository.unique_name,
+            repr(rule))
+
+    def test_is_exact(self):
+        repository = self.factory.makeGitRepository()
+        for ref_pattern, is_exact in (
+                ("refs/heads/master", True),
+                ("refs/heads/*", False),
+                # XXX cjwatson 2018-09-25: We may want to support ? and
+                # [...] in the future, since they're potentially useful and
+                # don't collide with valid ref names.
+                ("refs/heads/?", True),
+                ("refs/heads/[abc]", True),
+                (r"refs/heads/.\$", True),
+                ):
+            self.assertEqual(
+                is_exact,
+                self.factory.makeGitRule(
+                    repository=repository,
+                    ref_pattern=ref_pattern).is_exact)
+
+    def test_grants(self):
+        rule = self.factory.makeGitRule()
+        other_rule = self.factory.makeGitRule(
+            repository=rule.repository, ref_pattern="refs/heads/stable/*")
+        grantees = [self.factory.makePerson() for _ in range(2)]
+        self.factory.makeGitRuleGrant(
+            rule=rule, grantee=GitGranteeType.REPOSITORY_OWNER,
+            can_create=True)
+        self.factory.makeGitRuleGrant(
+            rule=rule, grantee=grantees[0], can_push=True)
+        self.factory.makeGitRuleGrant(
+            rule=rule, grantee=grantees[1], can_force_push=True)
+        self.factory.makeGitRuleGrant(
+            rule=other_rule, grantee=grantees[0], can_push=True)
+        self.assertThat(rule.grants, MatchesSetwise(
+            MatchesStructure(
+                rule=Equals(rule),
+                grantee_type=Equals(GitGranteeType.REPOSITORY_OWNER),
+                grantee=Is(None),
+                can_create=Is(True),
+                can_push=Is(False),
+                can_force_push=Is(False)),
+            MatchesStructure(
+                rule=Equals(rule),
+                grantee_type=Equals(GitGranteeType.PERSON),
+                grantee=Equals(grantees[0]),
+                can_create=Is(False),
+                can_push=Is(True),
+                can_force_push=Is(False)),
+            MatchesStructure(
+                rule=Equals(rule),
+                grantee_type=Equals(GitGranteeType.PERSON),
+                grantee=Equals(grantees[1]),
+                can_create=Is(False),
+                can_push=Is(False),
+                can_force_push=Is(True))))
+
+    def test_destroySelf(self):
+        repository = self.factory.makeGitRepository()
+        rules = [
+            self.factory.makeGitRule(
+                repository=repository,
+                ref_pattern=self.factory.getUniqueUnicode(
+                    prefix="refs/heads/"))
+            for _ in range(4)]
+        self.assertEqual([0, 1, 2, 3], [rule.position for rule in rules])
+        self.assertEqual(rules, list(repository.rules))
+        with person_logged_in(repository.owner):
+            rules[1].destroySelf()
+        del rules[1]
+        self.assertEqual([0, 1, 2], [rule.position for rule in rules])
+        self.assertEqual(rules, list(repository.rules))
+
+    def test_destroySelf_removes_grants(self):
+        repository = self.factory.makeGitRepository()
+        rule = self.factory.makeGitRule(repository=repository)
+        grant = self.factory.makeGitRuleGrant(rule=rule)
+        self.assertEqual([grant], list(repository.grants))
+        with person_logged_in(repository.owner):
+            rule.destroySelf()
+        self.assertEqual([], list(repository.grants))
+
+
+class TestGitRuleGrant(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_implements_IGitRuleGrant(self):
+        grant = self.factory.makeGitRuleGrant()
+        verifyObject(IGitRuleGrant, grant)
+
+    def test_properties_owner(self):
+        owner = self.factory.makeTeam()
+        member = self.factory.makePerson(member_of=[owner])
+        rule = self.factory.makeGitRule(owner=owner)
+        grant = self.factory.makeGitRuleGrant(
+            rule=rule, grantee=GitGranteeType.REPOSITORY_OWNER, grantor=member,
+            can_create=True, can_force_push=True)
+        now = get_transaction_timestamp(Store.of(grant))
+        self.assertThat(grant, MatchesStructure(
+            repository=Equals(rule.repository),
+            rule=Equals(rule),
+            grantee_type=Equals(GitGranteeType.REPOSITORY_OWNER),
+            grantee=Is(None),
+            can_create=Is(True),
+            can_push=Is(False),
+            can_force_push=Is(True),
+            grantor=Equals(member),
+            date_created=Equals(now),
+            date_last_modified=Equals(now)))
+
+    def test_properties_person(self):
+        owner = self.factory.makeTeam()
+        member = self.factory.makePerson(member_of=[owner])
+        rule = self.factory.makeGitRule(owner=owner)
+        grantee = self.factory.makePerson()
+        grant = self.factory.makeGitRuleGrant(
+            rule=rule, grantee=grantee, grantor=member, can_push=True)
+        now = get_transaction_timestamp(Store.of(rule))
+        self.assertThat(grant, MatchesStructure(
+            repository=Equals(rule.repository),
+            rule=Equals(rule),
+            grantee_type=Equals(GitGranteeType.PERSON),
+            grantee=Equals(grantee),
+            can_create=Is(False),
+            can_push=Is(True),
+            can_force_push=Is(False),
+            grantor=Equals(member),
+            date_created=Equals(now),
+            date_last_modified=Equals(now)))
+
+    def test_repr_owner(self):
+        rule = self.factory.makeGitRule()
+        grant = self.factory.makeGitRuleGrant(
+            rule=rule, grantee=GitGranteeType.REPOSITORY_OWNER,
+            can_create=True, can_push=True)
+        self.assertEqual(
+            "<GitRuleGrant [create, push] to repository owner for %s:%s>" % (
+                rule.repository.unique_name, rule.ref_pattern),
+            repr(grant))
+
+    def test_repr_person(self):
+        rule = self.factory.makeGitRule()
+        grantee = self.factory.makePerson()
+        grant = self.factory.makeGitRuleGrant(
+            rule=rule, grantee=grantee, can_push=True)
+        self.assertEqual(
+            "<GitRuleGrant [push] to ~%s for %s:%s>" % (
+                grantee.name, rule.repository.unique_name, rule.ref_pattern),
+            repr(grant))
+
+    def test_destroySelf(self):
+        rule = self.factory.makeGitRule()
+        grants = [
+            self.factory.makeGitRuleGrant(
+                rule=rule, grantee=GitGranteeType.REPOSITORY_OWNER,
+                can_create=True),
+            self.factory.makeGitRuleGrant(rule=rule, can_push=True),
+            ]
+        with person_logged_in(rule.repository.owner):
+            grants[1].destroySelf()
+        self.assertThat(rule.grants, MatchesSetwise(Equals(grants[0])))

=== modified file 'lib/lp/registry/personmerge.py'
--- lib/lp/registry/personmerge.py	2018-10-02 12:05:48 +0000
+++ lib/lp/registry/personmerge.py	2018-10-05 14:49:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Person/team merger implementation."""
@@ -141,6 +141,42 @@
         ''' % vars())
 
 
+def _mergeGitRuleGrant(cur, from_id, to_id):
+    # Transfer GitRuleGrants that only exist on from_person.
+    cur.execute('''
+        UPDATE GitRuleGrant
+        SET grantee=%(to_id)d
+        WHERE
+            grantee = %(from_id)d
+            AND rule NOT IN (
+                SELECT rule
+                FROM GitRuleGrant
+                WHERE grantee = %(to_id)d
+                )
+        ''' % vars())
+    # Merge permissions on GitRuleGrants that exist on both from_person and
+    # to_person.  When multiple grants match a user we take the union of the
+    # permissions they confer, so it's safe to do that here too.
+    cur.execute('''
+        UPDATE GitRuleGrant
+        SET
+            can_create = GitRuleGrant.can_create OR other.can_create,
+            can_push = GitRuleGrant.can_push OR other.can_push,
+            can_force_push =
+                GitRuleGrant.can_force_push OR other.can_force_push
+        FROM GitRuleGrant AS other
+        WHERE
+            GitRuleGrant.grantee = %(to_id)d
+            AND other.grantee = %(from_id)d
+            AND GitRuleGrant.rule = other.rule
+        ''' % vars())
+    # Delete the remaining GitRuleGrants for from_person, which have now all
+    # been either transferred or merged.
+    cur.execute('''
+        DELETE FROM GitRuleGrant WHERE grantee = %(from_id)d
+        ''' % vars())
+
+
 def _mergeBranches(from_person, to_person):
     # This shouldn't use removeSecurityProxy.
     branches = getUtility(IBranchCollection).ownedBy(from_person)
@@ -775,8 +811,10 @@
 
     _mergeAccessArtifactGrant(cur, from_id, to_id)
     _mergeAccessPolicyGrant(cur, from_id, to_id)
+    _mergeGitRuleGrant(cur, from_id, to_id)
     skip.append(('accessartifactgrant', 'grantee'))
     skip.append(('accesspolicygrant', 'grantee'))
+    skip.append(('gitrulegrant', 'grantee'))
 
     # Update the Branches that will not conflict, and fudge the names of
     # ones that *do* conflict.

=== modified file 'lib/lp/registry/tests/test_personmerge.py'
--- lib/lp/registry/tests/test_personmerge.py	2016-06-28 21:10:18 +0000
+++ lib/lp/registry/tests/test_personmerge.py	2018-10-05 14:49:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for merge_people."""
@@ -7,7 +7,10 @@
 from operator import attrgetter
 
 import pytz
-from testtools.matchers import MatchesStructure
+from testtools.matchers import (
+    MatchesSetwise,
+    MatchesStructure,
+    )
 import transaction
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
@@ -494,6 +497,73 @@
                 grantee=person,
                 date_created=person_grant_date))
 
+    def test_merge_transfers_non_conflicting_gitrulegrants(self):
+        # GitRuleGrants are transferred from the duplicate.
+        rule = self.factory.makeGitRule()
+        person = self.factory.makePerson()
+        grant = self.factory.makeGitRuleGrant(rule=rule)
+        self._do_premerge(grant.grantee, person)
+
+        self.assertEqual(grant.grantee, rule.grants.one().grantee)
+        with person_logged_in(person):
+            self._do_merge(grant.grantee, person)
+        self.assertEqual(person, rule.grants.one().grantee)
+
+    def test_merge_conflicting_gitrulegrants(self):
+        # Conflicting GitRuleGrants have their permissions merged.
+        rule = self.factory.makeGitRule()
+
+        person = self.factory.makePerson()
+        person_grant = self.factory.makeGitRuleGrant(
+            rule=rule, grantee=person, can_create=True)
+        person_grant_date = person_grant.date_created
+
+        duplicate = self.factory.makePerson()
+        self.factory.makeGitRuleGrant(
+            rule=rule, grantee=duplicate, can_push=True)
+
+        other_person = self.factory.makePerson()
+        self.factory.makeGitRuleGrant(
+            rule=rule, grantee=other_person, can_push=True)
+
+        other_rule = self.factory.makeGitRule(
+            rule.repository, ref_pattern=u"refs/heads/other/*")
+        self.factory.makeGitRuleGrant(
+            rule=other_rule, grantee=other_person, can_force_push=True)
+
+        self._do_premerge(duplicate, person)
+        with person_logged_in(person):
+            self._do_merge(duplicate, person)
+
+        # Only two grants for the rule exist: the retained person's, with
+        # the union of permissions from the duplicate and target grants, and
+        # the grant to an unrelated person.
+        self.assertThat(
+            list(rule.grants),
+            MatchesSetwise(
+                MatchesStructure.byEquality(
+                    rule=rule,
+                    grantee=person,
+                    date_created=person_grant_date,
+                    can_create=True,
+                    can_push=True,
+                    can_force_push=False),
+                MatchesStructure.byEquality(
+                    rule=rule,
+                    grantee=other_person,
+                    can_create=False,
+                    can_push=True,
+                    can_force_push=False)))
+        # A grant for another rule is untouched.
+        self.assertThat(
+            other_rule.grants.one(),
+            MatchesStructure.byEquality(
+                rule=other_rule,
+                grantee=other_person,
+                can_create=False,
+                can_push=False,
+                can_force_push=True))
+
     def test_mergeAsync(self):
         # mergeAsync() creates a new `PersonMergeJob`.
         from_person = self.factory.makePerson()

=== modified file 'lib/lp/security.py'
--- lib/lp/security.py	2018-09-15 11:38:45 +0000
+++ lib/lp/security.py	2018-10-05 14:49:26 +0000
@@ -87,6 +87,10 @@
     IGitRepository,
     user_has_special_git_repository_access,
     )
+from lp.code.interfaces.gitrule import (
+    IGitRule,
+    IGitRuleGrant,
+    )
 from lp.code.interfaces.sourcepackagerecipe import ISourcePackageRecipe
 from lp.code.interfaces.sourcepackagerecipebuild import (
     ISourcePackageRecipeBuild,
@@ -2343,6 +2347,42 @@
         super(EditGitRef, self).__init__(obj, obj.repository)
 
 
+class ViewGitRule(DelegatedAuthorization):
+    """Anyone who can see a Git repository can see its access rules."""
+    permission = 'launchpad.View'
+    usedfor = IGitRule
+
+    def __init__(self, obj):
+        super(ViewGitRule, self).__init__(obj, obj.repository)
+
+
+class EditGitRule(DelegatedAuthorization):
+    """Anyone who can edit a Git repository can edit its access rules."""
+    permission = 'launchpad.Edit'
+    usedfor = IGitRule
+
+    def __init__(self, obj):
+        super(EditGitRule, self).__init__(obj, obj.repository)
+
+
+class ViewGitRuleGrant(DelegatedAuthorization):
+    """Anyone who can see a Git repository can see its access grants."""
+    permission = 'launchpad.View'
+    usedfor = IGitRuleGrant
+
+    def __init__(self, obj):
+        super(ViewGitRuleGrant, self).__init__(obj, obj.repository)
+
+
+class EditGitRuleGrant(DelegatedAuthorization):
+    """Anyone who can edit a Git repository can edit its access grants."""
+    permission = 'launchpad.Edit'
+    usedfor = IGitRuleGrant
+
+    def __init__(self, obj):
+        super(EditGitRuleGrant, self).__init__(obj, obj.repository)
+
+
 class AdminDistroSeriesTranslations(AuthorizationBase):
     permission = 'launchpad.TranslationsAdmin'
     usedfor = IDistroSeries

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2018-09-13 15:21:05 +0000
+++ lib/lp/testing/factory.py	2018-10-05 14:49:26 +0000
@@ -1828,6 +1828,31 @@
             path = self.getUniqueString('refs/heads/path').decode('utf-8')
         return getUtility(IGitRefRemoteSet).new(repository_url, path)
 
+    def makeGitRule(self, repository=None, ref_pattern=u"refs/heads/*",
+                    creator=None, position=None, **repository_kwargs):
+        """Create a Git repository access rule."""
+        if repository is None:
+            repository = self.makeGitRepository(**repository_kwargs)
+        if creator is None:
+            creator = repository.owner
+        with person_logged_in(creator):
+            return repository.addRule(ref_pattern, creator, position=position)
+
+    def makeGitRuleGrant(self, rule=None, grantee=None, grantor=None,
+                         can_create=False, can_push=False,
+                         can_force_push=False):
+        """Create a Git repository access grant."""
+        if rule is None:
+            rule = self.makeGitRule()
+        if grantee is None:
+            grantee = self.makePerson()
+        if grantor is None:
+            grantor = rule.repository.owner
+        with person_logged_in(grantor):
+            return rule.addGrant(
+                grantee, grantor, can_create=can_create, can_push=can_push,
+                can_force_push=can_force_push)
+
     def makeBug(self, target=None, owner=None, bug_watch_url=None,
                 information_type=None, date_closed=None, title=None,
                 date_created=None, description=None, comment=None,

=== modified file 'scripts/close-account.py'
--- scripts/close-account.py	2018-05-08 13:44:21 +0000
+++ scripts/close-account.py	2018-10-05 14:49:26 +0000
@@ -1,6 +1,6 @@
 #!/usr/bin/python -S
 #
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Remove personal details of a user from the database, leaving a stub."""
@@ -162,6 +162,9 @@
 
         # Pending items in queues
         ('POExportRequest', 'person'),
+
+        # Access grants
+        ('GitRuleGrant', 'grantee'),
         ]
     for table, person_id_column in removals:
         table_notification(table)