From 2b3fc2e3b80f191b4cd126abccf6f94b1302ac11 Mon Sep 17 00:00:00 2001 From: Nicolas Lempereur <nle@odoo.com> Date: Thu, 12 Dec 2019 17:26:16 +0000 Subject: [PATCH] [FIX] base: attachment check linked record 'write' When an attachment is linked to a record (res_id and res_model are set) we check the access rights and access rules of that record. The access we check on linked record has changed as follow: - 15905e78 (2013) => we check `write` access right/rule for `create` - f5ebc509 (2014) => we check `write` access right for all mode - 66644e85 (2015) => we check write access right for all but create mode So currently we check on the linked record for each mode: - create: write access right / write access rule - read: read access right / read access rule - write: write access right / write access rule - unlink: write access right / unlink access rule The behavior is not expected for `unlink`, we should check if we have write access through access rules instead of checking unlink access. Without the change, the added test failed with a `unlink` access rule AccessError on the linked record. opw-2154448 closes #41814 closes odoo/odoo#42236 X-original-commit: 2cadce5628691aef46906be9de9cfec93ac448a2 Signed-off-by: Nicolas Lempereur (nle) <nle@odoo.com> --- odoo/addons/base/models/ir_attachment.py | 11 +++--- odoo/addons/base/tests/test_ir_attachment.py | 37 ++++++++++++++++++++ 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/odoo/addons/base/models/ir_attachment.py b/odoo/addons/base/models/ir_attachment.py index ef2d7cc1333c..597a0fe3c933 100644 --- a/odoo/addons/base/models/ir_attachment.py +++ b/odoo/addons/base/models/ir_attachment.py @@ -372,8 +372,9 @@ class IrAttachment(models.Model): require_employee = True # For related models, check if we can write to the model, as unlinking # and creating attachments can be seen as an update to the model - records.check_access_rights('write' if mode in ('create', 'unlink') else mode) - records.check_access_rule(mode) + access_mode = 'write' if mode in ('create', 'unlink') else mode + records.check_access_rights(access_mode) + records.check_access_rule(access_mode) if require_employee: if not (self.env.is_admin() or self.env.user.has_group('base.group_user')): @@ -469,9 +470,9 @@ class IrAttachment(models.Model): return len(result) if count else list(result) - def read(self, fields=None, load='_classic_read'): + def _read(self, fields): self.check('read') - return super(IrAttachment, self).read(fields, load=load) + return super(IrAttachment, self)._read(fields) def write(self, vals): self.check('write', values=vals) @@ -519,7 +520,7 @@ class IrAttachment(models.Model): record_tuple_set.add(record_tuple) for record_tuple in record_tuple_set: (res_model, res_id) = record_tuple - self.check('write', values={'res_model':res_model, 'res_id':res_id}) + self.check('create', values={'res_model':res_model, 'res_id':res_id}) return super(IrAttachment, self).create(vals_list) def _post_add_create(self): diff --git a/odoo/addons/base/tests/test_ir_attachment.py b/odoo/addons/base/tests/test_ir_attachment.py index 8c3769b11780..122995fe0946 100644 --- a/odoo/addons/base/tests/test_ir_attachment.py +++ b/odoo/addons/base/tests/test_ir_attachment.py @@ -4,6 +4,7 @@ import base64 import hashlib import os +from odoo.exceptions import AccessError from odoo.tests.common import TransactionCase HASH_SPLIT = 2 # FIXME: testing implementations detail is not a good idea @@ -69,3 +70,39 @@ class TestIrAttachment(TransactionCase): a2_fn = os.path.join(self.filestore, a2_store_fname2) self.assertTrue(os.path.isfile(a2_fn)) + + def test_06_linked_record_permission(self): + model_ir_attachment = self.env.ref('base.model_ir_attachment') + Attachment = self.Attachment.with_user(self.env.ref('base.user_demo').id) + a1 = self.Attachment.create({'name': 'a1'}) + vals = {'name': 'attach', 'res_id': a1.id, 'res_model': 'ir.attachment'} + a2 = Attachment.create(vals) + + # remove access to linked record a1 + rule = self.env['ir.rule'].create({ + 'name': 'test_rule', 'domain_force': "[('id', '!=', %s)]" % a1.id, + 'model_id': self.env.ref('base.model_ir_attachment').id, + }) + a2.invalidate_cache(ids=a2.ids) + + # no read permission on linked record + with self.assertRaises(AccessError): + a2.datas + + # read permission on linked record + rule.perm_read = False + a2.datas + + # no write permission on linked record + with self.assertRaises(AccessError): + a3 = Attachment.create(vals) + with self.assertRaises(AccessError): + a2.write({'datas': self.blob2_b64}) + with self.assertRaises(AccessError): + a2.unlink() + + # write permission on linked record + rule.perm_write = False + a4 = Attachment.create(vals) + a4.write({'datas': self.blob2_b64}) + a4.unlink() -- GitLab