[FIX] mail: allow bypassing message attachments check

addons/mail/i18n/mail.pot

@@ -7667,7 +7667,7 @@ msgstr ""
 #. module: mail
 #: 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

@@ -13,16 +13,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/mail/static/src/components/message/tests/message_tests.js

@@ -12,6 +12,8 @@ import {
 } from '@mail/utils/test_utils';
 
 import Bus from 'web.Bus';
+import { registerCleanup } from "@web/../tests/helpers/cleanup";
+import { session } from '@web/session';
 
 QUnit.module('mail', {}, function () {
 QUnit.module('components', {}, function () {
@@ -704,6 +706,11 @@ QUnit.test('allow attachment delete on authored message', async function (assert
 QUnit.test('prevent attachment delete on non-authored message in channels', async function (assert) {
     assert.expect(2);
 
+    // mock a regular user
+    const sessionAdminState = session.is_admin;
+    session.is_admin = false;
+    registerCleanup(() => session.is_admin = sessionAdminState);
+
     const { createMessageComponent } = await this.start();
     const message = this.messaging.models['mail.message'].create({
         attachments: insertAndReplace({

addons/mail/static/src/models/attachment/attachment.js

@@ -4,6 +4,8 @@ import { registerNewModel } from '@mail/model/model_core';
 import { attr, many2many, many2one, one2many } from '@mail/model/model_field';
 import { clear, insert } from '@mail/model/model_field_command';
 
+import { session } from '@web/session';
+
 function factory(dependencies) {
 
     class Attachment extends dependencies['mail.model'] {
@@ -197,7 +199,9 @@ function factory(dependencies) {
             if (!this.messaging) {
                 return;
             }
-
+            if (session.is_admin) {
+                return true;
+            }
             if (this.messages.length) {
                 return this.messages.some(message => (
                     message.canBeDeleted ||

addons/test_mail/tests/test_ir_attachment.py

@@ -58,7 +58,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()
@@ -73,8 +73,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')