Add support for localized system account to windows_task resource

Signed-off-by: Anton Kvashenkin <anton.jugatsu@gmail.com>

6 files changed, 292 insertions, 241 deletions

diff --git a/lib/chef/provider/windows_task.rb b/lib/chef/provider/windows_task.rb
index 98dd8795fa..d2ac9fdc59 100644
--- a/lib/chef/provider/windows_task.rb
+++ b/lib/chef/provider/windows_task.rb
@@ -570,7 +570,16 @@ class Chef
         def logon_type
           # Ref: https://msdn.microsoft.com/en-us/library/windows/desktop/aa383566(v=vs.85).aspx
           # if nothing is passed as logon_type the TASK_LOGON_SERVICE_ACCOUNT is getting set as default so using that for comparision.
-          new_resource.password.nil? ? TaskScheduler::TASK_LOGON_SERVICE_ACCOUNT : TaskScheduler::TASK_LOGON_PASSWORD
+          user_id = new_resource.user
+          if Chef::ReservedNames::Win32::Security::SID.service_account_user?(user_id)
+            TaskScheduler::TASK_LOGON_SERVICE_ACCOUNT
+          elsif Chef::ReservedNames::Win32::Security::SID.group_user?(user_id)
+            TaskScheduler::TASK_LOGON_GROUP
+          elsif !new_resource.password.to_s.empty? # password is present
+            TaskScheduler::TASK_LOGON_PASSWORD
+          else
+            TaskScheduler::TASK_LOGON_INTERACTIVE_TOKEN
+          end
         end
 
         # This method checks if task and command properties exist since those two are mandatory properties to create a schedules task.
diff --git a/lib/chef/resource/windows_task.rb b/lib/chef/resource/windows_task.rb
index 0232fe3064..405846cb9e 100644
--- a/lib/chef/resource/windows_task.rb
+++ b/lib/chef/resource/windows_task.rb
@@ -17,318 +17,320 @@
 #
 
 require "chef/resource"
+require "chef/win32/security" if Chef::Platform.windows?
 
 class Chef
   class Resource
     class WindowsTask < Chef::Resource
-      resource_name :windows_task
-      provides(:windows_task) { true }
+      if Chef::Platform.windows?
+        resource_name :windows_task
+        provides(:windows_task) { true }
 
-      description "Use the windows_task resource to create, delete or run a Windows scheduled task. Requires Windows Server 2008 or later due to API usage."
-      introduced "13.0"
+        description "Use the windows_task resource to create, delete or run a Windows scheduled task. Requires Windows Server 2008 or later due to API usage."
+        introduced "13.0"
 
-      allowed_actions :create, :delete, :run, :end, :enable, :disable, :change
-      default_action :create
+        allowed_actions :create, :delete, :run, :end, :enable, :disable, :change
+        default_action :create
 
-      property :task_name, String, regex: [/\A[^\/\:\*\?\<\>\|]+\z/],
-               description: "The task name, such as 'Task Name' or '/Task Name'",
-               name_property: true
+        property :task_name, String, regex: [/\A[^\/\:\*\?\<\>\|]+\z/],
+                 description: "The task name, such as 'Task Name' or '/Task Name'",
+                 name_property: true
 
-      property :command, String,
-               description: "The command to be executed by the windows scheduled task."
+        property :command, String,
+                 description: "The command to be executed by the windows scheduled task."
 
-      property :cwd, String,
-               description: "The directory the task will be run from."
+        property :cwd, String,
+                 description: "The directory the task will be run from."
 
-      property :user, String,
-               description: "The user to run the task as.",
-               default: "SYSTEM"
+        property :user, String,
+                 description: "The user to run the task as.",
+                 default: Chef::ReservedNames::Win32::Security::SID.LocalSystem.account_simple_name
 
-      property :password, String,
-               description: "The user’s password. The user property must be set if using this property."
+        property :password, String,
+                 description: "The user’s password. The user property must be set if using this property."
 
-      property :run_level, Symbol, equal_to: [:highest, :limited],
-               description: "Run with ':limited' or ':highest' privileges.",
-               default: :limited
+        property :run_level, Symbol, equal_to: [:highest, :limited],
+                 description: "Run with ':limited' or ':highest' privileges.",
+                 default: :limited
 
-      property :force, [TrueClass, FalseClass],
-               description: "When used with create, will update the task.",
-               default: false
+        property :force, [TrueClass, FalseClass],
+                 description: "When used with create, will update the task.",
+                 default: false
 
-      property :interactive_enabled, [TrueClass, FalseClass],
-               description: "Allow task to run interactively or non-interactively. Requires user and password to also be set.",
-               default: false
+        property :interactive_enabled, [TrueClass, FalseClass],
+                 description: "Allow task to run interactively or non-interactively. Requires user and password to also be set.",
+                 default: false
 
-      property :frequency_modifier, [Integer, String],
-               default: 1
+        property :frequency_modifier, [Integer, String],
+                 default: 1
 
-      property :frequency, Symbol, equal_to: [:minute,
-                                              :hourly,
-                                              :daily,
-                                              :weekly,
-                                              :monthly,
-                                              :once,
-                                              :on_logon,
-                                              :onstart,
-                                              :on_idle,
-                                              :none],
-               description: "The frequency with which to run the task."
+        property :frequency, Symbol, equal_to: [:minute,
+                                                :hourly,
+                                                :daily,
+                                                :weekly,
+                                                :monthly,
+                                                :once,
+                                                :on_logon,
+                                                :onstart,
+                                                :on_idle,
+                                                :none],
+                 description: "The frequency with which to run the task."
 
-      property :start_day, String,
-               description: "Specifies the first date on which the task runs in MM/DD/YYYY format."
+        property :start_day, String,
+                 description: "Specifies the first date on which the task runs in MM/DD/YYYY format."
 
-      property :start_time, String,
-               description: "Specifies the start time to run the task, in HH:mm format."
+        property :start_time, String,
+                 description: "Specifies the start time to run the task, in HH:mm format."
 
-      property :day, [String, Integer],
-               description: "The day(s) on which the task runs."
+        property :day, [String, Integer],
+                 description: "The day(s) on which the task runs."
 
-      property :months, String,
-               description: "The Months of the year on which the task runs, such as: 'JAN, FEB' or '\*'. Multiple months should be comma delimited. e.g. 'Jan, Feb, Mar, Dec'."
+        property :months, String,
+                 description: "The Months of the year on which the task runs, such as: 'JAN, FEB' or '\*'. Multiple months should be comma delimited. e.g. 'Jan, Feb, Mar, Dec'."
 
-      property :idle_time, Integer,
-               description: "For :on_idle frequency, the time (in minutes) without user activity that must pass to trigger the task, from 1 - 999."
+        property :idle_time, Integer,
+                 description: "For :on_idle frequency, the time (in minutes) without user activity that must pass to trigger the task, from 1 - 999."
 
-      property :random_delay, [String, Integer],
-               description: "Delays the task up to a given time (in seconds)."
+        property :random_delay, [String, Integer],
+                 description: "Delays the task up to a given time (in seconds)."
 
-      property :execution_time_limit, [String, Integer],
-               description: "The maximum time (in seconds) the task will run.",
-               default: "PT72H" # 72 hours in ISO8601 duration format
+        property :execution_time_limit, [String, Integer],
+                 description: "The maximum time (in seconds) the task will run.",
+                 default: "PT72H" # 72 hours in ISO8601 duration format
 
-      property :minutes_duration, [String, Integer],
-               description: ""
+        property :minutes_duration, [String, Integer],
+                 description: ""
 
-      property :minutes_interval, [String, Integer],
-               description: ""
+        property :minutes_interval, [String, Integer],
+                 description: ""
 
-      property :priority, Integer,
-               description: "Use to set Priority Levels range from 0 to 10.",
-               default: 7, callbacks: { "should be in range of 0 to 10" => proc { |v| v >= 0 && v <= 10 } }
+        property :priority, Integer,
+                 description: "Use to set Priority Levels range from 0 to 10.",
+                 default: 7, callbacks: { "should be in range of 0 to 10" => proc { |v| v >= 0 && v <= 10 } }
 
-      property :disallow_start_if_on_batteries, [TrueClass, FalseClass],
-               introduced: "14.4", default: false,
-               description: "Disallow start of the task if the system is running on battery power."
+        property :disallow_start_if_on_batteries, [TrueClass, FalseClass],
+                 introduced: "14.4", default: false,
+                 description: "Disallow start of the task if the system is running on battery power."
 
-      property :stop_if_going_on_batteries, [TrueClass, FalseClass],
-               introduced: "14.4", default: false,
-               description: "Scheduled task option when system is switching on battery."
+        property :stop_if_going_on_batteries, [TrueClass, FalseClass],
+                 introduced: "14.4", default: false,
+                 description: "Scheduled task option when system is switching on battery."
 
-      attr_accessor :exists, :task, :command_arguments
+        attr_accessor :exists, :task, :command_arguments
 
-      SYSTEM_USERS = ['NT AUTHORITY\SYSTEM', "SYSTEM", 'NT AUTHORITY\LOCALSERVICE', 'NT AUTHORITY\NETWORKSERVICE', 'BUILTIN\USERS', "USERS"].freeze
-      VALID_WEEK_DAYS = %w{ mon tue wed thu fri sat sun * }.freeze
-      VALID_DAYS_OF_MONTH = ("1".."31").to_a << "last" << "lastday"
-      VALID_MONTHS = %w{JAN FEB MAR APR MAY JUN JUL AUG SEP OCT NOV DEC *}.freeze
-      VALID_WEEKS = %w{FIRST SECOND THIRD FOURTH LAST LASTDAY}.freeze
+        VALID_WEEK_DAYS = %w{ mon tue wed thu fri sat sun * }.freeze
+        VALID_DAYS_OF_MONTH = ("1".."31").to_a << "last" << "lastday"
+        VALID_MONTHS = %w{JAN FEB MAR APR MAY JUN JUL AUG SEP OCT NOV DEC *}.freeze
+        VALID_WEEKS = %w{FIRST SECOND THIRD FOURTH LAST LASTDAY}.freeze
 
-      def after_created
-        if random_delay
-          validate_random_delay(random_delay, frequency)
-          random_delay(sec_to_min(random_delay))
-        end
+        def after_created
+          if random_delay
+            validate_random_delay(random_delay, frequency)
+            random_delay(sec_to_min(random_delay))
+          end
 
-        if execution_time_limit
-          execution_time_limit(259200) if execution_time_limit == "PT72H"
-          raise ArgumentError, "Invalid value passed for `execution_time_limit`. Please pass seconds as an Integer (e.g. 60) or a String with numeric values only (e.g. '60')." unless numeric_value_in_string?(execution_time_limit)
-          execution_time_limit(sec_to_min(execution_time_limit))
-        end
+          if execution_time_limit
+            execution_time_limit(259200) if execution_time_limit == "PT72H"
+            raise ArgumentError, "Invalid value passed for `execution_time_limit`. Please pass seconds as an Integer (e.g. 60) or a String with numeric values only (e.g. '60')." unless numeric_value_in_string?(execution_time_limit)
+            execution_time_limit(sec_to_min(execution_time_limit))
+          end
 
-        validate_frequency(frequency) if action.include?(:create) || action.include?(:change)
-        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
-        validate_frequency_monthly(frequency_modifier, months, day) if frequency == :monthly
-        validate_idle_time(idle_time, frequency)
-        idempotency_warning_for_frequency_weekly(day, start_day) if frequency == :weekly
-      end
+          validate_frequency(frequency) if action.include?(:create) || action.include?(:change)
+          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
+          validate_frequency_monthly(frequency_modifier, months, day) if frequency == :monthly
+          validate_idle_time(idle_time, frequency)
+          idempotency_warning_for_frequency_weekly(day, start_day) if frequency == :weekly
+        end
 
-      private
+        private
 
-      ## Resource is not idempotent when day, start_day is not provided with frequency :weekly
-      ## we set start_day when not given by user as current date based on which we set the day property for current current date day is monday ..
-      ## we set the monday as the day so at next run when  new_resource.day is nil and current_resource day is monday due to which udpate gets called
-      def idempotency_warning_for_frequency_weekly(day, start_day)
-        if start_day.nil? && day.nil?
-          logger.warn "To maintain idempotency for frequency :weekly provide start_day, start_time and day."
+        ## Resource is not idempotent when day, start_day is not provided with frequency :weekly
+        ## we set start_day when not given by user as current date based on which we set the day property for current current date day is monday ..
+        ## we set the monday as the day so at next run when  new_resource.day is nil and current_resource day is monday due to which udpate gets called
+        def idempotency_warning_for_frequency_weekly(day, start_day)
+          if start_day.nil? && day.nil?
+            logger.warn "To maintain idempotency for frequency :weekly provide start_day, start_time and day."
+          end
         end
-      end
 
-      # Validate the passed value is numeric values only if it is a string
-      def numeric_value_in_string?(val)
-        return true if Integer(val)
-      rescue ArgumentError
-        false
-      end
+        # Validate the passed value is numeric values only if it is a string
+        def numeric_value_in_string?(val)
+          return true if Integer(val)
+        rescue ArgumentError
+          false
+        end
 
-      def validate_frequency(frequency)
-        if frequency.nil? || !([:minute, :hourly, :daily, :weekly, :monthly, :once, :on_logon, :onstart, :on_idle, :none].include?(frequency))
-          raise ArgumentError, "Frequency needs to be provided. Valid frequencies are :minute, :hourly, :daily, :weekly, :monthly, :once, :on_logon, :onstart, :on_idle, :none."
+        def validate_frequency(frequency)
+          if frequency.nil? || !([:minute, :hourly, :daily, :weekly, :monthly, :once, :on_logon, :onstart, :on_idle, :none].include?(frequency))
+            raise ArgumentError, "Frequency needs to be provided. Valid frequencies are :minute, :hourly, :daily, :weekly, :monthly, :once, :on_logon, :onstart, :on_idle, :none."
+          end
         end
-      end
 
-      def validate_frequency_monthly(frequency_modifier, months, day)
-        # validates the frequency :monthly and raises error if frequency_modifier is first, second, thrid etc and day is not provided
-        if (frequency_modifier != 1) && (frequency_modifier_includes_days_of_weeks?(frequency_modifier)) && !(day)
-          raise ArgumentError, "Please select day on which you want to run the task e.g. 'Mon, Tue'. Multiple values must be seprated by comma."
+        def validate_frequency_monthly(frequency_modifier, months, day)
+          # validates the frequency :monthly and raises error if frequency_modifier is first, second, thrid etc and day is not provided
+          if (frequency_modifier != 1) && (frequency_modifier_includes_days_of_weeks?(frequency_modifier)) && !(day)
+            raise ArgumentError, "Please select day on which you want to run the task e.g. 'Mon, Tue'. Multiple values must be seprated by comma."
+          end
+
+          # frequency_modifer 2-12 is used to set every (n) months, so using :months propety with frequency_modifer is not valid since they both used to set months.
+          # Not checking value 1 here for frequecy_modifier since we are setting that as default value it won't break anything since preference is given to months property
+          if (frequency_modifier.to_i.between?(2, 12)) && !(months.nil?)
+            raise ArgumentError, "For frequency :monthly either use property months or frequency_modifier to set months."
+          end
         end
 
-        # frequency_modifer 2-12 is used to set every (n) months, so using :months propety with frequency_modifer is not valid since they both used to set months.
-        # Not checking value 1 here for frequecy_modifier since we are setting that as default value it won't break anything since preference is given to months property
-        if (frequency_modifier.to_i.between?(2, 12)) && !(months.nil?)
-          raise ArgumentError, "For frequency :monthly either use property months or frequency_modifier to set months."
+        # returns true if frequency_modifer has values First, second, third, fourth, last, lastday
+        def frequency_modifier_includes_days_of_weeks?(frequency_modifier)
+          frequency_modifier = frequency_modifier.to_s.split(",")
+          frequency_modifier.map! { |value| value.strip.upcase }
+          (frequency_modifier - VALID_WEEKS).empty?
         end
-      end
 
-      # returns true if frequency_modifer has values First, second, third, fourth, last, lastday
-      def frequency_modifier_includes_days_of_weeks?(frequency_modifier)
-        frequency_modifier = frequency_modifier.to_s.split(",")
-        frequency_modifier.map! { |value| value.strip.upcase }
-        (frequency_modifier - VALID_WEEKS).empty?
-      end
+        def validate_random_delay(random_delay, frequency)
+          if [:on_logon, :onstart, :on_idle, :none].include? frequency
+            raise ArgumentError, "`random_delay` property is supported only for frequency :once, :minute, :hourly, :daily, :weekly and :monthly"
+          end
 
-      def validate_random_delay(random_delay, frequency)
-        if [:on_logon, :onstart, :on_idle, :none].include? frequency
-          raise ArgumentError, "`random_delay` property is supported only for frequency :once, :minute, :hourly, :daily, :weekly and :monthly"
+          raise ArgumentError, "Invalid value passed for `random_delay`. Please pass seconds as an Integer (e.g. 60) or a String with numeric values only (e.g. '60')." unless numeric_value_in_string?(random_delay)
         end
 
-        raise ArgumentError, "Invalid value passed for `random_delay`. Please pass seconds as an Integer (e.g. 60) or a String with numeric values only (e.g. '60')." unless numeric_value_in_string?(random_delay)
-      end
+        # @todo when we drop ruby 2.3 support this should be converted to .match?() instead of =~f
+        def validate_start_day(start_day, frequency)
+          if start_day && frequency == :none
+            raise ArgumentError, "`start_day` property is not supported with frequency: #{frequency}"
+          end
 
-      # @todo when we drop ruby 2.3 support this should be converted to .match?() instead of =~f
-      def validate_start_day(start_day, frequency)
-        if start_day && frequency == :none
-          raise ArgumentError, "`start_day` property is not supported with frequency: #{frequency}"
+          # make sure the start_day is in MM/DD/YYYY format: http://rubular.com/r/cgjHemtWl5
+          if start_day
+            raise ArgumentError, "`start_day` property must be in the MM/DD/YYYY format." unless /^(0[1-9]|1[012])[- \/.](0[1-9]|[12][0-9]|3[01])[- \/.](19|20)\d\d$/ =~ start_day
+          end
         end
 
-        # make sure the start_day is in MM/DD/YYYY format: http://rubular.com/r/cgjHemtWl5
-        if start_day
-          raise ArgumentError, "`start_day` property must be in the MM/DD/YYYY format." unless /^(0[1-9]|1[012])[- \/.](0[1-9]|[12][0-9]|3[01])[- \/.](19|20)\d\d$/ =~ start_day
+        # @todo when we drop ruby 2.3 support this should be converted to .match?() instead of =~
+        def validate_start_time(start_time, frequency)
+          if start_time
+            raise ArgumentError, "`start_time` property is not supported with `frequency :none`" if frequency == :none
+            raise ArgumentError, "`start_time` property must be in the HH:mm format (e.g. 6:20pm -> 18:20)." unless /^[0-2][0-9]:[0-5][0-9]$/ =~ start_time
+          else
+            raise ArgumentError, "`start_time` needs to be provided with `frequency :once`" if frequency == :once
+          end
         end
-      end
 
-      # @todo when we drop ruby 2.3 support this should be converted to .match?() instead of =~
-      def validate_start_time(start_time, frequency)
-        if start_time
-          raise ArgumentError, "`start_time` property is not supported with `frequency :none`" if frequency == :none
-          raise ArgumentError, "`start_time` property must be in the HH:mm format (e.g. 6:20pm -> 18:20)." unless /^[0-2][0-9]:[0-5][0-9]$/ =~ start_time
-        else
-          raise ArgumentError, "`start_time` needs to be provided with `frequency :once`" if frequency == :once
+        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("', '")}'"
+          end
         end
-      end
 
-      def validate_user_and_password(user, password)
-        if password_required?(user) && password.nil?
-          raise ArgumentError, %q{Cannot specify a user other than the system users without specifying a password!. Valid passwordless users: 'NT AUTHORITY\SYSTEM', 'SYSTEM', 'NT AUTHORITY\LOCALSERVICE', 'NT AUTHORITY\NETWORKSERVICE', 'BUILTIN\USERS', 'USERS'}
+        def password_required?(user)
+          return false if user.nil?
+          @password_required ||= !Chef::ReservedNames::Win32::Security::SID.system_user?(user)
         end
-      end
-
-      def password_required?(user)
-        return false if user.nil?
-        @password_required ||= !SYSTEM_USERS.include?(user.upcase)
-      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
-
-      def validate_create_frequency_modifier(frequency, frequency_modifier)
-        if ([:on_logon, :onstart, :on_idle, :none].include?(frequency)) && ( frequency_modifier != 1)
-          raise ArgumentError, "frequency_modifier property not supported with frequency :#{frequency}"
+        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
 
-        if frequency == :monthly
-          unless (1..12).cover?(frequency_modifier.to_i) || frequency_modifier_includes_days_of_weeks?(frequency_modifier)
-            raise ArgumentError, "frequency_modifier value #{frequency_modifier} is invalid. Valid values for :monthly frequency are 1 - 12, 'FIRST', 'SECOND', 'THIRD', 'FOURTH', 'LAST'."
+        def validate_create_frequency_modifier(frequency, frequency_modifier)
+          if ([:on_logon, :onstart, :on_idle, :none].include?(frequency)) && ( frequency_modifier != 1)
+            raise ArgumentError, "frequency_modifier property not supported with frequency :#{frequency}"
           end
-        else
-          unless frequency.nil? || frequency_modifier.nil?
-            frequency_modifier = frequency_modifier.to_i
-            min = 1
-            max = case frequency
-                    when :minute
-                      1439
-                    when :hourly
-                      23
-                    when :daily
-                      365
-                    when :weekly
-                      52
-                    else
-                      min
-                  end
-            unless frequency_modifier.between?(min, max)
-              raise ArgumentError, "frequency_modifier value #{frequency_modifier} is invalid. Valid values for :#{frequency} frequency are #{min} - #{max}."
+
+          if frequency == :monthly
+            unless (1..12).cover?(frequency_modifier.to_i) || frequency_modifier_includes_days_of_weeks?(frequency_modifier)
+              raise ArgumentError, "frequency_modifier value #{frequency_modifier} is invalid. Valid values for :monthly frequency are 1 - 12, 'FIRST', 'SECOND', 'THIRD', 'FOURTH', 'LAST'."
+            end
+          else
+            unless frequency.nil? || frequency_modifier.nil?
+              frequency_modifier = frequency_modifier.to_i
+              min = 1
+              max = case frequency
+                      when :minute
+                        1439
+                      when :hourly
+                        23
+                      when :daily
+                        365
+                      when :weekly
+                        52
+                      else
+                        min
+                    end
+              unless frequency_modifier.between?(min, max)
+                raise ArgumentError, "frequency_modifier value #{frequency_modifier} is invalid. Valid values for :#{frequency} frequency are #{min} - #{max}."
+              end
             end
           end
         end
-      end
-
-      def validate_create_day(day, frequency, frequency_modifier)
-        raise ArgumentError, "day property is only valid for tasks that run monthly or weekly" unless [:weekly, :monthly].include?(frequency)
 
-        # This has been verified with schtask.exe https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/schtasks#d-dayday--
-        # verified with earlier code if day "*" is given with frequency it raised exception Invalid value for /D option
-        raise ArgumentError, "day wild card (*) is only valid with frequency :weekly" if frequency == :monthly && day == "*"
-
-        if day.is_a?(String) && day.to_i.to_s != day
-          days = day.split(",")
-          if days_includes_days_of_months?(days)
-            # Following error will be raise if day is set as 1-31 and frequency is selected as :weekly since those values are valid with only frequency :monthly
-            raise ArgumentError, "day values 1-31 or last is only valid with frequency :monthly" if frequency == :weekly
-          else
-            days.map! { |day| day.to_s.strip.downcase }
-            unless (days - VALID_WEEK_DAYS).empty?
-              raise ArgumentError, "day property invalid. Only valid values are: #{VALID_WEEK_DAYS.map(&:upcase).join(', ')}. Multiple values must be separated by a comma."
+        def validate_create_day(day, frequency, frequency_modifier)
+          raise ArgumentError, "day property is only valid for tasks that run monthly or weekly" unless [:weekly, :monthly].include?(frequency)
+
+          # This has been verified with schtask.exe https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/schtasks#d-dayday--
+          # verified with earlier code if day "*" is given with frequency it raised exception Invalid value for /D option
+          raise ArgumentError, "day wild card (*) is only valid with frequency :weekly" if frequency == :monthly && day == "*"
+
+          if day.is_a?(String) && day.to_i.to_s != day
+            days = day.split(",")
+            if days_includes_days_of_months?(days)
+              # Following error will be raise if day is set as 1-31 and frequency is selected as :weekly since those values are valid with only frequency :monthly
+              raise ArgumentError, "day values 1-31 or last is only valid with frequency :monthly" if frequency == :weekly
+            else
+              days.map! { |day| day.to_s.strip.downcase }
+              unless (days - VALID_WEEK_DAYS).empty?
+                raise ArgumentError, "day property invalid. Only valid values are: #{VALID_WEEK_DAYS.map(&:upcase).join(', ')}. Multiple values must be separated by a comma."
+              end
             end
           end
         end
-      end
 
-      def validate_create_months(months, frequency)
-        raise ArgumentError, "months property is only valid for tasks that run monthly" if frequency != :monthly
-        if months.is_a?(String)
-          months = months.split(",")
-          months.map! { |month| month.strip.upcase }
-          unless (months - VALID_MONTHS).empty?
-            raise ArgumentError, "months property invalid. Only valid values are: #{VALID_MONTHS.join(', ')}. Multiple values must be separated by a comma."
+        def validate_create_months(months, frequency)
+          raise ArgumentError, "months property is only valid for tasks that run monthly" if frequency != :monthly
+          if months.is_a?(String)
+            months = months.split(",")
+            months.map! { |month| month.strip.upcase }
+            unless (months - VALID_MONTHS).empty?
+              raise ArgumentError, "months property invalid. Only valid values are: #{VALID_MONTHS.join(', ')}. Multiple values must be separated by a comma."
+            end
           end
         end
-      end
-
-      # This method returns true if day has values from 1-31 which is a days of moths and used with frequency :monthly
-      def days_includes_days_of_months?(days)
-        days.map! { |day| day.to_s.strip.downcase }
-        (days - VALID_DAYS_OF_MONTH).empty?
-      end
 
-      def validate_idle_time(idle_time, frequency)
-        if !idle_time.nil? && frequency != :on_idle
-          raise ArgumentError, "idle_time property is only valid for tasks that run on_idle"
+        # This method returns true if day has values from 1-31 which is a days of moths and used with frequency :monthly
+        def days_includes_days_of_months?(days)
+          days.map! { |day| day.to_s.strip.downcase }
+          (days - VALID_DAYS_OF_MONTH).empty?
         end
-        if idle_time.nil? && frequency == :on_idle
-          raise ArgumentError, "idle_time value should be set for :on_idle frequency."
-        end
-        unless idle_time.nil? || idle_time > 0 && idle_time <= 999
-          raise ArgumentError, "idle_time value #{idle_time} is invalid. Valid values for :on_idle frequency are 1 - 999."
+
+        def validate_idle_time(idle_time, frequency)
+          if !idle_time.nil? && frequency != :on_idle
+            raise ArgumentError, "idle_time property is only valid for tasks that run on_idle"
+          end
+          if idle_time.nil? && frequency == :on_idle
+            raise ArgumentError, "idle_time value should be set for :on_idle frequency."
+          end
+          unless idle_time.nil? || idle_time > 0 && idle_time <= 999
+            raise ArgumentError, "idle_time value #{idle_time} is invalid. Valid values for :on_idle frequency are 1 - 999."
+          end
         end
-      end
 
-      # Converts the number of seconds to an ISO8601 duration format and returns it.
-      # Ref : https://github.com/arnau/ISO8601/blob/master/lib/iso8601/duration.rb#L18-L23
-      # e.g.
-      # ISO8601::Duration.new(65707200).to_s
-      # returns 'PT65707200S'
-      def sec_to_dur(seconds)
-        ISO8601::Duration.new(seconds.to_i).to_s
-      end
+        # Converts the number of seconds to an ISO8601 duration format and returns it.
+        # Ref : https://github.com/arnau/ISO8601/blob/master/lib/iso8601/duration.rb#L18-L23
+        # e.g.
+        # ISO8601::Duration.new(65707200).to_s
+        # returns 'PT65707200S'
+        def sec_to_dur(seconds)
+          ISO8601::Duration.new(seconds.to_i).to_s
+        end
 
-      def sec_to_min(seconds)
-        seconds.to_i / 60
+        def sec_to_min(seconds)
+          seconds.to_i / 60
+        end
       end
     end
   end
diff --git a/lib/chef/win32/security/sid.rb b/lib/chef/win32/security/sid.rb
index b551cbd2e3..43efc6e3fe 100644
--- a/lib/chef/win32/security/sid.rb
+++ b/lib/chef/win32/security/sid.rb
@@ -246,6 +246,45 @@ class Chef
           SID.from_account("#{::ENV['USERDOMAIN']}\\#{::ENV['USERNAME']}")
         end
 
+        SERVICE_ACCOUNT_USERS = [self.LocalSystem,
+                                 self.NtLocal,
+                                 self.NtNetwork].flat_map do |user_type|
+          [user_type.account_simple_name.upcase,
+           user_type.account_name.upcase]
+        end.freeze
+
+        BUILT_IN_GROUPS = [self.BuiltinAdministrators,
+                           self.BuiltinUsers, self.Guests].flat_map do |user_type|
+          [user_type.account_simple_name.upcase,
+           user_type.account_name.upcase]
+        end.freeze
+
+        SYSTEM_USER = SERVICE_ACCOUNT_USERS + BUILT_IN_GROUPS
+
+        # Сheck if the user belongs to service accounts category
+        #
+        # @return [Boolean] True or False
+        #
+        def self.service_account_user?(user)
+          SERVICE_ACCOUNT_USERS.include?(user.to_s.upcase)
+        end
+
+        # Сheck if the user is in builtin system group
+        #
+        # @return [Boolean] True or False
+        #
+        def self.group_user?(user)
+          BUILT_IN_GROUPS.include?(user.to_s.upcase)
+        end
+
+        # Сheck if the user belongs to system users category
+        #
+        # @return [Boolean] True or False
+        #
+        def self.system_user?(user)
+          SYSTEM_USER.include?(user.to_s.upcase)
+        end
+
         # See https://technet.microsoft.com/en-us/library/cc961992.aspx
         # In practice, this is SID.Administrators if the current_user is an admin (even if not
         # running elevated), and is current_user otherwise.
diff --git a/spec/functional/resource/windows_task_spec.rb b/spec/functional/resource/windows_task_spec.rb
index a2cd4b97d7..cb14da80d8 100644
--- a/spec/functional/resource/windows_task_spec.rb
+++ b/spec/functional/resource/windows_task_spec.rb
@@ -1571,7 +1571,7 @@ describe Chef::Resource::WindowsTask, :windows_only do
       it "raises error" do
         subject.user "Administrator"
         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: 'NT AUTHORITY\SYSTEM', 'SYSTEM', 'NT AUTHORITY\LOCALSERVICE', 'NT AUTHORITY\NETWORKSERVICE', 'BUILTIN\USERS', 'USERS'})
+        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'})
       end
     end
 
diff --git a/spec/unit/provider/windows_task_spec.rb b/spec/unit/provider/windows_task_spec.rb
index 3a046ab5de..3565da1ca3 100644
--- a/spec/unit/provider/windows_task_spec.rb
+++ b/spec/unit/provider/windows_task_spec.rb
@@ -416,6 +416,7 @@ describe Chef::Provider::WindowsTask, :windows_only do
     end
 
     it "return logon_type bindary value as 1 as if password is not nil" do
+      new_resource.user = "Administrator"
       new_resource.password = "abc"
       expect(provider.send(:logon_type)).to be(1)
     end
diff --git a/spec/unit/resource/windows_task_spec.rb b/spec/unit/resource/windows_task_spec.rb
index 76e2a86ee8..efbc1b1846 100644
--- a/spec/unit/resource/windows_task_spec.rb
+++ b/spec/unit/resource/windows_task_spec.rb
@@ -73,7 +73,7 @@ describe Chef::Resource::WindowsTask, :windows_only do
     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: 'NT AUTHORITY\SYSTEM', 'SYSTEM', 'NT AUTHORITY\LOCALSERVICE', 'NT AUTHORITY\NETWORKSERVICE', 'BUILTIN\USERS', 'USERS'})
+      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
 
     it "does not raise an error if the user is a system user" do