[IMP] core: allow some users to bypass the sanitize of HTML field

Add the possibility to flag a HTML field as `sanitize_overridable`.
The sanitizer will then be bypassed if the user doing the operation is
part of the new group `base.group_sanitize_override`.
The "Settings" users are part of that new group.

If such a user wrote some HTML that would have normally been removed by
the sanitizer, then users without the right to bypass the sanitizer
won't be able to write on that field anymore.
Otherwise, it would sanitize the previously written data.
Such cases are detected, and the modification prevented by the system,
which will warn the user about it.

For instance, with a `field.html(sanitize_overridable=True)`: one being
part of `group_sanitize_override` could write `<script></script>`.
Then, someone not part of the group trying to add an element inside that
field like appending a `<p/>` -> `<script></script><p>New Content</p>`
would not be able to because it would go through the sanitizer and
ultimately, removing part of the original value:
`<p>New Content</p>` (`<script></script>` would be removed).

== Real use case ==
In the website builder, there is 2 editor rights:
- group_website_publisher: restricted editor
- group_website_designer: editor & designer

The designer editor can edit pages and views, while the restricted
editor can't do anything unless he is part of other groups.
In edit mode, the restricted editor will only be able to edit fields of
record he has access to.
For instance, being a sales manager allows you to edit a product
description on the website.
Being an event manager -> edit event. Slide manager -> Slides etc.
As those restricted editor are able to edit those fields, they are
(almost) always sanitized to prevent them to introduce malicious code.

Since those fields are sanitized, even admins / designer editor are not
able to fully use the website builder in such fields.

Some clients don't really care about that sanitation, they'd prefer to
avoid it as they trust their manager and would prefer to have the full
builder capability instead.
This is typically the case in small project (butcher, hairdresser,
reseller etc) and in SMEs.

With the new `sanitize_overridable` feature, they will be able to do
that, as the "Designer & Editor" group now also receive the group
`base.group_sanitize_override` (done in next commit).

Part-of: odoo/odoo#97398

addons/web_editor/models/ir_qweb_fields.py

@@ -380,7 +380,7 @@ class HTML(models.AbstractModel):
         attrs = super().attributes(record, field_name, options, values)
         if options.get('inherit_branding'):
             field = record._fields[field_name]
-            if field.sanitize:
+            if field.sanitize and not (field.sanitize_overridable and record.user_has_groups('base.group_sanitize_override')):
                 attrs['data-oe-sanitize'] = 1 if field.sanitize_form else 'allow_form'
         return attrs
 

odoo/addons/base/security/base_groups.xml

@@ -12,9 +12,13 @@
             <field name="implied_ids" eval="[Command.link(ref('group_user'))]"/>
         </record>
 
+        <record id="group_sanitize_override" model="res.groups">
+            <field name="name">Bypass HTML Field Sanitize</field>
+        </record>
+
         <record model="res.groups" id="group_system">
             <field name="name">Settings</field>
-            <field name="implied_ids" eval="[Command.link(ref('group_erp_manager'))]"/>
+            <field name="implied_ids" eval="[Command.link(ref('group_erp_manager')), Command.link(ref('group_sanitize_override'))]"/>
             <field name="users" eval="[Command.link(ref('base.user_root')), Command.link(ref('base.user_admin'))]"/>
         </record>
 

odoo/addons/test_new_api/models/test_new_api.py

@@ -315,6 +315,7 @@ class MixedModel(models.Model):
     comment2 = fields.Html(sanitize_attributes=True, strip_classes=False)
     comment3 = fields.Html(sanitize_attributes=True, strip_classes=True)
     comment4 = fields.Html(sanitize_attributes=True, strip_style=True)
+    comment5 = fields.Html(sanitize_overridable=True, sanitize_attributes=False)
 
     currency_id = fields.Many2one('res.currency', default=lambda self: self.env.ref('base.EUR'))
     amount = fields.Monetary()

odoo/addons/test_new_api/tests/test_new_fields.py

@@ -2969,6 +2969,55 @@ class TestHtmlField(common.TransactionCase):
 
         self.assertNotIn('<tr style="', record.comment4, 'Style attr should have been stripped')
 
+    def test_01_sanitize_groups(self):
+        self.assertEqual(self.model._fields['comment5'].sanitize, True)
+        self.assertEqual(self.model._fields['comment5'].sanitize_overridable, True)
+
+        internal_user = self.env['res.users'].create({
+            'name': 'test internal user',
+            'login': 'test_sanitize',
+            'groups_id': [(6, 0, [self.ref('base.group_user')])],
+        })
+        bypass_user = self.env['res.users'].create({
+            'name': 'test bypass user',
+            'login': 'test_sanitize2',
+            'groups_id': [(6, 0, [self.ref('base.group_user'), self.ref('base.group_sanitize_override')])],
+        })
+        record = self.env['test_new_api.mixed'].create({})
+
+        # 1. Test main use case: prevent restricted user to wipe non restricted
+        #    user previous change
+        val = '<script></script>'
+        write_vals = {'comment5': val}
+
+        record.with_user(internal_user).write(write_vals)
+        self.assertEqual(record.comment5, '',
+                         "should be sanitized (not in groups)")
+        record.with_user(bypass_user).write(write_vals)
+        self.assertEqual(record.comment5, val,
+                         "should not be sanitized (has group)")
+        with self.assertRaises(UserError):
+            # should crash (not in groups and sanitize would break content of
+            # other user that bypassed the sanitize)
+            record.with_user(internal_user).write(write_vals)
+
+        # 2. Make sure field compare in `_convert` is working as expected with
+        #    special content / format
+        val = '<span  attr1 ="att1"   attr2=\'attr2\'>é@&nbsp;</span><p><span/></p>'
+        write_vals = {'comment5': val}
+        # Once sent through `html_sanitize()` this is becoming:
+        # `<span attr1="att1" attr2="attr2">é@\xa0</span><p><span></span></p>`
+        # Notice those change:
+        # -     `attr1 =` -> `attr1=`    (space before `=`)
+        # -    `   attr2` -> ` attr2`    (multi space -> single space)
+        # -  `=\'attr2\'` -> `="attr2"`  (escaped single quote -> double quote)
+        # -      `&nbsp;` -> `\xa0`
+        # Still, those 2 archs should be considered equals and not raise
+
+        record.with_user(bypass_user).write(write_vals)
+        # Next write shouldn't raise a sanitize right error
+        record.with_user(internal_user).write(write_vals)
+
 
 class TestMagicFields(common.TransactionCase):
 

odoo/fields.py

@@ -5,6 +5,7 @@
 
 from collections import defaultdict
 from datetime import date, datetime, time
+from lxml import etree, html
 from operator import attrgetter
 from xmlrpc.client import MAXINT
 import base64
@@ -1817,6 +1818,8 @@ class Html(_String):
     """ Encapsulates an html code content.
 
     :param bool sanitize: whether value must be sanitized (default: ``True``)
+    :param bool sanitize_overridable: whether the sanitation can be bypassed by
+        the users part of the `base.group_sanitize_override` group (default: ``False``)
     :param bool sanitize_tags: whether to sanitize tags
         (only a white list of attributes is accepted, default: ``True``)
     :param bool sanitize_attributes: whether to sanitize attributes
@@ -1831,6 +1834,7 @@ class Html(_String):
     column_cast_from = ('varchar',)
 
     sanitize = True                     # whether value must be sanitized
+    sanitize_overridable = False        # whether the sanitation can be bypassed by the users part of the `base.group_sanitize_override` group
     sanitize_tags = True                # whether to sanitize tags (only a white list of attributes is accepted)
     sanitize_attributes = True          # whether to sanitize attributes (only a white list of attributes is accepted)
     sanitize_style = False              # whether to sanitize style attributes
@@ -1861,32 +1865,51 @@ class Html(_String):
     _description_strip_classes = property(attrgetter('strip_classes'))
 
     def convert_to_column(self, value, record, values=None, validate=True):
-        if value is None or value is False:
-            return None
-        if self.sanitize:
-            return html_sanitize(
-                value, silent=True,
-                sanitize_tags=self.sanitize_tags,
-                sanitize_attributes=self.sanitize_attributes,
-                sanitize_style=self.sanitize_style,
-                sanitize_form=self.sanitize_form,
-                strip_style=self.strip_style,
-                strip_classes=self.strip_classes)
-        return value
+        return self._convert(value, record, True)
 
     def convert_to_cache(self, value, record, validate=True):
+        return self._convert(value, record, validate)
+
+    def _convert(self, value, record, validate):
         if value is None or value is False:
             return None
-        if validate and self.sanitize:
-            return html_sanitize(
-                value, silent=True,
-                sanitize_tags=self.sanitize_tags,
-                sanitize_attributes=self.sanitize_attributes,
-                sanitize_style=self.sanitize_style,
-                sanitize_form=self.sanitize_form,
-                strip_style=self.strip_style,
-                strip_classes=self.strip_classes)
-        return value
+
+        if not validate or not self.sanitize:
+            return value
+
+        sanitize_vals = {
+            'silent': True,
+            'sanitize_tags': self.sanitize_tags,
+            'sanitize_attributes': self.sanitize_attributes,
+            'sanitize_style': self.sanitize_style,
+            'sanitize_form': self.sanitize_form,
+            'strip_style': self.strip_style,
+            'strip_classes': self.strip_classes
+        }
+
+        if self.sanitize_overridable:
+            if record.user_has_groups('base.group_sanitize_override'):
+                return value
+
+            original_value = record[self.name]
+            if original_value:
+                initial_value_sanitized = html_sanitize(original_value, **sanitize_vals)
+
+                def get_parsed(val):
+                    return etree.tostring(html.fromstring(val))
+
+                # could have been emptied by the sanitizer
+                if (
+                    not initial_value_sanitized
+                    or get_parsed(original_value) != get_parsed(initial_value_sanitized)
+                ):
+                    # The field contains element(s) that would be removed if
+                    # sanitized. It means that someone who was part of a group
+                    # allowing to bypass the sanitation saved that field
+                    # previously.
+                    raise UserError(_("Someone with escalated rights previously modified this field (%s %s), you are therefore not able to modify it yourself.", record._description, self.string))
+
+        return html_sanitize(value, **sanitize_vals)
 
     def convert_to_record(self, value, record):
         r = super().convert_to_record(value, record)