[FIX] mail: add exceptions to message attachment checks

Some modules may use attachments from mail messages directly.

In that case it may be desirable to at least be able to write
over the name and other non-critical information even if the
attachment is linked to a document.

The restrictions on writing on message attachments is reduced
to only apply to data fields, as those are the only ones that
we really don't want people to change.

Also return True instead of None in the override of `check`
to match the behavior of the parent.

Also reword the error message to convey writing is also forbidden.

Complementary to 4c4e63f

task-3519815

closes #165173

Related: odoo/enterprise#62343
Signed-off-by: Warnon Aurélien (awa) <awa@odoo.com>
Signed-off-by: Renaud Thiry (reth) <reth@odoo.com>

addons/mail/i18n/mail.pot

@@ -9885,7 +9885,7 @@ msgstr ""
 #. odoo-python
 #: code:addons/mail/models/ir_attachment.py:0
 #, python-format
-msgid "You may not unlink attachments from other people's messages"
+msgid "You may not unlink or modify the content of attachments from other people's messages"
 msgstr ""
 
 #. module: mail

addons/mail/models/ir_attachment.py

@@ -15,16 +15,18 @@ class IrAttachment(models.Model):
     @api.model
     def check(self, mode, values=None):
         super().check(mode, values=values)
+        if mode == 'write' and not {'datas', 'db_datas', 'raw'} & (values or {}).keys():
+            return True
         if mode not in ('unlink', 'write') or not self or self.env.is_admin():
-            return
+            return True
         if self.create_uid == self.env.user:
-            return
+            return True
         linked_messages = self.env['mail.message'].sudo().search([('attachment_ids', 'in', self.ids)])
         if not linked_messages:
-            return
+            return True
         authors = linked_messages.author_id
         if len(authors) > 1 or authors != self.env.user.partner_id:
-            raise AccessError(_("You may not unlink attachments from other people's messages"))
+            raise AccessError(_("You may not unlink or modify the content of attachments from other people's messages"))
 
     def _check_attachments_access(self, attachment_tokens):
         """This method relies on access rules/rights and therefore it should not be called from a sudo env."""

addons/test_mail/tests/test_ir_attachment.py

@@ -59,7 +59,7 @@ def test_attachment_forbid_unlink(self):
         for user, attachment in forbidden_list:
             with self.subTest(user=user.name, attachment=attachment.name, method='write'):
                 with self.assertRaises(AccessError):
-                    attachment.with_user(user).write({'name': 'failed test name'})
+                    attachment.with_user(user).write({'datas': '0123'})
             with self.subTest(user=user.name, attachment=attachment.name, method='unlink'):
                 with self.assertRaises(AccessError):
                     attachment.with_user(user).unlink()
@@ -74,8 +74,11 @@ def test_attachment_forbid_unlink(self):
         ]
         for user, attachment, sudo in allowed_list:
             with self.subTest(user=user.name, attachment=attachment.name, sudo=sudo, method='write'):
-                attachment.with_user(user).sudo(sudo).write({'name': 'successful test name'})
-                self.assertEqual(attachment.name, 'successful test name')
+                attachment.with_user(user).sudo(sudo).write({'datas': '1234'})
+                self.assertEqual(attachment.datas, b'1234')
             with self.subTest(user=user.name, attachment=attachment.name, sudo=sudo, method='unlink'):
                 attachment.with_user(user).sudo(sudo).unlink()
                 self.assertFalse(attachment.exists())
+
+        shared_attachment_employee.with_user(user_second_employee).write({'name': 'Successful write to shared attachment'})
+        self.assertEqual(shared_attachment_employee.name, 'Successful write to shared attachment', 'Only data fields should be protected')