launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #28526
[Merge] ~lgp171188/launchpad:new-security-role-permission-ui into launchpad:master
Guruprasad has proposed merging ~lgp171188/launchpad:new-security-role-permission-ui into launchpad:master.
Commit message:
Implement the UI for showing and setting Distribution.security_admin
Also export IDistribution.security_admin in the web service.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~lgp171188/launchpad/+git/launchpad/+merge/423670
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/launchpad:new-security-role-permission-ui into launchpad:master.
diff --git a/lib/lp/registry/browser/configure.zcml b/lib/lp/registry/browser/configure.zcml
index bc05ae3..cd1bee9 100644
--- a/lib/lp/registry/browser/configure.zcml
+++ b/lib/lp/registry/browser/configure.zcml
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2020 Canonical Ltd. This software is licensed under the
+<!-- Copyright 2009-2022 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
@@ -2332,6 +2332,13 @@
template="../../app/templates/generic-edit.pt"
/>
<browser:page
+ name="+select-security-admins"
+ for="lp.registry.interfaces.distribution.IDistribution"
+ class="lp.registry.browser.distribution.DistributionChangeSecurityAdminView"
+ permission="launchpad.Edit"
+ template="../../app/templates/generic-edit.pt"
+ />
+ <browser:page
name="+pubconf"
for="lp.registry.interfaces.distribution.IDistribution"
class="lp.registry.browser.distribution.DistributionPublisherConfigView"
diff --git a/lib/lp/registry/browser/distribution.py b/lib/lp/registry/browser/distribution.py
index 5fccb6c..80ea77e 100644
--- a/lib/lp/registry/browser/distribution.py
+++ b/lib/lp/registry/browser/distribution.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2022 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Browser views for distributions."""
@@ -12,6 +12,7 @@ __all__ = [
'DistributionChangeMembersView',
'DistributionChangeMirrorAdminView',
'DistributionChangeOCIProjectAdminView',
+ 'DistributionChangeSecurityAdminView',
'DistributionCountryArchiveMirrorsView',
'DistributionDisabledMirrorsView',
'DistributionEditView',
@@ -417,6 +418,7 @@ class DistributionOverviewMenu(ApplicationMenu, DistributionLinksMixin):
'members',
'mirror_admin',
'oci_project_admin',
+ 'security_admin',
'reassign',
'addseries',
'series',
@@ -510,6 +512,11 @@ class DistributionOverviewMenu(ApplicationMenu, DistributionLinksMixin):
text = 'Change OCI project admins'
return Link('+select-oci-project-admins', text, icon='edit')
+ @enabled_with_permission('launchpad.Edit')
+ def security_admin(self):
+ text = 'Change security admins'
+ return Link('+select-security-admins', text, icon='edit')
+
def search(self):
text = 'Search packages'
return Link('+search', text, icon='search')
@@ -793,6 +800,26 @@ class DistributionView(PillarViewMixin, HasAnnouncementsView, FeedsMixin):
null_display_value=empty_value,
step_title='Select a new OCI project administrator')
+ @property
+ def security_admin_widget(self):
+ if canWrite(self.context, 'security_admin'):
+ empty_value = ' Specify a security administrator'
+ else:
+ empty_value = 'None'
+
+ return InlinePersonEditPickerWidget(
+ self.context,
+ IDistribution['security_admin'],
+ format_link(
+ self.context.security_admin,
+ empty_value=empty_value,
+ ),
+ header='Change the security administrator',
+ edit_view='+select-security-admins',
+ null_display_value=empty_value,
+ step_title='Select a new security administrator'
+ )
+
def linkedMilestonesForSeries(self, series):
"""Return a string of linkified milestones in the series."""
# Listify to remove repeated queries.
@@ -1226,6 +1253,19 @@ class DistributionChangeOCIProjectAdminView(RegistryEditFormView):
self.context.displayname)
+class DistributionChangeSecurityAdminView(RegistryEditFormView):
+ """A vie to change the security administrator."""
+ schema = IDistribution
+ field_names = ['security_admin']
+
+ @property
+ def label(self):
+ """See `LaunchpadFormView`."""
+ return "Change the %s security administrator" % (
+ self.context.displayname
+ )
+
+
class DistributionChangeMembersView(RegistryEditFormView):
"""A view to change the members team."""
schema = IDistribution
diff --git a/lib/lp/registry/browser/tests/distribution-views.txt b/lib/lp/registry/browser/tests/distribution-views.txt
index 2407a78..eadb671 100644
--- a/lib/lp/registry/browser/tests/distribution-views.txt
+++ b/lib/lp/registry/browser/tests/distribution-views.txt
@@ -261,6 +261,55 @@ Only admins and owners can access the view.
False
+Changing a distribution security administrator
+----------------------------------------------
+
+The +select-security-admins view allows the owner or admin to change the
+security administrator.
+
+ >>> login("admin@xxxxxxxxxxxxx")
+ >>> view = create_view(distribution, '+select-security-admins')
+ >>> print(view.label)
+ Change the YoUbuntu security administrator
+
+ >>> print(view.page_title)
+ Change the YoUbuntu security administrator
+
+ >>> print(view.cancel_url)
+ http://launchpad.test/youbuntu
+
+The view accepts the security_admin field.
+
+ >>> print(distribution.security_admin)
+ None
+
+ >>> view.field_names
+ ['security_admin']
+
+ >>> form = {
+ ... 'field.security_admin': 'no-priv',
+ ... 'field.actions.change': 'Change',
+ ... }
+ >>> view = create_initialized_view(
+ ... distribution, '+select-security-admins', form=form)
+ >>> view.errors
+ []
+ >>> print(view.next_url)
+ http://launchpad.test/youbuntu
+
+ >>> print(distribution.security_admin.name)
+ no-priv
+
+Only admins and owners can access the view.
+
+ >>> check_permission('launchpad.Edit', view)
+ True
+
+ >>> login('no-priv@xxxxxxxxxxxxx')
+ >>> check_permission('launchpad.Edit', view)
+ False
+
+
Changing a distribution members team
------------------------------------
diff --git a/lib/lp/registry/interfaces/distribution.py b/lib/lp/registry/interfaces/distribution.py
index 727d204..7261a85 100644
--- a/lib/lp/registry/interfaces/distribution.py
+++ b/lib/lp/registry/interfaces/distribution.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2022 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Interfaces including and related to IDistribution."""
@@ -40,6 +40,7 @@ from lazr.restful.declarations import (
from lazr.restful.fields import (
CollectionField,
Reference,
+ ReferenceChoice,
)
from lazr.restful.interface import copy_field
from zope.interface import (
@@ -480,10 +481,14 @@ class IDistributionView(
has_current_commercial_subscription = Attribute(
"Whether the distribution has a current commercial subscription.")
- security_admin = Choice(
- title=_("Security Administrator"),
- description=_("The distribution security administrator."),
- required=False, vocabulary='ValidPersonOrTeam')
+ security_admin = exported(
+ ReferenceChoice(
+ title=_("Security Administrator"),
+ description=_("The distribution security administrator."),
+ required=False, vocabulary='ValidPersonOrTeam',
+ schema=IPerson,
+ ),
+ )
vulnerabilities = exported(
doNotSnapshot(CollectionField(
diff --git a/lib/lp/registry/stories/webservice/xx-distribution.txt b/lib/lp/registry/stories/webservice/xx-distribution.txt
index 51eeff6..ac21891 100644
--- a/lib/lp/registry/stories/webservice/xx-distribution.txt
+++ b/lib/lp/registry/stories/webservice/xx-distribution.txt
@@ -61,6 +61,7 @@ And for every distribution we publish most of its attributes.
redirect_release_uploads: False
registrant_link: 'http://.../~registry'
resource_type_link: 'http://.../#distribution'
+ security_admin_link: None
self_link: 'http://.../ubuntu'
series_collection_link: 'http://.../ubuntu/series'
summary: 'Ubuntu is a new approach to Linux Distribution...'
diff --git a/lib/lp/registry/templates/distribution-details.pt b/lib/lp/registry/templates/distribution-details.pt
index 1d2b0cf..60cf1ef 100644
--- a/lib/lp/registry/templates/distribution-details.pt
+++ b/lib/lp/registry/templates/distribution-details.pt
@@ -31,6 +31,11 @@
<dt>OCI project admins:</dt>
<dd tal:content="structure view/oci_project_admin_widget" />
</dl>
+
+ <dl id="security-admin">
+ <dt>Security admin:</dt>
+ <dd tal:content="structure view/security_admin_widget" />
+ </dl>
</div>
<dl id="uploaders" style="clear:left">
diff --git a/lib/lp/registry/tests/test_distribution.py b/lib/lp/registry/tests/test_distribution.py
index c44df04..9d0491d 100644
--- a/lib/lp/registry/tests/test_distribution.py
+++ b/lib/lp/registry/tests/test_distribution.py
@@ -1,9 +1,10 @@
-# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2022 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for Distribution."""
import datetime
+import json
from fixtures import FakeLogger
from lazr.lifecycle.snapshot import Snapshot
@@ -1809,6 +1810,103 @@ class TestDistributionWebservice(OCIConfigHelperMixin, TestCaseWithFactory):
self.assertTrue(len(mirrors) > 1, "Not enough mirrors")
self.assertEqual(main_mirror, mirrors[-1])
+ def test_distribution_security_admin_unset(self):
+ with person_logged_in(self.person):
+ distro = self.factory.makeDistribution(owner=self.person)
+ distro_url = api_url(distro)
+
+ response = self.webservice.get(distro_url)
+ json_body = response.jsonBody()
+ self.assertEqual(200, response.status)
+ self.assertIsNone(json_body['security_admin_link'])
+
+ def test_distribution_security_admin_set(self):
+ with person_logged_in(self.person):
+ distro = self.factory.makeDistribution(owner=self.person)
+ distro_url = api_url(distro)
+ person = self.factory.makePerson()
+ distro.security_admin = person
+ person_url = 'http://api.launchpad.test/devel' + api_url(person)
+
+ response = self.webservice.get(distro_url)
+ json_body = response.jsonBody()
+ self.assertEqual(200, response.status)
+ self.assertEqual(
+ person_url,
+ json_body['security_admin_link'],
+ )
+
+ def test_admin_can_set_distribution_security_admin(self):
+ with admin_logged_in():
+ distro = self.factory.makeDistribution()
+ person = self.factory.makePerson()
+ person_url = 'http://api.launchpad.test/devel' + api_url(person)
+ self.assertIsNone(distro.security_admin)
+ admin_user = getUtility(IPersonSet).getByEmail(
+ 'admin@xxxxxxxxxxxxx'
+ )
+ distro_url = api_url(distro)
+
+ webservice = webservice_for_person(
+ admin_user, permission=OAuthPermission.WRITE_PUBLIC,
+ default_api_version="devel"
+ )
+
+ response = webservice.patch(
+ distro_url,
+ "application/json",
+ json.dumps(
+ {
+ "security_admin_link": person_url
+ }
+ )
+ )
+ self.assertEqual(209, response.status)
+ with admin_logged_in():
+ self.assertEqual(person, distro.security_admin)
+
+ def test_distribution_owner_can_set_security_admin(self):
+ with person_logged_in(self.person):
+ distro = self.factory.makeDistribution(owner=self.person)
+ self.assertIsNone(distro.security_admin)
+ distro_url = api_url(distro)
+ person_url = (
+ 'http://api.launchpad.test/devel' + api_url(self.person)
+ )
+
+ response = self.webservice.patch(
+ distro_url,
+ 'application/json',
+ json.dumps(
+ {
+ 'security_admin_link': person_url,
+ }
+ )
+ )
+ self.assertEqual(209, response.status)
+
+ with person_logged_in(self.person):
+ self.assertEqual(self.person, distro.security_admin)
+
+ def test_others_cannot_set_distribution_security_admin(self):
+ with person_logged_in(self.person):
+ distro = self.factory.makeDistribution()
+ distro_url = api_url(distro)
+ person_url = (
+ 'http://api.launchpad.test/devel' + api_url(self.person)
+ )
+
+ response = self.webservice.patch(
+ distro_url,
+ 'application/json',
+ json.dumps(
+ {
+ 'security_admin_link': person_url,
+ }
+ )
+ )
+ self.assertEqual(401, response.status)
+
class TestDistributionVulnerabilities(TestCaseWithFactory):
