diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Firewall/Api/FilterController.php b/src/opnsense/mvc/app/controllers/OPNsense/Firewall/Api/FilterController.php index 4bc5e2969c..b04d0fd429 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/Firewall/Api/FilterController.php +++ b/src/opnsense/mvc/app/controllers/OPNsense/Firewall/Api/FilterController.php @@ -93,7 +93,7 @@ class FilterController extends FilterBaseController // interface param may be empty $interfaces = array_filter(explode(',', (string)$this->request->get('interface')), 'strlen'); - if ($show_all && !empty($interfaces)) { + if (!empty($interfaces)) { /* add groups which contain the selected interface when looking at full impact */ foreach ((new Group())->ifgroupentry->iterateItems() as $groupItem) { if (array_intersect($interfaces, $groupItem->members->getValues())) { @@ -125,15 +125,15 @@ class FilterController extends FilterBaseController $rule_stats = []; } - $filter_funct_rs = function (&$record) use ($categories, $interfaces, $rule_stats, $show_all) { + $filter_funct_rs = function (&$record) use ($categories, $interfaces, $rule_stats) { /* Filter criteria */ $r_categories = !empty($record['categories']) ? array_map('trim', explode(',', $record['categories'])) : []; $is_cat = empty($categories) || array_intersect($r_categories, $categories); $rule_interfaces = array_filter(explode(',', $record['interface'] ?? '')); - if ($interfaces === null || (empty($record['interface']) && $show_all)) { + if ($interfaces === null || (empty($record['interface']))) { /* ALL interfaces always matches, when inspecting, also show rules that apply to all */ $is_if = true; // ALL interfaces or floating always matches - } elseif (!empty($record['interfacenot']) && $show_all) { + } elseif (!empty($record['interfacenot'])) { /* Inverted interface, show where applicable when inspecting */ $is_if = !array_intersect($rule_interfaces, $interfaces ?? []); } elseif (empty($interfaces) && (count($rule_interfaces) != 1 || !empty($record['interfacenot']))) { @@ -142,7 +142,7 @@ class FilterController extends FilterBaseController } else { /* Interfaces overlap, when inspecting all overlaps are relevant, otherwise only exact matches */ $is_if = array_intersect($rule_interfaces, $interfaces ?? []) && ( - count($rule_interfaces) == 1 || $show_all + count($rule_interfaces) == 1 ) && empty($record['interfacenot']); } @@ -170,15 +170,8 @@ class FilterController extends FilterBaseController $record = array_merge($record, $rule_stats[$record['uuid']]); } - // Tag legacy rules as "Automatic generated rules" if they have an empty category - if (!empty($record['is_automatic'])) { - $label = gettext('Automatically generated rules'); - $record['categories'] = $label; // Grouping key for tree view - $record['category_colors'] = [['name' => $label]]; // Category formatter metadata - } else { - /* frontend can format categories with colors */ - $record['category_colors'] = $this->getCategoryColors($r_categories); - } + /* frontend can format categories with colors */ + $record['category_colors'] = $this->getCategoryColors($r_categories); /* frontend can format aliases with an alias icon */ foreach (['source_net','source_port','destination_net','destination_port'] as $field) { @@ -190,17 +183,16 @@ class FilterController extends FilterBaseController return true; }; - /* only fetch internal and legacy rules when 'show_all' is set */ - if ($show_all) { - $allrules = array_merge( - $allrules, - json_decode((new Backend())->configdRun('filter list non_mvc_rules'), true) ?? [] - ); - } + /* always fetch internal and legacy rules, automatic rules have their own category that is always visible */ + $allrules = array_merge( + $allrules, + json_decode((new Backend())->configdRun('filter list non_mvc_rules'), true) ?? [] + ); $search_clauses = []; $backend = new Backend(); foreach (preg_split('/\s+/', (string)$this->request->getPost('searchPhrase', null, '')) as $token) { + // XXX: ideally this should get its own parameter and not reuse show_all if ($show_all && Util::isIpAddress($token)) { $tmp = json_decode($backend->configdpRun('filter find_table_references', [$token]), true) ?? []; $aliases = [$token]; @@ -385,31 +377,15 @@ class FilterController extends FilterBaseController ], ]; - // Count rules per interface - $ruleCounts = []; - foreach ((new Filter())->rules->rule->iterateItems() as $rule) { - $interfaces = $rule->interface->getValues(); - - if (!$rule->interfacenot->isEmpty() || count($interfaces) !== 1) { - // floating: empty, multiple, or inverted interface - $ruleCounts['floating'] = ($ruleCounts['floating'] ?? 0) + 1; - } else { - // single interface - $ruleCounts[$interfaces[0]] = ($ruleCounts[$interfaces[0]] ?? 0) + 1; - } - } - $totalRules = array_sum($ruleCounts); - - // Helper to build item with label and count - $makeItem = fn($value, $label, $count, $type) => [ + // Helper to build item + $makeItem = fn($value, $label, $type) => [ 'value' => $value, 'label' => $label, - 'count' => $count, 'type' => $type ]; // Floating - $result['floating']['items'][] = $makeItem('__floating', gettext('Floating'), $ruleCounts['floating'] ?? 0, 'floating'); + $result['floating']['items'][] = $makeItem('__floating', gettext('Floating'), 'floating'); // Groups + Interfaces foreach (Config::getInstance()->object()->interfaces->children() as $key => $intf) { @@ -419,11 +395,11 @@ class FilterController extends FilterBaseController } $descr = !empty($intf->descr) ? (string)$intf->descr : strtoupper($key); $type = (string)$intf->type == 'group' ? 'group' : 'interface'; - $result["{$type}s"]['items'][] = $makeItem($key, $descr, $ruleCounts[$key] ?? 0, $type); + $result["{$type}s"]['items'][] = $makeItem($key, $descr, $type); } - // ALL rules - $result['any']['items'][] = $makeItem('__any', gettext('All rules'), $totalRules, 'any'); + // All rules + $result['any']['items'][] = $makeItem('__any', gettext('All rules'), 'any'); foreach ($result as &$section) { usort($section['items'], fn($a, $b) => strcasecmp($a['label'], $b['label'])); diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Firewall/forms/dialogFilterRule.xml b/src/opnsense/mvc/app/controllers/OPNsense/Firewall/forms/dialogFilterRule.xml index 6b657cdca7..dadce138db 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/Firewall/forms/dialogFilterRule.xml +++ b/src/opnsense/mvc/app/controllers/OPNsense/Firewall/forms/dialogFilterRule.xml @@ -14,6 +14,7 @@ rowtoggle 5 center + false @@ -48,7 +49,8 @@ category 1 - 80 + 65 + false @@ -153,6 +155,7 @@ any 35 + 80 @@ -162,6 +165,7 @@ any 40 + 80 diff --git a/src/opnsense/mvc/app/views/OPNsense/Firewall/filter_rule.volt b/src/opnsense/mvc/app/views/OPNsense/Firewall/filter_rule.volt index 1bd072931f..f2bdc931ed 100644 --- a/src/opnsense/mvc/app/views/OPNsense/Firewall/filter_rule.volt +++ b/src/opnsense/mvc/app/views/OPNsense/Firewall/filter_rule.volt @@ -67,41 +67,113 @@ // read interface from URL hash once, for the first grid load let pendingUrlInterface = getUrlHash('interface') || null; + const ruleTypeMap = { + '0': { label: "{{ lang._('Automatically generated rules') }}", icon: "fa-magic", tooltip: "{{ lang._('Automatically generated rules') }}", color: "text-secondary" }, + '1': { label: "{{ lang._('Automatically generated rules') }}", icon: "fa-magic", tooltip: "{{ lang._('Automatically generated rules') }}", color: "text-secondary" }, + '2': { label: "{{ lang._('Floating rules') }}", icon: "fa-layer-group", tooltip: "{{ lang._('Floating rule') }}", color: "text-primary" }, + '3': { label: "{{ lang._('Group rules') }}", icon: "fa-sitemap", tooltip: "{{ lang._('Group rule') }}", color: "text-warning" }, + '4': { label: "{{ lang._('Interface rules') }}", icon: "fa-ethernet", tooltip: "{{ lang._('Interface rule') }}", color: "text-info" }, + '5': { label: "{{ lang._('Automatically generated rules') }}", icon: "fa-magic", tooltip: "{{ lang._('Automatically generated rules') }}", color: "text-secondary" }, + }; + + const getRuleTypeDigit = function(row) { + const sortOrder = row.sort_order ? row.sort_order.toString() : ""; + return sortOrder.charAt(0); + }; + + const getRuleType = function(row) { + return ruleTypeMap[getRuleTypeDigit(row)] || null; + }; + // Lives outside the grid, so the logic of the response handler can be changed after grid initialization - function dynamicResponseHandler(resp) { - // convert the flat rows into a tree view (if enabled) - if (!treeViewEnabled) { - return resp; - } + function dynamicResponseHandler(response) { + const getCategoryLabel = function(row) { + return row["%categories"] || row.categories || ""; + }; - const buckets = []; - let current = null; + const makeBucket = function(label, uuid, categoryColors) { + return { + // ensure uuid is as unique as possible for persistence handling + uuid : uuid, + isGroup : true, + _label : label, // internal + categories : label, + /* + * Bucket rows reuse the category formatter. + * For category buckets, this copies the first child's category metadata + * so the bucket can render the same category icon/color as its rules. + * For rule type buckets, a synthetic categoryColors entry is supplied. + */ + category_colors: categoryColors, + children : [] + }; + }; - resp.rows.forEach(r => { - // readable label used for grouping - const label = (r["%categories"] || r.categories || ""); + const createBucket = function(parent, label, uuid, categoryColors) { + let bucket = parent.children.find(child => child.isGroup && child._label === label); - // start a new bucket whenever the label changes - if (!current || current._label !== label) { - current = { - // ensure uuid is as unique as possible for persistence handling - uuid : `${String(r.uuid).replace(/-/g, '')}`, - isGroup : true, - _label : label, // internal - children : [] - }; - - // copy the category info from the first child to use as parent - current.categories = label; - current.category_colors = r.category_colors || []; - - buckets.push(current); + if (!bucket) { + bucket = makeBucket(label, uuid, categoryColors); + parent.children.push(bucket); } - current.children.push(r); + return bucket; + }; + + const root = { children: [] }; + + response.rows.forEach(row => { + const ruleType = getRuleType(row); + const ruleTypeDigit = getRuleTypeDigit(row) || "other"; + const ruleTypeLabel = ruleType.label || "{{ lang._('Other rules') }}"; + const categoryLabel = getCategoryLabel(row); + + /* + * The first tree level is the default view, and always based on the rule priority/type. + * + * Automatic rules + * rule + * Interface rules + * rule + */ + const ruleTypeBucket = createBucket( + root, + ruleTypeLabel, + `ruletype${ruleTypeDigit}`, + [{ name: ruleTypeLabel }] + ); + + if (treeViewEnabled && row.is_automatic !== true && categoryLabel !== "") { + /* + * When tree view is enabled, categorized non-automatic rules are grouped + * one level deeper by category below their rule priority/type bucket. + * + * Automatic rules and uncategorized rules stay directly below their rule + * priority/type bucket to avoid redundant or low-value nesting. + * + * Automatic rules + * rule + * Interface rules + * rule + * Web (Category) + * rule + * Mail (Category) + * rule + */ + const categoryBucket = createBucket( + ruleTypeBucket, + categoryLabel, + `ruletype${ruleTypeDigit}category${String(categoryLabel).replace(/[^a-z0-9]/gi, '')}`, + row.category_colors || [] + ); + + categoryBucket.children.push(row); + } else { + ruleTypeBucket.children.push(row); + } }); - return Object.assign({}, resp, { rows: buckets }); + return Object.assign({}, response, { rows: root.children }); } $('#download_rules').click(function(e){ @@ -150,6 +222,7 @@ options: { responsive: true, sorting: false, + rowCount: [500,20,50,100,200,1000,2000,-1], initialSearchPhrase: getUrlHash('search'), requestHandler: function(request){ // Add category selectpicker @@ -343,26 +416,39 @@ return '*'; } }, - // The category formatter is special as it renders differently for the bucket row + // Bucket rows reuse the category column because Tabulator only renders one + // formatter per cell, so both category buckets and rule type buckets are + // represented here. category: function (column, row) { const isGroup = row.isGroup; const hasCategories = row.categories && Array.isArray(row.category_colors); + // Rows without category metadata render nothing in this column. + // This also avoids creating a fake label for rules that + // are intentionally kept directly below their rule type bucket. if (!hasCategories) { - - return isGroup - ? ` - - {{ lang._('Uncategorized') }} - ${(row.children && row.children.length) || 0} - ` - : ''; + return ''; } const categories = row.category_colors || []; const icons = categories.map(cat => { + /* + * Top-level tree icons, e.g. automatic/floating/interface rules, are + * resolved here as well because each row can only use one formatter for + * this column. Rule type buckets provide a synthetic category entry + * whose name matches ruleTypeMap, while real category buckets continue + * to render normal category tag icons. + */ + const ruleType = Object.values(ruleTypeMap).find(type => type.label === cat.name); + + if (isGroup && ruleType) { + return ` + + + `; + } + const bgColor = cat.color ? ` style="color:${cat.color};"` : ''; return ` @@ -375,8 +461,6 @@ ? ` ${icons} ${categories.map(cat => cat.name).join(', ')} - ${(row.children && row.children.length) || 0} ` : icons; @@ -416,22 +500,11 @@ let result = ""; // Rule Type Icons (Determined by first digit of sort_order) - const ruleTypeIcons = { - '0': { icon: "fa-magic", tooltip: "{{ lang._('Automatic Rule') }}", color: "text-secondary" }, - '1': { icon: "fa-magic", tooltip: "{{ lang._('Automatic Rule') }}", color: "text-secondary" }, - '2': { icon: "fa-layer-group", tooltip: "{{ lang._('Floating Rule') }}", color: "text-primary" }, - '3': { icon: "fa-sitemap", tooltip: "{{ lang._('Group Rule') }}", color: "text-warning" }, - '4': { icon: "fa-ethernet", tooltip: "{{ lang._('Interface Rule') }}", color: "text-info" }, - '5': { icon: "fa-magic", tooltip: "{{ lang._('Automatic Rule') }}", color: "text-secondary" }, - }; + const ruleType = getRuleType(row); - const sortOrder = row.sort_order ? row.sort_order.toString() : ""; - if (sortOrder.length > 0) { - const typeDigit = sortOrder.charAt(0); - if (ruleTypeIcons[typeDigit]) { - result += ` `; - } + if (ruleType) { + result += ` `; } // Action @@ -733,7 +806,7 @@ label: row.name, id: row.used > 0 ? row.uuid : undefined, 'data-content': row.used > 0 - ? `${row.used} ${optVal}` + ? `${optVal} ${row.used}` : undefined }; }); @@ -752,37 +825,31 @@ // Populate interface selectpicker function populateInterfaceSelectpicker() { const currentSelection = $("#interface_select").val(); + return $('#interface_select').fetch_options( '/api/firewall/filter/get_interface_list', {}, function (data) { for (const groupKey in data) { const group = data[groupKey]; - group.items = group.items.map(item => { - const count = item.count ?? 0; - const label = (item.label || ''); - const subtext = group.label; + const icon = group.icon || ''; - const bgClassMap = { - floating: 'label-primary', - group: 'label-warning', - interface: 'label-info', - any: 'label-primary', - }; - const badgeClass = bgClassMap[item.type] || 'label-info'; + group.items = group.items.map(item => { + const label = item.label || ''; return { value: item.value, label: label, 'data-content': ` - ${count > 0 ? `${count}` : ''} + ${icon ? `` : ''} ${label} `.trim() }; }); } + return data; }, false, @@ -801,7 +868,7 @@ interfaceInitialized = true; pendingUrlInterface = null; // consume the hash so it is not used again }, - true // render_html to show counts as badges + true // render_html to show icons ); } @@ -840,13 +907,12 @@ localStorage.setItem("firewall_rule_tree", treeViewEnabled ? "1" : "0"); $(this).toggleClass('active btn-primary', treeViewEnabled); $("#{{formGridFilterRule['table_id']}}").toggleClass("tree-enabled", treeViewEnabled); - $("#tree_expand_container").toggle(treeViewEnabled); grid.bootgrid("reload"); }); - // Visible only when tree view is enabled $("#tree_expand_container").detach().insertAfter("#tree_toggle_container"); - $("#tree_expand_container").toggle(treeViewEnabled); + $("#tree_expand_container").show(); + $('#expand_tree_button').on('click', function () { const $table = $('#{{ formGridFilterRule["table_id"] }}'); @@ -1116,10 +1182,8 @@ class="btn btn-default" data-toggle="tooltip" data-placement="bottom" - data-delay='{"show": 1000}' - title="{{ lang._('Show all rules and statistics') }}"> + title="{{ lang._('Show rule statistics') }}"> - {{ lang._('Inspect') }} @@ -1129,10 +1193,8 @@ class="btn btn-default" data-toggle="tooltip" data-placement="bottom" - data-delay='{"show": 1000}' - title="{{ lang._('Show all categories in a tree') }}"> - - {{ lang._('Tree') }} + title="{{ lang._('Show categories as folders') }}"> +
@@ -1141,7 +1203,6 @@ class="btn btn-default" data-toggle="tooltip" data-placement="bottom" - data-delay='{"show": 1000}' title="{{ lang._('Expand/Collapse all') }}">