workflows: check varname for FormWorkflowStatusItem (#49376)

tests/admin_pages/test_workflow.py

@@ -920,6 +920,17 @@ def test_workflows_edit_display_form_action(pub):
     assert 'You are about to remove the "foobar" field.' in resp.text
     assert 'Warning:' not in resp.text
 
+    resp = app.get('/backoffice/workflows/1/status/1/items/1/')
+    resp.form['varname'] = 'form'
+    resp = resp.form.submit('submit')
+    assert 'Wrong identifier detected: "form" prefix is forbidden.' in resp.text
+    resp.form['varname'] = 'form_foo'
+    resp = resp.form.submit('submit')
+    assert 'Wrong identifier detected: "form" prefix is forbidden.' in resp.text
+    resp.form['varname'] = 'formfoo'
+    resp = resp.form.submit('submit')
+    assert 'Wrong identifier detected: "form" prefix is forbidden.' not in resp.text
+
 
 def test_workflows_edit_choice_action(pub):
     create_superuser(pub)

wcs/admin/workflows.py

@@ -285,22 +285,23 @@ class WorkflowItemPage(Directory):
         if form.get_widget('cancel').parse():
             return redirect('..')
 
-        if not form.get_submit() == 'submit' or form.has_errors():
-            self.html_top('%s - %s' % (_('Workflow'), self.workflow.name))
-            r = TemplateIO(html=True)
-            r += htmltext('<h2>%s</h2>') % _(self.item.description)
-            r += form.render()
-            if self.item.support_substitution_variables:
-                r += get_publisher().substitutions.get_substitution_html_table()
-            return r.getvalue()
-        else:
+        if form.get_submit() == 'submit' and not form.has_errors():
             self.item.submit_admin_form(form)
-            self.workflow.store(
-                comment=_('Change in action "%(description)s" in status "%(status)s"') % {
-                    'description': self.item.render_as_line(),
-                    'status': self.parent.name,
-                })
-            return redirect('..')
+            if not form.has_errors():
+                self.workflow.store(
+                    comment=_('Change in action "%(description)s" in status "%(status)s"') % {
+                        'description': self.item.render_as_line(),
+                        'status': self.parent.name,
+                    })
+                return redirect('..')
+
+        self.html_top('%s - %s' % (_('Workflow'), self.workflow.name))
+        r = TemplateIO(html=True)
+        r += htmltext('<h2>%s</h2>') % _(self.item.description)
+        r += form.render()
+        if self.item.support_substitution_variables:
+            r += get_publisher().substitutions.get_substitution_html_table()
+        return r.getvalue()
 
     def delete(self):
         form = Form(enctype='multipart/form-data')

wcs/wf/form.py

@@ -117,6 +117,16 @@ class FormWorkflowStatusItem(WorkflowStatusItem):
     def get_parameters(self):
         return ('by', 'varname', 'condition')
 
+    def clean_varname(self, form):
+        widget = form.get_widget('varname')
+        new_value = widget.parse()
+
+        if new_value == 'form' or new_value.startswith('form_'):
+            widget.set_error(_('Wrong identifier detected: "form" prefix is forbidden.'))
+            return True
+
+        return False
+
     def migrate(self):
         changed = False
         if self.formdef and self.formdef.fields:

wcs/workflows.py

@@ -1943,6 +1943,10 @@ class WorkflowStatusItem(XmlSerialisable):
         for f in self.get_parameters():
             widget = form.get_widget(f)
             if widget:
+                if hasattr(self, 'clean_%s' % f):
+                    has_error = getattr(self, 'clean_%s' % f)(form)
+                    if has_error:
+                        continue
                 value = widget.parse()
                 if hasattr(self, '%s_parse' % f):
                     value = getattr(self, '%s_parse' % f)(value)
@@ -2376,8 +2380,7 @@ class CommentableWorkflowStatusItem(WorkflowStatusItem):
     def submit_admin_form(self, form):
         for f in self.get_parameters():
             widget = form.get_widget(f)
-            if widget:
-                setattr(self, f, widget.parse())
+            setattr(self, f, widget.parse())
 
     def fill_admin_form(self, form):
         if self.by and not type(self.by) is list: