From b90a8501b6263ff2e38b1cff29583872afc37439 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9rome=20Maes?= Date: Mon, 12 Dec 2016 17:05:12 +0100 Subject: [PATCH] [IMP] website_crm_partner_assign: access rights refactoring and add tests Portal user had too many access right on sensitive documents in this modules. Some ir.rule were missing, and some model access were useless. This commit clean all access right: ir.rule, ir.model.access and remove useless check in python code. Also, some tests were added to check access rights. --- .../__manifest__.py | 2 +- .../models/crm_lead.py | 7 +- .../security/ir.model.access.csv | 1 - .../security/ir_rule.xml | 28 ++++-- .../tests/test_partner_assign.py | 92 ++++++++++++++++++- 5 files changed, 111 insertions(+), 19 deletions(-) diff --git a/addons/website_crm_partner_assign/__manifest__.py b/addons/website_crm_partner_assign/__manifest__.py index de8449beca2..3bf2b7d2a2b 100644 --- a/addons/website_crm_partner_assign/__manifest__.py +++ b/addons/website_crm_partner_assign/__manifest__.py @@ -14,9 +14,9 @@ Publish and Assign Partner 'depends': ['base_geolocalize', 'crm', 'account', 'website_partner', 'website_google_map', 'website_portal'], 'data': [ + 'data/crm_partner_assign_data.xml', 'security/ir.model.access.csv', 'security/ir_rule.xml', - 'data/crm_partner_assign_data.xml', 'wizard/crm_forward_to_partner_view.xml', 'views/res_partner_views.xml', 'views/crm_lead_views.xml', diff --git a/addons/website_crm_partner_assign/models/crm_lead.py b/addons/website_crm_partner_assign/models/crm_lead.py index 6556deeec70..99a39ac9c91 100644 --- a/addons/website_crm_partner_assign/models/crm_lead.py +++ b/addons/website_crm_partner_assign/models/crm_lead.py @@ -190,24 +190,22 @@ class CrmLead(models.Model): @api.multi def partner_interested(self, comment=False): - self.check_access_rights('write') message = _('

I am interested by this lead.

') if comment: message += '

%s

' % comment for lead in self: lead.message_post(body=message, subtype="mail.mt_note") - lead.sudo().convert_opportunity(lead.partner_id.id) + lead.sudo().convert_opportunity(lead.partner_id.id) # sudo required to convert partner data @api.multi def partner_desinterested(self, comment=False, contacted=False, spam=False): - self.check_access_rights('write') if contacted: message = '

%s

' % _('I am not interested by this lead. I contacted the lead.') else: message = '

%s

' % _('I am not interested by this lead. I have not contacted the lead.') partner_ids = self.env['res.partner'].search( [('id', 'child_of', self.env.user.partner_id.commercial_partner_id.id)]) - self.sudo().message_unsubscribe(partner_ids=partner_ids.ids) + self.message_unsubscribe(partner_ids=partner_ids.ids) if comment: message += '

%s

' % comment self.message_post(body=message, subtype="mail.mt_note") @@ -226,7 +224,6 @@ class CrmLead(models.Model): @api.multi def update_lead_portal(self, values): self.check_access_rights('write') - for lead in self: lead_values = { 'planned_revenue': values['planned_revenue'], diff --git a/addons/website_crm_partner_assign/security/ir.model.access.csv b/addons/website_crm_partner_assign/security/ir.model.access.csv index 57a8da57da3..8f0f0c40dba 100644 --- a/addons/website_crm_partner_assign/security/ir.model.access.csv +++ b/addons/website_crm_partner_assign/security/ir.model.access.csv @@ -11,4 +11,3 @@ access_res_partner_activation_user,res.partner.activation.user,model_res_partner access_partner_activation_manager,res.partner.activation.manager,model_res_partner_activation,base.group_partner_manager,1,1,1,1 partner_access_crm_lead,crm.lead,crm.model_crm_lead,base.group_portal,1,1,0,0 access_res_partner_grade_invoicing_payment,res.partner.grade,model_res_partner_grade,account.group_account_invoice,1,0,0,0 -mail_message_subtype_portal_access,mail.message.subtype.portal,mail.model_mail_message_subtype,base.group_portal,1,1,1,0 diff --git a/addons/website_crm_partner_assign/security/ir_rule.xml b/addons/website_crm_partner_assign/security/ir_rule.xml index 2084cc5a8f4..23c72c27781 100644 --- a/addons/website_crm_partner_assign/security/ir_rule.xml +++ b/addons/website_crm_partner_assign/security/ir_rule.xml @@ -1,10 +1,24 @@ - - - odoo.portal.crm.lead - - - [('partner_assigned_id','child_of',user.commercial_partner_id.id)] - + + + Portal Graded Partner: read and write assigned leads + + + [('partner_assigned_id','child_of',user.commercial_partner_id.id)] + + + + + + + + + Portal/Public user: read only website published + + + [('website_published','=', True)] + + + diff --git a/addons/website_crm_partner_assign/tests/test_partner_assign.py b/addons/website_crm_partner_assign/tests/test_partner_assign.py index f5c056fa839..d38f908ac4c 100644 --- a/addons/website_crm_partner_assign/tests/test_partner_assign.py +++ b/addons/website_crm_partner_assign/tests/test_partner_assign.py @@ -1,18 +1,21 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +from odoo.exceptions import AccessError + from odoo.tests.common import TransactionCase +from odoo.addons.crm.tests.common import TestCrmCases class TestPartnerAssign(TransactionCase): - def test_00_partner_assign(self): + def test_partner_assign(self): + """ Test the automatic assignation using geolocalisation """ partner2 = self.env.ref('base.res_partner_2') lead = self.env.ref('crm.crm_case_21') - ''' - In order to test find nearest Partner functionality and assign to opportunity, - I Set Geo Lattitude and Longitude according to partner address. - ''' + + # In order to test find nearest Partner functionality and assign to opportunity, + # I Set Geo Lattitude and Longitude according to partner address. partner2.geo_localize() # I check Geo Latitude and Longitude of partner after set @@ -34,3 +37,82 @@ class TestPartnerAssign(TransactionCase): lead_forwarded.action_forward() except: pass + + +class TestPartnerLeadPortal(TestCrmCases): + + def setUp(self): + super(TestPartnerLeadPortal, self).setUp() + # Partner Grade + self.grade = self.env['res.partner.grade'].create({ + 'name': "Grade Test", + 'partner_weight': 42, + 'sequence': 3, + }) + # Integrating user/partner, having a salesman + self.portal_user = self.env['res.users'].with_context({'no_reset_password': True, 'mail_notrack': True}).create({ + 'name': 'Super Customer Odoo Intregrating Partner', + 'email': 'super.partner@ododo.com', + 'login': 'superpartner', + 'groups_id': [(4, self.env.ref('base.group_portal').id)], + 'user_id': self.crm_salesman.id, + 'grade_id': self.grade.id, + }) + self.portal_partner = self.portal_user.partner_id + # New lead, assigned to the new portal + self.lead = self.env['crm.lead'].with_context(mail_notrack=True).create({ + 'type': "lead", + 'name': "Test lead new", + 'user_id': False, + 'team_id': False, + 'description': "This is the description of the test new lead.", + 'partner_assigned_id': self.portal_partner.id + }) + # Sales Team of crm_salesman + self.team = self.env['crm.team'].with_context(mail_notrack=True).create({ + 'name': 'Test Team FOR THE WIN', + 'use_leads': True, + 'use_opportunities': True, + 'member_ids': [(6, 0, [self.crm_salesman.id])], + }) + + def test_partner_lead_accept(self): + """ Test an integrating partner accepting the lead """ + self.lead.sudo(self.portal_user.id).partner_interested(comment="Oh yeah, I take that lead !") + + self.assertEqual(self.lead.type, 'opportunity', 'Bad Type: accepted lead by portal user should become an opportunity.') + self.assertFalse(self.lead.team_id, 'Accepting lead does not change the sales team.') + self.assertFalse(self.lead.user_id, 'Accepting lead does not change the salesman.') + + def test_partner_lead_decline(self): + """ Test an integrating partner decline the lead """ + self.lead.sudo(self.portal_user.id).partner_desinterested(comment="No thanks, I have enough leads !", contacted=True, spam=False) + + self.assertFalse(self.lead.partner_assigned_id.id, 'The partner_assigned_id of the declined lead should be False.') + self.assertTrue(self.portal_user.partner_id in self.lead.sudo().partner_declined_ids, 'Partner who has declined the lead should be in the declined_partner_ids.') + + def test_lead_access_right(self): + """ Test another portal user can not write on every leads """ + # portal user having no right + poor_portal_user = self.env['res.users'].with_context({'no_reset_password': True, 'mail_notrack': True}).create({ + 'name': 'Poor Partner (not integrating one)', + 'email': 'poor.partner@ododo.com', + 'login': 'poorpartner', + 'groups_id': [(4, self.env.ref('base.group_portal').id)], + }) + # try to accept a lead that is not mine + with self.assertRaises(AccessError): + self.lead.sudo(poor_portal_user.id).partner_interested(comment="Oh yeah, I take that lead !") + + def test_lead_creation(self): + """ Test the opportinuty creation from portal """ + data = self.env['crm.lead'].sudo(self.portal_user.id).create_opp_portal({ + 'title': "L'ours bleu", + 'description': 'A good joke', + 'contact_name': 'Renaud Rutten', + }) + opportunity = self.env['crm.lead'].browse(data['id']) + salesmanteam = self.env['crm.team']._get_default_team_id(user_id=self.portal_user.user_id.id) + + self.assertEqual(opportunity.team_id, salesmanteam, 'The created opportunity should have the same team as the salesman default team of the opportunity creator.') + self.assertEqual(opportunity.partner_assigned_id, self.portal_partner, 'Assigned Partner of created opportunity is the (portal) creator.')