From 0f2de188499b2faffcc947ce87ac7971117340d4 Mon Sep 17 00:00:00 2001 From: Horacio Tellez Date: Thu, 3 Nov 2022 08:46:05 +0000 Subject: [PATCH] [IMP] payment: only give access to tokens when required by the flow Until this point the access to tokens was somehow arbitrary and illogical. After this commit we will uniformize the tokens access rule where by default an user can only access its own tokens by default and in function of the use case then relax the rules. Task - 2832561 closes odoo/odoo#104808 Related: odoo/enterprise#33541 Signed-off-by: Antoine Vandevenne (anv) --- .../account_payment/models/account_payment.py | 18 +++--------------- .../wizards/account_payment_register.py | 17 +++-------------- addons/payment/controllers/portal.py | 6 +++--- addons/payment/security/payment_security.xml | 8 +++++--- addons/payment/tests/test_payment_token.py | 11 ++++++++++- 5 files changed, 24 insertions(+), 36 deletions(-) diff --git a/addons/account_payment/models/account_payment.py b/addons/account_payment/models/account_payment.py index d20871cc52b..e858ed27347 100644 --- a/addons/account_payment/models/account_payment.py +++ b/addons/account_payment/models/account_payment.py @@ -62,17 +62,11 @@ class AccountPayment(models.Model): @api.depends('payment_method_line_id') def _compute_suitable_payment_token_ids(self): for payment in self: - related_partner_ids = ( - payment.partner_id - | payment.partner_id.commercial_partner_id - | payment.partner_id.commercial_partner_id.child_ids - )._origin - if payment.use_electronic_payment_method: payment.suitable_payment_token_ids = self.env['payment.token'].sudo().search([ ('company_id', '=', payment.company_id.id), ('provider_id.capture_manually', '=', False), - ('partner_id', 'in', related_partner_ids.ids), + ('partner_id', '=', payment.partner_id.id), ('provider_id', '=', payment.payment_method_line_id.payment_provider_id.id), ]) else: @@ -108,15 +102,9 @@ class AccountPayment(models.Model): self.payment_token_id = False return - related_partner_ids = ( - self.partner_id - | self.partner_id.commercial_partner_id - | self.partner_id.commercial_partner_id.child_ids - )._origin - - self.payment_token_id = self.env['payment.token'].sudo().search([ + self.payment_token_id = self.env['payment.token'].search([ ('company_id', '=', self.company_id.id), - ('partner_id', 'in', related_partner_ids.ids), + ('partner_id', '=', self.partner_id.id), ('provider_id.capture_manually', '=', False), ('provider_id', '=', self.payment_method_line_id.payment_provider_id.id), ], limit=1) diff --git a/addons/account_payment/wizards/account_payment_register.py b/addons/account_payment/wizards/account_payment_register.py index 243a5d8dd10..4c0e9e2c6d6 100644 --- a/addons/account_payment/wizards/account_payment_register.py +++ b/addons/account_payment/wizards/account_payment_register.py @@ -36,16 +36,10 @@ class AccountPaymentRegister(models.TransientModel): def _compute_suitable_payment_token_ids(self): for wizard in self: if wizard.can_edit_wizard and wizard.use_electronic_payment_method: - related_partner_ids = ( - wizard.partner_id - | wizard.partner_id.commercial_partner_id - | wizard.partner_id.commercial_partner_id.child_ids - )._origin - wizard.suitable_payment_token_ids = self.env['payment.token'].sudo().search([ ('company_id', '=', wizard.company_id.id), ('provider_id.capture_manually', '=', False), - ('partner_id', 'in', related_partner_ids.ids), + ('partner_id', '=', wizard.partner_id.id), ('provider_id', '=', wizard.payment_method_line_id.payment_provider_id.id), ]) else: @@ -63,19 +57,14 @@ class AccountPaymentRegister(models.TransientModel): def _compute_payment_token_id(self): codes = [key for key in dict(self.env['payment.provider']._fields['code']._description_selection(self.env))] for wizard in self: - related_partner_ids = ( - wizard.partner_id - | wizard.partner_id.commercial_partner_id - | wizard.partner_id.commercial_partner_id.child_ids - )._origin if wizard.can_edit_wizard \ and wizard.payment_method_line_id.code in codes \ and wizard.journal_id \ - and related_partner_ids: + and wizard.partner_id: wizard.payment_token_id = self.env['payment.token'].sudo().search([ ('company_id', '=', wizard.company_id.id), - ('partner_id', 'in', related_partner_ids.ids), + ('partner_id', '=', wizard.partner_id.id), ('provider_id.capture_manually', '=', False), ('provider_id', '=', wizard.payment_method_line_id.payment_provider_id.id), ], limit=1) diff --git a/addons/payment/controllers/portal.py b/addons/payment/controllers/portal.py index e2587bad825..e7f14cbde91 100644 --- a/addons/payment/controllers/portal.py +++ b/addons/payment/controllers/portal.py @@ -112,9 +112,9 @@ class PaymentPortal(portal.CustomerPortal): ) # In sudo mode to read the fields of providers and partner (if not logged in) if provider_id in providers_sudo.ids: # Only keep the desired provider if it's suitable providers_sudo = providers_sudo.browse(provider_id) - payment_tokens = request.env['payment.token']._get_available_tokens( + tokens_sudo = request.env['payment.token'].sudo()._get_available_tokens( providers_sudo.ids, partner_id=partner_sudo.id, logged_in=logged_in - ) + ) # In sudo mode to be able to read tokens of other partners. # Make sure that the partner's company matches the company passed as parameter. company_mismatch = not PaymentPortal._can_partner_pay_in_company(partner_sudo, company) @@ -135,7 +135,7 @@ class PaymentPortal(portal.CustomerPortal): } payment_form_values = { 'providers': providers_sudo, - 'tokens': payment_tokens, + 'tokens': tokens_sudo, 'fees_by_provider': fees_by_provider, 'show_tokenize_input': self._compute_show_tokenize_input_mapping( providers_sudo, logged_in=logged_in, **kwargs diff --git a/addons/payment/security/payment_security.xml b/addons/payment/security/payment_security.xml index fc0b23f4339..efeb7ebf9f2 100644 --- a/addons/payment/security/payment_security.xml +++ b/addons/payment/security/payment_security.xml @@ -27,10 +27,12 @@ - Access only tokens belonging to commercial partner + Users can access only their own tokens - [('partner_id', 'child_of', user.partner_id.commercial_partner_id.id)] - + [('partner_id', '=', user.partner_id.id)] + diff --git a/addons/payment/tests/test_payment_token.py b/addons/payment/tests/test_payment_token.py index 2e3428ac4b1..cb653dad9cd 100644 --- a/addons/payment/tests/test_payment_token.py +++ b/addons/payment/tests/test_payment_token.py @@ -2,8 +2,9 @@ from datetime import date -from odoo.exceptions import UserError +from odoo.exceptions import AccessError, UserError from odoo.tests import tagged +from odoo.tools import mute_logger from odoo.addons.payment.tests.common import PaymentCommon @@ -11,6 +12,14 @@ from odoo.addons.payment.tests.common import PaymentCommon @tagged('-at_install', 'post_install') class TestPaymentToken(PaymentCommon): + @mute_logger('odoo.addons.base.models.ir_rule') + def test_users_have_no_access_to_other_users_tokens(self): + users = [self.public_user, self.portal_user, self.internal_user] + token = self._create_token(partner_id=self.admin_partner.id) + for user in users: + with self.assertRaises(AccessError): + token.with_user(user).read() + def test_token_cannot_be_unarchived(self): """ Test that unarchiving disabled tokens is forbidden. """ token = self._create_token(active=False)