Merge pull request #8278 from jasonwbarnett/bugfix/8080

windows_service: Fix action :start to not resets credentials on service

2 files changed, 43 insertions, 16 deletions

diff --git a/lib/chef/provider/service/windows.rb b/lib/chef/provider/service/windows.rb
index 28988fdb3d..710cdd67c6 100644
--- a/lib/chef/provider/service/windows.rb
+++ b/lib/chef/provider/service/windows.rb
@@ -83,22 +83,7 @@ class Chef::Provider::Service::Windows < Chef::Provider::Service
 
   def start_service
     if Win32::Service.exists?(@new_resource.service_name)
-      # reconfiguration is idempotent, so just do it.
-      new_config = {
-        service_name: @new_resource.service_name,
-        service_start_name: @new_resource.run_as_user,
-        password: @new_resource.run_as_password,
-      }.reject { |k, v| v.nil? || v.length == 0 }
-
-      Win32::Service.configure(new_config)
-      logger.info "#{@new_resource} configured."
-
-      # LocalSystem is the default runas user, which is a special service account that should ultimately have the rights of BUILTIN\Administrators, but we wouldn't see that from get_account_right
-      if new_config.key?(:service_start_name) && new_config[:service_start_name].casecmp("localsystem") != 0
-        unless Chef::ReservedNames::Win32::Security.get_account_right(canonicalize_username(new_config[:service_start_name])).include?(SERVICE_RIGHT)
-          grant_service_logon(new_config[:service_start_name])
-        end
-      end
+      configure_service_run_as_properties
 
       state = current_state
       if state == RUNNING
@@ -281,6 +266,21 @@ class Chef::Provider::Service::Windows < Chef::Provider::Service
 
   private
 
+  def configure_service_run_as_properties
+    return unless new_resource.property_is_set?(:run_as_user)
+
+    new_config = {
+      service_name: new_resource.service_name,
+      service_start_name: new_resource.run_as_user,
+      password: new_resource.run_as_password,
+    }.reject { |k, v| v.nil? || v.length == 0 }
+
+    Win32::Service.configure(new_config)
+    logger.info "#{new_resource} configured."
+
+    grant_service_logon(new_resource.run_as_user) if new_resource.run_as_user.casecmp("localsystem") != 0
+  end
+
   def current_delayed_start
     if service = Win32::Service.services.find { |x| x.service_name == new_resource.service_name }
       service.delayed_start == 0 ? false : true
@@ -290,6 +290,8 @@ class Chef::Provider::Service::Windows < Chef::Provider::Service
   end
 
   def grant_service_logon(username)
+    return if Chef::ReservedNames::Win32::Security.get_account_right(canonicalize_username(username)).include?(SERVICE_RIGHT)
+
     begin
       Chef::ReservedNames::Win32::Security.add_account_right(canonicalize_username(username), SERVICE_RIGHT)
     rescue Chef::Exceptions::Win32APIError => err
diff --git a/spec/unit/provider/service/windows_spec.rb b/spec/unit/provider/service/windows_spec.rb
index 501301926a..00258f3a64 100644
--- a/spec/unit/provider/service/windows_spec.rb
+++ b/spec/unit/provider/service/windows_spec.rb
@@ -511,6 +511,31 @@ describe Chef::Provider::Service::Windows, "load_current_resource", :windows_onl
         double("StatusStruct", current_state: "running"))
     end
 
+    context "run_as_user user is specified" do
+      let(:run_as_user) { provider.new_resource.class.properties[:run_as_user].default }
+
+      before do
+        provider.new_resource.run_as_user run_as_user
+      end
+
+      it "configures service run_as_user and run_as_password" do
+        expect(provider).to receive(:configure_service_run_as_properties).and_call_original
+        expect(Win32::Service).to receive(:configure)
+        provider.start_service
+      end
+    end
+
+    context "run_as_user user is not specified" do
+      before do
+        expect(provider.new_resource.property_is_set?(:run_as_user)).to be false
+      end
+
+      it "does not configure service run_as_user and run_as_password" do
+        expect(Win32::Service).not_to receive(:configure)
+        provider.start_service
+      end
+    end
+
     context "start_command is specified" do
       let(:start_command) { "sc start #{chef_service_name}" }