]> git.proxmox.com Git - pve-manager.git/blobdiff - www/manager6/grid/FirewallRules.js
ui: firewall: refactor privilege checks and prevent double click
[pve-manager.git] / www / manager6 / grid / FirewallRules.js
index 1a6b2002e72bb4501de45c589542ed7966bdb449..18075eaa60ae53e607c740030d68f1f13e510ed8 100644 (file)
@@ -11,39 +11,140 @@ Ext.define('PVE.form.FWMacroSelector', {
                header: gettext('Macro'),
                dataIndex: 'macro',
                hideable: false,
-               width: 100
+               width: 100,
            },
            {
                header: gettext('Description'),
                renderer: Ext.String.htmlEncode,
                flex: 1,
-               dataIndex: 'descr'
-           }
-       ]
+               dataIndex: 'descr',
+           },
+       ],
     },
     initComponent: function() {
        var me = this;
 
        var store = Ext.create('Ext.data.Store', {
            autoLoad: true,
-           fields: [ 'macro', 'descr' ],
+           fields: ['macro', 'descr'],
            idProperty: 'macro',
            proxy: {
                type: 'proxmox',
-               url: "/api2/json/cluster/firewall/macros"
+               url: "/api2/json/cluster/firewall/macros",
            },
            sorters: {
                property: 'macro',
-               order: 'DESC'
-           }
+               direction: 'ASC',
+           },
        });
 
        Ext.apply(me, {
-           store: store
+           store: store,
        });
 
        me.callParent();
-    }
+    },
+});
+
+Ext.define('PVE.form.ICMPTypeSelector', {
+    extend: 'Proxmox.form.ComboGrid',
+    alias: 'widget.pveICMPTypeSelector',
+    allowBlank: true,
+    autoSelect: false,
+    valueField: 'name',
+    displayField: 'name',
+    listConfig: {
+       columns: [
+           {
+               header: gettext('Type'),
+               dataIndex: 'type',
+               hideable: false,
+               sortable: false,
+               width: 50,
+           },
+           {
+               header: gettext('Name'),
+               dataIndex: 'name',
+               hideable: false,
+               sortable: false,
+               flex: 1,
+           },
+       ],
+    },
+    setName: function(value) {
+       this.name = value;
+    },
+});
+
+let ICMP_TYPE_NAMES_STORE = Ext.create('Ext.data.Store', {
+    field: ['type', 'name'],
+    data: [
+       { type: 'any', name: 'any' },
+       { type: '0', name: 'echo-reply' },
+       { type: '3', name: 'destination-unreachable' },
+       { type: '3/0', name: 'network-unreachable' },
+       { type: '3/1', name: 'host-unreachable' },
+       { type: '3/2', name: 'protocol-unreachable' },
+       { type: '3/3', name: 'port-unreachable' },
+       { type: '3/4', name: 'fragmentation-needed' },
+       { type: '3/5', name: 'source-route-failed' },
+       { type: '3/6', name: 'network-unknown' },
+       { type: '3/7', name: 'host-unknown' },
+       { type: '3/9', name: 'network-prohibited' },
+       { type: '3/10', name: 'host-prohibited' },
+       { type: '3/11', name: 'TOS-network-unreachable' },
+       { type: '3/12', name: 'TOS-host-unreachable' },
+       { type: '3/13', name: 'communication-prohibited' },
+       { type: '3/14', name: 'host-precedence-violation' },
+       { type: '3/15', name: 'precedence-cutoff' },
+       { type: '4', name: 'source-quench' },
+       { type: '5', name: 'redirect' },
+       { type: '5/0', name: 'network-redirect' },
+       { type: '5/1', name: 'host-redirect' },
+       { type: '5/2', name: 'TOS-network-redirect' },
+       { type: '5/3', name: 'TOS-host-redirect' },
+       { type: '8', name: 'echo-request' },
+       { type: '9', name: 'router-advertisement' },
+       { type: '10', name: 'router-solicitation' },
+       { type: '11', name: 'time-exceeded' },
+       { type: '11/0', name: 'ttl-zero-during-transit' },
+       { type: '11/1', name: 'ttl-zero-during-reassembly' },
+       { type: '12', name: 'parameter-problem' },
+       { type: '12/0', name: 'ip-header-bad' },
+       { type: '12/1', name: 'required-option-missing' },
+       { type: '13', name: 'timestamp-request' },
+       { type: '14', name: 'timestamp-reply' },
+       { type: '17', name: 'address-mask-request' },
+       { type: '18', name: 'address-mask-reply' },
+    ],
+});
+let ICMPV6_TYPE_NAMES_STORE = Ext.create('Ext.data.Store', {
+    field: ['type', 'name'],
+    data: [
+       { type: '1', name: 'destination-unreachable' },
+       { type: '1/0', name: 'no-route' },
+       { type: '1/1', name: 'communication-prohibited' },
+       { type: '1/2', name: 'beyond-scope' },
+       { type: '1/3', name: 'address-unreachable' },
+       { type: '1/4', name: 'port-unreachable' },
+       { type: '1/5', name: 'failed-policy' },
+       { type: '1/6', name: 'reject-route' },
+       { type: '2', name: 'packet-too-big' },
+       { type: '3', name: 'time-exceeded' },
+       { type: '3/0', name: 'ttl-zero-during-transit' },
+       { type: '3/1', name: 'ttl-zero-during-reassembly' },
+       { type: '4', name: 'parameter-problem' },
+       { type: '4/0', name: 'bad-header' },
+       { type: '4/1', name: 'unknown-header-type' },
+       { type: '4/2', name: 'unknown-option' },
+       { type: '128', name: 'echo-request' },
+       { type: '129', name: 'echo-reply' },
+       { type: '133', name: 'router-solicitation' },
+       { type: '134', name: 'router-advertisement' },
+       { type: '135', name: 'neighbour-solicitation' },
+       { type: '136', name: 'neighbour-advertisement' },
+       { type: '137', name: 'redirect' },
+    ],
 });
 
 Ext.define('PVE.FirewallRulePanel', {
@@ -59,18 +160,18 @@ Ext.define('PVE.FirewallRulePanel', {
        // hack: editable ComboGrid returns nothing when empty, so we need to set ''
        // Also, disabled text fields return nothing, so we need to set ''
 
-       Ext.Array.each(['source', 'dest', 'macro', 'proto', 'sport', 'dport', 'log'], function(key) {
+       Ext.Array.each(['source', 'dest', 'macro', 'proto', 'sport', 'dport', 'icmp-type', 'log'], function(key) {
            if (values[key] === undefined) {
                values[key] = '';
            }
        });
 
        delete values.modified_marker;
+
        return values;
     },
 
-    initComponent : function() {
+    initComponent: function() {
        var me = this;
 
        if (!me.list_refs_url) {
@@ -80,11 +181,11 @@ Ext.define('PVE.FirewallRulePanel', {
        me.column1 = [
            {
                // hack: we use this field to mark the form 'dirty' when the
-               // record has errors- so that the user can safe the unmodified 
+               // record has errors- so that the user can safe the unmodified
                // form again.
                xtype: 'hiddenfield',
                name: 'modified_marker',
-               value: ''
+               value: '',
            },
            {
                xtype: 'proxmoxKVComboBox',
@@ -92,7 +193,7 @@ Ext.define('PVE.FirewallRulePanel', {
                value: 'in',
                comboItems: [['in', 'in'], ['out', 'out']],
                fieldLabel: gettext('Direction'),
-               allowBlank: false
+               allowBlank: false,
            },
            {
                xtype: 'proxmoxKVComboBox',
@@ -100,8 +201,8 @@ Ext.define('PVE.FirewallRulePanel', {
                value: 'ACCEPT',
                comboItems: [['ACCEPT', 'ACCEPT'], ['DROP', 'DROP'], ['REJECT', 'REJECT']],
                fieldLabel: gettext('Action'),
-               allowBlank: false
-           }
+               allowBlank: false,
+           },
         ];
 
        if (me.allow_iface) {
@@ -110,13 +211,13 @@ Ext.define('PVE.FirewallRulePanel', {
                name: 'iface',
                deleteEmpty: !me.isCreate,
                value: '',
-               fieldLabel: gettext('Interface')
+               fieldLabel: gettext('Interface'),
            });
        } else {
            me.column1.push({
                xtype: 'displayfield',
                fieldLabel: '',
-               value: ''
+               value: '',
            });
        }
 
@@ -125,7 +226,7 @@ Ext.define('PVE.FirewallRulePanel', {
                xtype: 'displayfield',
                fieldLabel: '',
                height: 7,
-               value: ''
+               value: '',
            },
            {
                xtype: 'pveIPRefSelector',
@@ -134,8 +235,9 @@ Ext.define('PVE.FirewallRulePanel', {
                editable: true,
                base_url: me.list_refs_url,
                value: '',
-               fieldLabel: gettext('Source')
-
+               fieldLabel: gettext('Source'),
+               maxLength: 512,
+               maxLengthText: gettext('Too long, consider using IP sets.'),
            },
            {
                xtype: 'pveIPRefSelector',
@@ -144,18 +246,20 @@ Ext.define('PVE.FirewallRulePanel', {
                editable: true,
                base_url: me.list_refs_url,
                value: '',
-               fieldLabel: gettext('Destination')
-           }
+               fieldLabel: gettext('Destination'),
+               maxLength: 512,
+               maxLengthText: gettext('Too long, consider using IP sets.'),
+           },
        );
 
-       
+
        me.column2 = [
            {
                xtype: 'proxmoxcheckbox',
                name: 'enable',
                checked: false,
                uncheckedValue: 0,
-               fieldLabel: gettext('Enable')
+               fieldLabel: gettext('Enable'),
            },
            {
                xtype: 'pveFWMacroSelector',
@@ -177,8 +281,8 @@ Ext.define('PVE.FirewallRulePanel', {
                            me.down('field[name=dport]').setDisabled(true);
                            me.down('field[name=dport]').setValue('');
                        }
-                    }
-                }
+                    },
+                },
            },
            {
                xtype: 'pveIPProtocolSelector',
@@ -186,31 +290,82 @@ Ext.define('PVE.FirewallRulePanel', {
                autoSelect: false,
                editable: true,
                value: '',
-               fieldLabel: gettext('Protocol')
+               fieldLabel: gettext('Protocol'),
+               listeners: {
+                   change: function(f, value) {
+                       if (value === 'icmp' || value === 'icmpv6' || value === 'ipv6-icmp') {
+                           me.down('field[name=dport]').setHidden(true);
+                           me.down('field[name=dport]').setDisabled(true);
+                           if (value === 'icmp') {
+                               me.down('#icmpv4-type').setHidden(false);
+                               me.down('#icmpv4-type').setDisabled(false);
+                               me.down('#icmpv6-type').setHidden(true);
+                               me.down('#icmpv6-type').setDisabled(true);
+                           } else {
+                               me.down('#icmpv6-type').setHidden(false);
+                               me.down('#icmpv6-type').setDisabled(false);
+                               me.down('#icmpv4-type').setHidden(true);
+                               me.down('#icmpv4-type').setDisabled(true);
+                           }
+                       } else {
+                           me.down('#icmpv4-type').setHidden(true);
+                           me.down('#icmpv4-type').setDisabled(true);
+                           me.down('#icmpv6-type').setHidden(true);
+                           me.down('#icmpv6-type').setDisabled(true);
+                           me.down('field[name=dport]').setHidden(false);
+                           me.down('field[name=dport]').setDisabled(false);
+                       }
+                   },
+               },
            },
            {
                xtype: 'displayfield',
                fieldLabel: '',
                height: 7,
-               value: ''
+               value: '',
            },
            {
                xtype: 'textfield',
                name: 'sport',
                value: '',
-               fieldLabel: gettext('Source port')
+               fieldLabel: gettext('Source port'),
            },
            {
                xtype: 'textfield',
                name: 'dport',
                value: '',
-               fieldLabel: gettext('Dest. port')
+               fieldLabel: gettext('Dest. port'),
+           },
+           {
+               xtype: 'pveICMPTypeSelector',
+               name: 'icmp-type',
+               id: 'icmpv4-type',
+               autoSelect: false,
+               editable: true,
+               hidden: true,
+               disabled: true,
+               value: '',
+               fieldLabel: gettext('ICMP type'),
+               store: ICMP_TYPE_NAMES_STORE,
            },
+           {
+               xtype: 'pveICMPTypeSelector',
+               name: 'icmp-type',
+               id: 'icmpv6-type',
+               autoSelect: false,
+               editable: true,
+               hidden: true,
+               disabled: true,
+               value: '',
+               fieldLabel: gettext('ICMP type'),
+               store: ICMPV6_TYPE_NAMES_STORE,
+           },
+       ];
+
+       me.advancedColumn1 = [
            {
                xtype: 'pveFirewallLogLevels',
-               name: 'log',
-               fieldLabel: gettext('Log level')
-           }
+           },
        ];
 
        me.columnB = [
@@ -218,12 +373,12 @@ Ext.define('PVE.FirewallRulePanel', {
                xtype: 'textfield',
                name: 'comment',
                value: '',
-               fieldLabel: gettext('Comment')
-           }
+               fieldLabel: gettext('Comment'),
+           },
        ];
 
        me.callParent();
-    }
+    },
 });
 
 Ext.define('PVE.FirewallRuleEdit', {
@@ -234,8 +389,7 @@ Ext.define('PVE.FirewallRuleEdit', {
 
     allow_iface: false,
 
-    initComponent : function() {
-
+    initComponent: function() {
        var me = this;
 
        if (!me.base_url) {
@@ -245,7 +399,7 @@ Ext.define('PVE.FirewallRuleEdit', {
            throw "no list_refs_url specified";
        }
 
-       me.isCreate = (me.rule_pos === undefined);
+       me.isCreate = me.rule_pos === undefined;
 
        if (me.isCreate) {
             me.url = '/api2/extjs' + me.base_url;
@@ -259,13 +413,13 @@ Ext.define('PVE.FirewallRuleEdit', {
            isCreate: me.isCreate,
            list_refs_url: me.list_refs_url,
            allow_iface: me.allow_iface,
-           rule_pos: me.rule_pos
+           rule_pos: me.rule_pos,
        });
 
        Ext.apply(me, {
             subject: gettext('Rule'),
            isAdd: true,
-           items: [ ipanel ]
+           items: [ipanel],
        });
 
        me.callParent();
@@ -275,6 +429,10 @@ Ext.define('PVE.FirewallRuleEdit', {
                success: function(response, options) {
                    var values = response.result.data;
                    ipanel.setValues(values);
+                   // set icmp-type again after protocol has been set
+                   if (values["icmp-type"] !== undefined) {
+                       ipanel.setValues({ "icmp-type": values["icmp-type"] });
+                   }
                    if (values.errors) {
                        var field = me.query('[isFormField][name=modified_marker]')[0];
                        field.setValue(1);
@@ -283,12 +441,12 @@ Ext.define('PVE.FirewallRuleEdit', {
                            form.markInvalid(values.errors);
                        }, 100);
                    }
-               }
+               },
            });
        } else if (me.rec) {
            ipanel.setValues(me.rec.data);
        }
-    }
+    },
 });
 
 Ext.define('PVE.FirewallGroupRuleEdit', {
@@ -298,11 +456,10 @@ Ext.define('PVE.FirewallGroupRuleEdit', {
 
     allow_iface: false,
 
-    initComponent : function() {
-
+    initComponent: function() {
        var me = this;
 
-       me.isCreate = (me.rule_pos === undefined);
+       me.isCreate = me.rule_pos === undefined;
 
        if (me.isCreate) {
             me.url = '/api2/extjs' + me.base_url;
@@ -316,15 +473,15 @@ Ext.define('PVE.FirewallGroupRuleEdit', {
            {
                xtype: 'hiddenfield',
                name: 'type',
-               value: 'group'
+               value: 'group',
            },
            {
                xtype: 'pveSecurityGroupsSelector',
                name: 'action',
                value: '',
                fieldLabel: gettext('Security Group'),
-               allowBlank: false
-           }
+               allowBlank: false,
+           },
        ];
 
        if (me.allow_iface) {
@@ -333,7 +490,7 @@ Ext.define('PVE.FirewallGroupRuleEdit', {
                name: 'iface',
                deleteEmpty: !me.isCreate,
                value: '',
-               fieldLabel: gettext('Interface')
+               fieldLabel: gettext('Interface'),
            });
        }
 
@@ -346,36 +503,36 @@ Ext.define('PVE.FirewallGroupRuleEdit', {
                    name: 'enable',
                    checked: false,
                    uncheckedValue: 0,
-                   fieldLabel: gettext('Enable')
-               }
+                   fieldLabel: gettext('Enable'),
+               },
            ],
            columnB: [
                {
                    xtype: 'textfield',
                    name: 'comment',
                    value: '',
-                   fieldLabel: gettext('Comment')
-               }
-           ]
+                   fieldLabel: gettext('Comment'),
+               },
+           ],
        });
 
        Ext.apply(me, {
             subject: gettext('Rule'),
            isAdd: true,
-           items: [ ipanel ]
+           items: [ipanel],
        });
 
        me.callParent();
 
        if (!me.isCreate) {
            me.load({
-               success:  function(response, options) {
+               success: function(response, options) {
                    var values = response.result.data;
                    ipanel.setValues(values);
-               }
+               },
            });
        }
-    }
+    },
 });
 
 Ext.define('PVE.FirewallRules', {
@@ -412,14 +569,17 @@ Ext.define('PVE.FirewallRules', {
            }
            me.store.removeAll();
        } else {
-           me.addBtn.setDisabled(false);
-           me.removeBtn.baseurl = url + '/';
-           if (me.groupBtn) {
-               me.groupBtn.setDisabled(false);
+           if (me.canEdit) {
+               me.addBtn.setDisabled(false);
+               if (me.groupBtn) {
+                   me.groupBtn.setDisabled(false);
+               }
            }
+           me.removeBtn.baseurl = url + '/';
+
            me.store.setProxy({
                type: 'proxmox',
-               url: '/api2/json' + url
+               url: '/api2/json' + url,
            });
 
            me.store.load();
@@ -429,7 +589,7 @@ Ext.define('PVE.FirewallRules', {
     moveRule: function(from, to) {
         var me = this;
 
-       if (!me.base_url) { 
+       if (!me.base_url) {
            return;
        }
 
@@ -443,14 +603,14 @@ Ext.define('PVE.FirewallRules', {
            },
            callback: function() {
                me.store.load();
-           }
+           },
        });
     },
 
     updateRule: function(rule) {
         var me = this;
 
-       if (!me.base_url) { 
+       if (!me.base_url) {
            return;
        }
 
@@ -470,21 +630,20 @@ Ext.define('PVE.FirewallRules', {
            },
            callback: function() {
                me.store.load();
-           }
+           },
        });
     },
 
 
     initComponent: function() {
-       /*jslint confusion: true */
         var me = this;
 
        if (!me.list_refs_url) {
            throw "no list_refs_url specified";
        }
 
-       var store = Ext.create('Ext.data.Store',{
-           model: 'pve-fw-rule'
+       var store = Ext.create('Ext.data.Store', {
+           model: 'pve-fw-rule',
        });
 
        var reload = function() {
@@ -493,9 +652,12 @@ Ext.define('PVE.FirewallRules', {
 
        var sm = Ext.create('Ext.selection.RowModel', {});
 
+       me.caps = Ext.state.Manager.get('GuiCap');
+       me.canEdit = !!me.caps.vms['VM.Config.Network'] || !!me.caps.dc['Sys.Modify'] || !!me.caps.nodes['Sys.Modify'];
+
        var run_editor = function() {
            var rec = sm.getSelection()[0];
-           if (!rec) {
+           if (!rec || !me.canEdit) {
                return;
            }
            var type = rec.data.type;
@@ -514,85 +676,81 @@ Ext.define('PVE.FirewallRules', {
                allow_iface: me.allow_iface,
                base_url: me.base_url,
                list_refs_url: me.list_refs_url,
-               rule_pos: rec.data.pos
+               rule_pos: rec.data.pos,
            });
 
            win.show();
            win.on('destroy', reload);
        };
 
-       me.editBtn = Ext.create('Proxmox.button.Button',{
+       me.editBtn = Ext.create('Proxmox.button.Button', {
            text: gettext('Edit'),
            disabled: true,
+           enableFn: rec => me.canEdit,
            selModel: sm,
-           handler: run_editor
+           handler: run_editor,
        });
 
-       me.addBtn =  Ext.create('Ext.Button', {
+       me.addBtn = Ext.create('Ext.Button', {
            text: gettext('Add'),
            disabled: true,
            handler: function() {
                var win = Ext.create('PVE.FirewallRuleEdit', {
                    allow_iface: me.allow_iface,
                    base_url: me.base_url,
-                   list_refs_url: me.list_refs_url
+                   list_refs_url: me.list_refs_url,
                });
                win.on('destroy', reload);
                win.show();
-           }
+           },
        });
 
        var run_copy_editor = function() {
-           var rec = sm.getSelection()[0];
-
+           let rec = sm.getSelection()[0];
            if (!rec) {
                return;
            }
-           var type = rec.data.type;
-
-
+           let type = rec.data.type;
            if (!(type === 'in' || type === 'out')) {
                return;
            }
 
-           var win = Ext.create('PVE.FirewallRuleEdit', {
+           let win = Ext.create('PVE.FirewallRuleEdit', {
                allow_iface: me.allow_iface,
                base_url: me.base_url,
                list_refs_url: me.list_refs_url,
-               rec: rec
+               rec: rec,
            });
-
            win.show();
            win.on('destroy', reload);
        };
 
-       me.copyBtn = Ext.create('Proxmox.button.Button',{
+       me.copyBtn = Ext.create('Proxmox.button.Button', {
            text: gettext('Copy'),
            selModel: sm,
-           enableFn: function(rec) {
-               return (rec.data.type === 'in' || rec.data.type === 'out');
-           },
+           enableFn: ({ data }) => (data.type === 'in' || data.type === 'out') && me.canEdit,
            disabled: true,
-           handler: run_copy_editor
+           handler: run_copy_editor,
        });
 
        if (me.allow_groups) {
-           me.groupBtn =  Ext.create('Ext.Button', {
-               text: gettext('Insert') + ': ' + 
+           me.groupBtn = Ext.create('Ext.Button', {
+               text: gettext('Insert') + ': ' +
                    gettext('Security Group'),
                disabled: true,
                handler: function() {
                    var win = Ext.create('PVE.FirewallGroupRuleEdit', {
                        allow_iface: me.allow_iface,
-                       base_url: me.base_url
+                       base_url: me.base_url,
                    });
                    win.on('destroy', reload);
                    win.show();
-               }
+               },
            });
        }
 
-       me.removeBtn = Ext.create('Proxmox.button.StdRemoveButton',{
+       me.removeBtn = Ext.create('Proxmox.button.StdRemoveButton', {
+           enableFn: rec => me.canEdit,
            selModel: sm,
            baseurl: me.base_url + '/',
            confirmMsg: false,
@@ -603,48 +761,49 @@ Ext.define('PVE.FirewallRules', {
            },
            callback: function() {
                me.store.load();
-           }
+           },
        });
 
-       var tbar = me.tbar_prefix ? [ me.tbar_prefix ] : [];
+       let tbar = me.tbar_prefix ? [me.tbar_prefix] : [];
        tbar.push(me.addBtn, me.copyBtn);
        if (me.groupBtn) {
            tbar.push(me.groupBtn);
        }
        tbar.push(me.removeBtn, me.editBtn);
 
-       var render_errors = function(name, value, metaData, record) {
-           var errors = record.data.errors;
+       let render_errors = function(name, value, metaData, record) {
+           let errors = record.data.errors;
            if (errors && errors[name]) {
                metaData.tdCls = 'proxmox-invalid-row';
-               var html = '<p>' +  Ext.htmlEncode(errors[name]) + '</p>';
-               metaData.tdAttr = 'data-qwidth=600 data-qtitle="ERROR" data-qtip="' + 
-                   html.replace(/\"/g,'&quot;') + '"';
+               let html = '<p>' + Ext.htmlEncode(errors[name]) + '</p>';
+               metaData.tdAttr = 'data-qwidth=600 data-qtitle="ERROR" data-qtip="' + html + '"';
            }
            return value;
        };
 
-       var columns = [
+       let columns = [
            {
                // similar to xtype: 'rownumberer',
                dataIndex: 'pos',
                resizable: false,
-               width: 23,
+               minWidth: 65,
+               maxWidth: 83,
+               flex: 1,
                sortable: false,
-               align: 'right',
                hideable: false,
                menuDisabled: true,
-               renderer: function(value, metaData, record, rowIdx, colIdx, store) {
+               renderer: function(value, metaData, record, rowIdx, colIdx) {
                    metaData.tdCls = Ext.baseCSSPrefix + 'grid-cell-special';
+                   let dragHandle = "<i class='pve-grid-fa fa fa-fw fa-reorder cursor-move'></i>";
                    if (value >= 0) {
-                       return value;
+                       return dragHandle + value;
                    }
-                   return '';
-               }
+                   return dragHandle;
+               },
            },
            {
                xtype: 'checkcolumn',
-               header: gettext('Enable'),
+               header: gettext('On'),
                dataIndex: 'enable',
                listeners: {
                    checkchange: function(column, recordIndex, checked) {
@@ -658,9 +817,9 @@ Ext.define('PVE.FirewallRules', {
                            delete data.iface;
                        }
                        me.updateRule(data);
-                   }
+                   },
                },
-               width: 50
+               width: 40,
            },
            {
                header: gettext('Type'),
@@ -668,7 +827,9 @@ Ext.define('PVE.FirewallRules', {
                renderer: function(value, metaData, record) {
                    return render_errors('type', value, metaData, record);
                },
-               width: 50
+               minWidth: 60,
+               maxWidth: 80,
+               flex: 2,
            },
            {
                header: gettext('Action'),
@@ -676,7 +837,9 @@ Ext.define('PVE.FirewallRules', {
                renderer: function(value, metaData, record) {
                    return render_errors('action', value, metaData, record);
                },
-               width: 80
+               minWidth: 80,
+               maxWidth: 200,
+               flex: 2,
            },
            {
                header: gettext('Macro'),
@@ -684,8 +847,9 @@ Ext.define('PVE.FirewallRules', {
                renderer: function(value, metaData, record) {
                    return render_errors('macro', value, metaData, record);
                },
-               width: 80
-           }
+               minWidth: 80,
+               flex: 2,
+           },
        ];
 
        if (me.allow_iface) {
@@ -695,50 +859,53 @@ Ext.define('PVE.FirewallRules', {
                renderer: function(value, metaData, record) {
                    return render_errors('iface', value, metaData, record);
                },
-               width: 80
+               minWidth: 80,
+               flex: 2,
            });
        }
 
        columns.push(
+           {
+               header: gettext('Protocol'),
+               dataIndex: 'proto',
+               renderer: function(value, metaData, record) {
+                   return render_errors('proto', value, metaData, record);
+               },
+               width: 75,
+           },
            {
                header: gettext('Source'),
                dataIndex: 'source',
                renderer: function(value, metaData, record) {
                    return render_errors('source', value, metaData, record);
                },
-               width: 100
+               minWidth: 100,
+               flex: 2,
            },
            {
-               header: gettext('Destination'),
-               dataIndex: 'dest',
+               header: gettext('S.Port'),
+               dataIndex: 'sport',
                renderer: function(value, metaData, record) {
-                   return render_errors('dest', value, metaData, record);
+                   return render_errors('sport', value, metaData, record);
                },
-               width: 100
+               width: 75,
            },
            {
-               header: gettext('Protocol'),
-               dataIndex: 'proto',
+               header: gettext('Destination'),
+               dataIndex: 'dest',
                renderer: function(value, metaData, record) {
-                   return render_errors('proto', value, metaData, record);
+                   return render_errors('dest', value, metaData, record);
                },
-               width: 100
+               minWidth: 100,
+               flex: 2,
            },
            {
-               header: gettext('Dest. port'),
+               header: gettext('D.Port'),
                dataIndex: 'dport',
                renderer: function(value, metaData, record) {
                    return render_errors('dport', value, metaData, record);
                },
-               width: 100
-           },
-           {
-               header: gettext('Source port'),
-               dataIndex: 'sport',
-               renderer: function(value, metaData, record) {
-                   return render_errors('sport', value, metaData, record);
-               },
-               width: 100
+               width: 75,
            },
            {
                header: gettext('Log level'),
@@ -746,48 +913,53 @@ Ext.define('PVE.FirewallRules', {
                renderer: function(value, metaData, record) {
                    return render_errors('log', value, metaData, record);
                },
-               width: 100
+               width: 100,
            },
            {
                header: gettext('Comment'),
                dataIndex: 'comment',
-               flex: 1,
+               flex: 10,
+               minWidth: 75,
                renderer: function(value, metaData, record) {
-                   return render_errors('comment', Ext.util.Format.htmlEncode(value), metaData, record);
-               }
-           }
+                   let comment = render_errors('comment', Ext.util.Format.htmlEncode(value), metaData, record) || '';
+                   if (comment.length * 12 > metaData.column.cellWidth) {
+                       comment = `<span data-qtip="${comment}">${comment}</span>`;
+                   }
+                   return comment;
+               },
+           },
        );
 
        Ext.apply(me, {
            store: store,
            selModel: sm,
            tbar: tbar,
-            viewConfig: {
+           viewConfig: {
                plugins: [
                    {
                        ptype: 'gridviewdragdrop',
                        dragGroup: 'FWRuleDDGroup',
-                       dropGroup: 'FWRuleDDGroup'
-                   }
+                       dropGroup: 'FWRuleDDGroup',
+                   },
                ],
                listeners: {
-                    beforedrop: function(node, data, dropRec, dropPosition) {
+                   beforedrop: function(node, data, dropRec, dropPosition) {
                        if (!dropRec) {
                            return false; // empty view
                        }
-                       var moveto = dropRec.get('pos');
+                       let moveto = dropRec.get('pos');
                        if (dropPosition === 'after') {
                            moveto++;
                        }
-                       var pos = data.records[0].get('pos');
+                       let pos = data.records[0].get('pos');
                        me.moveRule(pos, moveto);
                        return 0;
                     },
-                   itemdblclick: run_editor
-               }
+                   itemdblclick: run_editor,
+               },
            },
            sortableColumns: false,
-           columns: columns
+           columns: columns,
        });
 
        me.callParent();
@@ -795,15 +967,26 @@ Ext.define('PVE.FirewallRules', {
        if (me.base_url) {
            me.setBaseUrl(me.base_url); // load
        }
-    }
+    },
 }, function() {
-
     Ext.define('pve-fw-rule', {
        extend: 'Ext.data.Model',
-       fields: [ { name: 'enable', type: 'boolean' },
-                 'type', 'action', 'macro', 'source', 'dest', 'proto', 'iface',
-                 'dport', 'sport', 'comment', 'pos', 'digest', 'errors' ],
-       idProperty: 'pos'
+       fields: [
+           { name: 'enable', type: 'boolean' },
+           'type',
+           'action',
+           'macro',
+           'source',
+           'dest',
+           'proto',
+           'iface',
+           'dport',
+           'sport',
+           'comment',
+           'pos',
+           'digest',
+           'errors',
+       ],
+       idProperty: 'pos',
     });
-
 });