[IMP] payment_*: improve handling of webhook notifications
Notification handling in some acquirers presents a subset of the following issues: 1. The signature of synchronous notifications (redirect payloads) is not checked. (Alipay, Authorize, Buckaroo, Mollie, PayU money, PayULatam) 2. When the signature check fails, we raise a ValidationError which counts as an HTTP 200 for some providers (it's not the case if they expect a specific string). (Adyen, Paypal, Sips, Stripe) 3. If a ValidationError is raised when processing the feedback data, it is allowed to bubble up to the provider. (Alipay, Ogone) The issues are respectively addressed as follows: 1. If the acquirer implements payments with redirection, make sure that if either makes a request to the provider to validate the data or that it verifies the signature. Verifying the origin of the request is not enough: the payload must be checked too. 2. Instead of raising ValidationError's, raise an HTTP 403 FORBIDDEN error if the signature check fails. 3. Wrap the call to `_handle_feedback_data` of the webhook method inside a try/except clause to catch any ValidationError, log a warning, and acknowledge the notification to avoid having the provider disable the webhook because of too many failures. task-2688139 task-2693293 closes odoo/odoo#81607 Signed-off-by: Antoine Vandevenne (anv) <anv@odoo.com> Co-authored-by: Lucie Van Nieuwenhuyze <luvn@odoo.com>
This commit is contained in:
Antoine Vandevenne (anv)
co-authored by
Lucie Van Nieuwenhuyze
Lucie Van Nieuwenhuyze
parent
ced94b7863
commit
00259dc44a
@@ -1,9 +1,12 @@
|
||||
# Part of Odoo. See LICENSE file for full copyright and licensing details.
|
||||
# Original Copyright 2015 Eezee-It, modified and maintained by Odoo.
|
||||
|
||||
import hmac
|
||||
import logging
|
||||
import pprint
|
||||
|
||||
from werkzeug.exceptions import Forbidden
|
||||
|
||||
from odoo import http
|
||||
from odoo.exceptions import ValidationError
|
||||
from odoo.http import request
|
||||
@@ -12,14 +15,14 @@ _logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class SipsController(http.Controller):
|
||||
_return_url = '/payment/sips/dpn/'
|
||||
_notify_url = '/payment/sips/ipn/'
|
||||
_return_url = '/payment/sips/return/'
|
||||
_webhook_url = '/payment/sips/webhook/'
|
||||
|
||||
@http.route(
|
||||
_return_url, type='http', auth='public', methods=['POST'], csrf=False, save_session=False
|
||||
)
|
||||
def sips_dpn(self, **post):
|
||||
""" Process the data returned by SIPS after redirection.
|
||||
def sips_return_from_checkout(self, **data):
|
||||
""" Process the notification data sent by SIPS after redirection from checkout.
|
||||
|
||||
The route is flagged with `save_session=False` to prevent Odoo from assigning a new session
|
||||
to the user if they are redirected to this route with a POST request. Indeed, as the session
|
||||
@@ -29,47 +32,60 @@ class SipsController(http.Controller):
|
||||
will satisfy any specification of the `SameSite` attribute, the session of the user will be
|
||||
retrieved and with it the transaction which will be immediately post-processed.
|
||||
|
||||
:param dict post: The feedback data to process
|
||||
:param dict data: The notification data
|
||||
"""
|
||||
_logger.info("handling redirection from SIPS with data:\n%s", pprint.pformat(post))
|
||||
try:
|
||||
if self._sips_validate_data(post):
|
||||
request.env['payment.transaction'].sudo()._handle_feedback_data('sips', post)
|
||||
except ValidationError:
|
||||
pass
|
||||
_logger.info("handling redirection from SIPS with data:\n%s", pprint.pformat(data))
|
||||
|
||||
# Check the integrity of the notification
|
||||
tx_sudo = request.env['payment.transaction'].sudo()._get_tx_from_feedback_data(
|
||||
'sips', data
|
||||
)
|
||||
self._verify_notification_signature(data, tx_sudo)
|
||||
|
||||
# Handle the notification data
|
||||
request.env['payment.transaction'].sudo()._handle_feedback_data('sips', data)
|
||||
return request.redirect('/payment/status')
|
||||
|
||||
@http.route(_notify_url, type='http', auth='public', methods=['POST'], csrf=False)
|
||||
def sips_ipn(self, **post):
|
||||
""" Sips IPN. """
|
||||
_logger.info("notification received from SIPS with data:\n%s", pprint.pformat(post))
|
||||
if not post:
|
||||
# SIPS sometimes sends empty notifications, the reason why is unclear but they tend to
|
||||
# pollute logs and do not provide any meaningful information; log as a warning instead
|
||||
# of a traceback.
|
||||
_logger.warning("unable to handle the data; skipping to acknowledge the notification")
|
||||
else:
|
||||
try:
|
||||
if self._sips_validate_data(post):
|
||||
request.env['payment.transaction'].sudo()._handle_feedback_data('sips', post)
|
||||
except ValidationError:
|
||||
pass # Acknowledge the notification to avoid getting spammed
|
||||
@http.route(_webhook_url, type='http', auth='public', methods=['POST'], csrf=False)
|
||||
def sips_webhook(self, **data):
|
||||
""" Process the notification data sent by SIPS to the webhook.
|
||||
|
||||
:param dict data: The notification data
|
||||
:return: An empty string to acknowledge the notification
|
||||
:rtype: str
|
||||
"""
|
||||
_logger.info("notification received from SIPS with data:\n%s", pprint.pformat(data))
|
||||
try:
|
||||
# Check the integrity of the notification
|
||||
tx_sudo = request.env['payment.transaction'].sudo()._get_tx_from_feedback_data(
|
||||
'sips', data
|
||||
)
|
||||
self._verify_notification_signature(data, tx_sudo)
|
||||
|
||||
# Handle the notification data
|
||||
request.env['payment.transaction'].sudo()._handle_feedback_data('sips', data)
|
||||
except ValidationError:
|
||||
_logger.exception("unable to handle the notification data; skipping to acknowledge")
|
||||
return ''
|
||||
|
||||
def _sips_validate_data(self, post):
|
||||
tx_sudo = request.env['payment.transaction'].sudo()._get_tx_from_feedback_data('sips', post)
|
||||
acquirer_sudo = tx_sudo.acquirer_id
|
||||
security = acquirer_sudo._sips_generate_shasign(post['Data'])
|
||||
if security == post['Seal']:
|
||||
_logger.debug(
|
||||
"authenticity of notification data verified for transaction with reference %s",
|
||||
tx_sudo.reference
|
||||
)
|
||||
return True
|
||||
else:
|
||||
_logger.warning(
|
||||
"unable to verify the authenticity of notification data for transaction with "
|
||||
"reference %s",
|
||||
tx_sudo.reference,
|
||||
)
|
||||
return False
|
||||
@staticmethod
|
||||
def _verify_notification_signature(notification_data, tx_sudo):
|
||||
""" Check that the received signature matches the expected one.
|
||||
|
||||
:param dict notification_data: The notification data
|
||||
:param recordset tx_sudo: The sudoed transaction referenced by the notification data, as a
|
||||
`payment.transaction` record
|
||||
:return: None
|
||||
:raise: :class:`werkzeug.exceptions.Forbidden` if the signatures don't match
|
||||
"""
|
||||
# Retrieve the received signature from the data
|
||||
received_signature = notification_data.get('Seal')
|
||||
if not received_signature:
|
||||
_logger.warning("received notification with missing signature")
|
||||
raise Forbidden()
|
||||
|
||||
# Compare the received signature with the expected signature computed from the data
|
||||
expected_signature = tx_sudo.acquirer_id._sips_generate_shasign(notification_data['Data'])
|
||||
if not hmac.compare_digest(received_signature, expected_signature):
|
||||
_logger.warning("received notification with invalid signature")
|
||||
raise Forbidden()
|
||||
|
||||
@@ -66,7 +66,7 @@ class PaymentTransaction(models.Model):
|
||||
'currencyCode': SUPPORTED_CURRENCIES[self.currency_id.name], # The ISO 4217 code
|
||||
'merchantId': self.acquirer_id.sips_merchant_id,
|
||||
'normalReturnUrl': urls.url_join(base_url, SipsController._return_url),
|
||||
'automaticResponseUrl': urls.url_join(base_url, SipsController._notify_url),
|
||||
'automaticResponseUrl': urls.url_join(base_url, SipsController._webhook_url),
|
||||
'transactionReference': self.reference,
|
||||
'statementReference': self.reference,
|
||||
'keyVersion': self.acquirer_id.sips_key_version,
|
||||
@@ -91,8 +91,6 @@ class PaymentTransaction(models.Model):
|
||||
:return: The transaction if found
|
||||
:rtype: recordset of `payment.transaction`
|
||||
:raise: ValidationError if the data match no transaction
|
||||
:raise: ValidationError if the currency is not supported
|
||||
:raise: ValidationError if the amount mismatch
|
||||
"""
|
||||
tx = super()._get_tx_from_feedback_data(provider, data)
|
||||
if provider != 'sips':
|
||||
@@ -111,22 +109,6 @@ class PaymentTransaction(models.Model):
|
||||
"Sips: " + _("No transaction found matching reference %s.", reference)
|
||||
)
|
||||
|
||||
sips_currency = SUPPORTED_CURRENCIES.get(tx.currency_id.name)
|
||||
if not sips_currency:
|
||||
raise ValidationError(
|
||||
"Sips: " + _("This currency is not supported: %s.", tx.currency_id.name)
|
||||
)
|
||||
|
||||
amount_converted = payment_utils.to_major_currency_units(
|
||||
float(data.get('amount', '0.0')), tx.currency_id
|
||||
)
|
||||
if tx.currency_id.compare_amounts(amount_converted, tx.amount) != 0:
|
||||
raise ValidationError(
|
||||
"Sips: " + _(
|
||||
"Incorrect amount: received %(received).2f, expected %(expected).2f",
|
||||
received=amount_converted, expected=tx.amount
|
||||
)
|
||||
)
|
||||
return tx
|
||||
|
||||
def _process_feedback_data(self, data):
|
||||
|
||||
@@ -1,9 +1,32 @@
|
||||
# Part of Odoo. See LICENSE file for full copyright and licensing details.
|
||||
|
||||
from odoo.addons.payment.tests.common import PaymentCommon
|
||||
|
||||
|
||||
class SipsCommon(PaymentCommon):
|
||||
|
||||
NOTIFICATION_DATA = {
|
||||
'Data': 'captureDay=0|captureMode=AUTHOR_CAPTURE|currencyCode=840'
|
||||
'|merchantId=002001000000001|orderChannel=INTERNET|responseCode=00'
|
||||
'|transactionDateTime=2022-01-19T18:01:06+01:00'
|
||||
'|transactionReference=Test Transaction' # Shamefully copy-pasted from payment
|
||||
'|keyVersion=1|acquirerResponseCode=00|amount=10000|authorisationId=12345'
|
||||
'|guaranteeIndicator=Y|cardCSCResultCode=4D|panExpiryDate=202201'
|
||||
'|paymentMeanBrand=VISA|paymentMeanType=CARD|customerIpAddress=111.11.111.11'
|
||||
'|maskedPan=4100##########00|returnContext={"reference": "Test Transaction"}'
|
||||
'|holderAuthentRelegation=N|holderAuthentStatus=3D_SUCCESS'
|
||||
'|tokenPan=dp528b9xwknujmkw|transactionOrigin=INTERNET|paymentPattern=ONE_SHOT'
|
||||
'|customerMobilePhone=null|mandateAuthentMethod=null|mandateUsage=null'
|
||||
'|transactionActors=null|mandateId=null|captureLimitDate=20220119|dccStatus=null'
|
||||
'|dccResponseCode=null|dccAmount=null|dccCurrencyCode=null|dccExchangeRate=null'
|
||||
'|dccExchangeRateValidity=null|dccProvider=null|statementReference=tx20220119170050'
|
||||
'|panEntryMode=MANUAL|walletType=null|holderAuthentMethod=NOT_SPECIFIED',
|
||||
'Encode': '',
|
||||
'InterfaceVersion': 'HP_2.4',
|
||||
'Seal': '8a4c1f8b268832600a7bf40ddaa5d487f07d61dea81b8119ab8bab3c8a0861f3',
|
||||
'locale': 'en',
|
||||
}
|
||||
|
||||
@classmethod
|
||||
def setUpClass(cls, chart_template_ref=None):
|
||||
super().setUpClass(chart_template_ref=chart_template_ref)
|
||||
|
||||
@@ -1,19 +1,23 @@
|
||||
# Part of Odoo. See LICENSE file for full copyright and licensing details.
|
||||
|
||||
import json
|
||||
from unittest.mock import patch
|
||||
|
||||
from freezegun import freeze_time
|
||||
from werkzeug.exceptions import Forbidden
|
||||
|
||||
from odoo.exceptions import ValidationError
|
||||
from odoo.tests import tagged
|
||||
from odoo.tools import mute_logger
|
||||
|
||||
from .common import SipsCommon
|
||||
from ..controllers.main import SipsController
|
||||
from ..models.payment_acquirer import SUPPORTED_CURRENCIES
|
||||
from odoo.addons.payment.tests.http_common import PaymentHttpCommon
|
||||
from odoo.addons.payment_sips.controllers.main import SipsController
|
||||
from odoo.addons.payment_sips.models.payment_acquirer import SUPPORTED_CURRENCIES
|
||||
from odoo.addons.payment_sips.tests.common import SipsCommon
|
||||
|
||||
|
||||
@tagged('post_install', '-at_install')
|
||||
class SipsTest(SipsCommon):
|
||||
class SipsTest(SipsCommon, PaymentHttpCommon):
|
||||
|
||||
def test_compatible_acquirers(self):
|
||||
for curr in SUPPORTED_CURRENCIES:
|
||||
@@ -51,71 +55,85 @@ class SipsTest(SipsCommon):
|
||||
self.assertEqual(form_info['action'], self.sips.sips_test_url)
|
||||
self.assertEqual(form_inputs['InterfaceVersion'], self.sips.sips_version)
|
||||
return_url = self._build_url(SipsController._return_url)
|
||||
notify_url = self._build_url(SipsController._notify_url)
|
||||
self.assertEqual(form_inputs['Data'],
|
||||
f'amount=111111|currencyCode=978|merchantId=dummy_mid|normalReturnUrl={return_url}|' \
|
||||
f'automaticResponseUrl={notify_url}|transactionReference={self.reference}|' \
|
||||
f'statementReference={self.reference}|keyVersion={self.sips.sips_key_version}|' \
|
||||
f'returnContext={json.dumps(dict(reference=self.reference))}'
|
||||
notify_url = self._build_url(SipsController._webhook_url)
|
||||
self.assertEqual(
|
||||
form_inputs['Data'],
|
||||
f'amount=111111|currencyCode=978|merchantId=dummy_mid|normalReturnUrl={return_url}|'
|
||||
f'automaticResponseUrl={notify_url}|transactionReference={self.reference}|'
|
||||
f'statementReference={self.reference}|keyVersion={self.sips.sips_key_version}|'
|
||||
f'returnContext={json.dumps(dict(reference=self.reference))}',
|
||||
)
|
||||
self.assertEqual(
|
||||
form_inputs['Seal'], '99d1d2d46a841de7fe313ac0b2d13a9e42cad50b444d35bf901879305818d9b2'
|
||||
)
|
||||
self.assertEqual(form_inputs['Seal'],
|
||||
'4d7cc67c0168e8ce11c25fbe1937231c644861e320702ab544022b032b9eb4a2')
|
||||
|
||||
def test_feedback_processing(self):
|
||||
# typical data posted by Sips after client has successfully paid
|
||||
sips_post_data = {
|
||||
'Data': 'captureDay=0|captureMode=AUTHOR_CAPTURE|currencyCode=840|'
|
||||
'merchantId=002001000000001|orderChannel=INTERNET|'
|
||||
'responseCode=00|transactionDateTime=2020-04-08T06:15:59+02:00|'
|
||||
'transactionReference=SO100x1|keyVersion=1|'
|
||||
'acquirerResponseCode=00|amount=31400|authorisationId=0020000006791167|'
|
||||
'paymentMeanBrand=IDEAL|paymentMeanType=CREDIT_TRANSFER|'
|
||||
'customerIpAddress=127.0.0.1|returnContext={"return_url": '
|
||||
'"/payment/process", "reference": '
|
||||
'"SO100x1"}|holderAuthentRelegation=N|holderAuthentStatus=|'
|
||||
'transactionOrigin=INTERNET|paymentPattern=ONE_SHOT|customerMobilePhone=null|'
|
||||
'mandateAuthentMethod=null|mandateUsage=null|transactionActors=null|'
|
||||
'mandateId=null|captureLimitDate=20200408|dccStatus=null|dccResponseCode=null|'
|
||||
'dccAmount=null|dccCurrencyCode=null|dccExchangeRate=null|'
|
||||
'dccExchangeRateValidity=null|dccProvider=null|'
|
||||
'statementReference=SO100x1|panEntryMode=MANUAL|walletType=null|'
|
||||
'holderAuthentMethod=NO_AUTHENT_METHOD',
|
||||
'Encode': '',
|
||||
'InterfaceVersion': 'HP_2.4',
|
||||
'Seal': 'f03f64da6f57c171904d12bf709b1d6d3385131ac914e97a7e1db075ed438f3e',
|
||||
'locale': 'en',
|
||||
}
|
||||
# Unknown transaction
|
||||
with self.assertRaises(ValidationError):
|
||||
self.env['payment.transaction']._handle_feedback_data('sips', self.NOTIFICATION_DATA)
|
||||
|
||||
with self.assertRaises(ValidationError): # unknown transaction
|
||||
self.env['payment.transaction']._handle_feedback_data('sips', sips_post_data)
|
||||
# Confirmed transaction
|
||||
tx = self.create_transaction('redirect')
|
||||
self.env['payment.transaction']._handle_feedback_data('sips', self.NOTIFICATION_DATA)
|
||||
self.assertEqual(tx.state, 'done')
|
||||
self.assertEqual(tx.acquirer_reference, self.reference)
|
||||
|
||||
self.amount = 314.0
|
||||
self.reference = 'SO100x1'
|
||||
# Cancelled transaction
|
||||
old_reference = self.reference
|
||||
self.reference = 'Test Transaction 2'
|
||||
tx = self.create_transaction('redirect')
|
||||
payload = dict(
|
||||
self.NOTIFICATION_DATA,
|
||||
Data=self.NOTIFICATION_DATA['Data'].replace(old_reference, self.reference)
|
||||
.replace('responseCode=00', 'responseCode=12')
|
||||
)
|
||||
self.env['payment.transaction']._handle_feedback_data('sips', payload)
|
||||
self.assertEqual(tx.state, 'cancel')
|
||||
|
||||
tx = self.create_transaction(flow="redirect")
|
||||
@mute_logger('odoo.addons.payment_sips.controllers.main')
|
||||
def test_webhook_notification_confirms_transaction(self):
|
||||
""" Test the processing of a webhook notification. """
|
||||
tx = self.create_transaction('redirect')
|
||||
url = self._build_url(SipsController._return_url)
|
||||
with patch(
|
||||
'odoo.addons.payment_sips.controllers.main.SipsController'
|
||||
'._verify_notification_signature'
|
||||
):
|
||||
self._make_http_post_request(url, data=self.NOTIFICATION_DATA)
|
||||
self.assertEqual(tx.state, 'done')
|
||||
|
||||
# Validate the transaction
|
||||
self.env['payment.transaction']._handle_feedback_data('sips', sips_post_data)
|
||||
self.assertEqual(tx.state, 'done', 'Sips: validation did not put tx into done state')
|
||||
self.assertEqual(tx.acquirer_reference, self.reference, 'Sips: validation did not update tx id')
|
||||
@mute_logger('odoo.addons.payment_sips.controllers.main')
|
||||
def test_webhook_notification_triggers_signature_check(self):
|
||||
""" Test that receiving a webhook notification triggers a signature check. """
|
||||
self.create_transaction('redirect')
|
||||
url = self._build_url(SipsController._webhook_url)
|
||||
with patch(
|
||||
'odoo.addons.payment_sips.controllers.main.SipsController'
|
||||
'._verify_notification_signature'
|
||||
) as signature_check_mock, patch(
|
||||
'odoo.addons.payment.models.payment_transaction.PaymentTransaction'
|
||||
'._handle_feedback_data'
|
||||
):
|
||||
self._make_http_post_request(url, data=self.NOTIFICATION_DATA)
|
||||
self.assertEqual(signature_check_mock.call_count, 1)
|
||||
|
||||
# same process for an payment in error on sips's end
|
||||
sips_post_data = {
|
||||
'Data': 'captureDay=0|captureMode=AUTHOR_CAPTURE|currencyCode=840|'
|
||||
'merchantId=002001000000001|orderChannel=INTERNET|responseCode=12|'
|
||||
'transactionDateTime=2020-04-08T06:24:08+02:00|transactionReference=SO100x2|'
|
||||
'keyVersion=1|amount=31400|customerIpAddress=127.0.0.1|returnContext={"return_url": '
|
||||
'"/payment/process", "reference": '
|
||||
'"SO100x2"}|paymentPattern=ONE_SHOT|customerMobilePhone=null|mandateAuthentMethod=null|'
|
||||
'mandateUsage=null|transactionActors=null|mandateId=null|captureLimitDate=null|'
|
||||
'dccStatus=null|dccResponseCode=null|dccAmount=null|dccCurrencyCode=null|'
|
||||
'dccExchangeRate=null|dccExchangeRateValidity=null|dccProvider=null|'
|
||||
'statementReference=SO100x2|panEntryMode=null|walletType=null|holderAuthentMethod=null',
|
||||
'InterfaceVersion': 'HP_2.4',
|
||||
'Seal': '6e1995ea5432580860a04d8515b6eb1507996f97b3c5fa04fb6d9568121a16a2'
|
||||
}
|
||||
self.reference = 'SO100x2'
|
||||
tx2 = self.create_transaction(flow="redirect")
|
||||
def test_accept_notification_with_valid_signature(self):
|
||||
""" Test the verification of a notification with a valid signature. """
|
||||
tx = self.create_transaction('redirect')
|
||||
self._assert_does_not_raise(
|
||||
Forbidden, SipsController._verify_notification_signature, self.NOTIFICATION_DATA, tx
|
||||
)
|
||||
|
||||
self.env['payment.transaction']._handle_feedback_data('sips', sips_post_data)
|
||||
self.assertEqual(tx2.state, 'cancel', 'Sips: erroneous validation did not put tx into error state')
|
||||
@mute_logger('odoo.addons.payment_sips.controllers.main')
|
||||
def test_reject_notification_with_missing_signature(self):
|
||||
""" Test the verification of a notification with a missing signature. """
|
||||
tx = self.create_transaction('redirect')
|
||||
payload = dict(self.NOTIFICATION_DATA, Seal=None)
|
||||
self.assertRaises(Forbidden, SipsController._verify_notification_signature, payload, tx)
|
||||
|
||||
@mute_logger('odoo.addons.payment_sips.controllers.main')
|
||||
def test_reject_notification_with_invalid_signature(self):
|
||||
""" Test the verification of a notification with an invalid signature. """
|
||||
tx = self.create_transaction('redirect')
|
||||
payload = dict(self.NOTIFICATION_DATA, Seal='dummy')
|
||||
self.assertRaises(Forbidden, SipsController._verify_notification_signature, payload, tx)
|
||||
|
||||
Reference in New Issue
Block a user