From 319f9bf5ea5aa374522befa8afb62c57d4fe90c7 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Wed, 18 May 2022 07:35:37 +0000 Subject: [PATCH] [IMP] mail: allow admins to remove channels they're not members of The only mail.channel rule was that a user only had access to channels they're members of, or can subscribe to (public, or group based). This rule applied to admins as well, forcing them to switch to super-admin mode in order to manage mail channels. This is undesirable on lots of axis: - superadmin mode is a bit of a last-ditch feature, as a result it's somewhat hidden - there is a much higher risk of screwing up as superadmin mode basically lifts all the access rules, which can have correctness implications - auditing completely breaks down when using superadmin mode, as the real identity of the user is lost OPW-2857136 closes odoo/odoo#92299 X-original-commit: 1a77ab5726bbcc477d12f40f67433d474ea85316 Signed-off-by: Xavier Morel (xmo) --- addons/mail/security/ir.model.access.csv | 3 ++- addons/mail/security/mail_security.xml | 14 ++++++++++++++ addons/mail/tests/test_mail_channel.py | 5 +++-- addons/test_discuss_full/tests/test_performance.py | 2 +- .../test_mail/tests/test_mail_thread_internals.py | 2 +- 5 files changed, 21 insertions(+), 5 deletions(-) diff --git a/addons/mail/security/ir.model.access.csv b/addons/mail/security/ir.model.access.csv index 5158c30bae2..cc467cc9b15 100644 --- a/addons/mail/security/ir.model.access.csv +++ b/addons/mail/security/ir.model.access.csv @@ -13,7 +13,8 @@ access_mail_notification_portal,mail.notification.portal,model_mail_notification access_mail_notification_user,mail.notification.user,model_mail_notification,base.group_user,1,1,1,0 access_mail_notification_system, mail.notification.system,model_mail_notification,base.group_system,1,1,1,1 access_mail_channel_all,mail.group.all,model_mail_channel,,1,0,0,0 -access_mail_channel_user,mail.group.user,model_mail_channel,base.group_user,1,1,1,1 +access_mail_channel_user,mail.group.user,model_mail_channel,base.group_user,1,1,1,0 +access_mail_channel_admin,mail.group.system,model_mail_channel,base.group_system,1,1,1,1 access_mail_channel_partner_public,mail.channel.partner.public,model_mail_channel_partner,base.group_public,1,0,0,0 access_mail_channel_partner_portal,mail.channel.partner.portal,model_mail_channel_partner,base.group_portal,1,1,1,1 access_mail_channel_partner_user,mail.channel.partner.user,model_mail_channel_partner,base.group_user,1,1,1,1 diff --git a/addons/mail/security/mail_security.xml b/addons/mail/security/mail_security.xml index 1c5fd4a30eb..cc6fca4671a 100644 --- a/addons/mail/security/mail_security.xml +++ b/addons/mail/security/mail_security.xml @@ -13,6 +13,13 @@ + + Mail.channel: admin full access + + + [(1, '=', 1)] + + mail.channel.partner: write its own entries @@ -27,6 +34,13 @@ + + mail.channel.partner: admin can manipulate all entries + + + [(1, '=', 1)] + + mail.notifications: group_user: write its own entries diff --git a/addons/mail/tests/test_mail_channel.py b/addons/mail/tests/test_mail_channel.py index c5c404b1848..a7f49bdd67e 100644 --- a/addons/mail/tests/test_mail_channel.py +++ b/addons/mail/tests/test_mail_channel.py @@ -85,8 +85,9 @@ class TestChannelAccessRights(MailCommon): # Employee update employee-based group: ok group_groups.write({'name': 'modified'}) - # Employee unlink employee-based group: ok - group_groups.unlink() + # Employee unlink employee-based group: ko + with self.assertRaises(AccessError): + group_groups.unlink() # Employee cannot read a private group with self.assertRaises(AccessError): diff --git a/addons/test_discuss_full/tests/test_performance.py b/addons/test_discuss_full/tests/test_performance.py index f430c8e5892..95ef678e4cd 100644 --- a/addons/test_discuss_full/tests/test_performance.py +++ b/addons/test_discuss_full/tests/test_performance.py @@ -60,7 +60,7 @@ class TestDiscussFullPerformance(TransactionCase): def test_init_messaging(self): """Test performance of `_init_messaging`.""" channel_general = self.env.ref('mail.channel_all_employees') # Unfortunately #general cannot be deleted. Assertions below assume data from a fresh db with demo. - self.env['mail.channel'].search([('id', '!=', channel_general.id)]).unlink() + self.env['mail.channel'].sudo().search([('id', '!=', channel_general.id)]).unlink() user_root = self.env.ref('base.user_root') # create public channels channel_channel_public_1 = self.env['mail.channel'].browse(self.env['mail.channel'].channel_create(name='public 1', privacy='public')['id']) diff --git a/addons/test_mail/tests/test_mail_thread_internals.py b/addons/test_mail/tests/test_mail_thread_internals.py index a86f2b30bb4..5d351c3a2ec 100644 --- a/addons/test_mail/tests/test_mail_thread_internals.py +++ b/addons/test_mail/tests/test_mail_thread_internals.py @@ -273,7 +273,7 @@ class TestDiscuss(TestMailCommon, TestRecipients): channel_message = self.env['mail.message'].sudo().search([('model', '=', 'mail.channel'), ('res_id', 'in', channel.ids)]) self.assertEqual(len(channel_message), 1, "Test message should have been posted") - channel.unlink() + channel.sudo().unlink() remaining_message = channel_message.exists() self.assertEqual(len(remaining_message), 0, "Test message should have been deleted")