launchpad-reviewers team mailing list archive

[Paul Hummer <paul@xxxxxxxxxxxxxxxxxxxx>]

Paul Hummer has proposed merging lp:~rockstar/launchpad/fix-spr-builds-perms into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #615513 Recipe operations that are admin only should be launchpad.Admin
  https://bugs.launchpad.net/bugs/615513


This branch fixes the issue Aaron brought up in my last review, and fixes bug #615513.  It just swaps launchpad.Edit permissions for launchpad.Admin permissions.
-- 
https://code.launchpad.net/~rockstar/launchpad/fix-spr-builds-perms/+merge/32138
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rockstar/launchpad/fix-spr-builds-perms into lp:launchpad.

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2010-07-28 13:26:55 +0000
+++ lib/canonical/launchpad/security.py	2010-08-09 19:56:06 +0000
@@ -1171,14 +1171,17 @@
     usedfor = ICodeImportMachine
 
 
-class DeleteSourcePackageRecipeBuilds(OnlyBazaarExpertsAndAdmins):
-    """Control who can delete SourcePackageRecipeBuilds.
+class AdminSourcePackageRecipeBuilds(AuthorizationBase):
+    """Control who can edit SourcePackageRecipeBuilds.
 
-    Access is restricted to members of ~bazaar-experts and Launchpad admins.
+    Access is restricted to members of ~bazaar-experts and Buildd Admins.
     """
-    permission = 'launchpad.Edit'
+    permission = 'launchpad.Admin'
     usedfor = ISourcePackageRecipeBuild
 
+    def checkAuthenticated(self, user):
+        return user.in_bazaar_experts or user.in_buildd_admin
+
 
 class AdminDistributionTranslations(AuthorizationBase):
     """Class for deciding who can administer distribution translations.

=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml	2010-07-22 02:41:43 +0000
+++ lib/lp/code/browser/configure.zcml	2010-08-09 19:56:06 +0000
@@ -1129,13 +1129,13 @@
             class="lp.code.browser.sourcepackagerecipebuild.SourcePackageRecipeBuildCancelView"
             name="+cancel"
             template="../../app/templates/generic-edit.pt"
-            permission="launchpad.View"/>
+            permission="launchpad.Admin"/>
         <browser:page
             for="lp.code.interfaces.sourcepackagerecipebuild.ISourcePackageRecipeBuild"
             class="lp.code.browser.sourcepackagerecipebuild.SourcePackageRecipeBuildRescoreView"
             name="+rescore"
             template="../../app/templates/generic-edit.pt"
-            permission="launchpad.View"/>
+            permission="launchpad.Admin"/>
         <browser:menus
             classes="
                 SourcePackageRecipeNavigationMenu
@@ -1159,7 +1159,7 @@
         <browser:page
             for="lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipe"
             class="lp.code.browser.sourcepackagerecipe.SourcePackageRecipeDeleteView"
-            permission="zope.Public"
+            permission="launchpad.Edit"
             facet="branches"
             name="+delete"
             template="../../app/templates/generic-edit.pt"/>

=== modified file 'lib/lp/code/browser/sourcepackagerecipebuild.py'
--- lib/lp/code/browser/sourcepackagerecipebuild.py	2010-07-28 14:36:32 +0000
+++ lib/lp/code/browser/sourcepackagerecipebuild.py	2010-08-09 19:56:06 +0000
@@ -47,7 +47,7 @@
 
     links = ('cancel', 'rescore')
 
-    @enabled_with_permission('launchpad.Edit')
+    @enabled_with_permission('launchpad.Admin')
     def cancel(self):
         if self.context.status in UNEDITABLE_BUILD_STATES:
             enabled = False
@@ -55,7 +55,7 @@
             enabled = True
         return Link('+cancel', 'Cancel build', icon='remove', enabled=enabled)
 
-    @enabled_with_permission('launchpad.Edit')
+    @enabled_with_permission('launchpad.Admin')
     def rescore(self):
         if self.context.status in UNEDITABLE_BUILD_STATES:
             enabled = False

=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2010-08-07 14:54:40 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2010-08-09 19:56:06 +0000
@@ -13,6 +13,7 @@
 from textwrap import dedent
 
 import transaction
+from mechanize import LinkNotFoundError
 from pytz import utc
 from zope.security.interfaces import Unauthorized
 from zope.security.proxy import removeSecurityProxy
@@ -922,3 +923,19 @@
         self.assertEqual(
             'http://code.launchpad.dev/~chef',
             browser.url)
+
+    def test_delete_recipe_no_permissions(self):
+        recipe = self.factory.makeSourcePackageRecipe(owner=self.chef)
+        nopriv_person = self.factory.makePerson()
+        recipe_url = canonical_url(recipe)
+
+        browser = self.getUserBrowser(
+            recipe_url, user=nopriv_person)
+
+        self.assertRaises(
+            LinkNotFoundError,
+            browser.getLink, 'Delete recipe')
+
+        self.assertRaises(
+            Unauthorized,
+            self.getUserBrowser, recipe_url + '/+delete', user=nopriv_person)

=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipebuild.py	2010-08-01 22:31:42 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipebuild.py	2010-08-09 19:56:06 +0000
@@ -9,6 +9,7 @@
 from mechanize import LinkNotFoundError
 import transaction
 from zope.component import getUtility
+from zope.security.interfaces import Unauthorized
 from zope.security.proxy import removeSecurityProxy
 
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
@@ -96,6 +97,10 @@
             LinkNotFoundError,
             browser.getLink, 'Cancel build')
 
+        self.assertRaises(
+            Unauthorized,
+            self.getUserBrowser, build_url + '/+cancel', user=self.chef)
+
     def test_cancel_build_wrong_state(self):
         """If the build isn't queued, you can't cancel it."""
         experts = getUtility(ILaunchpadCelebrities).bazaar_experts.teamowner
@@ -176,6 +181,10 @@
             LinkNotFoundError,
             browser.getLink, 'Rescore build')
 
+        self.assertRaises(
+            Unauthorized,
+            self.getUserBrowser, build_url + '/+rescore', user=self.chef)
+
     def test_rescore_build_wrong_state(self):
         """If the build isn't queued, you can't rescore it."""
         experts = getUtility(ILaunchpadCelebrities).bazaar_experts.teamowner