[FIX] core: searching parent_of on forbidden records

Before this commit, the implementation crashes when the ancestors of a
record contain non-accessible records.  This fixes the code to avoid it
to crash.

We also make the semantics of hierarchical searches more consistent in
this case: searching with operators 'child_of' and 'parent_of' returns
the subset of accessible records that satisfy the hierarchy operator.
We actually make it coincide with the results of the search when using
the "_parent_store" optimization.

closes odoo/odoo#73962

X-original-commit: 3e1b960acf3f6a726338476ba9e62e5ef5c84ef9
Signed-off-by: Raphael Collet (rco) <rco@openerp.com>

odoo/addons/base/tests/test_expression.py

@@ -197,6 +197,40 @@ class TestExpression(SavepointCaseWithUserDemo):
             cats = self._search(Category, [('id', 'parent_of', False)])
         self.assertEqual(len(cats), 0)
 
+    @mute_logger('odoo.models.unlink')
+    def test_10_hierarchy_access(self):
+        Partner = self.env['res.partner'].with_user(self.user_demo)
+        top = Partner.create({'name': 'Top'})
+        med = Partner.create({'name': 'Medium', 'parent_id': top.id})
+        bot = Partner.create({'name': 'Bottom', 'parent_id': med.id})
+
+        # restrict access of user Demo to partners Top and Bottom
+        accessible = top + bot
+        self.env['ir.rule'].search([]).unlink()
+        self.env['ir.rule'].create({
+            'name': 'partners rule',
+            'model_id': self.env['ir.model']._get('res.partner').id,
+            'domain_force': str([('id', 'in', accessible.ids)]),
+        })
+
+        # these searches should return the subset of accessible nodes that are
+        # in the given hierarchy
+        self.assertEqual(Partner.search([]), accessible)
+        self.assertEqual(Partner.search([('id', 'child_of', top.ids)]), accessible)
+        self.assertEqual(Partner.search([('id', 'parent_of', bot.ids)]), accessible)
+
+        # same kind of search from another model
+        Bank = self.env['res.partner.bank'].with_user(self.user_demo)
+        bank_top, _bank_med, bank_bot = Bank.create([
+            {'acc_number': '1', 'partner_id': top.id},
+            {'acc_number': '2', 'partner_id': med.id},
+            {'acc_number': '3', 'partner_id': bot.id},
+        ])
+
+        self.assertEqual(Bank.search([('partner_id', 'in', accessible.ids)]), bank_top + bank_bot)
+        self.assertEqual(Bank.search([('partner_id', 'child_of', top.ids)]), bank_top + bank_bot)
+        self.assertEqual(Bank.search([('partner_id', 'parent_of', bot.ids)]), bank_top + bank_bot)
+
     def test_10_eq_lt_gt_lte_gte(self):
         # test if less/greater than or equal operators work
         currency = self.env['res.currency'].search([], limit=1)

odoo/osv/expression.py

@@ -530,42 +530,52 @@ class expression(object):
             if not ids:
                 return [FALSE_LEAF]
             if left_model._parent_store:
-                doms = OR([
+                domain = OR([
                     [('parent_path', '=like', rec.parent_path + '%')]
                     for rec in left_model.browse(ids)
                 ])
-                if prefix:
-                    return [(left, 'in', left_model._search(doms, order='id'))]
-                return doms
             else:
+                # recursively retrieve all children nodes with sudo(); the
+                # filtering of forbidden records is done by the rest of the
+                # domain
                 parent_name = parent or left_model._parent_name
-                child_ids = set(ids)
-                while ids:
-                    ids = left_model._search([(parent_name, 'in', ids)], order='id')
-                    child_ids.update(ids)
-                return [(left, 'in', list(child_ids))]
+                child_ids = set()
+                records = left_model.sudo().browse(ids)
+                while records:
+                    child_ids.update(records._ids)
+                    records = records.search([(parent_name, 'in', records.ids)], order='id')
+                domain = [('id', 'in', list(child_ids))]
+            if prefix:
+                return [(left, 'in', left_model._search(domain, order='id'))]
+            return domain
 
         def parent_of_domain(left, ids, left_model, parent=None, prefix=''):
             """ Return a domain implementing the parent_of operator for [(left,parent_of,ids)],
                 either as a range using the parent_path tree lookup field
                 (when available), or as an expanded [(left,in,parent_ids)] """
+            if not ids:
+                return [FALSE_LEAF]
             if left_model._parent_store:
                 parent_ids = [
                     int(label)
                     for rec in left_model.browse(ids)
                     for label in rec.parent_path.split('/')[:-1]
                 ]
-                if prefix:
-                    return [(left, 'in', parent_ids)]
-                return [('id', 'in', parent_ids)]
+                domain = [('id', 'in', parent_ids)]
             else:
+                # recursively retrieve all parent nodes with sudo() to avoid
+                # access rights errors; the filtering of forbidden records is
+                # done by the rest of the domain
                 parent_name = parent or left_model._parent_name
                 parent_ids = set()
-                for record in left_model.browse(ids):
-                    while record:
-                        parent_ids.add(record.id)
-                        record = record[parent_name]
-                return [(left, 'in', list(parent_ids))]
+                records = left_model.sudo().browse(ids)
+                while records:
+                    parent_ids.update(records._ids)
+                    records = records[parent_name]
+                domain = [('id', 'in', list(parent_ids))]
+            if prefix:
+                return [(left, 'in', left_model._search(domain, order='id'))]
+            return domain
 
         HIERARCHY_FUNCS = {'child_of': child_of_domain,
                            'parent_of': parent_of_domain}