[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.
--- 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
--- 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)
--- 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)
--- 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)
--- 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'},
--- 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
--- 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