From f6ed42db867fdad6da59a5b348b603793a59803b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 13 Sep 2012 19:05:00 +0200 Subject: [PATCH] [WIP] [FIX] mail: fixing access rights + related tests. WIP, already fixed mail.group and mail.alias, not readable by anonymous not-employee users. bzr revid: tde@openerp.com-20120913170500-um93rzw0nq6njl39 --- addons/mail/mail_message.py | 49 ++++++++++++---- addons/mail/security/ir.model.access.csv | 2 + addons/mail/tests/test_mail.py | 73 ++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 11 deletions(-) diff --git a/addons/mail/mail_message.py b/addons/mail/mail_message.py index 44ce76b94b0..cec3b89d9b6 100644 --- a/addons/mail/mail_message.py +++ b/addons/mail/mail_message.py @@ -24,6 +24,7 @@ import openerp import tools from email.header import decode_header +from openerp import SUPERUSER_ID from operator import itemgetter from osv import osv, fields from tools.translate import _ @@ -138,7 +139,8 @@ class mail_message(osv.Model): return [] def _get_default_author(self, cr, uid, context=None): - return self.pool.get('res.users').browse(cr, uid, uid, context=context).partner_id.id + # remove context to avoid possible hack in browse with superadmin using context keys that could trigger a specific behavior + return self.pool.get('res.users').browse(cr, SUPERUSER_ID, uid, context=None).partner_id.id _defaults = { 'type': 'email', @@ -274,15 +276,40 @@ class mail_message(osv.Model): if isinstance(ids, (int, long)): ids = [ids] - # check messages for which you have a notification - partner_id = self.pool.get('res.users').browse(cr, uid, uid, context=context).partner_id.id - not_obj = self.pool.get('mail.notification') - not_ids = not_obj.search(cr, uid, [ - ('partner_id', '=', partner_id), - ('message_id', 'in', ids), - ], context=context) - notified_ids = [notification.message_id.id for notification in not_obj.browse(cr, uid, not_ids, context=context) - if notification.message_id.id in ids] + # Rights + # - read: if + # - notification exist (I receive pushed message) OR + # - author_id = pid (I am the author) OR + # - I can read the related document + # - create: if + # - I can write on the related document OR + # - I am in the document message_follower_ids + # - write: if + # - I can write on the related document + # - unlink: if + # - I can write on the related document + # + + if operation != 'read': + notified_ids = [] + author_ids = [] + else: + # read: check for notifications + partner_id = self.pool.get('res.users').browse(cr, SUPERUSER_ID, uid, context=None).partner_id.id + not_obj = self.pool.get('mail.notification') + not_ids = not_obj.search(cr, uid, [ + ('partner_id', '=', partner_id), + ('message_id', 'in', ids), + ], context=context) + notified_ids = [notification.message_id.id for notification in not_obj.browse(cr, uid, not_ids, context=context)] + + # read: check messages you are author + author_ids = self.search(cr, uid, [('author_id', '=', partner_id), ('id', 'in', ids)], context=context) + + # if operation != 'create': + # follower_ids = [] + # else: + # write: check I am in the document followers # check messages linked to an existing document model_record_ids = {} @@ -300,7 +327,7 @@ class mail_message(osv.Model): model_obj.check_access_rule(cr, uid, mids, operation, context=context) # fall back on classic operation for other ids - other_ids = set(ids).difference(set(notified_ids), set(document_ids)) + other_ids = set(ids).difference(set(notified_ids), set(document_ids), set(author_ids)) super(mail_message, self).check_access_rule(cr, uid, other_ids, operation, context=None) def create(self, cr, uid, values, context=None): diff --git a/addons/mail/security/ir.model.access.csv b/addons/mail/security/ir.model.access.csv index c89c60756bd..e4f6faf735d 100644 --- a/addons/mail/security/ir.model.access.csv +++ b/addons/mail/security/ir.model.access.csv @@ -5,6 +5,8 @@ access_mail_mail_all,mail.mail.all,model_mail_mail,,1,1,1,1 access_mail_thread,mail.thread,model_mail_thread,base.group_user,1,1,1,0 access_mail_followers_all,mail.followers.all,model_mail_followers,,1,1,1,1 access_mail_notification_all,mail.notification.all,model_mail_notification,,1,1,1,1 +access_mail_group_all,mail.group.all,model_mail_group,,1,0,0,0 access_mail_group,mail.group,model_mail_group,base.group_user,1,1,1,1 +access_mail_alias_all,mail.alias.all,model_mail_alias,,1,0,0,0 access_mail_alias_user,mail.alias,model_mail_alias,base.group_user,1,1,1,0 access_mail_alias_system,mail.alias,model_mail_alias,base.group_system,1,1,1,1 diff --git a/addons/mail/tests/test_mail.py b/addons/mail/tests/test_mail.py index 1fab5dd1732..01646b6ad43 100644 --- a/addons/mail/tests/test_mail.py +++ b/addons/mail/tests/test_mail.py @@ -23,6 +23,7 @@ import tools from openerp.tests import common from openerp.tools.html_sanitize import html_sanitize +from osv.orm import except_orm MAIL_TEMPLATE = """Return-Path: To: {to} @@ -276,6 +277,78 @@ class test_mail(TestMailMockups): follower_ids = [follower.id for follower in group_pigs.message_follower_ids] self.assertEqual(follower_ids, [user_admin.partner_id.id], 'Admin must be the only Pigs fan') + def test_15_access_rights(self): + cr, uid = self.cr, self.uid + self.res_groups = self.registry('res.groups') + + self.mail_group.message_post(cr, uid, self.group_pigs_id, body='Message') + + # Find Employee group + group_employee_ref = self.registry('ir.model.data').get_object_reference(cr, uid, 'base', 'group_user') + group_employee_id = group_employee_ref and group_employee_ref[1] or False + group_employee = self.res_groups.browse(cr, uid, group_employee_id) + + self.group_jobs_id = self.mail_group.create(cr, uid, {'name': 'Jobs', 'public': 'public'}) + + user_bert_id = self.res_users.create(cr, uid, {'name': 'Bert Tartopoils', 'login': 'bert', 'groups_id': [(6, 0, [])]}) + user_raoul_id = self.res_users.create(cr, uid, {'name': 'Raoul Grosbedon', 'login': 'raoul', 'groups_id': [(6, 0, [group_employee_id])]}) + + user_bert = self.res_users.browse(cr, uid, user_bert_id) + user_raoul = self.res_users.browse(cr, uid, user_raoul_id) + + print '-----' + for group in user_bert.groups_id: + print group.name + print '-----' + for group in user_raoul.groups_id: + print group.name + + # Summary + # group_pigs: groups (Employee) + # group_jobs: public + + # ---------------------------------------- + # CASE1: Bert, without groups + # ---------------------------------------- + + # Do: Bert creates a group, should crash because perm_create only for employees + self.assertRaises(except_orm, + self.mail_group.create, + cr, user_bert_id, {'name': 'Bert\'s Group'}) + # Do: Bert reads Jobs basic fields, ok because public = read access on the group + self.mail_group.read(cr, user_bert_id, self.group_jobs_id, ['name', 'description']) + # Do: Bert reads Jobs messages, ok because read access on the group = read access on its messages + jobs_message_ids = self.mail_group.read(cr, user_bert_id, self.group_jobs_id, ['message_ids'])['message_ids'] + self.mail_message.read(cr, user_bert_id, jobs_message_ids) + # Do: Bert reads Jobs followers, ko because partner are accessible to employees or partner manager + jobs_followers_ids = self.mail_group.read(cr, user_bert_id, self.group_jobs_id, ['message_follower_ids'])['message_follower_ids'] + self.assertRaises(except_orm, + self.res_partner.read, + cr, user_bert_id, jobs_followers_ids) + # Do: Bert comments Jobs, ko because not write access on the group and not in the followers + self.assertRaises(except_orm, + self.mail_group.message_post, + cr, user_bert_id, self.group_jobs_id, body='I love Pigs') + + # Do: Bert reads Pigs, should crash because mail.group security=groups only for employee group + self.assertRaises(except_orm, + self.mail_group.read, + cr, user_bert_id, self.group_pigs_id) + + # Do: add Bert to jobs followers + self.mail_group.message_subscribe(cr, uid, [self.group_pigs_id], [user_bert.partner_id.id]) + # Do: Bert comments Jobs, ok because he is in the followers + self.mail_group.message_post(cr, user_bert_id, self.group_jobs_id, body='I love Pigs') + + # ---------------------------------------- + # CASE1: Raoul, employee + # ---------------------------------------- + + # Do: Bert read Jobs, ok because public + self.mail_group.read(cr, user_raoul_id, self.group_pigs_id) + # Do: Bert read Jobs, ok because public + self.mail_group.read(cr, user_raoul_id, self.group_jobs_id) + def test_20_message_post(self): """ Tests designed for message_post. """ cr, uid = self.cr, self.uid