From 1a3ee131fe49121f183b40a81326ff106324d260 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Krier?= <ced@b2ck.com>
Date: Mon, 12 Nov 2018 17:18:06 +0100
Subject: [PATCH] Check read access on field in search domain

issue7766
review50591002
---
 CHANGELOG                     |  1 +
 trytond/model/modelsql.py     | 12 ++++------
 trytond/model/modelstorage.py | 43 +++++++++++++++++++++++++++++++----
 3 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 17cd1cbb83..0723167d37 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,4 @@
+* Check read access on field in search domain (issue7766)
 * Add active field on ModelAccess, ModelFieldAccess and Group
 * Use write mode by default to check create and delete of resources
 
diff --git a/trytond/model/modelsql.py b/trytond/model/modelsql.py
index cea3b6ab93..6fd3643adc 100644
--- a/trytond/model/modelsql.py
+++ b/trytond/model/modelsql.py
@@ -16,7 +16,6 @@ from trytond.model import ModelStorage, ModelView
 from trytond.model import fields
 from trytond import backend
 from trytond.tools import reduce_ids, grouped_slice, cursor_dict
-from trytond.const import OPERATORS
 from trytond.transaction import Transaction
 from trytond.pool import Pool
 from trytond.cache import LRUDict
@@ -24,7 +23,7 @@ from trytond.exceptions import ConcurrencyException
 from trytond.rpc import RPC
 from trytond.config import config
 
-from .modelstorage import cache_size
+from .modelstorage import cache_size, is_leaf
 
 
 class Constraint(object):
@@ -1166,6 +1165,9 @@ class ModelSQL(ModelStorage):
         transaction = Transaction()
         cursor = transaction.connection.cursor()
 
+        super(ModelSQL, cls).search(
+            domain, offset=offset, limit=limit, order=order, count=count)
+
         # Get domain clauses
         tables, expression = cls.search_domain(domain)
 
@@ -1335,12 +1337,6 @@ class ModelSQL(ModelStorage):
             else:
                 tables[None] = (cls.__table__(), None)
 
-        def is_leaf(expression):
-            return (isinstance(expression, (list, tuple))
-                and len(expression) > 2
-                and isinstance(expression[1], str)
-                and expression[1] in OPERATORS)  # TODO remove OPERATORS test
-
         def convert(domain):
             if is_leaf(domain):
                 fname = domain[0].split('.', 1)[0]
diff --git a/trytond/model/modelstorage.py b/trytond/model/modelstorage.py
index 4f666a493c..797d2c89dd 100644
--- a/trytond/model/modelstorage.py
+++ b/trytond/model/modelstorage.py
@@ -42,6 +42,13 @@ def cache_size():
         config.getint('cache', 'record'))
 
 
+def is_leaf(expression):
+    return (isinstance(expression, (list, tuple))
+        and len(expression) > 2
+        and isinstance(expression[1], str)
+        and expression[1] in OPERATORS)  # TODO remove OPERATORS test
+
+
 class ModelStorage(Model):
     """
     Define a model with storage capability in Tryton.
@@ -381,6 +388,36 @@ class ModelStorage(Model):
         '''
         Return a list of records that match the domain.
         '''
+        pool = Pool()
+        transaction = Transaction()
+        ModelAccess = pool.get('ir.model.access')
+        ModelFieldAccess = pool.get('ir.model.field.access')
+
+        ModelAccess.check(cls.__name__, 'read')
+
+        def check(domain, cls, to_check):
+            if is_leaf(domain):
+                local, relate = (domain[0].split('.', 1) + [None])[:2]
+                to_check[cls.__name__].add(local)
+                if relate:
+                    if len(domain) >= 4:
+                        target = pool.get(domain[3])
+                    else:
+                        target = cls._fields[local].get_target()
+                    target_domain = [(relate,) + tuple(domain[1:])]
+                    check(target_domain, target, to_check)
+            elif not domain:
+                return
+            else:
+                i = 1 if domain[0] in ['OR', 'AND'] else 0
+                for d in domain[i:]:
+                    check(d, cls, to_check)
+        if transaction.user and transaction.context.get('_check_access'):
+            to_check = defaultdict(set)
+            check(domain, cls, to_check)
+            for name, fields_names in to_check.items():
+                ModelAccess.check(name, 'read')
+                ModelFieldAccess.check(name, fields_names, 'read')
         if count:
             return 0
         return []
@@ -430,10 +467,7 @@ class ModelStorage(Model):
             while i < len(domain):
                 arg = domain[i]
                 # add test for xmlrpc that doesn't handle tuple
-                if (isinstance(arg, tuple)
-                        or (isinstance(arg, list)
-                            and len(arg) > 2
-                            and arg[1] in OPERATORS)):
+                if is_leaf(arg):
                     if arg[0] == 'active':
                         active_found = True
                 elif isinstance(arg, list):
@@ -856,6 +890,7 @@ class ModelStorage(Model):
         pass
 
     @classmethod
+    @without_check_access
     def _validate(cls, records, field_names=None):
         pool = Pool()
         # Ensure that records are readable