diff --git a/addons/digest/models/digest.py b/addons/digest/models/digest.py index cfd7fbad346..244c5106792 100644 --- a/addons/digest/models/digest.py +++ b/addons/digest/models/digest.py @@ -234,7 +234,7 @@ class Digest(models.Model): '|', ('group_id', 'in', user.groups_id.ids), ('group_id', '=', False) ], limit=tips_count) tip_descriptions = [ - self.env['mail.render.mixin']._render_template(tools.html_sanitize(tip.tip_description), 'digest.tip', tip.ids, post_process=True)[tip.id] + self.env['mail.render.mixin'].sudo()._render_template(tools.html_sanitize(tip.tip_description), 'digest.tip', tip.ids, post_process=True)[tip.id] for tip in tips ] if consumed: diff --git a/addons/mail/__manifest__.py b/addons/mail/__manifest__.py index 18a79b75080..08583e60bc5 100644 --- a/addons/mail/__manifest__.py +++ b/addons/mail/__manifest__.py @@ -10,6 +10,7 @@ 'website': 'https://www.odoo.com/app/discuss', 'depends': ['base', 'base_setup', 'bus', 'web_tour'], 'data': [ + 'data/mail_groups.xml', 'wizard/mail_blacklist_remove_views.xml', 'wizard/mail_compose_message_views.xml', 'wizard/mail_resend_cancel_views.xml', diff --git a/addons/mail/data/mail_groups.xml b/addons/mail/data/mail_groups.xml new file mode 100644 index 00000000000..c7ae40e52a3 --- /dev/null +++ b/addons/mail/data/mail_groups.xml @@ -0,0 +1,18 @@ + + + + + Mail Template Editor + + + + + + + + + + + + + diff --git a/addons/mail/models/ir_config_parameter.py b/addons/mail/models/ir_config_parameter.py index a3f67ce1331..977ad47c802 100644 --- a/addons/mail/models/ir_config_parameter.py +++ b/addons/mail/models/ir_config_parameter.py @@ -19,3 +19,17 @@ class IrConfigParameter(models.Model): if 'value' in vals and parameter.key in ['mail.bounce.alias', 'mail.catchall.alias'] and vals['value'] != parameter.value: vals['value'] = self.env['mail.alias']._clean_and_check_unique([vals.get('value')])[0] return super().write(vals) + + @api.model + def set_param(self, key, value): + if key == 'mail.restrict.template.rendering': + group_user = self.env.ref('base.group_user') + group_mail_template_editor = self.env.ref('mail.group_mail_template_editor') + + if not value and group_mail_template_editor not in group_user.implied_ids: + group_user.implied_ids |= group_mail_template_editor + + elif value and group_mail_template_editor in group_user.implied_ids: + group_user.implied_ids -= group_mail_template_editor + + return super(IrConfigParameter, self).set_param(key, value) diff --git a/addons/mail/models/mail_composer_mixin.py b/addons/mail/models/mail_composer_mixin.py index 03d4fcc6a87..6f531872905 100644 --- a/addons/mail/models/mail_composer_mixin.py +++ b/addons/mail/models/mail_composer_mixin.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo import api, fields, models +from odoo import api, fields, models, tools, _ class MailComposerMixin(models.AbstractModel): @@ -25,6 +25,7 @@ class MailComposerMixin(models.AbstractModel): body = fields.Html('Contents', sanitize_style=True, compute='_compute_body', store=True, readonly=False) template_id = fields.Many2one('mail.template', 'Mail Template', domain="[('model', '=', render_model)]") # Access + is_mail_template_editor = fields.Boolean('Is Editor', compute='_compute_is_mail_template_editor') can_edit_body = fields.Boolean('Can Edit Body', compute='_compute_can_edit_body') @api.depends('template_id') @@ -43,7 +44,50 @@ class MailComposerMixin(models.AbstractModel): elif not composer_mixin.body: composer_mixin.body = False - @api.depends('template_id') @api.depends_context('uid') + def _compute_is_mail_template_editor(self): + is_mail_template_editor = self.env.is_admin() or self.env.user.has_group('mail.group_mail_template_editor') + for record in self: + record.is_mail_template_editor = is_mail_template_editor + + @api.depends('template_id', 'is_mail_template_editor') def _compute_can_edit_body(self): - self.can_edit_body = True + for record in self: + record.can_edit_body = ( + record.is_mail_template_editor + or not record.template_id + ) + + def _render_field(self, field, *args, **kwargs): + """Render the given field on the given records. + This method bypass the rights when needed to + be able to render the template values in mass mode. + """ + if field not in self._fields: + raise ValueError(_("The field %s does not exist on the model %s", field, self._name)) + + composer_value = self[field] + + if ( + not self.template_id + or self.is_mail_template_editor + ): + # Do not need to bypass the verification + return super(MailComposerMixin, self)._render_field(field, *args, **kwargs) + + template_field = 'body_html' if field == 'body' else field + assert template_field in self.template_id._fields + template_value = self.template_id[template_field] + + if field == 'body': + sanitized_template_value = tools.html_sanitize(template_value) + if not self.can_edit_body or composer_value in (sanitized_template_value, template_value): + # Take the previous body which we can trust without HTML editor reformatting + self.body = self.template_id.body_html + return super(MailComposerMixin, self.sudo())._render_field(field, *args, **kwargs) + + elif composer_value == template_value: + # The value is the same as the mail template so we trust it + return super(MailComposerMixin, self.sudo())._render_field(field, *args, **kwargs) + + return super(MailComposerMixin, self)._render_field(field, *args, **kwargs) diff --git a/addons/mail/models/mail_render_mixin.py b/addons/mail/models/mail_render_mixin.py index 27b7613669f..9cbc788e7a6 100644 --- a/addons/mail/models/mail_render_mixin.py +++ b/addons/mail/models/mail_render_mixin.py @@ -3,18 +3,17 @@ import babel import copy -import functools import logging import re -import dateutil.relativedelta as relativedelta from lxml import html from markupsafe import Markup from werkzeug import urls from odoo import _, api, fields, models, tools -from odoo.exceptions import UserError +from odoo.exceptions import UserError, AccessError from odoo.tools import is_html_empty, safe_eval +from odoo.tools.jinja import jinja_safe_template_env, jinja_template_env, template_env_globals _logger = logging.getLogger(__name__) @@ -38,61 +37,15 @@ def format_time(env, time, tz=False, time_format='medium', lang_code=False): except babel.core.UnknownLocaleError: return time -def relativedelta_proxy(*args, **kwargs): - # dateutil.relativedelta is an old-style class and cannot be directly - # instanciated wihtin a jinja2 expression, so a lambda "proxy" is - # is needed, apparently - return relativedelta.relativedelta(*args, **kwargs) - -template_env_globals = { - 'str': str, - 'quote': urls.url_quote, - 'urlencode': urls.url_encode, - 'datetime': safe_eval.datetime, - 'len': len, - 'abs': abs, - 'min': min, - 'max': max, - 'sum': sum, - 'filter': filter, - 'reduce': functools.reduce, - 'map': map, - 'relativedelta': relativedelta_proxy, - 'round': round, -} - -try: - # We use a jinja2 sandboxed environment to render mako templates. - # Note that the rendering does not cover all the mako syntax, in particular - # arbitrary Python statements are not accepted, and not all expressions are - # allowed: only "public" attributes (not starting with '_') of objects may - # be accessed. - # This is done on purpose: it prevents incidental or malicious execution of - # Python code that may break the security of the server. - from jinja2.sandbox import SandboxedEnvironment - jinja_template_env = SandboxedEnvironment( - block_start_string="<%", - block_end_string="%>", - variable_start_string="${", - variable_end_string="}", - comment_start_string="<%doc>", - comment_end_string="", - line_statement_prefix="%", - line_comment_prefix="##", - trim_blocks=True, # do not output newline after blocks - autoescape=True, # XML/HTML automatic escaping - ) - jinja_template_env.globals.update(template_env_globals) - jinja_safe_template_env = copy.copy(jinja_template_env) - jinja_safe_template_env.autoescape = False -except ImportError: - _logger.warning("jinja2 not available, templating features will not work!") - class MailRenderMixin(models.AbstractModel): _name = 'mail.render.mixin' _description = 'Mail Render Mixin' + # If True, we trust the value on the model for rendering + # If False, we need the group "Template Editor" to render the model fields + _unrestricted_rendering = False + # language for rendering lang = fields.Char( 'Language', @@ -421,6 +374,19 @@ class MailRenderMixin(models.AbstractModel): _logger.info("Failed to load template %r", template_txt, exc_info=True) return results + if (not self._unrestricted_rendering and template.is_dynamic and not self.env.is_admin() and + not self.env.user.has_group('mail.group_mail_template_editor')): + group = self.env.ref('mail.group_mail_template_editor') + raise AccessError(_('Only users belonging to the "%s" group can modify dynamic templates.', group.name)) + + if not template.is_dynamic: + # Either the content is a raw text without placeholders, either we fail to + # detect placeholders code. In both case we skip the rendering and return + # the raw content, so even if we failed to detect dynamic code, + # non "mail_template_editor" users will not gain rendering tools available + # only for template specific group users + return {record_id: template_txt for record_id in res_ids} + # prepare template variables variables = self._render_jinja_eval_context() if add_context: diff --git a/addons/mail/models/mail_template.py b/addons/mail/models/mail_template.py index 9f79d717a7c..034500c55ff 100644 --- a/addons/mail/models/mail_template.py +++ b/addons/mail/models/mail_template.py @@ -18,6 +18,8 @@ class MailTemplate(models.Model): _description = 'Email Templates' _order = 'name' + _unrestricted_rendering = True + @api.model def default_get(self, fields): res = super(MailTemplate, self).default_get(fields) diff --git a/addons/mail/models/res_config_settings.py b/addons/mail/models/res_config_settings.py index f2272520160..d9686cbe4ac 100644 --- a/addons/mail/models/res_config_settings.py +++ b/addons/mail/models/res_config_settings.py @@ -14,6 +14,11 @@ class ResConfigSettings(models.TransientModel): fail_counter = fields.Integer('Fail Mail', readonly=True) alias_domain = fields.Char('Alias Domain', help="If you have setup a catch-all email domain redirected to " "the Odoo server, enter the domain name here.", config_parameter='mail.catchall.domain') + restrict_template_rendering = fields.Boolean( + 'Restrict Template Rendering', + config_parameter='mail.restrict.template.rendering', + help='Users will still be able to render templates.\n' + 'However only Mail Template Editors will be able to create new dynamic templates or modify existing ones.') @api.model def get_values(self): diff --git a/addons/mail/security/ir.model.access.csv b/addons/mail/security/ir.model.access.csv index 3ae8196eafb..cd3d0673ac5 100644 --- a/addons/mail/security/ir.model.access.csv +++ b/addons/mail/security/ir.model.access.csv @@ -27,7 +27,8 @@ access_mail_tracking_value_portal,mail.tracking.value.portal,model_mail_tracking access_mail_tracking_value_user,mail.tracking.value.user,model_mail_tracking_value,base.group_user,0,0,0,0 access_mail_tracking_value_system,mail.tracking.value.system,model_mail_tracking_value,base.group_system,1,1,1,1 access_publisher_warranty_contract_all,publisher.warranty.contract.all,model_publisher_warranty_contract,,1,1,1,1 -access_mail_template,mail.template,model_mail_template,base.group_user,1,1,1,0 +access_mail_template,mail.template,model_mail_template,base.group_user,1,0,0,0 +access_mail_template_editor,mail.template_editor,model_mail_template,mail.group_mail_template_editor,1,1,1,1 access_mail_template_system,mail.template_system,model_mail_template,base.group_system,1,1,1,1 access_mail_shortcode,mail.shortcode,model_mail_shortcode,base.group_user,1,1,1,1 access_mail_shortcode_portal,mail.shortcode.portal,model_mail_shortcode,base.group_portal,1,0,0,0 diff --git a/addons/mail/tests/__init__.py b/addons/mail/tests/__init__.py index 2e47bdac589..4f4c25c7772 100644 --- a/addons/mail/tests/__init__.py +++ b/addons/mail/tests/__init__.py @@ -5,6 +5,7 @@ from . import test_mail_channel from . import test_mail_channel_partner from . import test_mail_full_composer from . import test_mail_render +from . import test_mail_template from . import test_mail_tools from . import test_res_partner from . import test_res_users_settings diff --git a/addons/mail/tests/common.py b/addons/mail/tests/common.py index a62105585a6..aa423cf60cf 100644 --- a/addons/mail/tests/common.py +++ b/addons/mail/tests/common.py @@ -890,10 +890,12 @@ class MailCommon(common.TransactionCase, MailCase): cls.user_root = cls.env.ref('base.user_root') cls.partner_root = cls.user_root.partner_id + cls.env['ir.config_parameter'].set_param('mail.restrict.template.rendering', False) + # test standard employee cls.user_employee = mail_new_test_user( cls.env, login='employee', - groups='base.group_user', + groups='base.group_user,mail.group_mail_template_editor', company_id=cls.company_admin.id, name='Ernest Employee', notification_type='inbox', diff --git a/addons/mail/tests/test_mail_render.py b/addons/mail/tests/test_mail_render.py index 51775d9e0ce..0d2c1d1bc93 100644 --- a/addons/mail/tests/test_mail_render.py +++ b/addons/mail/tests/test_mail_render.py @@ -2,6 +2,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from odoo.addons.mail.tests import common +from odoo.exceptions import AccessError from odoo.tests import tagged, users @@ -37,7 +38,17 @@ class TestMailRender(common.MailCommon): % else Other Speaker % endif -

""" +

""", + """ +

${13 + 13}

+

This is a test

+ """, + """ + Test + % if False: + Code not executed + % endif + """, ] cls.base_jinja_bits_fr = [ '

Bonjour

', @@ -126,6 +137,20 @@ class TestMailRender(common.MailCommon): 'res_id': cls.test_template_jinja.id, }) + # Enable group-based template management + cls.env['ir.config_parameter'].set_param('mail.restrict.template.rendering', True) + + # User without the group "mail.group_mail_template_editor" + cls.user_rendering_restricted = common.mail_new_test_user( + cls.env, login='user_rendering_restricted', + groups='base.group_user', + company_id=cls.company_admin.id, + name='Jinja Restricted User', + notification_type='inbox', + signature='--\nErnest' + ) + cls.user_rendering_restricted.groups_id -= cls.env.ref('mail.group_mail_template_editor') + @users('employee') def test_evaluation_context(self): """ Test evaluation context and various ways of tweaking it. """ @@ -210,12 +235,16 @@ class TestMailRender(common.MailCommon): )[partner.id] self.assertIn(expected, result) - @users('employee') + @users('user_rendering_restricted') def test_template_rendering_function_call(self): """Test the case when the template call a custom function. + This function should not be called when the template is not rendered. """ - partner = self.env['res.partner'].browse(self.render_object.ids) + model = 'res.partner' + res_ids = self.env[model].search([], limit=1).ids + partner = self.env[model].browse(res_ids) + MailRenderMixin = self.env['mail.render.mixin'] def cust_function(): # Can not use "MagicMock" in a Jinja sand-boxed environment @@ -231,13 +260,40 @@ class TestMailRender(common.MailCommon):

return value

""" context = {'cust_function': cust_function} - result = self.env['mail.render.mixin']._render_template_jinja( + result = self.env['mail.render.mixin'].with_user(self.user_admin)._render_template_jinja( src, partner._name, partner.ids, add_context=context )[partner.id] self.assertEqual(expected, result) self.assertTrue(cust_function.call) + with self.assertRaises(AccessError, msg='Simple user should not be able to render Jinja code'): + MailRenderMixin._render_template_jinja(src, model, res_ids, add_context=context) + + @users('user_rendering_restricted') + def test_template_render_static(self): + """Test that we render correctly static templates (without placeholders).""" + model = 'res.partner' + res_ids = self.env[model].search([], limit=1).ids + MailRenderMixin = self.env['mail.render.mixin'] + + result = MailRenderMixin._render_template_jinja(self.base_jinja_bits[0], model, res_ids)[res_ids[0]] + self.assertEqual(result, self.base_jinja_bits[0]) + + @users('user_rendering_restricted') + def test_template_rendering_restricted(self): + """Test if we correctly detect static template.""" + res_ids = self.env['res.partner'].search([], limit=1).ids + with self.assertRaises(AccessError, msg='Simple user should not be able to render Jinja code'): + self.env['mail.render.mixin']._render_template_jinja(self.base_jinja_bits[3], 'res.partner', res_ids) + + @users('employee') + def test_template_rendering_unrestricted(self): + """Test if we correctly detect static template.""" + res_ids = self.env['res.partner'].search([], limit=1).ids + result = self.env['mail.render.mixin']._render_template_jinja(self.base_jinja_bits[3], 'res.partner', res_ids)[res_ids[0]] + self.assertIn('26', result, 'Template Editor should be able to render Jinja code') + @users('employee') def test_template_rendering_various(self): """ Test static rendering """ @@ -319,3 +375,17 @@ class TestMailRender(common.MailCommon): src, partner._name, partner.ids, engine=engine, )[partner.id] self.assertEqual(result, expected) + + @users('user_rendering_restricted') + def test_is_jinja_template_condition_block_restricted(self): + """Test if we correctly detect condition block (which might contains code).""" + res_ids = self.env['res.partner'].search([], limit=1).ids + with self.assertRaises(AccessError, msg='Simple user should not be able to render Jinja code'): + self.env['mail.render.mixin']._render_template_jinja(self.base_jinja_bits[4], 'res.partner', res_ids) + + @users('employee') + def test_is_jinja_template_condition_block_unrestricted(self): + """Test if we correctly detect condition block (which might contains code).""" + res_ids = self.env['res.partner'].search([], limit=1).ids + result = self.env['mail.render.mixin']._render_template_jinja(self.base_jinja_bits[4], 'res.partner', res_ids)[res_ids[0]] + self.assertNotIn('Code not executed', result, 'The condition block did not work') diff --git a/addons/mail/tests/test_mail_template.py b/addons/mail/tests/test_mail_template.py new file mode 100644 index 00000000000..9c42e4d6fa3 --- /dev/null +++ b/addons/mail/tests/test_mail_template.py @@ -0,0 +1,65 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo.tests import Form, users +from odoo.exceptions import AccessError +from odoo.addons.mail.tests.common import MailCommon + + +class TestMailTemplate(MailCommon): + @classmethod + def setUpClass(cls): + super(TestMailTemplate, cls).setUpClass() + # Enable the Jinja rendering restriction + cls.env['ir.config_parameter'].set_param('mail.restrict.template.rendering', True) + cls.user_employee.groups_id -= cls.env.ref('mail.group_mail_template_editor') + + cls.mail_template = cls.env['mail.template'].create({ + 'name': 'Test template', + 'subject': '${1 + 5}', + 'body_html': '${4 + 9}', + 'lang': '${object.lang}', + 'auto_delete': True, + 'model_id': cls.env.ref('base.model_res_partner').id, + }) + + @users('employee') + def test_mail_compose_message_content_from_template(self): + form = Form(self.env['mail.compose.message']) + form.template_id = self.mail_template + mail_compose_message = form.save() + + self.assertEqual(mail_compose_message.subject, '6', 'We must trust mail template values') + + @users('employee') + def test_mail_compose_message_content_from_template_mass_mode(self): + mail_compose_message = self.env['mail.compose.message'].create({ + 'composition_mode': 'mass_mail', + 'model': 'res.partner', + 'template_id': self.mail_template.id, + 'subject': '${1 + 5}', + }) + + values = mail_compose_message.get_mail_values(self.partner_employee.ids) + + self.assertEqual(values[self.partner_employee.id]['subject'], '6', 'We must trust mail template values') + self.assertIn('13', values[self.partner_employee.id]['body_html'], 'We must trust mail template values') + + def test_mail_template_acl(self): + # Sanity check + self.assertTrue(self.user_admin.has_group('mail.group_mail_template_editor')) + self.assertFalse(self.user_employee.has_group('mail.group_mail_template_editor')) + + # Group System can create / write / unlink mail template + mail_template = self.env['mail.template'].with_user(self.user_admin).create({'name': 'Test template'}) + self.assertEqual(mail_template.name, 'Test template') + + mail_template.with_user(self.user_admin).name = 'New name' + self.assertEqual(mail_template.name, 'New name') + + # Standard employee can not + with self.assertRaises(AccessError): + self.env['mail.template'].with_user(self.user_employee).create({}) + + with self.assertRaises(AccessError): + mail_template.with_user(self.user_employee).name = 'Test write' diff --git a/addons/mail/views/res_config_settings_views.xml b/addons/mail/views/res_config_settings_views.xml index d210a49cc4a..2b79e01497d 100644 --- a/addons/mail/views/res_config_settings_views.xml +++ b/addons/mail/views/res_config_settings_views.xml @@ -44,6 +44,18 @@ +
+
+ +
+
+
+
diff --git a/addons/mail/wizard/mail_compose_message.py b/addons/mail/wizard/mail_compose_message.py index 8f51f4e83ba..f9f032e04f7 100644 --- a/addons/mail/wizard/mail_compose_message.py +++ b/addons/mail/wizard/mail_compose_message.py @@ -167,6 +167,14 @@ class MailComposer(models.TransientModel): for fname, value in values.items(): setattr(self, fname, value) + def _compute_can_edit_body(self): + """Can edit the body if we are not in "mass_mail" mode because the template is + rendered before it's modified. + """ + non_mass_mail = self.filtered(lambda m: m.composition_mode != 'mass_mail') + non_mass_mail.can_edit_body = True + super(MailComposer, self - non_mass_mail)._compute_can_edit_body() + @api.model def get_record_data(self, values): """ Returns a defaults-like dict with initial values for the composition diff --git a/addons/mail/wizard/mail_compose_message_views.xml b/addons/mail/wizard/mail_compose_message_views.xml index 259e0d584fe..603958d9b29 100644 --- a/addons/mail/wizard/mail_compose_message_views.xml +++ b/addons/mail/wizard/mail_compose_message_views.xml @@ -68,7 +68,8 @@ attrs="{'invisible':['|', ('reply_to_mode', '=', 'update'), ('composition_mode', '!=', 'mass_mail')], 'required':[('reply_to_mode', '!=', 'update'), ('composition_mode', '=', 'mass_mail')]}"/> - + +