[IMP] hr_timesheet: Improve access rights

TaskID: 2088586

The low level access right is now "See my timesheets".
This access disallow to a user to view and manage the timesheets of others users.

When the visibility of a project is set on "Invited employees",
the users with the access rules : 'See my timesheet' and
'See all timesheets' can show the timesheets, if these users
follow the project.

5 files changed, 81 insertions, 15 deletions

diff --git a/addons/hr_timesheet/models/hr_timesheet.py b/addons/hr_timesheet/models/hr_timesheet.py
index ddd0f05a64e..f7cad827761 100644
--- a/addons/hr_timesheet/models/hr_timesheet.py
+++ b/addons/hr_timesheet/models/hr_timesheet.py
@@ -5,8 +5,8 @@ from lxml import etree
 import re
 
 from odoo import api, fields, models, _
-from odoo.exceptions import UserError
-
+from odoo.exceptions import UserError, AccessError
+from odoo.osv import expression
 
 class AccountAnalyticLine(models.Model):
     _inherit = 'account.analytic.line'
@@ -20,11 +20,29 @@ class AccountAnalyticLine(models.Model):
             result['employee_id'] = self.env['hr.employee'].search([('user_id', '=', result['user_id'])], limit=1).id
         return result
 
+    def _domain_project_id(self):
+        domain = [('allow_timesheets', '=', True)]
+        if not self.user_has_groups('hr_timesheet.group_timesheet_manager'):
+            return expression.AND([domain,
+                ['|', ('privacy_visibility', '!=', 'followers'), ('message_partner_ids', 'in', [self.env.user.partner_id.id])]
+            ])
+        return domain
+
+    def _domain_employee_id(self):
+        if not self.user_has_groups('hr_timesheet.group_hr_timesheet_approver'):
+            return [('user_id', '=', self.env.user.id)]
+        return []
+
+    def _domain_task_id(self):
+        if not self.user_has_groups('hr_timesheet.group_hr_timesheet_approver'):
+            return ['|', ('privacy_visibility', '!=', 'followers'), ('message_partner_ids', 'in', [self.env.user.partner_id.id])]
+        return []
+
     task_id = fields.Many2one(
         'project.task', 'Task', index=True,
         domain="[('company_id', '=', company_id), ('project_id.allow_timesheets', '=', True), ('project_id', '=?', project_id)]"
     )
-    project_id = fields.Many2one('project.project', 'Project', domain=[('allow_timesheets', '=', True)])
+    project_id = fields.Many2one('project.project', 'Project', domain=_domain_project_id)
 
     employee_id = fields.Many2one('hr.employee', "Employee", check_company=True)
     department_id = fields.Many2one('hr.department', "Department", compute='_compute_department_id', store=True, compute_sudo=True)
@@ -81,6 +99,12 @@ class AccountAnalyticLine(models.Model):
         return lines
 
     def write(self, values):
+        # If it's a basic user then check if the timesheet is his own.
+        for record in self:
+            if not self.user_has_groups('hr_timesheet.group_hr_timesheet_approver'):
+                if self.env.user.id != record.user_id.id:
+                    raise AccessError(_("You aren't allowed to access to the timesheet that it's not yours"))
+
         values = self._timesheet_preprocess(values)
         result = super(AccountAnalyticLine, self).write(values)
         # applied only for timesheet
diff --git a/addons/hr_timesheet/security/hr_timesheet_security.xml b/addons/hr_timesheet/security/hr_timesheet_security.xml
index 64033b482ec..ca969ce739d 100644
--- a/addons/hr_timesheet/security/hr_timesheet_security.xml
+++ b/addons/hr_timesheet/security/hr_timesheet_security.xml
@@ -7,14 +7,14 @@
         </record>
 
         <record id="group_hr_timesheet_user" model="res.groups">
-            <field name="name">See all Timesheets</field>
+            <field name="name">See own timesheets</field>
             <field name="category_id" ref="base.module_category_operations_timesheets"/>
             <field name="implied_ids" eval="[(4, ref('base.group_user'))]"/>
             <field name="users" eval="[(4, ref('base.user_root')), (4, ref('base.user_admin'))]"/>
         </record>
 
         <record id="base.default_user" model="res.users">
-            <field name="groups_id" eval="[(4,ref('group_hr_timesheet_user'))]"/>
+            <field name="groups_id" eval="[(4,ref('group_timesheet_manager'))]"/>
         </record>
     </data>
 
@@ -23,16 +23,24 @@
         <record id="timesheet_line_rule_user" model="ir.rule">
             <field name="name">account.analytic.line.timesheet.user</field>
             <field name="model_id" ref="analytic.model_account_analytic_line"/>
-            <field name="domain_force">[('user_id', '=', user.id), ('project_id', '!=', False)]</field>
+            <field name="domain_force">[
+                ('user_id', '=', user.id),
+                ('project_id', '!=', False),
+                '|',
+                    ('project_id.privacy_visibility', '!=', 'followers'),
+                    '|',
+                        ('project_id.message_partner_ids', 'in', [user.partner_id.id]),
+                        ('task_id.message_partner_ids', 'in', [user.partner_id.id])
+            ]</field>
             <field name="groups" eval="[(4, ref('group_hr_timesheet_user'))]"/>
             <field name="perm_create" eval="1"/>
             <field name="perm_write" eval="1"/>
             <field name="perm_unlink" eval="1"/>
-            <field name="perm_read" eval="0"/>
+            <field name="perm_read" eval="1"/>
         </record>
 
-        <record id="hr_timesheet.group_hr_timesheet_approver" model="res.groups">
-            <field name="name">Team Approver</field>
+        <record id="group_hr_timesheet_approver" model="res.groups">
+            <field name="name">See all timesheets</field>
             <field name="category_id" ref="base.module_category_operations_timesheets"/>
             <field name="implied_ids" eval="[(4, ref('hr_timesheet.group_hr_timesheet_user'))]"/>
         </record>
@@ -40,7 +48,7 @@
         <record id="group_timesheet_manager" model="res.groups">
             <field name="name">Administrator</field>
             <field name="category_id" ref="base.module_category_operations_timesheets"/>
-            <field name="implied_ids" eval="[(4, ref('group_hr_timesheet_approver')), (4, ref('hr.group_hr_user'))]"/>
+            <field name="implied_ids" eval="[(4, ref('hr_timesheet.group_hr_timesheet_approver')), (4, ref('hr.group_hr_user'))]"/>
             <field name="users" eval="[(4, ref('base.user_root')), (4, ref('base.user_admin'))]"/>
         </record>
     </data>
@@ -60,4 +68,19 @@
 
     </data>
 
+    <record id="timesheet_line_rule_approver" model="ir.rule">
+        <field name="name">account.analytic.line.timesheet.approver</field>
+        <field name="model_id" ref="analytic.model_account_analytic_line" />
+        <field name="domain_force">[
+            ('project_id', '!=', False),
+            '|',
+                ('project_id.privacy_visibility', '!=', 'followers'),
+                ('project_id.message_partner_ids', 'in', [user.partner_id.id])
+        ]</field>
+        <field name="groups" eval="[(4, ref('hr_timesheet.group_hr_timesheet_approver'))]" />
+        <field name="perm_create" eval="1"/>
+        <field name="perm_write" eval="1"/>
+        <field name="perm_unlink" eval="1"/>
+        <field name="perm_read" eval="1"/>
+    </record>
 </odoo>
diff --git a/addons/hr_timesheet/security/ir.model.access.csv b/addons/hr_timesheet/security/ir.model.access.csv
index cb11fb119f7..84cb43d62e1 100644
--- a/addons/hr_timesheet/security/ir.model.access.csv
+++ b/addons/hr_timesheet/security/ir.model.access.csv
@@ -1,5 +1,7 @@
 id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink

-access_account_analytic_line_user,analytic.account.analytic.line.timesheet.user,analytic.model_account_analytic_line,hr_timesheet.group_hr_timesheet_user,1,1,1,1

+access_account_analytic_line_manager,analytic.account.analytic.line.timesheet.manager,analytic.model_account_analytic_line,hr_timesheet.group_timesheet_manager,1,1,1,1

+access_account_analytic_line_approver,analytic.account.analytic.line.timesheet.approver,analytic.model_account_analytic_line,hr_timesheet.group_hr_timesheet_approver,1,1,1,1

+access_account_analytic_line_user,analytic.account.analytic.line.timesheet.user,analytic.model_account_analytic_line,hr_timesheet.group_hr_timesheet_user,1,1,1,1,1

 access_account_analytic_user,analytic.account.analytic.timesheet.user,analytic.model_account_analytic_account,hr_timesheet.group_hr_timesheet_user,1,1,0,0

 access_uom_uom_hr_timesheet,uom.uom.timesheet.user,uom.model_uom_uom,hr_timesheet.group_hr_timesheet_user,1,0,0,0

 access_project_project,project.project.timesheet.user,model_project_project,hr_timesheet.group_hr_timesheet_user,1,0,0,0

diff --git a/addons/hr_timesheet/tests/test_timesheet.py b/addons/hr_timesheet/tests/test_timesheet.py
index 2245c40707e..8fd1bde9d0e 100644
--- a/addons/hr_timesheet/tests/test_timesheet.py
+++ b/addons/hr_timesheet/tests/test_timesheet.py
@@ -16,7 +16,7 @@ class TestCommonTimesheet(TransactionCase):
         rule = self.env.ref('timesheet_grid.hr_timesheet_rule_approver_update', raise_if_not_found=False)
         if rule:
             rule.active = False
-        
+
         # customer partner
         self.partner = self.env['res.partner'].create({
             'name': 'Customer Task',
@@ -85,6 +85,16 @@ class TestCommonTimesheet(TransactionCase):
 
 class TestTimesheet(TestCommonTimesheet):
 
+    def setUp(self):
+        super(TestTimesheet, self).setUp()
+
+        # Crappy hack to disable the rule from timesheet grid, if it exists
+        # The registry doesn't contain the field timesheet_manager_id.
+        # but there is an ir.rule about it, crashing during its evaluation
+        rule = self.env.ref('timesheet_grid.timesheet_line_rule_user_update-unlink', raise_if_not_found=False)
+        if rule:
+            rule.active = False
+
     def test_log_timesheet(self):
         """ Test when log timesheet : check analytic account, user and employee are correctly set. """
         Timesheet = self.env['account.analytic.line']
@@ -260,12 +270,18 @@ class TestTimesheet(TestCommonTimesheet):
         timesheets = timesheet_1 + timesheet_2
 
         # increase unit_amount to trigger amount recomputation
-        timesheets.sudo().write({
+        with self.assertRaises(AccessError):
+            # because the employee 1 is the sudo and he doesn't have the access right to update timesheet of employee 2
+            timesheets.sudo().write({
+                'unit_amount': 2,
+            })
+
+        timesheets.with_user(self.user_manager).write({
             'unit_amount': 2,
         })
 
         # since timesheet costs are different for both employees, we should get different amounts
-        self.assertRecordValues(timesheets, [{
+        self.assertRecordValues(timesheets.with_user(self.user_manager), [{
             'amount': -10.0,
         }, {
             'amount': -12.0,
diff --git a/addons/hr_timesheet/views/hr_timesheet_views.xml b/addons/hr_timesheet/views/hr_timesheet_views.xml
index eaf5013e498..c97cc5b0cc3 100644
--- a/addons/hr_timesheet/views/hr_timesheet_views.xml
+++ b/addons/hr_timesheet/views/hr_timesheet_views.xml
@@ -258,7 +258,8 @@
         <menuitem id="timesheet_menu_activity_all"
             name="All Timesheets"
             parent="menu_hr_time_tracking"
-            action="timesheet_action_all"/>
+            action="timesheet_action_all"
+            groups="hr_timesheet.group_hr_timesheet_approver"/>
 
     </data>
 </odoo>