[FIX] orm: access error due to prefetch of indirectly referenced records.

The new-api record prefetching algorithm attempts
to load data for all known records from the requested
model (i.e. all IDs present in the environment cache),
regardless of how indirectly/remotely they were
referenced. An indirect parent record may therefore
be prefetched along with its directly browsed children,
possibly crossing company boundaries involuntarily.

This patch implements a fallback mechanism when
the prefetching failed due to what looks like an
ACL restriction.

The implementation of `_read_from_database` handle
ACL directly and set an `AccessError` as cache value
for restricted records.

If a model (like `mail.message`) overwrites `read` to
implements its own ACL checks and raises an `AccessError`
before calling `super()` (which will then call
`_read_from_database`), the cache will be not fill,
leading to an unexpected exception.

If this commit messae looks familiar to you, that's
simply because this is the new-api counterpart of
b7865502e4

openerp/addons/test_new_api/models.py

@@ -19,6 +19,7 @@
 #
 ##############################################################################
 
+from openerp.exceptions import AccessError
 from openerp.osv import osv, fields
 
 class res_partner(osv.Model):
@@ -49,6 +50,8 @@ class Category(models.Model):
     name = fields.Char(required=True)
     parent = fields.Many2one('test_new_api.category')
     display_name = fields.Char(compute='_compute_display_name', inverse='_inverse_display_name')
+    discussions = fields.Many2many('test_new_api.discussion', 'test_new_api_discussion_category',
+                                   'category', 'discussion')
 
     @api.one
     @api.depends('name', 'parent.display_name')     # this definition is recursive
@@ -74,6 +77,10 @@ class Category(models.Model):
         # assign name of last category, and reassign display_name (to normalize it)
         self.name = names[-1].strip()
 
+    def read(self, fields=None, load='_classic_read'):
+        if self.search_count([('id', 'in', self._ids), ('name', '=', 'NOACCESS')]):
+            raise AccessError('Sorry')
+        return super(Category, self).read(fields, load)
 
 class Discussion(models.Model):
     _name = 'test_new_api.discussion'

openerp/addons/test_new_api/tests/test_new_fields.py

@@ -4,6 +4,7 @@
 from datetime import date, datetime
 from collections import defaultdict
 
+from openerp.exceptions import AccessError
 from openerp.tests import common
 from openerp.exceptions import except_orm
 
@@ -339,6 +340,24 @@ class TestNewFields(common.TransactionCase):
             self.assertEqual(data['display_name'], display_name)
             self.assertEqual(data['size'], size)
 
+    def test_31_prefetch(self):
+        """ test prefetch of records handle AccessError """
+        Category = self.env['test_new_api.category']
+        cat_1 = Category.create({'name': 'NOACCESS'}).id
+        cat_2 = Category.create({'name': 'ACCESS', 'parent': cat_1}).id
+
+        self.env.clear()
+
+        cat = Category.browse(cat_2)
+        self.assertEqual(cat.name, 'ACCESS')
+        # both categories should be in prefetch ids
+        self.assertSetEqual(self.env.prefetch[Category._name], set([cat_1, cat_2]))
+        # but due to our (lame) overwrite of `read`, it should not forbid us to read records we have access to
+        self.assertFalse(len(cat.discussions))
+        self.assertEqual(cat.parent.id, cat_1)
+        with self.assertRaises(AccessError):
+            Category.browse(cat_1).name
+
     def test_40_new(self):
         """ test new records. """
         discussion = self.env.ref('test_new_api.discussion_0')

openerp/models.py

@@ -3230,7 +3230,8 @@ class BaseModel(object):
         try:
             result = records.read(list(fnames), load='_classic_write')
         except AccessError:
-            pass
+            # not all records may be accessible, try with only current record
+            result = self.read(list(fnames), load='_classic_write')
 
         # check the cache, and update it if necessary
         if not self._cache.contains(field):