From cd3790c4ea19efd6800284a6471158b9c75b29ca Mon Sep 17 00:00:00 2001
From: Raphael Collet <rco@odoo.com>
Date: Fri, 18 Jan 2019 12:56:01 +0000
Subject: [PATCH] [FIX] models: check groups on inverse fields

Before this, groups on non-stored inverse fields were not checked upon write.
The impact on existing fields is pretty small, since the inverse methods of
those fields are subject to access rights on the records they use.

closes odoo/odoo#30356
---
 .../test_new_api/tests/test_new_fields.py     | 30 +++++++++++++++++++
 odoo/models.py                                |  7 ++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/odoo/addons/test_new_api/tests/test_new_fields.py b/odoo/addons/test_new_api/tests/test_new_fields.py
index a29f5d38292..ea0b8cdd69c 100644
--- a/odoo/addons/test_new_api/tests/test_new_fields.py
+++ b/odoo/addons/test_new_api/tests/test_new_fields.py
@@ -276,6 +276,16 @@ class TestFields(common.TransactionCase):
         self.assertEqual(record.bar, 'Ho')
         self.assertEqual(record.counts, {'compute': 0, 'inverse': 1})
 
+    def test_13_inverse_access(self):
+        """ test access rights on inverse fields """
+        foo = self.env['test_new_api.category'].create({'name': 'Foo'})
+        user = self.env['res.users'].create({'name': 'Foo', 'login': 'foo'})
+        self.assertFalse(user.has_group('base.group_system'))
+        # add group on non-stored inverse field
+        self.patch(type(foo).display_name, 'groups', 'base.group_system')
+        with self.assertRaises(AccessError):
+            foo.sudo(user).display_name = 'Forbidden'
+
     def test_14_search(self):
         """ test search on computed fields """
         discussion = self.env.ref('test_new_api.discussion_0')
@@ -577,6 +587,26 @@ class TestFields(common.TransactionCase):
         self.assertEqual(attribute_record.bar, 'DEFDEF')
         self.assertFalse(self.env.has_todo())
 
+        # add group on company-dependent field
+        self.assertFalse(user0.has_group('base.group_system'))
+        self.patch(type(record).foo, 'groups', 'base.group_system')
+        with self.assertRaises(AccessError):
+            record.sudo(user0).foo = 'forbidden'
+
+        user0.write({'groups_id': [(4, self.env.ref('base.group_system').id)]})
+        record.sudo(user0).foo = 'yes we can'
+
+        # add ir.rule to prevent access on record
+        self.assertTrue(user0.has_group('base.group_user'))
+        model_id = self.env['ir.model'].search([('model', '=', record._name)]).id
+        rule = self.env['ir.rule'].create({
+            'model_id': model_id,
+            'groups': [self.env.ref('base.group_user').id],
+            'domain_force': str([('id', '!=', record.id)]),
+        })
+        with self.assertRaises(AccessError):
+            record.sudo(user0).foo = 'forbidden'
+
     def test_28_sparse(self):
         """ test sparse fields. """
         record = self.env['test_new_api.sparse'].create({})
diff --git a/odoo/models.py b/odoo/models.py
index af3453a8b38..5b42a6e64c7 100644
--- a/odoo/models.py
+++ b/odoo/models.py
@@ -3568,10 +3568,15 @@ class BaseModel(object):
         protected_fields = map(self._fields.get, new_vals)
         with self.env.protecting(protected_fields, self):
             # write old-style fields with (low-level) method _write
-            if old_vals:
+            if old_vals or new_vals:
+                # if log_access is enabled, this updates 'write_date' and
+                # 'write_uid' and check access rules, even when old_vals is
+                # empty
                 self._write(old_vals)
 
             if new_vals:
+                self.check_field_access_rights('write', list(new_vals))
+
                 self.modified(set(new_vals) - set(old_vals))
 
                 # put the values of fields into cache, and inverse them