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
=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py 2010-02-04 00:01:23 +0000
+++ lib/canonical/launchpad/security.py 2010-02-08 14:16:33 +0000
@@ -29,6 +29,7 @@
29from lp.bugs.interfaces.bugattachment import IBugAttachment29from lp.bugs.interfaces.bugattachment import IBugAttachment
30from lp.bugs.interfaces.bugbranch import IBugBranch30from lp.bugs.interfaces.bugbranch import IBugBranch
31from lp.bugs.interfaces.bugnomination import IBugNomination31from lp.bugs.interfaces.bugnomination import IBugNomination
32from lp.bugs.interfaces.bugsubscription import IBugSubscription
32from lp.bugs.interfaces.bugtracker import IBugTracker33from lp.bugs.interfaces.bugtracker import IBugTracker
33from lp.soyuz.interfaces.build import IBuild34from lp.soyuz.interfaces.build import IBuild
34from lp.buildmaster.interfaces.builder import IBuilder, IBuilderSet35from lp.buildmaster.interfaces.builder import IBuilder, IBuilderSet
@@ -63,6 +64,7 @@
63from lp.registry.interfaces.mailinglist import IMailingListSet64from lp.registry.interfaces.mailinglist import IMailingListSet
64from lp.registry.interfaces.milestone import (65from lp.registry.interfaces.milestone import (
65 IMilestone, IProjectMilestone)66 IMilestone, IProjectMilestone)
67from canonical.launchpad.interfaces.message import IMessage
66from canonical.launchpad.interfaces.oauth import (68from canonical.launchpad.interfaces.oauth import (
67 IOAuthAccessToken, IOAuthRequestToken)69 IOAuthAccessToken, IOAuthRequestToken)
68from lp.soyuz.interfaces.packageset import IPackageset, IPackagesetSet70from lp.soyuz.interfaces.packageset import IPackageset, IPackagesetSet
@@ -170,6 +172,19 @@
170 return True172 return True
171173
172174
175class AnonymousAuthorization(AuthorizationBase):
176 """Allow any authenticated and unauthenticated user access."""
177 permission = 'launchpad.View'
178
179 def checkUnauthenticated(self):
180 """Any unauthorized user can see this object."""
181 return True
182
183 def checkAuthenticated(self, user):
184 """Any authorized user can see this object."""
185 return True
186
187
173class AdminByAdminsTeam(AuthorizationBase):188class AdminByAdminsTeam(AuthorizationBase):
174 permission = 'launchpad.Admin'189 permission = 'launchpad.Admin'
175 usedfor = Interface190 usedfor = Interface
@@ -813,24 +828,9 @@
813 user.in_admin)828 user.in_admin)
814829
815830
816class ViewProductSeries(AuthorizationBase):831class ViewProductSeries(AnonymousAuthorization):
817832
818 usedfor = IProductSeries833 usedfor = IProductSeries
819 permision = 'launchpad.View'
820
821 def checkUnauthenticated(self):
822 """See `IAuthorization.checkUnauthenticated`.
823
824 :return: True or False.
825 """
826 return True
827
828 def checkAuthenticated(self, user):
829 """See `IAuthorization.checkAuthenticated`.
830
831 :return: True or False.
832 """
833 return True
834834
835835
836class EditProductSeries(EditByOwnersOrAdmins):836class EditProductSeries(EditByOwnersOrAdmins):
@@ -978,6 +978,16 @@
978 self, bugattachment.bug)978 self, bugattachment.bug)
979979
980980
981class ViewBugSubscription(AnonymousAuthorization):
982
983 usedfor = IBugSubscription
984
985
986class ViewBugMessage(AnonymousAuthorization):
987
988 usedfor = IMessage
989
990
981class ViewAnnouncement(AuthorizationBase):991class ViewAnnouncement(AuthorizationBase):
982 permission = 'launchpad.View'992 permission = 'launchpad.View'
983 usedfor = IAnnouncement993 usedfor = IAnnouncement
@@ -1281,7 +1291,16 @@
1281 self, user)1291 self, user)
12821292
12831293
1284class AdminTranslationImportQueueEntry(AuthorizationBase):1294<<<<<<< TREE
1295class AdminTranslationImportQueueEntry(AuthorizationBase):
1296=======
1297class ViewProductRelease(AnonymousAuthorization):
1298
1299 usedfor = IProductRelease
1300
1301
1302class AdminTranslationImportQueueEntry(AuthorizationBase):
1303>>>>>>> MERGE-SOURCE
1285 permission = 'launchpad.Admin'1304 permission = 'launchpad.Admin'
1286 usedfor = ITranslationImportQueueEntry1305 usedfor = ITranslationImportQueueEntry
12871306
12881307
=== modified file 'lib/lp/bugs/browser/configure.zcml'
=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
--- lib/lp/bugs/interfaces/bugtask.py 2010-02-06 12:23:17 +0000
+++ lib/lp/bugs/interfaces/bugtask.py 2010-02-08 14:16:33 +0000
@@ -474,11 +474,9 @@
474 description=_("The age of this task, expressed as the "474 description=_("The age of this task, expressed as the "
475 "length of time between the creation date "475 "length of time between the creation date "
476 "and now."))476 "and now."))
477 task_age = exported(477 task_age = Int(title=_("Age of the bug task"),
478 Int(title=_("Age of the bug task"),
479 description=_("The age of this task in seconds, a delta between "478 description=_("The age of this task in seconds, a delta between "
480 "now and the date the bug task was created."),479 "now and the date the bug task was created."))
481 readonly=True))
482 owner = exported(480 owner = exported(
483 Reference(title=_("The owner"), schema=IPerson, readonly=True))481 Reference(title=_("The owner"), schema=IPerson, readonly=True))
484 target = exported(Reference(482 target = exported(Reference(
485483
=== modified file 'lib/lp/bugs/stories/webservice/xx-bug.txt'
--- lib/lp/bugs/stories/webservice/xx-bug.txt 2010-02-03 18:57:40 +0000
+++ lib/lp/bugs/stories/webservice/xx-bug.txt 2010-02-08 14:16:33 +0000
@@ -255,6 +255,12 @@
255 >>> webservice.get("/messages").status255 >>> webservice.get("/messages").status
256 404256 404
257257
258Bug messages can be accessed anonymously.
259
260 >>> messages = anon_webservice.get("/bugs/5/messages").jsonBody()['entries']
261 >>> print messages[0]['self_link']
262 http://.../firefox/+bug/5/comments/0
263
258We can add a new message to a bug by calling the newMessage method.264We can add a new message to a bug by calling the newMessage method.
259265
260 >>> print webservice.named_post(266 >>> print webservice.named_post(
@@ -332,15 +338,8 @@
332 self_link: u'http://api.../debian/+source/mozilla-firefox/+bug/1'338 self_link: u'http://api.../debian/+source/mozilla-firefox/+bug/1'
333 status: u'Confirmed'339 status: u'Confirmed'
334 target_link: u'http://api.../debian/+source/mozilla-firefox'340 target_link: u'http://api.../debian/+source/mozilla-firefox'
335 task_age: ...
336 title: u'Bug #1 in mozilla-firefox (Debian): "Firefox does not support SVG"'341 title: u'Bug #1 in mozilla-firefox (Debian): "Firefox does not support SVG"'
337342
338Because task age is a dynamic calculation between now and the date_created
339we will just verify that an integer is returned.
340
341 >>> print type(bug_one_bugtasks[0]['task_age'])
342 <type 'int'>
343
344The collection of bug tasks is not exposed as a resource:343The collection of bug tasks is not exposed as a resource:
345344
346 >>> webservice.get("/bug_tasks").status345 >>> webservice.get("/bug_tasks").status
@@ -869,6 +868,12 @@
869 self_link: u'http://.../bugs/1/+subscription/stevea'868 self_link: u'http://.../bugs/1/+subscription/stevea'
870 subscribed_by_link: u'http://.../~janitor'869 subscribed_by_link: u'http://.../~janitor'
871870
871Subscriptions can also be accessed anonymously.
872
873 >>> subscriptions = anon_webservice.get(bug_one_subscriptions_url).jsonBody()
874 >>> print subscriptions['entries'][0]['self_link']
875 http://.../bugs/1/+subscription/stevea
876
872We can also create new subscriptions.877We can also create new subscriptions.
873878
874 >>> new_subscription = webservice.named_post(879 >>> new_subscription = webservice.named_post(
875880
=== modified file 'lib/lp/code/model/branchmergeproposaljob.py'
=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml 2010-01-30 18:15:36 +0000
+++ lib/lp/registry/browser/configure.zcml 2010-02-08 14:16:33 +0000
@@ -1606,7 +1606,7 @@
1606 template="../templates/productseries-ubuntupkg.pt"1606 template="../templates/productseries-ubuntupkg.pt"
1607 for="lp.registry.interfaces.productseries.IProductSeries"1607 for="lp.registry.interfaces.productseries.IProductSeries"
1608 class="lp.registry.browser.productseries.ProductSeriesUbuntuPackagingView"1608 class="lp.registry.browser.productseries.ProductSeriesUbuntuPackagingView"
1609 permission="launchpad.View"/>1609 permission="launchpad.AnyPerson"/>
1610 <browser:pages1610 <browser:pages
1611 for="lp.registry.interfaces.productseries.IProductSeries"1611 for="lp.registry.interfaces.productseries.IProductSeries"
1612 class="lp.registry.browser.productseries.ProductSeriesView"1612 class="lp.registry.browser.productseries.ProductSeriesView"
16131613
=== modified file 'lib/lp/registry/browser/productseries.py'
--- lib/lp/registry/browser/productseries.py 2010-01-22 16:44:39 +0000
+++ lib/lp/registry/browser/productseries.py 2010-02-08 14:16:33 +0000
@@ -205,7 +205,7 @@
205 summary = "Register a new Bazaar branch for this series' project"205 summary = "Register a new Bazaar branch for this series' project"
206 return Link('+addbranch', text, summary, icon='add')206 return Link('+addbranch', text, summary, icon='add')
207207
208 @enabled_with_permission('launchpad.View')208 @enabled_with_permission('launchpad.AnyPerson')
209 def ubuntupkg(self):209 def ubuntupkg(self):
210 """Return a link to link this series to an ubuntu sourcepackage."""210 """Return a link to link this series to an ubuntu sourcepackage."""
211 text = 'Link to Ubuntu package'211 text = 'Link to Ubuntu package'
212212
=== modified file 'lib/lp/registry/stories/webservice/xx-project-registry.txt'
--- lib/lp/registry/stories/webservice/xx-project-registry.txt 2010-01-27 19:12:10 +0000
+++ lib/lp/registry/stories/webservice/xx-project-registry.txt 2010-02-08 14:16:33 +0000
@@ -282,6 +282,13 @@
282 >>> print series_1_0['self_link']282 >>> print series_1_0['self_link']
283 http://.../firefox/1.0283 http://.../firefox/1.0
284284
285Series can also be accessed anonymously.
286
287 >>> response = anon_webservice.get(firefox['series_collection_link'])
288 >>> series = response.jsonBody()
289 >>> print series['total_size']
290 2
291
285A list of releases can be accessed through the releases_collection_link.292A list of releases can be accessed through the releases_collection_link.
286293
287 >>> response = webservice.get(firefox['releases_collection_link'])294 >>> response = webservice.get(firefox['releases_collection_link'])
@@ -302,6 +309,13 @@
302 >>> print release_0_9_1['self_link']309 >>> print release_0_9_1['self_link']
303 http://.../firefox/trunk/0.9.1310 http://.../firefox/trunk/0.9.1
304311
312Releases can also be accessed anonymously.
313
314 >>> response = anon_webservice.get(firefox['releases_collection_link'])
315 >>> releases = response.jsonBody()
316 >>> print releases['total_size']
317 4
318
305The development focus series can be accessed through the319The development focus series can be accessed through the
306development_focus_link.320development_focus_link.
307321
308322
=== modified file 'lib/lp/services/job/runner.py'
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-02-06 12:23:17 +0000
+++ lib/lp/testing/factory.py 2010-02-08 14:16:33 +0000
@@ -36,8 +36,22 @@
36from canonical.autodecorate import AutoDecorate36from canonical.autodecorate import AutoDecorate
37from canonical.config import config37from canonical.config import config
38from canonical.database.constants import UTC_NOW38from canonical.database.constants import UTC_NOW
39<<<<<<< TREE
40=======
41from lp.code.interfaces.sourcepackagerecipe import ISourcePackageRecipeSource
42from lp.code.interfaces.sourcepackagerecipebuild import (
43 ISourcePackageRecipeBuildSource,
44 )
45from lp.codehosting.codeimport.worker import CodeImportSourceDetails
46>>>>>>> MERGE-SOURCE
39from canonical.database.sqlbase import flush_database_updates47from canonical.database.sqlbase import flush_database_updates
48<<<<<<< TREE
4049
50=======
51from lp.soyuz.adapters.packagelocation import PackageLocation
52from lp.soyuz.interfaces.publishing import PackagePublishingStatus
53from lp.soyuz.interfaces.section import ISectionSet
54>>>>>>> MERGE-SOURCE
41from canonical.launchpad.database.account import Account55from canonical.launchpad.database.account import Account
42from canonical.launchpad.database.emailaddress import EmailAddress56from canonical.launchpad.database.emailaddress import EmailAddress
43from canonical.launchpad.database.message import Message, MessageChunk57from canonical.launchpad.database.message import Message, MessageChunk
@@ -83,6 +97,7 @@
83from lp.code.interfaces.codeimportmachine import ICodeImportMachineSet97from lp.code.interfaces.codeimportmachine import ICodeImportMachineSet
84from lp.code.interfaces.codeimportresult import ICodeImportResultSet98from lp.code.interfaces.codeimportresult import ICodeImportResultSet
85from lp.code.interfaces.revision import IRevisionSet99from lp.code.interfaces.revision import IRevisionSet
100<<<<<<< TREE
86from lp.code.interfaces.sourcepackagerecipe import ISourcePackageRecipeSource101from lp.code.interfaces.sourcepackagerecipe import ISourcePackageRecipeSource
87from lp.code.interfaces.sourcepackagerecipebuild import (102from lp.code.interfaces.sourcepackagerecipebuild import (
88 ISourcePackageRecipeBuildSource,103 ISourcePackageRecipeBuildSource,
@@ -91,6 +106,9 @@
91from lp.codehosting.codeimport.worker import CodeImportSourceDetails106from lp.codehosting.codeimport.worker import CodeImportSourceDetails
92107
93from lp.registry.interfaces.distribution import IDistributionSet108from lp.registry.interfaces.distribution import IDistributionSet
109=======
110from lp.code.model.diff import Diff, PreviewDiff
111>>>>>>> MERGE-SOURCE
94from lp.registry.model.distributionsourcepackage import (112from lp.registry.model.distributionsourcepackage import (
95 DistributionSourcePackage)113 DistributionSourcePackage)
96from lp.registry.interfaces.distroseries import IDistroSeries114from lp.registry.interfaces.distroseries import IDistroSeries

Subscribers

People subscribed via source and target branches

to status/vote changes: