[IMP] payment, sale: access token for payment link wizard

When generating a link for a payment through the website_payment/pay
route, including the partner_id 'blindly' is somewhat of a security
issue since it means you could potentially create payments for any
partner of your choosing.

This commit instead introduces an access token mechanism where the
partner_id, amount and currency_id are used to generate a unique access
token that can be checked upon accessing the payment page. This token
is only checked if the partner_id field is set (otherwise this is just
an anonymous payment like any other).

addons/payment/controllers/portal.py

@@ -132,10 +132,27 @@ class WebsitePayment(http.Controller):
         return request.render("payment.pay_methods", values)
 
     @http.route(['/website_payment/pay'], type='http', auth='public', website=True, sitemap=False)
-    def pay(self, reference='', order_id=None, amount=False, currency_id=None, acquirer_id=None, partner_id=False, **kw):
+    def pay(self, reference='', order_id=None, amount=False, currency_id=None, acquirer_id=None, partner_id=False, access_token=None, **kw):
+        """
+        Generic payment page allowing public and logged in users to pay an arbitrary amount.
+
+        In the case of a public user access, we need to ensure that the payment is made anonymously - e.g. it should not be
+        possible to pay for a specific partner simply by setting the partner_id GET param to a random id. In the case where
+        a partner_id is set, we do an access_token check based on the payment.link.wizard model (since links for specific
+        partners should be created from there and there only). Also noteworthy is the filtering of s2s payment methods -
+        we don't want to create payment tokens for public users.
+
+        In the case of a logged in user, then we let access rights and security rules do their job.
+        """
         env = request.env
         user = env.user.sudo()
         reference = normalize('NFKD', reference).encode('ascii','ignore').decode('utf-8')
+        if partner_id and not access_token:
+            raise werkzeug.exceptions.NotFound
+        if partner_id and access_token:
+            token_ok = request.env['payment.link.wizard'].check_token(access_token, int(partner_id), float(amount), int(currency_id))
+            if not token_ok:
+                raise werkzeug.exceptions.NotFound
 
         # Default values
         values = {
@@ -185,6 +202,11 @@ class WebsitePayment(http.Controller):
 
         # Check partner
         if not user._is_public():
+            # NOTE: this means that if the partner was set in the GET param, it gets overwritten here
+            # This is something we want, since security rules are based on the partner - assuming the
+            # access_token checked out at the start, this should have no impact on the payment itself
+            # existing besides making reconciliation possibly more difficult (if the payment partner is
+            # not the same as the invoice partner, for example)
             partner_id = user.partner_id.id
         elif partner_id:
             partner_id = int(partner_id)
@@ -195,6 +217,7 @@ class WebsitePayment(http.Controller):
             'error_msg': kw.get('error_msg')
         })
 
+        # s2s mode will always generate a token, which we don't want for public users
         valid_flows = ['form', 's2s'] if not user._is_public() else ['form']
         values['acquirers'] = [acq for acq in acquirers if acq.payment_flow in valid_flows]
         values['pms'] = request.env['payment.token'].search([('acquirer_id', 'in', acquirers.ids)])
@@ -255,14 +278,14 @@ class WebsitePayment(http.Controller):
             'reference': reference,
             'amount': float(amount),
             'currency_id': int(currency_id),
-            'partner_id': partner_id,
-            'payment_token_id': pm_id,
+            'partner_id': int(partner_id),
+            'payment_token_id': int(pm_id),
             'type': 'server2server',
             'return_url': return_url,
         }
 
         if order_id:
-            values['sale_order_ids'] = [(6, 0, [order_id])]
+            values['sale_order_ids'] = [(6, 0, [int(order_id)])]
 
         tx = request.env['payment.transaction'].sudo().with_context(lang=None).create(values)
         PaymentProcessing.add_payment_transaction(tx)
@@ -272,7 +295,7 @@ class WebsitePayment(http.Controller):
             secret = request.env['ir.config_parameter'].sudo().get_param('database.secret')
             token_str = '%s%s%s' % (tx.id, tx.reference, tx.amount)
             token = hmac.new(secret.encode('utf-8'), token_str.encode('utf-8'), hashlib.sha256).hexdigest()
-            tx.return_url = return_url or '/website_payment/confirm?tx_id=%d&access_token=%s' % tx.id
+            tx.return_url = return_url or '/website_payment/confirm?tx_id=%d&access_token=%s' % (tx.id, token)
         except Exception as e:
             _logger.exception(e)
         return request.redirect('/payment/process')

addons/payment/wizards/payment_link_wizard.py

@@ -1,8 +1,11 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
+import hashlib
+import hmac
 
 from odoo import api, fields, models, _
 from odoo.exceptions import ValidationError
+from odoo.tools import ustr, consteq
 
 
 class PaymentLinkWizard(models.TransientModel):
@@ -32,8 +35,9 @@ class PaymentLinkWizard(models.TransientModel):
     amount_max = fields.Monetary(currency_field='currency_id')
     currency_id = fields.Many2one('res.currency')
     partner_id = fields.Many2one('res.partner')
-    link = fields.Char(string='Payment Link')
+    link = fields.Char(string='Payment Link', compute='_compute_values')
     description = fields.Char('Payment Ref')
+    access_token = fields.Char(compute='_compute_values')
 
     @api.onchange('amount', 'description')
     def _onchange_amount(self):
@@ -41,5 +45,26 @@ class PaymentLinkWizard(models.TransientModel):
             raise ValidationError(_("Please set an amount smaller than %s.") % (self.amount_max))
         if self.amount <= 0:
             raise ValidationError(_("The value of the payment amount must be positive."))
+
+    @api.depends('amount', 'description', 'partner_id', 'currency_id')
+    def _compute_values(self):
+        secret = self.env['ir.config_parameter'].sudo().get_param('database.secret')
+        for payment_link in self:
+            token_str = '%s%s%s' % (payment_link.partner_id.id, payment_link.amount, payment_link.currency_id.id)
+            payment_link.access_token = hmac.new(secret.encode('utf-8'), token_str.encode('utf-8'), hashlib.sha256).hexdigest()
+        # must be called after token generation, obvsly - the link needs an up-to-date token
+        self._generate_link()
+
+    def _generate_link(self):
         base_url = self.env['ir.config_parameter'].sudo().get_param('web.base.url')
-        self.link = '%s/website_payment/pay?reference=%s&amount=%s&currency_id=%s&partner_id=%s' % (base_url, self.description, self.amount, self.currency_id.id, self.partner_id.id)
+        for payment_link in self:
+            payment_link.link = '%s/website_payment/pay?reference=%s&amount=%s&currency_id=%s&partner_id=%s&access_token=%s' % (base_url, payment_link.description, payment_link.amount, payment_link.currency_id.id, payment_link.partner_id.id, payment_link.access_token)
+
+    @api.model
+    def check_token(self, access_token, partner_id, amount, currency_id):
+        secret = self.env['ir.config_parameter'].sudo().get_param('database.secret')
+        token_str = '%s%s%s' % (partner_id, amount, currency_id)
+        correct_token = hmac.new(secret.encode('utf-8'), token_str.encode('utf-8'), hashlib.sha256).hexdigest()
+        if consteq(ustr(access_token), correct_token):
+            return True
+        return False

addons/payment/wizards/payment_link_wizard_views.xml

@@ -13,6 +13,7 @@
                     <field name="description"/>
                     <field name="amount"/>
                     <field name="currency_id" invisible="1"/>
+                    <field name="access_token" invisible="1"/>
                 </group>
                 <group>
                     <field name="link" readonly="1" widget="CopyClipboardChar"/>

addons/sale/wizard/sale_payment_link.py

@@ -22,9 +22,8 @@ class SalePaymentLink(models.TransientModel):
             })
         return res
 
-    @api.onchange('amount', 'description')
-    def _onchange_amount(self):
-        res = super(SalePaymentLink, self)._onchange_amount()
-        if self.res_model == 'sale.order':
-            self.link = "%s&order_id=%d" % (self.link, self.res_id)
-        return res
+    def _generate_link(self):
+        """ Override of the base method to add the order_id in the link. """
+        base_url = self.env['ir.config_parameter'].sudo().get_param('web.base.url')
+        for payment_link in self:
+            payment_link.link = '%s/website_payment/pay?reference=%s&amount=%s&currency_id=%s&partner_id=%s&order_id=%s&access_token=%s' % (base_url, payment_link.description, payment_link.amount, payment_link.currency_id.id, payment_link.partner_id.id, payment_link.res_id, payment_link.access_token)