launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #11662
[Merge] lp:~adeuring/launchpad/specification-grants into lp:launchpad
Abel Deuring has proposed merging lp:~adeuring/launchpad/specification-grants into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~adeuring/launchpad/specification-grants/+merge/123062
This branch makes it possible to grant access to a single specification that
is not publicly visible. (The security adapter for ISpecification does not
use yet these grants -- the MP would become overly long with this change...)
= Details =
lib/lp/blueprints/configure.zcml,
lib/lp/blueprints/model/specification.py,
lib/lp/blueprints/tests/test_specification.py:
The property Specification.information_type should not be set directly
because instances that are not publicly visible need a record in
the table AccessPolicyArtifact that links the specification to an
AccessPolicy. These links are maintained by calling
reconcile_access_for_artifact() in Specifcation.transitionToInformationType().
This change leads to a number of changes in
lib/lp/blueprints/tests/test_specification.py, where severeal tests
changed information_type with a simple assignment.
Furthermore, reconcile_access_for_artifact() obviously fails if no
matching AccessPolicy record exists; these records are created by
calling speicifcaton.target._ensurePolicies(). This is a bit hackish,
but the class Product does not yet have a method
setSpecifcationSharingPolicy() which would manage AccessPolicy records
"officially".
lib/lp/registry/interfaces/accesspolicy.py:
The attribute specification_id was simply missing in the interface class;
it is already used in the model.
lib/lp/registry/model/accesspolicy.py,
lib/lp/registry/services/sharingservice.py,
lib/lp/registry/services/tests/test_sharingservice.py:
The core of the change: AccessArtifact.ensure() and
SharingService.ensureAccessGrants() must be able to deal
not only with bugs and branches, but also with specifications.
= tests =
./bin/test -vvt lp.registry.services.tests.test_sharingservice.*test_ensureAccessGrants
./bin/test -vvt lp.blueprints.tests.test_specification.SpecificationTests.test.*access
./bin/test -vvt lp.registry.tests.test_accesspolicy.TestAccessArtifactSpecification
no lint
--
https://code.launchpad.net/~adeuring/launchpad/specification-grants/+merge/123062
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/specification-grants into lp:launchpad.
=== modified file 'lib/lp/blueprints/configure.zcml'
--- lib/lp/blueprints/configure.zcml 2012-08-20 16:38:10 +0000
+++ lib/lp/blueprints/configure.zcml 2012-09-06 12:10:25 +0000
@@ -150,10 +150,6 @@
<class class="lp.blueprints.model.specification.Specification">
<allow interface="lp.blueprints.interfaces.specification.ISpecificationPublic"/>
<require
- permission="launchpad.Edit"
- set_attributes="information_type" />
-
- <require
permission="launchpad.LimitedView"
interface="lp.blueprints.interfaces.specification.ISpecificationView" />
=== modified file 'lib/lp/blueprints/model/specification.py'
--- lib/lp/blueprints/model/specification.py 2012-08-31 18:54:29 +0000
+++ lib/lp/blueprints/model/specification.py 2012-09-06 12:10:25 +0000
@@ -820,12 +820,17 @@
return set(InformationType.items)
def transitionToInformationType(self, information_type, who):
- """See `IBug`."""
+ """See ISpecification."""
+ # avoid circular imports.
+ from lp.registry.model.accesspolicy import (
+ reconcile_access_for_artifact,
+ )
if self.information_type == information_type:
return False
if information_type not in self.getAllowedInformationTypes(who):
raise CannotChangeInformationType("Forbidden by project policy.")
self.information_type = information_type
+ reconcile_access_for_artifact(self, information_type, [self.target])
return True
@property
=== modified file 'lib/lp/blueprints/tests/test_specification.py'
--- lib/lp/blueprints/tests/test_specification.py 2012-09-06 00:01:38 +0000
+++ lib/lp/blueprints/tests/test_specification.py 2012-09-06 12:10:25 +0000
@@ -193,9 +193,9 @@
'launchpad.AnyAllowedPerson': set(('whiteboard', )),
'launchpad.Edit': set((
'approver', 'assignee', 'definition_status', 'distribution',
- 'drafter', 'implementation_status', 'information_type',
- 'man_days', 'milestone', 'name', 'product', 'specurl',
- 'summary', 'superseded_by', 'title')),
+ 'drafter', 'implementation_status', 'man_days', 'milestone',
+ 'name', 'product', 'specurl', 'summary', 'superseded_by',
+ 'title')),
}
specification = self.factory.makeSpecification()
checker = getChecker(specification)
@@ -253,23 +253,30 @@
specification = self.factory.makeSpecification()
for information_type in PUBLIC_INFORMATION_TYPES:
with person_logged_in(specification.owner):
- specification.information_type = information_type
+ specification.transitionToInformationType(
+ information_type, specification.owner)
self.read_access_to_ISpecificationView(
ANONYMOUS, specification, error_expected=False)
# ...but not to private specifications.
+ removeSecurityProxy(specification.target)._ensurePolicies(
+ PRIVATE_INFORMATION_TYPES)
for information_type in PRIVATE_INFORMATION_TYPES:
with person_logged_in(specification.owner):
- specification.information_type = information_type
+ specification.transitionToInformationType(
+ information_type, specification.owner)
self.read_access_to_ISpecificationView(
ANONYMOUS, specification, error_expected=True)
def test_anon_write_access(self):
# Anonymous users do not have write access to specifications.
specification = self.factory.makeSpecification()
+ removeSecurityProxy(specification.target)._ensurePolicies(
+ PRIVATE_INFORMATION_TYPES)
for information_type in (PUBLIC_INFORMATION_TYPES +
PRIVATE_INFORMATION_TYPES):
with person_logged_in(specification.owner):
- specification.information_type = information_type
+ specification.transitionToInformationType(
+ information_type, specification.owner)
self.write_access_to_ISpecificationView(
ANONYMOUS, specification, error_expected=True,
attribute='whiteboard', value='foo')
@@ -283,13 +290,17 @@
user = self.factory.makePerson()
for information_type in PUBLIC_INFORMATION_TYPES:
with person_logged_in(specification.owner):
- specification.information_type = information_type
+ specification.transitionToInformationType(
+ information_type, specification.owner)
self.read_access_to_ISpecificationView(
user, specification, error_expected=False)
# ...but not to private specifications.
+ removeSecurityProxy(specification.target)._ensurePolicies(
+ PRIVATE_INFORMATION_TYPES)
for information_type in PRIVATE_INFORMATION_TYPES:
with person_logged_in(specification.owner):
- specification.information_type = information_type
+ specification.transitionToInformationType(
+ information_type, specification.owner)
self.read_access_to_ISpecificationView(
user, specification, error_expected=True)
@@ -300,7 +311,8 @@
user = self.factory.makePerson()
for information_type in PUBLIC_INFORMATION_TYPES:
with person_logged_in(specification.owner):
- specification.information_type = information_type
+ specification.transitionToInformationType(
+ information_type, specification.owner)
self.write_access_to_ISpecificationView(
user, specification, error_expected=False,
attribute='whiteboard', value='foo')
@@ -308,9 +320,12 @@
user, specification, error_expected=True,
attribute='name', value='foo')
# The cannot change any attribute of private specifcations.
+ removeSecurityProxy(specification.target)._ensurePolicies(
+ PRIVATE_INFORMATION_TYPES)
for information_type in PRIVATE_INFORMATION_TYPES:
with person_logged_in(specification.owner):
- specification.information_type = information_type
+ specification.transitionToInformationType(
+ information_type, specification.owner)
self.write_access_to_ISpecificationView(
user, specification, error_expected=True,
attribute='whiteboard', value='foo')
@@ -322,10 +337,13 @@
# Users with special privileges can aceess the attributes
# of public and private specifcations.
specification = self.factory.makeSpecification()
+ removeSecurityProxy(specification.target)._ensurePolicies(
+ PRIVATE_INFORMATION_TYPES)
for information_type in (PUBLIC_INFORMATION_TYPES +
PRIVATE_INFORMATION_TYPES):
with person_logged_in(specification.owner):
- specification.information_type = information_type
+ specification.transitionToInformationType(
+ information_type, specification.owner)
self.read_access_to_ISpecificationView(
specification.owner, specification, error_expected=False)
@@ -333,10 +351,13 @@
# Users with special privileges can change the attributes
# of public and private specifcations.
specification = self.factory.makeSpecification()
+ removeSecurityProxy(specification.target)._ensurePolicies(
+ PRIVATE_INFORMATION_TYPES)
for information_type in (PUBLIC_INFORMATION_TYPES +
PRIVATE_INFORMATION_TYPES):
with person_logged_in(specification.owner):
- specification.information_type = information_type
+ specification.transitionToInformationType(
+ information_type, specification.owner)
self.write_access_to_ISpecificationView(
specification.owner, specification, error_expected=False,
attribute='whiteboard', value='foo')
=== modified file 'lib/lp/registry/interfaces/accesspolicy.py'
--- lib/lp/registry/interfaces/accesspolicy.py 2012-09-03 11:22:25 +0000
+++ lib/lp/registry/interfaces/accesspolicy.py 2012-09-06 12:10:25 +0000
@@ -35,6 +35,7 @@
concrete_artifact = Attribute("Concrete artifact")
bug_id = Attribute("bug_id")
branch_id = Attribute("branch_id")
+ specification_id = Attribute("specification_id")
class IAccessArtifactGrant(Interface):
=== modified file 'lib/lp/registry/interfaces/sharingservice.py'
--- lib/lp/registry/interfaces/sharingservice.py 2012-09-05 03:39:50 +0000
+++ lib/lp/registry/interfaces/sharingservice.py 2012-09-06 12:10:25 +0000
@@ -247,13 +247,15 @@
branches=List(
Reference(schema=IBranch), title=_('Branches'), required=False))
@operation_for_version('devel')
- def ensureAccessGrants(grantees, user, branches=None, bugs=None):
+ def ensureAccessGrants(grantees, user, branches=None, bugs=None,
+ specifications=None):
"""Ensure a grantee has an access grant to the specified artifacts.
:param grantees: the people or teams for whom to grant access
:param user: the user making the request
:param bugs: the bugs for which to grant access
:param branches: the branches for which to grant access
+ :param specifications: the specifications for which to grant access
"""
@export_write_operation()
=== modified file 'lib/lp/registry/model/accesspolicy.py'
--- lib/lp/registry/model/accesspolicy.py 2012-09-03 11:22:25 +0000
+++ lib/lp/registry/model/accesspolicy.py 2012-09-06 12:10:25 +0000
@@ -70,7 +70,7 @@
(pillar, information_type) for pillar in pillars)
missing_pillars = set(pillars) - set([ap.pillar for ap in aps])
if len(missing_pillars):
- pillar_str = ', '.join([p.name for p in missing_pillars])
+ pillar_str = ', '.join([p.name for p in missing_pillars])
raise AssertionError(
"Pillar(s) %s require an access policy for information type "
"%s." % (pillar_str, information_type.title))
@@ -97,20 +97,25 @@
bug = Reference(bug_id, 'Bug.id')
branch_id = Int(name='branch')
branch = Reference(branch_id, 'Branch.id')
+ specification_id = Int(name='specification')
+ specification = Reference(specification_id, 'Specification.id')
@property
def concrete_artifact(self):
- artifact = self.bug or self.branch
+ artifact = self.bug or self.branch or self.specification
return artifact
@classmethod
def _constraintForConcrete(cls, concrete_artifact):
+ from lp.blueprints.interfaces.specification import ISpecification
from lp.bugs.interfaces.bug import IBug
from lp.code.interfaces.branch import IBranch
if IBug.providedBy(concrete_artifact):
col = cls.bug
elif IBranch.providedBy(concrete_artifact):
col = cls.branch
+ elif ISpecification.providedBy(concrete_artifact):
+ col = cls.specification
else:
raise ValueError(
"%r is not a valid artifact" % concrete_artifact)
@@ -128,6 +133,7 @@
@classmethod
def ensure(cls, concrete_artifacts):
"""See `IAccessArtifactSource`."""
+ from lp.blueprints.interfaces.specification import ISpecification
from lp.bugs.interfaces.bug import IBug
from lp.code.interfaces.branch import IBranch
@@ -143,12 +149,16 @@
insert_values = []
for concrete in needed:
if IBug.providedBy(concrete):
- insert_values.append((concrete, None))
+ insert_values.append((concrete, None, None))
elif IBranch.providedBy(concrete):
- insert_values.append((None, concrete))
+ insert_values.append((None, concrete, None))
+ elif ISpecification.providedBy(concrete):
+ insert_values.append((None, None, concrete))
else:
raise ValueError("%r is not a supported artifact" % concrete)
- new = create((cls.bug, cls.branch), insert_values, get_objects=True)
+ new = create(
+ (cls.bug, cls.branch, cls.specification),
+ insert_values, get_objects=True)
return list(existing) + new
@classmethod
=== modified file 'lib/lp/registry/services/sharingservice.py'
--- lib/lp/registry/services/sharingservice.py 2012-09-05 22:02:39 +0000
+++ lib/lp/registry/services/sharingservice.py 2012-09-06 12:10:25 +0000
@@ -523,7 +523,7 @@
user, artifacts, grantee=grantee, pillar=pillar)
def ensureAccessGrants(self, grantees, user, branches=None, bugs=None,
- ignore_permissions=False):
+ specifications=None, ignore_permissions=False):
"""See `ISharingService`."""
artifacts = []
@@ -531,6 +531,8 @@
artifacts.extend(branches)
if bugs:
artifacts.extend(bugs)
+ if specifications:
+ artifacts.extend(specifications)
if not ignore_permissions:
# The user needs to have launchpad.Edit permission on all supplied
# bugs and branches or else we raise an Unauthorized exception.
=== modified file 'lib/lp/registry/services/tests/test_sharingservice.py'
--- lib/lp/registry/services/tests/test_sharingservice.py 2012-09-05 22:02:39 +0000
+++ lib/lp/registry/services/tests/test_sharingservice.py 2012-09-06 12:10:25 +0000
@@ -13,6 +13,7 @@
from zope.traversing.browser.absoluteurl import absoluteURL
from lp.app.interfaces.services import IService
+from lp.blueprints.interfaces.specification import ISpecification
from lp.bugs.interfaces.bug import IBug
from lp.code.enums import (
BranchSubscriptionNotificationLevel,
@@ -43,6 +44,7 @@
admin_logged_in,
login,
login_person,
+ person_logged_in,
StormStatementRecorder,
TestCaseWithFactory,
WebServiceTestCase,
@@ -1011,22 +1013,26 @@
ValueError, self.service.revokeAccessGrants,
product, grantee, product.owner)
- def _assert_ensureAccessGrants(self, user, bugs, branches,
+ def _assert_ensureAccessGrants(self, user, bugs, branches, specifications,
grantee=None):
# Creating access grants works as expected.
if not grantee:
grantee = self.factory.makePerson()
self.service.ensureAccessGrants(
- [grantee], user, bugs=bugs, branches=branches)
+ [grantee], user, bugs=bugs, branches=branches,
+ specifications=specifications)
# Check that grantee has expected access grants.
shared_bugs = []
shared_branches = []
+ shared_specifications = []
all_pillars = []
for bug in bugs or []:
all_pillars.extend(bug.affected_pillars)
for branch in branches or []:
all_pillars.append(branch.target.context)
+ for specification in specifications or []:
+ all_pillars.append(specification.target)
policies = getUtility(IAccessPolicySource).findByPillar(all_pillars)
apgfs = getUtility(IAccessPolicyGrantFlatSource)
@@ -1036,8 +1042,11 @@
shared_bugs.append(a.concrete_artifact)
elif IBranch.providedBy(a.concrete_artifact):
shared_branches.append(a.concrete_artifact)
+ elif ISpecification.providedBy(a.concrete_artifact):
+ shared_specifications.append(a.concrete_artifact)
self.assertContentEqual(bugs or [], shared_bugs)
self.assertContentEqual(branches or [], shared_branches)
+ self.assertContentEqual(specifications or [], shared_specifications)
def test_ensureAccessGrantsBugs(self):
# Access grants can be created for bugs.
@@ -1047,7 +1056,7 @@
bug = self.factory.makeBug(
target=distro, owner=owner,
information_type=InformationType.USERDATA)
- self._assert_ensureAccessGrants(owner, [bug], None)
+ self._assert_ensureAccessGrants(owner, [bug], None, None)
def test_ensureAccessGrantsBranches(self):
# Access grants can be created for branches.
@@ -1057,7 +1066,19 @@
branch = self.factory.makeBranch(
product=product, owner=owner,
information_type=InformationType.USERDATA)
- self._assert_ensureAccessGrants(owner, None, [branch])
+ self._assert_ensureAccessGrants(owner, None, [branch], None)
+
+ def test_ensureAccessGrantsSpecifications(self):
+ # Access grants can be created for branches.
+ owner = self.factory.makePerson()
+ product = self.factory.makeProduct(owner=owner)
+ login_person(owner)
+ specification = self.factory.makeSpecification(
+ product=product, owner=owner)
+ with person_logged_in(owner):
+ specification.transitionToInformationType(
+ InformationType.USERDATA, owner)
+ self._assert_ensureAccessGrants(owner, None, None, [specification])
def test_ensureAccessGrantsExisting(self):
# Any existing access grants are retained and new ones created.
@@ -1075,7 +1096,8 @@
self.service.ensureAccessGrants([grantee], owner, bugs=[bug])
# Test with a new bug as well as the one for which access is already
# granted.
- self._assert_ensureAccessGrants(owner, [bug, bug2], None, grantee)
+ self._assert_ensureAccessGrants(
+ owner, [bug, bug2], None, None, grantee)
def _assert_ensureAccessGrantsUnauthorized(self, user):
# ensureAccessGrants raises an Unauthorized exception if the user
=== modified file 'lib/lp/registry/tests/test_accesspolicy.py'
--- lib/lp/registry/tests/test_accesspolicy.py 2012-09-03 11:22:25 +0000
+++ lib/lp/registry/tests/test_accesspolicy.py 2012-09-06 12:10:25 +0000
@@ -289,6 +289,13 @@
return self.factory.makeBug()
+class TestAccessArtifactSpecification(BaseAccessArtifactTests,
+ TestCaseWithFactory):
+
+ def getConcreteArtifact(self):
+ return self.factory.makeSpecification()
+
+
class TestAccessArtifactGrant(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
Follow ups
