win_firewall_rule only change arguments passed by user (#54297)

* win_firewall_rule only changes specified arguments defaults are controlled by com object integration test for built in rule * removed ignore psaliases for win_firewall_rule * direction and action are no longer required program and service respect default values documentation updated to reflect that defaults apply to rule creation added test to disable a rule and verify other values have not changed * fixed extra whitespace * Move each description sentance to a new entry
5 years ago · 2b049238d6
parent 365db9057c
commit 2b049238d6
4 changed files with 103 additions and 112 deletions
--- a/lib/ansible/modules/windows/win_firewall_rule.ps1
+++ b/lib/ansible/modules/windows/win_firewall_rule.ps1
@ -50,7 +50,7 @@ function Parse-Profiles
 {
    param($profilesList)

-    $profiles = ($profilesList | Select -uniq | ForEach {
+    $profiles = ($profilesList | Select-Object -Unique | ForEach-Object {
        switch ($_) {
            "domain" { return 1 }
            "private" { return 2 }
@ -67,7 +67,7 @@ function Parse-InterfaceTypes
 {
    param($interfaceTypes)

-    return ($interfaceTypes | Select -uniq | ForEach {
+    return ($interfaceTypes | Select-Object -Unique | ForEach-Object {
        switch ($_) {
            "wireless" { return "Wireless" }
            "lan" { return "Lan" }
@ -102,60 +102,6 @@ function Parse-SecureFlags
    }
 }

-function New-FWRule
-{
-    param (
-        [string]$name,
-        [string]$description,
-        [string]$applicationName,
-        [string]$serviceName,
-        [string]$protocol,
-        [string]$localPorts,
-        [string]$remotePorts,
-        [string]$localAddresses,
-        [string]$remoteAddresses,
-        [string]$direction,
-        [string]$action,
-        [bool]$enabled,
-        [string[]]$profiles,
-        [string[]]$interfaceTypes,
-        [string]$edgeTraversalOptions,
-        [string]$secureFlags
-    )
-
-    # INetFwRule interface description: https://msdn.microsoft.com/en-us/library/windows/desktop/aa365344(v=vs.85).aspx
-    $rule = New-Object -ComObject HNetCfg.FWRule
-    $rule.Name = $name
-    $rule.Enabled = $enabled
-    if ($description) { $rule.Description = $description }
-    if ($applicationName) { $rule.ApplicationName = $applicationName }
-    if ($serviceName) { $rule.ServiceName = $serviceName }
-    if ($protocol -and $protocol -ne "any") { $rule.Protocol = Parse-ProtocolType -protocol $protocol }
-    if ($localPorts -and $localPorts -ne "any") { $rule.LocalPorts = $localPorts }
-    if ($remotePorts -and $remotePorts -ne "any") { $rule.RemotePorts = $remotePorts }
-    if ($localAddresses -and $localAddresses -ne "any") { $rule.LocalAddresses = $localAddresses }
-    if ($remoteAddresses -and $remoteAddresses -ne "any") { $rule.RemoteAddresses = $remoteAddresses }
-    if ($direction) { $rule.Direction = Parse-Direction -directionStr $direction }
-    if ($action) { $rule.Action = Parse-Action -actionStr $action }
-    # Profiles value cannot be a uint32, but the "all profiles" value (0x7FFFFFFF) will often become a uint32, so must cast to [int]
-    if ($profiles) { $rule.Profiles = [int](Parse-Profiles -profilesList $profiles) }
-    if ($interfaceTypes -and @(Compare-Object $interfaceTypes @("any")).Count -ne 0) { $rule.InterfaceTypes = Parse-InterfaceTypes -interfaceTypes $interfaceTypes }
-    if ($edgeTraversalOptions -and $edgeTraversalOptions -ne "no") {
-        # EdgeTraversalOptions property exists only from Windows 7/Windows Server 2008 R2: https://msdn.microsoft.com/en-us/library/windows/desktop/dd607256(v=vs.85).aspx
-        if ($rule | Get-Member -Name 'EdgeTraversalOptions') {
-            $rule.EdgeTraversalOptions = Parse-EdgeTraversalOptions -edgeTraversalOptionsStr $edgeTraversalOptions
-        }
-    }
-    if ($secureFlags -and $secureFlags -ne "notrequired") {
-        # SecureFlags property exists only from Windows 8/Windows Server 2012: https://msdn.microsoft.com/en-us/library/windows/desktop/hh447465(v=vs.85).aspx
-        if ($rule | Get-Member -Name 'SecureFlags') {
-            $rule.SecureFlags = Parse-SecureFlags -secureFlagsStr $secureFlags
-        }
-    }
-
-    return $rule
-}
-
 $ErrorActionPreference = "Stop"

 $result = @{
@ -168,20 +114,20 @@ $diff_support = Get-AnsibleParam -obj $params -name "_ansible_diff" -type "bool"

 $name = Get-AnsibleParam -obj $params -name "name" -failifempty $true
 $description = Get-AnsibleParam -obj $params -name "description" -type "str"
-$direction = Get-AnsibleParam -obj $params -name "direction" -type "str" -failifempty $true -validateset "in","out"
-$action = Get-AnsibleParam -obj $params -name "action" -type "str" -failifempty $true -validateset "allow","block"
+$direction = Get-AnsibleParam -obj $params -name "direction" -type "str" -validateset "in","out"
+$action = Get-AnsibleParam -obj $params -name "action" -type "str" -validateset "allow","block"
 $program = Get-AnsibleParam -obj $params -name "program" -type "str"
 $service = Get-AnsibleParam -obj $params -name "service" -type "str"
-$enabled = Get-AnsibleParam -obj $params -name "enabled" -type "bool" -default $true -aliases "enable"
-$profiles = Get-AnsibleParam -obj $params -name "profiles" -type "list" -default @("domain", "private", "public") -aliases "profile"
-$localip = Get-AnsibleParam -obj $params -name "localip" -type "str" -default "any"
-$remoteip = Get-AnsibleParam -obj $params -name "remoteip" -type "str" -default "any"
+$enabled = Get-AnsibleParam -obj $params -name "enabled" -type "bool" -aliases "enable"
+$profiles = Get-AnsibleParam -obj $params -name "profiles" -type "list" -aliases "profile"
+$localip = Get-AnsibleParam -obj $params -name "localip" -type "str"
+$remoteip = Get-AnsibleParam -obj $params -name "remoteip" -type "str"
 $localport = Get-AnsibleParam -obj $params -name "localport" -type "str"
 $remoteport = Get-AnsibleParam -obj $params -name "remoteport" -type "str"
-$protocol = Get-AnsibleParam -obj $params -name "protocol" -type "str" -default "any"
-$interfacetypes = Get-AnsibleParam -obj $params -name "interfacetypes" -type "list" -default @("any")
-$edge = Get-AnsibleParam -obj $params -name "edge" -type "str" -default "no" -validateset "no","yes","deferapp","deferuser"
-$security = Get-AnsibleParam -obj $params -name "security" -type "str" -default "notrequired" -validateset "notrequired","authnoencap","authenticate","authdynenc","authenc"
+$protocol = Get-AnsibleParam -obj $params -name "protocol" -type "str"
+$interfacetypes = Get-AnsibleParam -obj $params -name "interfacetypes" -type "list"
+$edge = Get-AnsibleParam -obj $params -name "edge" -type "str" -validateset "no","yes","deferapp","deferuser"
+$security = Get-AnsibleParam -obj $params -name "security" -type "str" -validateset "notrequired","authnoencap","authenticate","authdynenc","authenc"

 $state = Get-AnsibleParam -obj $params -name "state" -type "str" -default "present" -validateset "present","absent"

@ -198,33 +144,48 @@ if ($diff_support) {
 try {
    $fw = New-Object -ComObject HNetCfg.FwPolicy2

-    $existingRule = $fw.Rules | Where { $_.Name -eq $name }
+    $existingRule = $fw.Rules | Where-Object { $_.Name -eq $name }

    if ($existingRule -is [System.Array]) {
        Fail-Json $result "Multiple firewall rules with name '$name' found."
    }

-    $rule = New-FWRule -name $name `
-                       -description $description `
-                       -direction $direction `
-                       -action $action `
-                       -applicationName $program `
-                       -serviceName $service `
-                       -enabled $enabled `
-                       -profiles $profiles `
-                       -localAddresses $localip `
-                       -remoteAddresses $remoteip `
-                       -localPorts $localport `
-                       -remotePorts $remoteport `
-                       -protocol $protocol `
-                       -interfaceTypes $interfacetypes `
-                       -edgeTraversalOptions $edge `
-                       -secureFlags $security
+        # INetFwRule interface description: https://msdn.microsoft.com/en-us/library/windows/desktop/aa365344(v=vs.85).aspx
+    $new_rule = New-Object -ComObject HNetCfg.FWRule
+    $new_rule.Name = $name
+    # the default for enabled in module description is "true", but the actual COM object defaults to "false" when created
+    if ($null -ne $enabled) { $new_rule.Enabled = $enabled } else { $new_rule.Enabled = $true }
+    if ($null -ne $description) { $new_rule.Description = $description }
+    if ($null -ne $program -and $program -ne "any") { $new_rule.ApplicationName = $program }
+    if ($null -ne $service -and $program -ne "any") { $new_rule.ServiceName = $service }
+    if ($null -ne $protocol -and $protocol -ne "any") { $new_rule.Protocol = Parse-ProtocolType -protocol $protocol }
+    if ($null -ne $localport -and $localport -ne "any") { $new_rule.LocalPorts = $localport }
+    if ($null -ne $remoteport -and $remoteport -ne "any") { $new_rule.RemotePorts = $remoteport }
+    if ($null -ne $localip -and $localip -ne "any") { $new_rule.LocalAddresses = $localip }
+    if ($null -ne $remoteip -and $remoteip -ne "any") { $new_rule.RemoteAddresses = $remoteip }
+    if ($null -ne $direction) { $new_rule.Direction = Parse-Direction -directionStr $direction }
+    if ($null -ne $action) { $new_rule.Action = Parse-Action -actionStr $action }
+    # Profiles value cannot be a uint32, but the "all profiles" value (0x7FFFFFFF) will often become a uint32, so must cast to [int]
+    if ($null -ne $profiles) { $new_rule.Profiles = [int](Parse-Profiles -profilesList $profiles) }
+    if ($null -ne $interfacetypes -and @(Compare-Object -ReferenceObject $interfacetypes -DifferenceObject @("any")).Count -ne 0) { $new_rule.InterfaceTypes = Parse-InterfaceTypes -interfaceTypes $interfacetypes }
+    if ($null -ne $edge -and $edge -ne "no") {
+        # EdgeTraversalOptions property exists only from Windows 7/Windows Server 2008 R2: https://msdn.microsoft.com/en-us/library/windows/desktop/dd607256(v=vs.85).aspx
+        if ($new_rule | Get-Member -Name 'EdgeTraversalOptions') {
+            $new_rule.EdgeTraversalOptions = Parse-EdgeTraversalOptions -edgeTraversalOptionsStr $edge
+        }
+    }
+    if ($null -ne $security -and $security -ne "notrequired") {
+        # SecureFlags property exists only from Windows 8/Windows Server 2012: https://msdn.microsoft.com/en-us/library/windows/desktop/hh447465(v=vs.85).aspx
+        if ($new_rule | Get-Member -Name 'SecureFlags') {
+            $new_rule.SecureFlags = Parse-SecureFlags -secureFlagsStr $security
+        }
+    }

    $fwPropertiesToCompare = @('Name','Description','Direction','Action','ApplicationName','ServiceName','Enabled','Profiles','LocalAddresses','RemoteAddresses','LocalPorts','RemotePorts','Protocol','InterfaceTypes', 'EdgeTraversalOptions', 'SecureFlags')
+    $userPassedArguments = @($name, $description, $direction, $action, $program, $service, $enabled, $profiles, $localip, $remoteip, $localport, $remoteport, $protocol, $interfacetypes, $edge, $security)

    if ($state -eq "absent") {
-        if ($existingRule -eq $null) {
+        if ($null -eq $existingRule) {
            $result.msg = "Firewall rule '$name' does not exist."
        } else {
            if ($diff_support) {
@ -240,40 +201,42 @@ try {
            $result.msg = "Firewall rule '$name' removed."
        }
    } elseif ($state -eq "present") {
-        if ($existingRule -eq $null) {
+        if ($null -eq $existingRule) {
            if ($diff_support) {
                foreach ($prop in $fwPropertiesToCompare) {
-                    $result.diff.prepared += "+[$($prop)='$($existingRule.$prop)']`n"
+                    $result.diff.prepared += "+[$($prop)='$($new_rule.$prop)']`n"
                }
            }

            if (-not $check_mode) {
-                $fw.Rules.Add($rule)
+                $fw.Rules.Add($new_rule)
            }
            $result.changed = $true
            $result.msg = "Firewall rule '$name' created."
        } else {
-            foreach ($prop in $fwPropertiesToCompare) {
-                if ($existingRule.$prop -ne $rule.$prop) {
-                    if ($diff_support) {
-                        $result.diff.prepared += "-[$($prop)='$($existingRule.$prop)']`n"
-                        $result.diff.prepared += "+[$($prop)='$($rule.$prop)']`n"
-                    }
-
-                    if (-not $check_mode) {
-                        # Profiles value cannot be a uint32, but the "all profiles" value (0x7FFFFFFF) will often become a uint32, so must cast to [int]
-                        # to prevent InvalidCastException under PS5+
-                        If($prop -eq 'Profiles') {
-                            $existingRule.Profiles = [int] $rule.$prop
+            for($i = 0; $i -lt $fwPropertiesToCompare.Length; $i++) {
+                $prop = $fwPropertiesToCompare[$i]
+                if($null -ne $userPassedArguments[$i]) { # only change values the user passes in task definition
+                    if ($existingRule.$prop -ne $new_rule.$prop) {
+                        if ($diff_support) {
+                            $result.diff.prepared += "-[$($prop)='$($existingRule.$prop)']`n"
+                            $result.diff.prepared += "+[$($prop)='$($new_rule.$prop)']`n"
                        }
-                        Else {
-                            $existingRule.$prop = $rule.$prop
+
+                        if (-not $check_mode) {
+                            # Profiles value cannot be a uint32, but the "all profiles" value (0x7FFFFFFF) will often become a uint32, so must cast to [int]
+                            # to prevent InvalidCastException under PS5+
+                            If($prop -eq 'Profiles') {
+                                $existingRule.Profiles = [int] $new_rule.$prop
+                            }
+                            Else {
+                                $existingRule.$prop = $new_rule.$prop
+                            }
                        }
+                        $result.changed = $true
                    }
-                    $result.changed = $true
                }
            }
-
            if ($result.changed) {
                $result.msg = "Firewall rule '$name' changed."
            } else {
--- a/lib/ansible/modules/windows/win_firewall_rule.py
+++ b/lib/ansible/modules/windows/win_firewall_rule.py
@ -20,8 +20,8 @@ options:
  enabled:
    description:
      - Whether this firewall rule is enabled or disabled.
+      - Defaults to C(true) when creating a new rule.
    type: bool
-    default: yes
    aliases: [ enable ]
  state:
    description:
@ -31,20 +31,20 @@ options:
    default: present
  name:
    description:
-      - The rules name.
+      - The rule's display name.
    type: str
    required: yes
  direction:
    description:
      - Whether this rule is for inbound or outbound traffic.
+      - Defaults to C(in) when creating a new rule.
    type: str
-    required: yes
    choices: [ in, out ]
  action:
    description:
      - What to do with the items this rule is for.
+      - Defaults to C(allow) when creating a new rule.
    type: str
-    required: yes
    choices: [ allow, block ]
  description:
    description:
@ -53,39 +53,50 @@ options:
  localip:
    description:
      - The local ip address this rule applies to.
+      - Set to C(any) to apply to all local ip addresses.
+      - Defaults to C(any) when creating a new rule.
    type: str
-    default: any
  remoteip:
    description:
      - The remote ip address/range this rule applies to.
+      - Set to C(any) to apply to all remote ip addresses.
+      - Defaults to C(any) when creating a new rule.
    type: str
-    default: any
  localport:
    description:
      - The local port this rule applies to.
+      - Set to C(any) to apply to all local ports.
+      - Defaults to C(any) when creating a new rule.
    type: str
  remoteport:
    description:
      - The remote port this rule applies to.
+      - Set to C(any) to apply to all remote ports.
+      - Defaults to C(any) when creating a new rule.
    type: str
  program:
    description:
      - The program this rule applies to.
+      - Set to C(any) to apply to all programs.
+      - Defaults to C(any) when creating a new rule.
    type: str
  service:
    description:
      - The service this rule applies to.
+      - Set to C(any) to apply to all services.
+      - Defaults to C(any) when creating a new rule.
    type: str
  protocol:
    description:
      - The protocol this rule applies to.
+      - Set to C(any) to apply to all services.
+      - Defaults to C(any) when creating a new rule.
    type: str
-    default: any
  profiles:
    description:
      - The profile this rule applies to.
+      - Defaults to C(domain,private,public) when creating a new rule.
    type: list
-    default: domain,private,public
    aliases: [ profile ]
  force:
    description:
--- a/test/integration/targets/win_firewall_rule/tasks/main.yml
+++ b/test/integration/targets/win_firewall_rule/tasks/main.yml
@ -95,6 +95,24 @@
    that:
    - change_firewall_rule.changed == true

+- name: Disable firewall rule
+  win_firewall_rule:
+    name: http
+    enabled: no
+
+- name: Get the actual values from the changed firewall rule
+  win_shell: '(New-Object -ComObject HNetCfg.FwPolicy2).Rules | Where-Object { $_.Name -eq "http" } | Foreach-Object { $_.LocalPorts; $_.Enabled; $_.Action; $_.Direction; $_.Protocol }'
+  register: firewall_rule_actual
+
+- name: Ensure that disabling the rule did not change the previous values
+  assert:
+    that:
+      - "firewall_rule_actual.stdout_lines[0] == '80'" # LocalPorts = 80
+      - "firewall_rule_actual.stdout_lines[1] == 'False'" # Enabled = False
+      - "firewall_rule_actual.stdout_lines[2] == '0'" # Action = block
+      - "firewall_rule_actual.stdout_lines[3] == '1'" # Direction = in
+      - "firewall_rule_actual.stdout_lines[4] == '6'" # Protocol = tcp
+
 - name: Add firewall rule when remoteip is range
  win_firewall_rule:
    name: http
--- a/test/sanity/pslint/ignore.txt
+++ b/test/sanity/pslint/ignore.txt
@ -72,7 +72,6 @@ lib/ansible/modules/windows/win_file_version.ps1 PSCustomUseLiteralPath
 lib/ansible/modules/windows/win_find.ps1 PSAvoidTrailingWhitespace
 lib/ansible/modules/windows/win_find.ps1 PSAvoidUsingEmptyCatchBlock
 lib/ansible/modules/windows/win_find.ps1 PSAvoidUsingWMICmdlet
-lib/ansible/modules/windows/win_firewall_rule.ps1 PSAvoidUsingCmdletAliases
 lib/ansible/modules/windows/win_firewall_rule.ps1 PSUseApprovedVerbs
 lib/ansible/modules/windows/win_get_url.ps1 PSUsePSCredentialType  # Credential param can take a base64 encoded string as well as a PSCredential
 lib/ansible/modules/windows/win_hotfix.ps1 PSAvoidTrailingWhitespace