launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #13513
[Merge] lp:~adeuring/launchpad/productseries-sec-adapter into lp:launchpad
Abel Deuring has proposed merging lp:~adeuring/launchpad/productseries-sec-adapter into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~adeuring/launchpad/productseries-sec-adapter/+merge/130305
This branch adds a "sharing aware" security adapter for IProductSeries.
Serieses of private products are now only shown to persons
with policy grants for the product; the only exception are some
attributes that do not leak any "real" information: the database ID,
and the method userCanView().
Details of the change:
lp/registry/configure.zcml:
Access to most attributes and to "partial" interfaces that were public
now requires the permission launchpad.View; the permission
launchpad.AnyPerson is replaced with launchapd.AnyAllowedPerson.
(lp.services.webapp.authorization.LaunchpadSecurityPolicy.checkPermission()
has a "shortcut" for the permission launchpad.AnyPerson: no
dedicated security adapters are looked up for this permission,
so the new rule "data for serieses of private products should only
be visible for persons having a policy grant" cannot be implemented
with this permission.)
lp/security.py:
The existing class ViewProductSeries derived AnonymousAuthorization.
This does not make sense anymore, instead the class now derives
AuthorizationBase and calls the new method ProductSeries,userCanView()
for real authorization check.
The new class ChangeProductSeries does the authorization check for
the permission launchpad.AnyAllowedPerson.
lp/registry/interfaces/productseries.py
The existing interface IProductSeriesPublic now defines only the DB ID
and the method userCanView(), all other attributes are defined in
the new class IProductSeriesView.
lp/registry/model/productseries.py:
The new method userCanView(). The actual permission check is done
by IProduct.userCanView().
lp/registry/tests/test_productseries.py:
Tests for the permissions.
The test class properties expected_get_permissions and
expected_set_permissions are also intended to document which permissions
are acutally used for IProductSeries.
test:
./bin/test registry -vvt lp.registry.tests.test_productseries.ProductSeriesSecurityAdaperTestCase
no lint
--
https://code.launchpad.net/~adeuring/launchpad/productseries-sec-adapter/+merge/130305
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/productseries-sec-adapter into lp:launchpad.
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2012-10-16 14:47:54 +0000
+++ lib/lp/registry/configure.zcml 2012-10-18 09:12:23 +0000
@@ -1500,10 +1500,16 @@
<allow
interface="lp.registry.interfaces.productseries.IProductSeriesPublic"/>
<require
+ permission="launchpad.View"
+ interface="lp.registry.interfaces.productseries.IProductSeriesView"/>
+ <require
permission="launchpad.Edit"
interface="lp.registry.interfaces.productseries.IProductSeriesEditRestricted"/>
- <allow interface="lp.bugs.interfaces.bugsummary.IBugSummaryDimension"/>
- <allow
+ <require
+ permission="launchpad.View"
+ interface="lp.bugs.interfaces.bugsummary.IBugSummaryDimension"/>
+ <require
+ permission="launchpad.View"
interface="lp.bugs.interfaces.bugtarget.ISeriesBugTarget"/>
<require
permission="launchpad.Edit"
@@ -1514,16 +1520,17 @@
translations_branch status releasefileglob
translations_autoimport_mode"/>
<require
- permission="launchpad.AnyPerson"
+ permission="launchpad.AnyAllowedPerson"
set_attributes="importstatus rcstype cvsroot cvsmodule
cvstarfileurl cvsbranch svnrepository"/>
<!-- IStructuralSubscriptionTarget -->
- <allow
+ <require
+ permission="launchpad.View"
interface="lp.bugs.interfaces.structuralsubscription.IStructuralSubscriptionTargetRead" />
<require
- permission="launchpad.AnyPerson"
+ permission="launchpad.AnyAllowedPerson"
interface="lp.bugs.interfaces.structuralsubscription.IStructuralSubscriptionTargetWrite" />
</class>
=== modified file 'lib/lp/registry/interfaces/productseries.py'
--- lib/lp/registry/interfaces/productseries.py 2012-10-06 23:40:20 +0000
+++ lib/lp/registry/interfaces/productseries.py 2012-10-18 09:12:23 +0000
@@ -12,6 +12,7 @@
'IProductSeriesEditRestricted',
'IProductSeriesPublic',
'IProductSeriesSet',
+ 'IProductSeriesView',
'NoSuchProductSeries',
'ITimelineProductSeries',
]
@@ -129,13 +130,18 @@
"""Create a new milestone for this ProjectSeries."""
-class IProductSeriesPublic(
+class IProductSeriesPublic(Interface):
+ """Public IProductSeries properties."""
+ id = Int(title=_('ID'))
+
+ def userCanView(user):
+ """True if the given user has access to this product."""
+
+
+class IProductSeriesView(
ISeriesMixin, IHasAppointedDriver, IHasOwner, IBugTarget,
ISpecificationGoal, IHasMilestones, IHasOfficialBugTags, IHasExpirableBugs,
IHasTranslationImports, IHasTranslationTemplates, IServiceUsage):
- """Public IProductSeries properties."""
- id = Int(title=_('ID'))
-
product = exported(
ReferenceChoice(title=_('Project'), required=True,
vocabulary='Product', schema=Interface), # really IProduct
@@ -330,7 +336,7 @@
class IProductSeries(IProductSeriesEditRestricted, IProductSeriesPublic,
- IStructuralSubscriptionTarget):
+ IProductSeriesView, 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/model/productseries.py'
--- lib/lp/registry/model/productseries.py 2012-10-11 15:01:49 +0000
+++ lib/lp/registry/model/productseries.py 2012-10-18 09:12:23 +0000
@@ -679,6 +679,11 @@
return OrderedBugTask(3, bugtask.id, bugtask)
return weight_function
+ def userCanView(self, user):
+ """See `IproductSeriesPublic`."""
+ # Deleate the permission check to the parent product.
+ return self.product.userCanView(user)
+
class TimelineProductSeries:
"""See `ITimelineProductSeries`."""
=== modified file 'lib/lp/registry/tests/test_productseries.py'
--- lib/lp/registry/tests/test_productseries.py 2012-10-11 18:32:36 +0000
+++ lib/lp/registry/tests/test_productseries.py 2012-10-18 09:12:23 +0000
@@ -5,12 +5,20 @@
__metaclass__ = type
+from storm.exceptions import NoneError
from testtools.testcase import ExpectedException
import transaction
+from zope.security.checker import (
+ CheckerPublic,
+ getChecker,
+ )
from zope.component import getUtility
+from zope.security.interfaces import Unauthorized
from zope.security.proxy import removeSecurityProxy
from lp.app.enums import InformationType
+from lp.app.interfaces.services import IService
+from lp.registry.enums import SharingPermission
from lp.registry.errors import CannotPackageProprietaryProduct
from lp.registry.interfaces.distribution import IDistributionSet
from lp.registry.interfaces.distroseries import IDistroSeriesSet
@@ -21,7 +29,10 @@
from lp.registry.interfaces.series import SeriesStatus
from lp.services.database.lpstorm import IStore
from lp.testing import (
+ ANONYMOUS,
+ celebrity_logged_in,
login,
+ person_logged_in,
TestCaseWithFactory,
WebServiceTestCase,
)
@@ -552,3 +563,293 @@
mode = TranslationsBranchImportMode.IMPORT_TRANSLATIONS
ws_series.translations_autoimport_mode = mode.title
ws_series.lp_save()
+
+
+class ProductSeriesSecurityAdaperTestCase(TestCaseWithFactory):
+ """Test for permissions of IProductSeries."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(ProductSeriesSecurityAdaperTestCase, self).setUp()
+ self.public_product = self.factory.makeProduct()
+ self.public_series = self.factory.makeProductSeries(
+ product=self.public_product)
+ self.proprietary_product_owner = self.factory.makePerson()
+ self.proprietary_product = self.factory.makeProduct(
+ owner=self.proprietary_product_owner,
+ information_type=InformationType.PROPRIETARY)
+ self.proprietary_series = self.factory.makeProductSeries(
+ product=self.proprietary_product)
+
+ expected_get_permissions = {
+ CheckerPublic: set((
+ 'id', 'userCanView',
+ )),
+ 'launchpad.AnyAllowedPerson': set((
+ 'addBugSubscription', 'addBugSubscriptionFilter',
+ 'addSubscription', 'removeBugSubscription',
+ )),
+ 'launchpad.Edit': set(('newMilestone', )),
+ 'launchpad.View': set((
+ '_all_specifications', '_getOfficialTagClause',
+ '_valid_specifications', 'active', 'all_milestones',
+ 'answers_usage', 'blueprints_usage', 'branch',
+ 'bug_reported_acknowledgement', 'bug_reporting_guidelines',
+ 'bug_subscriptions', 'bug_supervisor', 'bug_tracking_usage',
+ 'bugtargetdisplayname', 'bugtarget_parent', 'bugtargetname',
+ 'codehosting_usage', 'createBug', 'datecreated', 'displayname',
+ 'driver', 'drivers', 'enable_bugfiling_duplicate_search',
+ 'getAllowedSpecificationInformationTypes',
+ 'getBugSummaryContextWhereClause',
+ 'getBugTaskWeightFunction', 'getCachedReleases',
+ 'getCurrentTemplatesCollection', 'getCurrentTranslationFiles',
+ 'getCurrentTranslationTemplates',
+ 'getDefaultSpecificationInformationType',
+ 'getFirstEntryToImport', 'getLatestRelease', 'getPOTemplate',
+ 'getPackage', 'getPackagingInDistribution', 'getRelease',
+ 'getSharingPartner', 'getSpecification', 'getSubscription',
+ 'getSubscriptions', 'getTemplatesAndLanguageCounts',
+ 'getTemplatesCollection', 'getTimeline',
+ 'getTranslationImportQueueEntries',
+ 'getTranslationTemplateByName', 'getTranslationTemplateFormats',
+ 'getTranslationTemplates', 'getUbuntuTranslationFocusPackage',
+ 'getUsedBugTagsWithOpenCounts',
+ 'has_current_translation_templates', 'has_milestones',
+ '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',
+ 'parent_subscription_target',
+ 'personHasDriverRights', 'pillar', 'potemplate_count', 'product',
+ 'productID', 'productserieslanguages', 'release_files',
+ 'releasefileglob', 'releases', 'releaseverstyle', 'searchTasks',
+ 'series', 'setPackaging', 'sourcepackages', 'specifications',
+ 'status', 'summary', 'target_type_display', 'title',
+ 'translations_autoimport_mode', 'userCanAlterBugSubscription',
+ 'userCanAlterSubscription', 'userHasBugSubscriptions',
+ 'translations_branch', 'translations_usage', 'uses_launchpad',
+ )),
+ }
+
+ def test_get_permissions(self):
+ checker = getChecker(self.public_series)
+ self.checkPermissions(
+ self.expected_get_permissions, checker.get_permissions, 'get')
+
+ expected_set_permissions = {
+ 'launchpad.Edit': set((
+ 'answers_usage', 'blueprints_usage', 'branch',
+ 'bug_tracking_usage', 'codehosting_usage', 'driver', 'name',
+ 'owner', 'product', 'releasefileglob', 'status', 'summary',
+ 'translations_autoimport_mode', 'translations_branch',
+ 'translations_usage', 'uses_launchpad',
+ )),
+ 'launchpad.AnyAllowedPerson': set((
+ 'cvsbranch', 'cvsmodule', 'cvsroot', 'cvstarfileurl',
+ 'importstatus', 'rcstype', 'svnrepository',
+ )),
+ }
+
+ def test_set_permissions(self):
+ checker = getChecker(self.public_series)
+ self.checkPermissions(
+ self.expected_set_permissions, checker.set_permissions, 'set')
+
+ def assertAccessAuthorized(self, attribute_names, obj):
+ # Try to access the given attributes of obj. No exception
+ # should be raised.
+ for name in attribute_names:
+ getattr(obj, name)
+
+ def assertAccessUnauthorized(self, attribute_names, obj):
+ # Try to access the given attributes of obj. Unauthorized
+ # should be raised.
+ for name in attribute_names:
+ self.assertRaises(Unauthorized, getattr, obj, name)
+
+ def assertChangeAuthorized(self, attribute_names, obj):
+ # Try to changes the given attributes of obj. Unauthorized
+ # should not be raised.
+ for name in attribute_names:
+ # Not all attributes declared in configure.zcml to be
+ # settable actually exist or are settable. Attempts to set
+ # them raise an AttributeError. Similary, the naive attempt
+ # to set an attribute to None may raise a NoneError
+ #
+ # Both errors can be ignored here: This method intends only
+ # to prove that Unauthorized is not raised.
+ try:
+ setattr(obj, name, None)
+ except (AttributeError, NoneError):
+ pass
+
+ def assertChangeUnauthorized(self, attribute_names, obj):
+ # Try to changes the given attributes of obj. Unauthorized
+ # should be raised.
+ for name in attribute_names:
+ self.assertRaises(Unauthorized, setattr, obj, name, None)
+
+ def test_access_for_anonymous(self):
+ # Anonymous users have access to public attributes of
+ # a series for a private or public product.
+ with person_logged_in(ANONYMOUS):
+ self.assertAccessAuthorized(
+ self.expected_get_permissions[CheckerPublic],
+ self.public_series)
+ self.assertAccessAuthorized(
+ self.expected_get_permissions[CheckerPublic],
+ self.proprietary_series)
+
+ # They have access to attributes requiring the permission
+ # launchpad.View of a series for a public product...
+ self.assertAccessAuthorized(
+ self.expected_get_permissions['launchpad.View'],
+ 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)
+
+ # 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'):
+ continue
+ self.assertAccessUnauthorized(names, self.public_series)
+ self.assertAccessUnauthorized(names, self.proprietary_series)
+
+ # They cannot change any attributes.
+ for permission, names in self.expected_set_permissions.items():
+ self.assertChangeUnauthorized(names, self.public_series)
+ self.assertChangeUnauthorized(names, self.proprietary_series)
+
+ def test_access_for_regular_user(self):
+ # Regular users have access to public attributes of
+ # a series for a private or public product.
+ user = self.factory.makePerson()
+ with person_logged_in(user):
+ self.assertAccessAuthorized(
+ self.expected_get_permissions[CheckerPublic],
+ self.public_series)
+ self.assertAccessAuthorized(
+ self.expected_get_permissions[CheckerPublic],
+ self.proprietary_series)
+
+ # They have access to attributes requiring the permissions
+ # launchpad.View 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.AnyAllowedPerson'],
+ 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.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',
+ 'launchpad.AnyAllowedPerson'):
+ continue
+ self.assertAccessUnauthorized(names, self.public_series)
+ self.assertAccessUnauthorized(names, self.proprietary_series)
+
+ # They can change attributes requiring the permission
+ # launchpad.AnyAllowedPerson of a series for a public project...
+ self.assertChangeAuthorized(
+ self.expected_set_permissions['launchpad.AnyAllowedPerson'],
+ self.public_series)
+ #... but not for a private project.
+ self.assertChangeUnauthorized(
+ self.expected_set_permissions['launchpad.AnyAllowedPerson'],
+ self.proprietary_series)
+
+ # They cannot change any attributes that require another
+ # permission than launchpad.AnyALlowedPerson.
+ for permission, names in self.expected_set_permissions.items():
+ if permission == 'launchpad.AnyAllowedPerson':
+ continue
+ self.assertChangeUnauthorized(names, self.public_series)
+ self.assertChangeUnauthorized(names, self.proprietary_series)
+
+ 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.
+ 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})
+ with person_logged_in(user):
+ self.assertAccessAuthorized(
+ self.expected_get_permissions['launchpad.View'],
+ self.proprietary_series)
+ self.assertAccessAuthorized(
+ self.expected_get_permissions['launchpad.AnyAllowedPerson'],
+ 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.View',
+ 'launchpad.AnyAllowedPerson'):
+ continue
+ self.assertAccessUnauthorized(names, self.proprietary_series)
+
+ # They can change attributes requiring the permission
+ # launchpad.AnyAllowedPerson of a series for a provate project...
+ self.assertChangeAuthorized(
+ self.expected_set_permissions['launchpad.AnyAllowedPerson'],
+ self.proprietary_series)
+
+ # They cannot change any attributes that require another
+ # permission than launchpad.AnyALlowedPerson.
+ for permission, names in self.expected_set_permissions.items():
+ if permission == 'launchpad.AnyAllowedPerson':
+ continue
+ 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.
+ with person_logged_in(self.proprietary_product_owner):
+ for permission, names in self.expected_get_permissions.items():
+ self.assertAccessAuthorized(names, self.proprietary_series)
+
+ # They can change all attributes.
+ for permission, names in self.expected_set_permissions.items():
+ self.assertChangeAuthorized(names, self.proprietary_series)
+
+ with person_logged_in(self.public_product.owner):
+ for permission, names in self.expected_get_permissions.items():
+ self.assertAccessAuthorized(names, self.public_series)
+
+ # They can change all attributes.
+ for permission, names in self.expected_set_permissions.items():
+ self.assertChangeAuthorized(names, self.public_series)
+
+ def test_access_for_lp_admins(self):
+ # Launchpad admins can access and change any attribute of a series
+ # of public and private product.
+ with celebrity_logged_in('admin'):
+ for permission, names in self.expected_get_permissions.items():
+ self.assertAccessAuthorized(names, self.public_series)
+ self.assertAccessAuthorized(names, self.proprietary_series)
+
+ # They can change all attributes.
+ for permission, names in self.expected_set_permissions.items():
+ self.assertChangeAuthorized(names, self.public_series)
+ self.assertChangeAuthorized(names, self.proprietary_series)
=== modified file 'lib/lp/security.py'
--- lib/lp/security.py 2012-10-17 10:11:15 +0000
+++ lib/lp/security.py 2012-10-18 09:12:23 +0000
@@ -155,6 +155,7 @@
)
from lp.registry.interfaces.productseries import (
IProductSeries,
+ IProductSeriesView,
ITimelineProductSeries,
)
from lp.registry.interfaces.projectgroup import (
@@ -1286,9 +1287,26 @@
or False)
-class ViewProductSeries(AnonymousAuthorization):
-
- usedfor = IProductSeries
+class ViewProductSeries(AuthorizationBase):
+ permission = 'launchpad.View'
+ usedfor = IProductSeriesView
+
+ def checkAuthenticated(self, user):
+ return self.obj.userCanView(user)
+
+ def checkUnauthenticated(self):
+ return self.obj.userCanView(None)
+
+
+class ChangeProductSeries(ViewProductSeries):
+ permission = 'launchpad.AnyAllowedPerson'
+ usedfor = IProductSeriesView
+
+ def checkAuthenticated(self, user):
+ return self.obj.userCanView(user)
+
+ def checkUnauthenticated(self):
+ return False
class EditProductSeries(EditByOwnersOrAdmins):
Follow ups
