From 8aba05dc08b2619800151f6cd380f3196044aef7 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Fri, 30 Sep 2016 19:49:59 +0200 Subject: [PATCH 1/6] [FIX] base_import_module, tools: raise a clear error for unsupported file types Also restrict XML data attribute evaluation context even for real module data files. This will prevent accidentally depending on context parameters that would not be available inside base_import_module. --- addons/base_import_module/models/ir_module.py | 4 ++ openerp/tools/convert.py | 41 +++++++++---------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/addons/base_import_module/models/ir_module.py b/addons/base_import_module/models/ir_module.py index cee635ca25c..7f5dbe626f3 100644 --- a/addons/base_import_module/models/ir_module.py +++ b/addons/base_import_module/models/ir_module.py @@ -44,6 +44,10 @@ class ir_module_module(osv.osv): for kind in ['data', 'init_xml', 'update_xml']: for filename in terp[kind]: + ext = os.path.splitext(filename)[1].lower() + if ext not in ('.xml', '.csv', '.sql'): + _logger.info("module %s: skip unsupported file %s", module, filename) + continue _logger.info("module %s: loading %s", module, filename) noupdate = False if filename.endswith('.csv') and kind in ('init', 'init_xml'): diff --git a/openerp/tools/convert.py b/openerp/tools/convert.py index 3fcd781e4a0..500df8be3c4 100644 --- a/openerp/tools/convert.py +++ b/openerp/tools/convert.py @@ -32,11 +32,8 @@ from yaml_import import convert_yaml_import _logger = logging.getLogger(__name__) -# Import of XML records requires the unsafe eval as well, -# almost everywhere, which is ok because it supposedly comes -# from trusted data, but at least we make it obvious now. -unsafe_eval = eval -from safe_eval import safe_eval as eval +from safe_eval import safe_eval as s_eval +safe_eval = lambda expr, ctx={}: s_eval(expr, ctx, nocopy=True) class ParseError(Exception): def __init__(self, msg, text, filename, lineno): @@ -101,7 +98,7 @@ def _eval_xml(self, node, pool, cr, uid, idref, context=None): idref2 = {} if f_search: idref2 = _get_idref(self, cr, uid, f_model, context, idref) - q = unsafe_eval(f_search, idref2) + q = safe_eval(f_search, idref2) ids = pool[f_model].search(cr, uid, q) if f_use != 'id': ids = map(lambda x: x[f_use], pool[f_model].read(cr, uid, ids, [f_use])) @@ -118,7 +115,7 @@ def _eval_xml(self, node, pool, cr, uid, idref, context=None): if a_eval: idref2 = _get_idref(self, cr, uid, f_model, context, idref) try: - return unsafe_eval(a_eval, idref2) + return safe_eval(a_eval, idref2) except Exception: logging.getLogger('openerp.tools.convert.init').error( 'Could not eval(%s) for %s in %s', a_eval, node.get('name'), context) @@ -190,7 +187,7 @@ def _eval_xml(self, node, pool, cr, uid, idref, context=None): # FIXME: should probably be exclusive if a_eval: idref['ref'] = lambda x: self.id_get(cr, x) - args = unsafe_eval(a_eval, idref) + args = safe_eval(a_eval, idref) for n in node: return_val = _eval_xml(self,n, pool, cr, uid, idref, context) if return_val is not None: @@ -223,12 +220,12 @@ class xml_import(object): for ctx in (data_node_context, node_context): if ctx: try: - ctx_res = unsafe_eval(ctx, eval_dict) + ctx_res = safe_eval(ctx, eval_dict) if isinstance(context, dict): context.update(ctx_res) else: context = ctx_res - except NameError: + except ValueError, NameError: # Some contexts contain references that are only valid at runtime at # client-side, so in that case we keep the original context string # as it is. We also log it, just in case. @@ -267,7 +264,7 @@ form: module.record_id""" % (xml_id,) if d_search: idref = _get_idref(self, cr, self.uid, d_model, context={}, idref={}) try: - ids = self.pool[d_model].search(cr, self.uid, unsafe_eval(d_search, idref)) + ids = self.pool[d_model].search(cr, self.uid, safe_eval(d_search, idref)) except ValueError: _logger.warning('Skipping deletion for failed search `%r`', d_search, exc_info=True) pass @@ -300,14 +297,14 @@ form: module.record_id""" % (xml_id,) if rec.get(field): res[dest] = rec.get(field).encode('utf8') if rec.get('auto'): - res['auto'] = eval(rec.get('auto','False')) + res['auto'] = safe_eval(rec.get('auto','False')) if rec.get('sxw'): sxw_content = misc.file_open(rec.get('sxw')).read() res['report_sxw_content'] = sxw_content if rec.get('header'): - res['header'] = eval(rec.get('header','False')) + res['header'] = safe_eval(rec.get('header','False')) - res['multi'] = rec.get('multi') and eval(rec.get('multi','False')) + res['multi'] = rec.get('multi') and safe_eval(rec.get('multi','False')) xml_id = rec.get('id','').encode('utf8') self._test_xml_id(xml_id) @@ -331,12 +328,12 @@ form: module.record_id""" % (xml_id,) id = self.pool['ir.model.data']._update(cr, self.uid, "ir.actions.report.xml", self.module, res, xml_id, noupdate=self.isnoupdate(data_node), mode=self.mode) self.idref[xml_id] = int(id) - if not rec.get('menu') or eval(rec.get('menu','False')): + if not rec.get('menu') or safe_eval(rec.get('menu','False')): keyword = str(rec.get('keyword', 'client_print_multi')) value = 'ir.actions.report.xml,'+str(id) ir_values_id = self.pool['ir.values'].set_action(cr, self.uid, res['name'], keyword, res['model'], value) self.pool['ir.actions.report.xml'].write(cr, self.uid, id, {'ir_values_id': ir_values_id}) - elif self.mode=='update' and eval(rec.get('menu','False'))==False: + elif self.mode=='update' and safe_eval(rec.get('menu','False'))==False: # Special check for report having attribute menu=False on update value = 'ir.actions.report.xml,'+str(id) self._remove_ir_values(cr, res['name'], value, res['model']) @@ -409,7 +406,7 @@ form: module.record_id""" % (xml_id,) context = self.get_context(data_node, rec, eval_context) try: - domain = unsafe_eval(domain, eval_context) + domain = safe_eval(domain, eval_context) except NameError: # Some domains contain references that are only valid at runtime at # client-side, so in that case we keep the original domain string @@ -447,7 +444,7 @@ form: module.record_id""" % (xml_id,) if rec.get('target'): res['target'] = rec.get('target','') if rec.get('multi'): - res['multi'] = eval(rec.get('multi', 'False')) + res['multi'] = safe_eval(rec.get('multi', 'False')) id = self.pool['ir.model.data']._update(cr, self.uid, 'ir.actions.act_window', self.module, res, xml_id, noupdate=self.isnoupdate(data_node), mode=self.mode) self.idref[xml_id] = int(id) @@ -581,7 +578,7 @@ form: module.record_id""" % (xml_id,) if rec_id: ids = [self.id_get(cr, rec_id)] elif rec_src: - q = unsafe_eval(rec_src, eval_dict) + q = safe_eval(rec_src, eval_dict) ids = self.pool[rec_model].search(cr, uid, q, context=context) if rec_src_count: count = int(rec_src_count) @@ -612,7 +609,7 @@ form: module.record_id""" % (xml_id,) for test in rec.findall('./test'): f_expr = test.get("expr",'').encode('utf-8') expected_value = _eval_xml(self, test, self.pool, cr, uid, self.idref, context=context) or True - expression_value = unsafe_eval(f_expr, globals_dict) + expression_value = safe_eval(f_expr, globals_dict) if expression_value != expected_value: # assertion failed self.assertion_report.record_failure() msg = 'assertion "%s" failed!\n' \ @@ -631,7 +628,7 @@ form: module.record_id""" % (xml_id,) rec_id = rec.get("id",'').encode('ascii') rec_context = rec.get("context", {}) if rec_context: - rec_context = unsafe_eval(rec_context) + rec_context = safe_eval(rec_context) if self.xml_filename and rec_id: rec_context['install_mode_data'] = dict( @@ -678,7 +675,7 @@ form: module.record_id""" % (xml_id,) f_val = False if f_search: - q = unsafe_eval(f_search, self.idref) + q = safe_eval(f_search, self.idref) assert f_model, 'Define an attribute model="..." in your .XML file !' f_obj = self.pool[f_model] # browse the objects searched From 0ea7842c5e0c0c9322e4238b693dcb91f12913f3 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Mon, 3 Oct 2016 17:39:11 +0200 Subject: [PATCH 2/6] [FIX] base: force local cursor to avoid mismatch --- openerp/addons/base/res/res_users.py | 1 + 1 file changed, 1 insertion(+) diff --git a/openerp/addons/base/res/res_users.py b/openerp/addons/base/res/res_users.py index 4d6da4b6edd..3ec40621575 100644 --- a/openerp/addons/base/res/res_users.py +++ b/openerp/addons/base/res/res_users.py @@ -501,6 +501,7 @@ class res_users(osv.osv): if not passwd: # empty passwords disallowed for obvious security reasons raise openerp.exceptions.AccessDenied() + db = self.pool.db_name if self.__uid_cache.setdefault(db, {}).get(uid) == passwd: return cr = self.pool.cursor() From 0b87f7662892d60e03f1016e4150d53aef979a06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 19 Sep 2016 12:54:58 +0200 Subject: [PATCH 3/6] [IMP] mail: separate management of mail routes from redirection Move code related to redirection in its own class method. This way all controllers perform their own actions and checks before calling the redirection itself. Moreover it is necessary for some controllers to be able to call the redirection independently outside of the mail/view controller scope. This commit also fixes some links generated for mail notification email buttons. Assign and (un)follow buttons now effectively use the link helper when generating the associated URL instead of manually crafting it. Custom routes for notification buttons used in HR and CRM are added. Indeed in order to be able to fine-tune the behavior called by those buttons, using dedicated controllers is simpler than trying to use a generic one. As there are few custom buttons in notification emails using dedicated controllers does not lead to a code overhead. --- addons/crm/__init__.py | 1 + addons/crm/controllers/__init__.py | 2 + addons/crm/controllers/main.py | 38 ++++ addons/crm/crm_lead.py | 6 +- addons/hr_holidays/__init__.py | 1 + addons/hr_holidays/controllers/__init__.py | 2 + addons/hr_holidays/controllers/main.py | 28 +++ addons/hr_holidays/hr_holidays.py | 4 +- addons/mail/controllers/main.py | 203 +++++++++++---------- addons/mail/models/mail_thread.py | 45 +++-- 10 files changed, 211 insertions(+), 119 deletions(-) create mode 100644 addons/crm/controllers/__init__.py create mode 100644 addons/crm/controllers/main.py create mode 100644 addons/hr_holidays/controllers/__init__.py create mode 100644 addons/hr_holidays/controllers/main.py diff --git a/addons/crm/__init__.py b/addons/crm/__init__.py index 927f765e815..3d074563717 100644 --- a/addons/crm/__init__.py +++ b/addons/crm/__init__.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +import controllers import res_users import crm_stage import crm_lead diff --git a/addons/crm/controllers/__init__.py b/addons/crm/controllers/__init__.py new file mode 100644 index 00000000000..12d2d759f34 --- /dev/null +++ b/addons/crm/controllers/__init__.py @@ -0,0 +1,2 @@ +# -*- coding: utf-8 -* +import main diff --git a/addons/crm/controllers/main.py b/addons/crm/controllers/main.py new file mode 100644 index 00000000000..e924f8a76cc --- /dev/null +++ b/addons/crm/controllers/main.py @@ -0,0 +1,38 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from openerp.addons.mail.controllers.main import MailController +from openerp import http + + +class CrmController(http.Controller): + + @http.route('/lead/case_mark_won', type='http', auth='user', methods=['GET']) + def crm_lead_case_mark_won(self, res_id, token): + comparison, record, redirect = MailController._check_token_and_record_or_redirect('crm.lead', int(res_id), token) + if comparison and record: + try: + record.case_mark_won() + except Exception: + return MailController._redirect_to_messaging() + return redirect + + @http.route('/lead/case_mark_lost', type='http', auth='user', methods=['GET']) + def crm_lead_case_mark_lost(self, res_id, token): + comparison, record, redirect = MailController._check_token_and_record_or_redirect('crm.lead', int(res_id), token) + if comparison and record: + try: + record.case_mark_lost() + except Exception: + return MailController._redirect_to_messaging() + return redirect + + @http.route('/lead/convert', type='http', auth='user', methods=['GET']) + def crm_lead_convert(self, res_id, token): + comparison, record, redirect = MailController._check_token_and_record_or_redirect('crm.lead', int(res_id), token) + if comparison and record: + try: + record.convert_opportunity(record.partner_id.id) + except Exception: + return MailController._redirect_to_messaging() + return redirect diff --git a/addons/crm/crm_lead.py b/addons/crm/crm_lead.py index c49abd90a14..71279ec6cf5 100644 --- a/addons/crm/crm_lead.py +++ b/addons/crm/crm_lead.py @@ -976,9 +976,9 @@ Update your business card, phone book, social media,... Send an email right now lead = self.browse(cr, uid, ids[0], context=context) - won_action = self._notification_link_helper(cr, uid, ids, 'method', context=context, method='case_mark_won') - lost_action = self._notification_link_helper(cr, uid, ids, 'method', context=context, method='case_mark_lost') - convert_action = self._notification_link_helper(cr, uid, ids, 'method', context=context, method='convert_opportunity', partner_id=lead.partner_id.id) + won_action = self._notification_link_helper(cr, uid, ids, 'controller', controller='/lead/case_mark_won', context=context) + lost_action = self._notification_link_helper(cr, uid, ids, 'controller', controller='/lead/case_mark_lost', context=context) + convert_action = self._notification_link_helper(cr, uid, ids, 'controller', controller='/lead/convert', context=context) if lead.type == 'lead': res['group_sale_salesman'] = { diff --git a/addons/hr_holidays/__init__.py b/addons/hr_holidays/__init__.py index f9bd12244c0..4739125fc00 100644 --- a/addons/hr_holidays/__init__.py +++ b/addons/hr_holidays/__init__.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +import controllers import hr_department import hr_holidays import report diff --git a/addons/hr_holidays/controllers/__init__.py b/addons/hr_holidays/controllers/__init__.py new file mode 100644 index 00000000000..12d2d759f34 --- /dev/null +++ b/addons/hr_holidays/controllers/__init__.py @@ -0,0 +1,2 @@ +# -*- coding: utf-8 -* +import main diff --git a/addons/hr_holidays/controllers/main.py b/addons/hr_holidays/controllers/main.py new file mode 100644 index 00000000000..ecbb47d4ca0 --- /dev/null +++ b/addons/hr_holidays/controllers/main.py @@ -0,0 +1,28 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from openerp.addons.mail.controllers.main import MailController +from openerp import http + + +class HrHolidaysController(http.Controller): + + @http.route('/hr_holidays/validate', type='http', auth='user', methods=['GET']) + def hr_holidays_validate(self, res_id, token): + comparison, record, redirect = MailController._check_token_and_record_or_redirect('hr.holidays', int(res_id), token) + if comparison and record: + try: + record.signal_workflow('validate') + except Exception: + return MailController._redirect_to_messaging() + return redirect + + @http.route('/hr_holidays/refuse', type='http', auth='user', methods=['GET']) + def hr_holidays_refuse(self, res_id, token): + comparison, record, redirect = MailController._check_token_and_record_or_redirect('hr.holidays', int(res_id), token) + if comparison and record: + try: + record.signal_workflow('refuse') + except Exception: + return MailController._redirect_to_messaging() + return redirect diff --git a/addons/hr_holidays/hr_holidays.py b/addons/hr_holidays/hr_holidays.py index 3697e7bfa92..0c3ab256965 100644 --- a/addons/hr_holidays/hr_holidays.py +++ b/addons/hr_holidays/hr_holidays.py @@ -523,8 +523,8 @@ class hr_holidays(osv.osv): def _notification_get_recipient_groups(self, cr, uid, ids, message, recipients, context=None): res = super(hr_holidays, self)._notification_get_recipient_groups(cr, uid, ids, message, recipients, context=context) - app_action = '/mail/workflow?%s' % url_encode({'model': self._name, 'res_id': ids[0], 'signal': 'validate'}) - ref_action = '/mail/workflow?%s' % url_encode({'model': self._name, 'res_id': ids[0], 'signal': 'refuse'}) + app_action = self._notification_link_helper(cr, uid, ids, 'controller', controller='/hr_holidays/validate', context=context) + ref_action = self._notification_link_helper(cr, uid, ids, 'controller', controller='/hr_holidays/refuse', context=context) holiday = self.browse(cr, uid, ids[0], context=context) actions = [] diff --git a/addons/mail/controllers/main.py b/addons/mail/controllers/main.py index 784b26671c6..d9e55765df1 100644 --- a/addons/mail/controllers/main.py +++ b/addons/mail/controllers/main.py @@ -1,5 +1,6 @@ import base64 import json +import logging from operator import itemgetter import psycopg2 import werkzeug @@ -8,20 +9,90 @@ from werkzeug import url_encode import openerp from openerp import SUPERUSER_ID from openerp import http +from openerp import _ from openerp.exceptions import AccessError from openerp.http import request +from openerp.tools import consteq from openerp.addons.web.controllers.main import binary_content +_logger = logging.getLogger(__name__) + class MailController(http.Controller): _cp_path = '/mail' - def _redirect_to_messaging(self): + @classmethod + def _redirect_to_messaging(cls): messaging_action = request.env['mail.thread']._get_inbox_action_xml_id() url = '/web#%s' % url_encode({'action': messaging_action}) return werkzeug.utils.redirect(url) + @classmethod + def _check_token(cls, token): + base_link = request.httprequest.path + params = dict(request.params) + params.pop('token', '') + valid_token = request.env['mail.thread']._generate_notification_token(base_link, params) + return consteq(valid_token, str(token)) + + @classmethod + def _check_token_and_record_or_redirect(cls, model, res_id, token): + comparison = cls._check_token(token) + if not comparison: + _logger.warning(_('Invalid token in route %s') % request.httprequest.url) + return comparison, None, cls._redirect_to_messaging() + try: + record = request.env[model].browse(res_id).exists() + except Exception: + record = None + redirect = cls._redirect_to_messaging() + else: + redirect = cls._redirect_to_record(model, res_id) + return comparison, record, redirect + + @classmethod + def _redirect_to_record(cls, model, res_id): + uid = request.session.uid + + # no model / res_id, meaning no possible record -> redirect to login + if not model or not res_id or model not in request.env: + return cls._redirect_to_messaging() + + # find the access action using sudo to have the details about the access link + RecordModel = request.env[model] + record_sudo = RecordModel.sudo().browse(res_id).exists() + if not record_sudo: + # record does not seem to exist -> redirect to login + return cls._redirect_to_messaging() + record_action = record_sudo.get_access_action() + + # the record has an URL redirection: use it directly + if record_action['type'] == 'ir.actions.act_url': + return werkzeug.utils.redirect(record_action['url']) + # other choice: act_window (no support of anything else currently) + elif not record_action['type'] == 'ir.actions.act_window': + return cls._redirect_to_messaging() + + # the record has a window redirection: check access rights + if not RecordModel.sudo(uid).check_access_rights('read', raise_exception=False): + return cls._redirect_to_messaging() + try: + record_sudo.sudo(uid).check_access_rule('read') + except AccessError: + return cls._redirect_to_messaging() + + url_params = { + 'view_type': record_action['view_type'], + 'model': model, + 'id': res_id, + 'active_id': res_id, + 'view_id': record_sudo.get_formview_id(), + 'action': record_action.get('id'), + } + url = '/web?#%s' % url_encode(url_params) + return werkzeug.utils.redirect(url) + @http.route('/mail/receive', type='json', auth='none') def receive(self, req): """ End-point to receive mail from an external SMTP server. """ @@ -95,8 +166,6 @@ class MailController(http.Controller): - users without read access are redirected to the Messaging - not logged users are redirected to the login page """ - uid = request.session.uid - if message_id: try: message = request.env['mail.message'].sudo().browse(int(message_id)).exists() @@ -110,110 +179,48 @@ class MailController(http.Controller): elif res_id and isinstance(res_id, basestring): res_id = int(res_id) - # no model / res_id, meaning no possible record -> redirect to login - if not model or not res_id or model not in request.env: - return self._redirect_to_messaging() + return self._redirect_to_record(model, res_id) - # find the access action using sudo to have the details about the access link - RecordModel = request.env[model] - record_sudo = RecordModel.sudo().browse(res_id).exists() - if not record_sudo: - # record does not seem to exist -> redirect to login - return self._redirect_to_messaging() - record_action = record_sudo.get_access_action() + @http.route('/mail/follow', type='http', auth='user', methods=['GET']) + def mail_action_follow(self, model, res_id, token=None): + comparison, record, redirect = self._check_token_and_record_or_redirect(model, int(res_id), token) + if comparison and record: + try: + record.sudo().message_subscribe_users() + except Exception: + return self._redirect_to_messaging() + return redirect - # the record has an URL redirection: use it directly - if record_action['type'] == 'ir.actions.act_url': - return werkzeug.utils.redirect(record_action['url']) - # other choice: act_window (no support of anything else currently) - elif not record_action['type'] == 'ir.actions.act_window': - return self._redirect_to_messaging() - - # the record has a window redirection: check access rights - if not RecordModel.sudo(uid).check_access_rights('read', raise_exception=False): - return self._redirect_to_messaging() - try: - RecordModel.sudo(uid).browse(res_id).exists().check_access_rule('read') - except AccessError: - return self._redirect_to_messaging() - - query = {} - url_params = { - 'view_type': record_action['view_type'], - 'model': model, - 'id': res_id, - 'active_id': res_id, - 'view_id': record_sudo.get_formview_id(), - 'action': record_action.get('id'), - } - url = '/web?%s#%s' % (url_encode(query), url_encode(url_params)) - return werkzeug.utils.redirect(url) - - @http.route('/mail/follow', type='http', auth='user') - def mail_action_follow(self, model, res_id): - if model not in request.env: - return self._redirect_to_messaging() - Model = request.env[model] - try: - Model.browse(res_id).message_subscribe_users() - except: - return self._redirect_to_messaging() - return werkzeug.utils.redirect('/mail/view?%s' % url_encode({'model': model, 'res_id': res_id})) - - @http.route('/mail/unfollow', type='http', auth='user') - def mail_action_unfollow(self, model, res_id): - if model not in request.env: - return self._redirect_to_messaging() - Model = request.env[model] - try: - Model.browse(res_id).sudo().message_unsubscribe_users([request.uid]) - except: - return self._redirect_to_messaging() - return werkzeug.utils.redirect('/mail/view?%s' % url_encode({'model': model, 'res_id': res_id})) + @http.route('/mail/unfollow', type='http', auth='user', methods=['GET']) + def mail_action_unfollow(self, model, res_id, token=None): + comparison, record, redirect = self._check_token_and_record_or_redirect(model, int(res_id), token) + if comparison and record: + try: + # TDE CHECKME: is sudo really necessary ? + record.sudo().message_unsubscribe_users([request.uid]) + except Exception: + return self._redirect_to_messaging() + return redirect @http.route('/mail/new', type='http', auth='user') - def mail_action_new(self, model, res_id, **kwargs): + def mail_action_new(self, model, res_id, action_id): if model not in request.env: return self._redirect_to_messaging() params = {'view_type': 'form', 'model': model} - if kwargs.get('action_id'): - params['action'] = kwargs['action_id'] + if action_id: + # Probably something to do + params['action'] = action_id return werkzeug.utils.redirect('/web?#%s' % url_encode(params)) - @http.route('/mail/method', type='http', auth='user') - def mail_action_method(self, model, res_id, method, **kwargs): - # only public methods / check exists - if method.strip().startswith('_') or model not in request.env: - return self._redirect_to_messaging() - Model = request.env[model] - try: - record = Model.browse(int(res_id)).exists() - getattr(record, method)(**json.loads(kwargs.get('params', {}))) - except: - return self._redirect_to_messaging() - return werkzeug.utils.redirect('/mail/view?%s' % url_encode({'model': model, 'res_id': res_id})) - - @http.route('/mail/assign', type='http', auth='user') - def mail_action_assign(self, model, res_id, **kwargs): - if model not in request.env: - return self._redirect_to_messaging() - Model = request.env[model] - try: - Model.browse(int(res_id)).exists().write({'user_id': request.uid}) - except: - return self._redirect_to_messaging() - return werkzeug.utils.redirect('/mail/view?%s' % url_encode({'model': model, 'res_id': res_id})) - - @http.route('/mail/workflow', type='http', auth='user') - def mail_action_workflow(self, model, res_id, signal, **kwargs): - if model not in request.env: - return self._redirect_to_messaging() - Model = request.env[model] - try: - Model.browse(int(res_id)).exists().signal_workflow(signal) - except: - return self._redirect_to_messaging() - return werkzeug.utils.redirect('/mail/view?%s' % url_encode({'model': model, 'res_id': res_id})) + @http.route('/mail/assign', type='http', auth='user', methods=['GET']) + def mail_action_assign(self, model, res_id, token=None): + comparison, record, redirect = self._check_token_and_record_or_redirect(model, int(res_id), token) + if comparison and record: + try: + record.write({'user_id': request.uid}) + except Exception: + return self._redirect_to_messaging() + return redirect @http.route('/mail///avatar/', type='http', auth='public') def avatar(self, res_model, res_id, partner_id): diff --git a/addons/mail/models/mail_thread.py b/addons/mail/models/mail_thread.py index b4b44244cff..e99fb435aae 100644 --- a/addons/mail/models/mail_thread.py +++ b/addons/mail/models/mail_thread.py @@ -4,6 +4,8 @@ import base64 import datetime import dateutil import email +import hashlib +import hmac import json import lxml from lxml import etree @@ -539,6 +541,13 @@ class MailThread(models.AbstractModel): because portal users have a different Inbox action than classic users. """ return 'mail.mail_channel_action_client_chat' + @api.model + def _generate_notification_token(self, base_link, params): + secret = self.env['ir.config_parameter'].sudo().get_param('database.secret') + token = '%s?%s' % (base_link, ' '.join('%s=%s' % (key, params[key]) for key in sorted(params.keys()))) + hm = hmac.new(str(secret), token, hashlib.sha1).hexdigest() + return hm + @api.multi def _notification_link_helper(self, link_type, **kwargs): if kwargs.get('message_id'): @@ -546,26 +555,30 @@ class MailThread(models.AbstractModel): 'message_id': kwargs.pop('message_id') } else: - self.ensure_one() base_params = { - 'model': self._name, - 'res_id': self.ids[0], + 'model': kwargs.get('model', self._name), + 'res_id': kwargs.get('res_id', self.ids and self.ids[0] or False), } - link = False if link_type in ['view', 'assign', 'follow', 'unfollow']: params = dict(base_params) - link = '/mail/view?%s' % url_encode(params) - elif link_type == 'workflow': - params = dict(base_params, signal=kwargs['signal']) - link = '/mail/workflow?%s' % url_encode(params) - elif link_type == 'method': - method = kwargs.pop('method') - params = dict(base_params, method=method, params=json.dumps(kwargs)) - link = '/mail/method?%s' % url_encode(params) + base_link = '/mail/%s' % link_type elif link_type == 'new': - params = dict(base_params, action_id=kwargs.get('action_id')) - link = '/mail/new?%s' % url_encode(params) + params = dict(base_params, action_id=kwargs.get('action_id', '')) + base_link = '/mail/new' + elif link_type == 'controller': + controller = kwargs.pop('controller') + params = dict(base_params) + params.pop('model') + base_link = '%s' % controller + else: + return '' + + if link_type not in ['view', 'new']: + token = self._generate_notification_token(base_link, params) + params['token'] = token + + link = '%s?%s' % (base_link, url_encode(params)) return link @api.multi @@ -639,8 +652,8 @@ class MailThread(models.AbstractModel): 'followers': self.env['res.partner'], 'not_followers': self.env['res.partner'], 'button_access': {'url': access_link, 'title': view_title}, - 'button_follow': {'url': '/mail/follow?%s' % url_encode({'model': message.model, 'res_id': message.res_id}), 'title': _('Follow')}, - 'button_unfollow': {'url': '/mail/unfollow?%s' % url_encode({'model': message.model, 'res_id': message.res_id}), 'title': _('Unfollow')}, + 'button_follow': {'url': self._notification_link_helper('follow', model=message.model, res_id=message.res_id), 'title': _('Follow')}, + 'button_unfollow': {'url': self._notification_link_helper('unfollow', model=message.model, res_id=message.res_id), 'title': _('Unfollow')}, 'actions': list(), } group_data[category] = self.env['res.partner'] From 12634e1227ab8446e34b747e02865f62087b5e86 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Fri, 9 Sep 2016 15:08:10 +0200 Subject: [PATCH 4/6] [FIX] base_import_module, tools: raise a clear error for unsupported file types Also restrict XML data attribute evaluation context even for real module data files. This will prevent accidentally depending on context parameters that would not be available inside base_import_module. --- addons/base_import_module/models/ir_module.py | 4 ++ openerp/tools/convert.py | 41 +++++++++---------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/addons/base_import_module/models/ir_module.py b/addons/base_import_module/models/ir_module.py index df5587569ff..e81035bda31 100644 --- a/addons/base_import_module/models/ir_module.py +++ b/addons/base_import_module/models/ir_module.py @@ -41,6 +41,10 @@ class view(osv.osv): for kind in ['data', 'init_xml', 'update_xml']: for filename in terp[kind]: + ext = os.path.splitext(filename)[1].lower() + if ext not in ('.xml', '.csv', '.sql'): + _logger.info("module %s: skip unsupported file %s", module, filename) + continue _logger.info("module %s: loading %s", module, filename) noupdate = False if filename.endswith('.csv') and kind in ('init', 'init_xml'): diff --git a/openerp/tools/convert.py b/openerp/tools/convert.py index be30b32c3cd..da1db9bf4c0 100644 --- a/openerp/tools/convert.py +++ b/openerp/tools/convert.py @@ -61,11 +61,8 @@ from misc import pickle, unquote from openerp import SUPERUSER_ID -# Import of XML records requires the unsafe eval as well, -# almost everywhere, which is ok because it supposedly comes -# from trusted data, but at least we make it obvious now. -unsafe_eval = eval -from safe_eval import safe_eval as eval +from safe_eval import safe_eval as s_eval +safe_eval = lambda expr, ctx={}: s_eval(expr, ctx, nocopy=True) class ParseError(Exception): def __init__(self, msg, text, filename, lineno): @@ -130,7 +127,7 @@ def _eval_xml(self, node, pool, cr, uid, idref, context=None): idref2 = {} if f_search: idref2 = _get_idref(self, cr, uid, f_model, context, idref) - q = unsafe_eval(f_search, idref2) + q = safe_eval(f_search, idref2) ids = pool[f_model].search(cr, uid, q) if f_use != 'id': ids = map(lambda x: x[f_use], pool[f_model].read(cr, uid, ids, [f_use])) @@ -147,7 +144,7 @@ def _eval_xml(self, node, pool, cr, uid, idref, context=None): if a_eval: idref2 = _get_idref(self, cr, uid, f_model, context, idref) try: - return unsafe_eval(a_eval, idref2) + return safe_eval(a_eval, idref2) except Exception: logging.getLogger('openerp.tools.convert.init').error( 'Could not eval(%s) for %s in %s', a_eval, node.get('name'), context) @@ -219,7 +216,7 @@ def _eval_xml(self, node, pool, cr, uid, idref, context=None): # FIXME: should probably be exclusive if a_eval: idref['ref'] = lambda x: self.id_get(cr, x) - args = unsafe_eval(a_eval, idref) + args = safe_eval(a_eval, idref) for n in node: return_val = _eval_xml(self,n, pool, cr, uid, idref, context) if return_val is not None: @@ -255,12 +252,12 @@ class xml_import(object): for ctx in (data_node_context, node_context): if ctx: try: - ctx_res = unsafe_eval(ctx, eval_dict) + ctx_res = safe_eval(ctx, eval_dict) if isinstance(context, dict): context.update(ctx_res) else: context = ctx_res - except NameError: + except ValueError, NameError: # Some contexts contain references that are only valid at runtime at # client-side, so in that case we keep the original context string # as it is. We also log it, just in case. @@ -299,7 +296,7 @@ form: module.record_id""" % (xml_id,) if d_search: idref = _get_idref(self, cr, self.uid, d_model, context={}, idref={}) try: - ids = self.pool[d_model].search(cr, self.uid, unsafe_eval(d_search, idref)) + ids = self.pool[d_model].search(cr, self.uid, safe_eval(d_search, idref)) except ValueError: _logger.warning('Skipping deletion for failed search `%r`', d_search, exc_info=True) pass @@ -332,14 +329,14 @@ form: module.record_id""" % (xml_id,) if rec.get(field): res[dest] = rec.get(field).encode('utf8') if rec.get('auto'): - res['auto'] = eval(rec.get('auto','False')) + res['auto'] = safe_eval(rec.get('auto','False')) if rec.get('sxw'): sxw_content = misc.file_open(rec.get('sxw')).read() res['report_sxw_content'] = sxw_content if rec.get('header'): - res['header'] = eval(rec.get('header','False')) + res['header'] = safe_eval(rec.get('header','False')) - res['multi'] = rec.get('multi') and eval(rec.get('multi','False')) + res['multi'] = rec.get('multi') and safe_eval(rec.get('multi','False')) xml_id = rec.get('id','').encode('utf8') self._test_xml_id(xml_id) @@ -359,12 +356,12 @@ form: module.record_id""" % (xml_id,) id = self.pool['ir.model.data']._update(cr, self.uid, "ir.actions.report.xml", self.module, res, xml_id, noupdate=self.isnoupdate(data_node), mode=self.mode) self.idref[xml_id] = int(id) - if not rec.get('menu') or eval(rec.get('menu','False')): + if not rec.get('menu') or safe_eval(rec.get('menu','False')): keyword = str(rec.get('keyword', 'client_print_multi')) value = 'ir.actions.report.xml,'+str(id) replace = rec.get('replace', True) self.pool['ir.model.data'].ir_set(cr, self.uid, 'action', keyword, res['name'], [res['model']], value, replace=replace, isobject=True, xml_id=xml_id) - elif self.mode=='update' and eval(rec.get('menu','False'))==False: + elif self.mode=='update' and safe_eval(rec.get('menu','False'))==False: # Special check for report having attribute menu=False on update value = 'ir.actions.report.xml,'+str(id) self._remove_ir_values(cr, res['name'], value, res['model']) @@ -448,7 +445,7 @@ form: module.record_id""" % (xml_id,) context = self.get_context(data_node, rec, eval_context) try: - domain = unsafe_eval(domain, eval_context) + domain = safe_eval(domain, eval_context) except NameError: # Some domains contain references that are only valid at runtime at # client-side, so in that case we keep the original domain string @@ -486,7 +483,7 @@ form: module.record_id""" % (xml_id,) if rec.get('target'): res['target'] = rec.get('target','') if rec.get('multi'): - res['multi'] = eval(rec.get('multi', 'False')) + res['multi'] = safe_eval(rec.get('multi', 'False')) id = self.pool['ir.model.data']._update(cr, self.uid, 'ir.actions.act_window', self.module, res, xml_id, noupdate=self.isnoupdate(data_node), mode=self.mode) self.idref[xml_id] = int(id) @@ -643,7 +640,7 @@ form: module.record_id""" % (xml_id,) if rec_id: ids = [self.id_get(cr, rec_id)] elif rec_src: - q = unsafe_eval(rec_src, eval_dict) + q = safe_eval(rec_src, eval_dict) ids = self.pool[rec_model].search(cr, uid, q, context=context) if rec_src_count: count = int(rec_src_count) @@ -674,7 +671,7 @@ form: module.record_id""" % (xml_id,) for test in rec.findall('./test'): f_expr = test.get("expr",'').encode('utf-8') expected_value = _eval_xml(self, test, self.pool, cr, uid, self.idref, context=context) or True - expression_value = unsafe_eval(f_expr, globals_dict) + expression_value = safe_eval(f_expr, globals_dict) if expression_value != expected_value: # assertion failed self.assertion_report.record_failure() msg = 'assertion "%s" failed!\n' \ @@ -693,7 +690,7 @@ form: module.record_id""" % (xml_id,) rec_id = rec.get("id",'').encode('ascii') rec_context = rec.get("context", None) if rec_context: - rec_context = unsafe_eval(rec_context) + rec_context = safe_eval(rec_context) self._test_xml_id(rec_id) # in update mode, the record won't be updated if the data node explicitely # opt-out using @noupdate="1". A second check will be performed in @@ -732,7 +729,7 @@ form: module.record_id""" % (xml_id,) f_val = False if f_search: - q = unsafe_eval(f_search, self.idref) + q = safe_eval(f_search, self.idref) assert f_model, 'Define an attribute model="..." in your .XML file !' f_obj = self.pool[f_model] # browse the objects searched From ba7880a249cb2c469bff6c2aef3ae3300c2a3e44 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Mon, 3 Oct 2016 17:43:44 +0200 Subject: [PATCH 5/6] [FIX] base: force local cursor in case of mismatch --- openerp/addons/base/res/res_users.py | 1 + 1 file changed, 1 insertion(+) diff --git a/openerp/addons/base/res/res_users.py b/openerp/addons/base/res/res_users.py index a181e141eb4..58e0d216a07 100644 --- a/openerp/addons/base/res/res_users.py +++ b/openerp/addons/base/res/res_users.py @@ -511,6 +511,7 @@ class res_users(osv.osv): if not passwd: # empty passwords disallowed for obvious security reasons raise openerp.exceptions.AccessDenied() + db = self.pool.db_name if self.__uid_cache.setdefault(db, {}).get(uid) == passwd: return cr = self.pool.cursor() From 3d55aff1c83d3157228852d1fe28efe82d7cc329 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Mon, 3 Oct 2016 23:41:44 +0200 Subject: [PATCH 6/6] [FIX] convert: properly catch Value/Name Errors --- openerp/tools/convert.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openerp/tools/convert.py b/openerp/tools/convert.py index da1db9bf4c0..a24f6640896 100644 --- a/openerp/tools/convert.py +++ b/openerp/tools/convert.py @@ -257,7 +257,7 @@ class xml_import(object): context.update(ctx_res) else: context = ctx_res - except ValueError, NameError: + except (ValueError, NameError): # Some contexts contain references that are only valid at runtime at # client-side, so in that case we keep the original context string # as it is. We also log it, just in case. @@ -446,7 +446,7 @@ form: module.record_id""" % (xml_id,) try: domain = safe_eval(domain, eval_context) - except NameError: + except (ValueError, NameError): # Some domains contain references that are only valid at runtime at # client-side, so in that case we keep the original domain string # as it is. We also log it, just in case.