From 90fa3e3c60e9b96e8d9ba071ae655a576d949a96 Mon Sep 17 00:00:00 2001 From: ryv-odoo Date: Thu, 10 Oct 2019 08:04:29 +0000 Subject: [PATCH] [REF] base: simplify check method attachments now always requiring employee rights Since commit 62c9dedafda0cbdc618641eaa80c49535ab0b5f6 attachments almost always require employee user access, unless some specific flow grants super user rights. Moreover other flows like portal checks user rights before going into attachments management with superuser rights. In this commit we simplify code by directly checking current user belongs to internal employee group (or is a superuser / admin). We consider tt is the role of controller/model using attachments to check and grants access to attachments. Some history of attachments management improvement (not complete, just an overview of discussed points) * 3620cb68a0190031d3e1a963d8554834ab7f192b: improve attachment management in portal; * de7c119e666d108564b3889d1c08e5bf2e4da082: improve message_post and its attachments management; * 62c9dedafda0cbdc618641eaa80c49535ab0b5f6: clean ACLs; Task ID 2004031 PR #38358 closes odoo/odoo#39869 Signed-off-by: Xavier Morel (xmo) --- odoo/addons/base/models/ir_attachment.py | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/odoo/addons/base/models/ir_attachment.py b/odoo/addons/base/models/ir_attachment.py index 2dbbdd77eff..51a386fce34 100644 --- a/odoo/addons/base/models/ir_attachment.py +++ b/odoo/addons/base/models/ir_attachment.py @@ -343,15 +343,14 @@ class IrAttachment(models.Model): @api.model def check(self, mode, values=None): - """Restricts the access to an ir.attachment, according to referred model - In the 'document' module, it is overridden to relax this hard rule, since - more complex ones apply there. - """ + """ Restricts the access to an ir.attachment, according to referred mode """ if self.env.is_superuser(): return True + # Always require an internal user (aka, employee) to access to a attachment + if not (self.env.is_admin() or self.env.user.has_group('base.group_user')): + raise AccessError(_("Sorry, you are not allowed to access this document.")) # collect the records to check (by model) model_ids = defaultdict(set) # {model_name: set(ids)} - require_employee = False if self: # DLE P173: `test_01_portal_attachment` self.env['ir.attachment'].flush(['res_model', 'res_id', 'create_uid', 'public', 'res_field']) @@ -362,8 +361,6 @@ class IrAttachment(models.Model): if public and mode == 'read': continue if not (res_model and res_id): - if create_uid != self._uid: - require_employee = True continue model_ids[res_model].add(res_id) if values and values.get('res_model') and values.get('res_id'): @@ -375,24 +372,18 @@ class IrAttachment(models.Model): # when checking access rights (resource was deleted but attachment # was not) if res_model not in self.env: - require_employee = True continue - elif res_model == 'res.users' and len(res_ids) == 1 and self._uid == list(res_ids)[0]: + if res_model == 'res.users' and len(res_ids) == 1 and self.env.uid == list(res_ids)[0]: # by default a user cannot write on itself, despite the list of writeable fields # e.g. in the case of a user inserting an image into his image signature # we need to bypass this check which would needlessly throw us away continue records = self.env[res_model].browse(res_ids).exists() - if len(records) < len(res_ids): - 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) - if require_employee: - if not (self.env.is_admin() or self.env.user.has_group('base.group_user')): - raise AccessError(_("Sorry, you are not allowed to access this document.")) def _read_group_allowed_fields(self): return ['type', 'company_id', 'res_id', 'create_date', 'create_uid', 'name', 'mimetype', 'id', 'url', 'res_field', 'res_model']