[hooks/security] provide attribute "add" permission
authorAurelien Campeas <aurelien.campeas@logilab.fr>
Thu, 24 Oct 2013 13:15:53 +0200
changeset 9395 96dba2efd16d
parent 9394 4b89ca0b11ad
child 9396 e83cbc116352
[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.
doc/3.18.rst
doc/book/en/devrepo/datamodel/definition.rst
hooks/security.py
misc/migration/3.18.0_Any.py
schema.py
schemas/bootstrap.py
server/schemaserial.py
server/test/data/schema.py
server/test/unittest_migractions.py
server/test/unittest_repository.py
server/test/unittest_schemaserial.py
server/test/unittest_security.py
--- 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