Add functional spec and review comment

Signed-off-by: NAshwini <ashwini.nehate@msystechnologies.com>

2 files changed, 124 insertions, 27 deletions

diff --git a/lib/chef/resource/windows_security_policy.rb b/lib/chef/resource/windows_security_policy.rb
index aae32950bd..7623d50355 100644
--- a/lib/chef/resource/windows_security_policy.rb
+++ b/lib/chef/resource/windows_security_policy.rb
@@ -17,15 +17,15 @@
 # limitations under the License.
 
 require_relative "../resource"
-require_relative "../mixin/powershell_out"
 
 class Chef
   class Resource
     class WindowsSecurityPolicy < Chef::Resource
-      include Chef::Mixin::PowershellOut
       resource_name :windows_security_policy
 
-      policy_names = %w(MinimumPasswordAge
+      # The valid policy_names options found here
+      # https://github.com/ChrisAWalker/cSecurityOptions under 'AccountSettings'
+      policy_names = %w{MinimumPasswordAge
                 MaximumPasswordAge
                 MinimumPasswordLength
                 PasswordComplexity
@@ -39,39 +39,46 @@ class Chef
                 LSAAnonymousNameLookup
                 EnableAdminAccount
                 EnableGuestAccount
-                )
+                }
+      description "Use the windows_security_policy resource to set a security policy on the Microsoft Windows platform."
+      introduced "16.0"
 
-      property :id, String, name_property: true, equal_to: policy_names
-      property :secoption, String, required: false, equal_to: policy_names
-      property :secvalue, String, required: true
-      property :sensitive, [true, false], default: true
-     
-      default_action :set
+      property :id, String, name_property: true, equal_to: policy_names,
+      description: "The name of the policy to be set on windows platform to maintain its security."
 
-      action :set do
-        new_resource.secoption.nil? ? (security_option = new_resource.id) : (security_option = new_resource.secoption)
+      property :secoption, String, equal_to: policy_names,
+      description: "An optional property for the name of the policy."
+
+      property :secvalue, String, required: true,
+      description: "Policy value to be set for policy name."
 
-        psversion = node['languages']['powershell']['version'].to_i
+      property :sensitive, [true, false], default: true,
+      description: "Ensure that sensitive resource data is not logged by Chef Infra Client.",
+      default_description: "true"
+
+      action :set do
+        security_option = new_resource.secoption.nil? ? new_resource.id : new_resource.secoption
+        psversion = node["languages"]["powershell"]["version"].to_i
         if psversion >= 5
-          if powershell_out!('(Get-PackageSource -Name PSGallery -WarningAction SilentlyContinue).name').stdout.empty? || powershell_out!('(Get-Package -Name cSecurityOptions -WarningAction SilentlyContinue).name').stdout.empty?
+          if powershell_out!("(Get-PackageSource -Name PSGallery -WarningAction SilentlyContinue).name").stdout.empty? || powershell_out!("(Get-Package -Name cSecurityOptions -WarningAction SilentlyContinue).name").stdout.empty?
             raise "This resource needs Powershell module cSecurityOptions to be installed. \n Please install it and then re-run the recipe. \n https://www.powershellgallery.com/packages/cSecurityOptions/3.1.3"
           end
-      
+
           sec_hash = {
             security_option => new_resource.secvalue,
           }
-          dsc_resource 'AccountSettings' do
-            module_name 'cSecurityOptions'
+          dsc_resource "AccountSettings" do
+            module_name "cSecurityOptions"
             resource :AccountAndBasicAuditing
-            property :Enable, '$true'
+            property :Enable, "$true"
             property :AccountAndBasicAuditing, sec_hash
             sensitive new_resource.sensitive
           end
-        elsif security_option == ('NewAdministratorName' || 'NewGuestName')
+        elsif security_option == ("NewAdministratorName" || "NewGuestName")
           desiredname = new_resource.secvalue
 
-          uid = '500' if security_option == 'NewAdministratorName'
-          uid = '501' if security_option == 'NewGuestName'
+          uid = "500" if security_option == "NewAdministratorName"
+          uid = "501" if security_option == "NewGuestName"
 
           powershell_script security_option do
             code <<-EOH
@@ -80,11 +87,11 @@ class Chef
                   $Administrator = Get-WmiObject -query "Select * From Win32_UserAccount Where LocalAccount = TRUE AND SID LIKE 'S-1-5%-#{uid}'"
                   $Administrator.rename("#{desiredname}")
                 }
-              EOH
+            EOH
             guard_interpreter :powershell_script
-              only_if <<-EOH
-                  Get-WMIObject -Class Win32_Account -Filter "LocalAccount = True And SID Like '%#{uid}%'").Name -ne '#{desiredname}')
-                EOH
+            only_if <<-EOH
+              Get-WMIObject -Class Win32_Account -Filter "LocalAccount = True And SID Like '%#{uid}%'").Name -ne '#{desiredname}')
+            EOH
           end
         else
           security_value = new_resource.secvalue
@@ -93,7 +100,7 @@ class Chef
             code <<-EOH
               $#{security_option}_Export = secedit /export /cfg 'c:\\temp\\#{security_option}_Export.inf'
               $#{security_option}_ExportAudit = (Get-Content c:\\temp\\#{security_option}_Export.inf | Select-String -Pattern #{security_option})
-              if ($#{security_option}_ExportAudit -match '#{security_option} = #{security_value}') 
+              if ($#{security_option}_ExportAudit -match '#{security_option} = #{security_value}')
                 { Remove-Item 'c:\\temp\\#{security_option}_Export.inf' -force }
               else
                 {
@@ -108,4 +115,3 @@ class Chef
     end
   end
 end
-
diff --git a/spec/functional/resource/windows_security_policy_spec.rb b/spec/functional/resource/windows_security_policy_spec.rb
new file mode 100644
index 0000000000..9950e511e2
--- /dev/null
+++ b/spec/functional/resource/windows_security_policy_spec.rb
@@ -0,0 +1,91 @@
+#
+# Author:: Ashwini Nehate (<anehate@chef.io>)
+# Copyright:: Copyright 2019-2020, Chef Software, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require "spec_helper"
+require "functional/resource/base"
+require "chef/mixin/powershell_out"
+
+describe Chef::Resource::WindowsSecurityPolicy, :windows_only do
+  include Chef::Mixin::PowershellOut
+  before(:all) {
+    powershell_out!("Install-Module -Name cSecurityOptions -Force") if powershell_out!("(Get-Package -Name cSecurityOptions -WarningAction SilentlyContinue).name").stdout.empty?
+  }
+
+  let(:id) { "MaximumPasswordAge" }
+  let(:secvalue) { "30" }
+  let(:windows_test_run_context) do
+    node = Chef::Node.new
+    node.consume_external_attrs(OHAI_SYSTEM.data, {}) # node[:languages][:powershell][:version]
+    node.automatic["os"] = "windows"
+    node.automatic["platform"] = "windows"
+    node.automatic["platform_version"] = "6.1"
+    node.automatic["kernel"][:machine] = :x86_64 # Only 64-bit architecture is supported
+    empty_events = Chef::EventDispatch::Dispatcher.new
+    Chef::RunContext.new(node, {}, empty_events)
+  end
+
+  subject do
+    new_resource = Chef::Resource::WindowsSecurityPolicy.new(id, windows_test_run_context)
+    new_resource.id = id
+    new_resource.secvalue = secvalue
+    new_resource
+  end
+
+  describe "Set MaximumPasswordAge Policy" do
+    after {
+      subject.secvalue("60")
+      subject.run_action(:set)
+    }
+
+    it "should set MaximumPasswordAge to 30" do
+      subject.secvalue("30")
+      subject.run_action(:set)
+      expect(subject).to be_updated_by_last_action
+    end
+
+    it "should be idempotent" do
+      subject.run_action(:set)
+      subject.run_action(:set)
+      expect(subject).not_to be_updated_by_last_action
+    end
+  end
+
+  describe "secoption and id: " do
+    it "accepts 'MinimumPasswordAge', 'MinimumPasswordAge', 'MaximumPasswordAge', 'MinimumPasswordLength', 'PasswordComplexity', 'PasswordHistorySize', 'LockoutBadCount', 'RequireLogonToChangePassword', 'ForceLogoffWhenHourExpire', 'NewAdministratorName', 'NewGuestName', 'ClearTextPassword', 'LSAAnonymousNameLookup', 'EnableAdminAccount', 'EnableGuestAccount' " do
+      expect { subject.id("MinimumPasswordAge") }.not_to raise_error
+      expect { subject.id("MaximumPasswordAge") }.not_to raise_error
+      expect { subject.id("MinimumPasswordLength") }.not_to raise_error
+      expect { subject.id("PasswordComplexity") }.not_to raise_error
+      expect { subject.id("PasswordHistorySize") }.not_to raise_error
+      expect { subject.id("LockoutBadCount") }.not_to raise_error
+      expect { subject.id("RequireLogonToChangePassword") }.not_to raise_error
+      expect { subject.id("ForceLogoffWhenHourExpire") }.not_to raise_error
+      expect { subject.id("NewAdministratorName") }.not_to raise_error
+      expect { subject.id("NewGuestName") }.not_to raise_error
+      expect { subject.id("ClearTextPassword") }.not_to raise_error
+      expect { subject.id("LSAAnonymousNameLookup") }.not_to raise_error
+      expect { subject.id("EnableAdminAccount") }.not_to raise_error
+      expect { subject.id("EnableGuestAccount") }.not_to raise_error
+    end
+
+    it "rejects any other option" do
+      expect { subject.id "XYZ" }.to raise_error(ArgumentError)
+      expect { subject.secoption "ABC" }.to raise_error(ArgumentError)
+    end
+  end
+end