From 2ee4251d0740fde12a8822cc8977793c4bf977c5 Mon Sep 17 00:00:00 2001 From: Laura Schauer Date: Mon, 20 Jun 2022 13:20:39 +0000 Subject: [PATCH] [IMP] payment(_adyen, _authorize): disable tokens of disabled acquirers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this commit, zombie payment tokens (= tokens linked to disabled acquirers) could still 1) be used by internal users and 2) reactivated when the acquirer’s state changed to ‘test’ or ‘enabled’. This is not desirable because zombie tokens should neither be used, nor reactivated. After this commit, all tokens related to an acquirer are unassigned from linked documents and archived as soon as the acquirer’s state is changed to ‘disabled’. Creating a payment with an archived token is prohibited. In addition, archived tokens cannot be un-archived anymore. task-2649806 closes odoo/odoo#93774 Related: odoo/enterprise#28661 Signed-off-by: Antoine Vandevenne (anv) --- addons/payment/models/payment_acquirer.py | 44 ++++++++++++ addons/payment/models/payment_token.py | 52 +++++--------- addons/payment/models/payment_transaction.py | 72 ++++++++++++------- addons/payment/tests/common.py | 1 + addons/payment/tests/test_payment_token.py | 16 +++++ addons/payment_adyen/models/payment_token.py | 43 ----------- addons/payment_adyen/tests/test_adyen.py | 6 -- .../payment_authorize/models/payment_token.py | 27 ------- .../payment_authorize/tests/test_authorize.py | 6 -- addons/payment_ogone/models/payment_token.py | 18 ----- 10 files changed, 124 insertions(+), 161 deletions(-) create mode 100644 addons/payment/tests/test_payment_token.py diff --git a/addons/payment/models/payment_acquirer.py b/addons/payment/models/payment_acquirer.py index 3f4c7583877..590727e2ab8 100644 --- a/addons/payment/models/payment_acquirer.py +++ b/addons/payment/models/payment_acquirer.py @@ -233,6 +233,35 @@ class PaymentAcquirer(models.Model): self.ensure_one() return self.env.ref('account.account_payment_method_manual_in').id + #=== ONCHANGE METHODS ===# + + @api.onchange('state') + def _onchange_state(self): + """ Display a warning about the consequences of disabling an acquirer. + + Let the user know that tokens related to an acquirer get archived if it is disabled or if + its state is changed from 'test' to 'enabled' and vice versa. + + :return: The warning message in a client action. + :rtype: dict + """ + self.ensure_one() + + if self._origin.state in ('test', 'enabled') and self._origin.state != self.state: + related_tokens = self.env['payment.token'].search( + [('acquirer_id', '=', self._origin.id)] + ) + if related_tokens: + return { + 'warning': { + 'title': _("Warning"), + 'message': _( + "This action will also archive %s tokens that are registered with this " + "acquirer. Archiving tokens is irreversible.", len(related_tokens) + ) + } + } + #=== CONSTRAINT METHODS ===# @api.constrains('fees_dom_var', 'fees_int_var') @@ -257,8 +286,16 @@ class PaymentAcquirer(models.Model): return acquirers def write(self, values): + # Handle acquirer disabling. + if 'state' in values: + state_changed_acquirers = self.filtered( + lambda acq: acq.state not in ('disabled', values['state']) + ) # Don't handle acquirers being enabled or whose state is not updated. + state_changed_acquirers._handle_state_change() + result = super().write(values) self._check_required_if_provider() + return result def _check_required_if_provider(self): @@ -287,6 +324,13 @@ class PaymentAcquirer(models.Model): _("The following fields must be filled: %s", ", ".join(field_names)) ) + def _handle_state_change(self): + """ Archive all the payment tokens linked to these acquirers. + + :return: None + """ + self.env['payment.token'].search([('acquirer_id', 'in', self.ids)]).write({'active': False}) + #=== ACTION METHODS ===# def button_immediate_install(self): diff --git a/addons/payment/models/payment_token.py b/addons/payment/models/payment_token.py index e9c2b408e4d..f9b958d23af 100644 --- a/addons/payment/models/payment_token.py +++ b/addons/payment/models/payment_token.py @@ -2,7 +2,8 @@ import logging -from odoo import api, fields, models +from odoo import _, api, fields, models +from odoo.exceptions import UserError _logger = logging.getLogger(__name__) @@ -60,54 +61,33 @@ class PaymentToken(models.Model): return dict() def write(self, values): - """ Delegate the handling of active state switch to dedicated methods. + """ Prevent unarchiving tokens and handle their archiving. - Unless an exception is raised in the handling methods, the toggling proceeds no matter what. - This is because allowing users to hide their saved payment methods comes before making sure - that the recorded payment details effectively get deleted. - - :return: The result of the write + :return: The result of the call to the parent method. :rtype: bool + :raise UserError: If at least one token is being unarchived. """ - # Let acquirers handle activation/deactivation requests if 'active' in values: - for token in self: - # Call handlers in sudo mode because this method might have been called by RPC - if values['active'] and not token.active: - token.sudo()._handle_reactivation_request() - elif not values['active'] and token.active: - token.sudo()._handle_deactivation_request() + if values['active']: + if any(not token.active for token in self): + raise UserError(_("A token cannot be unarchived once it has been archived.")) + else: + # Call the handlers in sudo mode because this method might have been called by RPC. + self.filtered('active').sudo()._handle_archiving() - # Proceed with the toggling of the active state return super().write(values) #=== BUSINESS METHODS ===# - def _handle_deactivation_request(self): - """ Handle the request for deactivation of the token. + def _handle_archiving(self): + """ Handle the archiving of the current tokens. - For an acquirer to support deactivation of tokens, or perform additional operations when a - token is deactivated, it must overwrite this method and raise an UserError if the token - cannot be deactivated. - - Note: self.ensure_one() + For a module to perform additional operations when a token is archived, it must override + this method. :return: None """ - self.ensure_one() - - def _handle_reactivation_request(self): - """ Handle the request for reactivation of the token. - - For an acquirer to support reactivation of tokens, or perform additional operations when a - token is reactivated, it must overwrite this method and raise an UserError if the token - cannot be reactivated. - - Note: self.ensure_one() - - :return: None - """ - self.ensure_one() + return None def get_linked_records_info(self): """ Return a list of information about records linked to the current token. diff --git a/addons/payment/models/payment_transaction.py b/addons/payment/models/payment_transaction.py index fd2b0f59564..b1eae1c03a0 100644 --- a/addons/payment/models/payment_transaction.py +++ b/addons/payment/models/payment_transaction.py @@ -10,7 +10,7 @@ import psycopg2 from dateutil import relativedelta from odoo import _, api, fields, models -from odoo.exceptions import ValidationError +from odoo.exceptions import UserError, ValidationError from odoo.tools import consteq, format_amount, ustr from odoo.tools.misc import hmac as hmac_tool @@ -174,6 +174,12 @@ class PaymentTransaction(models.Model): ', '.join(set(illegal_authorize_state_txs.mapped('acquirer_id.name'))) )) + @api.constrains('token_id') + def _check_token_is_active(self): + """ Check that the token used to create the transaction is active. """ + if self.token_id and not self.token_id.active: + raise ValidationError(_("Creating a transaction from an archived token is forbidden.")) + #=== CRUD METHODS ===# @api.model_create_multi @@ -547,6 +553,7 @@ class PaymentTransaction(models.Model): :return: None """ self.ensure_one() + self._ensure_acquirer_is_not_disabled() self._log_sent_message() def _send_refund_request(self, amount_to_refund=None, create_refund_transaction=True): @@ -563,6 +570,7 @@ class PaymentTransaction(models.Model): :rtype: recordset of `payment.transaction` """ self.ensure_one() + self._ensure_acquirer_is_not_disabled() if create_refund_transaction: refund_tx = self._create_refund_transaction(amount_to_refund=amount_to_refund) @@ -571,30 +579,6 @@ class PaymentTransaction(models.Model): else: return self.env['payment.transaction'] - def _send_capture_request(self): - """ Request the provider of the acquirer handling the transaction to capture it. - - For an acquirer to support authorization, it must override this method and request a capture - to its provider. - - Note: self.ensure_one() - - :return: None - """ - self.ensure_one() - - def _send_void_request(self): - """ Request the provider of the acquirer handling the transaction to void it. - - For an acquirer to support authorization, it must override this method and request the - transaction to be voided to its provider. - - Note: self.ensure_one() - - :return: None - """ - self.ensure_one() - def _create_refund_transaction(self, amount_to_refund=None, **custom_create_values): """ Create a new transaction with operation 'refund' and link it to the current transaction. @@ -617,6 +601,44 @@ class PaymentTransaction(models.Model): **custom_create_values, }) + def _send_capture_request(self): + """ Request the provider of the acquirer handling the transaction to capture it. + + For an acquirer to support authorization, it must override this method and request a capture + to its provider. + + Note: self.ensure_one() + + :return: None + """ + self.ensure_one() + self._ensure_acquirer_is_not_disabled() + + def _send_void_request(self): + """ Request the provider of the acquirer handling the transaction to void it. + + For an acquirer to support authorization, it must override this method and request the + transaction to be voided to its provider. + + Note: self.ensure_one() + + :return: None + """ + self.ensure_one() + self._ensure_acquirer_is_not_disabled() + + def _ensure_acquirer_is_not_disabled(self): + """ Ensure that the acquirer's state is not 'disabled' before sending a request to its + provider. + + :return: None + :raise UserError: If the acquirer's state is 'disabled'. + """ + if self.acquirer_id.state == 'disabled': + raise UserError(_( + "Making a request to the provider is not possible because the acquirer is disabled." + )) + def _handle_notification_data(self, provider, notification_data): """ Match the transaction with the notification data, update its state and return it. diff --git a/addons/payment/tests/common.py b/addons/payment/tests/common.py index 85b5831e4ab..940847c25bd 100644 --- a/addons/payment/tests/common.py +++ b/addons/payment/tests/common.py @@ -192,6 +192,7 @@ class PaymentCommon(AccountTestInvoicingCommon): 'acquirer_id': self.acquirer.id, 'partner_id': self.partner.id, 'acquirer_ref': "Acquirer Ref (TEST)", + 'active': True, } return self.env['payment.token'].sudo(sudo).create(dict(default_values, **values)) diff --git a/addons/payment/tests/test_payment_token.py b/addons/payment/tests/test_payment_token.py new file mode 100644 index 00000000000..c788cb9ebc6 --- /dev/null +++ b/addons/payment/tests/test_payment_token.py @@ -0,0 +1,16 @@ +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo.exceptions import UserError +from odoo.tests import tagged + +from odoo.addons.payment.tests.common import PaymentCommon + + +@tagged('-at_install', 'post_install') +class TestPaymentToken(PaymentCommon): + + def test_token_cannot_be_unarchived(self): + """ Test that unarchiving disabled tokens is forbidden. """ + token = self._create_token(active=False) + with self.assertRaises(UserError): + token.active = True diff --git a/addons/payment_adyen/models/payment_token.py b/addons/payment_adyen/models/payment_token.py index d885eacf938..b72a8594943 100644 --- a/addons/payment_adyen/models/payment_token.py +++ b/addons/payment_adyen/models/payment_token.py @@ -1,7 +1,6 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from odoo import _, fields, models -from odoo.exceptions import UserError, ValidationError class PaymentToken(models.Model): @@ -10,45 +9,3 @@ class PaymentToken(models.Model): adyen_shopper_reference = fields.Char( string="Shopper Reference", help="The unique reference of the partner owning this token", readonly=True) - - #=== BUSINESS METHODS ===# - - def _handle_deactivation_request(self): - """ Override of payment to request request Adyen to delete the token. - - Note: self.ensure_one() - - :return: None - """ - super()._handle_deactivation_request() - if self.provider != 'adyen': - return - - data = { - 'merchantAccount': self.acquirer_id.adyen_merchant_account, - 'shopperReference': self.adyen_shopper_reference, - 'recurringDetailReference': self.acquirer_ref, - } - try: - self.acquirer_id._adyen_make_request( - url_field_name='adyen_recurring_api_url', - endpoint='/disable', - payload=data, - method='POST' - ) - except ValidationError: - pass # Deactivating the token in Odoo is more important than in Adyen - - def _handle_reactivation_request(self): - """ Override of payment to raise an error informing that Adyen tokens cannot be restored. - - Note: self.ensure_one() - - :return: None - :raise: UserError if the token is managed by Adyen - """ - super()._handle_reactivation_request() - if self.provider != 'adyen': - return - - raise UserError(_("Saved payment methods cannot be restored once they have been deleted.")) diff --git a/addons/payment_adyen/tests/test_adyen.py b/addons/payment_adyen/tests/test_adyen.py index 881d225e36c..5d3b7e69029 100644 --- a/addons/payment_adyen/tests/test_adyen.py +++ b/addons/payment_adyen/tests/test_adyen.py @@ -39,12 +39,6 @@ class AdyenTest(AdyenCommon, PaymentHttpCommon): processing_values['access_token'], self.reference, converted_amount, self.partner.id )) - def test_token_activation(self): - """Activation of disabled adyen tokens is forbidden""" - token = self._create_token(active=False) - with self.assertRaises(UserError): - token._handle_reactivation_request() - @mute_logger('odoo.addons.payment_adyen.models.payment_transaction') def test_send_refund_request(self): self.acquirer.support_refund = 'full_only' # Should simply not be False diff --git a/addons/payment_authorize/models/payment_token.py b/addons/payment_authorize/models/payment_token.py index 214d972bb01..89a3a1e513c 100644 --- a/addons/payment_authorize/models/payment_token.py +++ b/addons/payment_authorize/models/payment_token.py @@ -23,30 +23,3 @@ class PaymentToken(models.Model): selection=[("credit_card", "Credit Card"), ("bank_account", "Bank Account (USA Only)")], ) - def _handle_deactivation_request(self): - """ Override of payment to request Authorize.Net to delete the token. - - Note: self.ensure_one() - - :return: None - """ - super()._handle_deactivation_request() - if self.provider != 'authorize': - return - - authorize_API = AuthorizeAPI(self.acquirer_id) - res_content = authorize_API.delete_customer_profile(self.authorize_profile) - _logger.info("delete_customer_profile request response:\n%s", pprint.pformat(res_content)) - - def _handle_reactivation_request(self): - """ Override of payment to raise an error informing that Auth.net tokens cannot be restored. - - Note: self.ensure_one() - - :return: None - """ - super()._handle_reactivation_request() - if self.provider != 'authorize': - return - - raise UserError(_("Saved payment methods cannot be restored once they have been deleted.")) diff --git a/addons/payment_authorize/tests/test_authorize.py b/addons/payment_authorize/tests/test_authorize.py index 9b90542ee72..548ce3499dc 100644 --- a/addons/payment_authorize/tests/test_authorize.py +++ b/addons/payment_authorize/tests/test_authorize.py @@ -49,12 +49,6 @@ class AuthorizeTest(AuthorizeCommon): self.assertEqual(self.authorize._get_validation_amount(), 0.01) self.assertEqual(self.authorize._get_validation_currency(), self.currency_usd) - def test_token_activation(self): - """Activation of disabled authorize tokens is forbidden""" - token = self._create_token(active=False) - with self.assertRaises(UserError): - token._handle_reactivation_request() - def test_authorize_neutralize(self): self.env['payment.acquirer']._neutralize() diff --git a/addons/payment_ogone/models/payment_token.py b/addons/payment_ogone/models/payment_token.py index 40b89deb9ce..75bb4cb2129 100644 --- a/addons/payment_ogone/models/payment_token.py +++ b/addons/payment_ogone/models/payment_token.py @@ -6,21 +6,3 @@ from odoo.exceptions import UserError class PaymentToken(models.Model): _inherit = 'payment.token' - - def _handle_reactivation_request(self): - """ Override of payment to raise an error informing that Ogone tokens cannot be restored. - - More specifically, permanents tokens are never deleted in Ogone's backend but we don't - distinguish them from temporary tokens which are archived at creation time. So we simply - block the reactivation of every token. - - Note: self.ensure_one() - - :return: None - :raise: UserError if the token is managed by Ogone - """ - super()._handle_reactivation_request() - if self.provider != 'ogone': - return - - raise UserError(_("Saved payment methods cannot be restored once they have been archived."))