utils: fix interpolation of error messages in condition_validator (#86266)

condition_validator should not re-raise a new ValidationError, it breaks
interpolation of e.params in e.message.

src/authentic2/utils/evaluate.py

@@ -161,7 +161,10 @@ class BaseExpressionValidator(ast.NodeVisitor):
             ok = True
         if not ok:
             raise ExpressionError(
-                _('expression "%(expression)s" is forbidden'), node=node, code='forbidden-expression'
+                _('expression "%(expression)s" is forbidden'),
+                node=node,
+                code='forbidden-expression',
+                params={'expression': ast.unparse(node)},
             )
 
         # specific node class check
@@ -178,6 +181,14 @@ class BaseExpressionValidator(ast.NodeVisitor):
             # set the nearer expr node as the node of the error
             if e.node is None and hasattr(node, 'col_offset'):
                 e.set_node(node)
+            expression = ast.unparse(node)
+            if expression:
+                if not e.text:
+                    e.text = expression
+                if not e.params:
+                    e.params = {}
+                if not e.params.get('expression'):
+                    e.params['expression'] = expression
             raise e
 
     @lru_cache(maxsize=1024)
@@ -196,7 +207,10 @@ class BaseExpressionValidator(ast.NodeVisitor):
         except ExpressionError as e:
             if e.text is None:
                 e.text = expression
-            e.params = {'expression': expression}
+            if not e.params:
+                e.params = {}
+            if 'expression' not in e.params:
+                e.params['expression'] = expression
             raise e
         return compile(tree, expression, mode='eval')
 
@@ -279,10 +293,7 @@ validate_condition = ConditionValidator()
 
 
 def condition_validator(value):
-    try:
-        validate_condition(value)
-    except ExpressionError as e:
-        raise ValidationError(e.message)
+    validate_condition(value)
 
 
 condition_safe_globals = {

tests/test_utils_evaluate.py

@@ -19,12 +19,14 @@ import ast
 from unittest import mock
 
 import pytest
+from django.core.exceptions import ValidationError
 
 from authentic2.utils.evaluate import (
     BaseExpressionValidator,
     ConditionValidator,
     ExpressionError,
     HTTPHeaders,
+    condition_validator,
     evaluate_condition,
     make_condition_context,
 )
@@ -50,7 +52,7 @@ def test_base():
         assert v('x')
 
 
-def test_condition_validator():
+def test_condition_validator_klass():
     v = ConditionValidator()
     assert v('x < 2 and y == \'u\' or \'a\' in z')
     with pytest.raises(ExpressionError) as raised:
@@ -166,3 +168,13 @@ def test_sp_next_url():
 def test_evaluate_condition_template():
     assert evaluate_condition_template('foo == "bar"', {'foo': 'bar'}) is True
     assert evaluate_condition_template('foo != "bar"', {'foo': 'bar'}) is False
+
+
+def test_condition_validator():
+    with pytest.raises(ValidationError) as raised:
+        condition_validator(
+            "'backoffice' not in login_hint and not ('X-Entrouvert' in headers or remote_addr == '176.31.123.109' or remote_addr in dnsbl('ddns.entrouvert.org'))"
+        )
+    assert raised.value.messages == [
+        'expression "not (\'X-Entrouvert\' in headers or remote_addr == \'176.31.123.109\' or remote_addr in dnsbl(\'ddns.entrouvert.org\'))" is forbidden'
+    ]