windows_task resource: allow non-system users without password for interactive taskstasks

- Minor changes in `validate_user_and_password` to support this feature
- `validate_interactive_setting` would no longer be needed
- Added test cases
- Added support to latest version of win32-taskscheduler gem (2.0.4)
- Fixes MSYS-924

Signed-off-by: Nimesh <nimesh.patni@msystechnologies.com>

5 files changed, 134 insertions, 40 deletions

diff --git a/Gemfile.lock b/Gemfile.lock
index 4e484b1be9..071a1415d1 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -130,9 +130,9 @@ GEM
       multipart-post (>= 1.2, < 3)
     faraday_middleware (0.12.2)
       faraday (>= 0.7.4, < 1.0)
-    ffi (1.9.25)
-    ffi (1.9.25-x64-mingw32)
-    ffi (1.9.25-x86-mingw32)
+    ffi (1.10.0)
+    ffi (1.10.0-x64-mingw32)
+    ffi (1.10.0-x86-mingw32)
     ffi-win32-extensions (1.0.3)
       ffi
     ffi-yajl (2.3.1)
diff --git a/lib/chef/provider/windows_task.rb b/lib/chef/provider/windows_task.rb
index 0d77d8c939..1d77c6867b 100644
--- a/lib/chef/provider/windows_task.rb
+++ b/lib/chef/provider/windows_task.rb
@@ -145,7 +145,7 @@ class Chef
               task.working_directory = new_resource.cwd if new_resource.cwd
               task.configure_settings(config_settings)
               task.configure_principals(principal_settings)
-              task.set_account_information(new_resource.user, new_resource.password)
+              task.set_account_information(new_resource.user, new_resource.password, new_resource.interactive_enabled)
               task.creator = new_resource.user
               task.description = new_resource.description unless new_resource.description.nil?
               task.activate(new_resource.task_name)
@@ -246,7 +246,7 @@ class Chef
 
         def update_task(task)
           converge_by("#{new_resource} task updated") do
-            task.set_account_information(new_resource.user, new_resource.password)
+            task.set_account_information(new_resource.user, new_resource.password, new_resource.interactive_enabled)
             task.application_name = new_resource.command if new_resource.command
             task.parameters = new_resource.command_arguments if new_resource.command_arguments
             task.working_directory = new_resource.cwd if new_resource.cwd
diff --git a/lib/chef/resource/windows_task.rb b/lib/chef/resource/windows_task.rb
index 5bd6d7e557..f664fc57be 100644
--- a/lib/chef/resource/windows_task.rb
+++ b/lib/chef/resource/windows_task.rb
@@ -143,7 +143,6 @@ class Chef
         validate_start_time(start_time, frequency)
         validate_start_day(start_day, frequency) if start_day
         validate_user_and_password(user, password)
-        validate_interactive_setting(interactive_enabled, password)
         validate_create_frequency_modifier(frequency, frequency_modifier) if frequency_modifier
         validate_create_day(day, frequency, frequency_modifier) if day
         validate_create_months(months, frequency) if months
@@ -226,20 +225,30 @@ class Chef
         end
       end
 
+      # System users will not require a password
+      # Other users will require a password if the task is non-interactive.
+      #
+      # @param [String] user
+      # @param [String] password
+      #
       def validate_user_and_password(user, password)
-        if password_required?(user) && password.nil?
-          raise ArgumentError, "Cannot specify a user other than the system users without specifying a password!. Valid passwordless users: '#{Chef::ReservedNames::Win32::Security::SID::SYSTEM_USER.join("', '")}'"
+        if non_system_user?(user)
+          if password.nil? && !interactive_enabled
+            raise ArgumentError, "Please provide a password or check if this task needs to be interactive! Valid passwordless users are: '#{Chef::ReservedNames::Win32::Security::SID::SYSTEM_USER.join("', '")}'"
+          end
+        else
+          unless password.nil?
+            raise ArgumentError, "Password is not required for system users."
+          end
         end
       end
 
+      # Password is not required for system user and required for non-system user.
       def password_required?(user)
-        return false if user.nil?
-        @password_required ||= !Chef::ReservedNames::Win32::Security::SID.system_user?(user)
+        @password_required ||= (!user.nil? && !Chef::ReservedNames::Win32::Security::SID.system_user?(user))
       end
 
-      def validate_interactive_setting(interactive_enabled, password)
-        raise ArgumentError, "Please provide the password when attempting to set interactive/non-interactive." if interactive_enabled && password.nil?
-      end
+      alias non_system_user? password_required?
 
       def validate_create_frequency_modifier(frequency, frequency_modifier)
         if ([:on_logon, :onstart, :on_idle, :none].include?(frequency)) && ( frequency_modifier != 1)
diff --git a/spec/functional/resource/windows_task_spec.rb b/spec/functional/resource/windows_task_spec.rb
index 9b355d37ed..a2e1163a88 100644
--- a/spec/functional/resource/windows_task_spec.rb
+++ b/spec/functional/resource/windows_task_spec.rb
@@ -1599,26 +1599,70 @@ describe Chef::Resource::WindowsTask, :windows_only do
     end
 
     context "when start_day is passed with frequency :onstart" do
-      it "not raises error" do
+      it "does not raises error" do
         subject.frequency :onstart
         subject.start_day "09/20/2017"
         expect { subject.after_created }.not_to raise_error
       end
     end
 
-    context "when a non-system user is passed without password" do
+    context "when a non system user is passed without password" do
       it "raises error" do
-        subject.user "Administrator"
+        subject.user "USER"
         subject.frequency :onstart
-        expect { subject.after_created }.to raise_error(%q{Cannot specify a user other than the system users without specifying a password!. Valid passwordless users: 'SYSTEM', 'NT AUTHORITY\SYSTEM', 'LOCAL SERVICE', 'NT AUTHORITY\LOCAL SERVICE', 'NETWORK SERVICE', 'NT AUTHORITY\NETWORK SERVICE', 'ADMINISTRATORS', 'BUILTIN\ADMINISTRATORS', 'USERS', 'BUILTIN\USERS', 'GUESTS', 'BUILTIN\GUESTS'})
+        expect { subject.after_created }.to raise_error(%q{Please provide a password or check if this task needs to be interactive! Valid passwordless users are: 'SYSTEM', 'NT AUTHORITY\SYSTEM', 'LOCAL SERVICE', 'NT AUTHORITY\LOCAL SERVICE', 'NETWORK SERVICE', 'NT AUTHORITY\NETWORK SERVICE', 'ADMINISTRATORS', 'BUILTIN\ADMINISTRATORS', 'USERS', 'BUILTIN\USERS', 'GUESTS', 'BUILTIN\GUESTS'})
+      end
+      it "does not raises error when task is interactive" do
+        subject.user "USER"
+        subject.frequency :onstart
+        subject.interactive_enabled true
+        expect { subject.after_created }.not_to raise_error
       end
     end
 
-    context "when interactive_enabled is passed for a System user without password" do
-      it "raises error" do
+    context "when a system user is passed without password" do
+      it "does not raises error" do
+        subject.user "ADMINISTRATORS"
+        subject.frequency :onstart
+        expect { subject.after_created }.not_to raise_error
+      end
+      it "does not raises error when task is interactive" do
+        subject.user "ADMINISTRATORS"
+        subject.frequency :onstart
+        subject.interactive_enabled true
+        expect { subject.after_created }.not_to raise_error
+      end
+    end
+
+    context "when a non system user is passed with password" do
+      it "does not raises error" do
+        subject.user "USER"
+        subject.password "XXXX"
+        subject.frequency :onstart
+        expect { subject.after_created }.not_to raise_error
+      end
+      it "does not raises error when task is interactive" do
+        subject.user "USER"
+        subject.password "XXXX"
+        subject.frequency :onstart
         subject.interactive_enabled true
+        expect { subject.after_created }.not_to raise_error
+      end
+    end
+
+    context "when a system user is passed with password" do
+      it "raises error" do
+        subject.user "ADMINISTRATORS"
+        subject.password "XXXX"
         subject.frequency :onstart
-        expect { subject.after_created }.to raise_error("Please provide the password when attempting to set interactive/non-interactive.")
+        expect { subject.after_created }.to raise_error("Password is not required for system users.")
+      end
+      it "raises error when task is interactive" do
+        subject.user "ADMINISTRATORS"
+        subject.password "XXXX"
+        subject.frequency :onstart
+        subject.interactive_enabled true
+        expect { subject.after_created }.to raise_error("Password is not required for system users.")
       end
     end
 
diff --git a/spec/unit/resource/windows_task_spec.rb b/spec/unit/resource/windows_task_spec.rb
index efbc1b1846..96482d3d56 100644
--- a/spec/unit/resource/windows_task_spec.rb
+++ b/spec/unit/resource/windows_task_spec.rb
@@ -67,23 +67,70 @@ describe Chef::Resource::WindowsTask, :windows_only do
     end
   end
 
-  context "when user is set but password is not" do
-    before do
-      resource.frequency :hourly
-    end
-    it "raises an error if the user is a non-system user" do
-      resource.user "bob"
-      expect { resource.after_created }.to raise_error(ArgumentError, %q{Cannot specify a user other than the system users without specifying a password!. Valid passwordless users: 'SYSTEM', 'NT AUTHORITY\SYSTEM', 'LOCAL SERVICE', 'NT AUTHORITY\LOCAL SERVICE', 'NETWORK SERVICE', 'NT AUTHORITY\NETWORK SERVICE', 'ADMINISTRATORS', 'BUILTIN\ADMINISTRATORS', 'USERS', 'BUILTIN\USERS', 'GUESTS', 'BUILTIN\GUESTS'})
-    end
+  describe "#validate_user_and_password" do
+    context "a System User" do
+      before do
+        resource.frequency :hourly
+        resource.user 'NT AUTHORITY\SYSTEM'
+      end
 
-    it "does not raise an error if the user is a system user" do
-      resource.user 'NT AUTHORITY\SYSTEM'
-      expect { resource.after_created }.to_not raise_error
+      context "for an interactive task" do
+        before { resource.interactive_enabled true }
+        it "does not require a password" do
+          expect { resource.after_created }.to_not raise_error
+        end
+        it "raises an error when a password is given" do
+          resource.password "XXXX"
+          expect { resource.after_created }.to raise_error(ArgumentError, "Password is not required for system users.")
+        end
+        it "does not raises an error even when user is in lowercase" do
+          resource.user 'nt authority\system'
+          expect { resource.after_created }.to_not raise_error
+        end
+      end
+
+      context "for a non-interactive task" do
+        before { resource.interactive_enabled false }
+        it "does not require a password" do
+          expect { resource.after_created }.to_not raise_error
+        end
+        it "raises an error when a password is given" do
+          resource.password "XXXX"
+          expect { resource.after_created }.to raise_error(ArgumentError, "Password is not required for system users.")
+        end
+        it "does not raises an error even when user is in lowercase" do
+          resource.user 'nt authority\system'
+          expect { resource.after_created }.to_not raise_error
+        end
+      end
     end
 
-    it "does not raise an error if the user is a system user even if lowercase" do
-      resource.user 'nt authority\system'
-      expect { resource.after_created }.to_not raise_error
+    context "a Non-System User" do
+      before do
+        resource.frequency :hourly
+        resource.user "bob"
+      end
+      context "for an interactive task" do
+        before { resource.interactive_enabled true }
+        it "does not require a password" do
+          expect { resource.after_created }.to_not raise_error
+        end
+        it "does not raises an error when a password is given" do
+          resource.password "XXXX"
+          expect { resource.after_created }.to_not raise_error
+        end
+      end
+
+      context "for a non-interactive task" do
+        before { resource.interactive_enabled false }
+        it "require a password" do
+          expect { resource.after_created }.to raise_error(ArgumentError, %q{Please provide a password or check if this task needs to be interactive! Valid passwordless users are: 'SYSTEM', 'NT AUTHORITY\SYSTEM', 'LOCAL SERVICE', 'NT AUTHORITY\LOCAL SERVICE', 'NETWORK SERVICE', 'NT AUTHORITY\NETWORK SERVICE', 'ADMINISTRATORS', 'BUILTIN\ADMINISTRATORS', 'USERS', 'BUILTIN\USERS', 'GUESTS', 'BUILTIN\GUESTS'})
+        end
+        it "does not raises an error when a password is given" do
+          resource.password "XXXX"
+          expect { resource.after_created }.to_not raise_error
+        end
+      end
     end
   end
 
@@ -231,12 +278,6 @@ describe Chef::Resource::WindowsTask, :windows_only do
     end
   end
 
-  context "#validate_interactive_setting" do
-    it "raises error when interactive_enabled is passed without password" do
-      expect { resource.send(:validate_interactive_setting, true, nil) }.to raise_error("Please provide the password when attempting to set interactive/non-interactive.")
-    end
-  end
-
   context "#validate_create_frequency_modifier" do
     context "when frequency is :minute" do
       it "raises error if frequency_modifier > 1439" do