Merge pull request #4321 from chef/service_user

fix run_as_user of windows_service

2 files changed, 23 insertions, 29 deletions

diff --git a/spec/functional/resource/windows_service_spec.rb b/spec/functional/resource/windows_service_spec.rb
index 90545429e5..9ee4adde87 100644
--- a/spec/functional/resource/windows_service_spec.rb
+++ b/spec/functional/resource/windows_service_spec.rb
@@ -23,7 +23,7 @@ describe Chef::Resource::WindowsService, :windows_only, :system_windows_service_
   include_context "using Win32::Service"
 
   let(:username) { "service_spec_user"}
-  let(:qualified_username) { ".\\#{username}"}
+  let(:qualified_username) { "#{ENV['COMPUTERNAME']}\\#{username}"}
   let(:password) { "1a2b3c4X!&narf"}
 
   let(:user_resource) {
@@ -93,6 +93,9 @@ describe Chef::Resource::WindowsService, :windows_only, :system_windows_service_
       service_resource.run_action(:start)
     end
 
-    it "raises an exception when it can't grant the logon privilege"
+    it "grants the user the log on as service right" do
+      service_resource.run_action(:start)
+      expect(Chef::ReservedNames::Win32::Security.get_account_right(qualified_username)).to include("SeServiceLogonRight")
+    end
   end
 end
diff --git a/spec/unit/provider/service/windows_spec.rb b/spec/unit/provider/service/windows_spec.rb
index 4c9f5b3377..34140fdd7b 100644
--- a/spec/unit/provider/service/windows_spec.rb
+++ b/spec/unit/provider/service/windows_spec.rb
@@ -30,6 +30,7 @@ describe Chef::Provider::Service::Windows, "load_current_resource" do
     prvdr.current_resource = Chef::Resource::WindowsService.new("current-chef")
     prvdr
   end
+  let(:service_right) { Chef::Provider::Service::Windows::SERVICE_RIGHT }
 
   before(:all) do
     Win32::Service = Class.new
@@ -46,6 +47,7 @@ describe Chef::Provider::Service::Windows, "load_current_resource" do
       double("ConfigStruct", :start_type => "auto start"))
     allow(Win32::Service).to receive(:exists?).and_return(true)
     allow(Win32::Service).to receive(:configure).and_return(Win32::Service)
+    allow(Chef::ReservedNames::Win32::Security).to receive(:get_account_right).and_return([])
   end
 
   after(:each) do
@@ -164,6 +166,13 @@ describe Chef::Provider::Service::Windows, "load_current_resource" do
         expect(provider).to receive(:grant_service_logon).and_return(true)
         provider.start_service
       end
+
+      it "does not grant user SeServiceLogonRight if it already has it" do
+        expect(Win32::Service).to receive(:start)
+        expect(Chef::ReservedNames::Win32::Security).to receive(:get_account_right).with("wallace").and_return([service_right])
+        expect(Chef::ReservedNames::Win32::Security).not_to receive(:add_account_right).with("wallace", service_right)
+        provider.start_service
+      end
     end
   end
 
@@ -417,37 +426,19 @@ describe Chef::Provider::Service::Windows, "load_current_resource" do
     include_context "testing private methods"
 
     let(:username) { "unit_test_user" }
-    let(:success_string) { "The task has completed successfully.\r\nSee logfile etc." }
-    let(:failure_string) { "Look on my works, ye Mighty, and despair!" }
-    let(:command) {
-      dbfile = provider.grant_dbfile_name(username)
-      policyfile = provider.grant_policyfile_name(username)
-      logfile = provider.grant_logfile_name(username)
-
-      %Q{secedit.exe /configure /db "#{dbfile}" /cfg "#{policyfile}" /areas USER_RIGHTS SECURITYPOLICY SERVICES /log "#{logfile}"}
-    }
-    let(:shellout_env) { {:environment=>{"LC_ALL"=>"en_US.UTF-8"}} }
 
-    before {
-      expect_any_instance_of(described_class).to receive(:shell_out).with(command).and_call_original
-      expect_any_instance_of(Mixlib::ShellOut).to receive(:run_command).and_return(nil)
-    }
-
-    after {
-      # only needed for the second test.
-      ::File.delete(provider.grant_policyfile_name(username)) rescue nil
-      ::File.delete(provider.grant_logfile_name(username)) rescue nil
-      ::File.delete(provider.grant_dbfile_name(username)) rescue nil
-    }
-
-    it "calls Mixlib::Shellout with the correct command string" do
-      expect_any_instance_of(Mixlib::ShellOut).to receive(:exitstatus).and_return(0)
+    it "calls win32 api to grant user SeServiceLogonRight" do
+      expect(Chef::ReservedNames::Win32::Security).to receive(:add_account_right).with(username, service_right)
       expect(provider.grant_service_logon(username)).to equal true
     end
 
-    it "raises an exception when the grant command fails" do
-      expect_any_instance_of(Mixlib::ShellOut).to receive(:exitstatus).and_return(1)
-      expect_any_instance_of(Mixlib::ShellOut).to receive(:stdout).and_return(failure_string)
+    it "strips '.\' from user name when sending to win32 api" do
+      expect(Chef::ReservedNames::Win32::Security).to receive(:add_account_right).with(username, service_right)
+      expect(provider.grant_service_logon(".\\#{username}")).to equal true
+    end
+
+    it "raises an exception when the grant fails" do
+      expect(Chef::ReservedNames::Win32::Security).to receive(:add_account_right).and_raise(Chef::Exceptions::Win32APIError, "barf")
       expect { provider.grant_service_logon(username) }.to raise_error(Chef::Exceptions::Service)
     end
   end