From 60534c65ae1ea65a86fa12b344a4a0f55c41d8f6 Mon Sep 17 00:00:00 2001 From: Nans Lefebvre Date: Tue, 8 Oct 2019 12:34:40 +0000 Subject: [PATCH] [FIX] models, test_access_rights: restore normal ACL check Because of 4b1cb41cf7a3f936a1e6d00a2bb6a6a29e82d711, we might try to read the field 'active' on a record on which we can't read fields besides the name. This thus triggers an access error where there should not have been. In particular, this is the case for portal users: most often the records they can access point to records that they can't read (e.g. the partner of the internal user assigned to the ticket). As a result, clicking on any link creates an ACL, and thus redirects to the home page. It turns out that filtered_domain was primarily used on already loaded records, typically for the write, so it was assumed that the records could be read in the first place. However in the use-case of the portal, there is an explicit check on the read rights with the portal user, explaining the discrepancy. Since in the general case filtered_domain should be able to read all fields to evaluate the domain, we put it in sudo. fix co-authored with @rco closes odoo/odoo#38540 X-original-commit: cff5cc8cc78a32eb204368e84ec4980324046405 Signed-off-by: Nans Lefebvre (len) --- .../addons/test_access_rights/__manifest__.py | 1 + .../test_access_rights/ir.model.access.csv | 2 + odoo/addons/test_access_rights/models.py | 11 +++++ odoo/addons/test_access_rights/security.xml | 12 ++++++ .../test_access_rights/tests/__init__.py | 1 + .../tests/test_check_access.py | 40 +++++++++++++++++++ odoo/models.py | 2 +- 7 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 odoo/addons/test_access_rights/security.xml create mode 100644 odoo/addons/test_access_rights/tests/test_check_access.py diff --git a/odoo/addons/test_access_rights/__manifest__.py b/odoo/addons/test_access_rights/__manifest__.py index 84db875a150..0b15fe2830e 100644 --- a/odoo/addons/test_access_rights/__manifest__.py +++ b/odoo/addons/test_access_rights/__manifest__.py @@ -5,6 +5,7 @@ 'category': 'Tests', 'data': [ 'ir.model.access.csv', + 'security.xml', 'data.xml', ], } diff --git a/odoo/addons/test_access_rights/ir.model.access.csv b/odoo/addons/test_access_rights/ir.model.access.csv index 9edd8503b72..f47fcf74b17 100644 --- a/odoo/addons/test_access_rights/ir.model.access.csv +++ b/odoo/addons/test_access_rights/ir.model.access.csv @@ -3,3 +3,5 @@ access_test_access_right_some_obj,access_test_access_right_some_obj,model_test_a access_test_access_right_container,access_test_access_right_container,model_test_access_right_container,,1,1,1,1 access_test_access_right_parent,access_test_access_right_parent,model_test_access_right_parent,,1,1,1,1 access_test_access_right_obj_categ,access_test_access_right_obj_categ,model_test_access_right_obj_categ,,1,1,1,1 +access_test_ticket_portal,access_test_ticket_portal,model_test_access_right_ticket,base.group_portal,1,0,0,0 +access_test_ticket_user,access_test_ticket_user,model_test_access_right_ticket,base.group_user,1,1,1,1 diff --git a/odoo/addons/test_access_rights/models.py b/odoo/addons/test_access_rights/models.py index bef14bad2c2..1ce608325f7 100644 --- a/odoo/addons/test_access_rights/models.py +++ b/odoo/addons/test_access_rights/models.py @@ -40,3 +40,14 @@ class ObjCateg(models.Model): if self.env.context.get('only_media'): args += [('name', '=', 'Media')] return super(ObjCateg, self).search(args, **kwargs) + + +class FakeTicket(models.Model): + """We want to simulate a record that would typically be accessed by a portal user, + with a relational field to records that could not be accessed by a portal user. + """ + _name = 'test_access_right.ticket' + _description = 'Fake ticket For Test Access Right' + + name = fields.Char() + message_partner_ids = fields.Many2many(comodel_name='res.partner') diff --git a/odoo/addons/test_access_rights/security.xml b/odoo/addons/test_access_rights/security.xml new file mode 100644 index 00000000000..d71e815cf95 --- /dev/null +++ b/odoo/addons/test_access_rights/security.xml @@ -0,0 +1,12 @@ + + + Fake Ticket: portal or following + + [ + '|', + ('message_partner_ids', 'child_of', [user.partner_id.commercial_partner_id.id]), + ('message_partner_ids', 'in', [user.partner_id.id]) + ] + + + diff --git a/odoo/addons/test_access_rights/tests/__init__.py b/odoo/addons/test_access_rights/tests/__init__.py index cba993681cf..1e05ba54c86 100644 --- a/odoo/addons/test_access_rights/tests/__init__.py +++ b/odoo/addons/test_access_rights/tests/__init__.py @@ -1,2 +1,3 @@ from . import test_ir_rules from . import test_feedback +from . import test_check_access diff --git a/odoo/addons/test_access_rights/tests/test_check_access.py b/odoo/addons/test_access_rights/tests/test_check_access.py new file mode 100644 index 00000000000..be6c19e60f7 --- /dev/null +++ b/odoo/addons/test_access_rights/tests/test_check_access.py @@ -0,0 +1,40 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. +import odoo.tests + + +class TestAccess(odoo.tests.HttpCase): + def setUp(self): + super(TestAccess, self).setUp() + + self.portal_user = self.env['res.users'].create({ + 'login': 'P', + 'name': 'P', + 'groups_id': [(6, 0, [self.env.ref('base.group_portal').id])], + }) + # a partner that can't be read by the portal user, would typically be a user's + self.internal_user_partner = self.env['res.partner'].create({'name': 'I'}) + + self.document = self.env['test_access_right.ticket'].create({ + 'name': 'Need help here', + 'message_partner_ids': [(6, 0, [self.portal_user.partner_id.id, + self.internal_user_partner.id])], + }) + + def test_check_access(self): + """Typically, a document consulted by a portal user P + will point to other records that P cannot read. + For example, if P wants to consult a ticket of his, + the ticket will have a reviewer or assigned user that is internal, + and which partner cannot be read by P. + This should not block P from accessing the ticket. + """ + document = self.document.with_user(self.portal_user) + # at this point, some fields might already be loaded in cache. + # if so, it means we would bypass the ACL when trying to read the field + # while this is bad, this is not the object of this test + self.internal_user_partner.invalidate_cache(fnames=['active']) + # from portal's _document_check_access: + document.check_access_rights('read') + document.check_access_rule('read') + # no raise, because we are supposed to be able to read our ticket diff --git a/odoo/models.py b/odoo/models.py index 879b209e395..2f346874189 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -3204,7 +3204,7 @@ Record ids: %(records)s def _filter_access_rules_python(self, operation): dom = self.env['ir.rule']._compute_domain(self._name, operation) - return self.filtered_domain(dom or []) + return self.sudo().filtered_domain(dom or []) def unlink(self): """ unlink()