From 2afb5b43ef0c7aa69b63145052a8d183d3d9d14b Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Wed, 25 Nov 2015 09:40:34 +0100 Subject: [PATCH] [IMP] form builder fields blacklisting * blacklist all fields by default * don't use blacklist in get_authorized_fields which is called to see if a field can be added to a form, instead only use it afterwards to see if the field can be written to by the formbuilder. That way formbuilder can whitelist fields which are actually added to forms on-demand resulting in a more secure interaction --- addons/website_crm/data/website_crm_data.xml | 11 ++++ addons/website_form/controllers/main.py | 5 +- addons/website_form/models/models.py | 66 +++++++++++++++++-- .../data/config_data.xml | 11 ++++ .../website_issue/data/website_issue_data.xml | 5 -- 5 files changed, 84 insertions(+), 14 deletions(-) diff --git a/addons/website_crm/data/website_crm_data.xml b/addons/website_crm/data/website_crm_data.xml index c61746b2ad7..5c454fc942c 100644 --- a/addons/website_crm/data/website_crm_data.xml +++ b/addons/website_crm/data/website_crm_data.xml @@ -6,6 +6,17 @@ True Create a lead + + crm.lead + + diff --git a/addons/website_form/controllers/main.py b/addons/website_form/controllers/main.py index f7f5f0d07ef..8aaa673f6ae 100644 --- a/addons/website_form/controllers/main.py +++ b/addons/website_form/controllers/main.py @@ -21,7 +21,6 @@ class WebsiteForm(http.Controller): try: data = self.extract_data(model_record, ** kwargs) - # If we encounter an issue while extracting data except ValidationError, e: # I couldn't find a cleaner way to pass data to an exception @@ -96,7 +95,7 @@ class WebsiteForm(http.Controller): 'custom': '', # Custom fields values } - authorized_fields = model.sudo().get_authorized_fields(); + authorized_fields = model.sudo()._get_form_writable_fields() error_fields = [] for field_name, field_value in kwargs.items(): @@ -183,7 +182,7 @@ class WebsiteForm(http.Controller): def insert_attachment(self, model, id_record, files): orphan_attachment_ids = [] record = model.env[model.model].browse(id_record) - authorized_fields = model.sudo().get_authorized_fields() + authorized_fields = model.sudo()._get_form_writable_fields() for file in files: custom_field = file.field_name not in authorized_fields attachment_value = { diff --git a/addons/website_form/models/models.py b/addons/website_form/models/models.py index e6e769f59e5..b1a2934e3f5 100644 --- a/addons/website_form/models/models.py +++ b/addons/website_form/models/models.py @@ -1,3 +1,5 @@ +import itertools + from openerp import tools from openerp import models, fields, api @@ -16,18 +18,36 @@ class website_form_model(models.Model): website_form_default_field_id = fields.Many2one('ir.model.fields', 'Field for custom form data', domain="[('model', '=', model), ('ttype', '=', 'text')]", help="Specify the field wich will contain meta and custom form fields datas.") website_form_label = fields.Char("Label for form action", help="Form action label. Ex: crm.lead could be 'Send an e-mail' and project.issue could be 'Create an Issue'.") + def _all_inherited_model_ids(self): + return list(itertools.chain( + [self.id], + *(m._all_inherited_model_ids() for m in self.inherited_model_ids) + )) - def all_inherited_model_ids(self): - return [self.id] + [m.all_inherited_model_ids() for m in self.inherited_model_ids] + def _get_form_writable_fields(self): + """ + Restriction of "authorized fields" (fields which can be used in the + form builders) to fields which have actually been opted into form + builders and are writable. By default no field is writable by the + form builder. + """ + excluded = { + field.name + for field in self.env['ir.model.fields'].sudo().search([ + ('model_id', 'in', self._all_inherited_model_ids()), + ('website_form_blacklisted', '=', True) + ]) + } + return { + k: v for k, v in self.get_authorized_fields().iteritems() + if k not in excluded + } @api.multi def get_authorized_fields(self): model = self.env[self.model] fields_get = model.fields_get() - for elem in self.env['ir.model.fields'].search([('model_id', 'in', self.all_inherited_model_ids())]): - if elem.website_form_blacklisted: - fields_get.pop(elem.name, None) for key, val in model._inherits.iteritems(): fields_get.pop(val,None) @@ -49,4 +69,38 @@ class website_form_model_fields(models.Model): _name = 'ir.model.fields' _inherit = 'ir.model.fields' - website_form_blacklisted = fields.Boolean('Blacklisted in web forms', help='Blacklist this field for web forms') + def init(self, cr): + # set all existing unset website_form_blacklisted fields to ``true`` + # (so that we can use it as a whitelist rather than a blacklist) + cr.execute('UPDATE ir_model_fields' + ' SET website_form_blacklisted=true' + ' WHERE website_form_blacklisted IS NULL') + # add an SQL-level default value on website_form_blacklisted to that + # pure-SQL ir.model.field creations (e.g. in _field_create) generate + # the right default value for a whitelist (aka fields should be + # blacklisted by default) + cr.execute('ALTER TABLE ir_model_fields ' + ' ALTER COLUMN website_form_blacklisted SET DEFAULT true') + + @api.model + def formbuilder_whitelist(self, model, fields): + """ + :param str model: name of the model on which to whitelist fields + :param list(str) fields: list of fields to whitelist on the model + :return: nothing of import + """ + # todo: check access rights yo + # the ORM only allows writing on custom fields and will trigger a + # registry reload once that's happened. We want to be able to + # whitelist non-custom fields and the registry reload absolutely + # isn't desirable, so go with a method and raw SQL + self.env.cr.execute( + "UPDATE ir_model_fields" + " SET website_form_blacklisted=false" + " WHERE model=%s AND name in %s", (model, tuple(fields))) + return False + + website_form_blacklisted = fields.Boolean( + 'Blacklisted in web forms', default=True, select=True, # required=True, + help='Blacklist this field for web forms' + ) diff --git a/addons/website_hr_recruitment/data/config_data.xml b/addons/website_hr_recruitment/data/config_data.xml index e61fb99c41f..24e4ed8b30f 100644 --- a/addons/website_hr_recruitment/data/config_data.xml +++ b/addons/website_hr_recruitment/data/config_data.xml @@ -23,5 +23,16 @@ True Apply for a Job + + hr.applicant + + diff --git a/addons/website_issue/data/website_issue_data.xml b/addons/website_issue/data/website_issue_data.xml index 8b043ee10e7..83f65b516e1 100644 --- a/addons/website_issue/data/website_issue_data.xml +++ b/addons/website_issue/data/website_issue_data.xml @@ -6,10 +6,5 @@ True Create an issue -