From aee3af0abe626b625682d367f45953423a8abaf7 Mon Sep 17 00:00:00 2001 From: Denis Ledoux Date: Tue, 18 Apr 2023 12:55:37 +0000 Subject: [PATCH] [FIX] core: access to `ir.attachment` without res_model/res_id This revision restores the behavior of 16.0 regarding the access to attachments with no `res_model`/`res_id` set. It was changed unexpectedly during the refactoring of `read`/`fetch`. Basically, the below used to raise an AccessError in 16.0: Create an attachment without res_model/res_id and access it as another user ```py attachment = self.env['ir.attachment'].create({'name': 'foo'}) attachment.invalidate_recordset() attachment.with_user(self.env.ref('base.user_demo')).datas ``` While it no longer raises the AccessError in saas-16.2 The reason is that the override of `_read` calling `check` has been removed in saas-16.2: https://github.com/odoo/odoo/commit/e962860c6f0d8ec9e50bb376e1faab5c7bc69374#diff-ab3dadc37163820f8e863edb1dd216f8b0e7ccf19cf4e2052577a7bae7ddd7e8L606-L608 in favor to do the check in `_search`: https://github.com/odoo/odoo/commit/ae31aebf095392d8c91f49ac279e1949997dc245 But there is a difference of behavior between the checks in `check` and `_search` regarding attachments without `res_model`/`res_id`: https://github.com/odoo/odoo-security/blob/bc538f6944461a642bac0f757ec96d7f8cc14c9a/odoo/addons/base/models/ir_attachment.py#L454-L465 https://github.com/odoo/odoo-security/blob/bc538f6944461a642bac0f757ec96d7f8cc14c9a/odoo/addons/base/models/ir_attachment.py#L566-L573 The goal of this revision is to have an unified behavior regarding the treatment of attachments without `res_model`/`res_id` in both `check` and `_search`. closes odoo/odoo#119768 X-original-commit: 82c36285b897b72a956da0585ebd62f4c2d33784 Signed-off-by: Denis Ledoux (dle) --- odoo/addons/base/models/ir_attachment.py | 6 ++-- odoo/addons/base/tests/test_ir_attachment.py | 29 +++++++++++++++++++- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/odoo/addons/base/models/ir_attachment.py b/odoo/addons/base/models/ir_attachment.py index 5ca7e67f0b8..72c8db66751 100644 --- a/odoo/addons/base/models/ir_attachment.py +++ b/odoo/addons/base/models/ir_attachment.py @@ -544,13 +544,13 @@ class IrAttachment(models.Model): model_attachments = defaultdict(lambda: defaultdict(set)) # {res_model: {res_id: set(ids)}} for id_, res_model, res_id, res_field, public, create_uid in rows: all_ids.append(id_) - if not res_model or public: + if public: allowed_ids.add(id_) continue - if res_model and not res_id and create_uid == self.env.uid: + if not res_id and (self.env.is_system() or create_uid == self.env.uid): allowed_ids.add(id_) continue - if not (res_field and disable_binary_fields_attachments): + if not (res_field and disable_binary_fields_attachments) and res_model and res_id: model_attachments[res_model][res_id].add(id_) # check permissions on records model by model diff --git a/odoo/addons/base/tests/test_ir_attachment.py b/odoo/addons/base/tests/test_ir_attachment.py index 7146629e8f3..4e32df4fd20 100644 --- a/odoo/addons/base/tests/test_ir_attachment.py +++ b/odoo/addons/base/tests/test_ir_attachment.py @@ -284,8 +284,10 @@ class TestPermissions(TransactionCase): self.env.flush_all() a.invalidate_recordset() - def test_no_read_permission(self): + def test_read_permission(self): """If the record can't be read, the attachment can't be read either + If the attachment is public, the attachment can be read even if the record can't be read + If the attachment has no res_model/res_id, it can be read by its author and admins only """ # check that the information can be read out of the box self.attachment.datas @@ -295,6 +297,31 @@ class TestPermissions(TransactionCase): with self.assertRaises(AccessError): self.attachment.datas + # Make the attachment public + self.attachment.sudo().public = True + # Check the information can be read again + self.attachment.datas + # Remove the public access + self.attachment.sudo().public = False + # Check the record can no longer be accessed + with self.assertRaises(AccessError): + self.attachment.datas + + # Create an attachment as user without res_model/res_id + attachment_user = self.Attachments.create({'name': 'foo'}) + # Check the user can access his own attachment + attachment_user.datas + # Create an attachment as superuser without res_model/res_id + attachment_admin = self.Attachments.with_user(odoo.SUPERUSER_ID).create({'name': 'foo'}) + # Check the record cannot be accessed by a regular user + with self.assertRaises(AccessError): + attachment_admin.with_user(self.env.user).datas + # Check the record can be accessed by an admin (other than superuser) + admin_user = self.env.ref('base.user_admin') + # Safety assert that base.user_admin is not the superuser, otherwise the test is useless + self.assertNotEqual(odoo.SUPERUSER_ID, admin_user.id) + attachment_admin.with_user(admin_user).datas + def test_with_write_permissions(self): """With write permissions to the linked record, attachment can be created, updated, or deleted (or copied).