Skip to content

Commit c27d978

Browse files
[FIX] hr_holidays: support portal employees when approving leaves
Description of the issue this commit addresses: In `hr.leave`, when an HR manager approves the leave request of an employee, through `_validate_leave_request`, an impersonation using `with_user` is done to create the `calendar.event` on behalf of the employee. The permissions checked are the ones of the impersonated employee rather than the ones of the manager. Some customers do a customization to allow employees to be linked to portal users for the holidays management, and added ACLs and record rules to support this. A bugfix revision added recently broke this customization: odoo/odoo-security@7e550f2 If the user linked to the employee is a portal user, the record rule with the below domain applies: `[('partner_ids', 'in', user.partner_id.id)]` It's a default ACL coming from the standard calendar module: https://github.com/odoo/odoo/blob/cc0060e889603eb2e47fa44a8a22a70d7d784185/addons/calendar/security/calendar_security.xml#L8 During the creation of the event, `partner_ids` is well passed to allow the portal user to create and read its own event: https://github.com/odoo/odoo/blob/cc0060e889603eb2e47fa44a8a22a70d7d784185/addons/hr_holidays/models/hr_leave.py#L926-L929 https://github.com/odoo/odoo/blob/cc0060e889603eb2e47fa44a8a22a70d7d784185/addons/hr_holidays/models/hr_leave.py#L892C59-L892C91 Also, in the `create` of `calendar.event`, `attendee_ids` are added automatically according to the `partner_ids`: https://github.com/odoo/odoo/blob/cc0060e889603eb2e47fa44a8a22a70d7d784185/addons/calendar/models/calendar_event.py#L760-L768 During the `create` of the ORM, the field `attendee_ids` is treated before the field `partner_ids`. The order is defined using the `_sequence` attribute of the fields: https://github.com/odoo/odoo/blob/cc0060e889603eb2e47fa44a8a22a70d7d784185/odoo/models.py#L4056 This has the unfortunate effect to call `create` of `calendar.attendee` before the `partner_ids` gets set, and to call: ```py _name = 'calendar.attendee' ... def create(self, vals_list): ... attendees.event_id.check_access_rule('write') ``` The access rule is checked before `partner_ids` is set, while the record rule domain relies on it: `[('partner_ids', 'in', user.partner_id.id)]` Therefore raising an `AccessError`, despite the fact the `partner_ids` were well passed to the `vals` in the `calendar.event.create` call. Desired behavior after this commit is merged: As a fix, the permissions that are checked are the one of the user creating the record, if the check passes, the record is created ignoring the permissions of the impersonated user. task-5136270 Co-authored-by: Denis Ledoux <dle@odoo.com>
1 parent 6545db8 commit c27d978

File tree

2 files changed

+105
-4
lines changed

2 files changed

+105
-4
lines changed

addons/hr_holidays/models/hr_leave.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
from odoo.exceptions import AccessError, UserError, ValidationError
1919
from odoo.tools import float_compare, format_date
2020
from odoo.tools.float_utils import float_round
21-
from odoo.tools.misc import format_date
21+
from odoo.tools.misc import clean_context
2222
from odoo.tools.translate import _
2323
from odoo.osv import expression
2424

@@ -1097,15 +1097,18 @@ def _validate_leave_request(self):
10971097
meeting_holidays = holidays.filtered(lambda l: l.holiday_status_id.create_calendar_meeting)
10981098
meetings = self.env['calendar.event']
10991099
if meeting_holidays:
1100+
Meeting = self.env['calendar.event']
1101+
Meeting.check_access_rights('create')
1102+
Meeting.check_access_rule('create')
11001103
meeting_values_for_user_id = meeting_holidays._prepare_holidays_meeting_values()
11011104
Meeting = self.env['calendar.event']
11021105
for user_id, meeting_values in meeting_values_for_user_id.items():
1103-
meetings += Meeting.with_user(user_id or self.env.uid).with_context(
1106+
meetings += Meeting.with_user(user_id or self.env.uid).sudo().with_context(clean_context({**self.env.context, **dict(
11041107
allowed_company_ids=[],
11051108
no_mail_to_attendees=True,
11061109
calendar_no_videocall=True,
11071110
active_model=self._name
1108-
).create(meeting_values)
1111+
)})).create(meeting_values)
11091112
Holiday = self.env['hr.leave']
11101113
for meeting in meetings:
11111114
Holiday.browse(meeting.res_id).meeting_id = meeting

addons/hr_holidays/tests/test_leave_requests.py

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from pytz import timezone, UTC
88

99
from odoo import fields
10-
from odoo.exceptions import UserError, ValidationError
10+
from odoo.exceptions import AccessError, UserError, ValidationError
1111
from odoo.tools import mute_logger
1212
from odoo.tests.common import Form
1313
from odoo.tests import tagged
@@ -1182,3 +1182,101 @@ def test_leave_creation_without_allocation(self):
11821182
'request_date_from': '2024-06-01',
11831183
'request_date_to': '2024-06-02',
11841184
})
1185+
1186+
def test_calendar_event_create_access_rights(self):
1187+
"""Test that a manager can validate a leave request for an employee linked to a portal user.
1188+
Customers defined custom ACLs and record rules to support the possibility to assign a portal user to employees
1189+
and still be able to manage their holidays.
1190+
"""
1191+
# Add the required ACLs and record rules to allow portal users to create `calendar.event`.
1192+
# This reflects the customization done by customers for the reason explained above.
1193+
self.env['ir.model.access'].create([
1194+
# Read access on `mail.activity.type` for portal required for
1195+
# https://github.com/odoo/odoo/blob/cc0060e889603eb2e47fa44a8a22a70d7d784185/addons/calendar/models/calendar_event.py#L734
1196+
{
1197+
'name': 'Portal can read mail.activity.type',
1198+
'model_id': self.env.ref('mail.model_mail_activity_type').id,
1199+
'group_id': self.env.ref('base.group_portal').id,
1200+
'perm_read': True, 'perm_create': False, 'perm_write': False, 'perm_unlink': False,
1201+
},
1202+
# Read access on `mail.activity` for portal required for
1203+
# https://github.com/odoo/odoo/blob/cc0060e889603eb2e47fa44a8a22a70d7d784185/addons/calendar/models/calendar_event.py#L786
1204+
# https://github.com/odoo/odoo/blob/cc0060e889603eb2e47fa44a8a22a70d7d784185/addons/calendar/models/calendar_event.py#L882
1205+
{
1206+
'name': 'Portal can read mail.activity',
1207+
'model_id': self.env.ref('mail.model_mail_activity').id,
1208+
'group_id': self.env.ref('base.group_portal').id,
1209+
'perm_read': True, 'perm_create': False, 'perm_write': False, 'perm_unlink': False,
1210+
},
1211+
# Read and create acess on `calendar.event` for portal required for
1212+
# https://github.com/odoo/odoo/blob/cc0060e889603eb2e47fa44a8a22a70d7d784185/addons/hr_holidays/models/hr_leave.py#L894-L898
1213+
# Write and unlink added to match the customer customization + out of common sense,
1214+
# if you give create to portal for their own events,
1215+
# you give write and unlink so they can manage their own events
1216+
{
1217+
'name': 'Portal all CRUD on calendar.event',
1218+
'model_id': self.env.ref('calendar.model_calendar_event').id,
1219+
'group_id': self.env.ref('base.group_portal').id,
1220+
'perm_read': True, 'perm_create': True, 'perm_write': True, 'perm_unlink': True,
1221+
},
1222+
# Read and create acess on `calendar.event` for portal required for
1223+
# https://github.com/odoo/odoo/blob/cc0060e889603eb2e47fa44a8a22a70d7d784185/addons/calendar/models/calendar_event.py#L760-L768
1224+
# Write and unlink added to match the customer customization + out of common sense,
1225+
# if you give create to portal for their own events attendees,
1226+
# you give write and unlink so they can manage their own attendees
1227+
{
1228+
'name': 'Portal all CRUD on calendar.attendee',
1229+
'model_id': self.env.ref('calendar.model_calendar_attendee').id,
1230+
'group_id': self.env.ref('base.group_portal').id,
1231+
'perm_read': True, 'perm_create': True, 'perm_write': True, 'perm_unlink': True,
1232+
}])
1233+
self.env['ir.rule'].create([
1234+
# Restrict portals to their own activities
1235+
# so they cannot read the activities of other users
1236+
{
1237+
'name': 'Portal own mail activity',
1238+
'model_id': self.env.ref('mail.model_mail_activity').id,
1239+
'groups': [(4, self.env.ref('base.group_portal').id)],
1240+
'domain_force': "['|', ('user_id', '=', user.id), ('create_uid', '=', user.id)]",
1241+
},
1242+
# Restrict portals to their own events
1243+
# so they cannot read the events of other users
1244+
{
1245+
'name': 'Portal own calendar events',
1246+
'model_id': self.env.ref('calendar.model_calendar_event').id,
1247+
'groups': [(4, self.env.ref('base.group_portal').id)],
1248+
'domain_force': "[('partner_ids', 'in', user.partner_id.id)]",
1249+
},
1250+
# Restrict portals to their own attendees
1251+
# so they cannot read the attendees of other users
1252+
{
1253+
'name': 'Portal own calendar attendees',
1254+
'model_id': self.env.ref('calendar.model_calendar_attendee').id,
1255+
'groups': [(4, self.env.ref('base.group_portal').id)],
1256+
'domain_force': "[('partner_id', '=', user.partner_id.id)]",
1257+
}
1258+
])
1259+
1260+
# Create a portal user and assign it to the employee
1261+
user_portal = self.env['res.users'].create({
1262+
'name': 'Portal', 'login': 'portal_user', 'password': 'portal_user',
1263+
'groups_id': [(6, 0, [self.env.ref('base.group_portal').id])],
1264+
})
1265+
self.employee_emp.user_id = user_portal
1266+
1267+
# As a manager, create a leave request for the employee linked to a portal user
1268+
leave = self.env['hr.leave'].with_user(self.user_hrmanager_id).create({
1269+
'name': 'Holiday Request',
1270+
'employee_id': self.employee_emp_id,
1271+
'holiday_status_id': self.holidays_type_1.id,
1272+
'date_from': (datetime.today() - relativedelta(days=1)),
1273+
'date_to': datetime.today(),
1274+
'number_of_days': 1,
1275+
})
1276+
1277+
# Assert the employee cannot approve his own leave request
1278+
with self.assertRaises(AccessError):
1279+
leave.with_user(self.user_employee_id).action_approve()
1280+
1281+
# Assert the manager can approve the leave request assign to portal employee
1282+
leave.with_user(self.user_hrmanager_id).action_approve()

0 commit comments

Comments
 (0)