launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #15182
[Merge] lp:~stevenk/launchpad/new-perm-for-archive-subscribers into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/new-perm-for-archive-subscribers into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/new-perm-for-archive-subscribers/+merge/148974
Currently, subscribers to P3As are given launchpad.View on the archive. This makes restricting things that we don't want subscribers seeing, such as Archive:+packages full of gross hacks. I have ripped those out, and added a new permission, launchpad.SubscriberView and allowed it to see Archive:+index. Archive:+packages remains as launchpad.View.
--
https://code.launchpad.net/~stevenk/launchpad/new-perm-for-archive-subscribers/+merge/148974
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/new-perm-for-archive-subscribers into lp:launchpad.
=== modified file 'lib/lp/app/browser/tales.py'
--- lib/lp/app/browser/tales.py 2013-02-06 04:22:43 +0000
+++ lib/lp/app/browser/tales.py 2013-02-18 04:04:21 +0000
@@ -778,7 +778,6 @@
return "sprite %s" % sprite_string
-
def default_logo_resource(self, context):
# XXX: mars 2008-08-22 bug=260468
# This should be refactored. We shouldn't have to do type-checking
@@ -1041,7 +1040,7 @@
if self._context.priority:
priority = self._context.priority.title.lower()
- sprite_str = sprite_str + "-%s" % priority
+ sprite_str = sprite_str + "-%s" % priority
if self._context.private:
sprite_str = sprite_str + ' private'
@@ -2602,8 +2601,9 @@
privacy = IPrivacy(view_context, None)
if privacy is None or not privacy.private:
return False
- can_view = check_permission('launchpad.View', view_context)
- return not can_view
+ if check_permission('launchpad.SubscriberView', view_context):
+ return False
+ return not check_permission('launchpad.View', view_context)
def pagetype(self):
return getattr(self.context, '__pagetype__', 'unset')
=== modified file 'lib/lp/permissions.zcml'
--- lib/lp/permissions.zcml 2012-08-20 16:38:10 +0000
+++ lib/lp/permissions.zcml 2013-02-18 04:04:21 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009 Canonical Ltd. This software is licensed under the
+<!-- Copyright 2009-2013 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
@@ -16,6 +16,8 @@
id="launchpad.LimitedView" title="View basic details like name, URL" access_level="read" />
<permission
+ id="launchpad.SubscriberView" title="View shallow details" access_level="read" />
+ <permission
id="launchpad.AnyPerson" title="Any Authenticated Person"
access_level="write" />
=== modified file 'lib/lp/security.py'
--- lib/lp/security.py 2013-01-25 04:59:23 +0000
+++ lib/lp/security.py 2013-02-18 04:04:21 +0000
@@ -118,7 +118,6 @@
from lp.registry.interfaces.irc import IIrcID
from lp.registry.interfaces.location import IPersonLocation
from lp.registry.interfaces.milestone import (
- IAbstractMilestone,
IMilestone,
IProjectGroupMilestone,
)
@@ -2469,6 +2468,18 @@
usedfor = IHWDeviceClass
+class SubscriberViewArchive(AuthorizationBase):
+ """Restrict viewing of private archives."""
+ permission = 'launchpad.SubscriberView'
+ usedfor = IArchive
+
+ def checkAuthenticated(self, user):
+ filter = get_enabled_archive_filter(
+ user.person, include_subscribed=True)
+ return not IStore(self.obj).find(
+ Archive.id, And(Archive.id == self.obj.id, filter)).is_empty()
+
+
class ViewArchive(AuthorizationBase):
"""Restrict viewing of private archives.
@@ -2498,8 +2509,7 @@
if user.inTeam(self.obj.owner):
return True
- filter = get_enabled_archive_filter(
- user.person, include_subscribed=True)
+ filter = get_enabled_archive_filter(user.person)
return not IStore(self.obj).find(
Archive.id, And(Archive.id == self.obj.id, filter)).is_empty()
=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py 2012-12-12 05:27:55 +0000
+++ lib/lp/soyuz/browser/archive.py 2013-02-18 04:04:21 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Browser views for archive."""
@@ -56,7 +56,6 @@
SimpleTerm,
SimpleVocabulary,
)
-from zope.security.interfaces import Unauthorized
from zope.security.proxy import removeSecurityProxy
from lp import _
@@ -1005,16 +1004,6 @@
"""Detailed packages view for an archive."""
implements(IArchivePackagesActionMenu)
- def initialize(self):
- super(ArchivePackagesView, self).initialize()
- if self.context.private:
- # To see the +packages page, you must be an uploader, or a
- # commercial admin.
- if not check_permission('launchpad.Append', self.context):
- admins = getUtility(ILaunchpadCelebrities).commercial_admin
- if not self.user.inTeam(admins):
- raise Unauthorized
-
@property
def page_title(self):
return smartquote('Packages in "%s"' % self.context.displayname)
=== modified file 'lib/lp/soyuz/browser/configure.zcml'
--- lib/lp/soyuz/browser/configure.zcml 2012-11-01 03:41:36 +0000
+++ lib/lp/soyuz/browser/configure.zcml 2013-02-18 04:04:21 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+<!-- Copyright 2009-2013 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
@@ -251,7 +251,7 @@
class="lp.app.browser.launchpad.Macro"/>
<browser:page
for="lp.soyuz.interfaces.archive.IArchive"
- permission="launchpad.View"
+ permission="launchpad.SubscriberView"
name="+index"
template="../templates/archive-index.pt"
class="lp.soyuz.browser.archive.ArchiveView"/>
=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml 2013-02-06 09:22:35 +0000
+++ lib/lp/soyuz/configure.zcml 2013-02-18 04:04:21 +0000
@@ -403,6 +403,9 @@
<allow
interface="lp.soyuz.interfaces.archive.IArchiveRestricted"/>
<require
+ permission="launchpad.SubscriberView"
+ interface="lp.soyuz.interfaces.archive.IArchiveSubscriberView"/>
+ <require
permission="launchpad.View"
interface="lp.soyuz.interfaces.archive.IArchiveView"/>
<require
=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py 2013-02-05 06:13:57 +0000
+++ lib/lp/soyuz/interfaces/archive.py 2013-02-18 04:04:21 +0000
@@ -22,10 +22,11 @@
'IArchiveAppend',
'IArchiveCommercial',
'IArchiveEdit',
- 'IArchiveView',
'IArchiveEditDependenciesForm',
+ 'IArchiveSubscriberView',
'IArchivePublic',
'IArchiveSet',
+ 'IArchiveView',
'IDistributionArchive',
'InsufficientUploadRights',
'InvalidComponent',
@@ -355,16 +356,109 @@
"""
-class IArchiveView(IHasBuildRecords):
- """Archive interface for operations restricted by view privilege."""
-
- title = TextLine(title=_("Name"), required=False, readonly=True)
-
+class IArchiveSubscriberView(Interface):
+
+ description = exported(
+ Text(
+ title=_("Description"), required=False,
+ description=_(
+ "A short description of the archive. URLs are allowed and "
+ "will be rendered as links.")))
+ is_active = Bool(
+ title=_("True if the archive is in the active state"),
+ required=False, readonly=True)
+ is_copy = Attribute("True if this archive is a copy archive.")
+ num_pkgs_building = Attribute(
+ "Tuple of packages building and waiting to build")
publish = Bool(
title=_("Publish"), required=False,
description=_("Whether or not to update the APT repository. If "
"disabled, nothing will be published. If the archive is "
"private then additionally no builds will be dispatched."))
+ series_with_sources = Attribute(
+ "DistroSeries to which this archive has published sources")
+ signing_key = Object(
+ title=_('Repository sigining key.'), required=False, schema=IGPGKey)
+
+ @rename_parameters_as(name="source_name", distroseries="distro_series")
+ @operation_parameters(
+ name=TextLine(title=_("Source package name"), required=False),
+ version=TextLine(title=_("Version"), required=False),
+ status=Choice(
+ title=_('Package Publishing Status'),
+ description=_('The status of this publishing record'),
+ # Really PackagePublishingStatus, circular import fixed below.
+ vocabulary=DBEnumeratedType,
+ required=False),
+ distroseries=Reference(
+ # Really IDistroSeries, fixed below to avoid circular import.
+ Interface,
+ title=_("Distroseries name"), required=False),
+ pocket=Choice(
+ title=_("Pocket"),
+ description=_("The pocket into which this entry is published"),
+ # Really PackagePublishingPocket, circular import fixed below.
+ vocabulary=DBEnumeratedType,
+ required=False, readonly=True),
+ exact_match=Bool(
+ title=_("Exact Match"),
+ description=_("Whether or not to filter source names by exact"
+ " matching."),
+ required=False),
+ created_since_date=Datetime(
+ title=_("Created Since Date"),
+ description=_("Return entries whose `date_created` is greater "
+ "than or equal to this date."),
+ required=False),
+ component_name=TextLine(title=_("Component name"), required=False),
+ )
+ # Really returns ISourcePackagePublishingHistory, see below for
+ # patch to avoid circular import.
+ @call_with(eager_load=True)
+ @operation_returns_collection_of(Interface)
+ @export_read_operation()
+ def getPublishedSources(name=None, version=None, status=None,
+ distroseries=None, pocket=None,
+ exact_match=False, created_since_date=None,
+ eager_load=False, component_name=None):
+ """All `ISourcePackagePublishingHistory` target to this archive.
+
+ :param name: source name filter (exact match or SQL LIKE controlled
+ by 'exact_match' argument).
+ Name can be a single string or a list of strings.
+ :param version: source version filter (always exact match).
+ :param status: `PackagePublishingStatus` filter, can be a sequence.
+ :param distroseries: `IDistroSeries` filter.
+ :param pocket: `PackagePublishingPocket` filter. This may be an
+ iterable of more than one pocket or a single pocket.
+ :param exact_match: either or not filter source names by exact
+ matching.
+ :param created_since_date: Only return results whose `date_created`
+ is greater than or equal to this date.
+ :param component_name: component filter. Only return source packages
+ that are in this component.
+
+ :return: SelectResults containing `ISourcePackagePublishingHistory`,
+ ordered by name. If there are multiple results for the same
+ name then they are sub-ordered newest first.
+ """
+
+ def newAuthToken(person, token=None, date_created=None):
+ """Create a new authorisation token.
+
+ :param person: An IPerson whom this token is for
+ :param token: Optional unicode text to use as the token. One will be
+ generated if not given
+ :param date_created: Optional, defaults to now
+
+ :return: A new IArchiveAuthToken
+ """
+
+
+class IArchiveView(IHasBuildRecords):
+ """Archive interface for operations restricted by view privilege."""
+
+ title = TextLine(title=_("Name"), required=False, readonly=True)
require_virtualized = exported(
Bool(
@@ -402,9 +496,6 @@
"Concatenation of the source and binary packages published in this "
"archive. Its content is used for indexed searches across archives.")
- signing_key = Object(
- title=_('Repository sigining key.'), required=False, schema=IGPGKey)
-
debug_archive = Attribute(
"The archive into which debug binaries should be uploaded.")
@@ -418,14 +509,6 @@
is_partner = Attribute("True if this archive is a partner archive.")
- is_copy = Attribute("True if this archive is a copy archive.")
-
- is_active = Bool(
- title=_("True if the archive is in the active state"),
- required=False, readonly=True)
-
- series_with_sources = Attribute(
- "DistroSeries to which this archive has published sources")
number_of_sources = Attribute(
'The number of sources published in the context archive.')
number_of_binaries = Attribute(
@@ -912,9 +995,6 @@
:return: True if the person is allowed to upload the source package.
"""
- num_pkgs_building = Attribute(
- "Tuple of packages building and waiting to build")
-
def updatePackageDownloadCount(bpr, day, country, count):
"""Update the daily download count for a given package.
@@ -949,13 +1029,6 @@
# Really IArchiveDependency
readonly=True))
- description = exported(
- Text(
- title=_("Description"), required=False,
- description=_(
- "A short description of the archive. URLs are allowed and "
- "will be rendered as links.")))
-
signing_key_fingerprint = exported(
Text(
title=_("Archive signing key fingerprint"), required=False,
@@ -963,69 +1036,6 @@
"for this PPA or None if there is no signing "
"key available.")))
- @rename_parameters_as(name="source_name", distroseries="distro_series")
- @operation_parameters(
- name=TextLine(title=_("Source package name"), required=False),
- version=TextLine(title=_("Version"), required=False),
- status=Choice(
- title=_('Package Publishing Status'),
- description=_('The status of this publishing record'),
- # Really PackagePublishingStatus, circular import fixed below.
- vocabulary=DBEnumeratedType,
- required=False),
- distroseries=Reference(
- # Really IDistroSeries, fixed below to avoid circular import.
- Interface,
- title=_("Distroseries name"), required=False),
- pocket=Choice(
- title=_("Pocket"),
- description=_("The pocket into which this entry is published"),
- # Really PackagePublishingPocket, circular import fixed below.
- vocabulary=DBEnumeratedType,
- required=False, readonly=True),
- exact_match=Bool(
- title=_("Exact Match"),
- description=_("Whether or not to filter source names by exact"
- " matching."),
- required=False),
- created_since_date=Datetime(
- title=_("Created Since Date"),
- description=_("Return entries whose `date_created` is greater "
- "than or equal to this date."),
- required=False),
- component_name=TextLine(title=_("Component name"), required=False),
- )
- # Really returns ISourcePackagePublishingHistory, see below for
- # patch to avoid circular import.
- @call_with(eager_load=True)
- @operation_returns_collection_of(Interface)
- @export_read_operation()
- def getPublishedSources(name=None, version=None, status=None,
- distroseries=None, pocket=None,
- exact_match=False, created_since_date=None,
- eager_load=False, component_name=None):
- """All `ISourcePackagePublishingHistory` target to this archive.
-
- :param name: source name filter (exact match or SQL LIKE controlled
- by 'exact_match' argument).
- Name can be a single string or a list of strings.
- :param version: source version filter (always exact match).
- :param status: `PackagePublishingStatus` filter, can be a sequence.
- :param distroseries: `IDistroSeries` filter.
- :param pocket: `PackagePublishingPocket` filter. This may be an
- iterable of more than one pocket or a single pocket.
- :param exact_match: either or not filter source names by exact
- matching.
- :param created_since_date: Only return results whose `date_created`
- is greater than or equal to this date.
- :param component_name: component filter. Only return source packages
- that are in this component.
-
- :return: SelectResults containing `ISourcePackagePublishingHistory`,
- ordered by name. If there are multiple results for the same
- name then they are sub-ordered newest first.
- """
-
@rename_parameters_as(
name="binary_name", distroarchseries="distro_arch_series")
@operation_parameters(
@@ -1305,17 +1315,6 @@
:return: A IArchiveAuthToken, or None if the user has none.
"""
- def newAuthToken(person, token=None, date_created=None):
- """Create a new authorisation token.
-
- :param person: An IPerson whom this token is for
- :param token: Optional unicode text to use as the token. One will be
- generated if not given
- :param date_created: Optional, defaults to now
-
- :return: A new IArchiveAuthToken
- """
-
@call_with(person=REQUEST_USER)
@operation_parameters(
source_name=TextLine(title=_("Source package name")),
@@ -1938,8 +1937,9 @@
"with a higher score will build sooner.")))
-class IArchive(IArchivePublic, IArchiveAppend, IArchiveEdit, IArchiveView,
- IArchiveCommercial, IArchiveRestricted):
+class IArchive(IArchivePublic, IArchiveAppend, IArchiveEdit,
+ IArchiveSubscriberView, IArchiveView, IArchiveCommercial,
+ IArchiveRestricted):
"""Main Archive interface."""
export_as_webservice_entry()
=== modified file 'lib/lp/soyuz/tests/test_archive_subscriptions.py'
--- lib/lp/soyuz/tests/test_archive_subscriptions.py 2012-05-26 21:20:16 +0000
+++ lib/lp/soyuz/tests/test_archive_subscriptions.py 2013-02-18 04:04:21 +0000
@@ -1,8 +1,10 @@
-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test Archive features."""
+from urlparse import urljoin
+
from zope.security.interfaces import Unauthorized
from lp.registry.interfaces.person import PersonVisibility
@@ -85,12 +87,6 @@
self.archive, '+index', principal=self.subscriber)
self.assertIn(self.archive.displayname, view.render())
- # Just to double check, by default, the subscriber still can't see the
- # +packages view which requires extra permissions.
- self.assertRaises(
- Unauthorized, create_initialized_view,
- self.archive, '+packages', principal=self.subscriber)
-
def test_new_subscription_sends_email(self):
# Creating a new subscription sends an email to all members
# of the person or team subscribed.
@@ -146,3 +142,12 @@
self.assertIsNotNone(find_tag_by_id(content, 'ppa-install'))
self.assertIsNotNone(
find_tag_by_id(content, 'portlet-latest-updates'))
+
+ def test_unauthorized_subscriber_for_plus_packages(self):
+ with person_logged_in(self.owner):
+ self.archive.newSubscription(
+ self.subscriber, registrant=self.archive.owner)
+ with person_logged_in(self.subscriber):
+ url = urljoin(canonical_url(self.archive), '+packages')
+ browser = setupBrowserForUser(self.subscriber)
+ self.assertRaises(Unauthorized, browser.open, url)
Follow ups
