[rql security] fix rql bug when using yams inheritance and read permissions (closes #2410156) stable
authorFlorent Cayré <florent.cayre@logilab.fr>
Wed, 04 Jul 2012 17:56:46 +0200
branchstable
changeset 8452 1ad42383a9ec
parent 8451 49e965bba1ec
child 8453 f441056a2b61
[rql security] fix rql bug when using yams inheritance and read permissions (closes #2410156) problems occurings when querying using is_instance_of while a subtype has some permissions not shared by the parent type
rqlrewrite.py
server/test/unittest_security.py
--- a/rqlrewrite.py	Fri Jul 06 09:01:42 2012 +0200
+++ b/rqlrewrite.py	Wed Jul 04 17:56:46 2012 +0200
@@ -77,12 +77,26 @@
                 mytyperel = None
         possibletypes = allpossibletypes[varname]
         if mytyperel is not None:
-            # variable as already some types restriction. new possible types
-            # can only be a subset of existing ones, so only remove no more
-            # possible types
-            for cst in mytyperel.get_nodes(n.Constant):
-                if not cst.value in possibletypes:
-                    cst.parent.remove(cst)
+            if mytyperel.r_type == 'is_instance_of':
+                # turn is_instance_of relation into a is relation since we've
+                # all possible solutions and don't want to bother with
+                # potential is_instance_of incompatibility
+                mytyperel.r_type = 'is'
+                if len(possibletypes) > 1:
+                    node = n.Function('IN')
+                    for etype in possibletypes:
+                        node.append(n.Constant(etype, 'etype'))
+                else:
+                    node = n.Constant(etype, 'etype')
+                comp = mytyperel.children[1]
+                comp.replace(comp.children[0], node)
+            else:
+                # variable has already some strict types restriction. new
+                # possible types can only be a subset of existing ones, so only
+                # remove no more possible types
+                for cst in mytyperel.get_nodes(n.Constant):
+                    if not cst.value in possibletypes:
+                        cst.parent.remove(cst)
         else:
             # we have to add types restriction
             if stinfo.get('scope') is not None:
--- a/server/test/unittest_security.py	Fri Jul 06 09:01:42 2012 +0200
+++ b/server/test/unittest_security.py	Wed Jul 04 17:56:46 2012 +0200
@@ -1,4 +1,4 @@
-# copyright 2003-2010 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# copyright 2003-2012 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
 # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
 #
 # This file is part of CubicWeb.
@@ -23,8 +23,10 @@
 from cubicweb.devtools.testlib import CubicWebTC
 
 from cubicweb import Unauthorized, ValidationError, QueryError
+from cubicweb.schema import ERQLExpression
 from cubicweb.server.querier import check_read_access
 
+
 class BaseSecurityTC(CubicWebTC):
 
     def setup_database(self):
@@ -468,6 +470,28 @@
         cnx.rollback()
         cnx.close()
 
+    def test_yams_inheritance_and_security_bug(self):
+        oldperms = self.schema['Division'].permissions
+        try:
+            self.schema['Division'].permissions = {
+                'read': ('managers', ERQLExpression('X owned_by U')),
+                'add': ('managers', 'users'),
+                'update': ('managers', 'owners'),
+                'delete': ('managers', 'owners')}
+            self.login('iaminusersgrouponly')
+            querier = self.repo.querier
+            rqlst = querier.parse('Any X WHERE X is_instance_of Societe')
+            querier.solutions(self.session, rqlst, {})
+            querier._annotate(rqlst)
+            plan = querier.plan_factory(rqlst, {}, self.session)
+            plan.preprocess(rqlst)
+            self.assertEqual(
+                rqlst.as_string(),
+                '(Any X WHERE X is IN(SubDivision, Societe)) UNION (Any X WHERE X is Division, EXISTS(X owned_by %(B)s))')
+        finally:
+            self.schema['Division'].permissions = oldperms
+
+
 class BaseSchemaSecurityTC(BaseSecurityTC):
     """tests related to the base schema permission configuration"""