[ADD] pos_mercado_pago: integration of Mercado Pago payment terminal

[papyDoctor]

This PR add the Mercado Pago "Smart Point" payment terminal dedicated to the LATAM (Latin America) region

task-3350386

mool

---

I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

[robodoo]

Pull request status dashboard.

[davidmonnom]

First check but LGTM 👍

Will see with security team for secu review.

BTW, be carefull because endpoint URL contains external input / not hardcoded input

[Ysoroko]

Good stuff!
Need to check together for some things as it looks to me like lots of things can be simplified

1. It seems like we're saving a lot of potentially unnecessary information (although it does look useful to the user). Maybe we can find a way to reduce the number of fields in the model
2. Same for some requests, although these seem pretty reduced
3. Need to check how to simplify the weird JS loop in addons/pos_mercado_pago/static/src/app/payment_mercado_pago.js

Will discuss this together next week

[qle-odoo]

@papyDoctor you can ping security team and explai nwhy your PR is safe.
Check all warning and explain why in your case it is ok and security can override the flag
https://runbot.odoo.com/runbot/build/61010219

[papyDoctor]

@odoo/rd-security
This module address a new payment method for the POS: Mercado Pago
The module needs to deal with some security risks on two files:

1. The file main.py receives a webhook from Mercado Pago. In order to verify the sender authenticity, Mercado Pago uses a timestamped signature that we have to verify using hmac and hashlib. Also, we need to use the _sendone() from the request lib to notify the frontend but no informations are transmitted.
2. The file mercado_pago_pos_request.py needs the requests lib to deals with Mercado Pago api, the address of which is hardcoded.

Can you review the code on these files,
Thank you

[xmo-odoo]

A few notes.

[xmo-odoo]

Why point_smart in device.get('id')?

• it'll raise an exception if device does not have an id
• assuming id is not a list, is it normal that we are looking up devices by substring rather than exact serial number, and might thus get multiple matches?

[papyDoctor]

• If device exists, it always has an id
• The exact serial number is not known by the user, only part of it
  (example: serial number written on the back of the terminal: 14941269XX, exact serial number: PAX_A910__SMARTPOS14941269XX)
  No more than one can exist unless user enter an incomplete SN, in that case we take the first in the returned list

[xmo-odoo]

• If device exists, it always has an id

Then why is the code using dict.get?

[papyDoctor]

Ok, replaced with device['id']

[xmo-odoo]

So if an mp_bearer_token is configured we update the mp_id_point_smart on every write, for some reason unless we're touching the name?

And self.mp_id_point_smart should already have been validated previously, why are we re-validating it on every write?

[papyDoctor]

First question: Yes, but I figured out that this logic is better:
if any(elem in vals for elem in['mp_id_point_smart', 'mp_bearer_token']):

Second question: each time user changes one (or the two) of the the two fields above, we have to recheck with Mercado data:

• user change the bearer_token: check if correct
• user change the id_point_smart: check to get the correct complete one

[xmo-odoo]

But since the entire thing is in an if self.mp_bearer_token: it ignors the case where we would be setting the bearer token no?

[papyDoctor]

To make things clearer but also to avoid infinite loop in the write I've added a new field "mp_id_point_smart_complet" that contains the complete id retrieved from Mercado Pago. It is used in the code instead of "mp_id_point_smart". The field "mp_id_point_smart" (the one entered by the user) is used only for seeking the complete one:

   def write(self, vals):
        records = super().write(vals)

        if 'mp_id_point_smart' in vals or 'mp_bearer_token' in vals:
            self.mp_id_point_smart_complet = self._find_terminal(self.mp_bearer_token, self.mp_id_point_smart)

        return records

[papyDoctor]

@mart-e and/or @xmo-odoo
Can you recheck please, I've almost included all your remarks, I just left unresolved the ones that are still in discussions
Thank you

[xmo-odoo]

But since the entire thing is in an if self.mp_bearer_token: it ignors the case where we would be setting the bearer token no?

[xmo-odoo]

• If device exists, it always has an id

Then why is the code using dict.get?

[xmo-odoo]

Not unless you raise_for_exception, by default a non-200 response is just that, a response with a status code which is no in the 200 range.

[papyDoctor]

@qle-odoo modifications finished

[papyDoctor]

@qle-odoo recheck please

[papyDoctor]

@mart-e
@xmo-odoo
Can you re-check please?
Thank you,

[xmo-odoo]

Are you sure the entries are guaranteed to be in that order and you don't need to verify the keys?

[papyDoctor]

Indeed, it's the case now but since it's not guaranteed by Mercado Pago, I'll use a regex (already imported now) verification:

        ts_m = re.search(r"ts=(\d+)", x_signature)
        v1_m = re.search(r"v1=([a-f0-9]+)", x_signature)
        ts = ts_m.group(1) if ts_m else None
        v1 = v1_m.group(1) if v1_m else None
        if not ts or not v1:
            _logger.debug('Webhook bad X-Signature, ts: %s, v1: %s', ts, v1)
            return http.Response(status=400)

Thank you

[xmo-odoo]

This does not seem to have been updated or replied to?

[xmo-odoo]

The error doesn't seem to have much to do with the actual behaviour?

[mart-e]

@robodoo override=ci/security
a few nitpicking comments from Xavier remaining but fine security wise.
Thanks for the updates

[papyDoctor]

@mart-e
I did a change related to @xmo-odoo comment on the signature