From 5ec7364f9c1230e79a752ae74b6fd940b541350b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Mart=C3=ADnez?= Date: Mon, 14 Aug 2023 17:13:30 +0200 Subject: [PATCH] [FIX] hr_employee_document: Remove record rules from hr.employee to avoid side effects. The purpose of this module is to allow a basic user (without hr permissions) to view the attachments related to his employee (hr.employee). By default a basic user cannot access to hr.employee model and therefore cannot see the attachments (ir.attachment) linked to it. This change overrides some things to allow the user's employee attachments to be displayed. TT44536 --- hr_employee_document/__manifest__.py | 1 - hr_employee_document/models/__init__.py | 1 + hr_employee_document/models/hr_employee.py | 27 ++++++++++++-- .../models/hr_employee_public.py | 11 +++--- hr_employee_document/models/ir_rule.py | 36 ++++++++++++++++++ hr_employee_document/security/security.xml | 32 ---------------- .../tests/test_hr_employee_document.py | 37 ++++++++++++------- 7 files changed, 89 insertions(+), 56 deletions(-) create mode 100644 hr_employee_document/models/ir_rule.py delete mode 100644 hr_employee_document/security/security.xml diff --git a/hr_employee_document/__manifest__.py b/hr_employee_document/__manifest__.py index 4d145fa4b5f..206e2ca47dc 100644 --- a/hr_employee_document/__manifest__.py +++ b/hr_employee_document/__manifest__.py @@ -15,7 +15,6 @@ "summary": "Documents attached to the employee profile", "depends": ["hr"], "data": [ - "security/security.xml", "views/hr_employee.xml", "views/hr_employee_public.xml", ], diff --git a/hr_employee_document/models/__init__.py b/hr_employee_document/models/__init__.py index 39b647b36f5..9e792963e5a 100644 --- a/hr_employee_document/models/__init__.py +++ b/hr_employee_document/models/__init__.py @@ -2,3 +2,4 @@ from . import hr_employee from . import hr_employee_public +from . import ir_rule diff --git a/hr_employee_document/models/hr_employee.py b/hr_employee_document/models/hr_employee.py index 684f367d1a4..8c8b201117f 100644 --- a/hr_employee_document/models/hr_employee.py +++ b/hr_employee_document/models/hr_employee.py @@ -1,7 +1,8 @@ # Copyright 2018 Brainbean Apps (https://brainbeanapps.com) +# Copyright 2023 Tecnativa - Víctor Martínez # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -from odoo import fields, models +from odoo import api, fields, models class HrEmployeeBase(models.AbstractModel): @@ -22,15 +23,33 @@ def _compute_document_count(self): for record in self: record.document_count = count_dict.get(record.id, 0) + @api.model + def check_access_rights(self, operation, raise_exception=True): + """Return access to the hr.employee model if we pass a specific context, + is a trick to list the attachments related to an employee.""" + if ( + not self.env.is_superuser() + and not self.env.user.has_group("hr.group_hr_user") + and operation == "read" + and self._name == "hr.employee" + ): + if ( + self.env.context.get("search_attachments_from_hr_employee") + or self == self.env.user.employee_ids + ): + return True + return super().check_access_rights( + operation=operation, raise_exception=raise_exception + ) + def action_get_attachment_tree_view(self): action = self.env["ir.actions.act_window"]._for_xml_id("base.action_attachment") action["context"] = { "default_res_model": self._name, "default_res_id": self.ids[0], + "search_attachments_from_hr_employee": True, } - action["domain"] = str( - [("res_model", "=", self._name), ("res_id", "in", self.ids)] - ) + action["domain"] = [("res_model", "=", self._name), ("res_id", "in", self.ids)] action["search_view_id"] = ( self.env.ref("hr_employee_document.ir_attachment_view_search").id, ) diff --git a/hr_employee_document/models/hr_employee_public.py b/hr_employee_document/models/hr_employee_public.py index 1fa193a355e..c70f94ba1a0 100644 --- a/hr_employee_document/models/hr_employee_public.py +++ b/hr_employee_document/models/hr_employee_public.py @@ -20,13 +20,12 @@ def action_get_attachment_tree_view(self): action["context"] = { "default_res_model": "hr.employee", "default_res_id": self.env.user.employee_id.id, + "search_attachments_from_hr_employee": True, } - action["domain"] = str( - [ - ("res_model", "=", "hr.employee"), - ("res_id", "=", self.env.user.employee_id.id), - ] - ) + action["domain"] = [ + ("res_model", "=", "hr.employee"), + ("res_id", "=", self.env.user.employee_id.id), + ] action["search_view_id"] = ( self.env.ref("hr_employee_document.ir_attachment_view_search").id, ) diff --git a/hr_employee_document/models/ir_rule.py b/hr_employee_document/models/ir_rule.py new file mode 100644 index 00000000000..a9b7b6bf066 --- /dev/null +++ b/hr_employee_document/models/ir_rule.py @@ -0,0 +1,36 @@ +# Copyright 2023 Tecnativa - Víctor Martínez +# License AGPL-3 - See http://www.gnu.org/licenses/agpl-3.0.html + +from odoo import api, models, tools +from odoo.osv import expression +from odoo.tools import config + + +class IrRule(models.Model): + _inherit = "ir.rule" + + @api.model + @tools.conditional( + "xml" not in config["dev_mode"], + tools.ormcache( + "self.env.uid", + "self.env.su", + "model_name", + "mode", + "tuple(self._compute_domain_context_values())", + ), + ) + def _compute_domain(self, model_name, mode="read"): + """We need to add for security purposes an extra domain in the hr.employee + model to restrict only the user's employees when search employee attachments.""" + res = super()._compute_domain(model_name, mode=mode) + user = self.env.user + if ( + model_name == "hr.employee" + and not self.env.su + and not user.has_group("hr.group_hr_manager") + and self.env.context.get("search_attachments_from_hr_employee") + ): + extra_domain = [[("id", "in", user.employee_ids.ids)]] + res = expression.AND(extra_domain + [res]) + return res diff --git a/hr_employee_document/security/security.xml b/hr_employee_document/security/security.xml deleted file mode 100644 index d4ce1750679..00000000000 --- a/hr_employee_document/security/security.xml +++ /dev/null @@ -1,32 +0,0 @@ - - - - hr.employee system user override - - - - - - - - - User can only read their own employees - - - - - - - [('id','in', user.employee_ids.ids)] - - - Hr user can access to all records - - - - - - - [(1, '=', 1)] - - diff --git a/hr_employee_document/tests/test_hr_employee_document.py b/hr_employee_document/tests/test_hr_employee_document.py index 2d827dcb61c..aa8dce5b380 100644 --- a/hr_employee_document/tests/test_hr_employee_document.py +++ b/hr_employee_document/tests/test_hr_employee_document.py @@ -11,17 +11,19 @@ class TestHrEmployeeDocument(common.TransactionCase): def setUp(self): super().setUp() - ctx = { - "mail_create_nolog": True, - "mail_create_nosubscribe": True, - "mail_notrack": True, - "no_reset_password": True, - } - self.user_1 = new_test_user(self.env, login="test-user-1", context=ctx) - self.user_2 = new_test_user(self.env, login="test-user-2", context=ctx) - new_test_user( - self.env, login="test-user-manager", groups="hr.group_hr_user", context=ctx + self.env = self.env( + context=dict( + self.env.context, + mail_create_nolog=True, + mail_create_nosubscribe=True, + mail_notrack=True, + no_reset_password=True, + tracking_disable=True, + ) ) + self.user_1 = new_test_user(self.env, login="test-user-1") + self.user_2 = new_test_user(self.env, login="test-user-2") + new_test_user(self.env, login="test-user-manager", groups="hr.group_hr_user") self.employee_1 = self.env["hr.employee"].create( {"name": "Employee #1", "user_id": self.user_1.id} ) @@ -51,19 +53,28 @@ def test_employee_attachment(self): employee_public = self.env["hr.employee.public"].browse(employee.id) self.assertEqual(employee_public.document_count, 1) + def _get_attachments_from_employee(self, employee): + res = employee.action_get_attachment_tree_view() + return ( + self.env[res["res_model"]] + .with_context(**res["context"]) + .search(res["domain"]) + ) + @users("test-user-2") def test_employee_attachment_tree_view(self): employee = self.env.user.employee_id self.assertNotEqual(employee.action_get_attachment_tree_view(), None) employee_public = self.env["hr.employee.public"].browse(employee.id) - self.assertNotEqual(employee_public.action_get_attachment_tree_view(), None) + items = self._get_attachments_from_employee(employee_public) + self.assertEqual(len(items), 0) @users("test-user-1") def test_attachments_access_user_1(self): # create attachments attachment_1 = self._create_attachment(self.employee_1) attachment_2 = self._create_attachment(self.employee_2) - records = self.env["ir.attachment"].search([]) + records = self._get_attachments_from_employee(self.env.user.employee_id) self.assertIn(attachment_1, records) self.assertNotIn(attachment_2, records) @@ -72,7 +83,7 @@ def test_attachments_access_user_2(self): # create attachments attachment_1 = self._create_attachment(self.employee_1) attachment_2 = self._create_attachment(self.employee_2) - records = self.env["ir.attachment"].search([]) + records = self._get_attachments_from_employee(self.env.user.employee_id) self.assertNotIn(attachment_1, records) self.assertIn(attachment_2, records)