launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05713
[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."""