From 13b40dbdb91f18bd87c939e364fcedfe450a7514 Mon Sep 17 00:00:00 2001 From: BornToBeRoot <16019165+BornToBeRoot@users.noreply.github.com> Date: Mon, 19 Oct 2026 17:49:48 +0200 Subject: [PATCH 1/2] Feature: Delete / Enable / Disable multiple firewall rules --- .../Resources/Strings.Designer.cs | 29 ++++ .../Resources/Strings.resx | 11 ++ .../Firewall/Firewall.cs | 96 ++++++++--- .../ViewModels/FirewallViewModel.cs | 156 +++++++++++++++--- Source/NETworkManager/Views/FirewallView.xaml | 39 +++++ .../NETworkManager/Views/FirewallView.xaml.cs | 2 + Website/docs/application/firewall.md | 24 ++- Website/docs/changelog/next-release.md | 1 + 8 files changed, 296 insertions(+), 62 deletions(-) diff --git a/Source/NETworkManager.Localization/Resources/Strings.Designer.cs b/Source/NETworkManager.Localization/Resources/Strings.Designer.cs index de4abb87d1..80a1962455 100644 --- a/Source/NETworkManager.Localization/Resources/Strings.Designer.cs +++ b/Source/NETworkManager.Localization/Resources/Strings.Designer.cs @@ -2981,6 +2981,17 @@ public static string DeleteFirewallRuleMessage { } } + /// + /// Looks up a localized string similar to The selected firewall rules are permanently deleted: + /// + ///{0}. + /// + public static string DeleteFirewallRulesMessage { + get { + return ResourceManager.GetString("DeleteFirewallRulesMessage", resourceCulture); + } + } + /// /// Looks up a localized string similar to Delete group. /// @@ -3120,6 +3131,24 @@ public static string DeleteRule { } } + /// + /// Looks up a localized string similar to Delete rules. + /// + public static string DeleteRules { + get { + return ResourceManager.GetString("DeleteRules", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Delete rules ({0}). + /// + public static string DeleteRulesX { + get { + return ResourceManager.GetString("DeleteRulesX", resourceCulture); + } + } + /// /// Looks up a localized string similar to Delete SNTP server. /// diff --git a/Source/NETworkManager.Localization/Resources/Strings.resx b/Source/NETworkManager.Localization/Resources/Strings.resx index b9a01cc5ef..c5c09cdc54 100644 --- a/Source/NETworkManager.Localization/Resources/Strings.resx +++ b/Source/NETworkManager.Localization/Resources/Strings.resx @@ -4729,4 +4729,15 @@ Cloudflare may log your IP address and network information. See Cloudflare's pri Copy rule + + The selected firewall rules are permanently deleted: + +{0} + + + Delete rules + + + Delete rules ({0}) + \ No newline at end of file diff --git a/Source/NETworkManager.Models/Firewall/Firewall.cs b/Source/NETworkManager.Models/Firewall/Firewall.cs index 915692d24c..3e9f09a2da 100644 --- a/Source/NETworkManager.Models/Firewall/Firewall.cs +++ b/Source/NETworkManager.Models/Firewall/Firewall.cs @@ -168,9 +168,7 @@ public static async Task> GetRulesAsync() } /// - /// Enables or disables the given by running - /// Enable-NetFirewallRule or Disable-NetFirewallRule against - /// the rule's internal . + /// Enables or disables the given . /// /// /// The firewall rule to modify. @@ -179,43 +177,83 @@ public static async Task> GetRulesAsync() /// to enable the rule; to disable it. /// /// - /// Thrown when the PowerShell pipeline reports one or more errors. + /// Thrown when the rule no longer exists or the PowerShell pipeline reports one or more errors. /// - public static async Task SetRuleEnabledAsync(FirewallRule rule, bool enabled) + public static Task SetRuleEnabledAsync(FirewallRule rule, bool enabled) { - await RunspaceLock.WaitAsync(); - try - { - await Task.Run(() => - { - using var ps = SMA.PowerShell.Create(); - ps.Runspace = SharedRunspace; + return SetRulesEnabledAsync([rule], enabled); + } - ps.AddScript($@"{(enabled ? "Enable" : "Disable")}-NetFirewallRule -Name '{rule.Id}'"); - ps.Invoke(); + /// + /// Enables or disables the given by running + /// Enable-NetFirewallRule or Disable-NetFirewallRule against the rules' + /// internal . Rules that are already in the target state are skipped. + /// Nothing is changed if one of the rules no longer exists. + /// + /// + /// The firewall rules to modify. + /// + /// + /// to enable the rules; to disable them. + /// + /// + /// Thrown when a rule no longer exists or the PowerShell pipeline reports one or more errors. + /// + public static Task SetRulesEnabledAsync(IEnumerable rules, bool enabled) + { + var ids = rules.Where(rule => rule.IsEnabled != enabled).Select(rule => rule.Id).ToList(); - if (ps.Streams.Error.Count > 0) - throw new Exception(string.Join("; ", ps.Streams.Error)); - }); - } - finally - { - RunspaceLock.Release(); - } + return InvokeRuleCommandAsync(enabled ? "Enable-NetFirewallRule" : "Disable-NetFirewallRule", ids); } /// - /// Permanently removes the given by running - /// Remove-NetFirewallRule against the rule's internal . + /// Permanently removes the given . /// /// /// The firewall rule to delete. /// /// - /// Thrown when the PowerShell pipeline reports one or more errors. + /// Thrown when the rule no longer exists or the PowerShell pipeline reports one or more errors. + /// + public static Task DeleteRuleAsync(FirewallRule rule) + { + return DeleteRulesAsync([rule]); + } + + /// + /// Permanently removes the given by running + /// Remove-NetFirewallRule against the rules' internal . + /// Nothing is deleted if one of the rules no longer exists. + /// + /// + /// The firewall rules to delete. + /// + /// + /// Thrown when a rule no longer exists or the PowerShell pipeline reports one or more errors. /// - public static async Task DeleteRuleAsync(FirewallRule rule) + public static Task DeleteRulesAsync(IEnumerable rules) { + return InvokeRuleCommandAsync("Remove-NetFirewallRule", rules.Select(rule => rule.Id).ToList()); + } + + /// + /// Runs the given firewall cmdlet (e.g. Enable-NetFirewallRule) for all rule IDs in a + /// single PowerShell call. All rules must still exist, otherwise nothing is changed (all or nothing). + /// + /// + /// The cmdlet to run. Must accept the rule IDs via -Name. + /// + /// + /// The internal IDs of the rules. Nothing is done if the list is empty. + /// + /// + /// Thrown when a rule no longer exists or the PowerShell pipeline reports one or more errors. + /// + private static async Task InvokeRuleCommandAsync(string command, IReadOnlyCollection ids) + { + if (ids.Count == 0) + return; + await RunspaceLock.WaitAsync(); try { @@ -224,7 +262,11 @@ await Task.Run(() => using var ps = SMA.PowerShell.Create(); ps.Runspace = SharedRunspace; - ps.AddScript($@"Remove-NetFirewallRule -Name '{rule.Id}'"); + // Check that all rules still exist before changing anything + ps.AddScript($@" +$ids = {ToPsArray(ids)} +Get-NetFirewallRule -Name $ids -ErrorAction Stop | Out-Null +{command} -Name $ids"); ps.Invoke(); if (ps.Streams.Error.Count > 0) diff --git a/Source/NETworkManager/ViewModels/FirewallViewModel.cs b/Source/NETworkManager/ViewModels/FirewallViewModel.cs index 9e05b03eca..706676bee9 100644 --- a/Source/NETworkManager/ViewModels/FirewallViewModel.cs +++ b/Source/NETworkManager/ViewModels/FirewallViewModel.cs @@ -28,6 +28,11 @@ public class FirewallViewModel : ViewModelBase { #region Variables + /// + /// Maximum number of rules listed in the delete confirmation dialog. + /// + private const int DeleteRulesMaxDisplayed = 10; + private static readonly ILog Log = LogManager.GetLogger(typeof(FirewallViewModel)); private readonly bool _isLoading; @@ -76,6 +81,16 @@ public IList SelectedResults } } = new ArrayList(); + /// + /// Indicates whether at least one of the selected rules is enabled (used to show "Disable"). + /// + public bool IsAnySelectedEntryEnabled => GetTargetEntries().Any(rule => rule.IsEnabled); + + /// + /// Indicates whether at least one of the selected rules is disabled (used to show "Enable"). + /// + public bool IsAnySelectedEntryDisabled => GetTargetEntries().Any(rule => !rule.IsEnabled); + /// /// Gets or sets the search text for filtering rules. /// @@ -109,6 +124,22 @@ public bool IsRefreshing } } + /// + /// Gets or sets whether a rule is currently being modified (added, edited, enabled, disabled or deleted). + /// + public bool IsModifying + { + get; + set + { + if (value == field) + return; + + field = value; + OnPropertyChanged(); + } + } + /// /// Gets or sets whether the status message bar is shown. /// @@ -215,6 +246,8 @@ private async Task AddRule() childWindow.IsOpen = false; ConfigurationManager.Current.IsChildWindowOpen = false; + IsModifying = true; + try { await Firewall.AddRuleAsync(BuildRule(instance)); @@ -227,6 +260,8 @@ private async Task AddRule() StatusMessage = ex.Message; IsStatusMessageDisplayed = true; } + + IsModifying = false; }, _ => { childWindow.IsOpen = false; @@ -242,42 +277,48 @@ private async Task AddRule() } /// - /// Gets the command to enable the selected firewall rule. - /// Only executable when the rule is currently disabled and modification is allowed. + /// Gets the command to enable the selected firewall rule(s). + /// Only executable when at least one selected rule is disabled and modification is allowed. /// - public ICommand EnableRuleCommand => new RelayCommand(parameter => { _ = SetRuleEnabled(SelectedResult, true); }, _ => ModifyRule_CanExecute() && SelectedResult is { IsEnabled: false }); + public ICommand EnableRuleCommand => new RelayCommand(parameter => { _ = SetRulesEnabled(true); }, _ => ModifyRule_CanExecute() && IsAnySelectedEntryDisabled); /// - /// Gets the command to disable the selected firewall rule. - /// Only executable when the rule is currently enabled and modification is allowed. + /// Gets the command to disable the selected firewall rule(s). + /// Only executable when at least one selected rule is enabled and modification is allowed. /// - public ICommand DisableRuleCommand => new RelayCommand(parameter => { _ = SetRuleEnabled(SelectedResult, false); }, _ => ModifyRule_CanExecute() && SelectedResult is { IsEnabled: true }); + public ICommand DisableRuleCommand => new RelayCommand(parameter => { _ = SetRulesEnabled(false); }, _ => ModifyRule_CanExecute() && IsAnySelectedEntryEnabled); /// - /// Enables or disables the given via PowerShell, + /// Enables or disables the selected rule(s) via PowerShell (rules already in the target state are skipped), /// then reloads the rule list to reflect the updated state. /// Any PowerShell error is written to the log and shown in the status bar. /// - /// - /// The firewall rule to modify. - /// /// - /// to enable the rule; to disable it. + /// to enable the rules; to disable them. /// - private async Task SetRuleEnabled(FirewallRule rule, bool enabled) + private async Task SetRulesEnabled(bool enabled) { + var rules = GetTargetEntries(); + + if (rules.Count == 0) + return; + + IsModifying = true; + try { - await Firewall.SetRuleEnabledAsync(rule, enabled); + await Firewall.SetRulesEnabledAsync(rules, enabled); await Refresh(); } catch (Exception ex) { - Log.Error($"Error while {(enabled ? "enabling" : "disabling")} firewall rule", ex); + Log.Error($"Error while {(enabled ? "enabling" : "disabling")} firewall rule(s)", ex); StatusMessage = ex.Message; IsStatusMessageDisplayed = true; } + + IsModifying = false; } /// @@ -300,6 +341,8 @@ private async Task EditRule() childWindow.IsOpen = false; ConfigurationManager.Current.IsChildWindowOpen = false; + IsModifying = true; + try { await Firewall.DeleteRuleAsync(instance.Entry); @@ -313,6 +356,8 @@ private async Task EditRule() StatusMessage = ex.Message; IsStatusMessageDisplayed = true; } + + IsModifying = false; }, _ => { childWindow.IsOpen = false; @@ -347,6 +392,8 @@ private async Task CopyAsRule() childWindow.IsOpen = false; ConfigurationManager.Current.IsChildWindowOpen = false; + IsModifying = true; + try { await Firewall.AddRuleAsync(BuildRule(instance)); @@ -359,6 +406,8 @@ private async Task CopyAsRule() StatusMessage = ex.Message; IsStatusMessageDisplayed = true; } + + IsModifying = false; }, _ => { childWindow.IsOpen = false; @@ -374,52 +423,95 @@ private async Task CopyAsRule() } /// - /// Gets the command to permanently delete the selected firewall rule. + /// Gets the command to permanently delete the selected firewall rule(s). /// Only executable when a rule is selected and modification is allowed. /// - public ICommand DeleteRuleCommand => new RelayCommand(parameter => { _ = DeleteRule(); }, _ => ModifyRule_CanExecute() && SelectedResult != null); + public ICommand DeleteRuleCommand => new RelayCommand(parameter => { _ = DeleteRules(); }, _ => ModifyRule_CanExecute() && SelectedResult != null); /// - /// Shows a confirmation dialog and, if confirmed, deletes the selected firewall rule - /// via PowerShell and reloads the rule list. - /// Any PowerShell error is written to the log and shown in the status bar. + /// Asks for confirmation and permanently deletes the selected rule(s) via PowerShell, + /// then reloads the rule list. Any PowerShell error is written to the log and shown in the status bar. /// - private async Task DeleteRule() + private async Task DeleteRules() { + var rules = GetTargetEntries(); + + if (rules.Count == 0) + return; + + IsModifying = true; + + string message; + + if (rules.Count == 1) + { + message = string.Format(Strings.DeleteFirewallRuleMessage, rules[0].Name); + } + else + { + // Show the first rules, the rest is summarized as "(+X more)" + var lines = rules.Take(DeleteRulesMaxDisplayed).Select(rule => rule.Name).ToList(); + + if (rules.Count > DeleteRulesMaxDisplayed) + lines.Add(string.Format(Strings.PlusXMore, rules.Count - DeleteRulesMaxDisplayed)); + + message = string.Format(Strings.DeleteFirewallRulesMessage, string.Join(Environment.NewLine, lines)); + } + var result = await DialogHelper.ShowConfirmationMessageAsync( Application.Current.MainWindow, - Strings.DeleteRule, - string.Format(Strings.DeleteFirewallRuleMessage, SelectedResult.Name), + rules.Count == 1 ? Strings.DeleteRule : string.Format(Strings.DeleteRulesX, rules.Count), + message, ChildWindowIcon.Info, Strings.Delete); if (!result) + { + IsModifying = false; return; + } try { - await Firewall.DeleteRuleAsync(SelectedResult); + await Firewall.DeleteRulesAsync(rules); await Refresh(); } catch (Exception ex) { - Log.Error("Error while deleting firewall rule", ex); + Log.Error("Error while deleting firewall rule(s)", ex); StatusMessage = ex.Message; IsStatusMessageDisplayed = true; } + + IsModifying = false; } /// /// Returns when the application is running as administrator, - /// no dialog is open, and no child window is open — i.e. it is safe to modify a rule. + /// no dialog is open, no child window is open and no rule is being modified — i.e. it is safe to modify a rule. /// - private static bool ModifyRule_CanExecute() + private bool ModifyRule_CanExecute() { return ConfigurationManager.Current.IsAdmin && Application.Current.MainWindow != null && !((MetroWindow)Application.Current.MainWindow).IsAnyDialogOpen && - !ConfigurationManager.Current.IsChildWindowOpen; + !ConfigurationManager.Current.IsChildWindowOpen && + !IsModifying; + } + + /// + /// Gets the rules to modify: the multi-selection, or the single selected rule. + /// + private List GetTargetEntries() + { + if (SelectedResults != null && SelectedResults.Count > 1) + return [.. SelectedResults.Cast()]; + + if (SelectedResult != null) + return [SelectedResult]; + + return []; } /// @@ -638,6 +730,16 @@ private static List ParseAddressesString(string value) return [.. value.Split(';', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries)]; } + /// + /// Called when a context menu is opened. Updates the selection dependent properties, + /// because is changed in place (no change notification). + /// + public void OnContextMenuOpened() + { + OnPropertyChanged(nameof(IsAnySelectedEntryEnabled)); + OnPropertyChanged(nameof(IsAnySelectedEntryDisabled)); + } + /// /// Called when the view becomes visible. /// diff --git a/Source/NETworkManager/Views/FirewallView.xaml b/Source/NETworkManager/Views/FirewallView.xaml index dd6e198fb2..2731f6e963 100644 --- a/Source/NETworkManager/Views/FirewallView.xaml +++ b/Source/NETworkManager/Views/FirewallView.xaml @@ -227,6 +227,45 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/Source/NETworkManager/Views/FirewallView.xaml.cs b/Source/NETworkManager/Views/FirewallView.xaml.cs index aa92079beb..61d85c59b6 100644 --- a/Source/NETworkManager/Views/FirewallView.xaml.cs +++ b/Source/NETworkManager/Views/FirewallView.xaml.cs @@ -35,6 +35,8 @@ private void ContextMenu_Opened(object sender, RoutedEventArgs e) { if (sender is ContextMenu menu) menu.DataContext = _viewModel; + + _viewModel.OnContextMenuOpened(); } /// diff --git a/Website/docs/application/firewall.md b/Website/docs/application/firewall.md index 7e157bf701..3a83548d82 100644 --- a/Website/docs/application/firewall.md +++ b/Website/docs/application/firewall.md @@ -31,15 +31,17 @@ Without administrator privileges, the view is read-only. Use the **Restart as ad ### Context menu +Multiple rules can be selected (`Ctrl` / `Shift` + click) to enable, disable or delete them at once. + | Action | Description | |--------|-------------| -| **Enable** | Enables the selected rule | -| **Disable** | Disables the selected rule | -| **Edit** | Opens the edit dialog for the selected rule | -| **Copy as** | Opens the dialog pre-filled with the selected rule to create a new rule based on it | -| **Delete** | Removes the selected rule | -| **Copy** | Copies the rule information to the clipboard | -| **Export** | Exports the rule to a file | +| **Enable** | Enables the selected rule or rules | +| **Disable** | Disables the selected rule or rules | +| **Edit** | Opens the edit dialog for the selected rule (single rule only) | +| **Copy as** | Opens the dialog pre-filled with the selected rule to create a new rule based on it (single rule only) | +| **Delete** | Removes the selected rule or rules (after confirmation) | +| **Copy** | Copies the rule information to the clipboard (single rule only) | +| **Export** | Exports the selected rules (or all rules) to a file | ### Keyboard shortcuts @@ -47,7 +49,13 @@ Without administrator privileges, the view is read-only. Use the **Restart as ad |-----|--------| | `F5` | Refresh | | `F2` | Edit selected rule | -| `Del` | Delete selected rule | +| `Del` | Delete selected rule or rules | + +:::note + +If one of the selected rules was removed in the meantime (e.g. in the Windows Firewall management console), the action is not applied to any of the selected rules and an error is shown. The list is refreshed automatically, so the action can simply be repeated. + +::: ## Add rule diff --git a/Website/docs/changelog/next-release.md b/Website/docs/changelog/next-release.md index e0187a1f38..b4c6390e3d 100644 --- a/Website/docs/changelog/next-release.md +++ b/Website/docs/changelog/next-release.md @@ -88,6 +88,7 @@ Release date: **xx.xx.2026** **Firewall** - Added **Copy as...** to the context menu to create a new rule based on an existing one (name suffixed with ` - Copy`). [0544698](https://github.com/BornToBeRoot/NETworkManager/commit/0544698aefc325ab36ca4c8e80250e1fe0b62d4a) +- Multiple rules can now be selected to enable, disable or delete them at once. If one of the selected rules was removed in the meantime, nothing is changed. ## Bug Fixes From 82417a26d0b0cce30c5383438629e3fb89148cdd Mon Sep 17 00:00:00 2001 From: BornToBeRoot <16019165+BornToBeRoot@users.noreply.github.com> Date: Mon, 19 Oct 2026 18:43:08 +0200 Subject: [PATCH 2/2] Feature: Firewall bulk actions --- .../Firewall/Firewall.cs | 29 +-- Source/NETworkManager/DialogHelper.cs | 22 ++ .../ViewModels/FirewallViewModel.cs | 206 ++++++++---------- .../ViewModels/HostsFileEditorViewModel.cs | 16 +- Website/docs/application/firewall.md | 2 +- Website/docs/changelog/next-release.md | 2 +- 6 files changed, 120 insertions(+), 157 deletions(-) diff --git a/Source/NETworkManager.Models/Firewall/Firewall.cs b/Source/NETworkManager.Models/Firewall/Firewall.cs index 3e9f09a2da..baab14b0e6 100644 --- a/Source/NETworkManager.Models/Firewall/Firewall.cs +++ b/Source/NETworkManager.Models/Firewall/Firewall.cs @@ -167,27 +167,11 @@ public static async Task> GetRulesAsync() } } - /// - /// Enables or disables the given . - /// - /// - /// The firewall rule to modify. - /// - /// - /// to enable the rule; to disable it. - /// - /// - /// Thrown when the rule no longer exists or the PowerShell pipeline reports one or more errors. - /// - public static Task SetRuleEnabledAsync(FirewallRule rule, bool enabled) - { - return SetRulesEnabledAsync([rule], enabled); - } - /// /// Enables or disables the given by running /// Enable-NetFirewallRule or Disable-NetFirewallRule against the rules' - /// internal . Rules that are already in the target state are skipped. + /// internal . The cmdlet is run for all rules (also for rules that already + /// seem to be in the target state, since the state may have changed outside the application). /// Nothing is changed if one of the rules no longer exists. /// /// @@ -201,9 +185,8 @@ public static Task SetRuleEnabledAsync(FirewallRule rule, bool enabled) /// public static Task SetRulesEnabledAsync(IEnumerable rules, bool enabled) { - var ids = rules.Where(rule => rule.IsEnabled != enabled).Select(rule => rule.Id).ToList(); - - return InvokeRuleCommandAsync(enabled ? "Enable-NetFirewallRule" : "Disable-NetFirewallRule", ids); + return InvokeRuleCommandAsync(enabled ? "Enable-NetFirewallRule" : "Disable-NetFirewallRule", + rules.Select(rule => rule.Id).ToList()); } /// @@ -238,7 +221,9 @@ public static Task DeleteRulesAsync(IEnumerable rules) /// /// Runs the given firewall cmdlet (e.g. Enable-NetFirewallRule) for all rule IDs in a - /// single PowerShell call. All rules must still exist, otherwise nothing is changed (all or nothing). + /// single PowerShell call. It is checked first that all rules still exist, otherwise nothing is changed. + /// This is not a transaction: if the cmdlet fails for a single rule (e.g. access denied), the other + /// rules may already be changed. /// /// /// The cmdlet to run. Must accept the rule IDs via -Name. diff --git a/Source/NETworkManager/DialogHelper.cs b/Source/NETworkManager/DialogHelper.cs index a5882b633a..036a12e30a 100644 --- a/Source/NETworkManager/DialogHelper.cs +++ b/Source/NETworkManager/DialogHelper.cs @@ -4,6 +4,9 @@ using NETworkManager.Utilities; using NETworkManager.ViewModels; using NETworkManager.Views; +using System; +using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using System.Windows; @@ -94,5 +97,24 @@ public static async Task ShowConfirmationMessageAsync(Window parentWindow, return result; } + + /// + /// Formats the given items as a list (one item per line) to show them in a dialog message, + /// e.g. the entries that will be deleted. Only the first items are + /// listed, the remaining items are summarized as "(+X more)". + /// + /// The items to list. + /// The maximum number of items to list. + /// The formatted list. + public static string FormatItemList(IEnumerable items, int maxItems = 10) + { + var itemList = items.ToList(); + var lines = itemList.Take(maxItems).ToList(); + + if (itemList.Count > maxItems) + lines.Add(string.Format(Strings.PlusXMore, itemList.Count - maxItems)); + + return string.Join(Environment.NewLine, lines); + } } } diff --git a/Source/NETworkManager/ViewModels/FirewallViewModel.cs b/Source/NETworkManager/ViewModels/FirewallViewModel.cs index 706676bee9..35d6039af0 100644 --- a/Source/NETworkManager/ViewModels/FirewallViewModel.cs +++ b/Source/NETworkManager/ViewModels/FirewallViewModel.cs @@ -28,11 +28,6 @@ public class FirewallViewModel : ViewModelBase { #region Variables - /// - /// Maximum number of rules listed in the delete confirmation dialog. - /// - private const int DeleteRulesMaxDisplayed = 10; - private static readonly ILog Log = LogManager.GetLogger(typeof(FirewallViewModel)); private readonly bool _isLoading; @@ -84,12 +79,12 @@ public IList SelectedResults /// /// Indicates whether at least one of the selected rules is enabled (used to show "Disable"). /// - public bool IsAnySelectedEntryEnabled => GetTargetEntries().Any(rule => rule.IsEnabled); + public bool IsAnySelectedEntryEnabled => GetSelectedRules().Any(rule => rule.IsEnabled); /// /// Indicates whether at least one of the selected rules is disabled (used to show "Enable"). /// - public bool IsAnySelectedEntryDisabled => GetTargetEntries().Any(rule => !rule.IsEnabled); + public bool IsAnySelectedEntryDisabled => GetSelectedRules().Any(rule => !rule.IsEnabled); /// /// Gets or sets the search text for filtering rules. @@ -218,9 +213,9 @@ public FirewallViewModel() public ICommand RefreshCommand => new RelayCommand(parameter => { _ = RefreshAction(); }, Refresh_CanExecute); /// - /// Returns when no refresh is currently running. + /// Returns when no refresh is currently running and no rule is being modified. /// - private bool Refresh_CanExecute(object _) => !IsRefreshing; + private bool Refresh_CanExecute(object _) => !IsRefreshing && !IsModifying; /// /// Delegates to to reload the firewall rules. @@ -246,22 +241,7 @@ private async Task AddRule() childWindow.IsOpen = false; ConfigurationManager.Current.IsChildWindowOpen = false; - IsModifying = true; - - try - { - await Firewall.AddRuleAsync(BuildRule(instance)); - await Refresh(); - } - catch (Exception ex) - { - Log.Error("Error while adding firewall rule", ex); - - StatusMessage = ex.Message; - IsStatusMessageDisplayed = true; - } - - IsModifying = false; + await ModifyRulesAsync(() => Firewall.AddRuleAsync(BuildRule(instance)), "Error while adding firewall rule"); }, _ => { childWindow.IsOpen = false; @@ -289,7 +269,7 @@ private async Task AddRule() public ICommand DisableRuleCommand => new RelayCommand(parameter => { _ = SetRulesEnabled(false); }, _ => ModifyRule_CanExecute() && IsAnySelectedEntryEnabled); /// - /// Enables or disables the selected rule(s) via PowerShell (rules already in the target state are skipped), + /// Enables or disables the selected rule(s) via PowerShell, /// then reloads the rule list to reflect the updated state. /// Any PowerShell error is written to the log and shown in the status bar. /// @@ -303,29 +283,15 @@ private async Task SetRulesEnabled(bool enabled) if (rules.Count == 0) return; - IsModifying = true; - - try - { - await Firewall.SetRulesEnabledAsync(rules, enabled); - await Refresh(); - } - catch (Exception ex) - { - Log.Error($"Error while {(enabled ? "enabling" : "disabling")} firewall rule(s)", ex); - - StatusMessage = ex.Message; - IsStatusMessageDisplayed = true; - } - - IsModifying = false; + await ModifyRulesAsync(() => Firewall.SetRulesEnabledAsync(rules, enabled), + $"Error while {(enabled ? "enabling" : "disabling")} firewall rule(s)"); } /// /// Gets the command to open the dialog for editing the selected firewall rule. - /// Only executable when a rule is selected and modification is allowed. + /// Only executable when exactly one rule is selected and modification is allowed. /// - public ICommand EditRuleCommand => new RelayCommand(parameter => { _ = EditRule(); }, _ => ModifyRule_CanExecute() && SelectedResult != null); + public ICommand EditRuleCommand => new RelayCommand(parameter => { _ = EditRule(); }, _ => ModifyRule_CanExecute() && IsSingleRuleSelected); /// /// Opens the edit-firewall-rule dialog pre-filled with the selected rule's properties. @@ -341,23 +307,11 @@ private async Task EditRule() childWindow.IsOpen = false; ConfigurationManager.Current.IsChildWindowOpen = false; - IsModifying = true; - - try + await ModifyRulesAsync(async () => { await Firewall.DeleteRuleAsync(instance.Entry); await Firewall.AddRuleAsync(BuildRule(instance)); - await Refresh(); - } - catch (Exception ex) - { - Log.Error("Error while editing firewall rule", ex); - - StatusMessage = ex.Message; - IsStatusMessageDisplayed = true; - } - - IsModifying = false; + }, "Error while editing firewall rule"); }, _ => { childWindow.IsOpen = false; @@ -374,9 +328,9 @@ private async Task EditRule() /// /// Gets the command to copy the selected firewall rule. - /// Only enabled when the application is running as administrator. + /// Only executable when exactly one rule is selected and modification is allowed. /// - public ICommand CopyAsRuleCommand => new RelayCommand(parameter => { _ = CopyAsRule(); }, _ => ModifyRule_CanExecute() && SelectedResult != null); + public ICommand CopyAsRuleCommand => new RelayCommand(parameter => { _ = CopyAsRule(); }, _ => ModifyRule_CanExecute() && IsSingleRuleSelected); /// /// Opens the rule dialog pre-filled with the selected rule's properties (name suffixed with " - Copy"). @@ -392,22 +346,7 @@ private async Task CopyAsRule() childWindow.IsOpen = false; ConfigurationManager.Current.IsChildWindowOpen = false; - IsModifying = true; - - try - { - await Firewall.AddRuleAsync(BuildRule(instance)); - await Refresh(); - } - catch (Exception ex) - { - Log.Error("Error while copying firewall rule", ex); - - StatusMessage = ex.Message; - IsStatusMessageDisplayed = true; - } - - IsModifying = false; + await ModifyRulesAsync(() => Firewall.AddRuleAsync(BuildRule(instance)), "Error while copying firewall rule"); }, _ => { childWindow.IsOpen = false; @@ -439,24 +378,9 @@ private async Task DeleteRules() if (rules.Count == 0) return; - IsModifying = true; - - string message; - - if (rules.Count == 1) - { - message = string.Format(Strings.DeleteFirewallRuleMessage, rules[0].Name); - } - else - { - // Show the first rules, the rest is summarized as "(+X more)" - var lines = rules.Take(DeleteRulesMaxDisplayed).Select(rule => rule.Name).ToList(); - - if (rules.Count > DeleteRulesMaxDisplayed) - lines.Add(string.Format(Strings.PlusXMore, rules.Count - DeleteRulesMaxDisplayed)); - - message = string.Format(Strings.DeleteFirewallRulesMessage, string.Join(Environment.NewLine, lines)); - } + var message = rules.Count == 1 + ? string.Format(Strings.DeleteFirewallRuleMessage, rules[0].Name) + : string.Format(Strings.DeleteFirewallRulesMessage, DialogHelper.FormatItemList(rules.Select(rule => rule.Name))); var result = await DialogHelper.ShowConfirmationMessageAsync( Application.Current.MainWindow, @@ -466,25 +390,9 @@ private async Task DeleteRules() Strings.Delete); if (!result) - { - IsModifying = false; return; - } - - try - { - await Firewall.DeleteRulesAsync(rules); - await Refresh(); - } - catch (Exception ex) - { - Log.Error("Error while deleting firewall rule(s)", ex); - - StatusMessage = ex.Message; - IsStatusMessageDisplayed = true; - } - IsModifying = false; + await ModifyRulesAsync(() => Firewall.DeleteRulesAsync(rules), "Error while deleting firewall rule(s)"); } /// @@ -497,16 +405,18 @@ private bool ModifyRule_CanExecute() Application.Current.MainWindow != null && !((MetroWindow)Application.Current.MainWindow).IsAnyDialogOpen && !ConfigurationManager.Current.IsChildWindowOpen && + !IsRefreshing && !IsModifying; } /// - /// Gets the rules to modify: the multi-selection, or the single selected rule. + /// Gets the selected rules (the multi-selection, or the single selected rule) without copying them. + /// Used for checks that are evaluated often (e.g. ). /// - private List GetTargetEntries() + private IEnumerable GetSelectedRules() { - if (SelectedResults != null && SelectedResults.Count > 1) - return [.. SelectedResults.Cast()]; + if (SelectedResults is { Count: > 1 }) + return SelectedResults.Cast(); if (SelectedResult != null) return [SelectedResult]; @@ -514,6 +424,57 @@ private List GetTargetEntries() return []; } + /// + /// Indicates whether exactly one rule is selected (e.g. for edit or copy, which only work on a single rule). + /// + private bool IsSingleRuleSelected => SelectedResult != null && SelectedResults is not { Count: > 1 }; + + /// + /// Gets the rules to modify: the multi-selection, or the single selected rule. + /// + private List GetTargetEntries() => [.. GetSelectedRules()]; + + /// + /// Runs the given modification and reloads the rule list afterwards, also if the modification + /// failed (e.g. a rule was removed in the meantime), so the list always shows the actual state. + /// Any error is written to the log and shown in the status bar. + /// + /// The modification to run. + /// Message written to the log if the modification fails. + private async Task ModifyRulesAsync(Func action, string errorLogMessage) + { + IsModifying = true; + + try + { + Exception error = null; + + try + { + await action(); + } + catch (Exception ex) + { + Log.Error(errorLogMessage, ex); + error = ex; + } + + var refreshed = await Refresh(); + + // Show the error after the refresh, because the refresh overwrites the status message. + // If the refresh failed too, keep its message, so it's visible that the list is not up to date. + if (error != null) + { + StatusMessage = refreshed ? error.Message : error.Message + Environment.NewLine + StatusMessage; + IsStatusMessageDisplayed = true; + } + } + finally + { + IsModifying = false; + } + } + /// /// Gets the command to restart the application with administrator privileges. /// @@ -627,10 +588,11 @@ await DialogHelper.ShowMessageAsync(Application.Current.MainWindow, Strings.Erro /// When the initial UI delay is skipped so the first load /// on startup feels immediate. /// - private async Task Refresh(bool init = false) + /// if the rules were loaded successfully; otherwise, . + private async Task Refresh(bool init = false) { if (IsRefreshing) - return; + return false; IsRefreshing = true; StatusMessage = Strings.RefreshingDots; @@ -653,6 +615,8 @@ private async Task Refresh(bool init = false) StatusMessage = string.Format(Strings.ReloadedAtX, DateTime.Now.ToShortTimeString()); IsStatusMessageDisplayed = true; + + return true; } catch (Exception ex) { @@ -660,9 +624,13 @@ private async Task Refresh(bool init = false) StatusMessage = string.Format(Strings.FailedToLoadFirewallRulesMessage, ex.Message); IsStatusMessageDisplayed = true; - } - IsRefreshing = false; + return false; + } + finally + { + IsRefreshing = false; + } } /// diff --git a/Source/NETworkManager/ViewModels/HostsFileEditorViewModel.cs b/Source/NETworkManager/ViewModels/HostsFileEditorViewModel.cs index 4fa1fc4fd3..b4fb4c9a89 100644 --- a/Source/NETworkManager/ViewModels/HostsFileEditorViewModel.cs +++ b/Source/NETworkManager/ViewModels/HostsFileEditorViewModel.cs @@ -30,11 +30,6 @@ public class HostsFileEditorViewModel : ViewModelBase private static readonly ILog Log = LogManager.GetLogger(typeof(HostsFileEditorViewModel)); - /// - /// Maximum number of entries listed in the delete confirmation dialog. - /// - private const int DeleteEntriesMaxDisplayed = 10; - /// /// Indicates whether the view model is loading. /// @@ -580,15 +575,8 @@ private async Task DeleteEntryAction() } else { - // Show the first entries, the rest is summarized as "(+X more)" - var lines = entries.Take(DeleteEntriesMaxDisplayed) - .Select(e => $"{e.IPAddress} {e.Hostname} {(string.IsNullOrEmpty(e.Comment) ? "" : $"# {e.Comment}")}".TrimEnd()) - .ToList(); - - if (entries.Count > DeleteEntriesMaxDisplayed) - lines.Add(string.Format(Strings.PlusXMore, entries.Count - DeleteEntriesMaxDisplayed)); - - message = string.Format(Strings.DeleteHostsFileEntriesMessage, string.Join(Environment.NewLine, lines)); + message = string.Format(Strings.DeleteHostsFileEntriesMessage, DialogHelper.FormatItemList(entries.Select(e => + $"{e.IPAddress} {e.Hostname} {(string.IsNullOrEmpty(e.Comment) ? "" : $"# {e.Comment}")}".TrimEnd()))); } var result = await DialogHelper.ShowConfirmationMessageAsync( diff --git a/Website/docs/application/firewall.md b/Website/docs/application/firewall.md index 3a83548d82..9d08cf8e20 100644 --- a/Website/docs/application/firewall.md +++ b/Website/docs/application/firewall.md @@ -53,7 +53,7 @@ Multiple rules can be selected (`Ctrl` / `Shift` + click) to enable, disable or :::note -If one of the selected rules was removed in the meantime (e.g. in the Windows Firewall management console), the action is not applied to any of the selected rules and an error is shown. The list is refreshed automatically, so the action can simply be repeated. +Before enabling, disabling or deleting, it is checked that all selected rules still exist. If one of them was removed in the meantime (e.g. in the Windows Firewall management console), none of the selected rules is changed and an error is shown. If another error occurs while the rules are changed (e.g. access denied), some of the selected rules may already be changed. The rule list is reloaded after every action (also after an error), so it always shows the current state. ::: diff --git a/Website/docs/changelog/next-release.md b/Website/docs/changelog/next-release.md index b4c6390e3d..fb962aef29 100644 --- a/Website/docs/changelog/next-release.md +++ b/Website/docs/changelog/next-release.md @@ -88,7 +88,7 @@ Release date: **xx.xx.2026** **Firewall** - Added **Copy as...** to the context menu to create a new rule based on an existing one (name suffixed with ` - Copy`). [0544698](https://github.com/BornToBeRoot/NETworkManager/commit/0544698aefc325ab36ca4c8e80250e1fe0b62d4a) -- Multiple rules can now be selected to enable, disable or delete them at once. If one of the selected rules was removed in the meantime, nothing is changed. +- Multiple rules can now be selected to enable, disable or delete them at once. If one of the selected rules was removed in the meantime, nothing is changed. [#3627](https://github.com/BornToBeRoot/NETworkManager/pull/3627) ## Bug Fixes