[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) <xmo@odoo.com>

1 files 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']