Merge lp:~jamalta/launchpad/515761-anonymrelease into lp:launchpad/db-devel

Proposed by Jamal Fanaian
Status: Merged
Approved by: Brad Crittenden
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~jamalta/launchpad/515761-anonymrelease
Merge into: lp:launchpad/db-devel
Diff against target: 270 lines (+84/-30) (has conflicts)
7 files modified
lib/canonical/launchpad/security.py (+36/-17)
lib/lp/bugs/interfaces/bugtask.py (+2/-4)
lib/lp/bugs/stories/webservice/xx-bug.txt (+12/-7)
lib/lp/registry/browser/configure.zcml (+1/-1)
lib/lp/registry/browser/productseries.py (+1/-1)
lib/lp/registry/stories/webservice/xx-project-registry.txt (+14/-0)
lib/lp/testing/factory.py (+18/-0)
Text conflict in lib/canonical/launchpad/security.py
Text conflict in lib/lp/testing/factory.py
To merge this branch: bzr merge lp:~jamalta/launchpad/515761-anonymrelease
Reviewer Review Type Date Requested Status
Paul Hummer (community) code Approve
Review via email: mp+18641@code.launchpad.net

Commit message

Created View classes for IMessage, IProductRelease, and IBugSubscription to allow anonymous API access to these interfaces. Fixed misspelling of 'permission' attribute for ViewProductSeries. Fixed security problem in test xx-product-package-pages with ubuntupkg.

To post a comment you must log in.
Revision history for this message
Jamal Fanaian (jamalta) wrote :

= Summary =

Anonymous API Access to some collections returns nothing.

== Proposed fix ==

Create View classes in security.py for the interfaces that need anonymous access.

== Pre-implementation notes ==

Decision to make these collections available to everyone needs to be considered during review.

== Implementation details ==

Created ViewBugMessage, ViewProductRelease, and ViewBugSubscription in security.py with checkUnauthorized and checkAuthorized both returning True.

== Tests ==

% bin/test -vvct webservice/xx-bug.txt
% bin/test -vvct xx-project-registry

== Demo and Q/A ==

>>> from launchpadlib.launchpad import Launchpad
>>> launchpad = Launchpad.login('test', '', '', 'https://api.launchpad.dev/beta/')
>>> list(launchpad.projects['alsa-utils'].series)
[<project_series at https://api.launchpad.dev/beta/alsa-utils/trunk>]
>>> list(launchpad.bugs[1].messages)
[<message at https://api.launchpad.dev/beta/firefox/+bug/1/comments/0>, <message at https://api.launchpad.dev/beta/firefox/+bug/1/comments/1>]
>>> list(launchpad.bugs[1].subscriptions)
[<bug_subscription at https://api.launchpad.dev/beta/bugs/1/+subscription/stevea>, <bug_subscription at https://api.launchpad.dev/beta/bugs/1/+subscription/name12>]

== Launchpad lint ==

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/security.py
  lib/canonical/launchpad/apidoc/wadl-testrunner.xml
  lib/lp/bugs/stories/webservice/xx-bug.txt
  lib/lp/registry/stories/webservice/xx-project-registry.txt

Revision history for this message
Jamal Fanaian (jamalta) wrote :

= Summary =

Anonymous API Access to some collections returns nothing.

== Proposed fix ==

Create View classes in security.py for the interfaces that need anonymous access.

== Pre-implementation notes ==

Decision to make these collections available to everyone needs to be considered during review.

== Implementation details ==

Created ViewBugMessage, ViewProductRelease, and ViewBugSubscription in security.py with checkUnauthorized and checkAuthorized both returning True. Also had to modify permissions for ubuntupkg which bac caught.

== Tests ==

 % bin/test -vvct webservice/xx-bug.txt
 % bin/test -vvct xx-project-registry
 % bin/test -vvct xx-product-package-pages.txt

== Demo and Q/A ==

>>> from launchpadlib.launchpad import Launchpad
>>> launchpad = Launchpad.login('test', '', '', 'https://api.launchpad.dev/beta/')
>>> list(launchpad.projects['alsa-utils'].series)
[<project_series at https://api.launchpad.dev/beta/alsa-utils/trunk>]
>>> list(launchpad.bugs[1].messages)
[<message at https://api.launchpad.dev/beta/firefox/+bug/1/comments/0>, <message at https://api.launchpad.dev/beta/firefox/+bug/1/comments/1>]
>>> list(launchpad.bugs[1].subscriptions)
[<bug_subscription at https://api.launchpad.dev/beta/bugs/1/+subscription/stevea>, <bug_subscription at https://api.launchpad.dev/beta/bugs/1/+subscription/name12>]

== Launchpad lint ==

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/security.py
  lib/canonical/launchpad/apidoc/wadl-testrunner.xml
  lib/lp/bugs/stories/webservice/xx-bug.txt
  lib/lp/registry/stories/webservice/xx-project-registry.txt

Revision history for this message
Paul Hummer (rockstar) wrote :

<rockstar> jamalta, hm, maybe you should have a single class that inherits from AuthorizationBase that deals with checkAuthenticated and checkUnauthenticated - There's a lot of repetitive code here.
<jamalta> rockstar: i was thinking of doing that but no other class does that
<rockstar> jamalta, well, that may be for hysterical reasons. I don't see why new code should be like that.
<jamalta> and then there's the message on launchpad-dev from henning talking about refactoring security.py altogether
<jamalta> rockstar: i could do that if you would prefer though
<rockstar> jamalta, I say do it. Talk on a mailing list gets trumped by actual code.
<jamalta> rockstar: haha, sounds good :)
<jamalta> ViewByAnyUser a good name?
<rockstar> jamalta, AnonymousAuthorization ?
<jamalta> rockstar: even better

review: Needs Fixing (code)
Revision history for this message
Jamal Fanaian (jamalta) wrote :

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py 2010-02-04 22:39:09 +0000
+++ lib/canonical/launchpad/security.py 2010-02-04 23:14:01 +0000
@@ -172,6 +172,19 @@
         return True

+class AnonymousAuthorization(AuthorizationBase):
+ """Allow any authenticated and unauthenticated user access."""
+ permission = 'launchpad.View'
+
+ def checkUnauthenticated(self):
+ """Any unauthorized user can see this object."""
+ return True
+
+ def checkAuthenticated(self, user):
+ """Any authorized user can see this object."""
+ return True
+
+
 class AdminByAdminsTeam(AuthorizationBase):
     permission = 'launchpad.Admin'
     usedfor = Interface
@@ -815,24 +828,9 @@
                 user.in_admin)

-class ViewProductSeries(AuthorizationBase):
+class ViewProductSeries(AnonymousAuthorization):

     usedfor = IProductSeries
- permission = 'launchpad.View'
-
- def checkUnauthenticated(self):
- """See `IAuthorization.checkUnauthenticated`.
-
- :return: True or False.
- """
- return True
-
- def checkAuthenticated(self, user):
- """See `IAuthorization.checkAuthenticated`.
-
- :return: True or False.
- """
- return True

 class EditProductSeries(EditByOwnersOrAdmins):
@@ -980,44 +978,14 @@
             self, bugattachment.bug)

-class ViewBugSubscription(AuthorizationBase):
+class ViewBugSubscription(AnonymousAuthorization):

     usedfor = IBugSubscription
- permission = 'launchpad.View'
-
- def checkUnauthenticated(self):
- """See `IAuthorization.checkUnauthenticated`.
-
- :return: True or False.
- """
- return True
-
- def checkAuthenticated(self, user):
- """See `IAuthorization.checkAuthenticated`.
-
- :return: True or False.
- """
- return True
-
-
-class ViewBugMessage(AuthorizationBase):
+
+
+class ViewBugMessage(AnonymousAuthorization):

     usedfor = IMessage
- permission = 'launchpad.View'
-
- def checkUnauthenticated(self):
- """See `IAuthorization.checkUnauthenticated`.
-
- :return: True or False.
- """
- return True
-
- def checkAuthenticated(self, user):
- """See `IAuthorization.checkAuthenticated`.
-
- :return: True or False.
- """
- return True

 class ViewAnnouncement(AuthorizationBase):
@@ -1323,24 +1291,9 @@
             self, user)

-class ViewProductRelease(AuthorizationBase):
+class ViewProductRelease(AnonymousAuthorization):

     usedfor = IProductRelease
- permission = 'launchpad.View'
-
- def checkUnauthenticated(self):
- """See `IAuthorization.checkUnauthenticated`.
-
- :return: True or False.
- """
- return True
-
- def checkAuthenticated(self, user):
- """See `IAuthorization.checkAuthenticated`.
-
- :return: True or False.
- """
- return True

 class AdminTranslationImportQueueEntry(AuthorizationBase):

Revision history for this message
Jamal Fanaian (jamalta) wrote :

Paul,

That was the diff for the last changes you requested. Based on salgado's recommendation I posted a message to launchpad-dev to confirm any security concerns with these changes.

Thanks again for reviewing this.

Revision history for this message
Paul Hummer (rockstar) wrote :

Thanks for making the changes I asked for.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/security.py'
2--- lib/canonical/launchpad/security.py 2010-02-04 00:01:23 +0000
3+++ lib/canonical/launchpad/security.py 2010-02-08 14:16:33 +0000
4@@ -29,6 +29,7 @@
5 from lp.bugs.interfaces.bugattachment import IBugAttachment
6 from lp.bugs.interfaces.bugbranch import IBugBranch
7 from lp.bugs.interfaces.bugnomination import IBugNomination
8+from lp.bugs.interfaces.bugsubscription import IBugSubscription
9 from lp.bugs.interfaces.bugtracker import IBugTracker
10 from lp.soyuz.interfaces.build import IBuild
11 from lp.buildmaster.interfaces.builder import IBuilder, IBuilderSet
12@@ -63,6 +64,7 @@
13 from lp.registry.interfaces.mailinglist import IMailingListSet
14 from lp.registry.interfaces.milestone import (
15 IMilestone, IProjectMilestone)
16+from canonical.launchpad.interfaces.message import IMessage
17 from canonical.launchpad.interfaces.oauth import (
18 IOAuthAccessToken, IOAuthRequestToken)
19 from lp.soyuz.interfaces.packageset import IPackageset, IPackagesetSet
20@@ -170,6 +172,19 @@
21 return True
22
23
24+class AnonymousAuthorization(AuthorizationBase):
25+ """Allow any authenticated and unauthenticated user access."""
26+ permission = 'launchpad.View'
27+
28+ def checkUnauthenticated(self):
29+ """Any unauthorized user can see this object."""
30+ return True
31+
32+ def checkAuthenticated(self, user):
33+ """Any authorized user can see this object."""
34+ return True
35+
36+
37 class AdminByAdminsTeam(AuthorizationBase):
38 permission = 'launchpad.Admin'
39 usedfor = Interface
40@@ -813,24 +828,9 @@
41 user.in_admin)
42
43
44-class ViewProductSeries(AuthorizationBase):
45+class ViewProductSeries(AnonymousAuthorization):
46
47 usedfor = IProductSeries
48- permision = 'launchpad.View'
49-
50- def checkUnauthenticated(self):
51- """See `IAuthorization.checkUnauthenticated`.
52-
53- :return: True or False.
54- """
55- return True
56-
57- def checkAuthenticated(self, user):
58- """See `IAuthorization.checkAuthenticated`.
59-
60- :return: True or False.
61- """
62- return True
63
64
65 class EditProductSeries(EditByOwnersOrAdmins):
66@@ -978,6 +978,16 @@
67 self, bugattachment.bug)
68
69
70+class ViewBugSubscription(AnonymousAuthorization):
71+
72+ usedfor = IBugSubscription
73+
74+
75+class ViewBugMessage(AnonymousAuthorization):
76+
77+ usedfor = IMessage
78+
79+
80 class ViewAnnouncement(AuthorizationBase):
81 permission = 'launchpad.View'
82 usedfor = IAnnouncement
83@@ -1281,7 +1291,16 @@
84 self, user)
85
86
87-class AdminTranslationImportQueueEntry(AuthorizationBase):
88+<<<<<<< TREE
89+class AdminTranslationImportQueueEntry(AuthorizationBase):
90+=======
91+class ViewProductRelease(AnonymousAuthorization):
92+
93+ usedfor = IProductRelease
94+
95+
96+class AdminTranslationImportQueueEntry(AuthorizationBase):
97+>>>>>>> MERGE-SOURCE
98 permission = 'launchpad.Admin'
99 usedfor = ITranslationImportQueueEntry
100
101
102=== modified file 'lib/lp/bugs/browser/configure.zcml'
103=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
104--- lib/lp/bugs/interfaces/bugtask.py 2010-02-06 12:23:17 +0000
105+++ lib/lp/bugs/interfaces/bugtask.py 2010-02-08 14:16:33 +0000
106@@ -474,11 +474,9 @@
107 description=_("The age of this task, expressed as the "
108 "length of time between the creation date "
109 "and now."))
110- task_age = exported(
111- Int(title=_("Age of the bug task"),
112+ task_age = Int(title=_("Age of the bug task"),
113 description=_("The age of this task in seconds, a delta between "
114- "now and the date the bug task was created."),
115- readonly=True))
116+ "now and the date the bug task was created."))
117 owner = exported(
118 Reference(title=_("The owner"), schema=IPerson, readonly=True))
119 target = exported(Reference(
120
121=== modified file 'lib/lp/bugs/stories/webservice/xx-bug.txt'
122--- lib/lp/bugs/stories/webservice/xx-bug.txt 2010-02-03 18:57:40 +0000
123+++ lib/lp/bugs/stories/webservice/xx-bug.txt 2010-02-08 14:16:33 +0000
124@@ -255,6 +255,12 @@
125 >>> webservice.get("/messages").status
126 404
127
128+Bug messages can be accessed anonymously.
129+
130+ >>> messages = anon_webservice.get("/bugs/5/messages").jsonBody()['entries']
131+ >>> print messages[0]['self_link']
132+ http://.../firefox/+bug/5/comments/0
133+
134 We can add a new message to a bug by calling the newMessage method.
135
136 >>> print webservice.named_post(
137@@ -332,15 +338,8 @@
138 self_link: u'http://api.../debian/+source/mozilla-firefox/+bug/1'
139 status: u'Confirmed'
140 target_link: u'http://api.../debian/+source/mozilla-firefox'
141- task_age: ...
142 title: u'Bug #1 in mozilla-firefox (Debian): "Firefox does not support SVG"'
143
144-Because task age is a dynamic calculation between now and the date_created
145-we will just verify that an integer is returned.
146-
147- >>> print type(bug_one_bugtasks[0]['task_age'])
148- <type 'int'>
149-
150 The collection of bug tasks is not exposed as a resource:
151
152 >>> webservice.get("/bug_tasks").status
153@@ -869,6 +868,12 @@
154 self_link: u'http://.../bugs/1/+subscription/stevea'
155 subscribed_by_link: u'http://.../~janitor'
156
157+Subscriptions can also be accessed anonymously.
158+
159+ >>> subscriptions = anon_webservice.get(bug_one_subscriptions_url).jsonBody()
160+ >>> print subscriptions['entries'][0]['self_link']
161+ http://.../bugs/1/+subscription/stevea
162+
163 We can also create new subscriptions.
164
165 >>> new_subscription = webservice.named_post(
166
167=== modified file 'lib/lp/code/model/branchmergeproposaljob.py'
168=== modified file 'lib/lp/registry/browser/configure.zcml'
169--- lib/lp/registry/browser/configure.zcml 2010-01-30 18:15:36 +0000
170+++ lib/lp/registry/browser/configure.zcml 2010-02-08 14:16:33 +0000
171@@ -1606,7 +1606,7 @@
172 template="../templates/productseries-ubuntupkg.pt"
173 for="lp.registry.interfaces.productseries.IProductSeries"
174 class="lp.registry.browser.productseries.ProductSeriesUbuntuPackagingView"
175- permission="launchpad.View"/>
176+ permission="launchpad.AnyPerson"/>
177 <browser:pages
178 for="lp.registry.interfaces.productseries.IProductSeries"
179 class="lp.registry.browser.productseries.ProductSeriesView"
180
181=== modified file 'lib/lp/registry/browser/productseries.py'
182--- lib/lp/registry/browser/productseries.py 2010-01-22 16:44:39 +0000
183+++ lib/lp/registry/browser/productseries.py 2010-02-08 14:16:33 +0000
184@@ -205,7 +205,7 @@
185 summary = "Register a new Bazaar branch for this series' project"
186 return Link('+addbranch', text, summary, icon='add')
187
188- @enabled_with_permission('launchpad.View')
189+ @enabled_with_permission('launchpad.AnyPerson')
190 def ubuntupkg(self):
191 """Return a link to link this series to an ubuntu sourcepackage."""
192 text = 'Link to Ubuntu package'
193
194=== modified file 'lib/lp/registry/stories/webservice/xx-project-registry.txt'
195--- lib/lp/registry/stories/webservice/xx-project-registry.txt 2010-01-27 19:12:10 +0000
196+++ lib/lp/registry/stories/webservice/xx-project-registry.txt 2010-02-08 14:16:33 +0000
197@@ -282,6 +282,13 @@
198 >>> print series_1_0['self_link']
199 http://.../firefox/1.0
200
201+Series can also be accessed anonymously.
202+
203+ >>> response = anon_webservice.get(firefox['series_collection_link'])
204+ >>> series = response.jsonBody()
205+ >>> print series['total_size']
206+ 2
207+
208 A list of releases can be accessed through the releases_collection_link.
209
210 >>> response = webservice.get(firefox['releases_collection_link'])
211@@ -302,6 +309,13 @@
212 >>> print release_0_9_1['self_link']
213 http://.../firefox/trunk/0.9.1
214
215+Releases can also be accessed anonymously.
216+
217+ >>> response = anon_webservice.get(firefox['releases_collection_link'])
218+ >>> releases = response.jsonBody()
219+ >>> print releases['total_size']
220+ 4
221+
222 The development focus series can be accessed through the
223 development_focus_link.
224
225
226=== modified file 'lib/lp/services/job/runner.py'
227=== modified file 'lib/lp/testing/factory.py'
228--- lib/lp/testing/factory.py 2010-02-06 12:23:17 +0000
229+++ lib/lp/testing/factory.py 2010-02-08 14:16:33 +0000
230@@ -36,8 +36,22 @@
231 from canonical.autodecorate import AutoDecorate
232 from canonical.config import config
233 from canonical.database.constants import UTC_NOW
234+<<<<<<< TREE
235+=======
236+from lp.code.interfaces.sourcepackagerecipe import ISourcePackageRecipeSource
237+from lp.code.interfaces.sourcepackagerecipebuild import (
238+ ISourcePackageRecipeBuildSource,
239+ )
240+from lp.codehosting.codeimport.worker import CodeImportSourceDetails
241+>>>>>>> MERGE-SOURCE
242 from canonical.database.sqlbase import flush_database_updates
243+<<<<<<< TREE
244
245+=======
246+from lp.soyuz.adapters.packagelocation import PackageLocation
247+from lp.soyuz.interfaces.publishing import PackagePublishingStatus
248+from lp.soyuz.interfaces.section import ISectionSet
249+>>>>>>> MERGE-SOURCE
250 from canonical.launchpad.database.account import Account
251 from canonical.launchpad.database.emailaddress import EmailAddress
252 from canonical.launchpad.database.message import Message, MessageChunk
253@@ -83,6 +97,7 @@
254 from lp.code.interfaces.codeimportmachine import ICodeImportMachineSet
255 from lp.code.interfaces.codeimportresult import ICodeImportResultSet
256 from lp.code.interfaces.revision import IRevisionSet
257+<<<<<<< TREE
258 from lp.code.interfaces.sourcepackagerecipe import ISourcePackageRecipeSource
259 from lp.code.interfaces.sourcepackagerecipebuild import (
260 ISourcePackageRecipeBuildSource,
261@@ -91,6 +106,9 @@
262 from lp.codehosting.codeimport.worker import CodeImportSourceDetails
263
264 from lp.registry.interfaces.distribution import IDistributionSet
265+=======
266+from lp.code.model.diff import Diff, PreviewDiff
267+>>>>>>> MERGE-SOURCE
268 from lp.registry.model.distributionsourcepackage import (
269 DistributionSourcePackage)
270 from lp.registry.interfaces.distroseries import IDistroSeries

Subscribers

People subscribed via source and target branches

to status/vote changes: