# HG changeset patch # User Aurelien Campeas # Date 1390919279 -3600 # Node ID 7099bbd685aaba8ef731c0d4ed0ba147ff29da39 # Parent 91fbd3111828e9885f5c3c7f9cf16dd863c6c784 [hooks/security] allow edition of attributes with permissive permissions If an attribute has more permissive security rules than the entity type itself, we should be green and not deny action because of an early global entity permission check (with the more restrictive rules). Only if one attribute with the entity-level permission rules is edited will the global check be performed. Note: * the "if action == 'delete'" check at the entry of check_entity_attributes is a guard for a condition currently not happening in cubicweb itself (but application hooks could conceivably call this function with a 'delete' action) Closes #3489895. diff -r 91fbd3111828 -r 7099bbd685aa doc/book/en/devrepo/datamodel/definition.rst --- a/doc/book/en/devrepo/datamodel/definition.rst Tue Jul 29 14:40:29 2014 +0200 +++ b/doc/book/en/devrepo/datamodel/definition.rst Tue Jan 28 15:27:59 2014 +0100 @@ -300,7 +300,7 @@ * users and groups of users * a user belongs to at least one group of user -* permissions (read, update, create, delete) +* permissions (`read`, `update`, `create`, `delete`) * permissions are assigned to groups (and not to users) For *CubicWeb* in particular: @@ -320,10 +320,10 @@ * the permissions of this group are only checked on `update`/`delete` actions if all the other groups the user belongs to do not provide those permissions -Setting permissions is done with the attribute `__permissions__` of entities and -relation definition. The value of this attribute is a dictionary where the keys -are the access types (action), and the values are the authorized groups or -expressions. +Setting permissions is done with the class attribute `__permissions__` +of entity types and relation definitions. The value of this attribute +is a dictionary where the keys are the access types (action), and the +values are the authorized groups or rql expressions. For an entity type, the possible actions are `read`, `add`, `update` and `delete`. @@ -333,6 +333,19 @@ For an attribute, the possible actions are `read`, `add` and `update`, and they are a refinement of an entity type permission. +.. note:: + + By default, the permissions of an entity type attributes are + equivalent to the permissions of the entity type itself. + + It is possible to provide custom attribute permissions which are + stronger than, or are more lenient than the entity type + permissions. + + In a situation where all attributes were given custom permissions, + the entity type permissions would not be checked, except for the + `delete` action. + 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 provided if the user is in one of the listed groups or if one of the RQL condition @@ -368,6 +381,13 @@ 'add': ('managers', ERQLExpression('U has_add_permission X'), 'update': ('managers', ERQLExpression('U has_update_permission X')),} +.. note:: + + The default permissions for attributes are not syntactically + equivalent to the default permissions of the entity types, but the + rql expressions work by delegating to the entity type permissions. + + The standard user groups ```````````````````````` @@ -670,7 +690,7 @@ RelationType declaration which offers some advantages in the context of reusable cubes. - + Handling schema changes diff -r 91fbd3111828 -r 7099bbd685aa hooks/security.py --- a/hooks/security.py Tue Jul 29 14:40:29 2014 +0200 +++ b/hooks/security.py Tue Jan 28 15:27:59 2014 +0100 @@ -1,4 +1,4 @@ -# copyright 2003-2013 LOGILAB S.A. (Paris, FRANCE), all rights reserved. +# copyright 2003-2014 LOGILAB S.A. (Paris, FRANCE), all rights reserved. # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr # # This file is part of CubicWeb. @@ -34,11 +34,15 @@ def check_entity_attributes(session, entity, action, editedattrs=None): eid = entity.eid eschema = entity.e_schema + if action == 'delete': + eschema.check_perm(session, action, eid=eid) + return # ._cw_skip_security_attributes is there to bypass security for attributes # set by hooks by modifying the entity's dictionary if editedattrs is None: editedattrs = entity.cw_edited dontcheck = editedattrs.skip_security + etypechecked = False for attr in editedattrs: if attr in dontcheck: continue @@ -54,10 +58,10 @@ # 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. + # rule. This needs to be checked only once. + if not etypechecked: + entity.cw_check_perm(action) + etypechecked = True continue if perms == (): # That means an immutable attribute; as an optimization, avoid @@ -71,7 +75,6 @@ session = self.session for eid, action, edited in self.get_data(): entity = session.entity_from_eid(eid) - entity.cw_check_perm(action) check_entity_attributes(session, entity, action, edited) diff -r 91fbd3111828 -r 7099bbd685aa server/test/data/migratedapp/schema.py --- a/server/test/data/migratedapp/schema.py Tue Jul 29 14:40:29 2014 +0200 +++ b/server/test/data/migratedapp/schema.py Tue Jan 28 15:27:59 2014 +0100 @@ -91,6 +91,22 @@ attachment = SubjectRelation('File') +class Frozable(EntityType): + __permissions__ = { + 'read': ('managers', 'users'), + 'add': ('managers', 'users'), + 'update': ('managers', ERQLExpression('X frozen False'),), + 'delete': ('managers', ERQLExpression('X frozen False'),) + } + name = String() + frozen = Boolean(default=False, + __permissions__ = { + 'read': ('managers', 'users'), + 'add': ('managers', 'users'), + 'update': ('managers', 'owners') + }) + + class Personne(EntityType): __unique_together__ = [('nom', 'prenom', 'datenaiss')] nom = String(fulltextindexed=True, required=True, maxsize=64) diff -r 91fbd3111828 -r 7099bbd685aa server/test/data/schema.py --- a/server/test/data/schema.py Tue Jul 29 14:40:29 2014 +0200 +++ b/server/test/data/schema.py Tue Jan 28 15:27:59 2014 +0100 @@ -1,4 +1,4 @@ -# copyright 2003-2013 LOGILAB S.A. (Paris, FRANCE), all rights reserved. +# copyright 2003-2014 LOGILAB S.A. (Paris, FRANCE), all rights reserved. # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr # # This file is part of CubicWeb. @@ -110,6 +110,23 @@ 'S,Y')]) todo_by = SubjectRelation('CWUser') + +class Frozable(EntityType): + __permissions__ = { + 'read': ('managers', 'users'), + 'add': ('managers', 'users'), + 'update': ('managers', ERQLExpression('X frozen False'),), + 'delete': ('managers', ERQLExpression('X frozen False'),) + } + name = String() + frozen = Boolean(default=False, + __permissions__ = { + 'read': ('managers', 'users'), + 'add': ('managers', 'users'), + 'update': ('managers', 'owners') + }) + + class Personne(EntityType): __unique_together__ = [('nom', 'prenom', 'inline2')] nom = String(fulltextindexed=True, required=True, maxsize=64) diff -r 91fbd3111828 -r 7099bbd685aa server/test/unittest_msplanner.py --- a/server/test/unittest_msplanner.py Tue Jul 29 14:40:29 2014 +0200 +++ b/server/test/unittest_msplanner.py Tue Jan 28 15:27:59 2014 +0100 @@ -70,7 +70,7 @@ {'X': 'Card'}, {'X': 'Comment'}, {'X': 'Division'}, {'X': 'Email'}, {'X': 'EmailAddress'}, {'X': 'EmailPart'}, {'X': 'EmailThread'}, {'X': 'ExternalUri'}, {'X': 'File'}, - {'X': 'Folder'}, {'X': 'Note'}, {'X': 'Old'}, + {'X': 'Folder'}, {'X': 'Frozable'}, {'X': 'Note'}, {'X': 'Old'}, {'X': 'Personne'}, {'X': 'RQLExpression'}, {'X': 'Societe'}, {'X': 'State'}, {'X': 'SubDivision'}, {'X': 'SubWorkflowExitPoint'}, {'X': 'Tag'}, {'X': 'TrInfo'}, {'X': 'Transition'}, @@ -902,6 +902,7 @@ ALL_SOLS.remove({'X': 'CWSourceHostConfig'}) # not authorized ALL_SOLS.remove({'X': 'CWSourceSchemaConfig'}) # not authorized ALL_SOLS.remove({'X': 'CWDataImport'}) # not authorized + ALL_SOLS.remove({'X': 'Frozable'}) # not authorized self._test('Any MAX(X)', [('FetchStep', [('Any E WHERE E type "X", E is Note', [{'E': 'Note'}])], [self.cards, self.system], None, {'E': 'table1.C0'}, []), @@ -959,7 +960,8 @@ ueid = self.session.user.eid X_ET_ALL_SOLS = [] for s in X_ALL_SOLS: - if s in ({'X': 'CWSourceHostConfig'}, {'X': 'CWSourceSchemaConfig'}, {'X': 'CWDataImport'}): + if s in ({'X': 'CWSourceHostConfig'}, {'X': 'CWSourceSchemaConfig'}, + {'X': 'CWDataImport'}, {'X': 'Frozable'}): continue # not authorized ets = {'ET': 'CWEType'} ets.update(s) @@ -2676,7 +2678,7 @@ None, {'X': 'table0.C0'}, []), ('UnionStep', None, None, [('OneFetchStep', - [(u'Any X WHERE X owned_by U, U login "anon", U is CWUser, X is IN(Affaire, BaseTransition, Basket, Bookmark, CWAttribute, CWCache, CWConstraint, CWConstraintType, CWDataImport, CWEType, CWGroup, CWPermission, CWProperty, CWRType, CWRelation, CWSource, CWSourceHostConfig, CWSourceSchemaConfig, CWUniqueTogetherConstraint, CWUser, Division, Email, EmailAddress, EmailPart, EmailThread, ExternalUri, File, Folder, Old, Personne, RQLExpression, Societe, SubDivision, SubWorkflowExitPoint, Tag, TrInfo, Transition, Workflow, WorkflowTransition)', + [(u'Any X WHERE X owned_by U, U login "anon", U is CWUser, X is IN(Affaire, BaseTransition, Basket, Bookmark, CWAttribute, CWCache, CWConstraint, CWConstraintType, CWDataImport, CWEType, CWGroup, CWPermission, CWProperty, CWRType, CWRelation, CWSource, CWSourceHostConfig, CWSourceSchemaConfig, CWUniqueTogetherConstraint, CWUser, Division, Email, EmailAddress, EmailPart, EmailThread, ExternalUri, File, Folder, Frozable, Old, Personne, RQLExpression, Societe, SubDivision, SubWorkflowExitPoint, Tag, TrInfo, Transition, Workflow, WorkflowTransition)', [{'U': 'CWUser', 'X': 'Affaire'}, {'U': 'CWUser', 'X': 'BaseTransition'}, {'U': 'CWUser', 'X': 'Basket'}, @@ -2705,6 +2707,7 @@ {'U': 'CWUser', 'X': 'ExternalUri'}, {'U': 'CWUser', 'X': 'File'}, {'U': 'CWUser', 'X': 'Folder'}, + {'U': 'CWUser', 'X': 'Frozable'}, {'U': 'CWUser', 'X': 'Old'}, {'U': 'CWUser', 'X': 'Personne'}, {'U': 'CWUser', 'X': 'RQLExpression'}, diff -r 91fbd3111828 -r 7099bbd685aa server/test/unittest_querier.py --- a/server/test/unittest_querier.py Tue Jul 29 14:40:29 2014 +0200 +++ b/server/test/unittest_querier.py Tue Jan 28 15:27:59 2014 +0100 @@ -170,7 +170,7 @@ 'X': 'Affaire', 'ET': 'CWEType', 'ETN': 'String'}]) rql, solutions = partrqls[1] - self.assertEqual(rql, 'Any ETN,X WHERE X is ET, ET name ETN, ET is CWEType, X is IN(BaseTransition, Bookmark, CWAttribute, CWCache, CWConstraint, CWConstraintType, CWEType, CWGroup, CWPermission, CWProperty, CWRType, CWRelation, CWSource, CWUniqueTogetherConstraint, CWUser, Card, Comment, Division, Email, EmailPart, EmailThread, ExternalUri, File, Folder, Note, Old, Personne, RQLExpression, Societe, State, SubDivision, SubWorkflowExitPoint, Tag, TrInfo, Transition, Workflow, WorkflowTransition)') + self.assertEqual(rql, 'Any ETN,X WHERE X is ET, ET name ETN, ET is CWEType, X is IN(BaseTransition, Bookmark, CWAttribute, CWCache, CWConstraint, CWConstraintType, CWEType, CWGroup, CWPermission, CWProperty, CWRType, CWRelation, CWSource, CWUniqueTogetherConstraint, CWUser, Card, Comment, Division, Email, EmailPart, EmailThread, ExternalUri, File, Folder, Frozable, Note, Old, Personne, RQLExpression, Societe, State, SubDivision, SubWorkflowExitPoint, Tag, TrInfo, Transition, Workflow, WorkflowTransition)') self.assertListEqual(sorted(solutions), sorted([{'X': 'BaseTransition', 'ETN': 'String', 'ET': 'CWEType'}, {'X': 'Bookmark', 'ETN': 'String', 'ET': 'CWEType'}, @@ -196,6 +196,7 @@ {'X': 'ExternalUri', 'ETN': 'String', 'ET': 'CWEType'}, {'X': 'File', 'ETN': 'String', 'ET': 'CWEType'}, {'X': 'Folder', 'ETN': 'String', 'ET': 'CWEType'}, + {'X': 'Frozable', 'ETN': 'String', 'ET': 'CWEType'}, {'X': 'Note', 'ETN': 'String', 'ET': 'CWEType'}, {'X': 'Old', 'ETN': 'String', 'ET': 'CWEType'}, {'X': 'Personne', 'ETN': 'String', 'ET': 'CWEType'}, @@ -576,16 +577,16 @@ self.assertListEqual(rset.rows, [[u'description_format', 12], [u'description', 13], - [u'name', 17], - [u'created_by', 43], - [u'creation_date', 43], - [u'cw_source', 43], - [u'cwuri', 43], - [u'in_basket', 43], - [u'is', 43], - [u'is_instance_of', 43], - [u'modification_date', 43], - [u'owned_by', 43]]) + [u'name', 18], + [u'created_by', 44], + [u'creation_date', 44], + [u'cw_source', 44], + [u'cwuri', 44], + [u'in_basket', 44], + [u'is', 44], + [u'is_instance_of', 44], + [u'modification_date', 44], + [u'owned_by', 44]]) def test_select_aggregat_having_dumb(self): # dumb but should not raise an error diff -r 91fbd3111828 -r 7099bbd685aa server/test/unittest_security.py --- a/server/test/unittest_security.py Tue Jul 29 14:40:29 2014 +0200 +++ b/server/test/unittest_security.py Tue Jan 28 15:27:59 2014 +0100 @@ -1,4 +1,4 @@ -# copyright 2003-2012 LOGILAB S.A. (Paris, FRANCE), all rights reserved. +# copyright 2003-2014 LOGILAB S.A. (Paris, FRANCE), all rights reserved. # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr # # This file is part of CubicWeb. @@ -390,6 +390,22 @@ self.assertRaises(Unauthorized, self.commit) cu.execute('SET X web "http://www.logilab.org" WHERE X eid %(x)s', {'x': eid}) self.commit() + with self.login('iaminusersgrouponly') as cu: + eid = cu.execute('INSERT Frozable F: F name "Foo"') + self.commit() + cu.execute('SET F name "Bar" WHERE F is Frozable') + self.commit() + cu.execute('SET F name "BaBar" WHERE F is Frozable') + cu.execute('SET F frozen True WHERE F is Frozable') + with self.assertRaises(Unauthorized): + self.commit() + self.rollback() + cu.execute('SET F frozen True WHERE F is Frozable') + self.commit() + cu.execute('SET F name "Bar" WHERE F is Frozable') + with self.assertRaises(Unauthorized): + self.commit() + self.rollback() def test_attribute_security_rqlexpr(self): # Note.para attribute editable by managers or if the note is in "todo" state