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
1=== modified file 'lib/lp/registry/configure.zcml'
2--- lib/lp/registry/configure.zcml 2010-02-04 21:18:32 +0000
3+++ lib/lp/registry/configure.zcml 2010-02-10 10:40:30 +0000
4@@ -1614,14 +1614,17 @@
5
6 <!-- DistributionMirror -->
7 <class class="lp.registry.model.distributionmirror.DistributionMirror">
8- <allow interface="lp.registry.interfaces.distributionmirror.IDistributionMirror" />
9+ <allow
10+ interface="lp.registry.interfaces.distributionmirror.IDistributionMirrorPublic" />
11 <require
12 permission="launchpad.Edit"
13+ interface="lp.registry.interfaces.distributionmirror.IDistributionMirrorEditRestricted"
14 set_attributes="name displayname description whiteboard
15 http_base_url ftp_base_url rsync_base_url enabled
16 speed country content official_candidate owner" />
17 <require
18 permission="launchpad.Admin"
19+ interface="lp.registry.interfaces.distributionmirror.IDistributionMirrorAdminRestricted"
20 set_attributes="status reviewer date_reviewed"
21 attributes="destroySelf" />
22 </class>
23
24=== modified file 'lib/lp/registry/interfaces/distributionmirror.py'
25--- lib/lp/registry/interfaces/distributionmirror.py 2010-02-04 23:34:10 +0000
26+++ lib/lp/registry/interfaces/distributionmirror.py 2010-02-10 10:40:30 +0000
27@@ -7,6 +7,9 @@
28
29 __all__ = [
30 'IDistributionMirror',
31+'IDistributionMirrorAdminRestricted',
32+'IDistributionMirrorEditRestricted',
33+'IDistributionMirrorPublic',
34 'IMirrorDistroArchSeries',
35 'IMirrorDistroSeriesSource',
36 'IMirrorProbeRecord',
37@@ -280,20 +283,39 @@
38 def getMirrorByURI(self, url):
39 return getUtility(IDistributionMirrorSet).getByRsyncUrl(url)
40
41-
42-class IDistributionMirror(Interface):
43- """A mirror of a given distribution."""
44- export_as_webservice_entry()
45+class IDistributionMirrorAdminRestricted(Interface):
46+ """IDistributionMirror properties requiring launchpad.Admin permission."""
47+
48+ reviewer = exported(PublicPersonChoice(
49+ title=_('Reviewer'), required=False, readonly=True,
50+ vocabulary='ValidPersonOrTeam', description=_(
51+ "The person who last reviewed this mirror.")))
52+ date_reviewed = exported(Datetime(
53+ title=_('Date reviewed'), required=False, readonly=True,
54+ description=_(
55+ "The date on which this mirror was last reviewed by a mirror admin.")))
56+
57+
58+class IDistributionMirrorEditRestricted(Interface):
59+ """IDistributionMirror properties requiring launchpad.Edit permission."""
60+
61+ official_candidate = exported(Bool(
62+ title=_('Apply to be an official mirror of this distribution'),
63+ required=False, readonly=False, default=True))
64+ whiteboard = exported(Whiteboard(
65+ title=_('Whiteboard'), required=False, readonly=False,
66+ description=_("Notes on the current status of the mirror (only "
67+ "visible to admins and the mirror's registrant).")))
68+
69+
70+class IDistributionMirrorPublic(Interface):
71+ """Public IDistributionMirror properties."""
72
73 id = Int(title=_('The unique id'), required=True, readonly=True)
74 owner = exported(PublicPersonChoice(
75 title=_('Owner'), readonly=False, vocabulary='ValidOwner',
76 required=True, description=_(
77 "The person who is set as the current administrator of this mirror.")))
78- reviewer = exported(PublicPersonChoice(
79- title=_('Reviewer'), required=False, readonly=True,
80- vocabulary='ValidPersonOrTeam', description=_(
81- "The person who last reviewed this mirror.")))
82 distribution = exported(
83 Reference(
84 Interface,
85@@ -341,9 +363,6 @@
86 'this distribution. Choose "Archive" if this is a '
87 'mirror of packages for this distribution.'),
88 vocabulary=MirrorContent))
89- official_candidate = exported(Bool(
90- title=_('Apply to be an official mirror of this distribution'),
91- required=False, readonly=False, default=True))
92 status = exported(Choice(
93 title=_('Status'), required=True, readonly=False,
94 vocabulary=MirrorStatus,
95@@ -364,14 +383,6 @@
96 date_created = exported(Datetime(
97 title=_('Date Created'), required=True, readonly=True,
98 description=_("The date on which this mirror was registered.")))
99- date_reviewed = exported(Datetime(
100- title=_('Date reviewed'), required=False, readonly=True,
101- description=_(
102- "The date on which this mirror was last reviewed by a mirror admin.")))
103- whiteboard = exported(Whiteboard(
104- title=_('Whiteboard'), required=False, readonly=False,
105- description=_("Notes on the current status of the mirror (only "
106- "visible to admins and the mirror's registrant).")))
107
108 @invariant
109 def mirrorMustHaveHTTPOrFTPURL(mirror):
110@@ -507,6 +518,11 @@
111 Sources.gz file refer to and the path to the file itself.
112 """
113
114+class IDistributionMirror(IDistributionMirrorAdminRestricted,
115+ IDistributionMirrorEditRestricted, IDistributionMirrorPublic):
116+ """A mirror of a given distribution."""
117+ export_as_webservice_entry()
118+
119
120 class UnableToFetchCDImageFileList(Exception):
121 """Couldn't fetch the file list needed for probing cdimage mirrors."""
122
123=== modified file 'lib/lp/registry/stories/webservice/xx-distribution-mirror.txt'
124--- lib/lp/registry/stories/webservice/xx-distribution-mirror.txt 2010-02-05 13:48:10 +0000
125+++ lib/lp/registry/stories/webservice/xx-distribution-mirror.txt 2010-02-10 10:40:30 +0000
126@@ -70,16 +70,71 @@
127 >>> login(ANONYMOUS)
128 >>> karl_db = getUtility(IPersonSet).getByName('karl')
129 >>> test_db = getUtility(IPersonSet).getByName('name12')
130+ >>> no_priv_db = getUtility(IPersonSet).getByName('no-priv')
131 >>> karl_webservice = webservice_for_person(karl_db,
132 ... permission=OAuthPermission.WRITE_PUBLIC)
133 >>> test_webservice = webservice_for_person(test_db,
134 ... permission=OAuthPermission.WRITE_PUBLIC)
135+ >>> no_priv_webservice = webservice_for_person(no_priv_db,
136+ ... permission=OAuthPermission.READ_PUBLIC)
137 >>> logout()
138 >>> karl = webservice.get("/~karl").jsonBody()
139 >>> patch = {
140 ... u'owner_link': karl['self_link']
141 ... }
142
143+One must have special permissions to access certain attributes:
144+
145+ >>> archive_404_mirror = ubuntu_archive_mirrors['entries'][1]
146+ >>> response = no_priv_webservice.get(
147+ ... archive_404_mirror['self_link']).jsonBody()
148+ >>> pprint_entry(response)
149+ content: ...
150+ ...
151+ date_reviewed: ...redacted...
152+ ...
153+ official_candidate: ...redacted...
154+ ...
155+ reviewer_link: ...redacted...
156+ ...
157+ whiteboard: ...redacted...
158+
159+Mirror registrars may see some:
160+
161+ >>> response = test_webservice.get(
162+ ... archive_404_mirror['self_link']).jsonBody()
163+ >>> pprint_entry(response)
164+ content: ...
165+ ...
166+ date_reviewed: ...redacted...
167+ ...
168+ reviewer_link: ...redacted...
169+
170+Mirror listing admins may see all:
171+
172+ >>> response = karl_webservice.get(
173+ ... archive_404_mirror['self_link']).jsonBody()
174+ >>> pprint_entry(response)
175+ content: u'Archive'
176+ date_created: u'2006-10-16T18:31:43.438573+00:00'
177+ date_reviewed: None
178+ description: None
179+ displayname: None
180+ distribution_link: u'http://.../ubuntu'
181+ enabled: True
182+ ftp_base_url: None
183+ http_base_url: u'http://localhost:11375/archive-mirror/'
184+ name: u'archive-404-mirror'
185+ official_candidate: True
186+ owner_link: u'http://.../~name12'
187+ resource_type_link: u'http://.../#distribution_mirror'
188+ reviewer_link: None
189+ rsync_base_url: None
190+ self_link: u'http://.../ubuntu/+mirror/archive-404-mirror'
191+ speed: u'512 Kbps'
192+ status: u'Official'
193+ whiteboard: None
194+
195 Now trying to set the owner using Sample Person's webservice is not authorized.
196
197 >>> response = test_webservice.patch(