launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03361
[Merge] lp:~adeuring/launchpad/api-query-permissions-on-object into lp:launchpad
Abel Deuring has proposed merging lp:~adeuring/launchpad/api-query-permissions-on-object into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #758920 in Launchpad itself: "On translate sharing details page, usage settings behaves badly with permission issues"
https://bugs.launchpad.net/launchpad/+bug/758920
For more details, see:
https://code.launchpad.net/~adeuring/launchpad/api-query-permissions-on-object/+merge/58136
This branch adds two new methods IPerson.canAccess() and IPerson.canWrite(), which tell if a given person can access or change attributes of an object. The original idea was to expose these methods via the webservice API, but that seems to be at least quite difficult, if not impossible: The @operation_parameters decorator wants a quite precise type definition. Something like "obj=Reference(schema=Interface)" does not work.
The goal of this work is to allow Javascript code to figure out if the current user can access a property or method or if he can set a a property in order to decide if the current user should be shown some controls to change some settings. This can also be achieved by properties IObject.user_can_do_something, defined for the
The actual issue is bug 758920: The JS code for the translation sharing details page needs to know, if the current user can set the branch of a series, and if he can change some translation retaed settings. This branch adds a property IProductSeries.user_can_set_branch, which calls the new method IPseron.canWrite().
I'll add similar properties in another branch.
I discussed the details of the canWrite(), canAccess() methods wioth Gary; Aarn suggested the usage of properties.
tests:
./bin/test registry -vvt test_productseries.TestProductSeriesPermissionProperties
./bin/test registry -vvt TestPerson.test_can
--
https://code.launchpad.net/~adeuring/launchpad/api-query-permissions-on-object/+merge/58136
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/api-query-permissions-on-object into lp:launchpad.
=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py 2011-04-09 01:40:20 +0000
+++ lib/lp/registry/interfaces/person.py 2011-04-18 14:39:29 +0000
@@ -1570,6 +1570,24 @@
If no orderby is provided, Person.sortingColumns is used.
"""
+ def canAccess(obj, attributes):
+ """True if this person can access all given attributes of the object.
+
+ :param obj: The object to be checked.
+ :param attributes: a sequence of attribute names to check.
+ :return: True if the person can access all given attributes
+ of the given object, else False.
+ """
+
+ def canWrite(obj, attributes):
+ """True if this person can write all given attributes of the object.
+
+ :param obj: The object to be checked.
+ :param attributes: a sequence of attribute names to check.
+ :return: True if the person can write all given attributes
+ of the given object, else False.
+ """
+
class IPersonEditRestricted(Interface):
"""IPerson attributes that require launchpad.Edit permission."""
=== modified file 'lib/lp/registry/interfaces/productseries.py'
--- lib/lp/registry/interfaces/productseries.py 2011-04-11 02:32:50 +0000
+++ lib/lp/registry/interfaces/productseries.py 2011-04-18 14:39:29 +0000
@@ -309,6 +309,11 @@
is_development_focus = Attribute(
_("Is this series the development focus for the product?"))
+ user_can_set_branch = exported(
+ Bool(
+ title=u'Can the current user set the branch of the series?',
+ readonly=True, required=False))
+
@operation_parameters(
include_inactive=Bool(title=_("Include inactive"),
required=False, default=False))
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2011-04-08 21:42:59 +0000
+++ lib/lp/registry/model/person.py 2011-04-18 14:39:29 +0000
@@ -90,7 +90,13 @@
implements,
)
from zope.lifecycleevent import ObjectCreatedEvent
+from zope.principalregistry.principalregistry import UnauthenticatedPrincipal
from zope.publisher.interfaces import Unauthorized
+from zope.security.checker import (
+ canAccess,
+ canWrite,
+ )
+from zope.security.management import getInteraction
from zope.security.proxy import (
ProxyFactory,
removeSecurityProxy,
@@ -2805,6 +2811,34 @@
SourcePackageRecipe,
SourcePackageRecipe.owner == self)
+ def checkPermissionOnObject(self, obj, attributes, check_operation):
+ """Run check_operation on the given obj for the given attributes."""
+ interaction = getInteraction()
+ if len(interaction.participations) != 1:
+ raise NotImplemented(
+ 'checkPermissionOnObject() requires at present exactly '
+ 'one participation.' % check_operation.__name__)
+ principal = interaction.participations[0].principal
+ # For security reasons, and because zope.security.checker.canAccess()
+ # can only permissions for the current user, any user can
+ # only check permissions of him/herself.
+ # See also bug xxxxxxxxx
+ if isinstance(principal, UnauthenticatedPrincipal):
+ raise Unauthorized('Only logged in users may call canAccess()')
+ if principal.person != self:
+ raise Unauthorized('Users can query only their own permissions')
+
+ return all(
+ [check_operation(obj, attribute) for attribute in attributes])
+
+ def canAccess(self, obj, attributes):
+ """See `IPerson.`"""
+ return self.checkPermissionOnObject(obj, attributes, canAccess)
+
+ def canWrite(self, obj, attributes):
+ """See `IPerson.`"""
+ return self.checkPermissionOnObject(obj, attributes, canWrite)
+
class PersonSet:
"""The set of persons."""
=== modified file 'lib/lp/registry/model/productseries.py'
--- lib/lp/registry/model/productseries.py 2011-04-14 13:36:15 +0000
+++ lib/lp/registry/model/productseries.py 2011-04-18 14:39:29 +0000
@@ -41,6 +41,7 @@
sqlvalues,
)
from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
+from canonical.launchpad.webapp.interfaces import ILaunchBag
from canonical.launchpad.webapp.publisher import canonical_url
from canonical.launchpad.webapp.sorting import sorted_dotted_numbers
from lp.app.errors import NotFoundError
@@ -294,6 +295,14 @@
"""See `IProductSeries`."""
return self == self.product.development_focus
+ @property
+ def user_can_set_branch(self):
+ """See `IProductSeries`."""
+ user = getUtility(ILaunchBag).user
+ if user is None:
+ return False
+ return user.canWrite(self, ('branch', ))
+
def specifications(self, sort=None, quantity=None, filter=None,
prejoin_people=True):
"""See IHasSpecifications.
=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py 2011-04-09 02:27:34 +0000
+++ lib/lp/registry/tests/test_person.py 2011-04-18 14:39:29 +0000
@@ -12,6 +12,7 @@
import transaction
from zope.component import getUtility
from zope.interface import providedBy
+from zope.security.interfaces import Unauthorized
from zope.security.proxy import removeSecurityProxy
from canonical.database.sqlbase import cursor
@@ -312,6 +313,88 @@
user = self.factory.makePerson()
self.assertFalse(user.selfgenerated_bugnotifications)
+ def test_canAccess__anonymous(self):
+ # Anonymous users cannot call Person.canAccess()
+ person = self.factory.makePerson()
+ product = self.factory.makeProduct()
+ self.assertRaises(
+ Unauthorized, person.canAccess, product, ['licenses'])
+
+ def test_canAccess__checking_own_permissions(self):
+ # Logged in users can call Person.canAccess() on their own
+ # Person object.
+ person = self.factory.makePerson()
+ product = self.factory.makeProduct()
+ with person_logged_in(person):
+ self.assertTrue(person.canAccess(product, ['licenses']))
+ self.assertFalse(person.canAccess(product, ['newSeries']))
+
+ def test_canAccess__checking_permissions_of_others(self):
+ # Logged in users cannot call Person.canAccess() on Person
+ # object for other people.
+ person = self.factory.makePerson()
+ other = self.factory.makePerson()
+ product = self.factory.makeProduct()
+ with person_logged_in(person):
+ self.assertRaises(
+ Unauthorized, other.canAccess, product, ['licenses'])
+
+ def test_canAccess__check_two_attributes(self):
+ # If access to more than one attribute is checked,
+ # Person.canAccess() returns True if and only if
+ # the user has access to all given attributes.
+ person = self.factory.makePerson()
+ product = self.factory.makeProduct()
+ with person_logged_in(person):
+ self.assertTrue(person.canAccess(product, ['name']))
+ self.assertTrue(person.canAccess(product, ['name', 'licenses']))
+ self.assertFalse(person.canAccess(product, ['name', 'newSeries']))
+
+ def test_canWrite__anonymous(self):
+ # Anonymous users cannot call Person.canWrite()
+ person = self.factory.makePerson()
+ product = self.factory.makeProduct()
+ self.assertRaises(
+ Unauthorized, person.canWrite, product, ['displayname'])
+
+ def test_canWrite__checking_own_permissions(self):
+ # Logged in users can call Person.canWrite() on their own
+ # Person object.
+ person = self.factory.makePerson()
+ product = self.factory.makeProduct()
+ with person_logged_in(person):
+ self.assertFalse(person.canWrite(product, ['displayname']))
+ with person_logged_in(product.owner):
+ self.assertTrue(product.owner.canWrite(product, ['displayname']))
+
+ def test_canWrite__checking_permissions_of_others(self):
+ # Logged in users cannot call Person.canWrite() on Person
+ # object for other people.
+ person = self.factory.makePerson()
+ other = self.factory.makePerson()
+ product = self.factory.makeProduct()
+ with person_logged_in(person):
+ self.assertRaises(
+ Unauthorized, other.canWrite, product, ['displayname'])
+
+ def test_canWrite__check_two_attributes(self):
+ # If access to more than one attribute is checked,
+ # Person.canWrite() returns True if and only if
+ # the user has access to all given attributes.
+ bug_supervisor = self.factory.makePerson()
+ product = self.factory.makeProduct(bug_supervisor=bug_supervisor)
+ with person_logged_in(bug_supervisor):
+ allowed_1 = 'enable_bugfiling_duplicate_search'
+ allowed_2 = 'bug_reported_acknowledgement'
+ forbidden = 'description'
+ self.assertTrue(bug_supervisor.canWrite(product, [allowed_1]))
+ self.assertTrue(bug_supervisor.canWrite(product, [allowed_2]))
+ self.assertFalse(bug_supervisor.canWrite(product, [forbidden]))
+ self.assertTrue(
+ bug_supervisor.canWrite(product, [allowed_1, allowed_2]))
+ self.assertFalse(
+ bug_supervisor.canWrite(product, [allowed_1, forbidden]))
+
class TestPersonStates(TestCaseWithFactory):
=== modified file 'lib/lp/registry/tests/test_productseries.py'
--- lib/lp/registry/tests/test_productseries.py 2011-04-14 13:36:15 +0000
+++ lib/lp/registry/tests/test_productseries.py 2011-04-18 14:39:29 +0000
@@ -23,6 +23,7 @@
)
from lp.registry.interfaces.series import SeriesStatus
from lp.testing import (
+ person_logged_in,
TestCaseWithFactory,
WebServiceTestCase,
)
@@ -479,3 +480,27 @@
mode = TranslationsBranchImportMode.IMPORT_TRANSLATIONS
ws_series.translations_autoimport_mode = mode.title
ws_series.lp_save()
+
+
+class TestProductSeriesPermissionProperties(TestCaseWithFactory):
+ """Test for permission related properties of ProductSeries."""
+
+ layer = DatabaseFunctionalLayer
+
+ def test_user_can_set_branch__anonymous(self):
+ # user_can_set_branch is False for anonymous users.
+ productseries = self.factory.makeProductSeries()
+ self.assertFalse(productseries.user_can_set_branch)
+
+ def test_user_can_set_branch__unprivileged_user(self):
+ # user_can_set_branch is False for unprivileged users.
+ productseries = self.factory.makeProductSeries()
+ with person_logged_in(self.factory.makePerson()):
+ self.assertFalse(productseries.user_can_set_branch)
+
+ def test_user_can_set_branch__privileged_user(self):
+ # user_can_set_branch is True for privileged users, like the
+ # series owner.
+ productseries = self.factory.makeProductSeries()
+ with person_logged_in(productseries.owner):
+ self.assertTrue(productseries.user_can_set_branch)
