launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29174
[Merge] ~andrey-fedoseev/launchpad:packaging-security into launchpad:master
Andrey Fedoseev has proposed merging ~andrey-fedoseev/launchpad:packaging-security into launchpad:master.
Commit message:
New security model for `Packaging`
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~andrey-fedoseev/launchpad/+git/launchpad/+merge/429758
Only the product owners, package maintainers, admins and registry experts are now allowed to create, edit or remove package links
- {IProductSeries,ISourcePackage}.setPackaging is now protected with `launchpad.Edit` permission
- `launchpad.Edit` on `IPackaging`: True if user has `launchpad.Edit` on either corresponding `IProductSeries` or `ISourcePackage`
- /+ubuntupkg on `IProductSeries` is now protected with `launchpad.Edit`
- /+edit-packaging on `ISourcePackage` is now protected with `launchpad.Edit`
- /+remove-packaging on `ISourcePackage` is protected with `launchpad.Edit` applied to `sourcepackage.direct_packaging`; this allows product owners to unlink their products
- all tests are updated accordingly
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~andrey-fedoseev/launchpad:packaging-security into launchpad:master.
diff --git a/lib/lp/registry/browser/configure.zcml b/lib/lp/registry/browser/configure.zcml
index 6f041d4..b90507b 100644
--- a/lib/lp/registry/browser/configure.zcml
+++ b/lib/lp/registry/browser/configure.zcml
@@ -2022,7 +2022,7 @@
name="+ubuntupkg"
for="lp.registry.interfaces.productseries.IProductSeries"
class="lp.registry.browser.productseries.ProductSeriesUbuntuPackagingView"
- permission="launchpad.AnyPerson"
+ permission="launchpad.Edit"
template="../templates/productseries-ubuntupkg.pt"
/>
<browser:pages
@@ -2621,7 +2621,7 @@
name="+edit-packaging"
for="lp.registry.interfaces.sourcepackage.ISourcePackage"
class="lp.registry.browser.sourcepackage.SourcePackageChangeUpstreamView"
- permission="launchpad.AnyPerson"
+ permission="launchpad.Edit"
template="../templates/sourcepackage-edit-packaging.pt"
/>
<browser:page
diff --git a/lib/lp/registry/browser/productseries.py b/lib/lp/registry/browser/productseries.py
index 8759e95..9b05317 100644
--- a/lib/lp/registry/browser/productseries.py
+++ b/lib/lp/registry/browser/productseries.py
@@ -278,7 +278,7 @@ class ProductSeriesOverviewMenu(
summary = "Change the branch for this series"
return Link("+setbranch", text, summary, icon=icon)
- @enabled_with_permission("launchpad.AnyPerson")
+ @enabled_with_permission("launchpad.Edit")
def ubuntupkg(self):
"""Return a link to link this series to an ubuntu sourcepackage."""
text = "Link to Ubuntu package"
diff --git a/lib/lp/registry/browser/sourcepackage.py b/lib/lp/registry/browser/sourcepackage.py
index 3230cf4..461a963 100644
--- a/lib/lp/registry/browser/sourcepackage.py
+++ b/lib/lp/registry/browser/sourcepackage.py
@@ -34,6 +34,7 @@ from zope.schema.vocabulary import (
SimpleVocabulary,
getVocabularyRegistry,
)
+from zope.security.interfaces import Unauthorized
from lp import _
from lp.app.browser.launchpadform import (
@@ -61,9 +62,11 @@ from lp.services.webapp import (
canonical_url,
stepto,
)
+from lp.services.webapp.authorization import check_permission
from lp.services.webapp.breadcrumb import Breadcrumb
from lp.services.webapp.escaping import structured
from lp.services.webapp.interfaces import IBreadcrumb
+from lp.services.webapp.menu import enabled_with_permission
from lp.services.webapp.publisher import LaunchpadView
from lp.services.worlddata.helpers import browser_languages
from lp.services.worlddata.interfaces.country import ICountry
@@ -214,13 +217,9 @@ class SourcePackageOverviewMenu(ApplicationMenu):
def copyright(self):
return Link("+copyright", "View copyright", icon="info")
+ @enabled_with_permission("launchpad.Edit")
def edit_packaging(self):
- return Link(
- "+edit-packaging",
- "Change upstream link",
- icon="edit",
- enabled=self.userCanDeletePackaging(),
- )
+ return Link("+edit-packaging", "Change upstream link", icon="edit")
def remove_packaging(self):
return Link(
@@ -230,13 +229,9 @@ class SourcePackageOverviewMenu(ApplicationMenu):
enabled=self.userCanDeletePackaging(),
)
+ @enabled_with_permission("launchpad.Edit")
def set_upstream(self):
- return Link(
- "+edit-packaging",
- "Set upstream link",
- icon="add",
- enabled=self.userCanDeletePackaging(),
- )
+ return Link("+edit-packaging", "Set upstream link", icon="add")
def builds(self):
text = "Show builds"
@@ -245,8 +240,8 @@ class SourcePackageOverviewMenu(ApplicationMenu):
def userCanDeletePackaging(self):
packaging = self.context.direct_packaging
if packaging is None:
- return True
- return packaging.userCanDelete()
+ return False
+ return check_permission("launchpad.Edit", packaging)
class SourcePackageChangeUpstreamStepOne(ReturnToReferrerMixin, StepView):
@@ -425,10 +420,23 @@ class SourcePackageRemoveUpstreamView(
label = "Unlink an upstream project"
page_title = label
+ def initialize(self):
+ self.packaging = self.context.direct_packaging
+ if self.packaging is not None:
+ # We do permission check here rather than protecting the view
+ # in ZCML because product owners should also be allowed to unlink
+ # projects, even if they don't have edit permission on the source
+ # package.
+ if not check_permission("launchpad.Edit", self.packaging):
+ raise Unauthorized(
+ "You are not allowed to unlink an upstream project"
+ )
+ super().initialize()
+
@action("Unlink")
def unlink(self, action, data):
old_series = self.context.productseries
- if self.context.direct_packaging is not None:
+ if self.packaging is not None:
getUtility(IPackagingUtil).deletePackaging(
self.context.productseries,
self.context.sourcepackagename,
diff --git a/lib/lp/registry/browser/tests/packaging-views.rst b/lib/lp/registry/browser/tests/packaging-views.rst
index 3610369..260ec7f 100644
--- a/lib/lp/registry/browser/tests/packaging-views.rst
+++ b/lib/lp/registry/browser/tests/packaging-views.rst
@@ -30,6 +30,7 @@ The view has a label and requires a distro series and a source package name.
The distroseries field's vocabulary is the same as the ubuntu.series
attribute.
+ >>> _ = login_person(product.owner)
>>> view = create_view(productseries, "+ubuntupkg")
>>> print(view.label)
Ubuntu source packaging
@@ -200,8 +201,7 @@ and a new entry can be added to the packaging history.
... )
>>> grumpy_series.status = SeriesStatus.FROZEN
- >>> a_user = factory.makePerson(name="hedgehog")
- >>> ignored = login_person(a_user)
+ >>> _ = login_person(product.owner)
>>> form = {
... "field.sourcepackagename": "hot",
... "field.actions.continue": "Update",
diff --git a/lib/lp/registry/browser/tests/sourcepackage-views.rst b/lib/lp/registry/browser/tests/sourcepackage-views.rst
index b759ece..b3d2265 100644
--- a/lib/lp/registry/browser/tests/sourcepackage-views.rst
+++ b/lib/lp/registry/browser/tests/sourcepackage-views.rst
@@ -53,7 +53,9 @@ This is a multistep view. In the first step, the product is specified.
>>> print(view.view.request.form)
{'field.__visited_steps__': 'sourcepackage_change_upstream_step1'}
- >>> ignored = login_person(product.owner)
+ >>> from lp.registry.interfaces.person import IPersonSet
+ >>> admin = getUtility(IPersonSet).getByEmail("admin@xxxxxxxxxxxxx")
+ >>> _ = login_person(admin)
>>> form = {
... "field.product": "bonkers",
... "field.actions.continue": "Continue",
@@ -63,7 +65,7 @@ This is a multistep view. In the first step, the product is specified.
... package,
... name="+edit-packaging",
... form=form,
- ... principal=product.owner,
+ ... principal=admin,
... )
>>> view.view.errors
[]
@@ -88,7 +90,7 @@ product can be chosen from a list of options.
... package,
... name="+edit-packaging",
... form=form,
- ... principal=product.owner,
+ ... principal=admin,
... )
>>> ignored = view.view.render()
@@ -124,7 +126,7 @@ then the current product series will be the selected option.
... package,
... name="+edit-packaging",
... form=form,
- ... principal=product.owner,
+ ... principal=admin,
... )
>>> print(view.view.widgets.get("productseries")._getFormValue().name)
crazy
@@ -141,7 +143,7 @@ empty.
... package,
... name="+edit-packaging",
... form=form,
- ... principal=product.owner,
+ ... principal=admin,
... )
>>> for error in view.view.errors:
... print(pretty(error.args))
@@ -161,7 +163,7 @@ but there is no notification message that the upstream link was updated.
... package,
... name="+edit-packaging",
... form=form,
- ... principal=product.owner,
+ ... principal=admin,
... )
>>> print(view.view)
<...SourcePackageChangeUpstreamStepTwo object...>
@@ -386,7 +388,7 @@ to the project series.
>>> ignored = login_person(user)
>>> form = {"field.actions.unlink": "Unlink"}
>>> view = create_initialized_view(
- ... package, name="+remove-packaging", form=form, principal=user
+ ... package, name="+remove-packaging", form=form, principal=admin
... )
>>> view.errors
[]
@@ -401,7 +403,7 @@ they get a message telling them that the link has already been
deleted.
>>> view = create_initialized_view(
- ... package, name="+remove-packaging", form=form, principal=user
+ ... package, name="+remove-packaging", form=form, principal=admin
... )
>>> view.errors
[]
diff --git a/lib/lp/registry/browser/tests/test_packaging.py b/lib/lp/registry/browser/tests/test_packaging.py
index 7549a51..86bac22 100644
--- a/lib/lp/registry/browser/tests/test_packaging.py
+++ b/lib/lp/registry/browser/tests/test_packaging.py
@@ -12,7 +12,7 @@ from lp.registry.interfaces.packaging import IPackagingUtil, PackagingType
from lp.registry.interfaces.product import IProductSet
from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
from lp.services.features.testing import FeatureFixture
-from lp.testing import TestCaseWithFactory, login, logout
+from lp.testing import TestCaseWithFactory, login, logout, person_logged_in
from lp.testing.layers import DatabaseFunctionalLayer
from lp.testing.pages import setupBrowser
from lp.testing.views import create_initialized_view
@@ -49,7 +49,7 @@ class TestProductSeriesUbuntuPackagingView(WithScenarios, TestCaseWithFactory):
)
self.packaging_util = getUtility(IPackagingUtil)
- def test_no_error_when_trying_to_readd_same_package(self):
+ def test_no_error_when_trying_to_read_same_package(self):
# There is no reason to display an error when the user's action
# wouldn't cause a state change.
self.packaging_util.createPackaging(
@@ -65,9 +65,13 @@ class TestProductSeriesUbuntuPackagingView(WithScenarios, TestCaseWithFactory):
"field.sourcepackagename": self.sourcepackagename.name,
"field.actions.continue": "Continue",
}
- view = create_initialized_view(
- self.productseries, "+ubuntupkg", form=form
- )
+ with person_logged_in(self.product.owner):
+ view = create_initialized_view(
+ self.productseries,
+ "+ubuntupkg",
+ form=form,
+ principal=self.product.owner,
+ )
self.assertEqual([], view.errors)
def test_cannot_link_to_linked_package(self):
@@ -78,9 +82,13 @@ class TestProductSeriesUbuntuPackagingView(WithScenarios, TestCaseWithFactory):
"field.sourcepackagename": "hot",
"field.actions.continue": "Continue",
}
- view = create_initialized_view(
- self.productseries, "+ubuntupkg", form=form
- )
+ with person_logged_in(self.product.owner):
+ view = create_initialized_view(
+ self.productseries,
+ "+ubuntupkg",
+ form=form,
+ principal=self.product.owner,
+ )
self.assertEqual([], view.errors)
other_productseries = self.factory.makeProductSeries(
product=self.product, name="hottest"
@@ -90,9 +98,13 @@ class TestProductSeriesUbuntuPackagingView(WithScenarios, TestCaseWithFactory):
"field.sourcepackagename": "hot",
"field.actions.continue": "Continue",
}
- view = create_initialized_view(
- other_productseries, "+ubuntupkg", form=form
- )
+ with person_logged_in(self.product.owner):
+ view = create_initialized_view(
+ other_productseries,
+ "+ubuntupkg",
+ form=form,
+ principal=self.product.owner,
+ )
view_errors = [
'The <a href="http://launchpad.test/ubuntu/hoary/+source/hot";>'
"hot</a> package in Hoary is already linked to another series."
@@ -106,9 +118,13 @@ class TestProductSeriesUbuntuPackagingView(WithScenarios, TestCaseWithFactory):
"field.sourcepackagename": "",
"field.actions.continue": "Continue",
}
- view = create_initialized_view(
- self.productseries, "+ubuntupkg", form=form
- )
+ with person_logged_in(self.product.owner):
+ view = create_initialized_view(
+ self.productseries,
+ "+ubuntupkg",
+ form=form,
+ principal=self.product.owner,
+ )
self.assertEqual(1, len(view.errors))
self.assertEqual("sourcepackagename", view.errors[0].field_name)
self.assertEqual("Required input is missing.", view.errors[0].doc())
@@ -125,9 +141,13 @@ class TestProductSeriesUbuntuPackagingView(WithScenarios, TestCaseWithFactory):
"field.sourcepackagename": "vapor",
"field.actions.continue": "Continue",
}
- view = create_initialized_view(
- self.productseries, "+ubuntupkg", form=form
- )
+ with person_logged_in(self.product.owner):
+ view = create_initialized_view(
+ self.productseries,
+ "+ubuntupkg",
+ form=form,
+ principal=self.product.owner,
+ )
view_errors = ["The source package is not published in Hoary."]
self.assertEqual(view_errors, view.errors)
@@ -142,9 +162,13 @@ class TestProductSeriesUbuntuPackagingView(WithScenarios, TestCaseWithFactory):
"field.sourcepackagename": "hot",
"field.actions.continue": "Continue",
}
- view = create_initialized_view(
- self.productseries, "+ubuntupkg", form=form
- )
+ with person_logged_in(self.product.owner):
+ view = create_initialized_view(
+ self.productseries,
+ "+ubuntupkg",
+ form=form,
+ principal=self.product.owner,
+ )
self.assertEqual([], view.errors)
has_packaging = self.packaging_util.packagingEntryExists(
self.sourcepackagename, warty, self.productseries
diff --git a/lib/lp/registry/browser/tests/test_product.py b/lib/lp/registry/browser/tests/test_product.py
index de8f154..61618f0 100644
--- a/lib/lp/registry/browser/tests/test_product.py
+++ b/lib/lp/registry/browser/tests/test_product.py
@@ -862,7 +862,9 @@ class TestProductEditView(BrowserTestCase):
# It should be an error to make a Product private if it is packaged.
product = self.factory.makeProduct()
sourcepackage = self.factory.makeSourcePackage()
- sourcepackage.setPackaging(product.development_focus, product.owner)
+ removeSecurityProxy(sourcepackage).setPackaging(
+ product.development_focus, product.owner
+ )
browser = self.getViewBrowser(product, "+edit", user=product.owner)
info_type = browser.getControl(name="field.information_type")
info_type.value = ["PROPRIETARY"]
diff --git a/lib/lp/registry/browser/tests/test_sourcepackage_views.py b/lib/lp/registry/browser/tests/test_sourcepackage_views.py
index 291c28a..8044c1a 100644
--- a/lib/lp/registry/browser/tests/test_sourcepackage_views.py
+++ b/lib/lp/registry/browser/tests/test_sourcepackage_views.py
@@ -244,7 +244,7 @@ class TestSourcePackageUpstreamConnectionsView(TestCaseWithFactory):
distroseries=distroseries,
version="1.5-0ubuntu1",
)
- self.source_package.setPackaging(
+ removeSecurityProxy(self.source_package).setPackaging(
productseries, productseries.product.owner
)
@@ -301,80 +301,119 @@ class TestSourcePackagePackagingLinks(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
- def makeSourcePackageOverviewMenu(self, with_packaging, karma=None):
- sourcepackage = self.factory.makeSourcePackage()
- registrant = self.factory.makePerson()
- if with_packaging:
- self.factory.makePackagingLink(
- sourcepackagename=sourcepackage.sourcepackagename,
- distroseries=sourcepackage.distroseries,
- owner=registrant,
- )
- user = self.factory.makePerson(karma=karma)
+ def setUp(self, *args, **kwargs):
+ super().setUp(*args, **kwargs)
+ self.sourcepackage = self.factory.makeSourcePackage()
+ self.maintainer = self.sourcepackage.distribution.owner
+ self.product_owner = self.factory.makePerson()
+ self.product = self.factory.makeProduct(owner=self.product_owner)
+ self.productseries = self.factory.makeProductSeries(self.product)
+
+ def makePackaging(self):
+ self.factory.makePackagingLink(
+ sourcepackagename=self.sourcepackage.sourcepackagename,
+ distroseries=self.sourcepackage.distroseries,
+ productseries=self.productseries,
+ )
+
+ def makeSourcePackageOverviewMenu(self, user):
with person_logged_in(user):
- menu = SourcePackageOverviewMenu(sourcepackage)
- return menu, user
+ menu = SourcePackageOverviewMenu(self.sourcepackage)
+ return menu
- def test_edit_packaging_link__enabled_without_packaging(self):
- # If no packging exists, the edit_packaging link is always
+ def test_edit_packaging_link__enabled_without_packaging_maintainer(self):
+ # If no packaging exists, the edit_packaging link is always
# enabled.
- menu, user = self.makeSourcePackageOverviewMenu(False, None)
- with person_logged_in(user):
+ menu = self.makeSourcePackageOverviewMenu(self.maintainer)
+ with person_logged_in(self.maintainer):
self.assertTrue(menu.edit_packaging().enabled)
- def test_set_upstrem_link__enabled_without_packaging(self):
- # If no packging exists, the set_upstream link is always
+ def test_set_upstream_link__enabled_without_packaging_maintainer(self):
+ # If no packaging exists, the set_upstream link is always
# enabled.
- menu, user = self.makeSourcePackageOverviewMenu(False, None)
- with person_logged_in(user):
+ menu = self.makeSourcePackageOverviewMenu(self.maintainer)
+ with person_logged_in(self.maintainer):
self.assertTrue(menu.set_upstream().enabled)
- def test_remove_packaging_link__enabled_without_packaging(self):
- # If no packging exists, the remove_packaging link is always
- # enabled.
- menu, user = self.makeSourcePackageOverviewMenu(False, None)
- with person_logged_in(user):
- self.assertTrue(menu.remove_packaging().enabled)
+ def test_remove_packaging_link__enabled_without_packaging_maintainer(self):
+ # If no packaging exists, the remove_packaging link is always
+ # disabled.
+ menu = self.makeSourcePackageOverviewMenu(self.maintainer)
+ with person_logged_in(self.maintainer):
+ self.assertFalse(menu.remove_packaging().enabled)
- def test_edit_packaging_link__enabled_with_packaging_non_probation(self):
- # If a packging exists, the edit_packaging link is enabled
- # for the non-probationary users.
- menu, user = self.makeSourcePackageOverviewMenu(True, 100)
- with person_logged_in(user):
+ def test_edit_packaging_link__enabled_with_packaging_maintainer(self):
+ # If a packaging exists, the edit_packaging link is enabled
+ # for the package maintainer.
+ self.makePackaging()
+ menu = self.makeSourcePackageOverviewMenu(self.maintainer)
+ with person_logged_in(self.maintainer):
self.assertTrue(menu.edit_packaging().enabled)
- def test_set_upstrem_link__enabled_with_packaging_non_probation(self):
- # If a packging exists, the set_upstream link is enabled
- # for the non-probationary users.
- menu, user = self.makeSourcePackageOverviewMenu(True, 100)
- with person_logged_in(user):
+ def test_set_upstream_link__enabled_with_packaging_maintainer(self):
+ # If a packaging exists, the set_upstream link is enabled
+ # for the package maintainer.
+ self.makePackaging()
+ menu = self.makeSourcePackageOverviewMenu(self.maintainer)
+ with person_logged_in(self.maintainer):
self.assertTrue(menu.set_upstream().enabled)
- def test_remove_packaging_link__enabled_with_packaging_non_probation(self):
- # If a packging exists, the remove_packaging link is enabled
- # for the non-probationary users.
- menu, user = self.makeSourcePackageOverviewMenu(True, 100)
- with person_logged_in(user):
+ def test_remove_packaging_link__enabled_with_packaging_maintainer(self):
+ # If a packaging exists, the remove_packaging link is enabled
+ # for the package maintainer.
+ self.makePackaging()
+ menu = self.makeSourcePackageOverviewMenu(self.maintainer)
+ with person_logged_in(self.maintainer):
+ self.assertTrue(menu.remove_packaging().enabled)
+
+ def test_edit_packaging_link__disabled_for_product_owner(self):
+ # If a packaging exists, the edit_packaging link is disabled
+ # for the product owner.
+ self.makePackaging()
+ menu = self.makeSourcePackageOverviewMenu(self.product_owner)
+ with person_logged_in(self.product_owner):
+ self.assertFalse(menu.edit_packaging().enabled)
+
+ def test_set_upstream_link__disabled_for_product_owner(self):
+ # If a packaging exists, the set_upstream link is disabled
+ # for the product owner.
+ self.makePackaging()
+ menu = self.makeSourcePackageOverviewMenu(self.product_owner)
+ with person_logged_in(self.product_owner):
+ self.assertFalse(menu.set_upstream().enabled)
+
+ def test_remove_packaging_link__enabled_for_product_owner(self):
+ # If a packaging exists, the remove_packaging link is enabled
+ # for product owner.
+ self.makePackaging()
+ menu = self.makeSourcePackageOverviewMenu(self.product_owner)
+ with person_logged_in(self.product_owner):
self.assertTrue(menu.remove_packaging().enabled)
- def test_edit_packaging_link__enabled_with_packaging_probation(self):
- # If a packging exists, the edit_packaging link is not enabled
- # for probationary users.
- menu, user = self.makeSourcePackageOverviewMenu(True, None)
+ def test_edit_packaging_link__enabled_with_packaging_arbitrary(self):
+ # If a packaging exists, the edit_packaging link is not enabled
+ # for arbitrary users.
+ self.makePackaging()
+ user = self.factory.makePerson()
+ menu = self.makeSourcePackageOverviewMenu(user)
with person_logged_in(user):
self.assertFalse(menu.edit_packaging().enabled)
- def test_set_upstrem_link__enabled_with_packaging_probation(self):
- # If a packging exists, the set_upstream link is not enabled
- # for probationary users.
- menu, user = self.makeSourcePackageOverviewMenu(True, None)
+ def test_set_upstream_link__enabled_with_packaging_arbitrary(self):
+ # If a packaging exists, the set_upstream link is not enabled
+ # for arbitrary users.
+ self.makePackaging()
+ user = self.factory.makePerson()
+ menu = self.makeSourcePackageOverviewMenu(user)
with person_logged_in(user):
self.assertFalse(menu.set_upstream().enabled)
- def test_remove_packaging_link__enabled_with_packaging_probation(self):
- # If a packging exists, the remove_packaging link is not enabled
- # for probationary users.
- menu, user = self.makeSourcePackageOverviewMenu(True, None)
+ def test_remove_packaging_link__enabled_with_packaging_arbitrary(self):
+ # If a packaging exists, the remove_packaging link is not enabled
+ # for arbitrary users.
+ self.makePackaging()
+ user = self.factory.makePerson()
+ menu = self.makeSourcePackageOverviewMenu(user)
with person_logged_in(user):
self.assertFalse(menu.remove_packaging().enabled)
@@ -392,10 +431,9 @@ class TestSourcePackageChangeUpstreamView(BrowserTestCase):
owner=product_owner,
information_type=InformationType.PROPRIETARY,
)
- ubuntu_series = self.factory.makeUbuntuDistroSeries()
- sp = self.factory.makeSourcePackage(distroseries=ubuntu_series)
+ sp = self.factory.makeSourcePackage()
browser = self.getViewBrowser(
- sp, "+edit-packaging", user=product_owner
+ sp, "+edit-packaging", user=self.factory.makeAdministrator()
)
browser.getControl("Project").value = product_name
browser.getControl("Continue").click()
@@ -413,10 +451,9 @@ class TestSourcePackageChangeUpstreamView(BrowserTestCase):
)
series = self.factory.makeProductSeries(product=product)
series_displayname = series.displayname
- ubuntu_series = self.factory.makeUbuntuDistroSeries()
- sp = self.factory.makeSourcePackage(distroseries=ubuntu_series)
+ sp = self.factory.makeSourcePackage()
browser = self.getViewBrowser(
- sp, "+edit-packaging", user=product_owner
+ sp, "+edit-packaging", user=self.factory.makeAdministrator()
)
browser.getControl("Project").value = product_name
browser.getControl("Continue").click()
diff --git a/lib/lp/registry/configure.zcml b/lib/lp/registry/configure.zcml
index f2d0433..585bbf1 100644
--- a/lib/lp/registry/configure.zcml
+++ b/lib/lp/registry/configure.zcml
@@ -2188,8 +2188,10 @@
<allow
interface="lp.registry.interfaces.packaging.IPackaging"/>
<require
- permission="zope.Public"
- set_schema="lp.registry.interfaces.packaging.IPackaging"/>
+ permission="launchpad.Edit"
+ set_schema="lp.registry.interfaces.packaging.IPackaging"
+ attributes="
+ destroySelf" />
</class>
<!-- PackagingUtil -->
diff --git a/lib/lp/registry/interfaces/packaging.py b/lib/lp/registry/interfaces/packaging.py
index 5ebddd5..ae16be5 100644
--- a/lib/lp/registry/interfaces/packaging.py
+++ b/lib/lp/registry/interfaces/packaging.py
@@ -104,14 +104,6 @@ class IPackaging(IHasOwner):
)
)
- def userCanDelete():
- """True, if the current user is allowed to delete this packaging,
- else False.
-
- Non-probationary users can delete packaging links that they believe
- connect Ubuntu to bogus data.
- """
-
class IPackagingUtil(Interface):
"""Utilities to handle Packaging."""
@@ -121,6 +113,9 @@ class IPackagingUtil(Interface):
):
"""Create Packaging entry."""
+ def get(productseries, sourcepackagename, distroseries):
+ """Get Packaging entry."""
+
def deletePackaging(productseries, sourcepackagename, distroseries):
"""Delete a packaging entry."""
diff --git a/lib/lp/registry/interfaces/productseries.py b/lib/lp/registry/interfaces/productseries.py
index 62e1ef5..f985de4 100644
--- a/lib/lp/registry/interfaces/productseries.py
+++ b/lib/lp/registry/interfaces/productseries.py
@@ -102,6 +102,11 @@ class IProductSeriesEditRestricted(Interface):
def newMilestone(name, dateexpected=None, summary=None, code_name=None):
"""Create a new milestone for this ProjectSeries."""
+ def setPackaging(distroseries, sourcepackagename, owner):
+ """Create or update a Packaging record for this product series,
+ connecting it to the given distroseries and source package name.
+ """
+
class IProductSeriesPublic(Interface):
"""Public IProductSeries properties."""
@@ -343,11 +348,6 @@ class IProductSeriesView(
"""Return the SourcePackage that packages this project in Ubuntu's
translation focus or current series or any series, in that order."""
- def setPackaging(distroseries, sourcepackagename, owner):
- """Create or update a Packaging record for this product series,
- connecting it to the given distroseries and source package name.
- """
-
def getPackagingInDistribution(distribution):
"""Return all the Packaging entries for this product series for the
given distribution. Note that this only returns EXPLICT packaging
diff --git a/lib/lp/registry/interfaces/sourcepackage.py b/lib/lp/registry/interfaces/sourcepackage.py
index d2a4ad2..bd02454 100644
--- a/lib/lp/registry/interfaces/sourcepackage.py
+++ b/lib/lp/registry/interfaces/sourcepackage.py
@@ -224,32 +224,6 @@ class ISourcePackagePublic(
sourcepackagename compare not equal.
"""
- @operation_parameters(productseries=Reference(schema=IProductSeries))
- @call_with(owner=REQUEST_USER)
- @export_write_operation()
- @operation_for_version("devel")
- def setPackaging(productseries, owner):
- """Update the existing packaging record, or create a new packaging
- record, that links the source package to the given productseries,
- and record that it was done by the owner.
- """
-
- @operation_parameters(productseries=Reference(schema=IProductSeries))
- @call_with(owner=REQUEST_USER)
- @export_write_operation()
- @operation_for_version("devel")
- def setPackagingReturnSharingDetailPermissions(productseries, owner):
- """Like setPackaging(), but returns getSharingDetailPermissions().
-
- This method is intended for AJAX usage on the +sharing-details
- page.
- """
-
- @export_write_operation()
- @operation_for_version("devel")
- def deletePackaging():
- """Delete the packaging for this sourcepackage."""
-
def getSharingDetailPermissions():
"""Return a dictionary of user permissions for +sharing-details page.
@@ -364,6 +338,32 @@ class ISourcePackageEdit(Interface):
:return: None
"""
+ @operation_parameters(productseries=Reference(schema=IProductSeries))
+ @call_with(owner=REQUEST_USER)
+ @export_write_operation()
+ @operation_for_version("devel")
+ def setPackaging(productseries, owner):
+ """Update the existing packaging record, or create a new packaging
+ record, that links the source package to the given productseries,
+ and record that it was done by the owner.
+ """
+
+ @operation_parameters(productseries=Reference(schema=IProductSeries))
+ @call_with(owner=REQUEST_USER)
+ @export_write_operation()
+ @operation_for_version("devel")
+ def setPackagingReturnSharingDetailPermissions(productseries, owner):
+ """Like setPackaging(), but returns getSharingDetailPermissions().
+
+ This method is intended for AJAX usage on the +sharing-details
+ page.
+ """
+
+ @export_write_operation()
+ @operation_for_version("devel")
+ def deletePackaging():
+ """Delete the packaging for this sourcepackage."""
+
@exported_as_webservice_entry(as_of="beta")
class ISourcePackage(ISourcePackagePublic, ISourcePackageEdit):
diff --git a/lib/lp/registry/model/packaging.py b/lib/lp/registry/model/packaging.py
index 2735a16..047b0e5 100644
--- a/lib/lp/registry/model/packaging.py
+++ b/lib/lp/registry/model/packaging.py
@@ -7,10 +7,8 @@ from lazr.lifecycle.event import ObjectCreatedEvent, ObjectDeletedEvent
from zope.component import getUtility
from zope.event import notify
from zope.interface import implementer
-from zope.security.interfaces import Unauthorized
from lp.app.enums import InformationType
-from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.registry.errors import CannotPackageProprietaryProduct
from lp.registry.interfaces.packaging import (
IPackaging,
@@ -23,7 +21,6 @@ from lp.services.database.datetimecol import UtcDateTimeCol
from lp.services.database.enumcol import DBEnum
from lp.services.database.sqlbase import SQLBase
from lp.services.database.sqlobject import ForeignKey
-from lp.services.webapp.interfaces import ILaunchBag
@implementer(IPackaging)
@@ -66,29 +63,7 @@ class Packaging(SQLBase):
super().__init__(**kwargs)
notify(ObjectCreatedEvent(self))
- def userCanDelete(self):
- """See `IPackaging`."""
- user = getUtility(ILaunchBag).user
- if user is None:
- return False
- admin = getUtility(ILaunchpadCelebrities).admin
- registry_experts = getUtility(ILaunchpadCelebrities).registry_experts
- if (
- not user.is_probationary
- or user.inTeam(self.productseries.product.owner)
- or user.canAccess(self.sourcepackage, "setBranch")
- or user.inTeam(registry_experts)
- or user.inTeam(admin)
- ):
- return True
- return False
-
def destroySelf(self):
- if not self.userCanDelete():
- raise Unauthorized(
- "Only the person who created the packaging and package "
- "maintainers can delete it."
- )
notify(ObjectDeletedEvent(self))
super().destroySelf()
@@ -97,16 +72,15 @@ class Packaging(SQLBase):
class PackagingUtil:
"""Utilities for Packaging."""
- @classmethod
def createPackaging(
- cls, productseries, sourcepackagename, distroseries, packaging, owner
+ self, productseries, sourcepackagename, distroseries, packaging, owner
):
"""See `IPackaging`.
Raises an assertion error if there is already packaging for
the sourcepackagename in the distroseries.
"""
- if cls.packagingEntryExists(sourcepackagename, distroseries):
+ if self.packagingEntryExists(sourcepackagename, distroseries):
raise AssertionError(
"A packaging entry for %s in %s already exists."
% (sourcepackagename.name, distroseries.name)
@@ -132,9 +106,18 @@ class PackagingUtil:
owner=owner,
)
+ def get(self, productseries, sourcepackagename, distroseries):
+ criteria = {
+ "sourcepackagename": sourcepackagename,
+ "distroseries": distroseries,
+ }
+ if productseries is not None:
+ criteria["productseries"] = productseries
+ return Packaging.selectOneBy(**criteria)
+
def deletePackaging(self, productseries, sourcepackagename, distroseries):
"""See `IPackaging`."""
- packaging = Packaging.selectOneBy(
+ packaging = getUtility(IPackagingUtil).get(
productseries=productseries,
sourcepackagename=sourcepackagename,
distroseries=distroseries,
@@ -152,17 +135,13 @@ class PackagingUtil:
)
packaging.destroySelf()
- @staticmethod
def packagingEntryExists(
- sourcepackagename, distroseries, productseries=None
+ self, sourcepackagename, distroseries, productseries=None
):
"""See `IPackaging`."""
- criteria = dict(
- sourcepackagename=sourcepackagename, distroseries=distroseries
+ packaging = self.get(
+ productseries=productseries,
+ sourcepackagename=sourcepackagename,
+ distroseries=distroseries,
)
- if productseries is not None:
- criteria["productseries"] = productseries
- result = Packaging.selectOneBy(**criteria)
- if result is None:
- return False
- return True
+ return packaging is not None
diff --git a/lib/lp/registry/model/productseries.py b/lib/lp/registry/model/productseries.py
index 9574999..10d30ea 100644
--- a/lib/lp/registry/model/productseries.py
+++ b/lib/lp/registry/model/productseries.py
@@ -17,6 +17,7 @@ from storm.locals import And, Desc
from storm.store import Store
from zope.component import getUtility
from zope.interface import implementer
+from zope.security.proxy import removeSecurityProxy
from lp.app.enums import service_uses_launchpad
from lp.app.errors import NotFoundError
@@ -35,7 +36,7 @@ from lp.bugs.model.structuralsubscription import (
StructuralSubscriptionTargetMixin,
)
from lp.registry.errors import ProprietaryPillar
-from lp.registry.interfaces.packaging import PackagingType
+from lp.registry.interfaces.packaging import IPackagingUtil, PackagingType
from lp.registry.interfaces.person import validate_person
from lp.registry.interfaces.productrelease import IProductReleaseSet
from lp.registry.interfaces.productseries import (
@@ -45,7 +46,6 @@ from lp.registry.interfaces.productseries import (
)
from lp.registry.interfaces.series import SeriesStatus
from lp.registry.model.milestone import HasMilestonesMixin, Milestone
-from lp.registry.model.packaging import PackagingUtil
from lp.registry.model.productrelease import ProductRelease
from lp.registry.model.series import SeriesMixin
from lp.services.database.constants import UTC_NOW
@@ -445,14 +445,14 @@ class ProductSeries(
# ok, we didn't find a packaging record that matches, let's go ahead
# and create one
- pkg = PackagingUtil.createPackaging(
+ pkg = getUtility(IPackagingUtil).createPackaging(
distroseries=distroseries,
sourcepackagename=sourcepackagename,
productseries=self,
packaging=PackagingType.PRIME,
owner=owner,
)
- pkg.sync() # convert UTC_NOW to actual datetime
+ removeSecurityProxy(pkg).sync() # convert UTC_NOW to actual datetime
return pkg
def getPackagingInDistribution(self, distribution):
diff --git a/lib/lp/registry/model/sourcepackage.py b/lib/lp/registry/model/sourcepackage.py
index 1bfacba..0e55f83 100644
--- a/lib/lp/registry/model/sourcepackage.py
+++ b/lib/lp/registry/model/sourcepackage.py
@@ -33,14 +33,14 @@ from lp.code.model.seriessourcepackagebranch import (
SeriesSourcePackageBranchSet,
)
from lp.registry.interfaces.distribution import NoPartnerArchive
-from lp.registry.interfaces.packaging import PackagingType
+from lp.registry.interfaces.packaging import IPackagingUtil, PackagingType
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.interfaces.sourcepackage import (
ISourcePackage,
ISourcePackageFactory,
)
from lp.registry.model.hasdrivers import HasDriversMixin
-from lp.registry.model.packaging import Packaging, PackagingUtil
+from lp.registry.model.packaging import Packaging
from lp.registry.model.suitesourcepackage import SuiteSourcePackage
from lp.services.database.decoratedresultset import DecoratedResultSet
from lp.services.database.interfaces import IStore
@@ -562,7 +562,7 @@ class SourcePackage(
# Delete the current packaging and create a new one so
# that the translation sharing jobs are started.
self.direct_packaging.destroySelf()
- PackagingUtil.createPackaging(
+ getUtility(IPackagingUtil).createPackaging(
distroseries=self.distroseries,
sourcepackagename=self.sourcepackagename,
productseries=productseries,
@@ -595,8 +595,8 @@ class SourcePackage(
else:
permissions.update(
{
- "user_can_change_product_series": (
- self.direct_packaging.userCanDelete()
+ "user_can_change_product_series": user.canAccess(
+ self, "setPackaging"
),
"user_can_change_branch": user.canWrite(
productseries, "branch"
diff --git a/lib/lp/registry/security.py b/lib/lp/registry/security.py
index 2e14ae0..a5b2c3a 100644
--- a/lib/lp/registry/security.py
+++ b/lib/lp/registry/security.py
@@ -219,8 +219,16 @@ class EditProduct(EditByOwnersOrAdmins):
)
-class EditPackaging(EditByOwnersOrAdmins):
+class EditPackaging(AuthorizationBase):
usedfor = IPackaging
+ permission = "launchpad.Edit"
+
+ def checkAuthenticated(self, user):
+ edit_product_series = EditProductSeries(self.obj.productseries)
+ edit_source_package = EditSourcePackage(self.obj.sourcepackage)
+ return edit_product_series.checkAuthenticated(
+ user
+ ) or edit_source_package.checkAuthenticated(user)
class DownloadFullSourcePackageTranslations(OnlyRosettaExpertsAndAdmins):
diff --git a/lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging-concurrent-deletion.rst b/lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging-concurrent-deletion.rst
index f87fa18..d53abcf 100644
--- a/lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging-concurrent-deletion.rst
+++ b/lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging-concurrent-deletion.rst
@@ -5,12 +5,12 @@ When two browsers are used to concurrently delete the same packaging
association, only one of them can succeed. The other one does not oops
and displays a meaningful error message.
-The No Privilege User may open the same page in two browser tabs.
+The package maintainer may open the same page in two browser tabs.
- >>> first_browser = setupBrowser(auth="Basic test@xxxxxxxxxxxxx:test")
+ >>> first_browser = setupBrowser(auth="Basic admin@xxxxxxxxxxxxx:test")
>>> first_browser.open("http://launchpad.test/ubuntu/+source/alsa-utils";)
- >>> second_browser = setupBrowser(auth="Basic test@xxxxxxxxxxxxx:test")
+ >>> second_browser = setupBrowser(auth="Basic admin@xxxxxxxxxxxxx:test")
>>> second_browser.open("http://launchpad.test/ubuntu/+source/alsa-utils";)
Then the user click the "Delete Link" button in the first tab. The
diff --git a/lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging.rst b/lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging.rst
index c616717..3f66179 100644
--- a/lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging.rst
+++ b/lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging.rst
@@ -18,7 +18,7 @@ source package in each series of this distribution.
>>> anon_browser.open("http://launchpad.test/ubuntu/+source/alsa-utils";)
>>> content = anon_browser.contents
>>> print(extract_text(find_tag_by_id(content, "packages_list")))
- The Hoary Hedgehog Release (active development) Set upstream link
+ The Hoary Hedgehog Release (active development)
1.0.9a-4ubuntu1 release (main) 2005-09-15
The Warty Warthog Release (current stable release) alsa-utils trunk series
1.0.9a-4 release (main) 2005-09-16
@@ -28,12 +28,12 @@ source package in each series of this distribution.
Delete Link Button
------------------
-A button is displayed to authenticated users to delete existing
+A button is displayed to project owners to delete existing
packaging links.
- >>> user_browser = setupBrowser(auth="Basic test@xxxxxxxxxxxxx:test")
- >>> user_browser.open("http://launchpad.test/ubuntu/+source/alsa-utils";)
- >>> link = user_browser.getLink(
+ >>> owner_browser = setupBrowser(auth="Basic mark@xxxxxxxxxxx:test")
+ >>> owner_browser.open("http://launchpad.test/ubuntu/+source/alsa-utils";)
+ >>> link = owner_browser.getLink(
... url="/ubuntu/warty/+source/alsa-utils/+remove-packaging"
... )
>>> print(link)
@@ -49,12 +49,12 @@ This button is not displayed to anonymous users.
Clicking this button deletes the corresponding packaging association.
- >>> link = user_browser.getLink(
+ >>> link = owner_browser.getLink(
... url="/ubuntu/warty/+source/alsa-utils/+remove-packaging"
... )
>>> link.click()
- >>> user_browser.getControl("Unlink").click()
- >>> content = user_browser.contents
+ >>> owner_browser.getControl("Unlink").click()
+ >>> content = owner_browser.contents
>>> for tag in find_tags_by_class(content, "error"):
... print(extract_text(tag))
...
diff --git a/lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.rst b/lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.rst
index 9729757..e32b56a 100644
--- a/lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.rst
+++ b/lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.rst
@@ -10,23 +10,25 @@ Create test data.
>>> test_publisher.updatePackageCache(test_data["distroseries"])
>>> logout()
-No Privileges Person visit the distroseries upstream links page for Hoary
-and sees that pmount is not linked.
+A person with permissions to edit packaging visits the distroseries upstream
+links page for Hoary and sees that pmount is not linked.
- >>> user_browser.open(
+ >>> admin_browser = setupBrowser(auth='Basic admin@xxxxxxxxxxxxx:test')
+ >>> admin_browser.open(
... "http://launchpad.test/ubuntu/hoary/+needs-packaging";
... )
- >>> print(extract_text(find_tag_by_id(user_browser.contents, "packages")))
+ >>> print(extract_text(
+ ... find_tag_by_id(admin_browser.contents, "packages")))
Source Package Bugs Translations
pmount No bugs 64 strings ...
They look at the pmount source package page in Hoary and read that the
upstream project is not set.
- >>> user_browser.getLink("pmount").click()
+ >>> admin_browser.getLink("pmount").click()
>>> print(
... extract_text(
- ... find_tag_by_id(user_browser.contents, "no-upstreams")
+ ... find_tag_by_id(admin_browser.contents, "no-upstreams")
... )
... )
Launchpad...
@@ -36,44 +38,44 @@ upstream project is not set.
Choose another upstream project
Register the upstream project
-No Privileges Person knows that the pmount package comes from the thunderbird
+The person knows that the pmount package comes from the thunderbird
project. They set the upstream packaging link and see that it is set.
- >>> user_browser.getControl(
+ >>> admin_browser.getControl(
... "Choose another upstream project"
... ).selected = True
- >>> user_browser.getControl("Link to Upstream Project").click()
- >>> user_browser.getControl(name="field.product").value = "thunderbird"
- >>> user_browser.getControl("Continue").click()
- >>> user_browser.getControl(name="field.productseries").value = ["trunk"]
- >>> user_browser.getControl("Change").click()
+ >>> admin_browser.getControl("Link to Upstream Project").click()
+ >>> admin_browser.getControl(name="field.product").value = "thunderbird"
+ >>> admin_browser.getControl("Continue").click()
+ >>> admin_browser.getControl(name="field.productseries").value = ["trunk"]
+ >>> admin_browser.getControl("Change").click()
>>> print(
- ... extract_text(find_tag_by_id(user_browser.contents, "upstreams"))
+ ... extract_text(find_tag_by_id(admin_browser.contents, "upstreams"))
... )
The Mozilla Project...Mozilla Thunderbird...trunk...
They see the "Show upstream links" link and take a look at the project's
packaging in distributions.
- >>> user_browser.getLink("Show upstream links").click()
+ >>> admin_browser.getLink("Show upstream links").click()
>>> print(
... extract_text(
- ... find_tag_by_id(user_browser.contents, "distribution-series")
+ ... find_tag_by_id(admin_browser.contents, "distribution-series")
... )
... )
Distribution series Source package Version Project series
Hoary (5.04) pmount 0.1-2 Mozilla Thunderbird trunk...
-No Privileges Person returns to the pmount source package page, sees the
+The person returns to the pmount source package page, sees the
link to all versions and follows it to the distro source package page.
- >>> user_browser.getLink("pmount").click()
- >>> user_browser.getLink(
+ >>> admin_browser.getLink("pmount").click()
+ >>> admin_browser.getLink(
... "All versions of pmount source in Ubuntu"
... ).click()
>>> print(
... extract_text(
- ... find_tag_by_id(user_browser.contents, "packages_list")
+ ... find_tag_by_id(admin_browser.contents, "packages_list")
... )
... )
The Hoary Hedgehog Release (active development) ...
@@ -83,72 +85,72 @@ link to all versions and follows it to the distro source package page.
Register a project from a source package
----------------------------------------
-No Privileges Person can register a project for a package, and Launchpad
+The person can register a project for a package, and Launchpad
will use the data from the source package to prefill the first
step of the multistep form.
- >>> user_browser.open(
+ >>> admin_browser.open(
... "http://launchpad.test/youbuntu/busy/+source/bonkers";
... )
- >>> user_browser.getControl(
+ >>> admin_browser.getControl(
... "Register the upstream project"
... ).selected = True
- >>> user_browser.getControl("Link to Upstream Project").click()
- >>> print(user_browser.getControl(name="field.name").value)
+ >>> admin_browser.getControl("Link to Upstream Project").click()
+ >>> print(admin_browser.getControl(name="field.name").value)
bonkers
- >>> print(user_browser.getControl(name="field.display_name").value)
+ >>> print(admin_browser.getControl(name="field.display_name").value)
Bonkers
- >>> print(user_browser.getControl(name="field.summary").value)
+ >>> print(admin_browser.getControl(name="field.summary").value)
summary for flubber-bin
summary for flubber-lib
>>> print(
- ... extract_text(find_tag_by_id(user_browser.contents, "step-title"))
+ ... extract_text(find_tag_by_id(admin_browser.contents, "step-title"))
... )
Step 2 (of 2): Check for duplicate projects
-When No Privileges Person selects "Choose another upstream project" and
+When the person selects "Choose another upstream project" and
then finds out that the project doesn't exist, they use the
"Link to Upstream Project" button to register the project.
- >>> user_browser.open(
+ >>> admin_browser.open(
... "http://launchpad.test/youbuntu/busy/+source/bonkers/";
... )
- >>> user_browser.getControl(
+ >>> admin_browser.getControl(
... "Choose another upstream project"
... ).selected = True
- >>> user_browser.getControl("Link to Upstream Project").click()
- >>> print(user_browser.url)
+ >>> admin_browser.getControl("Link to Upstream Project").click()
+ >>> print(admin_browser.url)
http://launchpad.test/youbuntu/busy/+source/bonkers/+edit-packaging
- >>> user_browser.getLink("Register the upstream project").click()
- >>> print(user_browser.getControl(name="field.name").value)
+ >>> admin_browser.getLink("Register the upstream project").click()
+ >>> print(admin_browser.getControl(name="field.name").value)
bonkers
- >>> print(user_browser.getControl(name="field.display_name").value)
+ >>> print(admin_browser.getControl(name="field.display_name").value)
Bonkers
- >>> print(user_browser.getControl(name="field.summary").value)
+ >>> print(admin_browser.getControl(name="field.summary").value)
summary for flubber-bin
summary for flubber-lib
>>> print(
- ... extract_text(find_tag_by_id(user_browser.contents, "step-title"))
+ ... extract_text(find_tag_by_id(admin_browser.contents, "step-title"))
... )
Step 2 (of 2): Check for duplicate projects
-After No Privileges Person selects the licences, the user is redirected back
+After the person selects the licences, the user is redirected back
to the source package page and an informational message will be displayed.
- >>> user_browser.getControl(name="field.licenses").value = ["BSD"]
- >>> user_browser.getControl(
+ >>> admin_browser.getControl(name="field.licenses").value = ["BSD"]
+ >>> admin_browser.getControl(
... "Complete registration and link to bonkers package"
... ).click()
- >>> print(user_browser.url)
+ >>> print(admin_browser.url)
http://launchpad.test/youbuntu/busy/+source/bonkers
>>> for tag in find_tags_by_class(
- ... user_browser.contents, "informational message"
+ ... admin_browser.contents, "informational message"
... ):
... print(extract_text(tag))
Linked Bonkers project to bonkers source package.
>>> print(
- ... extract_text(find_tag_by_id(user_browser.contents, "upstreams"))
+ ... extract_text(find_tag_by_id(admin_browser.contents, "upstreams"))
... )
Bonkers ⇒ trunk
Change upstream link
diff --git a/lib/lp/registry/stories/product/xx-product-package-pages.rst b/lib/lp/registry/stories/product/xx-product-package-pages.rst
index 8d6f05a..8c160aa 100644
--- a/lib/lp/registry/stories/product/xx-product-package-pages.rst
+++ b/lib/lp/registry/stories/product/xx-product-package-pages.rst
@@ -42,11 +42,13 @@ Evolution.
>>> evo_owner.getLink(url="/ubuntu/hoary/+source/evolution") is not None
True
-Any logged in users can still see the links to create a packaging link.
+Arbitrary users can't see the links to create a packaging link.
>>> user_browser.open("http://launchpad.test/evolution/+packages";)
- >>> print(user_browser.getLink(url="/evolution/trunk/+ubuntupkg").url)
- http://launchpad.test/evolution/trunk/+ubuntupkg
+ >>> user_browser.getLink(url="/evolution/trunk/+ubuntupkg")
+ Traceback (most recent call last):
+ ...
+ zope.testbrowser.browser.LinkNotFoundError
>>> anon_browser.open("http://launchpad.test/evolution/+packages";)
>>> anon_browser.getLink(url="/evolution/trunk/+ubuntupkg")
diff --git a/lib/lp/registry/tests/test_distroseries.py b/lib/lp/registry/tests/test_distroseries.py
index d3f86df..ab3c6bf 100644
--- a/lib/lp/registry/tests/test_distroseries.py
+++ b/lib/lp/registry/tests/test_distroseries.py
@@ -580,7 +580,7 @@ class TestDistroSeriesPackaging(TestCaseWithFactory):
def linkPackage(self, name):
product_series = self.factory.makeProductSeries()
- product_series.setPackaging(
+ removeSecurityProxy(product_series).setPackaging(
self.series, self.packages[name].sourcepackagename, self.user
)
return product_series
diff --git a/lib/lp/registry/tests/test_packaging.py b/lib/lp/registry/tests/test_packaging.py
index 628bfb6..f4b4db6 100644
--- a/lib/lp/registry/tests/test_packaging.py
+++ b/lib/lp/registry/tests/test_packaging.py
@@ -19,6 +19,8 @@ from lp.registry.model.packaging import Packaging
from lp.testing import (
EventRecorder,
TestCaseWithFactory,
+ admin_logged_in,
+ anonymous_logged_in,
login,
person_logged_in,
)
@@ -61,7 +63,7 @@ class TestPackaging(TestCaseWithFactory):
packaging.distroseries,
)
- def test_destroySelf__not_allowed_for_probationary_user(self):
+ def test_destroySelf__not_allowed_for_random_user(self):
"""Arbitrary users cannot delete a packaging."""
packaging = self.factory.makePackagingLink()
packaging_util = getUtility(IPackagingUtil)
@@ -74,26 +76,6 @@ class TestPackaging(TestCaseWithFactory):
packaging.distroseries,
)
- def test_destroySelf__allowed_for_non_probationary_user(self):
- """An experienced user can delete a packaging."""
- packaging = self.factory.makePackagingLink()
- sourcepackagename = packaging.sourcepackagename
- distroseries = packaging.distroseries
- productseries = packaging.productseries
- packaging_util = getUtility(IPackagingUtil)
- user = self.factory.makePerson(karma=200)
- with person_logged_in(user):
- packaging_util.deletePackaging(
- packaging.productseries,
- packaging.sourcepackagename,
- packaging.distroseries,
- )
- self.assertFalse(
- packaging_util.packagingEntryExists(
- sourcepackagename, distroseries, productseries
- )
- )
-
def test_destroySelf__allowed_for_uploader(self):
"""A person with upload rights for the sourcepackage can
delete a packaging link.
@@ -335,8 +317,7 @@ class TestDeletePackaging(TestCaseWithFactory):
"""Deleting a Packaging creates a notification."""
packaging_util = getUtility(IPackagingUtil)
packaging = self.factory.makePackagingLink()
- user = self.factory.makePerson(karma=200)
- with person_logged_in(user):
+ with person_logged_in(packaging.productseries.product.owner):
with EventRecorder() as recorder:
packaging_util.deletePackaging(
packaging.productseries,
@@ -346,3 +327,61 @@ class TestDeletePackaging(TestCaseWithFactory):
(event,) = recorder.events
self.assertIsInstance(event, ObjectDeletedEvent)
self.assertIs(removeSecurityProxy(packaging), event.object)
+
+
+class TestPackagingSecurity(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self, *args, **kwargs):
+ super().setUp(*args, **kwargs)
+ self.packaging = self.factory.makePackagingLink()
+
+ def viewPackaging(self):
+ _ = self.packaging.packaging
+
+ def editPackaging(self):
+ self.packaging.packaging = PackagingType.PRIME
+
+ def test_anyone_can_view(self):
+ with anonymous_logged_in():
+ try:
+ self.viewPackaging()
+ except Unauthorized:
+ self.fail("Unauthorized exception raised")
+
+ def test_anonymous_person_cannot_edit(self):
+ with anonymous_logged_in():
+ self.assertRaises(Unauthorized, self.editPackaging)
+
+ def test_random_person_cannot_edit(self):
+ with person_logged_in(self.factory.makePerson()):
+ self.assertRaises(Unauthorized, self.editPackaging)
+
+ def test_admin_can_edit(self):
+ with admin_logged_in():
+ try:
+ self.editPackaging()
+ except Unauthorized:
+ self.fail("Unauthorized exception raised")
+
+ def test_product_owner_can_edit(self):
+ with person_logged_in(self.packaging.productseries.product.owner):
+ try:
+ self.editPackaging()
+ except Unauthorized:
+ self.fail("Unauthorized exception raised")
+
+ def test_productseries_owner_can_edit(self):
+ with person_logged_in(self.packaging.productseries.owner):
+ try:
+ self.editPackaging()
+ except Unauthorized:
+ self.fail("Unauthorized exception raised")
+
+ def test_distribution_owner_can_edit(self):
+ with person_logged_in(self.packaging.distroseries.distribution.owner):
+ try:
+ self.editPackaging()
+ except Unauthorized:
+ self.fail("Unauthorized exception raised")
diff --git a/lib/lp/registry/tests/test_productseries.py b/lib/lp/registry/tests/test_productseries.py
index e0ea6a0..8f6b0fb 100644
--- a/lib/lp/registry/tests/test_productseries.py
+++ b/lib/lp/registry/tests/test_productseries.py
@@ -665,7 +665,7 @@ class ProductSeriesSecurityAdaperTestCase(TestCaseWithFactory):
"addSubscription",
"removeBugSubscription",
},
- "launchpad.Edit": {"newMilestone"},
+ "launchpad.Edit": {"newMilestone", "setPackaging"},
"launchpad.LimitedView": {
"bugtargetdisplayname",
"bugtarget_parent",
@@ -746,7 +746,6 @@ class ProductSeriesSecurityAdaperTestCase(TestCaseWithFactory):
"releases",
"releaseverstyle",
"searchTasks",
- "setPackaging",
"sourcepackages",
"specifications",
"status",
diff --git a/lib/lp/registry/tests/test_sourcepackage.py b/lib/lp/registry/tests/test_sourcepackage.py
index 63e976a..3d101d7 100644
--- a/lib/lp/registry/tests/test_sourcepackage.py
+++ b/lib/lp/registry/tests/test_sourcepackage.py
@@ -33,6 +33,7 @@ from lp.testing import (
EventRecorder,
TestCaseWithFactory,
WebServiceTestCase,
+ admin_logged_in,
person_logged_in,
)
from lp.testing.layers import DatabaseFunctionalLayer
@@ -305,11 +306,10 @@ class TestSourcePackage(TestCaseWithFactory):
def test_deletePackaging(self):
"""Ensure deletePackaging completely removes packaging."""
- user = self.factory.makePerson(karma=200)
packaging = self.factory.makePackagingLink()
packaging_id = packaging.id
store = Store.of(packaging)
- with person_logged_in(user):
+ with person_logged_in(packaging.sourcepackage.distribution.owner):
packaging.sourcepackage.deletePackaging()
result = store.find(Packaging, Packaging.id == packaging_id)
self.assertIs(None, result.one())
@@ -318,7 +318,7 @@ class TestSourcePackage(TestCaseWithFactory):
"""setPackaging() creates a Packaging link."""
sourcepackage = self.factory.makeSourcePackage()
productseries = self.factory.makeProductSeries()
- sourcepackage.setPackaging(
+ removeSecurityProxy(sourcepackage).setPackaging(
productseries, owner=self.factory.makePerson()
)
packaging = sourcepackage.direct_packaging
@@ -329,10 +329,9 @@ class TestSourcePackage(TestCaseWithFactory):
sourcepackage = self.factory.makeSourcePackage()
productseries = self.factory.makeProductSeries()
other_series = self.factory.makeProductSeries()
- user = self.factory.makePerson(karma=200)
registrant = self.factory.makePerson()
with EventRecorder() as recorder:
- with person_logged_in(user):
+ with person_logged_in(sourcepackage.distribution.owner):
sourcepackage.setPackaging(productseries, owner=registrant)
sourcepackage.setPackaging(other_series, owner=registrant)
packaging = sourcepackage.direct_packaging
@@ -348,36 +347,43 @@ class TestSourcePackage(TestCaseWithFactory):
def test_refuses_PROPRIETARY(self):
"""Packaging cannot be created for PROPRIETARY productseries"""
- owner = self.factory.makePerson()
product = self.factory.makeProduct(
- owner=owner, information_type=InformationType.PROPRIETARY
+ information_type=InformationType.PROPRIETARY
)
series = self.factory.makeProductSeries(product=product)
ubuntu_series = self.factory.makeUbuntuDistroSeries()
sp = self.factory.makeSourcePackage(distroseries=ubuntu_series)
- with person_logged_in(owner):
+ with admin_logged_in():
with ExpectedException(
CannotPackageProprietaryProduct,
"Only Public project series can be packaged, not "
"Proprietary.",
):
- sp.setPackaging(series, owner)
+ sp.setPackaging(series, product.owner)
def test_setPackagingReturnSharingDetailPermissions__ordinary_user(self):
- """An ordinary user can create a packaging link but they cannot
- set the series' branch or translation syncronisation settings,
+ """An ordinary user can own a packaging link, but they cannot
+ set the series' branch or translation synchronization settings,
or the translation usage settings of the product.
"""
sourcepackage = self.factory.makeSourcePackage()
productseries = self.factory.makeProductSeries()
- packaging_owner = self.factory.makePerson(karma=100)
- with person_logged_in(packaging_owner):
+ packaging_owner = self.factory.makePerson()
+ with person_logged_in(sourcepackage.distribution.owner):
permissions = (
sourcepackage.setPackagingReturnSharingDetailPermissions(
productseries, packaging_owner
)
)
- self.assertEqual(productseries, sourcepackage.productseries)
+ expected = {
+ "user_can_change_product_series": True,
+ "user_can_change_branch": False,
+ "user_can_change_translation_usage": False,
+ "user_can_change_translations_autoimport_mode": False,
+ }
+ self.assertEqual(expected, permissions)
+ self.assertEqual(productseries, sourcepackage.productseries)
+ with person_logged_in(packaging_owner):
self.assertFalse(packaging_owner.canWrite(productseries, "branch"))
self.assertFalse(
packaging_owner.canWrite(
@@ -389,20 +395,13 @@ class TestSourcePackage(TestCaseWithFactory):
productseries.product, "translations_usage"
)
)
- expected = {
- "user_can_change_product_series": True,
- "user_can_change_branch": False,
- "user_can_change_translation_usage": False,
- "user_can_change_translations_autoimport_mode": False,
- }
- self.assertEqual(expected, permissions)
def test_getSharingDetailPermissions__ordinary_user(self):
"""An ordinary user cannot set the series' branch or translation
synchronisation settings, or the translation usage settings of the
product.
"""
- user = self.factory.makePerson(karma=100)
+ user = self.factory.makePerson()
packaging = self.factory.makePackagingLink()
sourcepackage = packaging.sourcepackage
productseries = packaging.productseries
@@ -417,7 +416,7 @@ class TestSourcePackage(TestCaseWithFactory):
user.canWrite(productseries.product, "translations_usage")
)
expected = {
- "user_can_change_product_series": True,
+ "user_can_change_product_series": False,
"user_can_change_branch": False,
"user_can_change_translation_usage": False,
"user_can_change_translations_autoimport_mode": False,
@@ -429,8 +428,8 @@ class TestSourcePackage(TestCaseWithFactory):
return self.factory.makeProductSeries(owner=self.factory.makePerson())
def test_getSharingDetailPermissions__product_owner(self):
- """A product owner can create a packaging link, and they can set the
- series' branch and the translation syncronisation settings, and the
+ """A product owner can't change a packaging link, and they can set the
+ series' branch and the translation synchronisation settings, and the
translation usage settings of the product.
"""
productseries = self.makeDistinctOwnerProductSeries()
@@ -454,7 +453,7 @@ class TestSourcePackage(TestCaseWithFactory):
)
)
expected = {
- "user_can_change_product_series": True,
+ "user_can_change_product_series": False,
"user_can_change_branch": True,
"user_can_change_translation_usage": True,
"user_can_change_translations_autoimport_mode": True,
@@ -464,34 +463,26 @@ class TestSourcePackage(TestCaseWithFactory):
def test_getSharingDetailPermissions_change_product(self):
"""Test user_can_change_product_series.
- Until a Packaging is created, anyone can change product series.
- Afterward, random people cannot change product series.
+ Random people cannot create package link or change product series.
"""
sourcepackage = self.factory.makeSourcePackage()
- person1 = self.factory.makePerson(karma=100)
- person2 = self.factory.makePerson()
+ person = self.factory.makePerson()
def can_change_product_series():
return sourcepackage.getSharingDetailPermissions()[
"user_can_change_product_series"
]
- with person_logged_in(person1):
- self.assertTrue(can_change_product_series())
- with person_logged_in(person2):
- self.assertTrue(can_change_product_series())
- self.factory.makePackagingLink(
- sourcepackage=sourcepackage, owner=person1
- )
- with person_logged_in(person1):
- self.assertTrue(can_change_product_series())
- with person_logged_in(person2):
+ with person_logged_in(person):
+ self.assertFalse(can_change_product_series())
+ self.factory.makePackagingLink(sourcepackage=sourcepackage)
+ with person_logged_in(person):
self.assertFalse(can_change_product_series())
def test_getSharingDetailPermissions_no_product_series(self):
sourcepackage = self.factory.makeSourcePackage()
expected = {
- "user_can_change_product_series": True,
+ "user_can_change_product_series": False,
"user_can_change_branch": False,
"user_can_change_translation_usage": False,
"user_can_change_translations_autoimport_mode": False,
@@ -558,8 +549,12 @@ class TestSourcePackageWebService(WebServiceTestCase):
self.assertIs(None, sourcepackage.direct_packaging)
productseries = self.factory.makeProductSeries()
transaction.commit()
- ws_sourcepackage = self.wsObject(sourcepackage)
- ws_productseries = self.wsObject(productseries)
+ ws_sourcepackage = self.wsObject(
+ sourcepackage, user=sourcepackage.distribution.owner
+ )
+ ws_productseries = self.wsObject(
+ productseries, user=sourcepackage.distribution.owner
+ )
ws_sourcepackage.setPackaging(productseries=ws_productseries)
transaction.commit()
self.assertEqual(
@@ -568,11 +563,12 @@ class TestSourcePackageWebService(WebServiceTestCase):
def test_deletePackaging(self):
"""Deleting a packaging should work."""
- user = self.factory.makePerson(karma=200)
packaging = self.factory.makePackagingLink()
sourcepackage = packaging.sourcepackage
transaction.commit()
- self.wsObject(sourcepackage, user=user).deletePackaging()
+ self.wsObject(
+ sourcepackage, user=sourcepackage.distribution.owner
+ ).deletePackaging()
transaction.commit()
self.assertIs(None, sourcepackage.direct_packaging)
@@ -580,7 +576,9 @@ class TestSourcePackageWebService(WebServiceTestCase):
"""Deleting when there's no packaging should be a no-op."""
sourcepackage = self.factory.makeSourcePackage()
transaction.commit()
- self.wsObject(sourcepackage).deletePackaging()
+ self.wsObject(
+ sourcepackage, user=sourcepackage.distribution.owner
+ ).deletePackaging()
transaction.commit()
self.assertIs(None, sourcepackage.direct_packaging)
@@ -737,7 +735,9 @@ class TestSourcePackageViews(TestCaseWithFactory):
def test_editpackaging_obsolete_series_in_vocabulary(self):
# The sourcepackage's current product series is included in
# the vocabulary even if it is obsolete.
- self.package.setPackaging(self.obsolete_productseries, self.owner)
+ removeSecurityProxy(self.package).setPackaging(
+ self.obsolete_productseries, self.owner
+ )
form = {
"field.product": "bonkers",
"field.actions.continue": "Continue",
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 3fd560c..9965749 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -208,6 +208,7 @@ from lp.registry.interfaces.ssh import ISSHKeySet
from lp.registry.model.commercialsubscription import CommercialSubscription
from lp.registry.model.karma import KarmaTotalCache
from lp.registry.model.milestone import Milestone
+from lp.registry.model.packaging import Packaging
from lp.registry.model.suitesourcepackage import SuiteSourcePackage
from lp.services.auth.interfaces import IAccessTokenSet
from lp.services.auth.utils import create_access_token_secret
@@ -1346,7 +1347,7 @@ class LaunchpadObjectFactory(ObjectFactory):
owner=None,
sourcepackage=None,
in_ubuntu=False,
- ):
+ ) -> Packaging:
assert sourcepackage is None or (
distroseries is None and sourcepackagename is None
), (
