From 96a223a5f0fc3ab63f2ce2010088148e8c8ae592 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 10 Apr 2018 17:31:22 +0200 Subject: [PATCH] [IMP] mail: prevent creation of activities to users that have no access to document This commit is related to task ID 58968 . Closes #24176. --- addons/mail/models/mail_activity.py | 29 ++++++++++++++++++ addons/test_mail/tests/test_mail_activity.py | 32 +++++++++++++++++++- 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/addons/mail/models/mail_activity.py b/addons/mail/models/mail_activity.py index 9cc9e8699a0..81975b9826d 100644 --- a/addons/mail/models/mail_activity.py +++ b/addons/mail/models/mail_activity.py @@ -186,6 +186,28 @@ class MailActivity(models.Model): _('The requested operation cannot be completed due to security restrictions. Please contact your system administrator.\n\n(Document type: %s, Operation: %s)') % (self._description, operation)) + @api.multi + def _check_access_assignation(self): + """ Check assigned user (user_id field) has access to the document. Purpose + is to allow assigned user to handle their activities. For that purpose + assigned user should be able to at least read the document. We therefore + raise an UserError if the assigned user has no access to the document. """ + for activity in self: + model = self.env[activity.res_model].sudo(activity.user_id.id) + try: + model.check_access_rights('read') + except exceptions.AccessError: + raise exceptions.UserError( + _('Assigned user %s has no access to the document and is not able to handle this activity.') % + activity.user_id.display_name) + else: + try: + model.browse(activity.res_id).check_access_rule('read') + except exceptions.AccessError: + raise exceptions.UserError( + _('Assigned user %s has no access to the document and is not able to handle this activity.') % + activity.user_id.display_name) + @api.model def create(self, values): # already compute default values to be sure those are computed using the current user @@ -196,6 +218,11 @@ class MailActivity(models.Model): activity = super(MailActivity, self.sudo()).create(values_w_defaults) activity_user = activity.sudo(self.env.user) activity_user._check_access('create') + + # check target user has rights on document otherwise we have to prevent activity creation + if activity_user.user_id != self.env.user: + activity_user._check_access_assignation() + self.env[activity_user.res_model].browse(activity_user.res_id).message_subscribe(partner_ids=[activity_user.user_id.partner_id.id]) if activity.date_deadline <= fields.Date.today(): self.env['bus.bus'].sendone( @@ -211,6 +238,8 @@ class MailActivity(models.Model): res = super(MailActivity, self.sudo()).write(values) if values.get('user_id'): + if values['user_id'] != self.env.uid: + self._check_access_assignation() for activity in self: self.env[activity.res_model].browse(activity.res_id).message_subscribe(partner_ids=[activity.user_id.partner_id.id]) if activity.date_deadline <= fields.Date.today(): diff --git a/addons/test_mail/tests/test_mail_activity.py b/addons/test_mail/tests/test_mail_activity.py index 64c87a43cc7..e8b9c980480 100644 --- a/addons/test_mail/tests/test_mail_activity.py +++ b/addons/test_mail/tests/test_mail_activity.py @@ -3,12 +3,15 @@ from datetime import date from dateutil.relativedelta import relativedelta +from unittest.mock import patch +from unittest.mock import DEFAULT from odoo import exceptions from odoo.addons.test_mail.tests.common import BaseFunctionalTest +from odoo.addons.test_mail.models.test_mail_models import MailTestActivity from odoo.tools import mute_logger - + class TestMailActivity(BaseFunctionalTest): @classmethod @@ -133,3 +136,30 @@ class TestMailActivity(BaseFunctionalTest): # Canceling activities should simply remove them self.assertEqual(self.test_record.activity_ids, self.env['mail.activity']) self.assertEqual(len(self.test_record.message_ids), 2) + + def test_activity_security_user_access(self): + activity = self.test_record.activity_schedule( + 'test_mail.mail_act_test_todo', + user_id=self.user_employee.id) + + activity2 = self.test_record.activity_schedule('test_mail.mail_act_test_todo') + activity2.write({'user_id': self.user_employee.id}) + + def test_activity_security_user_noaccess(self): + + def _employee_crash(*args, **kwargs): + """ If employee is test employee, consider he has no access on document """ + recordset = args[0] + if recordset.env.uid == self.user_employee.id: + raise exceptions.AccessError('Hop hop hop Ernest, please step back.') + return DEFAULT + + with patch.object(MailTestActivity, 'check_access_rights', autospec=True, side_effect=_employee_crash): + with self.assertRaises(exceptions.UserError): + activity = self.test_record.activity_schedule( + 'test_mail.mail_act_test_todo', + user_id=self.user_employee.id) + + activity2 = self.test_record.activity_schedule('test_mail.mail_act_test_todo') + with self.assertRaises(exceptions.UserError): + activity2.write({'user_id': self.user_employee.id})