From 347d0e5b468cf042cb6ea431c367bbbd54bdb39b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= <tde@openerp.com>
Date: Wed, 12 Dec 2012 13:55:18 +0100
Subject: [PATCH] [IMP] portal removed access rights of portal user on
 res_partner and derived tables. Updated several accesses to bypass the
 security issues. Updated portal tests to have a more complete test scenario.

bzr revid: tde@openerp.com-20121212125518-k0s07niojr8a1xtu
---
 addons/mail/mail_thread.py                    |  5 +-
 .../mail/wizard/mail_compose_message_view.xml |  2 +-
 addons/portal/mail_mail.py                    |  4 +-
 addons/portal/security/ir.model.access.csv    |  8 +--
 addons/portal/tests/test_portal.py            | 67 +++++++++++++------
 addons/portal_crm/contact.py                  |  6 +-
 6 files changed, 62 insertions(+), 30 deletions(-)

diff --git a/addons/mail/mail_thread.py b/addons/mail/mail_thread.py
index bb5d94843fc..ac2b0efb670 100644
--- a/addons/mail/mail_thread.py
+++ b/addons/mail/mail_thread.py
@@ -213,8 +213,11 @@ class mail_thread(osv.AbstractModel):
 
     def create(self, cr, uid, vals, context=None):
         """ Override to subscribe the current user. """
+        if context is None:
+            context = {}
         thread_id = super(mail_thread, self).create(cr, uid, vals, context=context)
-        self.message_subscribe_users(cr, uid, [thread_id], [uid], context=context)
+        if not context.get('mail_nosubscribe'):
+            self.message_subscribe_users(cr, uid, [thread_id], [uid], context=context)
         return thread_id
 
     def unlink(self, cr, uid, ids, context=None):
diff --git a/addons/mail/wizard/mail_compose_message_view.xml b/addons/mail/wizard/mail_compose_message_view.xml
index c0b329690e3..3b8e51f35d6 100644
--- a/addons/mail/wizard/mail_compose_message_view.xml
+++ b/addons/mail/wizard/mail_compose_message_view.xml
@@ -14,7 +14,7 @@
                         <field name="parent_id" invisible="1"/>
                         <!-- visible wizard -->
                         <label for="partner_ids" string="Recipients"/>
-                        <div>
+                        <div groups="base.group_user">
                             <span attrs="{'invisible':['|', ('model', '=', False), ('composition_mode', '!=', 'mass_mail')]}">
                                 Followers of selected items and
                             </span>
diff --git a/addons/portal/mail_mail.py b/addons/portal/mail_mail.py
index c7cf0eb5d65..a18c71759f8 100644
--- a/addons/portal/mail_mail.py
+++ b/addons/portal/mail_mail.py
@@ -19,10 +19,12 @@
 #
 ##############################################################################
 
+from openerp import SUPERUSER_ID
 from openerp.osv import osv
 from openerp.tools import append_content_to_html
 from openerp.tools.translate import _
 
+
 class mail_mail(osv.Model):
     """ Update of mail_mail class, to add the signin URL to notifications. """
     _inherit = 'mail.mail'
@@ -36,7 +38,7 @@ class mail_mail(osv.Model):
         body = super(mail_mail, self).send_get_mail_body(cr, uid, mail, partner, context=context)
         if partner:
             context = dict(context or {}, signup_valid=True)
-            partner = self.pool.get('res.partner').browse(cr, uid, partner.id, context)
+            partner = self.pool.get('res.partner').browse(cr, SUPERUSER_ID, partner.id, context=context)
             text = _("""Access your personal documents through <a href="%s">our Customer Portal</a>""") % partner.signup_url
             body = append_content_to_html(body, ("<div><p>%s</p></div>" % text), plaintext=False)
         return body
diff --git a/addons/portal/security/ir.model.access.csv b/addons/portal/security/ir.model.access.csv
index 59ca37e213d..88d50c739a5 100644
--- a/addons/portal/security/ir.model.access.csv
+++ b/addons/portal/security/ir.model.access.csv
@@ -1,10 +1,8 @@
 id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
-access_mail_message_portal,mail.message.portal,mail.model_mail_message,group_portal,1,0,1,1
+access_mail_message_portal,mail.message.portal,mail.model_mail_message,group_portal,1,1,1,1
+access_mail_mail_portal,mail.mail.portal,mail.model_mail_mail,group_portal,1,1,1,0
 access_mail_notification_portal,mail.notification.portal,mail.model_mail_notification,group_portal,1,1,1,0
-access_res_partner,res.partner,base.model_res_partner,portal.group_portal,1,0,0,0
-access_res_partner_address,res.partner_address,base.model_res_partner_address,portal.group_portal,1,0,0,0
-access_res_partner_category,res.partner_category,base.model_res_partner_category,portal.group_portal,1,0,0,0
-access_res_partner_title,res.partner_title,base.model_res_partner_title,portal.group_portal,1,0,0,0
+access_mail_followers_portal,mail.followers.portal,mail.model_mail_followers,group_portal,1,1,0,0
 access_acquirer,portal.payment.acquirer,portal.model_portal_payment_acquirer,,1,0,0,0
 access_acquirer_all,portal.payment.acquirer,portal.model_portal_payment_acquirer,base.group_system,1,1,1,1
 access_ir_attachment_group_portal,ir.attachment group_portal,base.model_ir_attachment,portal.group_portal,1,0,1,0
\ No newline at end of file
diff --git a/addons/portal/tests/test_portal.py b/addons/portal/tests/test_portal.py
index 99ece6cc36d..03f2491315b 100644
--- a/addons/portal/tests/test_portal.py
+++ b/addons/portal/tests/test_portal.py
@@ -35,41 +35,68 @@ class test_portal(TestMailBase):
         self.group_portal_id = group_portal.id
 
         # Create Chell (portal user)
-        self.user_chell_id = self.res_users.create(cr, uid, {'name': 'Chell Gladys', 'login': 'chell', 'groups_id': [(6, 0, [self.group_portal_id])]})
-        user_chell = self.res_users.browse(cr, uid, self.user_chell_id)
-        self.partner_chell_id = user_chell.partner_id.id
+        self.user_chell_id = self.res_users.create(cr, uid, {'name': 'Chell Gladys', 'login': 'chell', 'email': 'chell@gladys.portal', 'groups_id': [(6, 0, [self.group_portal_id])]})
+        self.user_chell = self.res_users.browse(cr, uid, self.user_chell_id)
+        self.partner_chell_id = self.user_chell.partner_id.id
+
+        # Create a PigsPortal group
+        self.group_port_id = self.mail_group.create(cr, uid, {'name': 'PigsPortal', 'public': 'groups', 'group_public_id': self.group_portal_id})
 
         # Set an email address for the user running the tests, used as Sender for outgoing mails
         self.res_users.write(cr, uid, uid, {'email': 'test@localhost'})
 
-    @mute_logger('openerp.addons.base.ir.ir_model')
-    def test_00_access_rights(self):
+    @mute_logger('openerp.addons.base.ir.ir_model', 'openerp.osv.orm')
+    def test_00_mail_access_rights(self):
         """ Test basic mail_message and mail_group access rights for portal users. """
         cr, uid = self.cr, self.uid
+        mail_compose = self.registry('mail.compose.message')
 
-        # Prepare group: Pigs (portal)
-        self.mail_group.message_post(cr, uid, self.group_pigs_id, body='Message')
-        self.mail_group.write(cr, uid, [self.group_pigs_id], {'name': 'Jobs', 'public': 'groups', 'group_public_id': self.group_portal_id})
+        # Prepare group: Pigs and PigsPortal
+        pigs_msg_id = self.mail_group.message_post(cr, uid, self.group_pigs_id, body='Message')
+        port_msg_id = self.mail_group.message_post(cr, uid, self.group_port_id, body='Message')
 
-        # ----------------------------------------
-        # CASE1: Chell will use the Chatter
-        # ----------------------------------------
-
-        # Do: Chell reads Pigs messages, ok because restricted to portal group
-        message_ids = self.mail_group.read(cr, self.user_chell_id, self.group_pigs_id, ['message_ids'])['message_ids']
-        self.mail_message.read(cr, self.user_chell_id, message_ids)
+        # Do: Chell browses Pigs -> ko, employee group
+        chell_pigs = self.mail_group.browse(cr, self.user_chell_id, self.group_pigs_id)
+        with self.assertRaises(except_orm):
+            trigger_read = chell_pigs.name
 
         # Do: Chell posts a message on Pigs, crash because can not write on group or is not in the followers
         with self.assertRaises(except_orm):
             self.mail_group.message_post(cr, self.user_chell_id, self.group_pigs_id, body='Message')
 
-        # Do: Chell is added to Pigs followers
-        self.mail_group.message_subscribe(cr, uid, [self.group_pigs_id], [self.partner_chell_id])
+        # Do: Chell is added into Pigs followers and browse it -> ok for messages, ko for partners (no read permission)
+        self.mail_group.message_subscribe_users(cr, uid, [self.group_pigs_id], [self.user_chell_id])
+        chell_pigs = self.mail_group.browse(cr, self.user_chell_id, self.group_pigs_id)
+        trigger_read = chell_pigs.name
+        for message in chell_pigs.message_ids:
+            trigger_read = message.subject
+        for partner in chell_pigs.message_follower_ids:
+            with self.assertRaises(except_orm):
+                trigger_read = partner.name
 
-        # Test: Chell posts a message on Pigs, ok because in the followers
-        self.mail_group.message_post(cr, self.user_chell_id, self.group_pigs_id, body='Message')
+        # Do: Chell comments Pigs, ok because he is now in the followers
+        self.mail_group.message_post(cr, self.user_chell_id, self.group_pigs_id, body='I love Pigs')
+        # Do: Chell creates a mail.compose.message record on Pigs, because he uses the wizard
+        compose_id = mail_compose.create(cr, self.user_chell_id,
+            {'subject': 'Subject', 'body': 'Body text', 'partner_ids': []},
+            {'default_composition_mode': 'comment', 'default_model': 'mail.group', 'default_res_id': self.group_pigs_id})
+        mail_compose.send_mail(cr, self.user_chell_id, [compose_id])
+        # Do: Chell replies to a Pigs message using the composer
+        compose_id = mail_compose.create(cr, self.user_chell_id,
+            {'subject': 'Subject', 'body': 'Body text'},
+            {'default_composition_mode': 'reply', 'default_parent_id': pigs_msg_id})
+        mail_compose.send_mail(cr, self.user_chell_id, [compose_id])
 
-    def test_50_mail_invite(self):
+        # Do: Chell browses PigsPortal -> ok because groups security, ko for partners (no read permission)
+        chell_port = self.mail_group.browse(cr, self.user_chell_id, self.group_port_id)
+        trigger_read = chell_port.name
+        for message in chell_port.message_ids:
+            trigger_read = message.subject
+        for partner in chell_port.message_follower_ids:
+            with self.assertRaises(except_orm):
+                trigger_read = partner.name
+
+    def test_10_mail_invite(self):
         cr, uid = self.cr, self.uid
         mail_invite = self.registry('mail.wizard.invite')
         base_url = self.registry('ir.config_parameter').get_param(cr, uid, 'web.base.url', default='')
diff --git a/addons/portal_crm/contact.py b/addons/portal_crm/contact.py
index 132f4db34d5..1cc10344d67 100644
--- a/addons/portal_crm/contact.py
+++ b/addons/portal_crm/contact.py
@@ -96,14 +96,16 @@ class crm_contact_us(osv.TransientModel):
         Therefore, user SUPERUSER_ID will perform the creation.
         """
         values['contact_name'] = values['partner_name']
-        crm_lead.create(cr, SUPERUSER_ID, dict(values,user_id=False), context)
+        crm_lead.create(cr, SUPERUSER_ID, dict(values, user_id=False), context)
 
         """
         Create an empty record in the contact table.
         Since the 'name' field is mandatory, give an empty string to avoid an integrity error.
+        Pass mail_nosubscribe key in context because otherwise the inheritance
+        leads to a message_subscribe_user, that triggers access right issues.
         """
         empty_values = dict((k, False) if k != 'name' else (k, '') for k, v in values.iteritems())
-        return super(crm_contact_us, self).create(cr, uid, empty_values)
+        return super(crm_contact_us, self).create(cr, uid, empty_values, {'mail_nosubscribe': True})
 
     def submit(self, cr, uid, ids, context=None):
         """ When the form is submitted, redirect the user to a "Thanks" message """