Merge lp:~rockstar/launchpad/fix-cancel-rescore-permissions into lp:launchpad/db-devel

Proposed by Paul Hummer
Status: Merged
Approved by: Aaron Bentley
Approved revision: no longer in the source branch.
Merged at revision: 9642
Proposed branch: lp:~rockstar/launchpad/fix-cancel-rescore-permissions
Merge into: lp:launchpad/db-devel
Diff against target: 119 lines (+35/-6)
4 files modified
lib/canonical/launchpad/security.py (+6/-3)
lib/lp/code/browser/configure.zcml (+3/-3)
lib/lp/code/browser/tests/test_sourcepackagerecipe.py (+17/-0)
lib/lp/code/browser/tests/test_sourcepackagerecipebuild.py (+9/-0)
To merge this branch: bzr merge lp:~rockstar/launchpad/fix-cancel-rescore-permissions
Reviewer Review Type Date Requested Status
Julian Edwards (community) rc Approve
Aaron Bentley (community) Approve
Review via email: mp+32126@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

We should change the name of the launchpad.Edit permission on SourcePackageRecipeBuild to launchpad.Admin in the future.

review: Approve
Revision history for this message
Julian Edwards (julian-edwards) :
review: Approve (rc)
Revision history for this message
Julian Edwards (julian-edwards) wrote :

This should have been a private MP!

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-07-28 13:26:55 +0000
+++ lib/canonical/launchpad/security.py 2010-08-09 17:17:44 +0000
@@ -1171,14 +1171,17 @@
1171 usedfor = ICodeImportMachine1171 usedfor = ICodeImportMachine
11721172
11731173
1174class DeleteSourcePackageRecipeBuilds(OnlyBazaarExpertsAndAdmins):1174class EditSourcePackageRecipeBuilds(AuthorizationBase):
1175 """Control who can delete SourcePackageRecipeBuilds.1175 """Control who can edit SourcePackageRecipeBuilds.
11761176
1177 Access is restricted to members of ~bazaar-experts and Launchpad admins.1177 Access is restricted to members of ~bazaar-experts and Buildd Admins.
1178 """1178 """
1179 permission = 'launchpad.Edit'1179 permission = 'launchpad.Edit'
1180 usedfor = ISourcePackageRecipeBuild1180 usedfor = ISourcePackageRecipeBuild
11811181
1182 def checkAuthenticated(self, user):
1183 return user.in_bazaar_experts or user.in_buildd_admin
1184
11821185
1183class AdminDistributionTranslations(AuthorizationBase):1186class AdminDistributionTranslations(AuthorizationBase):
1184 """Class for deciding who can administer distribution translations.1187 """Class for deciding who can administer distribution translations.
11851188
=== 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 17:17:44 +0000
@@ -1129,13 +1129,13 @@
1129 class="lp.code.browser.sourcepackagerecipebuild.SourcePackageRecipeBuildCancelView"1129 class="lp.code.browser.sourcepackagerecipebuild.SourcePackageRecipeBuildCancelView"
1130 name="+cancel"1130 name="+cancel"
1131 template="../../app/templates/generic-edit.pt"1131 template="../../app/templates/generic-edit.pt"
1132 permission="launchpad.View"/>1132 permission="launchpad.Edit"/>
1133 <browser:page1133 <browser:page
1134 for="lp.code.interfaces.sourcepackagerecipebuild.ISourcePackageRecipeBuild"1134 for="lp.code.interfaces.sourcepackagerecipebuild.ISourcePackageRecipeBuild"
1135 class="lp.code.browser.sourcepackagerecipebuild.SourcePackageRecipeBuildRescoreView"1135 class="lp.code.browser.sourcepackagerecipebuild.SourcePackageRecipeBuildRescoreView"
1136 name="+rescore"1136 name="+rescore"
1137 template="../../app/templates/generic-edit.pt"1137 template="../../app/templates/generic-edit.pt"
1138 permission="launchpad.View"/>1138 permission="launchpad.Edit"/>
1139 <browser:menus1139 <browser:menus
1140 classes="1140 classes="
1141 SourcePackageRecipeNavigationMenu1141 SourcePackageRecipeNavigationMenu
@@ -1159,7 +1159,7 @@
1159 <browser:page1159 <browser:page
1160 for="lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipe"1160 for="lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipe"
1161 class="lp.code.browser.sourcepackagerecipe.SourcePackageRecipeDeleteView"1161 class="lp.code.browser.sourcepackagerecipe.SourcePackageRecipeDeleteView"
1162 permission="zope.Public"1162 permission="launchpad.Edit"
1163 facet="branches"1163 facet="branches"
1164 name="+delete"1164 name="+delete"
1165 template="../../app/templates/generic-edit.pt"/>1165 template="../../app/templates/generic-edit.pt"/>
11661166
=== 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 17:17:44 +0000
@@ -13,6 +13,7 @@
13from textwrap import dedent13from textwrap import dedent
1414
15import transaction15import transaction
16from mechanize import LinkNotFoundError
16from pytz import utc17from pytz import utc
17from zope.security.interfaces import Unauthorized18from zope.security.interfaces import Unauthorized
18from zope.security.proxy import removeSecurityProxy19from zope.security.proxy import removeSecurityProxy
@@ -922,3 +923,19 @@
922 self.assertEqual(923 self.assertEqual(
923 'http://code.launchpad.dev/~chef',924 'http://code.launchpad.dev/~chef',
924 browser.url)925 browser.url)
926
927 def test_delete_recipe_no_permissions(self):
928 recipe = self.factory.makeSourcePackageRecipe(owner=self.chef)
929 nopriv_person = self.factory.makePerson()
930 recipe_url = canonical_url(recipe)
931
932 browser = self.getUserBrowser(
933 recipe_url, user=nopriv_person)
934
935 self.assertRaises(
936 LinkNotFoundError,
937 browser.getLink, 'Delete recipe')
938
939 self.assertRaises(
940 Unauthorized,
941 self.getUserBrowser, recipe_url + '/+delete', user=nopriv_person)
925942
=== 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 17:17:44 +0000
@@ -9,6 +9,7 @@
9from mechanize import LinkNotFoundError9from mechanize import LinkNotFoundError
10import transaction10import transaction
11from zope.component import getUtility11from zope.component import getUtility
12from zope.security.interfaces import Unauthorized
12from zope.security.proxy import removeSecurityProxy13from zope.security.proxy import removeSecurityProxy
1314
14from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities15from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
@@ -96,6 +97,10 @@
96 LinkNotFoundError,97 LinkNotFoundError,
97 browser.getLink, 'Cancel build')98 browser.getLink, 'Cancel build')
9899
100 self.assertRaises(
101 Unauthorized,
102 self.getUserBrowser, build_url + '/+cancel', user=self.chef)
103
99 def test_cancel_build_wrong_state(self):104 def test_cancel_build_wrong_state(self):
100 """If the build isn't queued, you can't cancel it."""105 """If the build isn't queued, you can't cancel it."""
101 experts = getUtility(ILaunchpadCelebrities).bazaar_experts.teamowner106 experts = getUtility(ILaunchpadCelebrities).bazaar_experts.teamowner
@@ -176,6 +181,10 @@
176 LinkNotFoundError,181 LinkNotFoundError,
177 browser.getLink, 'Rescore build')182 browser.getLink, 'Rescore build')
178183
184 self.assertRaises(
185 Unauthorized,
186 self.getUserBrowser, build_url + '/+rescore', user=self.chef)
187
179 def test_rescore_build_wrong_state(self):188 def test_rescore_build_wrong_state(self):
180 """If the build isn't queued, you can't rescore it."""189 """If the build isn't queued, you can't rescore it."""
181 experts = getUtility(ILaunchpadCelebrities).bazaar_experts.teamowner190 experts = getUtility(ILaunchpadCelebrities).bazaar_experts.teamowner

Subscribers

People subscribed via source and target branches

to status/vote changes: