[IMP] l10n_it_edi: remove many2one ir.attachment field

This is part of a general change for all the many2one ir.attachment
fields to binary in order to avoid security issues as the former would
allow access to all ir.attachment records.

In l10n_it_edi, we remove the l10n_it_edi_attachment_id and adapt the
use of the existing l10n_it_edi_attachment_field, so only this binary
field is used. The field l10n_it_edi_attachment_name is added, so it
can be used in views.

task-4771720

closes odoo#212726

Related: odoo/upgrade#7785
Signed-off-by: John Laterre (jol) <[email protected]>

Author: mairasalazar

addons/l10n_it_edi/models/account_move.py

@@ -1,6 +1,6 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
-from base64 import b64encode
+from base64 import b64encode, b64decode
 from collections import defaultdict
 from datetime import datetime
 import logging
@@ -82,12 +82,7 @@ class AccountMove(models.Model):
     )
     l10n_it_edi_transaction = fields.Char(copy=False, string="FatturaPA Transaction")
     l10n_it_edi_attachment_file = fields.Binary(copy=False, attachment=True)
-    l10n_it_edi_attachment_id = fields.Many2one(
-        comodel_name='ir.attachment',
-        string="FatturaPA Attachment",
-        compute=lambda self: self._compute_linked_attachment_id('l10n_it_edi_attachment_id', 'l10n_it_edi_attachment_file'),
-        depends=['l10n_it_edi_attachment_file'],
-    )
+    l10n_it_edi_attachment_name = fields.Char(string="FatturaPA Attachment")
     l10n_it_edi_proxy_mode = fields.Selection(related="company_id.l10n_it_edi_proxy_user_id.edi_mode", depends=['company_id'])
     l10n_it_edi_button_label = fields.Char(compute="_compute_l10n_it_edi_button_label")
     l10n_it_edi_is_self_invoice = fields.Boolean(compute="_compute_l10n_it_edi_is_self_invoice")
@@ -374,9 +369,10 @@ def action_l10n_it_edi_send(self):
             }
 
         attachment_vals = self._l10n_it_edi_get_attachment_values(pdf_values=None)
-        self.env['ir.attachment'].create(attachment_vals)
-        self.invalidate_recordset(fnames=['l10n_it_edi_attachment_id', 'l10n_it_edi_attachment_file'])
-        self.message_post(attachment_ids=self.l10n_it_edi_attachment_id.ids)
+        self.l10n_it_edi_attachment_file = b64encode(attachment_vals['raw'])
+        self.l10n_it_edi_attachment_name = attachment_vals['name']
+        self.invalidate_recordset(fnames=['l10n_it_edi_attachment_name', 'l10n_it_edi_attachment_file'])
+        self.message_post(attachments=[(self.l10n_it_edi_attachment_name, attachment_vals['raw'])])
         self._l10n_it_edi_send({self: attachment_vals})
         self.is_move_sent = True
 
@@ -398,18 +394,18 @@ def _get_invoice_legal_documents(self, filetype, allow_fallback=False):
         # EXTENDS 'account'
         self.ensure_one()
         if filetype == 'fatturapa':
-            if fatturapa_attachment := self.l10n_it_edi_attachment_id:
+            if fatturapa_attachment := self.l10n_it_edi_attachment_file:
                 return {
-                    'filename': fatturapa_attachment.name,
+                    'filename': self.l10n_it_edi_attachment_name,
                     'filetype': 'xml',
-                    'content': fatturapa_attachment.raw,
+                    'content': b64decode(fatturapa_attachment),
                 }
         return super()._get_invoice_legal_documents(filetype, allow_fallback=allow_fallback)
 
     def get_extra_print_items(self):
         # EXTENDS 'account' - add possibility to download all FatturaPA XML files
         print_items = super().get_extra_print_items()
-        if self.l10n_it_edi_attachment_id:
+        if self.filtered('l10n_it_edi_attachment_file'):
             print_items.append({
                 'key': 'download_xml_fatturapa',
                 'description': _('XML FatturaPA'),
@@ -418,7 +414,7 @@ def get_extra_print_items(self):
         return print_items
 
     def action_invoice_download_fatturapa(self):
-        if invoices_with_fatturapa := self.filtered('l10n_it_edi_attachment_id'):
+        if invoices_with_fatturapa := self.filtered('l10n_it_edi_attachment_file'):
             return {
                 'type': 'ir.actions.act_url',
                 'url': f'/account/download_invoice_documents/{",".join(map(str, invoices_with_fatturapa.ids))}/fatturapa',
@@ -1376,16 +1372,11 @@ def _l10n_it_edi_import_invoice(self, invoice, data, is_new):
                 })]
 
             for element in tree.xpath('.//Allegati'):
-                attachment_64 = self.env['ir.attachment'].create({
-                    'name': get_text(element, './/NomeAttachment'),
-                    'datas': str.encode(get_text(element, './/Attachment')),
-                    'type': 'binary',
-                    'res_model': 'account.move',
-                    'res_id': self.id,
-                })
+                self.l10n_it_edi_attachment_name = get_text(element, './/NomeAttachment')
+                self.l10n_it_edi_attachment_file = b64decode(get_text(element, './/Attachment'))
                 self.sudo().message_post(
                     body=(_("Attachment from XML")),
-                    attachment_ids=[attachment_64.id],
+                    attachments=[(self.l10n_it_edi_attachment_name, self.l10n_it_edi_attachment_file)],
                 )
 
             for message in message_to_log:
@@ -1822,7 +1813,7 @@ def _l10n_it_edi_update_send_state(self):
         proxy_user = self.company_id.l10n_it_edi_proxy_user_id
         if proxy_user.edi_mode == 'demo':
             for move in self:
-                filename = move.l10n_it_edi_attachment_id and move.l10n_it_edi_attachment_id.name or '???'
+                filename = move.l10n_it_edi_attachment_name or '???'
                 self._l10n_it_edi_write_send_state(
                     transformed_notification={
                         'l10n_it_edi_state': 'forwarded',
@@ -1916,8 +1907,8 @@ def _l10n_it_edi_transform_notification(self, parsed_notification):
         filename = parsed_notification.get('filename')
         errors = parsed_notification.get('errors', [])
         date = parsed_notification.get('date', fields.Date.today())
-        if not filename and self.l10n_it_edi_attachment_id:
-            filename = self.l10n_it_edi_attachment_id.name
+        if not filename and self.l10n_it_edi_attachment_name:
+            filename = self.l10n_it_edi_attachment_name
         outcome = parsed_notification.get('outcome', False)
         if not outcome:
             new_state = state_map.get(sdi_state, False)

addons/l10n_it_edi/models/account_move_send.py

@@ -1,6 +1,7 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from odoo import _, api, models
+import base64
 
 
 class AccountMoveSend(models.AbstractModel):
@@ -53,13 +54,17 @@ def _get_alerts(self, moves, moves_data):
 
     def _get_invoice_extra_attachments(self, invoice):
         # EXTENDS 'account'
-        return super()._get_invoice_extra_attachments(invoice) + invoice.l10n_it_edi_attachment_id
+        return super()._get_invoice_extra_attachments(invoice) + self.env['ir.attachment'].search([
+            ('res_model', '=', 'account.move'),
+            ('res_field', '=', 'l10n_it_edi_attachment_file'),
+            ('res_id', 'in', invoice.ids),
+        ])
 
     def _hook_invoice_document_before_pdf_report_render(self, invoice, invoice_data):
         # EXTENDS 'account'
         super()._hook_invoice_document_before_pdf_report_render(invoice, invoice_data)
         if (
-                ('it_edi_send' in invoice_data['extra_edis'] and not invoice.l10n_it_edi_attachment_id)
+                ('it_edi_send' in invoice_data['extra_edis'] and not invoice.l10n_it_edi_attachment_file)
                 or (invoice_data['invoice_edi_format'] == 'it_edi_xml' and invoice._l10n_it_edi_ready_for_xml_export())
         ):
             if errors := invoice._l10n_it_edi_export_data_check():
@@ -74,7 +79,7 @@ def _hook_invoice_document_after_pdf_report_render(self, invoice, invoice_data):
         if (
             invoice_data.get('pdf_attachment_values')
             and (
-                ('it_edi_send' in invoice_data['extra_edis'] and not invoice.l10n_it_edi_attachment_id)
+                ('it_edi_send' in invoice_data['extra_edis'] and not invoice.l10n_it_edi_attachment_file)
                 or (invoice_data['invoice_edi_format'] == 'it_edi_xml' and invoice._l10n_it_edi_ready_for_xml_export())
             )
         ):
@@ -88,8 +93,8 @@ def _call_web_service_after_invoice_pdf_render(self, invoices_data):
         moves = self.env['account.move']
         for move, move_data in invoices_data.items():
             if 'it_edi_send' in move_data['extra_edis']:
-                if attachment := move.l10n_it_edi_attachment_id:
-                    attachments_vals[move] = {'name': attachment.name, 'raw': attachment.raw}
+                if attachment := move.l10n_it_edi_attachment_file:
+                    attachments_vals[move] = {'name': move.l10n_it_edi_attachment_name, 'raw': attachment}
                     moves |= move
                 elif edi_values := move_data.get('l10n_it_edi_values'):
                     attachments_vals[move] = edi_values
@@ -100,12 +105,19 @@ def _link_invoice_documents(self, invoices_data):
         # EXTENDS 'account'
         super()._link_invoice_documents(invoices_data)
 
-        attachments_vals = [
-            invoice_data.get('l10n_it_edi_values')
-            for invoice_data in invoices_data.values()
-            if invoice_data.get('l10n_it_edi_values')
-        ]
-        if attachments_vals:
-            attachments = self.env['ir.attachment'].sudo().create(attachments_vals)
-            res_ids = attachments.mapped('res_id')
-            self.env['account.move'].browse(res_ids).invalidate_recordset(fnames=['l10n_it_edi_attachment_id', 'l10n_it_edi_attachment_file'])
+        move_ids_to_names = {}
+        for move, data in invoices_data.items():
+            if values := data.get('l10n_it_edi_values'):
+                move.l10n_it_edi_attachment_file = base64.b64encode(values['raw'])
+                move.l10n_it_edi_attachment_name = values['name']
+                move.invalidate_recordset(fnames=['l10n_it_edi_attachment_name', 'l10n_it_edi_attachment_file'])
+                move_ids_to_names[move.id] = values['name']
+
+        if move_ids_to_names:
+            attachments = self.env['ir.attachment'].search([
+                ('res_model', '=', 'account.move'),
+                ('res_field', '=', 'l10n_it_edi_attachment_file'),
+                ('res_id', 'in', list(move_ids_to_names)),
+            ])
+            for attachment in attachments:
+                attachment.name = move_ids_to_names.get(attachment.res_id)

addons/l10n_it_edi/tests/test_account_move_send.py

@@ -46,11 +46,11 @@ def _get_default_extra_edis(self, move):
         self.assertEqual((invoice1 + invoice2).mapped('sending_data'), [False, False])
         self.assertEqual(1, len(self.get_attachments(invoice1.id)))
         self.assertTrue(invoice1.invoice_pdf_report_id)
-        self.assertFalse(invoice1.l10n_it_edi_attachment_id)
+        self.assertFalse(invoice1.l10n_it_edi_attachment_file)
         self.assertFalse(invoice1.is_being_sent)
         self.assertEqual(1, len(self.get_attachments(invoice2.id)))
         self.assertTrue(invoice2.invoice_pdf_report_id)
-        self.assertFalse(invoice2.l10n_it_edi_attachment_id)
+        self.assertFalse(invoice2.l10n_it_edi_attachment_file)
         self.assertFalse(invoice2.is_being_sent)
 
     def test_invoice_multi_with_l10n_it_edi_xml_export(self):
@@ -72,11 +72,11 @@ def _get_default_extra_edis(self, move):
         self.assertEqual((invoice1 + invoice2).mapped('sending_data'), [False, False])
         self.assertEqual(2, len(self.get_attachments(invoice1.id)))
         self.assertTrue(invoice1.invoice_pdf_report_id)
-        self.assertTrue(invoice1.l10n_it_edi_attachment_id)
+        self.assertTrue(invoice1.l10n_it_edi_attachment_file)
         self.assertFalse(invoice1.is_being_sent)
         self.assertEqual(2, len(self.get_attachments(invoice2.id)))
         self.assertTrue(invoice2.invoice_pdf_report_id)
-        self.assertTrue(invoice2.l10n_it_edi_attachment_id)
+        self.assertTrue(invoice2.l10n_it_edi_attachment_file)
         self.assertFalse(invoice2.is_being_sent)
 
     def test_invoice_with_cig_or_cup_or_both(self):

addons/l10n_it_edi/views/l10n_it_view.xml

@@ -83,7 +83,7 @@
         <field name="arch" type="xml">
             <field name="status_in_payment" position="before">
                 <field name="l10n_it_edi_transaction" optional="hide"/>
-                <field name="l10n_it_edi_attachment_id" optional="hide"/>
+                <field name="l10n_it_edi_attachment_name" optional="hide"/>
                 <field name="l10n_it_edi_state" optional="hide"/>
             </field>
         </field>
@@ -96,7 +96,7 @@
         <field name="arch" type="xml">
             <xpath expr="//search/field[@name='journal_id']" position="after">
                 <field name="l10n_it_edi_transaction" groups="base.group_no_one"/>
-                <field name="l10n_it_edi_attachment_id" groups="base.group_no_one"/>
+                <field name="l10n_it_edi_attachment_name" groups="base.group_no_one"/>
                 <field name="l10n_it_edi_state" groups="base.group_no_one"/>
             </xpath>
         </field>
@@ -118,7 +118,6 @@
                 </xpath>
                 <xpath expr="//sheet" position="before">
                     <field name="l10n_it_edi_is_self_invoice" invisible="1"/>
-                    <field name="l10n_it_edi_attachment_id" invisible="1"/>
                     <div class="alert alert-warning" role="alert"
                         invisible="not l10n_it_edi_header 
                                    or state == 'draft'
@@ -147,8 +146,9 @@
                         invisible="move_type not in ('out_invoice', 'out_refund', 'in_invoice', 'in_refund') or country_code != 'IT'">
                         <group>
                             <group>
+                                <field name="l10n_it_edi_attachment_name" invisible="1"/>
                                 <field name="l10n_it_edi_transaction" groups="base.group_no_one" readonly="1"/>
-                                <field name="l10n_it_edi_attachment_id" groups="base.group_no_one" readonly="1"/>
+                                <field name="l10n_it_edi_attachment_file" widget="binary" filename="l10n_it_edi_attachment_name" groups="base.group_no_one" readonly="1"/>
                                 <field name="l10n_it_stamp_duty" readonly="state != 'draft'"/>
                                 <field name="l10n_it_ddt_id" readonly="state != 'draft'" invisible="move_type not in ('out_invoice', 'out_refund')"/>
                                 <field name="l10n_it_document_type"