[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.

authorAurelien Campeas <aurelien.campeas@logilab.fr>
changeset96dba2efd16d
branchdefault
phasepublic
hiddenno
parent revision#4b89ca0b11ad [server/test] reorganize data/migratedapp/schema.py to allow comparison with data/schema.py
child revision#e83cbc116352 merge stable
files modified by this revision
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
# HG changeset patch
# User Aurelien Campeas <aurelien.campeas@logilab.fr>
# Date 1382613353 -7200
# Thu Oct 24 13:15:53 2013 +0200
# 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 --git a/doc/3.18.rst b/doc/3.18.rst
@@ -8,10 +8,16 @@
1  --------------------
2 
3  * add a security debugging tool
4    (see `#2920304 <http://www.cubicweb.org/2920304>`_)
5 
6 +* introduce an `add` permission on attributes, to be interpreted at
7 +  entity creation time only and allow the implementation of complex
8 +  `update` rules that don't block entity creation (before that the
9 +  `update` attribute permission was interpreted at entity creation and
10 +  update time)
11 +
12  * the primary view display controller (uicfg) now has a
13    `set_fields_order` method similar to the one available for forms
14 
15  * new method `ResultSet.one(col=0)` to retrive a single entity and enforce the
16    result has only one row (see `#3352314 https://www.cubicweb.org/ticket/3352314`_)
diff --git a/doc/book/en/devrepo/datamodel/definition.rst b/doc/book/en/devrepo/datamodel/definition.rst
@@ -328,11 +328,12 @@
17  For an entity type, the possible actions are `read`, `add`, `update` and
18  `delete`.
19 
20  For a relation, the possible actions are `read`, `add`, and `delete`.
21 
22 -For an attribute, the possible actions are `read`, and `update`.
23 +For an attribute, the possible actions are `read`, `add` and `update`,
24 +and they are a refinement of an entity type permission.
25 
26  For each access type, a tuple indicates the name of the authorized groups and/or
27  one or multiple RQL expressions to satisfy to grant access. The access is
28  provided if the user is in one of the listed groups or if one of the RQL condition
29  is satisfied.
@@ -362,11 +363,12 @@
30  The default permissions for attributes are:
31 
32  .. sourcecode:: python
33 
34     __permissions__ = {'read': ('managers', 'users', 'guests',),
35 -                     'update': ('managers', ERQLExpression('U has_update_permission X')),}
36 +                      'add': ('managers', ERQLExpression('U has_add_permission X'),
37 +                      'update': ('managers', ERQLExpression('U has_update_permission X')),}
38 
39  The standard user groups
40  ````````````````````````
41 
42  * `guests`
diff --git a/hooks/security.py b/hooks/security.py
@@ -1,6 +1,6 @@
43 -# copyright 2003-2012 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
44 +# copyright 2003-2013 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
45  # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
46  #
47  # This file is part of CubicWeb.
48  #
49  # CubicWeb is free software: you can redistribute it and/or modify it under the
@@ -18,21 +18,22 @@
50  """Security hooks: check permissions to add/delete/update entities according to
51  the user connected to a session
52  """
53 
54  __docformat__ = "restructuredtext en"
55 +from warnings import warn
56 
57  from logilab.common.registry import objectify_predicate
58 
59  from yams import buildobjs
60 
61  from cubicweb import Unauthorized
62  from cubicweb.server import BEFORE_ADD_RELATIONS, ON_COMMIT_ADD_RELATIONS, hook
63 
64 
65 -_DEFAULT_UPDATE_ATTRPERM = buildobjs.DEFAULT_ATTRPERMS['update']
66 -def check_entity_attributes(session, entity, editedattrs=None, creation=False):
67 +
68 +def check_entity_attributes(session, entity, action, editedattrs=None):
69      eid = entity.eid
70      eschema = entity.e_schema
71      # ._cw_skip_security_attributes is there to bypass security for attributes
72      # set by hooks by modifying the entity's dictionary
73      if editedattrs is None:
@@ -41,41 +42,38 @@
74      for attr in editedattrs:
75          if attr in dontcheck:
76              continue
77          rdef = eschema.rdef(attr)
78          if rdef.final: # non final relation are checked by standard hooks
79 -            # attributes only have a specific 'update' permission
80 -            updateperm = rdef.permissions.get('update')
81 +            perms = rdef.permissions.get(action)
82              # comparison below works because the default update perm is:
83              #
84 -            #  ('managers', ERQLExpression(Any X WHERE U has_update_permission X, X eid %(x)s, U eid %(u)s))
85 +            #  ('managers', ERQLExpression(Any X WHERE U has_update_permission X,
86 +            #                              X eid %(x)s, U eid %(u)s))
87              #
88              # is deserialized in this order (groups first), and ERQLExpression
89 -            # implements comparison by expression.
90 -            if updateperm == _DEFAULT_UPDATE_ATTRPERM:
91 -                # The default update permission is to delegate to the entity
92 -                # update permission. This is an historical artefact but it is
93 -                # costly (in general). Hence we take this permission object as a
94 -                # marker saying "no specific" update permissions for this
95 -                # attribute. Thus we just do nothing.
96 +            # implements comparison by rql expression.
97 +            if perms == buildobjs.DEFAULT_ATTRPERMS[action]:
98 +                # The default rule is to delegate to the entity
99 +                # rule. This is an historical artefact. Hence we take
100 +                # this object as a marker saying "no specific"
101 +                # permission rule for this attribute. Thus we just do
102 +                # nothing.
103                  continue
104 -            if creation and updateperm == ():
105 -                # That actually means an immutable attribute.  We make an
106 -                # _exception_ to the `check attr update perms at entity create &
107 -                # update time` rule for this case.
108 -                continue
109 -            rdef.check_perm(session, 'update', eid=eid)
110 +            if perms == ():
111 +                # That means an immutable attribute.
112 +                raise Unauthorized(action, str(rdef))
113 +            rdef.check_perm(session, action, eid=eid)
114 
115 
116  class CheckEntityPermissionOp(hook.DataOperationMixIn, hook.LateOperation):
117      def precommit_event(self):
118          session = self.session
119          for eid, action, edited in self.get_data():
120              entity = session.entity_from_eid(eid)
121              entity.cw_check_perm(action)
122 -            check_entity_attributes(session, entity, edited,
123 -                                    creation=(action == 'add'))
124 +            check_entity_attributes(session, entity, action, edited)
125 
126 
127  class CheckRelationPermissionOp(hook.DataOperationMixIn, hook.LateOperation):
128      def precommit_event(self):
129          session = self.session
diff --git a/misc/migration/3.18.0_Any.py b/misc/migration/3.18.0_Any.py
@@ -116,5 +116,14 @@
130          rql_args = schemaserial.uniquetogether2rqls(eschema)
131          for rql, args in rql_args:
132              args['x'] = eschema.eid
133              session.execute(rql, args)
134          commit()
135 +
136 +
137 +add_relation_definition('CWAttribute', 'add_permission', 'CWGroup')
138 +add_relation_definition('CWAttribute', 'add_permission', 'RQLExpression')
139 +
140 +# all attributes perms have to be refreshed ...
141 +for rschema in schema.relations():
142 +    if relation.final:
143 +        sync_schema_props_perms(rschema.type, syncprops=False)
diff --git a/schema.py b/schema.py
@@ -328,10 +328,12 @@
144      @property
145      def minimal_rql(self):
146          return 'Any %s WHERE %s' % (','.join(sorted(self.mainvars)),
147                                      self.expression)
148 
149 +
150 +
151  # rql expressions for use in permission definition #############################
152 
153  class ERQLExpression(RQLExpression):
154      predefined_variables = 'XU'
155 
@@ -393,10 +395,20 @@
156              if toeid is None:
157                  return False
158              kwargs['o'] = toeid
159          return self._check(_cw, **kwargs)
160 
161 +
162 +# In yams, default 'update' perm for attributes granted to managers and owners.
163 +# Within cw, we want to default to users who may edit the entity holding the
164 +# attribute.
165 +# These default permissions won't be checked by the security hooks:
166 +# since they delegate checking to the entity, we can skip actual checks.
167 +ybo.DEFAULT_ATTRPERMS['update'] = ('managers', ERQLExpression('U has_update_permission X'))
168 +ybo.DEFAULT_ATTRPERMS['add'] = ('managers', ERQLExpression('U has_add_permission X'))
169 +
170 +
171  PUB_SYSTEM_ENTITY_PERMS = {
172      'read':   ('managers', 'users', 'guests',),
173      'add':    ('managers',),
174      'delete': ('managers',),
175      'update': ('managers',),
@@ -406,19 +418,21 @@
176      'add':    ('managers',),
177      'delete': ('managers',),
178      }
179  PUB_SYSTEM_ATTR_PERMS = {
180      'read':   ('managers', 'users', 'guests',),
181 +    'add': ('managers',),
182      'update': ('managers',),
183      }
184  RO_REL_PERMS = {
185      'read':   ('managers', 'users', 'guests',),
186      'add':    (),
187      'delete': (),
188      }
189  RO_ATTR_PERMS = {
190      'read':   ('managers', 'users', 'guests',),
191 +    'add': ybo.DEFAULT_ATTRPERMS['add'],
192      'update': (),
193      }
194 
195  # XXX same algorithm as in reorder_cubes and probably other place,
196  # may probably extract a generic function
@@ -949,16 +963,10 @@
197 
198      def schema_by_eid(self, eid):
199          return self._eid_index[eid]
200 
201 
202 -# in yams, default 'update' perm for attributes granted to managers and owners.
203 -# Within cw, we want to default to users who may edit the entity holding the
204 -# attribute.
205 -ybo.DEFAULT_ATTRPERMS['update'] = (
206 -    'managers', ERQLExpression('U has_update_permission X'))
207 -
208  # additional cw specific constraints ###########################################
209 
210  class BaseRQLConstraint(RRQLExpression, BaseConstraint):
211      """base class for rql constraints"""
212      distinct_query = None
diff --git a/schemas/bootstrap.py b/schemas/bootstrap.py
@@ -234,11 +234,11 @@
213 
214  class add_permission_cwgroup(RelationDefinition):
215      """groups allowed to add entities/relations of this type"""
216      __permissions__ = PUB_SYSTEM_REL_PERMS
217      name = 'add_permission'
218 -    subject = ('CWEType', 'CWRelation')
219 +    subject = ('CWEType', 'CWRelation', 'CWAttribute')
220      object = 'CWGroup'
221      cardinality = '**'
222 
223  class delete_permission_cwgroup(RelationDefinition):
224      """groups allowed to delete entities/relations of this type"""
@@ -267,11 +267,11 @@
225 
226  class add_permission_rqlexpr(RelationDefinition):
227      """rql expression allowing to add entities/relations of this type"""
228      __permissions__ = PUB_SYSTEM_REL_PERMS
229      name = 'add_permission'
230 -    subject = ('CWEType', 'CWRelation')
231 +    subject = ('CWEType', 'CWRelation', 'CWAttribute')
232      object = 'RQLExpression'
233      cardinality = '*?'
234      composite = 'subject'
235 
236  class delete_permission_rqlexpr(RelationDefinition):
diff --git a/server/schemaserial.py b/server/schemaserial.py
@@ -302,13 +302,10 @@
237      try:
238          thispermsdict = permsidx[erschema.eid]
239      except KeyError:
240          return
241      for action, somethings in thispermsdict.iteritems():
242 -        # XXX cw < 3.6.1 bw compat
243 -        if isinstance(erschema, schemamod.RelationDefinitionSchema) and erschema.final and action == 'add':
244 -            action = 'update'
245          erschema.permissions[action] = tuple(
246              isinstance(p, tuple) and erschema.rql_expression(*p) or p
247              for p in somethings)
248 
249 
diff --git a/server/test/data/schema.py b/server/test/data/schema.py
@@ -1,6 +1,6 @@
250 -# copyright 2003-2011 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
251 +# copyright 2003-2013 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
252  # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
253  #
254  # This file is part of CubicWeb.
255  #
256  # CubicWeb is free software: you can redistribute it and/or modify it under the
@@ -92,11 +92,16 @@
257      para = String(maxsize=512,
258                    __permissions__ = {
259                        'read':   ('managers', 'users', 'guests'),
260                        'update': ('managers', ERQLExpression('X in_state S, S name "todo"')),
261                        })
262 -
263 +    something = String(maxsize=1,
264 +                      __permissions__ = {
265 +                          'read': ('managers', 'users', 'guests'),
266 +                          'add': (ERQLExpression('NOT X para NULL'),),
267 +                          'update': ('managers', 'owners')
268 +                      })
269      migrated_from = SubjectRelation('Note')
270      attachment = SubjectRelation('File')
271      inline1 = SubjectRelation('Affaire', inlined=True, cardinality='?*',
272                                constraints=[RQLUniqueConstraint('S type T, S inline1 A1, A1 todo_by C, '
273                                                                'Y type T, Y inline1 A2, A2 todo_by C',
@@ -117,10 +122,11 @@
274      fax    = Int()
275      datenaiss = Datetime()
276      tzdatenaiss = TZDatetime()
277      test   = Boolean(__permissions__={
278          'read': ('managers', 'users', 'guests'),
279 +        'add': ('managers',),
280          'update': ('managers',),
281          })
282      description = String()
283      firstname = String(fulltextindexed=True, maxsize=64)
284 
diff --git a/server/test/unittest_migractions.py b/server/test/unittest_migractions.py
@@ -414,12 +414,12 @@
285                            'PE require_permission P, P name "add_note", P require_group G')
286          self.assertEqual([et.name for et in eexpr.reverse_add_permission], ['Note'])
287          self.assertEqual(eexpr.reverse_read_permission, ())
288          self.assertEqual(eexpr.reverse_delete_permission, ())
289          self.assertEqual(eexpr.reverse_update_permission, ())
290 -        # no more rqlexpr to delete and add para attribute
291 -        self.assertFalse(self._rrqlexpr_rset('add', 'para'))
292 +        self.assertTrue(self._rrqlexpr_rset('add', 'para'))
293 +        # no rqlexpr to delete para attribute
294          self.assertFalse(self._rrqlexpr_rset('delete', 'para'))
295          # new rql expr to add ecrit_par relation
296          rexpr = self._rrqlexpr_entity('add', 'ecrit_par')
297          self.assertEqual(rexpr.expression,
298                            'O require_permission P, P name "add_note", '
@@ -443,23 +443,28 @@
299          self.assertEqual(len(self._erqlexpr_rset('add', 'Affaire')), 1)
300          # no change for rqlexpr to add and delete concerne relation
301          self.assertEqual(len(self._rrqlexpr_rset('delete', 'concerne')), len(delete_concerne_rqlexpr))
302          self.assertEqual(len(self._rrqlexpr_rset('add', 'concerne')), len(add_concerne_rqlexpr))
303          # * migrschema involve:
304 -        #   * 7 rqlexprs deletion (2 in (Affaire read + Societe + travaille) + 1
305 -        #     in para attribute)
306 +        #   * 7 erqlexprs deletions (2 in (Affaire + Societe + Note.para) + 1 Note.something
307 +        #   * 2 rrqlexprs deletions (travaille)
308          #   * 1 update (Affaire update)
309          #   * 2 new (Note add, ecrit_par add)
310 -        #   * 2 implicit new for attributes update_permission (Note.para, Personne.test)
311 +        #   * 2 implicit new for attributes (Note.para, Person.test)
312          # remaining orphan rql expr which should be deleted at commit (composite relation)
313 -        self.assertEqual(cursor.execute('Any COUNT(X) WHERE X is RQLExpression, '
314 -                                         'NOT ET1 read_permission X, NOT ET2 add_permission X, '
315 -                                         'NOT ET3 delete_permission X, NOT ET4 update_permission X')[0][0],
316 -                          7+1)
317 +        # unattached expressions -> pending deletion on commit
318 +        self.assertEqual(cursor.execute('Any COUNT(X) WHERE X is RQLExpression, X exprtype "ERQLExpression",'
319 +                                        'NOT ET1 read_permission X, NOT ET2 add_permission X, '
320 +                                        'NOT ET3 delete_permission X, NOT ET4 update_permission X')[0][0],
321 +                          7)
322 +        self.assertEqual(cursor.execute('Any COUNT(X) WHERE X is RQLExpression, X exprtype "RRQLExpression",'
323 +                                        'NOT ET1 read_permission X, NOT ET2 add_permission X, '
324 +                                        'NOT ET3 delete_permission X, NOT ET4 update_permission X')[0][0],
325 +                          2)
326          # finally
327          self.assertEqual(cursor.execute('Any COUNT(X) WHERE X is RQLExpression')[0][0],
328 -                          nbrqlexpr_start + 1 + 2 + 2)
329 +                         nbrqlexpr_start + 1 + 2 + 2 + 2)
330          self.mh.commit()
331          # unique_together test
332          self.assertEqual(len(self.schema.eschema('Personne')._unique_together), 1)
333          self.assertCountEqual(self.schema.eschema('Personne')._unique_together[0],
334                                             ('nom', 'prenom', 'datenaiss'))
diff --git a/server/test/unittest_repository.py b/server/test/unittest_repository.py
@@ -276,11 +276,11 @@
335          # check order of attributes is respected
336          notin = set(('eid', 'is', 'is_instance_of', 'identity',
337                       'creation_date', 'modification_date', 'cwuri',
338                       'owned_by', 'created_by', 'cw_source',
339                       'update_permission', 'read_permission',
340 -                     'in_basket'))
341 +                     'add_permission', 'in_basket'))
342          self.assertListEqual(['relation_type',
343                                'from_entity', 'to_entity',
344                                'constrained_by',
345                                'cardinality', 'ordernum',
346                                'indexed', 'fulltextindexed', 'internationalizable',
diff --git a/server/test/unittest_schemaserial.py b/server/test/unittest_schemaserial.py
@@ -131,11 +131,16 @@
347              ('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',
348               {'se': None, 'rt': None, 'oe': None,
349                'description': u'groups allowed to add entities/relations of this type', 'composite': None, 'ordernum': 9999, 'cardinality': u'**'}),
350              ('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',
351               {'se': None, 'rt': None, 'oe': None,
352 -              'description': u'rql expression allowing to add entities/relations of this type', 'composite': 'subject', 'ordernum': 9999, 'cardinality': u'*?'})],
353 +              'description': u'rql expression allowing to add entities/relations of this type', 'composite': 'subject', 'ordernum': 9999, 'cardinality': u'*?'}),
354 +            ('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',
355 +            {'cardinality': u'**', 'composite': None, 'description': u'groups allowed to add entities/relations of this type',
356 +             'oe': None, 'ordernum': 9999, 'rt': None, 'se': None}),
357 +            ('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',
358 +             {'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})],
359                               list(rschema2rql(schema.rschema('add_permission'), cstrtypemap)))
360 
361      def test_rschema2rql3(self):
362          self.assertListEqual([
363              ('INSERT CWRType X: X description %(description)s,X final %(final)s,X fulltext_container %(fulltext_container)s,X inlined %(inlined)s,X name %(name)s,X symmetric %(symmetric)s',
@@ -264,10 +269,11 @@
364 
365      def test_rperms2rql3(self):
366          self.assertListEqual([('SET X read_permission Y WHERE Y eid %(g)s, X eid %(x)s', {'g': 0}),
367                                ('SET X read_permission Y WHERE Y eid %(g)s, X eid %(x)s', {'g': 1}),
368                                ('SET X read_permission Y WHERE Y eid %(g)s, X eid %(x)s', {'g': 2}),
369 +                              ('SET X add_permission Y WHERE Y eid %(g)s, X eid %(x)s', {'g': 0}),
370                                ('SET X update_permission Y WHERE Y eid %(g)s, X eid %(x)s', {'g': 0})],
371                               [(rql, kwargs)
372                                for rql, kwargs in erperms2rql(schema.rschema('name').rdef('CWEType', 'String'),
373                                                               self.GROUP_MAPPING)])
374 
diff --git a/server/test/unittest_security.py b/server/test/unittest_security.py
@@ -411,10 +411,20 @@
375              self.assertRaises(Unauthorized, self.commit)
376              note2.cw_adapt_to('IWorkflowable').fire_transition('redoit')
377              self.commit()
378              cu.execute("SET X para 'chouette' WHERE X eid %(x)s", {'x': note2.eid})
379              self.commit()
380 +            cu.execute("INSERT Note X: X something 'A'")
381 +            self.assertRaises(Unauthorized, self.commit)
382 +            cu.execute("INSERT Note X: X para 'zogzog', X something 'A'")
383 +            self.commit()
384 +            note = cu.execute("INSERT Note X").get_entity(0,0)
385 +            self.commit()
386 +            note.cw_set(something=u'B')
387 +            self.commit()
388 +            note.cw_set(something=None, para=u'zogzog')
389 +            self.commit()
390 
391      def test_attribute_read_security(self):
392          # anon not allowed to see users'login, but they can see users
393          login_rdef = self.repo.schema['CWUser'].rdef('login')
394          with self.temporary_permissions((login_rdef, {'read': ('users', 'managers')}),