← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~wgrant/launchpad/observer-model-artifacts into lp:launchpad

 

William Grant has proposed merging lp:~wgrant/launchpad/observer-model-artifacts into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/observer-model-artifacts/+merge/83556

This branch adds basic support for setting a bug's access policy. A setAccessPolicy method is added, and transitionToTarget is adjusted to validate and change the policy when the pillar is changed.

This won't work correctly for multi-pillar private bugs (nobody really quite knows how they'll work), but it's a start. Since there are no access policies on production yet and everything is a no-op if the policy is unset, this won't break anything.
-- 
https://code.launchpad.net/~wgrant/launchpad/observer-model-artifacts/+merge/83556
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/observer-model-artifacts into lp:launchpad.
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2011-11-28 03:51:37 +0000
+++ lib/lp/bugs/configure.zcml	2011-11-28 08:36:28 +0000
@@ -707,6 +707,7 @@
                 attributes="
                     id
                     private
+                    access_policy
                     bugtasks
                     default_bugtask
                     affected_pillars
@@ -835,6 +836,7 @@
                     setPrivate
                     setSecurityRelated
                     setPrivacyAndSecurityRelated
+                    setAccessPolicy
                     setStatus
                     subscribe
                     unlinkBranch

=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py	2011-11-16 10:43:04 +0000
+++ lib/lp/bugs/interfaces/bug.py	2011-11-28 08:36:28 +0000
@@ -248,6 +248,7 @@
     security_related = exported(
         Bool(title=_("This bug is a security vulnerability."),
              required=False, default=False, readonly=True))
+    access_policy = Attribute("Access policy")
     displayname = TextLine(title=_("Text of the form 'Bug #X"),
         readonly=True)
     activity = exported(
@@ -888,6 +889,9 @@
         Return (private_changed, security_related_changed) tuple.
         """
 
+    def setAccessPolicy(policy_type):
+        """Set the `IAccessPolicy` that controls access to this bug."""
+
     def getBugTask(target):
         """Return the bugtask with the specified target.
 

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2011-11-26 00:26:02 +0000
+++ lib/lp/bugs/model/bug.py	2011-11-28 08:36:28 +0000
@@ -186,6 +186,10 @@
     )
 from lp.code.interfaces.branchcollection import IAllBranches
 from lp.hardwaredb.interfaces.hwdb import IHWSubmissionBugSet
+from lp.registry.interfaces.accesspolicy import (
+    IAccessPolicySource,
+    UnsuitableAccessPolicyError,
+    )
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.distroseries import IDistroSeries
 from lp.registry.interfaces.person import (
@@ -357,6 +361,8 @@
         dbName='who_made_private', foreignKey='Person',
         storm_validator=validate_public_person, default=None)
     security_related = BoolCol(notNull=True, default=False)
+    access_policy_id = Int(name="access_policy")
+    access_policy = Reference(access_policy_id, 'AccessPolicy.id')
 
     # useful Joins
     activity = SQLMultipleJoin('BugActivity', joinColumn='bug', orderBy='id')
@@ -1784,6 +1790,19 @@
         return self.setPrivacyAndSecurityRelated(
             self.private, security_related, who)[1]
 
+    def setAccessPolicy(self, type):
+        """See `IBug`."""
+        if type is None:
+            policy = None
+        else:
+            policy = getUtility(IAccessPolicySource).getByPillarAndType(
+                self.default_bugtask.pillar, type)
+            if policy is None:
+                raise UnsuitableAccessPolicyError(
+                    "%s doesn't have a %s access policy."
+                    % (self.default_bugtask.pillar.name, type.title))
+        self.access_policy = policy
+
     def getRequiredSubscribers(self, for_private, for_security_related, who):
         """Return the mandatory subscribers for a bug with given attributes.
 

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2011-11-23 03:09:55 +0000
+++ lib/lp/bugs/model/bugtask.py	2011-11-28 08:36:28 +0000
@@ -135,6 +135,7 @@
     )
 from lp.bugs.model.bugnomination import BugNomination
 from lp.bugs.model.bugsubscription import BugSubscription
+from lp.registry.interfaces.accesspolicy import IAccessPolicySource
 from lp.registry.interfaces.distribution import (
     IDistribution,
     IDistributionSet,
@@ -392,7 +393,8 @@
 def validate_target(bug, target, retarget_existing=True):
     """Validate a bugtask target against a bug's existing tasks.
 
-    Checks that no conflicting tasks already exist.
+    Checks that no conflicting tasks already exist, and that the new
+    target's pillar supports the bug's access policy.
     """
     if bug.getBugTask(target):
         raise IllegalTarget(
@@ -425,6 +427,14 @@
                 "Private bugs cannot affect multiple projects."
                     % bug.default_bugtask.target.bugtargetdisplayname)
 
+    if (bug.access_policy is not None and
+        bug.access_policy.pillar != target.pillar and
+        not getUtility(IAccessPolicySource).getByPillarAndType(
+            target.pillar, bug.access_policy.type)):
+        raise IllegalTarget(
+            "%s doesn't have a %s access policy."
+            % (target.pillar.displayname, bug.access_policy.type.title))
+
 
 def validate_new_target(bug, target):
     """Validate a bugtask target to be added.
@@ -1223,6 +1233,12 @@
             setattr(self, name, value)
         self.updateTargetNameCache()
 
+        # If there's a policy set and we're changing to a another
+        # pillar, recalculate the access policy.
+        if (self.bug.access_policy is not None and
+            self.bug.access_policy.pillar != target.pillar):
+            self.bug.setAccessPolicy(self.bug.access_policy.type)
+
         # After the target has changed, we need to recalculate the maximum bug
         # heat for the new and old targets.
         if self.target != target_before_change:

=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py	2011-11-23 03:09:55 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py	2011-11-28 08:36:28 +0000
@@ -2188,6 +2188,22 @@
             (t.target for t in bug.bugtasks),
             [sp, sp.distribution_sourcepackage, other_distro])
 
+    def test_access_policy_changed(self):
+        # If an access policy is set, changing the pillar also switches
+        # to the matching policy on the new pillar.
+        orig_product = self.factory.makeProduct()
+        orig_policy = self.factory.makeAccessPolicy(pillar=orig_product)
+        new_product = self.factory.makeProduct()
+        new_policy = self.factory.makeAccessPolicy(
+            pillar=new_product, type=orig_policy.type)
+
+        bug = self.factory.makeBug(product=orig_product)
+        with person_logged_in(bug.owner):
+            bug.setAccessPolicy(orig_policy.type)
+            self.assertEqual(orig_policy, bug.access_policy)
+            bug.default_bugtask.transitionToTarget(new_product)
+            self.assertEqual(new_policy, bug.access_policy)
+
 
 class TestBugTargetKeys(TestCaseWithFactory):
     """Tests for bug_target_to_key and bug_target_from_key."""
@@ -2514,6 +2530,38 @@
             % (dsp.sourcepackagename.name, dsp.distribution.displayname),
             validate_target, task.bug, dsp)
 
+    def test_present_access_policy_works(self):
+        # If an access policy is set, changing the pillar is permitted
+        # if the target has an access policy of the same type.
+        orig_product = self.factory.makeProduct()
+        orig_policy = self.factory.makeAccessPolicy(pillar=orig_product)
+        new_product = self.factory.makeProduct()
+        self.factory.makeAccessPolicy(
+            pillar=new_product, type=orig_policy.type)
+
+        bug = self.factory.makeBug(product=orig_product)
+        with person_logged_in(bug.owner):
+            bug.setAccessPolicy(orig_policy.type)
+        self.assertEqual(orig_policy, bug.access_policy)
+        validate_target(bug, new_product)
+
+    def test_missing_access_policy_rejected(self):
+        # If the new pillar doesn't have a corresponding access polciy,
+        # the transition is forbidden.
+        orig_product = self.factory.makeProduct()
+        orig_policy = self.factory.makeAccessPolicy(pillar=orig_product)
+        new_product = self.factory.makeProduct()
+
+        bug = self.factory.makeBug(product=orig_product)
+        with person_logged_in(bug.owner):
+            bug.setAccessPolicy(orig_policy.type)
+        self.assertEqual(orig_policy, bug.access_policy)
+        self.assertRaisesWithContent(
+            IllegalTarget,
+            "%s doesn't have a %s access policy."
+            % (new_product.displayname, bug.access_policy.type.title),
+            validate_target, bug, new_product)
+
 
 class TestValidateNewTarget(TestCaseWithFactory, ValidateTargetMixin):
 

=== modified file 'lib/lp/bugs/tests/test_bug.py'
--- lib/lp/bugs/tests/test_bug.py	2011-09-14 17:57:44 +0000
+++ lib/lp/bugs/tests/test_bug.py	2011-11-28 08:36:28 +0000
@@ -24,6 +24,10 @@
     UserCannotEditBugTaskImportance,
     UserCannotEditBugTaskMilestone,
     )
+from lp.registry.interfaces.accesspolicy import (
+    AccessPolicyType,
+    UnsuitableAccessPolicyError,
+    )
 from lp.testing import (
     person_logged_in,
     StormStatementRecorder,
@@ -298,3 +302,38 @@
         params.setBugTarget(product=target)
         bug = getUtility(IBugSet).createBug(params)
         self.assertEqual([cve], [cve_link.cve for cve_link in bug.cve_links])
+
+
+class TestBugAccessPolicy(TestCaseWithFactory):
+    layer = DatabaseFunctionalLayer
+
+    def test_setAccessPolicy(self):
+        product = self.factory.makeProduct()
+        policy = self.factory.makeAccessPolicy(
+            pillar=product, type=AccessPolicyType.PRIVATE)
+        bug = self.factory.makeBug(product=product)
+        self.assertIs(None, bug.access_policy)
+        with person_logged_in(bug.owner):
+            bug.setAccessPolicy(AccessPolicyType.PRIVATE)
+        self.assertEqual(policy, bug.access_policy)
+
+    def test_setAccessPolicy_none(self):
+        product = self.factory.makeProduct()
+        policy = self.factory.makeAccessPolicy(pillar=product)
+        bug = self.factory.makeBug(product=product)
+        self.assertIs(None, bug.access_policy)
+        with person_logged_in(bug.owner):
+            bug.setAccessPolicy(policy.type)
+            bug.setAccessPolicy(None)
+        self.assertIs(None, bug.access_policy)
+
+    def test_setAccessPolicy_without_use(self):
+        # Attempting to set the access policy to a type that is not used
+        # by the pillar fails.
+        bug = self.factory.makeBug()
+        self.assertIs(None, bug.access_policy)
+        with person_logged_in(bug.owner):
+            self.assertRaises(
+                UnsuitableAccessPolicyError, bug.setAccessPolicy,
+                AccessPolicyType.PRIVATE)
+        self.assertIs(None, bug.access_policy)

=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py	2011-09-12 07:29:31 +0000
+++ lib/lp/code/interfaces/branch.py	2011-11-28 08:36:28 +0000
@@ -253,6 +253,8 @@
         description=_("This branch is explicitly marked private as opposed "
         "to being private because it is stacked on a private branch."))
 
+    access_policy = Attribute("Access policy")
+
 
 class IBranchAnyone(Interface):
     """Attributes of IBranch that can be changed by launchpad.AnyPerson."""

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2011-11-21 08:55:31 +0000
+++ lib/lp/code/model/branch.py	2011-11-28 08:36:28 +0000
@@ -180,6 +180,8 @@
     # transitively private branch. The value of this attribute is maintained
     # by a database trigger.
     transitively_private = BoolCol(dbName='transitively_private')
+    access_policy_id = Int(name="access_policy")
+    access_policy = Reference(access_policy_id, "AccessPolicy.id")
 
     @property
     def private(self):

=== modified file 'lib/lp/registry/interfaces/accesspolicy.py'
--- lib/lp/registry/interfaces/accesspolicy.py	2011-11-21 08:42:27 +0000
+++ lib/lp/registry/interfaces/accesspolicy.py	2011-11-28 08:36:28 +0000
@@ -12,18 +12,27 @@
     'IAccessPolicyArtifactSource',
     'IAccessPolicyGrant',
     'IAccessPolicySource',
+    'UnsuitableAccessPolicyError',
     ]
 
+import httplib
+
 from lazr.enum import (
     DBEnumeratedType,
     DBItem,
     )
+from lazr.restful.declarations import error_status
 from zope.interface import (
     Attribute,
     Interface,
     )
 
 
+@error_status(httplib.BAD_REQUEST)
+class UnsuitableAccessPolicyError(Exception):
+    pass
+
+
 class AccessPolicyType(DBEnumeratedType):
     """Access policy type."""