From c25341ed0cbd51909a0b604db1f9611299760be8 Mon Sep 17 00:00:00 2001 From: "Nicolas (vin)" Date: Thu, 2 Mar 2023 16:11:06 +0000 Subject: [PATCH] [IMP] account: payment scamming protection Odoo does not have much in terms of payment protection; you can create payments to any account you want without many checks. This changes it by requiring users with the right groups to trust an account before allowing it to be used for payment methods requiring. Also adds some warnings on the bank account view, and checks at the time of creating a payment. Task id # 3210415 closes odoo/odoo#114278 Related: odoo/enterprise#37789 Related: odoo/documentation#3737 Related: odoo/upgrade#4397 Signed-off-by: Florian Gilbert (flg) --- addons/account/models/account_move.py | 1 + addons/account/models/account_payment.py | 10 ++- addons/account/models/res_partner_bank.py | 79 ++++++++++++++++++- addons/account/security/account_security.xml | 6 ++ addons/account/views/account_menuitem.xml | 1 + addons/account/views/account_move_views.xml | 12 +-- addons/account/views/account_payment_view.xml | 10 ++- addons/account/views/partner_view.xml | 14 +--- .../account/views/res_partner_bank_views.xml | 74 +++++++++++++++++ .../wizard/account_payment_register.py | 71 ++++++++++++++++- .../wizard/account_payment_register_views.xml | 11 ++- odoo/addons/base/models/res_bank.py | 9 ++- odoo/addons/base/views/res_bank_views.xml | 10 +-- 13 files changed, 275 insertions(+), 33 deletions(-) diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py index 4a682a6db1f..87ac3b1e02d 100644 --- a/addons/account/models/account_move.py +++ b/addons/account/models/account_move.py @@ -321,6 +321,7 @@ class AccountMove(models.Model): "otherwise a Partner bank account number.", check_company=True, tracking=True, + ondelete='restrict', ) fiscal_position_id = fields.Many2one( 'account.fiscal.position', diff --git a/addons/account/models/account_payment.py b/addons/account/models/account_payment.py index 6183503fa37..42c8cec74bb 100644 --- a/addons/account/models/account_payment.py +++ b/addons/account/models/account_payment.py @@ -30,7 +30,9 @@ class AccountPayment(models.Model): readonly=False, store=True, tracking=True, compute='_compute_partner_bank_id', domain="[('id', 'in', available_partner_bank_ids)]", - check_company=True) + check_company=True, + ondelete='restrict', + ) is_internal_transfer = fields.Boolean(string="Internal Transfer", readonly=False, store=True, tracking=True, @@ -528,6 +530,7 @@ class AccountPayment(models.Model): for pay in self: if pay.state in ('draft', 'posted') \ and pay.partner_bank_id \ + and pay.partner_bank_id.allow_out_payment \ and pay.payment_method_line_id.code == 'manual' \ and pay.payment_type == 'outbound' \ and pay.currency_id: @@ -921,6 +924,11 @@ class AccountPayment(models.Model): def action_post(self): ''' draft -> posted ''' + # Do not allow to post if the account is required but not trusted + for payment in self: + if payment.require_partner_bank_account and not payment.partner_bank_id.allow_out_payment: + raise UserError(_('To record payments with %s, the recipient bank account must be manually validated. You should go on the partner bank account in order to validate it.', self.payment_method_line_id.name)) + self.move_id._post(soft=False) self.filtered( diff --git a/addons/account/models/res_partner_bank.py b/addons/account/models/res_partner_bank.py index 09b0683bc7d..701bc45d455 100644 --- a/addons/account/models/res_partner_bank.py +++ b/addons/account/models/res_partner_bank.py @@ -16,6 +16,21 @@ class ResPartnerBank(models.Model): journal_id = fields.One2many( 'account.journal', 'bank_account_id', domain=[('type', '=', 'bank')], string='Account Journal', readonly=True, help="The accounting journal corresponding to this bank account.") + has_iban_warning = fields.Boolean( + compute='_compute_display_account_warning', + help='Technical field used to display a warning if the IBAN country is different than the holder country.', + store=True, + ) + partner_country_name = fields.Char(related='partner_id.country_id.name') + has_money_transfer_warning = fields.Boolean( + compute='_compute_display_account_warning', + help='Technical field used to display a warning if the account is a transfer service account.', + store=True, + ) + money_transfer_service = fields.Char(compute='_compute_money_transfer_service_name') + partner_supplier_rank = fields.Integer(related='partner_id.supplier_rank') + partner_customer_rank = fields.Integer(related='partner_id.customer_rank') + related_moves = fields.One2many('account.move', inverse_name='partner_bank_id') # Add tracking to the base fields bank_id = fields.Many2one(tracking=True) @@ -23,7 +38,13 @@ class ResPartnerBank(models.Model): acc_number = fields.Char(tracking=True) acc_holder_name = fields.Char(tracking=True) partner_id = fields.Many2one(tracking=True) - allow_out_payment = fields.Boolean(tracking=True) + user_has_group_validate_bank_account = fields.Boolean(compute='_compute_user_has_group_validate_bank_account') + allow_out_payment = fields.Boolean( + tracking=True, + help='Sending fake invoices with a fraudulent account number is a common phishing practice. ' + 'To protect yourself, always verify new bank account numbers, preferably by calling the vendor, as phishing ' + 'usually happens when their emails are compromised. Once verified, you can activate the ability to send money.' + ) currency_id = fields.Many2one(tracking=True) @api.constrains('journal_id') @@ -32,6 +53,49 @@ class ResPartnerBank(models.Model): if len(bank.journal_id) > 1: raise ValidationError(_('A bank account can belong to only one journal.')) + @api.constrains('allow_out_payment') + def _check_allow_out_payment(self): + """ Block enabling the setting, but it can be set to false without the group. (For example, at creation) """ + for bank in self: + if bank.allow_out_payment: + if not self.user_has_groups('account.group_validate_bank_account'): + raise ValidationError(_('You do not have the right to trust or un-trust a bank account.')) + + @api.depends('partner_id.country_id', 'sanitized_acc_number', 'allow_out_payment', 'acc_type') + def _compute_display_account_warning(self): + for bank in self: + if bank.allow_out_payment or not bank.sanitized_acc_number or bank.acc_type != 'iban': + bank.has_iban_warning = False + bank.has_money_transfer_warning = False + continue + bank_country = bank.sanitized_acc_number[:2] + bank.has_iban_warning = bank.partner_id.country_id and bank_country != bank.partner_id.country_id.code + + bank_institution_code = bank.sanitized_acc_number[4:7] + bank.has_money_transfer_warning = bank_institution_code in bank._get_money_transfer_services() + + @api.depends('sanitized_acc_number', 'allow_out_payment') + def _compute_money_transfer_service_name(self): + for bank in self: + if bank.sanitized_acc_number: + bank_institution_code = bank.sanitized_acc_number[4:7] + bank.money_transfer_service = bank._get_money_transfer_services().get(bank_institution_code, False) + else: + bank.money_transfer_service = False + + def _get_money_transfer_services(self): + return { + '967': 'Wise', + '977': 'Paynovate', + '974': 'PPS EU SA', + } + + @api.depends('acc_number') + def _compute_user_has_group_validate_bank_account(self): + user_has_group_validate_bank_account = self.user_has_groups('account.group_validate_bank_account') + for bank in self: + bank.user_has_group_validate_bank_account = user_has_group_validate_bank_account + def _build_qr_code_vals(self, amount, free_communication, structured_communication, currency, debtor_partner, qr_method=None, silent_errors=True): """ Returns the QR-code vals needed to generate the QR-code report link to pay this account with the given parameters, or None if no QR-code could be generated. @@ -215,3 +279,16 @@ class ResPartnerBank(models.Model): msg = _("Bank Account %s with number %s deleted", account._get_html_link(title=f"#{account.id}"), account.acc_number) account.partner_id._message_log(body=msg) return super().unlink() + + def name_get(self): + res = super().name_get() + if self.env.context.get('display_account_trust'): + res = [] + for acc in self: + trusted_label = _('trusted') if acc.allow_out_payment else _('untrusted') + if acc.bank_id: + name = '{} - {} ({})'.format(acc.acc_number, acc.bank_id.name, trusted_label) + else: + name = '{} ({})'.format(acc.acc_number, trusted_label) + res.append((acc.id, name)) + return res diff --git a/addons/account/security/account_security.xml b/addons/account/security/account_security.xml index 0227f9ac26a..035b468a19d 100644 --- a/addons/account/security/account_security.xml +++ b/addons/account/security/account_security.xml @@ -82,6 +82,12 @@ + + Validate bank account + + + + diff --git a/addons/account/views/account_menuitem.xml b/addons/account/views/account_menuitem.xml index 85dcc0d9c03..7b275783a4d 100644 --- a/addons/account/views/account_menuitem.xml +++ b/addons/account/views/account_menuitem.xml @@ -20,6 +20,7 @@ + diff --git a/addons/account/views/account_move_views.xml b/addons/account/views/account_move_views.xml index 732a6a296da..31484b11b13 100644 --- a/addons/account/views/account_move_views.xml +++ b/addons/account/views/account_move_views.xml @@ -595,7 +595,7 @@ . + @@ -41,7 +45,7 @@ + context="{'display_account_trust': True}"/> @@ -79,8 +83,9 @@
-
diff --git a/odoo/addons/base/models/res_bank.py b/odoo/addons/base/models/res_bank.py index 987a3cc5e66..52b9cfcfb54 100644 --- a/odoo/addons/base/models/res_bank.py +++ b/odoo/addons/base/models/res_bank.py @@ -76,7 +76,7 @@ class ResPartnerBank(models.Model): acc_type = fields.Selection(selection=lambda x: x.env['res.partner.bank'].get_supported_account_types(), compute='_compute_acc_type', string='Type', help='Bank account type: Normal or IBAN. Inferred from the bank account number.') acc_number = fields.Char('Account Number', required=True) sanitized_acc_number = fields.Char(compute='_compute_sanitized_acc_number', string='Sanitized Account Number', readonly=True, store=True) - acc_holder_name = fields.Char(string='Account Holder Name', help="Account holder name, in case it is different than the name of the Account Holder") + acc_holder_name = fields.Char(string='Account Holder Name', help="Account holder name, in case it is different than the name of the Account Holder", compute='_compute_account_holder_name', readonly=False, store=True) partner_id = fields.Many2one('res.partner', 'Account Holder', ondelete='cascade', index=True, domain=['|', ('is_company', '=', True), ('parent_id', '=', False)], required=True) allow_out_payment = fields.Boolean('Send Money', help='This account can be used for outgoing payments', default=False, copy=False, readonly=False) bank_id = fields.Many2one('res.bank', string='Bank') @@ -102,12 +102,17 @@ class ResPartnerBank(models.Model): for bank in self: bank.acc_type = self.retrieve_acc_type(bank.acc_number) + @api.depends('partner_id') + def _compute_account_holder_name(self): + for bank in self: + bank.acc_holder_name = bank.partner_id.name + @api.model def retrieve_acc_type(self, acc_number): """ To be overridden by subclasses in order to support other account_types. """ return 'bank' - + def name_get(self): return [(acc.id, '{} - {}'.format(acc.acc_number, acc.bank_id.name) if acc.bank_id else acc.acc_number) for acc in self] diff --git a/odoo/addons/base/views/res_bank_views.xml b/odoo/addons/base/views/res_bank_views.xml index 1c6afd7e57c..56aeffa2949 100644 --- a/odoo/addons/base/views/res_bank_views.xml +++ b/odoo/addons/base/views/res_bank_views.xml @@ -87,12 +87,12 @@ - - + + - + @@ -107,10 +107,10 @@ res.partner.bank.tree res.partner.bank - + - +