From 154698de24fa44322ce6e8a6bf330adbcc7962c3 Mon Sep 17 00:00:00 2001 From: SilvioC2C Date: Thu, 26 May 2022 17:02:04 +0200 Subject: [PATCH 1/2] FIX base_user_role: use ``sudo()`` in ``user_ids`` compute When computing ``user_ids``, avoid AccessError due to multi-company rules if the compute is triggered by an admin user --- base_user_role/models/role.py | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/base_user_role/models/role.py b/base_user_role/models/role.py index 636c3830..30fb8096 100644 --- a/base_user_role/models/role.py +++ b/base_user_role/models/role.py @@ -36,26 +36,39 @@ class ResUsersRole(models.Model): @api.depends("line_ids.user_id") def _compute_user_ids(self): - for role in self: + for role in self.sudo() if self._bypass_rules() else self: role.user_ids = role.line_ids.mapped("user_id") + @api.model + def _bypass_rules(self): + # Run methods as super user to avoid problems by "Administrator/Access Right" + return self._name == "res.users.role" and self.env.user.has_group( + "base.group_erp_manager" + ) + @api.model def create(self, vals): - new_record = super(ResUsersRole, self).create(vals) + model = (self.sudo() if self._bypass_rules() else self).browse() + new_record = super(ResUsersRole, model).create(vals) new_record.update_users() return new_record + def read(self, fields=None, load="_classic_read"): + recs = self.sudo() if self._bypass_rules() else self + return super(ResUsersRole, recs).read(fields, load) + def write(self, vals): - # Workaround to solve issue with broken code in odoo that clear the cache - # during the write: see odoo/addons/base/models/res_users.py#L226 + recs = self.sudo() if self._bypass_rules() else self + # Workaround to solve issue with broken code in odoo that clear the + # cache during the write: see odoo/addons/base/models/res_users.py#L226 groups_vals = {} - for field in self.group_id._fields: + for field in recs.group_id._fields: if field in vals: groups_vals[field] = vals.pop(field) if groups_vals: - self.group_id.write(groups_vals) - res = super(ResUsersRole, self).write(vals) - self.update_users() + recs.group_id.write(groups_vals) + res = super(ResUsersRole, recs).write(vals) + recs.update_users() return res def unlink(self): From 94f386ef057203920ea5609c1181074224734a46 Mon Sep 17 00:00:00 2001 From: SilvioC2C Date: Thu, 26 May 2022 17:04:04 +0200 Subject: [PATCH 2/2] FIX base_user_role: update tests --- base_user_role/tests/test_user_role.py | 92 ++++++++++++++++++++------ 1 file changed, 72 insertions(+), 20 deletions(-) diff --git a/base_user_role/tests/test_user_role.py b/base_user_role/tests/test_user_role.py index 5c05b82a..6556ca02 100644 --- a/base_user_role/tests/test_user_role.py +++ b/base_user_role/tests/test_user_role.py @@ -3,34 +3,41 @@ import datetime from odoo import fields -from odoo.tests.common import TransactionCase +from odoo.exceptions import AccessError +from odoo.tests.common import SavepointCase -class TestUserRole(TransactionCase): - def setUp(self): - super(TestUserRole, self).setUp() - self.user_model = self.env["res.users"] - self.role_model = self.env["res.users.role"] +class TestUserRole(SavepointCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.env = cls.env( + context=dict(cls.env.context, tracking_disable=True, no_reset_password=True) + ) + cls.user_model = cls.env["res.users"] + cls.role_model = cls.env["res.users.role"] - self.default_user = self.env.ref("base.default_user") - self.user_id = self.user_model.create( + cls.company1 = cls.env.ref("base.main_company") + cls.company2 = cls.env["res.company"].create({"name": "company2"}) + cls.default_user = cls.env.ref("base.default_user") + cls.user_id = cls.user_model.create( {"name": "USER TEST (ROLES)", "login": "user_test_roles"} ) # ROLE_1 - self.group_user_id = self.env.ref("base.group_user") - self.group_no_one_id = self.env.ref("base.group_no_one") + cls.group_user_id = cls.env.ref("base.group_user") + cls.group_no_one_id = cls.env.ref("base.group_no_one") vals = { "name": "ROLE_1", - "implied_ids": [(6, 0, [self.group_user_id.id, self.group_no_one_id.id])], + "implied_ids": [(6, 0, [cls.group_user_id.id, cls.group_no_one_id.id])], } - self.role1_id = self.role_model.create(vals) + cls.role1_id = cls.role_model.create(vals) # ROLE_2 # Must have group_user in order to have sufficient groups. Check: # github.com/odoo/odoo/commit/c3717f3018ce0571aa41f70da4262cc946d883b4 - self.group_multi_currency_id = self.env.ref("base.group_multi_currency") - self.group_settings_id = self.env.ref("base.group_system") + cls.group_multi_currency_id = cls.env.ref("base.group_multi_currency") + cls.group_settings_id = cls.env.ref("base.group_system") vals = { "name": "ROLE_2", "implied_ids": [ @@ -38,16 +45,41 @@ class TestUserRole(TransactionCase): 6, 0, [ - self.group_user_id.id, - self.group_multi_currency_id.id, - self.group_settings_id.id, + cls.group_user_id.id, + cls.group_multi_currency_id.id, + cls.group_settings_id.id, ], ) ], } - self.role2_id = self.role_model.create(vals) - self.company1 = self.env.ref("base.main_company") - self.company2 = self.env["res.company"].create({"name": "company2"}) + cls.role2_id = cls.role_model.create(vals) + + # Setup for multi-company testing + cls.multicompany_user_1 = cls.user_model.create( + { + "name": "User 2", + "company_id": cls.company1.id, + "company_ids": [(6, 0, [cls.company1.id, cls.company2.id])], + "groups_id": [(6, 0, cls.env.ref("base.group_erp_manager").ids)], + "login": "multicompany_user_1", + } + ) + cls.multicompany_user_2 = cls.user_model.create( + { + "name": "User 2", + "company_id": cls.company2.id, + "company_ids": [(6, 0, [cls.company2.id])], + "groups_id": [(6, 0, cls.env.ref("base.group_user").ids)], + "login": "multicompany_user_2", + } + ) + cls.multicompany_role = cls.role_model.create( + { + "name": "MULTICOMPANY_ROLE", + "implied_ids": [(6, 0, [cls.group_user_id.id])], + "line_ids": [(0, 0, {"user_id": cls.multicompany_user_2.id})], + } + ) def test_role_1(self): self.user_id.write({"role_line_ids": [(0, 0, {"role_id": self.role1_id.id})]}) @@ -180,3 +212,23 @@ class TestUserRole(TransactionCase): self.role1_id.write({"name": "foo", "comment": "FOO"}) self.assertEqual(self.role1_id.group_id.name, "foo") self.assertEqual(self.role1_id.group_id.comment, "FOO") + + def test_role_multicompany(self): + """Test AccessError when admin-like user accesses a role""" + role = self.multicompany_role.with_user(self.multicompany_user_1) + # Dummy read to check that multicompany user 1 has read access + role.read() + # Dummy read to check that multicompany user 1 has read access on the + # whole role, even if it's using a different company than multicompany + # user 2 (which is included in the role) + role.with_context(allowed_company_ids=self.company1.ids).read() + # Downgrade multicompany user 1 to common user + self.multicompany_user_1.write( + {"groups_id": [(6, 0, self.env.ref("base.group_user").ids)]} + ) + # Check that the user cannot read multicompany data again since it lost + # its admin privileges + with self.assertRaisesRegex( + AccessError, "You are not allowed to access 'User role'" + ): + role.read()