here's a rewrite that's hopefully more inline with what @tas50 was thinking in the previous rounds of comments.

Signed-off-by: Davin Taddeo <davin@chef.io>

1 files changed, 107 insertions, 102 deletions

diff --git a/lib/chef/resource/windows_firewall_profile.rb b/lib/chef/resource/windows_firewall_profile.rb
index 83ac1b41a8..cc6feb0d67 100644
--- a/lib/chef/resource/windows_firewall_profile.rb
+++ b/lib/chef/resource/windows_firewall_profile.rb
@@ -29,12 +29,11 @@ class Chef
       ```ruby
       windows_firewall_profile 'Configure and Enable Windows Firewall Private Profile' do
         profiles 'Private'
-        profile_enabled true
-        default_inbound_block true
-        default_outbound_allow true
-        allow_inbound_rules true
-        display_notification false
-        action :configure
+        default_inbound_action 'True'
+        default_outbound_action 'True'
+        allow_inbound_rules 'True'
+        display_notification 'False'
+        action :enable
       end
       ```
 
@@ -43,124 +42,130 @@ class Chef
       ```ruby
       windows_firewall_profile 'Configure and Enable Windows Firewall Public Profile' do
         profile 'Public'
-        profile_enabled true
-        default_inbound_block true
-        default_outbound_allow true
-        allow_inbound_rules false
-        display_notification false
-        action :configure
+        default_inbound_action 'True'
+        default_outbound_action 'True'
+        allow_inbound_rules 'False'
+        display_notification 'False'
+        action :enable
       end
       ```
+
       **Disable the Domain Profile of the Windows Firewall**:
 
       ```ruby
       windows_firewall_profile 'Disable the Domain Profile of the Windows Firewall' do
         profile 'Domain'
-        profile_enabled false
-        action :configure
+        action :disable
       end
       ```
       DOC
 
       unified_mode true
 
-      property :profile, String, required: true, equal_to: %w{ Domain Public Private }, description: "Set the Windows Profile being configured"
-      property :profile_enabled, [true, false], default: true, description: "Set the status of the firewall profile to Enabled or Disabled"
-      property :default_inbound_block, [true, false, nil], default: true, description: "Set the default policy for inbound network traffic to blocked"
-      property :default_outbound_allow, [true, false, nil], default: true, description: "Set the default policy for outbound network traffic to allowed"
-      property :allow_inbound_rules, [true, false, nil], description: "Allow users to set inbound firewall rules"
-      property :allow_local_firewall_rules, [true, false, nil], description: "Merges inbound firewall rules into the policy"
-      property :allow_local_ipsec_rules, [true, false, nil], description: "Allow users to manage local connection security rules"
-      property :allow_user_apps, [true, false, nil], description: "Allow user applications to manage firewall"
-      property :allow_user_ports, [true, false, nil], description: "Allow users to manage firewall port rules"
-      property :allow_unicast_response, [true, false, nil], description: "Allow unicast responses to multicast and broadcast messages"
-      property :display_notification, [true, false, nil], description: "Display a notification when firewall blocks certain activity"
+      property :profile, String,
+        name_property: true,
+        equal_to: %w{ Domain Public Private },
+        description: "Set the Windows Profile being configured"
+
+      property :default_inbound_action, [String, nil],
+        equal_to: %w{ Allow Block NotConfigured },
+        description: "Set the default policy for inbound network traffic"
+
+      property :default_outbound_action, [String, nil],
+        equal_to: %w{ Allow Block NotConfigured },
+        description: "Set the default policy for outbound network traffic"
+
+      property :allow_inbound_rules, String, equal_to: %w{ True False NotConfigured }, description: "Allow users to set inbound firewall rules"
+      property :allow_local_firewall_rules, String, equal_to: %w{ True False NotConfigured },description: "Merges inbound firewall rules into the policy"
+      property :allow_local_ipsec_rules, String, equal_to: %w{ True False NotConfigured }, description: "Allow users to manage local connection security rules"
+      property :allow_user_apps, String, equal_to: %w{ True False NotConfigured }, description: "Allow user applications to manage firewall"
+      property :allow_user_ports, String, equal_to: %w{ True False NotConfigured }, description: "Allow users to manage firewall port rules"
+      property :allow_unicast_response, String, equal_to: %w{ True False NotConfigured }, description: "Allow unicast responses to multicast and broadcast messages"
+      property :display_notification, String, equal_to: %w{ True False NotConfigured }, description: "Display a notification when firewall blocks certain activity"
 
       load_current_value do |desired|
-        ps_results = powershell_exec(<<-CODE).result
-          $#{desired.profile} = Get-NetFirewallProfile -Profile #{desired.profile}
-          $#{desired.profile}.Enabled
-          $#{desired.profile}.DefaultInboundAction
-          $#{desired.profile}.DefaultOutboundAction
-          $#{desired.profile}.AllowInboundRules
-          $#{desired.profile}.AllowLocalFirewallRules
-          $#{desired.profile}.AllowLocalIPsecRules
-          $#{desired.profile}.AllowUserApps
-          $#{desired.profile}.AllowUserPorts
-          $#{desired.profile}.AllowUnicastResponseToMulticast
-          $#{desired.profile}.NotifyOnListen
-        CODE
-        profile_enabled case ps_results[0]; when 0 then false; when 1 then true; end
-        default_inbound_block case ps_results[1]; when 4 then true; when 2 then false; when 0 then nil; end
-        default_outbound_allow case ps_results[2]; when 4 then false; when 2 then true; when 0 then nil; end
-        allow_inbound_rules case ps_results[3]; when 0 then false; when 1 then true; when 2 then nil; end
-        allow_local_firewall_rules case ps_results[4]; when 0 then false; when 1 then true; when 2 then nil; end
-        allow_local_ipsec_rules case ps_results[5]; when 0 then false; when 1 then true; when 2 then nil; end
-        allow_user_apps case ps_results[6]; when 0 then false; when 1 then true; when 2 then nil; end
-        allow_user_ports case ps_results[7]; when 0 then false; when 1 then true; when 2 then nil; end
-        allow_unicast_response case ps_results[8]; when 0 then false; when 1 then true; when 2 then nil; end
-        display_notification case ps_results[9]; when 0 then false; when 1 then true; when 2 then nil; end
+        ps_get_net_fw_profile = load_firewall_state(desired.profile)
+        output = powershell_out(ps_get_net_fw_profile)
+        if output.stdout.empty?
+          current_value_does_not_exist!
+        else
+          state = Chef::JSONCompat.from_json(output.stdout)
+        end
+
+        default_inbound_action state["default_inbound_action"]
+        default_outbound_action state["default_outbound_action"]
+        allow_inbound_rules state["allow_inbound_rules"]
+        allow_local_firewall_rules state["allow_local_firewall_rules"]
+        allow_local_ipsec_rules state["allow_local_ipsec_rules"]
+        allow_user_apps state["allow_user_apps"]
+        allow_user_ports state["allow_user_ports"]
+        allow_unicast_response state["allow_unicast_response"]
+        display_notification state["display_notification"]
       end
 
-      action :configure do
-        converge_if_changed :profile_enabled do
-          cmd = "Set-NetFirewallProfile -Profile #{new_resource.profile} "
-          cmd += "-Enabled #{new_resource.profile_enabled ? "True" : "False"} "
-          powershell_exec(cmd)
-        end
-        converge_if_changed :default_inbound_block do
-          cmd = "Set-NetFirewallProfile -Profile #{new_resource.profile} "
-          cmd += "-DefaultInboundAction NotConfigured " if new_resource.default_inbound_block.nil?
-          cmd += "-DefaultInboundAction #{new_resource.default_inbound_block ? "Block" : "Allow"} " unless new_resource.default_inbound_block.nil?
-          powershell_exec(cmd)
-        end
-        converge_if_changed :default_outbound_allow do
-          cmd = "Set-NetFirewallProfile -Profile #{new_resource.profile} "
-          cmd += "-DefaultOutboundAction NotConfigured " if new_resource.default_outbound_allow.nil?
-          cmd += "-DefaultOutboundAction #{new_resource.default_outbound_allow ? "Allow" : "Block"} " unless new_resource.default_outbound_allow.nil?
-          powershell_exec(cmd)
+      action :enable do
+        converge_if_changed :default_inbound_action, :default_outbound_action, :allow_inbound_rules, :allow_local_firewall_rules,
+          :allow_local_ipsec_rules, :allow_user_apps, :allow_user_ports, :allow_unicast_response, :display_notification do
+          fw_cmd = firewall_command(new_resource.profile)
+          powershell_exec!(fw_cmd)
         end
-        converge_if_changed :allow_inbound_rules do
-          cmd = "Set-NetFirewallProfile -Profile #{new_resource.profile} "
-          cmd += "-AllowInboundRules NotConfigured " if new_resource.allow_inbound_rules.nil?
-          cmd += "-AllowInboundRules #{new_resource.allow_inbound_rules ? "True" : "False"} " unless new_resource.allow_inbound_rules.nil?
-          powershell_exec(cmd)
+        unless firewall_enabled?(new_resource.profile)
+          converge_by "Enable the #{new_resource.profile} Firewall Profile" do
+            cmd = "Set-NetFirewallProfile -Profile #{new_resource.profile} -Enabled \"True\""
+            powershell_exec!(cmd)
+          end
         end
-        converge_if_changed :allow_local_firewall_rules do
-          cmd = "Set-NetFirewallProfile -Profile #{new_resource.profile} "
-          cmd += "-AllowLocalFirewallRules NotConfigured " if new_resource.allow_local_firewall_rules.nil?
-          cmd += "-AllowLocalFirewallRules #{new_resource.allow_local_firewall_rules ? "True" : "False"} " unless new_resource.allow_local_firewall_rules.nil?
-          powershell_exec(cmd)
-        end
-        converge_if_changed :allow_local_ipsec_rules do
-          cmd = "Set-NetFirewallProfile -Profile #{new_resource.profile} "
-          cmd += "-AllowLocalIPsecRules NotConfigured " if new_resource.allow_local_ipsec_rules.nil?
-          cmd += "-AllowLocalIPsecRules #{new_resource.allow_local_ipsec_rules ? "True" : "False"} " unless new_resource.allow_local_ipsec_rules.nil?
-          powershell_exec(cmd)
-        end
-        converge_if_changed :allow_user_apps do
-          cmd = "Set-NetFirewallProfile -Profile #{new_resource.profile} "
-          cmd += "-AllowUserApps NotConfigured " if new_resource.allow_user_apps.nil?
-          cmd += "-AllowUserApps #{new_resource.allow_user_apps ? "True" : "False"} " unless new_resource.allow_user_apps.nil?
-          powershell_exec(cmd)
+      end
+
+      action :disable do
+        if firewall_enabled?(new_resource.profile)
+          converge_by "Disable the #{new_resource.profile} Firewall Profile" do
+            cmd = "Set-NetFirewallProfile -Profile #{new_resource.profile} -Enabled \"False\""
+            powershell_exec(cmd)
+          end
         end
-        converge_if_changed :allow_user_ports do
-          cmd = "Set-NetFirewallProfile -Profile #{new_resource.profile} "
-          cmd += "-AllowUserPorts NotConfigured " if new_resource.allow_user_ports.nil?
-          cmd += "-AllowUserPorts #{new_resource.allow_user_ports ? "True" : "False"} " unless new_resource.allow_user_ports.nil?
-          powershell_exec(cmd)
+      end
+
+      action_class do
+        def firewall_command(fw_profile)
+          cmd = "Set-NetFirewallProfile -Profile \"#{fw_profile}\""
+          cmd << " -DefaultInboundAction \"#{new_resource.default_inbound_action}\"" unless new_resource.default_inbound_action.nil?
+          cmd << " -DefaultOutboundAction \"#{new_resource.default_outbound_action}\"" unless new_resource.default_outbound_action.nil?
+          cmd << " -AllowInboundRules \"#{new_resource.allow_inbound_rules}\"" unless new_resource.allow_inbound_rules.nil?
+          cmd << " -AllowLocalFirewallRules \"#{new_resource.allow_local_firewall_rules}\""
+          cmd << " -AllowLocalIPsecRules \"#{new_resource.allow_local_ipsec_rules}\"" unless new_resource.allow_local_ipsec_rules.nil?
+          cmd << " -AllowUserApps \"#{new_resource.allow_user_apps}\"" unless new_resource.allow_user_apps.nil?
+          cmd << " -AllowUserPorts \"#{new_resource.allow_user_ports}\"" unless new_resource.allow_user_ports.nil?
+          cmd << " -AllowUnicastResponseToMulticast \"#{new_resource.allow_unicast_response}\"" unless new_resource.allow_unicast_response.nil?
+          cmd << " -NotifyOnListen \"#{new_resource.display_notification}\"" unless new_resource.display_notification.nil?
+          cmd
         end
-        converge_if_changed :allow_unicast_response do
-          cmd = "Set-NetFirewallProfile -Profile #{new_resource.profile} "
-          cmd += "-AllowUnicastResponseToMulticast NotConfigured " if new_resource.allow_unicast_response.nil?
-          cmd += "-AllowUnicastResponseToMulticast #{new_resource.allow_unicast_response ? "True" : "False"} " unless new_resource.allow_unicast_response.nil?
-          powershell_exec(cmd)
+
+        def load_firewall_state(profile_name)
+          <<-EOH
+            Remove-TypeData System.Array # workaround for PS bug here: https://bit.ly/2SRMQ8M
+            $#{profile_name} = Get-NetFirewallProfile -Profile #{profile_name}
+            ([PSCustomObject]@{
+              default_inbound_action = $#{profile_name}.DefaultInboundAction.ToString()
+              default_outbound_action = $#{profile_name}.DefaultOutboundAction.ToString()
+              allow_inbound_rules = $#{profile_name}.AllowInboundRules.ToString()
+              allow_local_firewall_rules = $#{profile_name}.AllowLocalFirewallRules.ToString()
+              allow_local_ipsec_rules = $#{profile_name}.AllowLocalIPsecRules.ToString()
+              allow_user_apps = $#{profile_name}.AllowUserApps.ToString()
+              allow_user_ports = $#{profile_name}.AllowUserPorts.ToString()
+              allow_unicast_response = $#{profile_name}.AllowUnicastResponseToMulticast.ToString()
+              display_notification = $#{profile_name}.NotifyOnListen.ToString()
+            }) | ConvertTo-Json
+          EOH
         end
-        converge_if_changed :display_notification do
-          cmd = "Set-NetFirewallProfile -Profile #{new_resource.profile} "
-          cmd += "-NotifyOnListen NotConfigured " if new_resource.display_notification.nil?
-          cmd += "-NotifyOnListen #{new_resource.display_notification ? "True" : "False"} " unless new_resource.display_notification.nil?
-          powershell_exec(cmd)
+
+        def firewall_enabled?(profile_name)
+          powershell_exec(<<-CODE).result
+            $#{profile_name} = Get-NetFirewallProfile -Profile #{profile_name}
+            if ($#{profile_name}.Enabled) {
+                return $true
+            } else {return $false}
+          CODE
         end
       end
     end