firewall/filter: Create custom command that can swap two firewall rul…

…es with each other. Change position of the source and destination inversions in form. Replace Port and Sequence labels in Grid view to safe space. Ensure sequence is always ascending and cannot be sortet. For #8367
opnsense · Feb 22, 2025 · efc505f · efc505f
1 parent 821e915
commit efc505f
Show file tree

Hide file tree

Showing 3 changed files with 128 additions and 16 deletions.
diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Firewall/Api/FilterController.php b/src/opnsense/mvc/app/controllers/OPNsense/Firewall/Api/FilterController.php
@@ -64,4 +64,35 @@ public function toggleRuleAction($uuid, $enabled = null)
     {
         return $this->toggleBase("rules.rule", $uuid, $enabled);
     }
+
+    /**
+     * Swap the sequence numbers of two firewall filter rules.
+     *
+     * @param string $uuid       First rule UUID
+     * @param string $other_uuid Second rule UUID
+     * @return array             Status
+     */
+    public function swapSequenceAction($uuid, $other_uuid)
+    {
+        if ($this->request->isPost()) {
+            $mdl = $this->getModel();
+            $ruleA = $mdl->getNodeByReference("rules.rule." . $uuid);
+            $ruleB = $mdl->getNodeByReference("rules.rule." . $other_uuid);
+
+            if ($ruleA === null || $ruleB === null) {
+                return ["status" => "error"];
+            }
+
+            $seqA = (string)$ruleA->sequence;
+            $seqB = (string)$ruleB->sequence;
+            $ruleA->sequence = $seqB;
+            $ruleB->sequence = $seqA;
+
+            $mdl->serializeToConfig();
+            \OPNsense\Core\Config::getInstance()->save();
+
+            return ["status" => "ok"];
+        }
+        return ["status" => "error"];
+    }
 }
diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Firewall/forms/dialogFilterRule.xml b/src/opnsense/mvc/app/controllers/OPNsense/Firewall/forms/dialogFilterRule.xml
@@ -9,6 +9,7 @@
             <type>boolean</type>
             <formatter>rowtoggle</formatter>
             <sequence>1</sequence>
+            <align>center</align>
             <sortable>false</sortable>
         </grid_view>
     </field>
@@ -18,9 +19,12 @@
         <type>text</type>
         <help>The order in which rules are being processed.</help>
         <grid_view>
-            <width>6em</width>
+            <width>4em</width>
             <sequence>2</sequence>
             <visible>false</visible>
+            <!-- The sequence order of firewall rules is absolute, no other field shall be sorted -->
+            <order>asc</order>
+            <sortable>false</sortable>
         </grid_view>
     </field>
     <field>
@@ -144,6 +148,15 @@
             <sortable>false</sortable>
         </grid_view>
     </field>
+    <field>
+        <id>rule.source_not</id>
+        <label>Source / Invert</label>
+        <type>checkbox</type>
+        <help>Use this option to invert the sense of the match.</help>
+        <grid_view>
+            <ignore>true</ignore>
+        </grid_view>
+    </field>
     <field>
         <id>rule.source_net</id>
         <label>Source</label>
@@ -167,8 +180,8 @@
         </grid_view>
     </field>
     <field>
-        <id>rule.source_not</id>
-        <label>Source / Invert</label>
+        <id>rule.destination_not</id>
+        <label>Destination / Invert</label>
         <type>checkbox</type>
         <help>Use this option to invert the sense of the match.</help>
         <grid_view>
@@ -186,15 +199,6 @@
             <sortable>false</sortable>
         </grid_view>
     </field>
-    <field>
-        <id>rule.destination_not</id>
-        <label>Destination / Invert</label>
-        <type>checkbox</type>
-        <help>Use this option to invert the sense of the match.</help>
-        <grid_view>
-            <ignore>true</ignore>
-        </grid_view>
-    </field>
     <field>
         <id>rule.destination_port</id>
         <label>Destination Port</label>

diff --git a/src/opnsense/mvc/app/views/OPNsense/Firewall/filter_rule.volt b/src/opnsense/mvc/app/views/OPNsense/Firewall/filter_rule.volt
@@ -27,7 +27,7 @@
 <script>
     $(document).ready(function() {
         // Add column for firewall rule icons
-        $('#{{formGridFilterRule['table_id']}} thead tr th[data-column-id="enabled"]')
+        $('#{{formGridFilterRule['table_id']}} thead tr th[data-column-id="sequence"]')
         .after(
             '<th ' +
                 'data-column-id="icons" ' +
@@ -39,14 +39,29 @@
             '</th>'
         );
 
+        // Show errors in modal
+        function showDialogAlert(type, title, message) {
+            BootstrapDialog.show({
+                type: type,
+                title: title,
+                message: message,
+                buttons: [{
+                    label: '{{ lang._('Close') }}',
+                    action: function(dialogRef) {
+                        dialogRef.close();
+                    }
+                }]
+            });
+        }
+
         const grid = $("#{{formGridFilterRule['table_id']}}").UIBootgrid({
             search:'/api/firewall/filter/search_rule/',
             get:'/api/firewall/filter/get_rule/',
             set:'/api/firewall/filter/set_rule/',
             add:'/api/firewall/filter/add_rule/',
             del:'/api/firewall/filter/del_rule/',
             toggle:'/api/firewall/filter/toggle_rule/',
-            options:{
+            options: {
                 triggerEditFor: getUrlHash('edit'),
                 initialSearchPhrase: getUrlHash('search'),
                 requestHandler: function(request){
@@ -128,12 +143,74 @@
                     },
 
                 },
-            }
+            },
+            commands: {
+                // Swap the sequence number of exactly two rules with one another
+                swap_sequence: {
+                    method: function(event) {
+                        // We want exactly one selected row
+                        const selected = $("#{{ formGridFilterRule['table_id'] }}").bootgrid("getSelectedRows");
+                        if (selected.length !== 1) {
+                            showDialogAlert(
+                                BootstrapDialog.TYPE_WARNING,
+                                "{{ lang._('Selection Error') }}",
+                                "{{ lang._('Please select exactly one other rule first.') }}"
+                            );
+                            return;
+                        }
 
+                        const currentUuid = $(this).data("row-id");
+                        const otherUuid = selected[0];
+
+                        // Make sure they are not the same rule
+                        if (currentUuid === otherUuid) {
+                            showDialogAlert(
+                                BootstrapDialog.TYPE_WARNING,
+                                "{{ lang._('Swap Error') }}",
+                                "{{ lang._('Cannot swap a rule with itself.') }}"
+                            );
+                            return;
+                        }
+
+                        ajaxCall(
+                            "/api/firewall/filter/swap_sequence/" + otherUuid + "/" + currentUuid,
+                            {},
+                            function(data, status) {
+                                if (data.status === "ok") {
+                                    std_bootgrid_reload("{{ formGridFilterRule['table_id'] }}");
+                                    // XXX: Success is implied by not emitting an error
+                                } else {
+                                    let msg = data.message || "{{ lang._('Error swapping sequence.') }}";
+                                    showDialogAlert(
+                                        BootstrapDialog.TYPE_WARNING,
+                                        "{{ lang._('Swap Error') }}",
+                                        msg
+                                    );
+                                }
+                            },
+                            function(xhr, textStatus, errorThrown) {
+                                showDialogAlert(
+                                    BootstrapDialog.TYPE_DANGER,
+                                    "{{ lang._('Request Failed') }}",
+                                    errorThrown
+                                );
+                            },
+                            'POST'
+                        );
+                    },
+                    classname: 'fa fa-fw fa-exchange',
+                    title: "{{ lang._('Swap Rule Sequence') }}",
+                    sequence: 10
+                }
+            }
         });
 
         $("#{{formGridFilterRule['table_id']}}").on('loaded.rs.jquery.bootgrid', function() {
                 $('[data-toggle="tooltip"]').tooltip();
+                // XXX: Replace these labels to save some space in the grid
+                $(this).find('th[data-column-id="sequence"] .text').text("{{ lang._('Seq') }}");
+                $(this).find('th[data-column-id="source_port"] .text').text("{{ lang._('Port') }}");
+                $(this).find('th[data-column-id="destination_port"] .text').text("{{ lang._('Port') }}");
         });
 
         // Reload categories before grid load
@@ -302,7 +379,7 @@
             </div>
         </div>
         <!-- tab page "rules" -->
-        {{ partial('layout_partials/base_bootgrid_table', formGridFilterRule)}}
+        {{ partial('layout_partials/base_bootgrid_table', formGridFilterRule + {'command_width': '10em'}) }}
     </div>
 </div>