# HG changeset patch # User Aurelien Campeas # Date 1382613353 -7200 # Node ID 96dba2efd16df59c30fa6b6d15b758d3f89fcfe7 # Parent 4b89ca0b11ad56a71de976341f6e71780e0d1409 [hooks/security] provide attribute "add" permission As of today, the update permission on attributes is checked at entity *creation* time. This forbids using update permissions the proper way. We set it to be checked at entity update time only. We introduce a specific 'add' permission rule for attributes. For backward compatibility, its default value will be the same as the current 'update' permission. Notes: * needs a new yams version (ticket #149216) * introduces two new 'add_permissions' rdefs (attribute - group|rqlexpr) * if the update permission was () and the bw compat kicks in, the rule is not enforced, to avoid un-creatable entity types -- this restriction will be lifted when the bw compat is gone * small internal refactoring on check_entity_attributes * one small pre 3.6.1 bw compat snippet must be removed from schemaserial Closes #2965518. diff -r 4b89ca0b11ad -r 96dba2efd16d doc/3.18.rst --- a/doc/3.18.rst Fri Jan 10 16:37:12 2014 +0100 +++ b/doc/3.18.rst Thu Oct 24 13:15:53 2013 +0200 @@ -10,6 +10,12 @@ * add a security debugging tool (see `#2920304 `_) +* introduce an `add` permission on attributes, to be interpreted at + entity creation time only and allow the implementation of complex + `update` rules that don't block entity creation (before that the + `update` attribute permission was interpreted at entity creation and + update time) + * the primary view display controller (uicfg) now has a `set_fields_order` method similar to the one available for forms diff -r 4b89ca0b11ad -r 96dba2efd16d doc/book/en/devrepo/datamodel/definition.rst --- a/doc/book/en/devrepo/datamodel/definition.rst Fri Jan 10 16:37:12 2014 +0100 +++ b/doc/book/en/devrepo/datamodel/definition.rst Thu Oct 24 13:15:53 2013 +0200 @@ -330,7 +330,8 @@ For a relation, the possible actions are `read`, `add`, and `delete`. -For an attribute, the possible actions are `read`, and `update`. +For an attribute, the possible actions are `read`, `add` and `update`, +and they are a refinement of an entity type permission. For each access type, a tuple indicates the name of the authorized groups and/or one or multiple RQL expressions to satisfy to grant access. The access is @@ -364,7 +365,8 @@ .. sourcecode:: python __permissions__ = {'read': ('managers', 'users', 'guests',), - 'update': ('managers', ERQLExpression('U has_update_permission X')),} + 'add': ('managers', ERQLExpression('U has_add_permission X'), + 'update': ('managers', ERQLExpression('U has_update_permission X')),} The standard user groups ```````````````````````` diff -r 4b89ca0b11ad -r 96dba2efd16d hooks/security.py --- a/hooks/security.py Fri Jan 10 16:37:12 2014 +0100 +++ b/hooks/security.py Thu Oct 24 13:15:53 2013 +0200 @@ -1,4 +1,4 @@ -# copyright 2003-2012 LOGILAB S.A. (Paris, FRANCE), all rights reserved. +# copyright 2003-2013 LOGILAB S.A. (Paris, FRANCE), all rights reserved. # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr # # This file is part of CubicWeb. @@ -20,6 +20,7 @@ """ __docformat__ = "restructuredtext en" +from warnings import warn from logilab.common.registry import objectify_predicate @@ -29,8 +30,8 @@ from cubicweb.server import BEFORE_ADD_RELATIONS, ON_COMMIT_ADD_RELATIONS, hook -_DEFAULT_UPDATE_ATTRPERM = buildobjs.DEFAULT_ATTRPERMS['update'] -def check_entity_attributes(session, entity, editedattrs=None, creation=False): + +def check_entity_attributes(session, entity, action, editedattrs=None): eid = entity.eid eschema = entity.e_schema # ._cw_skip_security_attributes is there to bypass security for attributes @@ -43,27 +44,25 @@ continue rdef = eschema.rdef(attr) if rdef.final: # non final relation are checked by standard hooks - # attributes only have a specific 'update' permission - updateperm = rdef.permissions.get('update') + perms = rdef.permissions.get(action) # comparison below works because the default update perm is: # - # ('managers', ERQLExpression(Any X WHERE U has_update_permission X, X eid %(x)s, U eid %(u)s)) + # ('managers', ERQLExpression(Any X WHERE U has_update_permission X, + # X eid %(x)s, U eid %(u)s)) # # is deserialized in this order (groups first), and ERQLExpression - # implements comparison by expression. - if updateperm == _DEFAULT_UPDATE_ATTRPERM: - # The default update permission is to delegate to the entity - # update permission. This is an historical artefact but it is - # costly (in general). Hence we take this permission object as a - # marker saying "no specific" update permissions for this - # attribute. Thus we just do nothing. + # implements comparison by rql expression. + if perms == buildobjs.DEFAULT_ATTRPERMS[action]: + # The default rule is to delegate to the entity + # rule. This is an historical artefact. Hence we take + # this object as a marker saying "no specific" + # permission rule for this attribute. Thus we just do + # nothing. continue - if creation and updateperm == (): - # That actually means an immutable attribute. We make an - # _exception_ to the `check attr update perms at entity create & - # update time` rule for this case. - continue - rdef.check_perm(session, 'update', eid=eid) + if perms == (): + # That means an immutable attribute. + raise Unauthorized(action, str(rdef)) + rdef.check_perm(session, action, eid=eid) class CheckEntityPermissionOp(hook.DataOperationMixIn, hook.LateOperation): @@ -72,8 +71,7 @@ for eid, action, edited in self.get_data(): entity = session.entity_from_eid(eid) entity.cw_check_perm(action) - check_entity_attributes(session, entity, edited, - creation=(action == 'add')) + check_entity_attributes(session, entity, action, edited) class CheckRelationPermissionOp(hook.DataOperationMixIn, hook.LateOperation): diff -r 4b89ca0b11ad -r 96dba2efd16d misc/migration/3.18.0_Any.py --- a/misc/migration/3.18.0_Any.py Fri Jan 10 16:37:12 2014 +0100 +++ b/misc/migration/3.18.0_Any.py Thu Oct 24 13:15:53 2013 +0200 @@ -118,3 +118,12 @@ args['x'] = eschema.eid session.execute(rql, args) commit() + + +add_relation_definition('CWAttribute', 'add_permission', 'CWGroup') +add_relation_definition('CWAttribute', 'add_permission', 'RQLExpression') + +# all attributes perms have to be refreshed ... +for rschema in schema.relations(): + if relation.final: + sync_schema_props_perms(rschema.type, syncprops=False) diff -r 4b89ca0b11ad -r 96dba2efd16d schema.py --- a/schema.py Fri Jan 10 16:37:12 2014 +0100 +++ b/schema.py Thu Oct 24 13:15:53 2013 +0200 @@ -330,6 +330,8 @@ return 'Any %s WHERE %s' % (','.join(sorted(self.mainvars)), self.expression) + + # rql expressions for use in permission definition ############################# class ERQLExpression(RQLExpression): @@ -395,6 +397,16 @@ kwargs['o'] = toeid return self._check(_cw, **kwargs) + +# In yams, default 'update' perm for attributes granted to managers and owners. +# Within cw, we want to default to users who may edit the entity holding the +# attribute. +# These default permissions won't be checked by the security hooks: +# since they delegate checking to the entity, we can skip actual checks. +ybo.DEFAULT_ATTRPERMS['update'] = ('managers', ERQLExpression('U has_update_permission X')) +ybo.DEFAULT_ATTRPERMS['add'] = ('managers', ERQLExpression('U has_add_permission X')) + + PUB_SYSTEM_ENTITY_PERMS = { 'read': ('managers', 'users', 'guests',), 'add': ('managers',), @@ -408,6 +420,7 @@ } PUB_SYSTEM_ATTR_PERMS = { 'read': ('managers', 'users', 'guests',), + 'add': ('managers',), 'update': ('managers',), } RO_REL_PERMS = { @@ -417,6 +430,7 @@ } RO_ATTR_PERMS = { 'read': ('managers', 'users', 'guests',), + 'add': ybo.DEFAULT_ATTRPERMS['add'], 'update': (), } @@ -951,12 +965,6 @@ return self._eid_index[eid] -# in yams, default 'update' perm for attributes granted to managers and owners. -# Within cw, we want to default to users who may edit the entity holding the -# attribute. -ybo.DEFAULT_ATTRPERMS['update'] = ( - 'managers', ERQLExpression('U has_update_permission X')) - # additional cw specific constraints ########################################### class BaseRQLConstraint(RRQLExpression, BaseConstraint): diff -r 4b89ca0b11ad -r 96dba2efd16d schemas/bootstrap.py --- a/schemas/bootstrap.py Fri Jan 10 16:37:12 2014 +0100 +++ b/schemas/bootstrap.py Thu Oct 24 13:15:53 2013 +0200 @@ -236,7 +236,7 @@ """groups allowed to add entities/relations of this type""" __permissions__ = PUB_SYSTEM_REL_PERMS name = 'add_permission' - subject = ('CWEType', 'CWRelation') + subject = ('CWEType', 'CWRelation', 'CWAttribute') object = 'CWGroup' cardinality = '**' @@ -269,7 +269,7 @@ """rql expression allowing to add entities/relations of this type""" __permissions__ = PUB_SYSTEM_REL_PERMS name = 'add_permission' - subject = ('CWEType', 'CWRelation') + subject = ('CWEType', 'CWRelation', 'CWAttribute') object = 'RQLExpression' cardinality = '*?' composite = 'subject' diff -r 4b89ca0b11ad -r 96dba2efd16d server/schemaserial.py --- a/server/schemaserial.py Fri Jan 10 16:37:12 2014 +0100 +++ b/server/schemaserial.py Thu Oct 24 13:15:53 2013 +0200 @@ -304,9 +304,6 @@ except KeyError: return for action, somethings in thispermsdict.iteritems(): - # XXX cw < 3.6.1 bw compat - if isinstance(erschema, schemamod.RelationDefinitionSchema) and erschema.final and action == 'add': - action = 'update' erschema.permissions[action] = tuple( isinstance(p, tuple) and erschema.rql_expression(*p) or p for p in somethings) diff -r 4b89ca0b11ad -r 96dba2efd16d server/test/data/schema.py --- a/server/test/data/schema.py Fri Jan 10 16:37:12 2014 +0100 +++ b/server/test/data/schema.py Thu Oct 24 13:15:53 2013 +0200 @@ -1,4 +1,4 @@ -# copyright 2003-2011 LOGILAB S.A. (Paris, FRANCE), all rights reserved. +# copyright 2003-2013 LOGILAB S.A. (Paris, FRANCE), all rights reserved. # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr # # This file is part of CubicWeb. @@ -94,7 +94,12 @@ 'read': ('managers', 'users', 'guests'), 'update': ('managers', ERQLExpression('X in_state S, S name "todo"')), }) - + something = String(maxsize=1, + __permissions__ = { + 'read': ('managers', 'users', 'guests'), + 'add': (ERQLExpression('NOT X para NULL'),), + 'update': ('managers', 'owners') + }) migrated_from = SubjectRelation('Note') attachment = SubjectRelation('File') inline1 = SubjectRelation('Affaire', inlined=True, cardinality='?*', @@ -119,6 +124,7 @@ tzdatenaiss = TZDatetime() test = Boolean(__permissions__={ 'read': ('managers', 'users', 'guests'), + 'add': ('managers',), 'update': ('managers',), }) description = String() diff -r 4b89ca0b11ad -r 96dba2efd16d server/test/unittest_migractions.py --- a/server/test/unittest_migractions.py Fri Jan 10 16:37:12 2014 +0100 +++ b/server/test/unittest_migractions.py Thu Oct 24 13:15:53 2013 +0200 @@ -416,8 +416,8 @@ self.assertEqual(eexpr.reverse_read_permission, ()) self.assertEqual(eexpr.reverse_delete_permission, ()) self.assertEqual(eexpr.reverse_update_permission, ()) - # no more rqlexpr to delete and add para attribute - self.assertFalse(self._rrqlexpr_rset('add', 'para')) + self.assertTrue(self._rrqlexpr_rset('add', 'para')) + # no rqlexpr to delete para attribute self.assertFalse(self._rrqlexpr_rset('delete', 'para')) # new rql expr to add ecrit_par relation rexpr = self._rrqlexpr_entity('add', 'ecrit_par') @@ -445,19 +445,24 @@ self.assertEqual(len(self._rrqlexpr_rset('delete', 'concerne')), len(delete_concerne_rqlexpr)) self.assertEqual(len(self._rrqlexpr_rset('add', 'concerne')), len(add_concerne_rqlexpr)) # * migrschema involve: - # * 7 rqlexprs deletion (2 in (Affaire read + Societe + travaille) + 1 - # in para attribute) + # * 7 erqlexprs deletions (2 in (Affaire + Societe + Note.para) + 1 Note.something + # * 2 rrqlexprs deletions (travaille) # * 1 update (Affaire update) # * 2 new (Note add, ecrit_par add) - # * 2 implicit new for attributes update_permission (Note.para, Personne.test) + # * 2 implicit new for attributes (Note.para, Person.test) # remaining orphan rql expr which should be deleted at commit (composite relation) - self.assertEqual(cursor.execute('Any COUNT(X) WHERE X is RQLExpression, ' - 'NOT ET1 read_permission X, NOT ET2 add_permission X, ' - 'NOT ET3 delete_permission X, NOT ET4 update_permission X')[0][0], - 7+1) + # unattached expressions -> pending deletion on commit + self.assertEqual(cursor.execute('Any COUNT(X) WHERE X is RQLExpression, X exprtype "ERQLExpression",' + 'NOT ET1 read_permission X, NOT ET2 add_permission X, ' + 'NOT ET3 delete_permission X, NOT ET4 update_permission X')[0][0], + 7) + self.assertEqual(cursor.execute('Any COUNT(X) WHERE X is RQLExpression, X exprtype "RRQLExpression",' + 'NOT ET1 read_permission X, NOT ET2 add_permission X, ' + 'NOT ET3 delete_permission X, NOT ET4 update_permission X')[0][0], + 2) # finally self.assertEqual(cursor.execute('Any COUNT(X) WHERE X is RQLExpression')[0][0], - nbrqlexpr_start + 1 + 2 + 2) + nbrqlexpr_start + 1 + 2 + 2 + 2) self.mh.commit() # unique_together test self.assertEqual(len(self.schema.eschema('Personne')._unique_together), 1) diff -r 4b89ca0b11ad -r 96dba2efd16d server/test/unittest_repository.py --- a/server/test/unittest_repository.py Fri Jan 10 16:37:12 2014 +0100 +++ b/server/test/unittest_repository.py Thu Oct 24 13:15:53 2013 +0200 @@ -278,7 +278,7 @@ 'creation_date', 'modification_date', 'cwuri', 'owned_by', 'created_by', 'cw_source', 'update_permission', 'read_permission', - 'in_basket')) + 'add_permission', 'in_basket')) self.assertListEqual(['relation_type', 'from_entity', 'to_entity', 'constrained_by', diff -r 4b89ca0b11ad -r 96dba2efd16d server/test/unittest_schemaserial.py --- a/server/test/unittest_schemaserial.py Fri Jan 10 16:37:12 2014 +0100 +++ b/server/test/unittest_schemaserial.py Thu Oct 24 13:15:53 2013 +0200 @@ -133,7 +133,12 @@ 'description': u'groups allowed to add entities/relations of this type', 'composite': None, 'ordernum': 9999, 'cardinality': u'**'}), ('INSERT CWRelation X: X cardinality %(cardinality)s,X composite %(composite)s,X description %(description)s,X ordernum %(ordernum)s,X relation_type ER,X from_entity SE,X to_entity OE WHERE SE eid %(se)s,ER eid %(rt)s,OE eid %(oe)s', {'se': None, 'rt': None, 'oe': None, - 'description': u'rql expression allowing to add entities/relations of this type', 'composite': 'subject', 'ordernum': 9999, 'cardinality': u'*?'})], + 'description': u'rql expression allowing to add entities/relations of this type', 'composite': 'subject', 'ordernum': 9999, 'cardinality': u'*?'}), + ('INSERT CWRelation X: X cardinality %(cardinality)s,X composite %(composite)s,X description %(description)s,X ordernum %(ordernum)s,X relation_type ER,X from_entity SE,X to_entity OE WHERE SE eid %(se)s,ER eid %(rt)s,OE eid %(oe)s', + {'cardinality': u'**', 'composite': None, 'description': u'groups allowed to add entities/relations of this type', + 'oe': None, 'ordernum': 9999, 'rt': None, 'se': None}), + ('INSERT CWRelation X: X cardinality %(cardinality)s,X composite %(composite)s,X description %(description)s,X ordernum %(ordernum)s,X relation_type ER,X from_entity SE,X to_entity OE WHERE SE eid %(se)s,ER eid %(rt)s,OE eid %(oe)s', + {'cardinality': u'*?', 'composite': u'subject', 'description': u'rql expression allowing to add entities/relations of this type', 'oe': None, 'ordernum': 9999, 'rt': None, 'se': None})], list(rschema2rql(schema.rschema('add_permission'), cstrtypemap))) def test_rschema2rql3(self): @@ -266,6 +271,7 @@ self.assertListEqual([('SET X read_permission Y WHERE Y eid %(g)s, X eid %(x)s', {'g': 0}), ('SET X read_permission Y WHERE Y eid %(g)s, X eid %(x)s', {'g': 1}), ('SET X read_permission Y WHERE Y eid %(g)s, X eid %(x)s', {'g': 2}), + ('SET X add_permission Y WHERE Y eid %(g)s, X eid %(x)s', {'g': 0}), ('SET X update_permission Y WHERE Y eid %(g)s, X eid %(x)s', {'g': 0})], [(rql, kwargs) for rql, kwargs in erperms2rql(schema.rschema('name').rdef('CWEType', 'String'), diff -r 4b89ca0b11ad -r 96dba2efd16d server/test/unittest_security.py --- a/server/test/unittest_security.py Fri Jan 10 16:37:12 2014 +0100 +++ b/server/test/unittest_security.py Thu Oct 24 13:15:53 2013 +0200 @@ -413,6 +413,16 @@ self.commit() cu.execute("SET X para 'chouette' WHERE X eid %(x)s", {'x': note2.eid}) self.commit() + cu.execute("INSERT Note X: X something 'A'") + self.assertRaises(Unauthorized, self.commit) + cu.execute("INSERT Note X: X para 'zogzog', X something 'A'") + self.commit() + note = cu.execute("INSERT Note X").get_entity(0,0) + self.commit() + note.cw_set(something=u'B') + self.commit() + note.cw_set(something=None, para=u'zogzog') + self.commit() def test_attribute_read_security(self): # anon not allowed to see users'login, but they can see users