[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.
--- 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 <http://www.cubicweb.org/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
--- 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
````````````````````````
--- 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):
--- 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)
--- 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):
--- 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'
--- 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)
--- 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()
--- 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)
--- 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',
--- 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'),
--- 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