Launchpad itself

Merge lp:~jpds/launchpad/fix_518232 into lp:launchpad

  1. fix_518232
  2. Merge into devel
Proposed by Jonathan Davies
Status: Merged
Approved by: Henning Eggers
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~jpds/launchpad/fix_518232
Merge into: lp:launchpad
Diff against target: 197 lines (+94/-20)
3 files modified
lib/lp/registry/configure.zcml (+4/-1)
lib/lp/registry/interfaces/distributionmirror.py (+35/-19)
lib/lp/registry/stories/webservice/xx-distribution-mirror.txt (+55/-0)
To merge this branch: bzr merge lp:~jpds/launchpad/fix_518232
Reviewer Review Type Date Requested Status
Henning Eggers (community) code Approve
Review via email: mp+18778@code.launchpad.net

Commit message

Restricted access to distribution mirror attributes with lp.Admin and lp.Edit accessible interfaces. Landed by henninge.

To post a comment you must log in.
Revision history for this message
Jonathan Davies (jpds) wrote :

= Summary =

Right now, anyone who uses the Launchpad API can access any attribute on IDistributionMirror, regardless if they don't have launchpad.Edit or launchpad.Admin powers.

This branch implements the necessary restrictions to allow people to access mirror data as they normally would be able to via the web UI.

Revision history for this message
Henning Eggers (henninge) wrote :

Thanks for making our webservice more secure. ;-)

Please, as discussed on IRC, make the test focus more on the attributes being restricted so that it's clearer what is beingn tested.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2010-02-04 21:18:32 +0000
+++ lib/lp/registry/configure.zcml 2010-02-10 10:40:30 +0000
@@ -1614,14 +1614,17 @@
16141614
1615 <!-- DistributionMirror -->1615 <!-- DistributionMirror -->
1616 <class class="lp.registry.model.distributionmirror.DistributionMirror">1616 <class class="lp.registry.model.distributionmirror.DistributionMirror">
1617 <allow interface="lp.registry.interfaces.distributionmirror.IDistributionMirror" />1617 <allow
1618 interface="lp.registry.interfaces.distributionmirror.IDistributionMirrorPublic" />
1618 <require1619 <require
1619 permission="launchpad.Edit"1620 permission="launchpad.Edit"
1621 interface="lp.registry.interfaces.distributionmirror.IDistributionMirrorEditRestricted"
1620 set_attributes="name displayname description whiteboard1622 set_attributes="name displayname description whiteboard
1621 http_base_url ftp_base_url rsync_base_url enabled1623 http_base_url ftp_base_url rsync_base_url enabled
1622 speed country content official_candidate owner" />1624 speed country content official_candidate owner" />
1623 <require1625 <require
1624 permission="launchpad.Admin"1626 permission="launchpad.Admin"
1627 interface="lp.registry.interfaces.distributionmirror.IDistributionMirrorAdminRestricted"
1625 set_attributes="status reviewer date_reviewed"1628 set_attributes="status reviewer date_reviewed"
1626 attributes="destroySelf" />1629 attributes="destroySelf" />
1627 </class>1630 </class>
16281631
=== modified file 'lib/lp/registry/interfaces/distributionmirror.py'
--- lib/lp/registry/interfaces/distributionmirror.py 2010-02-04 23:34:10 +0000
+++ lib/lp/registry/interfaces/distributionmirror.py 2010-02-10 10:40:30 +0000
@@ -7,6 +7,9 @@
77
8__all__ = [8__all__ = [
9'IDistributionMirror',9'IDistributionMirror',
10'IDistributionMirrorAdminRestricted',
11'IDistributionMirrorEditRestricted',
12'IDistributionMirrorPublic',
10'IMirrorDistroArchSeries',13'IMirrorDistroArchSeries',
11'IMirrorDistroSeriesSource',14'IMirrorDistroSeriesSource',
12'IMirrorProbeRecord',15'IMirrorProbeRecord',
@@ -280,20 +283,39 @@
280 def getMirrorByURI(self, url):283 def getMirrorByURI(self, url):
281 return getUtility(IDistributionMirrorSet).getByRsyncUrl(url)284 return getUtility(IDistributionMirrorSet).getByRsyncUrl(url)
282285
283286class IDistributionMirrorAdminRestricted(Interface):
284class IDistributionMirror(Interface):287 """IDistributionMirror properties requiring launchpad.Admin permission."""
285 """A mirror of a given distribution."""288
286 export_as_webservice_entry()289 reviewer = exported(PublicPersonChoice(
290 title=_('Reviewer'), required=False, readonly=True,
291 vocabulary='ValidPersonOrTeam', description=_(
292 "The person who last reviewed this mirror.")))
293 date_reviewed = exported(Datetime(
294 title=_('Date reviewed'), required=False, readonly=True,
295 description=_(
296 "The date on which this mirror was last reviewed by a mirror admin.")))
297
298
299class IDistributionMirrorEditRestricted(Interface):
300 """IDistributionMirror properties requiring launchpad.Edit permission."""
301
302 official_candidate = exported(Bool(
303 title=_('Apply to be an official mirror of this distribution'),
304 required=False, readonly=False, default=True))
305 whiteboard = exported(Whiteboard(
306 title=_('Whiteboard'), required=False, readonly=False,
307 description=_("Notes on the current status of the mirror (only "
308 "visible to admins and the mirror's registrant).")))
309
310
311class IDistributionMirrorPublic(Interface):
312 """Public IDistributionMirror properties."""
287313
288 id = Int(title=_('The unique id'), required=True, readonly=True)314 id = Int(title=_('The unique id'), required=True, readonly=True)
289 owner = exported(PublicPersonChoice(315 owner = exported(PublicPersonChoice(
290 title=_('Owner'), readonly=False, vocabulary='ValidOwner',316 title=_('Owner'), readonly=False, vocabulary='ValidOwner',
291 required=True, description=_(317 required=True, description=_(
292 "The person who is set as the current administrator of this mirror.")))318 "The person who is set as the current administrator of this mirror.")))
293 reviewer = exported(PublicPersonChoice(
294 title=_('Reviewer'), required=False, readonly=True,
295 vocabulary='ValidPersonOrTeam', description=_(
296 "The person who last reviewed this mirror.")))
297 distribution = exported(319 distribution = exported(
298 Reference(320 Reference(
299 Interface,321 Interface,
@@ -341,9 +363,6 @@
341 'this distribution. Choose "Archive" if this is a '363 'this distribution. Choose "Archive" if this is a '
342 'mirror of packages for this distribution.'),364 'mirror of packages for this distribution.'),
343 vocabulary=MirrorContent))365 vocabulary=MirrorContent))
344 official_candidate = exported(Bool(
345 title=_('Apply to be an official mirror of this distribution'),
346 required=False, readonly=False, default=True))
347 status = exported(Choice(366 status = exported(Choice(
348 title=_('Status'), required=True, readonly=False,367 title=_('Status'), required=True, readonly=False,
349 vocabulary=MirrorStatus,368 vocabulary=MirrorStatus,
@@ -364,14 +383,6 @@
364 date_created = exported(Datetime(383 date_created = exported(Datetime(
365 title=_('Date Created'), required=True, readonly=True,384 title=_('Date Created'), required=True, readonly=True,
366 description=_("The date on which this mirror was registered.")))385 description=_("The date on which this mirror was registered.")))
367 date_reviewed = exported(Datetime(
368 title=_('Date reviewed'), required=False, readonly=True,
369 description=_(
370 "The date on which this mirror was last reviewed by a mirror admin.")))
371 whiteboard = exported(Whiteboard(
372 title=_('Whiteboard'), required=False, readonly=False,
373 description=_("Notes on the current status of the mirror (only "
374 "visible to admins and the mirror's registrant).")))
375386
376 @invariant387 @invariant
377 def mirrorMustHaveHTTPOrFTPURL(mirror):388 def mirrorMustHaveHTTPOrFTPURL(mirror):
@@ -507,6 +518,11 @@
507 Sources.gz file refer to and the path to the file itself.518 Sources.gz file refer to and the path to the file itself.
508 """519 """
509520
521class IDistributionMirror(IDistributionMirrorAdminRestricted,
522 IDistributionMirrorEditRestricted, IDistributionMirrorPublic):
523 """A mirror of a given distribution."""
524 export_as_webservice_entry()
525
510526
511class UnableToFetchCDImageFileList(Exception):527class UnableToFetchCDImageFileList(Exception):
512 """Couldn't fetch the file list needed for probing cdimage mirrors."""528 """Couldn't fetch the file list needed for probing cdimage mirrors."""
513529
=== modified file 'lib/lp/registry/stories/webservice/xx-distribution-mirror.txt'
--- lib/lp/registry/stories/webservice/xx-distribution-mirror.txt 2010-02-05 13:48:10 +0000
+++ lib/lp/registry/stories/webservice/xx-distribution-mirror.txt 2010-02-10 10:40:30 +0000
@@ -70,16 +70,71 @@
70 >>> login(ANONYMOUS)70 >>> login(ANONYMOUS)
71 >>> karl_db = getUtility(IPersonSet).getByName('karl')71 >>> karl_db = getUtility(IPersonSet).getByName('karl')
72 >>> test_db = getUtility(IPersonSet).getByName('name12')72 >>> test_db = getUtility(IPersonSet).getByName('name12')
73 >>> no_priv_db = getUtility(IPersonSet).getByName('no-priv')
73 >>> karl_webservice = webservice_for_person(karl_db,74 >>> karl_webservice = webservice_for_person(karl_db,
74 ... permission=OAuthPermission.WRITE_PUBLIC)75 ... permission=OAuthPermission.WRITE_PUBLIC)
75 >>> test_webservice = webservice_for_person(test_db,76 >>> test_webservice = webservice_for_person(test_db,
76 ... permission=OAuthPermission.WRITE_PUBLIC)77 ... permission=OAuthPermission.WRITE_PUBLIC)
78 >>> no_priv_webservice = webservice_for_person(no_priv_db,
79 ... permission=OAuthPermission.READ_PUBLIC)
77 >>> logout()80 >>> logout()
78 >>> karl = webservice.get("/~karl").jsonBody()81 >>> karl = webservice.get("/~karl").jsonBody()
79 >>> patch = {82 >>> patch = {
80 ... u'owner_link': karl['self_link']83 ... u'owner_link': karl['self_link']
81 ... }84 ... }
8285
86One must have special permissions to access certain attributes:
87
88 >>> archive_404_mirror = ubuntu_archive_mirrors['entries'][1]
89 >>> response = no_priv_webservice.get(
90 ... archive_404_mirror['self_link']).jsonBody()
91 >>> pprint_entry(response)
92 content: ...
93 ...
94 date_reviewed: ...redacted...
95 ...
96 official_candidate: ...redacted...
97 ...
98 reviewer_link: ...redacted...
99 ...
100 whiteboard: ...redacted...
101
102Mirror registrars may see some:
103
104 >>> response = test_webservice.get(
105 ... archive_404_mirror['self_link']).jsonBody()
106 >>> pprint_entry(response)
107 content: ...
108 ...
109 date_reviewed: ...redacted...
110 ...
111 reviewer_link: ...redacted...
112
113Mirror listing admins may see all:
114
115 >>> response = karl_webservice.get(
116 ... archive_404_mirror['self_link']).jsonBody()
117 >>> pprint_entry(response)
118 content: u'Archive'
119 date_created: u'2006-10-16T18:31:43.438573+00:00'
120 date_reviewed: None
121 description: None
122 displayname: None
123 distribution_link: u'http://.../ubuntu'
124 enabled: True
125 ftp_base_url: None
126 http_base_url: u'http://localhost:11375/archive-mirror/'
127 name: u'archive-404-mirror'
128 official_candidate: True
129 owner_link: u'http://.../~name12'
130 resource_type_link: u'http://.../#distribution_mirror'
131 reviewer_link: None
132 rsync_base_url: None
133 self_link: u'http://.../ubuntu/+mirror/archive-404-mirror'
134 speed: u'512 Kbps'
135 status: u'Official'
136 whiteboard: None
137
83Now trying to set the owner using Sample Person's webservice is not authorized.138Now trying to set the owner using Sample Person's webservice is not authorized.
84139
85 >>> response = test_webservice.patch(140 >>> response = test_webservice.patch(