From 25ceed43770402bc01819d849946b5bb2c324f07 Mon Sep 17 00:00:00 2001 From: Pierre Verkest Date: Fri, 28 Apr 2023 13:51:35 +0200 Subject: [PATCH] [FIX] attachment_zipped_download: zip allowed document only The previous code allowed any authenticated to retreive any attachment present on odoo filesystem. So a WMS user could technically spoken download a zip with accounting documents. This implementation is calling attachemnt.check("read") to ensure access right and use attachemnt.raw attribute to retreive file to archive which (not test) should works with attachment saved somewhere else than the local filesystem (s3 storage, pgsql large object storage...). attachment_zipped_download 16.0.1.0.2 --- attachment_zipped_download/__manifest__.py | 2 +- .../models/ir_attachment.py | 11 ++- .../tests/test_attachment_zipped_download.py | 84 +++++++++++++++---- 3 files changed, 77 insertions(+), 20 deletions(-) diff --git a/attachment_zipped_download/__manifest__.py b/attachment_zipped_download/__manifest__.py index ab7a08db..209dc5e5 100644 --- a/attachment_zipped_download/__manifest__.py +++ b/attachment_zipped_download/__manifest__.py @@ -2,7 +2,7 @@ # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). { "name": "Attachment Zipped Download", - "version": "16.0.1.0.1", + "version": "16.0.1.0.2", "category": "Tools", "website": "https://github.com/OCA/knowledge", "author": "Tecnativa, Odoo Community Association (OCA)", diff --git a/attachment_zipped_download/models/ir_attachment.py b/attachment_zipped_download/models/ir_attachment.py index 1f58adad..f91d8588 100644 --- a/attachment_zipped_download/models/ir_attachment.py +++ b/attachment_zipped_download/models/ir_attachment.py @@ -28,9 +28,16 @@ class IrAttachment(models.Model): zip_buffer = BytesIO() with zipfile.ZipFile(zip_buffer, "a", zipfile.ZIP_DEFLATED, False) as zip_file: for attachment in self: - zip_file.write( - attachment._full_path(attachment.store_fname), attachment.name + attachment.check("read") + zip_file.writestr( + attachment._compute_zip_file_name(), + attachment.raw, ) zip_buffer.seek(0) zip_file.close() return zip_buffer + + def _compute_zip_file_name(self): + """Give a chance of easily changing the name of the file inside the ZIP.""" + self.ensure_one() + return self.name diff --git a/attachment_zipped_download/tests/test_attachment_zipped_download.py b/attachment_zipped_download/tests/test_attachment_zipped_download.py index 84a8910e..1422905e 100644 --- a/attachment_zipped_download/tests/test_attachment_zipped_download.py +++ b/attachment_zipped_download/tests/test_attachment_zipped_download.py @@ -1,12 +1,30 @@ # Copyright 2022-2023 Tecnativa - Víctor Martínez # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). import base64 +from unittest import TestCase -import odoo.tests -from odoo.tests import new_test_user +from odoo.exceptions import AccessError +from odoo.tests import HttpCase, SavepointCase, new_test_user -class TestAttachmentZippedDownload(odoo.tests.HttpCase): +class TestAttachmentZippedDownloadBase(TestCase): + @classmethod + def _create_attachment(cls, env, user, name, model=False, res_id=False): + return ( + env["ir.attachment"] + .with_user(user) + .create( + { + "name": name, + "datas": base64.b64encode(b"\xff data"), + "res_model": model, + "res_id": res_id, + } + ) + ) + + +class TestAttachmentZippedDownload(HttpCase, TestAttachmentZippedDownloadBase): def setUp(self): super().setUp() ctx = { @@ -20,24 +38,56 @@ class TestAttachmentZippedDownload(odoo.tests.HttpCase): login="test-user", context=ctx, ) - test_1 = self._create_attachment(self.user, "test1.txt") - test_2 = self._create_attachment(self.user, "test2.txt") + test_1 = self._create_attachment(self.env, self.user, "test1.txt") + test_2 = self._create_attachment(self.env, self.user, "test2.txt") self.attachments = test_1 + test_2 - def _create_attachment(self, user, name): - return ( - self.env["ir.attachment"] - .with_user(user) - .create( - { - "name": name, - "datas": base64.b64encode(b"\xff data"), - } - ) - ) - def test_action_attachments_download(self): self.authenticate("test-user", "test-user") res = self.attachments.action_attachments_download() response = self.url_open(res["url"], timeout=20) self.assertEqual(response.status_code, 200) + + +class TestAttachmentZipped(SavepointCase, TestAttachmentZippedDownloadBase): + @classmethod + def setUpClass(cls): + super().setUpClass() + ctx = { + "mail_create_nolog": True, + "mail_create_nosubscribe": True, + "mail_notrack": True, + "no_reset_password": True, + } + cls.user = new_test_user( + cls.env, + login="test-user", + password="test-user", + groups="base.group_user,base.group_partner_manager", + context=ctx, + ) + test_1 = cls._create_attachment(cls.env, cls.user, "test1.txt") + test_2 = cls._create_attachment(cls.env, cls.user, "test2.txt") + test_3 = cls._create_attachment( + cls.env, + cls.user, + "test3.txt", + model="res.partner", + res_id=cls.user.partner_id.id, + ) + cls.attachments = test_1 + test_2 + test_3 + + def test_create_temp_zip(self): + res = self.attachments._create_temp_zip() + self.assertTrue(res) + + def test_create_temp_zip_access_denined(self): + attachments = self.attachments | self._create_attachment( + self.env, + self.uid, + "test4.txt", + model="ir.ui.view", + res_id=self.env.ref("base.view_view_form").id, + ) + with self.assertRaises(AccessError): + attachments._create_temp_zip()