From dfa2760bb45c888e342f38bd09618a59f41060e4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= <fpeters@entrouvert.com>
Date: Mon, 20 Dec 2021 09:36:37 +0100
Subject: [PATCH] workflows: ignore reordering with unknown action ids (#59898)

---
 tests/admin_pages/test_workflow.py | 41 +++++++++++++++++++++
 wcs/admin/workflows.py             | 59 +++++++++++++++++-------------
 wcs/workflows.py                   |  5 +--
 3 files changed, 76 insertions(+), 29 deletions(-)

diff --git a/tests/admin_pages/test_workflow.py b/tests/admin_pages/test_workflow.py
index 72f094299..1ed7a9c07 100644
--- a/tests/admin_pages/test_workflow.py
+++ b/tests/admin_pages/test_workflow.py
@@ -790,6 +790,47 @@ def test_workflows_check_available_actions(pub):
         resp = resp.follow()  # redirect to ./
 
 
+def test_workflows_status_reorder_actions(pub):
+    create_superuser(pub)
+
+    Workflow.wipe()
+    workflow = Workflow(name='foo')
+    st1 = workflow.add_status(name='baz')
+    choice1 = st1.append_item('choice')
+    choice1.by = ['logged-users']
+    choice1.status = 'wf-%s' % st1.id
+    choice1.label = 'choice1'
+    choice2 = st1.append_item('choice')
+    choice2.by = ['logged-users']
+    choice2.status = 'wf-%s' % st1.id
+    choice2.label = 'choice2'
+    workflow.store()
+
+    app = login(get_app(pub))
+    resp = app.get('/backoffice/workflows/%s/status/%s/' % (workflow.id, st1.id))
+    assert resp.text.index('choice1') < resp.text.index('choice2')
+
+    resp = app.get(
+        '/backoffice/workflows/%s/status/%s/update_order?order=%s;%s;'
+        % (workflow.id, st1.id, choice1.id, choice2.id)
+    )
+    assert resp.json == {'err': 0}
+
+    resp = app.get(
+        '/backoffice/workflows/%s/status/%s/update_order?order=%s;%s;'
+        % (workflow.id, st1.id, choice2.id, choice1.id)
+    )
+    assert resp.json == {'err': 0}
+
+    resp = app.get('/backoffice/workflows/%s/status/%s/' % (workflow.id, st1.id))
+    assert resp.text.index('choice2') < resp.text.index('choice1')
+
+    resp = app.get(
+        '/backoffice/workflows/%s/status/%s/update_order?order=%s;%s;' % (workflow.id, st1.id, '123', '456')
+    )  # invalid action id
+    assert resp.json == {'err': 1}
+
+
 def test_workflows_edit_dispatch_action(pub):
     create_superuser(pub)
     pub.role_class.wipe()
diff --git a/wcs/admin/workflows.py b/wcs/admin/workflows.py
index bf384d3f5..67890a9ec 100644
--- a/wcs/admin/workflows.py
+++ b/wcs/admin/workflows.py
@@ -15,6 +15,7 @@
 # along with this program; if not, see <http://www.gnu.org/licenses/>.
 
 import io
+import json
 import textwrap
 import time
 import xml.etree.ElementTree as ET
@@ -69,6 +70,20 @@ def is_global_accessible():
     return get_publisher().get_backoffice_root().is_global_accessible('workflows')
 
 
+def update_order(elements):
+    request = get_request()
+    new_order = request.form['order'].strip(';').split(';')
+    new_elements = []
+    for y in new_order:
+        element = [x for x in elements if x.id == y]
+        if not element:
+            continue
+        new_elements.append(element[0])
+    if {element.id for element in new_elements} != {element.id for element in elements}:
+        return None
+    return new_elements
+
+
 def svg(tag):
     return '{http://www.w3.org/2000/svg}%s' % tag
 
@@ -623,11 +638,13 @@ class WorkflowStatusPage(Directory):
         return form
 
     def update_order(self):
-        request = get_request()
-        new_order = request.form['order'].strip(';').split(';')
-        self.status.items = [[x for x in self.status.items if x.id == y][0] for y in new_order]
+        get_response().set_content_type('application/json')
+        reordered_items = update_order(self.status.items)
+        if reordered_items is None:
+            return json.dumps({'err': 1})
+        self.status.items = reordered_items
         self.workflow.store(comment=_('Change in action order'))
-        return 'ok'
+        return json.dumps({'err': 0})
 
     def newitem(self):
         form = self.get_new_item_form()
@@ -1719,42 +1736,32 @@ class WorkflowPage(Directory):
         r += htmltext('</div>')
         return r.getvalue()
 
-    def _update_order(self, elements):
-        request = get_request()
-        new_order = request.form['order'].strip(';').split(';')
-        new_elements = []
-        for y in new_order:
-            status = [x for x in elements if x.id == y]
-            if not status:
-                continue
-            new_elements.append(status[0])
-        if {s.id for s in new_elements} != {s.id for s in elements}:
-            return None
-        return new_elements
-
     def update_order(self):
-        new_possible_status = self._update_order(self.workflow.possible_status)
+        get_response().set_content_type('application/json')
+        new_possible_status = update_order(self.workflow.possible_status)
         if new_possible_status is None:
-            return 'ko'
+            return json.dumps({'err': 1})
         self.workflow.possible_status = new_possible_status
         self.workflow.store(comment=_('Change in status order'))
-        return 'ok'
+        return json.dumps({'err': 0})
 
     def update_actions_order(self):
-        new_global_actions = self._update_order(self.workflow.global_actions)
+        get_response().set_content_type('application/json')
+        new_global_actions = update_order(self.workflow.global_actions)
         if new_global_actions is None:
-            return 'ko'
+            return json.dumps({'err': 1})
         self.workflow.global_actions = new_global_actions
         self.workflow.store(comment=_('Change in global actions order'))
-        return 'ok'
+        return json.dumps({'err': 0})
 
     def update_criticality_levels_order(self):
-        new_criticality_levels = self._update_order(self.workflow.criticality_levels)
+        get_response().set_content_type('application/json')
+        new_criticality_levels = update_order(self.workflow.criticality_levels)
         if new_criticality_levels is None:
-            return 'ko'
+            return json.dumps({'err': 1})
         self.workflow.criticality_levels = new_criticality_levels
         self.workflow.store(comment=_('Change in criticality levels order'))
-        return 'ok'
+        return json.dumps({'err': 0})
 
     def newstatus(self):
         form = Form(enctype='multipart/form-data', action='newstatus')
diff --git a/wcs/workflows.py b/wcs/workflows.py
index 3e3e8350c..91ce419c8 100644
--- a/wcs/workflows.py
+++ b/wcs/workflows.py
@@ -1792,9 +1792,8 @@ class WorkflowStatus:
                 else:
                     o.id = '1'
                 self.items.append(o)
-                break
-        else:
-            raise KeyError()
+                return o
+        raise KeyError()
 
     def get_item(self, id):
         for item in self.items:
-- 
2.34.1

