launchpad-reviewers team mailing list archive

[Paul Hummer <paul@xxxxxxxxxxxxxxxxxxxx>]

Paul Hummer has proposed merging lp:~rockstar/launchpad/fix-cancel-rescore-permissions into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


This branch fixes bug #615144 - It changes the access permissions on some views to make sure they can't be accessed except by people that could see the links to those pages in the first place.  They also put some tests in place to make sure that it is the way it's described.  I'm not sure we really have much (but some) precedent for testing that you DON'T have access, but the tests are there now anyway.  I also swapped permissions from requiring admin or bazaar experts to cancel the build to buildd-admin and bazaar experts to cancel the build, per bigjools' comment #6 in the bug.
-- 
https://code.launchpad.net/~rockstar/launchpad/fix-cancel-rescore-permissions/+merge/32126
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rockstar/launchpad/fix-cancel-rescore-permissions 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 16:59:19 +0000
@@ -1171,14 +1171,17 @@
     usedfor = ICodeImportMachine
 
 
-class DeleteSourcePackageRecipeBuilds(OnlyBazaarExpertsAndAdmins):
-    """Control who can delete SourcePackageRecipeBuilds.
+class EditSourcePackageRecipeBuilds(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'
     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 16:59:19 +0000
@@ -1129,13 +1129,13 @@
             class="lp.code.browser.sourcepackagerecipebuild.SourcePackageRecipeBuildCancelView"
             name="+cancel"
             template="../../app/templates/generic-edit.pt"
-            permission="launchpad.View"/>
+            permission="launchpad.Edit"/>
         <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.Edit"/>
         <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/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 16:59:19 +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 16:59:19 +0000
@@ -9,7 +9,11 @@
 from mechanize import LinkNotFoundError
 import transaction
 from zope.component import getUtility
+<<<<<<< TREE
 from zope.security.proxy import removeSecurityProxy
+=======
+from zope.security.interfaces import Unauthorized
+>>>>>>> MERGE-SOURCE
 
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.testing.pages import (
@@ -96,6 +100,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 +184,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