Launchpad itself

Merge lp:~henninge/launchpad/bug-515680-status into lp:launchpad

  1. bug-515680-status
  2. Merge into devel
Proposed by Henning Eggers
Status: Merged
Approved by: Henning Eggers
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~henninge/launchpad/bug-515680-status
Merge into: lp:launchpad
Diff against target: 217 lines (+80/-48)
4 files modified
lib/canonical/launchpad/security.py (+6/-20)
lib/lp/translations/interfaces/translationimportqueue.py (+5/-9)
lib/lp/translations/model/translationimportqueue.py (+26/-14)
lib/lp/translations/tests/test_translationimportqueue.py (+43/-5)
To merge this branch: bzr merge lp:~henninge/launchpad/bug-515680-status
Reviewer Review Type Date Requested Status
Eleanor Berger (community) code Approve
Review via email: mp+18517@code.launchpad.net

Commit message

Translation importers cannot set their queue entry to Blocked now.

To post a comment you must log in.
Revision history for this message
Henning Eggers (henninge) wrote :

= Bug 515680 =

The goal of this branch is to restrict access to the BLOCKED status to project owners and drivers (and admins, of course). The uploader should not be able to set their entry to BLOCKED.

== Proposed fix ==

Update TranslationImportQueueEntry.canSetStatus to disallow importers to set BLOCKED which it currently allows.

== Implementation details ==

I used this oportunity to update some very specific "is*" methods to be called "canEdit" and "canAdmin" and behave accordingly. This also changed their use in security.py.

== Test ==

./bin/test -vvct TestCanSetStatus

== Demo/QA ==

As the member of a translation team but not the owner or driver of the project, upload a file to a project. Go to them import queue and check that "Blocked" is not available to you. Go there as the maintainer or driver of the project and you can set it.

As a corner case, upload a file as the maintainer, Blocked should still be available.

= 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/lp/translations/interfaces/translationimportqueue.py
  lib/lp/translations/model/translationimportqueue.py
  lib/lp/translations/tests/test_translationimportqueue.py

== Pylint notices ==

lib/lp/translations/interfaces/translationimportqueue.py
    10: [F0401] Unable to import 'lazr.enum' (No module named enum)
    20: [F0401] Unable to import 'lazr.restful.interface' (No module named restful)
    21: [F0401] Unable to import 'lazr.restful.fields' (No module named restful)
    22: [F0401] Unable to import 'lazr.restful.declarations' (No module named restful)

Revision history for this message
Eleanor Berger (intellectronica) :
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-01-20 20:36:13 +0000
+++ lib/canonical/launchpad/security.py 2010-02-03 16:23:12 +0000
@@ -1283,23 +1283,15 @@
1283 self, user)1283 self, user)
12841284
12851285
1286class AdminTranslationImportQueueEntry(OnlyRosettaExpertsAndAdmins):1286class AdminTranslationImportQueueEntry(AuthorizationBase):
1287 permission = 'launchpad.Admin'1287 permission = 'launchpad.Admin'
1288 usedfor = ITranslationImportQueueEntry1288 usedfor = ITranslationImportQueueEntry
12891289
1290 def checkAuthenticated(self, user):1290 def checkAuthenticated(self, user):
1291 if OnlyRosettaExpertsAndAdmins.checkAuthenticated(self, user):1291 return self.obj.canAdmin(user)
1292 return True1292
12931293
1294 # As a special case, the Ubuntu translation group owners can1294class EditTranslationImportQueueEntry(AuthorizationBase):
1295 # manage Ubuntu uploads.
1296 if self.obj.isUbuntuAndIsUserTranslationGroupOwner(user.person):
1297 return True
1298
1299 return False
1300
1301
1302class EditTranslationImportQueueEntry(AdminTranslationImportQueueEntry):
1303 permission = 'launchpad.Edit'1295 permission = 'launchpad.Edit'
1304 usedfor = ITranslationImportQueueEntry1296 usedfor = ITranslationImportQueueEntry
13051297
@@ -1307,13 +1299,7 @@
1307 """Anyone who can admin an entry, plus its owner or the owner of the1299 """Anyone who can admin an entry, plus its owner or the owner of the
1308 product or distribution, can edit it.1300 product or distribution, can edit it.
1309 """1301 """
1310 if AdminTranslationImportQueueEntry.checkAuthenticated(self, user):1302 return self.obj.canEdit(user)
1311 return True
1312 if self.obj.isUserUploaderOrOwner(user.person):
1313 return True
1314
1315 return False
1316
13171303
1318class AdminTranslationImportQueue(OnlyRosettaExpertsAndAdmins):1304class AdminTranslationImportQueue(OnlyRosettaExpertsAndAdmins):
1319 permission = 'launchpad.Admin'1305 permission = 'launchpad.Admin'
13201306
=== modified file 'lib/lp/translations/interfaces/translationimportqueue.py'
--- lib/lp/translations/interfaces/translationimportqueue.py 2009-11-17 09:50:33 +0000
+++ lib/lp/translations/interfaces/translationimportqueue.py 2010-02-03 16:23:13 +0000
@@ -266,15 +266,11 @@
266 required=False,266 required=False,
267 readonly=True))267 readonly=True))
268268
269 def isUbuntuAndIsUserTranslationGroupOwner(self, user):269 def canAdmin(roles):
270 """Check for special Ubuntu Translation Group.270 """Check if the user can administer this entry."""
271271
272 Return true if the entry is targeted to Ubuntu and the user is in272 def canEdit(roles):
273 the team owning the Ubuntu translation group.273 """Check if the user can edit this entry."""
274 """
275
276 def isUserUploaderOrOwner(user):
277 """Check for entry uploader or series owner."""
278274
279 def canSetStatus(new_status, user):275 def canSetStatus(new_status, user):
280 """Check if the user can set this new status."""276 """Check if the user can set this new status."""
281277
=== modified file 'lib/lp/translations/model/translationimportqueue.py'
--- lib/lp/translations/model/translationimportqueue.py 2010-01-26 15:25:53 +0000
+++ lib/lp/translations/model/translationimportqueue.py 2010-02-03 16:23:13 +0000
@@ -277,26 +277,37 @@
277 distroseries=self.distroseries,277 distroseries=self.distroseries,
278 sourcepackagename=self.sourcepackagename).one()278 sourcepackagename=self.sourcepackagename).one()
279279
280 def isUbuntuAndIsUserTranslationGroupOwner(self, user):280 def canAdmin(self, roles):
281 """See `ITranslationImportQueueEntry`."""281 """See `ITranslationImportQueueEntry`."""
282 # As a special case, the Ubuntu translation group owners can282 # As a special case, the Ubuntu translation group owners can
283 # manage Ubuntu uploads.283 # manage Ubuntu uploads.
284 if self.is_targeted_to_ubuntu:284 if self.is_targeted_to_ubuntu:
285 group = self.distroseries.distribution.translationgroup285 group = self.distroseries.distribution.translationgroup
286 if group is not None and user.inTeam(group.owner):286 if group is not None and roles.inTeam(group.owner):
287 return True287 return True
288 # Rosetta experts and admins can administer the entry.
289 return roles.in_rosetta_experts or roles.in_admin
290
291 def _canEditExcludeImporter(self, roles):
292 """All people that can edit the entry except the importer."""
293 # Admin rights include edit rights.
294 if self.canAdmin(roles):
295 return True
296 # The maintainer and the drivers can edit the entry.
297 if self.productseries is not None:
298 return (roles.isOwner(self.productseries.product) or
299 roles.isOneOfDrivers(self.productseries))
300 if self.distroseries is not None:
301 return (roles.isOwner(self.distroseries.distribution) or
302 roles.isOneOfDrivers(self.distroseries))
288 return False303 return False
289304
290 def isUserUploaderOrOwner(self, user):305 def canEdit(self, roles):
291 """See `ITranslationImportQueueEntry`."""306 """See `ITranslationImportQueueEntry`."""
292 roles = IPersonRoles(user)307 # The importer can edit the entry.
293 if roles.inTeam(self.importer):308 if roles.inTeam(self.importer):
294 return True309 return True
295 if self.productseries is not None:310 return self._canEditExcludeImporter(roles)
296 return roles.isOwner(self.productseries.product)
297 if self.distroseries is not None:
298 return roles.isOwner(self.distroseries.distribution)
299 return False
300311
301 def canSetStatus(self, new_status, user):312 def canSetStatus(self, new_status, user):
302 """See `ITranslationImportQueueEntry`."""313 """See `ITranslationImportQueueEntry`."""
@@ -304,13 +315,11 @@
304 # Anonymous user cannot do anything.315 # Anonymous user cannot do anything.
305 return False316 return False
306 roles = IPersonRoles(user)317 roles = IPersonRoles(user)
307 can_admin = (roles.in_admin or roles.in_rosetta_experts or
308 self.isUbuntuAndIsUserTranslationGroupOwner(user))
309 if new_status == RosettaImportStatus.APPROVED:318 if new_status == RosettaImportStatus.APPROVED:
310 # Only administrators are able to set the APPROVED status, and319 # Only administrators are able to set the APPROVED status, and
311 # that's only possible if we know where to import it320 # that's only possible if we know where to import it
312 # (import_into not None).321 # (import_into not None).
313 return can_admin and self.import_into is not None322 return self.canAdmin(roles) and self.import_into is not None
314 if new_status == RosettaImportStatus.IMPORTED:323 if new_status == RosettaImportStatus.IMPORTED:
315 # Only rosetta experts are able to set the IMPORTED status, and324 # Only rosetta experts are able to set the IMPORTED status, and
316 # that's only possible if we know where to import it325 # that's only possible if we know where to import it
@@ -320,8 +329,11 @@
320 if new_status == RosettaImportStatus.FAILED:329 if new_status == RosettaImportStatus.FAILED:
321 # Only rosetta experts are able to set the FAILED status.330 # Only rosetta experts are able to set the FAILED status.
322 return roles.in_admin or roles.in_rosetta_experts331 return roles.in_admin or roles.in_rosetta_experts
323 # All other statuses can bset set by all authorized persons.332 if new_status == RosettaImportStatus.BLOCKED:
324 return self.isUserUploaderOrOwner(user) or can_admin333 # Importers are not allowed to set BLOCKED
334 return self._canEditExcludeImporter(roles)
335 # All other statuses can be set set by all authorized persons.
336 return self.canEdit(roles)
325337
326 def setStatus(self, new_status, user):338 def setStatus(self, new_status, user):
327 """See `ITranslationImportQueueEntry`."""339 """See `ITranslationImportQueueEntry`."""
328340
=== modified file 'lib/lp/translations/tests/test_translationimportqueue.py'
--- lib/lp/translations/tests/test_translationimportqueue.py 2009-12-18 10:15:14 +0000
+++ lib/lp/translations/tests/test_translationimportqueue.py 2010-02-03 16:23:13 +0000
@@ -33,6 +33,8 @@
33 self.rosetta_experts = (33 self.rosetta_experts = (
34 getUtility(ILaunchpadCelebrities).rosetta_experts)34 getUtility(ILaunchpadCelebrities).rosetta_experts)
35 self.productseries = self.factory.makeProductSeries()35 self.productseries = self.factory.makeProductSeries()
36 self.productseries.driver = self.factory.makePerson()
37 self.productseries.product.driver = self.factory.makePerson()
36 self.uploaderperson = self.factory.makePerson()38 self.uploaderperson = self.factory.makePerson()
3739
38 def _switch_dbuser(self):40 def _switch_dbuser(self):
@@ -84,11 +86,47 @@
84 # The uploader can set some statuses.86 # The uploader can set some statuses.
85 self._assertCanSetStatus(self.uploaderperson, self.entry,87 self._assertCanSetStatus(self.uploaderperson, self.entry,
86 # A B D F I NR88 # A B D F I NR
87 [False, True, True, False, False, True])89 [False, False, True, False, False, True])
8890
89 def test_canSetStatus_owner(self):91 def test_canSetStatus_product_owner(self):
90 # The owner gets the same permissions.92 # The owner (maintainer) of the product gets to set Blocked as well.
91 self._assertCanSetStatus(self.productseries.product.owner, self.entry,93 owner = self.productseries.product.owner
94 self._assertCanSetStatus(owner, self.entry,
95 # A B D F I NR
96 [False, True, True, False, False, True])
97
98 def test_canSetStatus_owner_and_uploader(self):
99 # Corner case: Nothing changes if the maintainer is also the uploader.
100 self.productseries.product.owner = self.uploaderperson
101 self._assertCanSetStatus(self.uploaderperson, self.entry,
102 # A B D F I NR
103 [False, True, True, False, False, True])
104
105 def test_canSetStatus_driver(self):
106 # The driver gets the same permissions as the maintainer.
107 driver = self.productseries.driver
108 self._assertCanSetStatus(driver, self.entry,
109 # A B D F I NR
110 [False, True, True, False, False, True])
111
112 def test_canSetStatus_driver_and_uploader(self):
113 # Corner case: Nothing changes if the driver is also the uploader.
114 self.productseries.driver = self.uploaderperson
115 self._assertCanSetStatus(self.uploaderperson, self.entry,
116 # A B D F I NR
117 [False, True, True, False, False, True])
118
119 def test_canSetStatus_product_driver(self):
120 # The driver of the product, too.
121 driver = self.productseries.product.driver
122 self._assertCanSetStatus(driver, self.entry,
123 # A B D F I NR
124 [False, True, True, False, False, True])
125
126 def test_canSetStatus_product_driver_and_uploader(self):
127 # Corner case: Nothing changes if the driver is also the uploader.
128 self.productseries.product.driver = self.uploaderperson
129 self._assertCanSetStatus(self.uploaderperson, self.entry,
92 # A B D F I NR130 # A B D F I NR
93 [False, True, True, False, False, True])131 [False, True, True, False, False, True])
94132