Commit 23b4ad68 authored by Aurelien Campeas's avatar Aurelien Campeas
Browse files

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

--HG--
branch : stable
parent 91fbd3111828
......@@ -300,7 +300,7 @@ The main principles are:
* 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 @@ For *CubicWeb* in particular:
* 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 a relation, the possible actions are `read`, `add`, and `delete`.
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 @@ The default permissions for attributes are:
'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 @@ the entity type for the object of the relation.
RelationType declaration which offers some advantages in the context
of reusable cubes.
Handling schema changes
......
# 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 @@ from cubicweb.server import BEFORE_ADD_RELATIONS, ON_COMMIT_ADD_RELATIONS, hook
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 @@ def check_entity_attributes(session, entity, action, editedattrs=None):
# 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 @@ class CheckEntityPermissionOp(hook.DataOperationMixIn, hook.LateOperation):
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)
......
......@@ -91,6 +91,22 @@ class Note(Para):
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)
......
# 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 @@ class Note(WorkflowableEntityType):
'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)
......
......@@ -70,7 +70,7 @@ X_ALL_SOLS = sorted([{'X': 'Affaire'}, {'X': 'BaseTransition'}, {'X': 'Basket'},
{'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 @@ class MSPlannerTC(BaseMSPlannerTC):
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 @@ class MSPlannerTC(BaseMSPlannerTC):
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 @@ class MSPlannerTwoSameExternalSourcesTC(BasePlannerTC):
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 @@ class MSPlannerTwoSameExternalSourcesTC(BasePlannerTC):
{'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'},
......
......@@ -170,7 +170,7 @@ class UtilsTC(BaseQuerierTC):
'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 @@ class UtilsTC(BaseQuerierTC):
{'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 @@ class QuerierTC(BaseQuerierTC):
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
......
# 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 @@ class SecurityTC(BaseSecurityTC):
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
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment