From 7fd5cec72b3cd8c30fbc08a48caafe61aedaa442 Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Tue, 29 Nov 2022 13:43:53 +0000 Subject: [PATCH] [IMP] base: view validation: detect js implementation details This commit adds checks in the view validation, to detect the use of implementation details in archs. The goal is to prevent people from using owl directives in archs (e.g. t-on-click). Recall that a view arch isn't an owl template, it must follow the DSL of the given view type. In kanban, gantt and activity archs, using `t-xxx` attributes is forbidden, except for directives historically supported by qweb (e.g. `t-esc`). In other views, using `t-xxx` attributes is forbidden (except for `t-translation`). In all views, using attributes `data-tooltip`, `data-tooltip-template` and `data-tooltip-info` is forbidden. Task 3085357 closes odoo/odoo#107083 Related: odoo/enterprise#35352 Signed-off-by: Aaron Bohy (aab) --- addons/mail/models/ir_ui_view.py | 3 + odoo/addons/base/models/ir_ui_view.py | 40 +++++++++++- odoo/addons/base/tests/test_views.py | 90 +++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 3 deletions(-) diff --git a/addons/mail/models/ir_ui_view.py b/addons/mail/models/ir_ui_view.py index db1a2d4f325..a819c0bbfa0 100644 --- a/addons/mail/models/ir_ui_view.py +++ b/addons/mail/models/ir_ui_view.py @@ -14,3 +14,6 @@ class View(models.Model): name_manager.has_field(node, node.get('name'), {}) return return super()._postprocess_tag_field(node, name_manager, node_info) + + def _is_qweb_based_view(self, view_type): + return view_type == "activity" or super()._is_qweb_based_view(view_type) diff --git a/odoo/addons/base/models/ir_ui_view.py b/odoo/addons/base/models/ir_ui_view.py index dd3b9391db9..ab21145fba3 100644 --- a/odoo/addons/base/models/ir_ui_view.py +++ b/odoo/addons/base/models/ir_ui_view.py @@ -2,15 +2,12 @@ import ast import collections -import datetime import functools import inspect import json import logging -import math import pprint import re -import time import uuid import warnings @@ -1417,6 +1414,7 @@ actual arch. model = self.env[model_name].with_context(lang=None) name_manager = NameManager(model) + view_type = node.tag # use a stack to recursively traverse the tree stack = [(node, editable, full)] while stack: @@ -1428,6 +1426,7 @@ actual arch. node_info = { 'editable': editable and self._editable_node(node, name_manager), 'validate': validate, + 'view_type': view_type, } # tag-specific validation @@ -1733,6 +1732,9 @@ actual arch. msg = 'o_progressbar class must have aria-valuemaxattribute' self._log_view_warning(msg, node) + def _is_qweb_based_view(self, view_type): + return view_type in ("kanban", "gantt") + def _validate_attrs(self, node, name_manager, node_info): """ Generic validation of node attrs. """ for attr, expr in node.items(): @@ -1813,6 +1815,12 @@ actual arch. msg = "attribute 'group' is not valid. Did you mean 'groups'?" self._log_view_warning(msg, node) + elif (re.match(r'^(t\-att\-|t\-attf\-)?data-tooltip(-template|-info)?$', attr)): + self._raise_view_error(_("Forbidden attribute used in arch (%s).", attr), node) + + elif (attr.startswith("t-")): + self._validate_qweb_directive(node, attr, node_info["view_type"]) + def _validate_classes(self, node, expr): """ Validate the classes present on node. """ classes = set(expr.split(' ')) @@ -1924,6 +1932,32 @@ actual arch. msg = '%s must have title in its tag, parents, descendants or have text' self._log_view_warning(msg % description, node) + def _validate_qweb_directive(self, node, directive, view_type): + """Some views (e.g. kanban, form) generate owl templates from the archs. + However, we don't want to see owl directives directly written in archs. + There are exceptions though, since the kanban and gantt archs define qweb templates. + We thus here validate that the given directive is allowed, according to the view_type. + """ + allowed_directives = ["t-translation"] + if self._is_qweb_based_view(view_type): + allowed_directives.extend([ + "t-name", + "t-esc", + "t-out", + "t-set", + "t-value", + "t-if", + "t-else", + "t-elif", + "t-foreach", + "t-as", + "t-key", + "t-att.*", + "t-call", + ]) + if (not next(filter(lambda regex: re.match(regex, directive), allowed_directives), None)): + self._raise_view_error(_("Forbidden owl directive used in arch (%s).", directive), node) + def _get_domain_identifiers(self, node, domain, use, expr=None): try: return get_domain_identifiers(domain) diff --git a/odoo/addons/base/tests/test_views.py b/odoo/addons/base/tests/test_views.py index 15ac8377fe7..e1631cbf8f6 100644 --- a/odoo/addons/base/tests/test_views.py +++ b/odoo/addons/base/tests/test_views.py @@ -3279,6 +3279,96 @@ class TestViews(ViewCase): "The view test_views_test_view_ref should not be in the views of the many2many field groups_id" ) + @mute_logger('odoo.addons.base.models.ir_ui_view') + def test_forbidden_owl_directives_in_form(self): + arch = "
%s
" + + self.assertInvalid( + arch % (''), + """Error while validating view near: + +
+Forbidden owl directive used in arch (t-esc).""", + ) + + self.assertInvalid( + arch % (''), + """Error while validating view near: + +
+Forbidden owl directive used in arch (t-on-click).""", + ) + + @mute_logger('odoo.addons.base.models.ir_ui_view') + def test_forbidden_owl_directives_in_kanban(self): + arch = "%s" + + self.assertValid(arch % ('')) + + self.assertInvalid( + arch % (''), + """Error while validating view near: + + +Forbidden owl directive used in arch (t-on-click).""", + ) + + @mute_logger('odoo.addons.base.models.ir_ui_view') + def test_forbidden_data_tooltip_attributes_in_form(self): + arch = "
%s
" + + self.assertInvalid( + arch % (''), + """Error while validating view near: + +
+Forbidden attribute used in arch (data-tooltip).""" + ) + + self.assertInvalid( + arch % (''), + """Error while validating view near: + +
+Forbidden attribute used in arch (data-tooltip-template).""" + ) + + @mute_logger('odoo.addons.base.models.ir_ui_view') + def test_forbidden_data_tooltip_attributes_in_kanban(self): + arch = "%s" + + self.assertInvalid( + arch % (''), + """Error while validating view near: + + +Forbidden attribute used in arch (data-tooltip).""" + ) + + self.assertInvalid( + arch % (''), + """Error while validating view near: + + +Forbidden attribute used in arch (data-tooltip-template).""" + ) + + self.assertInvalid( + arch % (''), + """Error while validating view near: + + +Forbidden attribute used in arch (t-att-data-tooltip).""" + ) + + self.assertInvalid( + arch % (''), + """Error while validating view near: + + +Forbidden attribute used in arch (t-attf-data-tooltip-template).""" + ) + class TestViewTranslations(common.TransactionCase): # these tests are essentially the same as in test_translate.py, but they use