Skip to content

Commit b854cbe

Browse files
committed
Chore: Refactoring
1 parent 709210d commit b854cbe

File tree

10 files changed

+136
-89
lines changed

10 files changed

+136
-89
lines changed

Source/NETworkManager.Models/Firewall/Firewall.cs

Lines changed: 64 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using System.Collections.Generic;
33
using System.IO;
44
using System.Linq;
5+
using System.Text;
56
using System.Threading.Tasks;
67
using log4net;
78
using NETworkManager.Models.Network;
@@ -20,7 +21,7 @@ public class Firewall
2021
/// <summary>
2122
/// The Logger.
2223
/// </summary>
23-
private readonly ILog _logger = LogManager.GetLogger(typeof(Firewall));
24+
private readonly ILog Log = LogManager.GetLogger(typeof(Firewall));
2425

2526
#endregion
2627

@@ -33,79 +34,85 @@ public class Firewall
3334
/// <returns>Returns <c>true</c> if the rules were successfully applied; otherwise, <c>false</c>.</returns>
3435
private bool ApplyRules(List<FirewallRule> rules)
3536
{
36-
string command = GetClearAllRulesCommand();
37+
// If there are no rules to apply, return true as there is nothing to do.
3738
if (rules.Count is 0)
3839
return true;
39-
command += "; ";
40-
foreach (FirewallRule rule in rules)
40+
41+
// Start by clearing all existing rules for the current profile to ensure a clean state.
42+
var sb = new StringBuilder(GetClearAllRulesCommand());
43+
sb.Append("; ");
44+
45+
foreach (var rule in rules)
4146
{
42-
string nextRule = string.Empty;
4347
try
4448
{
45-
nextRule += $"New-NetFirewallRule -DisplayName '{SanitizeStringArguments(rule.Name)}'";
49+
var ruleSb = new StringBuilder($"New-NetFirewallRule -DisplayName '{SanitizeStringArguments(rule.Name)}'");
50+
4651
if (!string.IsNullOrEmpty(rule.Description))
47-
nextRule += $" -Description '{SanitizeStringArguments(rule.Description)}'";
48-
nextRule += $" -Direction {Enum.GetName(rule.Direction)}";
49-
if (rule.LocalPorts.Count > 0
50-
&& rule.Protocol is FirewallProtocol.TCP or FirewallProtocol.UDP)
51-
{
52-
nextRule += $" -LocalPort {FirewallRule.PortsToString(rule.LocalPorts, ',', false)}";
53-
}
52+
ruleSb.Append($" -Description '{SanitizeStringArguments(rule.Description)}'");
53+
54+
ruleSb.Append($" -Direction {Enum.GetName(rule.Direction)}");
55+
56+
if (rule.LocalPorts.Count > 0 && rule.Protocol is FirewallProtocol.TCP or FirewallProtocol.UDP)
57+
ruleSb.Append($" -LocalPort {FirewallRule.PortsToString(rule.LocalPorts, ',', false)}");
58+
59+
if (rule.RemotePorts.Count > 0 && rule.Protocol is FirewallProtocol.TCP or FirewallProtocol.UDP)
60+
ruleSb.Append($" -RemotePort {FirewallRule.PortsToString(rule.RemotePorts, ',', false)}");
61+
62+
ruleSb.Append(rule.Protocol is FirewallProtocol.Any
63+
? " -Protocol Any"
64+
: $" -Protocol {(int)rule.Protocol}");
5465

55-
if (rule.RemotePorts.Count > 0
56-
&& rule.Protocol is FirewallProtocol.TCP or FirewallProtocol.UDP)
57-
nextRule += $" -RemotePort {FirewallRule.PortsToString(rule.RemotePorts, ',', false)}";
58-
if (rule.Protocol is FirewallProtocol.Any)
59-
nextRule += $" -Protocol Any";
60-
else
61-
nextRule += $" -Protocol {(int)rule.Protocol}";
6266
if (!string.IsNullOrWhiteSpace(rule.Program?.Name))
6367
{
64-
try
65-
{
66-
if (File.Exists(rule.Program.Name))
67-
nextRule += $" -Program '{SanitizeStringArguments(rule.Program.Name)}'";
68-
else
69-
continue;
70-
}
71-
catch
68+
if (File.Exists(rule.Program.Name))
69+
ruleSb.Append($" -Program '{SanitizeStringArguments(rule.Program.Name)}'");
70+
else
7271
{
72+
Log.Warn($"Program path '{rule.Program.Name}' in rule '{rule.Name}' does not exist. Skipping rule.");
7373
continue;
7474
}
7575
}
7676

7777
if (rule.InterfaceType is not FirewallInterfaceType.Any)
78-
nextRule += $" -InterfaceType {Enum.GetName(rule.InterfaceType)}";
78+
ruleSb.Append($" -InterfaceType {Enum.GetName(rule.InterfaceType)}");
79+
80+
// If not all network profiles are enabled, specify the ones that are.
7981
if (!rule.NetworkProfiles.All(x => x))
8082
{
81-
nextRule += $" -Profile ";
82-
for (int i = 0; i < rule.NetworkProfiles.Length; i++)
83-
{
84-
if (rule.NetworkProfiles[i])
85-
nextRule += $"{Enum.GetName(typeof(NetworkProfiles), i)},";
86-
}
87-
nextRule = nextRule[..^1];
83+
var profiles = Enumerable.Range(0, rule.NetworkProfiles.Length)
84+
.Where(i => rule.NetworkProfiles[i])
85+
.Select(i => Enum.GetName(typeof(NetworkProfiles), i));
86+
87+
ruleSb.Append($" -Profile {string.Join(',', profiles)}");
8888
}
89-
nextRule += $" -Action {Enum.GetName(rule.Action)}; ";
90-
command += nextRule;
89+
90+
ruleSb.Append($" -Action {Enum.GetName(rule.Action)}; ");
91+
92+
sb.Append(ruleSb);
9193
}
92-
catch (ArgumentException)
94+
catch (ArgumentException ex)
9395
{
96+
Log.Warn($"Failed to build firewall rule '{rule.Name}': {ex.Message}");
9497
}
9598
}
9699

97-
command = command[..^2];
100+
// Remove the trailing "; " from the last command.
101+
sb.Length -= 2;
102+
103+
var command = sb.ToString();
104+
105+
Log.Debug($"Applying rules:{Environment.NewLine}{command}");
106+
98107
try
99108
{
100-
_logger.Info($"[Firewall] Applying rules:{Environment.NewLine}{command}");
101109
PowerShellHelper.ExecuteCommand(command, true);
110+
return true;
102111
}
103112
catch (Exception)
104113
{
105114
return false;
106115
}
107-
108-
return true;
109116
}
110117

111118
/// <summary>
@@ -118,14 +125,23 @@ private bool ApplyRules(List<FirewallRule> rules)
118125
/// </remarks>
119126
public static void ClearAllRules()
120127
{
121-
PowerShellHelper.ExecuteCommand($"{GetClearAllRulesCommand()}", true);
128+
PowerShellHelper.ExecuteCommand(GetClearAllRulesCommand(), true);
122129
}
123130

131+
/// <summary>
132+
/// Generates a command string that removes all Windows Firewall rules with a display name starting with 'NwM_'.
133+
/// </summary>
134+
/// <returns>A command string that can be executed in PowerShell to remove the specified firewall rules.</returns>
124135
private static string GetClearAllRulesCommand()
125136
{
126-
return $"Remove-NetFirewallRule -DisplayName 'NwM_*'";
137+
return "Remove-NetFirewallRule -DisplayName 'NwM_*'";
127138
}
128139

140+
/// <summary>
141+
/// Sanitizes string arguments by replacing single quotes with double single quotes to prevent issues in PowerShell command execution.
142+
/// </summary>
143+
/// <param name="value">The input string to be sanitized.</param>
144+
/// <returns>A sanitized string with single quotes escaped.</returns>
129145
private static string SanitizeStringArguments(string value)
130146
{
131147
return value.Replace("'", "''");
@@ -136,9 +152,9 @@ private static string SanitizeStringArguments(string value)
136152
/// </summary>
137153
/// <param name="rules">A list of firewall rules to apply.</param>
138154
/// <returns>A task representing the asynchronous operation. The task result contains a boolean indicating whether the rules were successfully applied.</returns>
139-
public async Task<bool> ApplyRulesAsync(List<FirewallRule> rules)
155+
public async Task ApplyRulesAsync(List<FirewallRule> rules)
140156
{
141-
return await Task.Run(() => ApplyRules(rules));
157+
await Task.Run(() => ApplyRules(rules));
142158
}
143159
#endregion
144-
}
160+
}
Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,27 @@
11
namespace NETworkManager.Models.Firewall;
22

3+
/// <summary>
4+
/// Defines the types of network interfaces that can be used in firewall rules.
5+
/// </summary>
36
public enum FirewallInterfaceType
47
{
58
/// <summary>
69
/// Any interface type.
710
/// </summary>
811
Any = -1,
12+
913
/// <summary>
1014
/// Wired interface types, e.g. Ethernet.
1115
/// </summary>
1216
Wired,
17+
1318
/// <summary>
1419
/// Wireless interface types, e.g. Wi-Fi.
1520
/// </summary>
1621
Wireless,
22+
1723
/// <summary>
1824
/// Remote interface types, e.g. VPN, L2TP, OpenVPN, etc.
1925
/// </summary>
2026
RemoteAccess
21-
}
27+
}

Source/NETworkManager.Models/Firewall/FirewallPortLocation.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ public enum FirewallPortLocation
99
/// Ports of local host.
1010
/// </summary>
1111
LocalPorts,
12+
1213
/// <summary>
1314
/// Ports of remote host.
1415
/// </summary>

Source/NETworkManager.Models/Firewall/FirewallPortSpecification.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ public override string ToString()
6060
{
6161
if (Start is 0)
6262
return string.Empty;
63+
6364
return End is -1 or 0 ? $"{Start}" : $"{Start}-{End}";
6465
}
65-
}
66+
}

Source/NETworkManager.Models/Firewall/FirewallRule.cs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ public bool[] NetworkProfiles
107107
/// </summary>
108108
public FirewallRule()
109109
{
110+
110111
}
111112
#endregion
112113

@@ -119,16 +120,20 @@ public FirewallRule()
119120
/// <param name="separator">Separator character to use</param>
120121
/// <param name="spacing">Separate entries with a space.</param>
121122
/// <returns>A separated string containing all the port numbers from the input collection.</returns>
122-
public static string PortsToString(List<FirewallPortSpecification> ports, char separator = ';',
123-
bool spacing = true)
123+
public static string PortsToString(List<FirewallPortSpecification> ports, char separator = ';', bool spacing = true)
124124
{
125125
if (ports.Count is 0)
126126
return string.Empty;
127-
string whitespace = spacing ? " " : string.Empty;
127+
128+
var whitespace = spacing ? " " : string.Empty;
129+
128130
var builder = new StringBuilder();
131+
129132
foreach (var port in ports)
130133
builder.Append($"{port}{separator}{whitespace}");
131-
int offset = spacing ? 2 : 1;
134+
135+
var offset = spacing ? 2 : 1;
136+
132137
return builder.ToString()[..^offset];
133138
}
134139
#endregion

Source/NETworkManager.Models/Firewall/FirewallRuleAction.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ public enum FirewallRuleAction
1414
/// Represents the action to allow network traffic.
1515
/// </summary>
1616
Allow,
17+
1718
// Unsupported for now
1819
//AllowIPsec
1920
}

Source/NETworkManager.Models/Firewall/FirewallRuleProgram.cs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,23 @@ public class FirewallRuleProgram : ICloneable
1616
/// </summary>
1717
[JsonIgnore]
1818
[XmlIgnore]
19-
public FileInfo Executable { private set; get; }
19+
public FileInfo Executable { private init; get; }
2020

21+
/// <summary>
22+
/// Represents the name associated with the object.
23+
/// </summary>
2124
public string Name
2225
{
2326
get;
24-
set
27+
private init
2528
{
2629
if (string.IsNullOrWhiteSpace(value))
2730
return;
31+
2832
Executable = new FileInfo(value);
29-
if (!Executable.Exists)
30-
return;
3133
field = value;
3234
}
3335
}
34-
3536
#endregion
3637

3738
#region Constructor
@@ -40,6 +41,7 @@ public string Name
4041
/// </summary>
4142
public FirewallRuleProgram()
4243
{
44+
4345
}
4446

4547
/// <summary>
@@ -74,4 +76,4 @@ public object Clone()
7476
}
7577

7678
#endregion
77-
}
79+
}

Source/NETworkManager.Utilities/ListHelper.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
namespace NETworkManager.Utilities;
44
/// <summary>
5-
/// Helps to modify a list to implement history combo boxes.
5+
/// Helper class for modifying lists by adding new entries and removing old ones if the list exceeds a specified length.
66
/// </summary>
77
public static class ListHelper
88
{
@@ -17,21 +17,21 @@ public static class ListHelper
1717
/// <returns>Modified list with the new entry added and oldest entries removed if necessary.</returns>
1818
public static List<T> Modify<T>(List<T> list, T entry, int length)
1919
{
20+
var removedEntry = false;
21+
2022
int index;
21-
bool removedEntry = false;
22-
do
23+
24+
while ((index = list.IndexOf(entry)) != -1)
2325
{
24-
index = list.IndexOf(entry);
25-
if (index is -1)
26-
break;
2726
list.RemoveAt(index);
2827
removedEntry = true;
29-
} while (index >= 0);
28+
}
29+
3030
if (!removedEntry && list.Count == length)
3131
list.RemoveAt(length - 1);
3232

3333
list.Insert(0, entry);
3434

3535
return list;
3636
}
37-
}
37+
}

0 commit comments

Comments
 (0)