From c1b914188a3a3cb45d36c020c1e4aa3cf79aec6f Mon Sep 17 00:00:00 2001 From: "Antoine Vandevenne (anv)" Date: Fri, 21 Jan 2022 11:36:46 +0000 Subject: [PATCH] [FIX] payment_buckaroo: sort signing string's data keys in lower-case Before this commit, the implementation of Buckaroo's method for computing the signature of a dataset was upper-casing data keys before sorting them, in order for the sort to be done in a case-insensitive manner. The issue is that, as `ord('A') < ord('_') < ord('a')`, two keys could be incorrectly swapped if they shared the same prefix and one of the two contained an underscore. For example, `brq_transactions` would be appended to the signing string before `brq_transaction_type` whereas, if the sort was based on the upper-case keys, the second would be appended before the first. This commit changes the computation of the signature to rely on the lower-case keys rather than upper-case keys in order to use the same method as Buckaroo when they compute the signature on their end. closes odoo/odoo#83202 X-original-commit: 0112d69252a8a8396f9509e240c741fa2e261fb3 Signed-off-by: Victor Feyens (vfe) --- .../models/payment_acquirer.py | 4 ++-- addons/payment_buckaroo/tests/common.py | 2 +- .../payment_buckaroo/tests/test_buckaroo.py | 21 ++++++++++++++++--- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/addons/payment_buckaroo/models/payment_acquirer.py b/addons/payment_buckaroo/models/payment_acquirer.py index 4968b1136f3..099c74501e5 100644 --- a/addons/payment_buckaroo/models/payment_acquirer.py +++ b/addons/payment_buckaroo/models/payment_acquirer.py @@ -55,8 +55,8 @@ class PaymentAcquirer(models.Model): (k, v) for k, v in values.items() if any(k.upper().startswith(key_prefix) for key_prefix in ('ADD_', 'BRQ_', 'CUST_')) ] - # Sort parameters by upper-cased key name - sorted_items = sorted(items, key=lambda pair: pair[0].upper()) + # Sort parameters by lower-cased key. Not upper- because ord('A') < ord('_') < ord('a'). + sorted_items = sorted(items, key=lambda pair: pair[0].lower()) # Build the signing string by concatenating all parameters sign_string = ''.join(f'{k}={v or ""}' for k, v in sorted_items) # Append the pre-shared secret key to the signing string diff --git a/addons/payment_buckaroo/tests/common.py b/addons/payment_buckaroo/tests/common.py index bb1e9583096..9b1783486cb 100644 --- a/addons/payment_buckaroo/tests/common.py +++ b/addons/payment_buckaroo/tests/common.py @@ -11,7 +11,7 @@ class BuckarooCommon(PaymentCommon): cls.buckaroo = cls._prepare_acquirer('buckaroo', update_values={ 'buckaroo_website_key': 'dummy', - 'buckaroo_secret_key': 'dummy', + 'buckaroo_secret_key': 'test_key_123', }) # Override defaults diff --git a/addons/payment_buckaroo/tests/test_buckaroo.py b/addons/payment_buckaroo/tests/test_buckaroo.py index 32b05113948..855dd106920 100644 --- a/addons/payment_buckaroo/tests/test_buckaroo.py +++ b/addons/payment_buckaroo/tests/test_buckaroo.py @@ -18,7 +18,7 @@ class BuckarooTest(BuckarooCommon): 'Brq_amount': str(self.amount), 'Brq_currency': self.currency.name, 'Brq_invoicenumber': self.reference, - 'Brq_signature': '04c26578116990496770687a8bf225200e0f56e6', + 'Brq_signature': '669d4f64ea9cbb58cfefba9b802389667e4eef39', 'Brq_return': return_url, 'Brq_returncancel': return_url, 'Brq_returnerror': return_url, @@ -54,7 +54,7 @@ class BuckarooTest(BuckarooCommon): 'BRQ_SERVICE_PAYPAL_PAYERLASTNAME': u'Tester', 'BRQ_SERVICE_PAYPAL_PAYERMIDDLENAME': u'de', 'BRQ_SERVICE_PAYPAL_PAYERSTATUS': u'verified', - 'Brq_signature': u'e439f3af06b9752197631715628d6a198a25900f', + 'Brq_signature': u'e67e32ee1be1030a86c7764adfcc01856e00f9a7', 'BRQ_STATUSCODE': u'190', 'BRQ_STATUSCODE_DETAIL': u'S001', 'BRQ_STATUSMESSAGE': u'Transaction successfully processed', @@ -86,7 +86,7 @@ class BuckarooTest(BuckarooCommon): # simulate an error buckaroo_post_data['BRQ_STATUSCODE'] = '2' - buckaroo_post_data['Brq_signature'] = 'b8e54e26b2b5a5e697b8ed5085329ea712fd48b2' + buckaroo_post_data['Brq_signature'] = '3e67da5181b1a895d322987303e42bab2a376eec' # Avoid warning log bc of unknown status code with mute_logger('odoo.addons.payment_buckaroo.models.payment_transaction'): @@ -94,3 +94,18 @@ class BuckarooTest(BuckarooCommon): self.assertEqual(tx.state, 'error', 'Buckaroo: unexpected status code should put tx in error state') + + def test_signature_is_computed_based_on_lower_case_data_keys(self): + """ Test that lower case keys are used to execute the case-insensitive sort. """ + computed_signature = self.acquirer._buckaroo_generate_digital_sign({ + 'brq_a': '1', + 'brq_b': '2', + 'brq_c_first': '3', + 'brq_csecond': '4', + 'brq_D': '5', + }, incoming=False) + self.assertEqual( + computed_signature, + '937cca8f486b75e93df1e9811a5ebf43357fc3f2', + msg="The signing string items should be ordered based on a lower-case copy of the keys", + )