launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #14713
[Merge] lp:~adeuring/launchpad/bug-1086043 into lp:launchpad
Abel Deuring has proposed merging lp:~adeuring/launchpad/bug-1086043 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~adeuring/launchpad/bug-1086043/+merge/137882
This branch fixes 1086043: https://bugs.launchpad.net/~ broken
if the user subscribed to a bug with a task for a series of a
private project.
The cause of this bug is obvious: A few properties of a product
series and a milestone are displayed in bug listings, so the current
user needs to have the permission to access these properties, even
it they are allowed to view all details about the series/milestone.
In other words: These properties should be proected by the permission
lp.LimitedView, not lp.View. (Before we noicticed this bug, I would
have sworn that we already implemented this...)
Implementation details:
I added the new test class TestPersonBugListing, which adds
tests to render a person's bug page when the user is subscribed to
a bug related to a private project.
test_grant_for_bug_with_task_for_private_product() passes without further
changes, but the two other tests (where a bug task for a product series
exists, or the bug task is linked to a milestone) needed a few more
changes:
- registry/configure.zcml now requires the permission
lp.LimitedView for some properties of IProductSeries and IMilestone;
- new interface class IProductSeriesLimitedView which defines the
attributes that are protected by lp.LimitedView. (There no
corresponding change for IMilestone, because the permissions
are specified attribute-by-attribute in configure.zcml)
- new security adapters for lp.LimitedView and IProductSeries/IMilestone
- permision and access tests for IMilestone/IProductSeries updated/
extended.
tests:
./bin/test registry -vvt test_productseries.ProductSeriesSecurityAdaperTestCase
./bin/test bugs -vvt lp.bugs.browser.tests.test_buglisting.TestPersonBugListing.test_grant
./bin/test registry -vvt test_milestone.MilestoneSecurityAdaperTestCase
- tests that a person bug page can be rendered with an AAG bug with ersies
target and milestone
- permission lp.LimiedView for IMilestone and IProductSeries
no lint
--
https://code.launchpad.net/~adeuring/launchpad/bug-1086043/+merge/137882
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/bug-1086043 into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/tests/test_buglisting.py'
--- lib/lp/bugs/browser/tests/test_buglisting.py 2012-10-08 07:36:45 +0000
+++ lib/lp/bugs/browser/tests/test_buglisting.py 2012-12-04 15:43:24 +0000
@@ -11,6 +11,7 @@
)
from zope.component import getUtility
+from lp.app.enums import InformationType
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.services.features.testing import FeatureFixture
from lp.services.webapp.publisher import canonical_url
@@ -434,3 +435,55 @@
def _getBugTarget(self, dsp):
"""Return the current `ISourcePackage` for the dsp."""
return dsp.development_version
+
+
+class TestPersonBugListing(BrowserTestCase):
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestPersonBugListing, self).setUp()
+ self.user = self.factory.makePerson()
+ self.private_product_owner = self.factory.makePerson()
+ self.private_product = self.factory.makeProduct(
+ owner=self.private_product_owner,
+ information_type=InformationType.PROPRIETARY)
+
+ def test_grant_for_bug_with_task_for_private_product(self):
+ # A person's own bug page is correctly rendered when the person
+ # is subscribed to a bug with a task for a propritary product.
+ with person_logged_in(self.private_product_owner):
+ bug = self.factory.makeBug(
+ target=self.private_product, owner=self.private_product_owner)
+ bug.subscribe(self.user, subscribed_by=self.private_product_owner)
+ url = canonical_url(self.user, rootsite='bugs')
+ # Just ensure that no exception occurs when the page is rendered.
+ self.getUserBrowser(url, user=self.user)
+
+ def test_grant_for_bug_with_task_for_private_product_series(self):
+ # A person's own bug page is correctly rendered when the person
+ # is subscribed to a bug with a task for a propritary product series.
+ with person_logged_in(self.private_product_owner):
+ series = self.factory.makeProductSeries(
+ product=self.private_product)
+ bug = self.factory.makeBug(
+ target=self.private_product, series=series,
+ owner=self.private_product_owner)
+ bug.subscribe(self.user, subscribed_by=self.private_product_owner)
+ url = canonical_url(self.user, rootsite='bugs')
+ # Just ensure that no exception occurs when the page is rendered.
+ self.getUserBrowser(url, user=self.user)
+
+ def test_grant_for_bug_with_task_for_private_product_and_milestone(self):
+ # A person's own bug page is correctly rendered when the person
+ # is subscribed to a bug with a task for a propritary product and
+ # a milestone.
+ with person_logged_in(self.private_product_owner):
+ milestone = self.factory.makeMilestone(
+ product=self.private_product)
+ bug = self.factory.makeBug(
+ target=self.private_product, milestone=milestone,
+ owner=self.private_product_owner)
+ bug.subscribe(self.user, subscribed_by=self.private_product_owner)
+ url = canonical_url(self.user, rootsite='bugs')
+ # Just ensure that no exception occurs when the page is rendered.
+ self.getUserBrowser(url, user=self.user)
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2012-11-27 22:02:34 +0000
+++ lib/lp/registry/configure.zcml 2012-12-04 15:43:24 +0000
@@ -1035,20 +1035,24 @@
bugtasks
code_name
dateexpected
- displayname
distribution
distroseries
getSpecifications
getTags
getTagsData
- name
product
product_release
productseries
series_target
summary
+ title
+ "/>
+ <require
+ permission="launchpad.LimitedView"
+ attributes="
+ displayname
+ name
target
- title
"/>
<require
permission="launchpad.Edit"
@@ -1565,6 +1569,9 @@
<allow
interface="lp.registry.interfaces.productseries.IProductSeriesPublic"/>
<require
+ permission="launchpad.LimitedView"
+ interface="lp.registry.interfaces.productseries.IProductSeriesLimitedView"/>
+ <require
permission="launchpad.View"
interface="lp.registry.interfaces.productseries.IProductSeriesView"/>
<require
=== modified file 'lib/lp/registry/interfaces/productseries.py'
--- lib/lp/registry/interfaces/productseries.py 2012-10-18 14:05:41 +0000
+++ lib/lp/registry/interfaces/productseries.py 2012-12-04 15:43:24 +0000
@@ -10,6 +10,7 @@
__all__ = [
'IProductSeries',
'IProductSeriesEditRestricted',
+ 'IProductSeriesLimitedView',
'IProductSeriesPublic',
'IProductSeriesSet',
'IProductSeriesView',
@@ -138,16 +139,29 @@
"""True if the given user has access to this product."""
-class IProductSeriesView(
- ISeriesMixin, IHasAppointedDriver, IHasOwner, IBugTarget,
- ISpecificationGoal, IHasMilestones, IHasOfficialBugTags, IHasExpirableBugs,
- IHasTranslationImports, IHasTranslationTemplates, IServiceUsage):
+class IProductSeriesLimitedView(Interface):
+
+ name = exported(
+ ProductSeriesNameField(
+ title=_('Name'),
+ description=_(
+ "The name of the series is a short, unique name "
+ "that identifies it, being used in URLs. It must be all "
+ "lowercase, with no special characters. For example, '2.0' "
+ "or 'trunk'."),
+ constraint=name_validator))
+
product = exported(
ReferenceChoice(title=_('Project'), required=True,
vocabulary='Product', schema=Interface), # really IProduct
exported_as='project')
productID = Attribute('The product ID.')
+
+class IProductSeriesView(
+ ISeriesMixin, IHasAppointedDriver, IHasOwner, IBugTarget,
+ ISpecificationGoal, IHasMilestones, IHasOfficialBugTags, IHasExpirableBugs,
+ IHasTranslationImports, IHasTranslationTemplates, IServiceUsage):
status = exported(
Choice(
title=_('Status'), required=True, vocabulary=SeriesStatus,
@@ -155,16 +169,6 @@
parent = Attribute('The structural parent of this series - the product')
- name = exported(
- ProductSeriesNameField(
- title=_('Name'),
- description=_(
- "The name of the series is a short, unique name "
- "that identifies it, being used in URLs. It must be all "
- "lowercase, with no special characters. For example, '2.0' "
- "or 'trunk'."),
- constraint=name_validator))
-
datecreated = exported(
Datetime(title=_('Date Registered'),
required=True,
@@ -336,7 +340,8 @@
class IProductSeries(IProductSeriesEditRestricted, IProductSeriesPublic,
- IProductSeriesView, IStructuralSubscriptionTarget):
+ IProductSeriesView, IProductSeriesLimitedView,
+ IStructuralSubscriptionTarget):
"""A series of releases. For example '2.0' or '1.3' or 'dev'."""
export_as_webservice_entry('project_series')
=== modified file 'lib/lp/registry/tests/test_milestone.py'
--- lib/lp/registry/tests/test_milestone.py 2012-11-26 08:33:03 +0000
+++ lib/lp/registry/tests/test_milestone.py 2012-12-04 15:43:24 +0000
@@ -21,7 +21,6 @@
from lp.app.interfaces.informationtype import IInformationType
from lp.app.interfaces.services import IService
from lp.registry.enums import (
- BugSharingPolicy,
SharingPermission,
SpecificationSharingPolicy,
)
@@ -137,16 +136,17 @@
'id', 'checkAuthenticated', 'checkUnauthenticated',
'userCanView',
)),
+ 'launchpad.LimitedView': set(('displayname', 'name', 'target', )),
'launchpad.View': set((
'active', 'bug_subscriptions', 'bugtasks', 'code_name',
- 'dateexpected', 'displayname', 'distribution', 'distroseries',
+ 'dateexpected', 'distribution', 'distroseries',
'_getOfficialTagClause', 'getBugSummaryContextWhereClause',
'getBugTaskWeightFunction', 'getSpecifications',
'getSubscription', 'getSubscriptions', 'getTags', 'getTagsData',
- 'getUsedBugTagsWithOpenCounts', 'name', 'official_bug_tags',
+ 'getUsedBugTagsWithOpenCounts', 'official_bug_tags',
'parent_subscription_target', 'product', 'product_release',
'productseries', 'searchTasks', 'series_target',
- 'summary', 'target', 'target_type_display', 'title',
+ 'summary', 'target_type_display', 'title',
'userCanAlterBugSubscription', 'userCanAlterSubscription',
'userHasBugSubscriptions',
)),
@@ -232,20 +232,28 @@
self.proprietary_milestone)
# They have access to attributes requiring the permission
- # launchpad.View of milestones for public products...
+ # launchpad.View or launchpad.View of milestones for public
+ # products...
self.assertAccessAuthorized(
self.expected_get_permissions['launchpad.View'],
self.public_milestone)
+ self.assertAccessAuthorized(
+ self.expected_get_permissions['launchpad.LimitedView'],
+ self.public_milestone)
# ...but not to the same attributes of milestones for private
# products.
self.assertAccessUnauthorized(
self.expected_get_permissions['launchpad.View'],
self.proprietary_milestone)
+ self.assertAccessUnauthorized(
+ self.expected_get_permissions['launchpad.LimitedView'],
+ self.proprietary_milestone)
# They cannot access other attributes.
for permission, names in self.expected_get_permissions.items():
- if permission in (CheckerPublic, 'launchpad.View'):
+ if permission in (CheckerPublic, 'launchpad.View',
+ 'launchpad.LimitedView'):
continue
self.assertAccessUnauthorized(names, self.public_milestone)
self.assertAccessUnauthorized(
@@ -270,12 +278,16 @@
self.proprietary_milestone)
# They have access to attributes requiring the permission
- # launchpad.View or launchpad.AnyAllowedPerson of milestones
- # for public products...
+ # launchpad.View, launchpad.LimitedView or
+ # launchpad.AnyAllowedPerson of milestones for public
+ # products...
self.assertAccessAuthorized(
self.expected_get_permissions['launchpad.View'],
self.public_milestone)
self.assertAccessAuthorized(
+ self.expected_get_permissions['launchpad.LimitedView'],
+ self.public_milestone)
+ self.assertAccessAuthorized(
self.expected_get_permissions['launchpad.AnyAllowedPerson'],
self.public_milestone)
@@ -285,13 +297,16 @@
self.expected_get_permissions['launchpad.View'],
self.proprietary_milestone)
self.assertAccessUnauthorized(
+ self.expected_get_permissions['launchpad.LimitedView'],
+ self.proprietary_milestone)
+ self.assertAccessUnauthorized(
self.expected_get_permissions['launchpad.AnyAllowedPerson'],
self.proprietary_milestone)
# They cannot access other attributes.
for permission, names in self.expected_get_permissions.items():
if permission in (
- CheckerPublic, 'launchpad.View',
+ CheckerPublic, 'launchpad.View', 'launchpad.LimitedView',
'launchpad.AnyAllowedPerson'):
continue
self.assertAccessUnauthorized(names, self.public_milestone)
@@ -309,6 +324,42 @@
# to most attributes of the private product.
user = self.factory.makePerson()
with person_logged_in(self.proprietary_product_owner):
+ bug = self.factory.makeBug(
+ target=self.proprietary_product,
+ owner=self.proprietary_product_owner)
+ bug.subscribe(user, subscribed_by=self.proprietary_product_owner)
+
+ with person_logged_in(user):
+ self.assertAccessAuthorized(
+ self.expected_get_permissions[CheckerPublic],
+ self.proprietary_milestone)
+
+ # They have access to attributes requiring the permission
+ # launchpad.LimitedView of milestones for the private
+ # product.
+ self.assertAccessAuthorized(
+ self.expected_get_permissions['launchpad.LimitedView'],
+ self.proprietary_milestone)
+
+ # They cannot access other attributes.
+ for permission, names in self.expected_get_permissions.items():
+ if permission in (
+ CheckerPublic, 'launchpad.LimitedView'):
+ continue
+ self.assertAccessUnauthorized(
+ names, self.proprietary_milestone)
+
+ # They cannot change attributes.
+ for names in self.expected_set_permissions.values():
+ self.assertChangeUnauthorized(
+ names, self.proprietary_milestone)
+
+ def test_access_for_user_with_artifact_grant_for_private_product(self):
+ # Users with an artifact grant for a private product have access
+ # to attributes requiring the permission launchpad.LimitedView of
+ # milestones for the private product.
+ user = self.factory.makePerson()
+ with person_logged_in(self.proprietary_product_owner):
getUtility(IService, 'sharing').sharePillarInformation(
self.proprietary_product, user, self.proprietary_product_owner,
{InformationType.PROPRIETARY: SharingPermission.ALL})
@@ -319,19 +370,23 @@
self.proprietary_milestone)
# They have access to attributes requiring the permission
- # launchpad.View or launchpad.AnyAllowedPerson of milestones
- # for the private product.
+ # launchpad.View, launchpad.LimitedView or
+ # launchpad.AnyAllowedPerson of milestones for the private
+ # product.
self.assertAccessAuthorized(
self.expected_get_permissions['launchpad.View'],
self.proprietary_milestone)
self.assertAccessAuthorized(
+ self.expected_get_permissions['launchpad.LimitedView'],
+ self.proprietary_milestone)
+ self.assertAccessAuthorized(
self.expected_get_permissions['launchpad.AnyAllowedPerson'],
self.proprietary_milestone)
# They cannot access other attributes.
for permission, names in self.expected_get_permissions.items():
if permission in (
- CheckerPublic, 'launchpad.View',
+ CheckerPublic, 'launchpad.View', 'launchpad.LimitedView',
'launchpad.AnyAllowedPerson'):
continue
self.assertAccessUnauthorized(
=== modified file 'lib/lp/registry/tests/test_productseries.py'
--- lib/lp/registry/tests/test_productseries.py 2012-11-30 20:52:15 +0000
+++ lib/lp/registry/tests/test_productseries.py 2012-12-04 15:43:24 +0000
@@ -622,6 +622,7 @@
'addSubscription', 'removeBugSubscription',
)),
'launchpad.Edit': set(('newMilestone', )),
+ 'launchpad.LimitedView': set(('name', 'product', 'productID')),
'launchpad.View': set((
'_all_specifications', '_getOfficialTagClause',
'_valid_specifications', 'active', 'all_milestones',
@@ -650,10 +651,10 @@
'has_obsolete_translation_templates',
'has_sharing_translation_templates', 'has_translation_files',
'has_translation_templates', 'is_development_focus', 'milestones',
- 'name', 'official_bug_tags', 'owner', 'packagings', 'parent',
+ 'official_bug_tags', 'owner', 'packagings', 'parent',
'parent_subscription_target',
- 'personHasDriverRights', 'pillar', 'potemplate_count', 'product',
- 'productID', 'productserieslanguages', 'release_files',
+ 'personHasDriverRights', 'pillar', 'potemplate_count',
+ 'productserieslanguages', 'release_files',
'releasefileglob', 'releases', 'releaseverstyle', 'searchTasks',
'series', 'setPackaging', 'sourcepackages', 'specifications',
'status', 'summary', 'target_type_display', 'title',
@@ -733,21 +734,29 @@
self.proprietary_series)
# They have access to attributes requiring the permission
- # launchpad.View of a series for a public product...
+ # launchpad.Viewand launchpad.LimitedView of a series for a
+ # public product...
self.assertAccessAuthorized(
self.expected_get_permissions['launchpad.View'],
self.public_series)
+ self.assertAccessAuthorized(
+ self.expected_get_permissions['launchpad.LimitedView'],
+ self.public_series)
# ...but not to the same attributes of a series for s private
# product.
self.assertAccessUnauthorized(
self.expected_get_permissions['launchpad.View'],
self.proprietary_series)
+ self.assertAccessUnauthorized(
+ self.expected_get_permissions['launchpad.LimitedView'],
+ self.proprietary_series)
# The cannot access other attributes of a series for
# public and private products.
for permission, names in self.expected_get_permissions.items():
- if permission in (CheckerPublic, 'launchpad.View'):
+ if permission in (CheckerPublic, 'launchpad.LimitedView',
+ 'launchpad.View'):
continue
self.assertAccessUnauthorized(names, self.public_series)
self.assertAccessUnauthorized(names, self.proprietary_series)
@@ -770,12 +779,15 @@
self.proprietary_series)
# They have access to attributes requiring the permissions
- # launchpad.View and launchpadAnyAllowedPerson of a series
- # for a public product...
+ # launchpad.View, launchpad.LimitedView and
+ # launchpadAnyAllowedPerson of a series for a public product...
self.assertAccessAuthorized(
self.expected_get_permissions['launchpad.View'],
self.public_series)
self.assertAccessAuthorized(
+ self.expected_get_permissions['launchpad.LimitedView'],
+ self.public_series)
+ self.assertAccessAuthorized(
self.expected_get_permissions['launchpad.AnyAllowedPerson'],
self.public_series)
@@ -785,13 +797,17 @@
self.expected_get_permissions['launchpad.View'],
self.proprietary_series)
self.assertAccessUnauthorized(
+ self.expected_get_permissions['launchpad.LimitedView'],
+ self.proprietary_series)
+ self.assertAccessUnauthorized(
self.expected_get_permissions['launchpad.AnyAllowedPerson'],
self.proprietary_series)
# The cannot access other attributes of a series for
# public and private products.
for permission, names in self.expected_get_permissions.items():
- if permission in (CheckerPublic, 'launchpad.View',
+ if permission in (CheckerPublic, 'launchpad.LimitedView',
+ 'launchpad.View',
'launchpad.AnyAllowedPerson'):
continue
self.assertAccessUnauthorized(names, self.public_series)
@@ -817,8 +833,8 @@
def test_access_for_user_with_policy_grant(self):
# Users with a policy grant for the parent product can access
- # properties requring the permission lanchpad.View or
- # launchpad.ANyALlowedPerson of a series.
+ # properties requring the permission launchpad.LimitedView,
+ # launchpad.View or launchpad.AnyALlowedPerson of a series.
user = self.factory.makePerson()
with person_logged_in(self.proprietary_product_owner):
getUtility(IService, 'sharing').sharePillarInformation(
@@ -826,6 +842,9 @@
{InformationType.PROPRIETARY: SharingPermission.ALL})
with person_logged_in(user):
self.assertAccessAuthorized(
+ self.expected_get_permissions['launchpad.LimitedView'],
+ self.proprietary_series)
+ self.assertAccessAuthorized(
self.expected_get_permissions['launchpad.View'],
self.proprietary_series)
self.assertAccessAuthorized(
@@ -836,6 +855,7 @@
# private products.
for permission, names in self.expected_get_permissions.items():
if permission in (CheckerPublic, 'launchpad.View',
+ 'launchpad.LimitedView',
'launchpad.AnyAllowedPerson'):
continue
self.assertAccessUnauthorized(names, self.proprietary_series)
@@ -853,6 +873,33 @@
continue
self.assertChangeUnauthorized(names, self.proprietary_series)
+ def test_access_for_user_with_artifact_grant(self):
+ # Users with an artifact grant related to the parent product
+ # can access properties requring the permission launchpad.LimitedView
+ # of a series.
+ user = self.factory.makePerson()
+ with person_logged_in(self.proprietary_product_owner):
+ bug = self.factory.makeBug(
+ target=self.proprietary_product,
+ owner=self.proprietary_product_owner)
+ bug.subscribe(user, subscribed_by=self.proprietary_product_owner)
+
+ with person_logged_in(user):
+ self.assertAccessAuthorized(
+ self.expected_get_permissions['launchpad.LimitedView'],
+ self.proprietary_series)
+
+ # The cannot access other attributes of a series for
+ # private products.
+ for permission, names in self.expected_get_permissions.items():
+ if permission in (CheckerPublic, 'launchpad.LimitedView'):
+ continue
+ self.assertAccessUnauthorized(names, self.proprietary_series)
+
+ # They cannot change any attributes.
+ for permission, names in self.expected_set_permissions.items():
+ self.assertChangeUnauthorized(names, self.proprietary_series)
+
def test_access_for_product_owner(self):
# The owner of a project has access to all attributes of
# a product series.
=== modified file 'lib/lp/security.py'
--- lib/lp/security.py 2012-11-27 22:02:34 +0000
+++ lib/lp/security.py 2012-12-04 15:43:24 +0000
@@ -156,6 +156,7 @@
)
from lp.registry.interfaces.productseries import (
IProductSeries,
+ IProductSeriesLimitedView,
IProductSeriesView,
ITimelineProductSeries,
)
@@ -760,6 +761,15 @@
return False
+class LimitedViewMilestone(DelegatedAuthorization):
+ permission = 'launchpad.LimitedView'
+ usedfor = IMilestone
+
+ def __init__(self, obj):
+ super(LimitedViewMilestone, self).__init__(
+ obj, obj.target, 'launchpad.LimitedView')
+
+
class ViewMilestone(AuthorizationBase):
permission = 'launchpad.View'
usedfor = IMilestone
@@ -1319,6 +1329,15 @@
or False)
+class LimitedViewProductSeries(DelegatedAuthorization):
+ permission = 'launchpad.LimitedView'
+ usedfor = IProductSeriesLimitedView
+
+ def __init__(self, obj):
+ super(LimitedViewProductSeries, self).__init__(
+ obj, obj.product, 'launchpad.LimitedView')
+
+
class ViewProductSeries(AuthorizationBase):
permission = 'launchpad.View'
usedfor = IProductSeriesView
Follow ups
