From 5632414435b8c879f8331dc00299ea9b780c7c71 Mon Sep 17 00:00:00 2001 From: Denis Ledoux <dle@odoo.com> Date: Mon, 17 Apr 2023 12:44:58 +0200 Subject: [PATCH] [FIX] snailmail: accurate error message when no access to the attachment This revision follows - odoo/odoo@5e81850ea64b533f0576c919a427820554096284 - odoo/enterprise@e484b4da0c8b70225e91cc66b64ac1b3a0bcc685 --- addons/snailmail/models/snailmail_letter.py | 7 ++ addons/snailmail/tests/__init__.py | 4 + .../snailmail/tests/test_attachment_access.py | 104 ++++++++++++++++++ 3 files changed, 115 insertions(+) create mode 100644 addons/snailmail/tests/__init__.py create mode 100644 addons/snailmail/tests/test_attachment_access.py diff --git a/addons/snailmail/models/snailmail_letter.py b/addons/snailmail/models/snailmail_letter.py index 0ee0a23e00d7..818379eebfb9 100644 --- a/addons/snailmail/models/snailmail_letter.py +++ b/addons/snailmail/models/snailmail_letter.py @@ -114,8 +114,15 @@ class SnailmailLetter(models.Model): 'notification_status': 'ready', }) + letter.attachment_id.check('read') return letter + def write(self, vals): + res = super().write(vals) + if 'attachment_id' in vals: + self.attachment_id.check('read') + return res + def _fetch_attachment(self): """ This method will check if we have any existent attachement matching the model diff --git a/addons/snailmail/tests/__init__.py b/addons/snailmail/tests/__init__.py new file mode 100644 index 000000000000..88fe8443a5a2 --- /dev/null +++ b/addons/snailmail/tests/__init__.py @@ -0,0 +1,4 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from . import test_attachment_access diff --git a/addons/snailmail/tests/test_attachment_access.py b/addons/snailmail/tests/test_attachment_access.py new file mode 100644 index 000000000000..ed53494fcd79 --- /dev/null +++ b/addons/snailmail/tests/test_attachment_access.py @@ -0,0 +1,104 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. +import base64 + +from odoo.exceptions import AccessError +from odoo.tests import SavepointCase + + +class testAttachmentAccess(SavepointCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.user = cls.env['res.users'].create({ + 'name': "foo", + 'login': "foo", + 'email': "foo@bar.com", + 'groups_id': [(6, 0, [ + cls.env.ref('base.group_user').id, + cls.env.ref('base.group_partner_manager').id, + ])] + }) + cls.letter_defaults = { + 'model': cls.user.partner_id._name, + 'res_id': cls.user.partner_id.id, + 'partner_id': cls.user.partner_id.id, + } + + def test_user_letter_attachment_without_res_fields(self): + """Test an employee can create a letter linked to an attachment without res_model/res_id""" + env_user = self.env(user=self.user) + # As user, create an attachment without res_model/res_id + attachment = env_user['ir.attachment'].create({'name': 'foo', 'datas': base64.b64encode(b'foo')}) + # As user, create a snailmail.letter linked to that attachment + letter = env_user['snailmail.letter'].create({'attachment_id': attachment.id, **self.letter_defaults}) + # As user, ensure the content of the attachment can be read through the letter + self.assertEqual(base64.b64decode(letter.attachment_datas), b'foo') + # As user, create another attachment without res_model/res_id + attachment_2 = env_user['ir.attachment'].create({'name': 'foo', 'datas': base64.b64encode(b'bar')}) + # As user, change the attachment of the letter to this second attachment + letter.write({'attachment_id': attachment_2.id}) + # As user, ensure the content of this second attachment can be read through the letter + self.assertEqual(base64.b64decode(letter.attachment_datas), b'bar') + + def test_user_letter_attachment_without_res_fields_created_by_admin(self): + """Test an employee can read the content of the letter's attachment created by another user, the admin, + and the attachment does not have a res_model/res_id + """ + # As admin, create an attachment without res_model/res_id + attachment = self.env['ir.attachment'].create({'name': 'foo', 'datas': base64.b64encode(b'foo')}) + # As admin, create a snailmail.letter linked to that attachment + letter = self.env['snailmail.letter'].create({'attachment_id': attachment.id, **self.letter_defaults}) + + # As user, ensure the attachment itself cannot be read + attachment.invalidate_cache() + with self.assertRaises(AccessError): + attachment.with_user(self.user).datas + # But, as user, the content of the attachment can be read through the letter + self.assertEqual(base64.b64decode(letter.with_user(self.user).attachment_datas), b'foo') + + # As admin, create a second attachment without res_model/res_id + attachment = self.env['ir.attachment'].create({'name': 'bar', 'datas': base64.b64encode(b'bar')}) + # As admin, link this second attachment to the previously created letter (write instead of create) + letter.write({'attachment_id': attachment.id}) + + # As user ensure the attachment itself cannot be read + attachment.invalidate_cache() + with self.assertRaises(AccessError): + self.assertEqual(base64.b64decode(attachment.with_user(self.user).datas), b'bar') + # But, as user, the content of the attachment can be read through the letter + self.assertEqual(base64.b64decode(letter.with_user(self.user).attachment_datas), b'bar') + + def test_user_read_unallowed_attachment(self): + """Test a user cannot access an attachment he is not supposed to through a snailmail.letter""" + # As admin, create an attachment for which you require the settings group to access + base_module = self.env.ref('base.module_base') + attachment_forbidden = self.env['ir.attachment'].create({ + 'name': 'foo', 'datas': base64.b64encode(b'foo'), + 'res_model': base_module._name, 'res_id': base_module.id, + }) + # As user, make sure this is indeed not possible to access that attachment data directly + attachment_forbidden.invalidate_cache() + with self.assertRaises(AccessError): + attachment_forbidden.with_user(self.user).datas + # As user, create a letter pointing to that attachment + # and make sure it raises an access error + with self.assertRaises(AccessError): + letter = self.env['snailmail.letter'].with_user(self.user).create({ + 'attachment_id': attachment_forbidden.id, + **self.letter_defaults, + }) + letter.attachment_datas + + # As user, update the attachment of an existing letter to the unallowed attachment + # and make sure it raises an access error + attachment_tmp = self.env['ir.attachment'].with_user(self.user).create({ + 'name': 'bar', 'datas': base64.b64encode(b'bar'), + }) + letter = self.env['snailmail.letter'].with_user(self.user).create({ + 'attachment_id': attachment_tmp.id, + **self.letter_defaults, + }) + with self.assertRaises(AccessError): + letter.write({'attachment_id': attachment_forbidden.id}) + letter.attachment_datas -- GitLab