diff options
author | Vasu1105 <vasundhara.jagdale@msystechnologies.com> | 2020-02-26 01:19:01 -0800 |
---|---|---|
committer | Vasu1105 <vasundhara.jagdale@msystechnologies.com> | 2020-02-26 01:39:52 -0800 |
commit | 11a63c5b61ca1d703b368ddc1caecd7f16fc7e9c (patch) | |
tree | 53a0cd66b49b0f7e29fe582ef41c8c6ee7d3d230 | |
parent | ed559f9687b830648ae6a96d76a040c1fbeef284 (diff) | |
download | chef-11a63c5b61ca1d703b368ddc1caecd7f16fc7e9c.tar.gz |
More optimization of code and updated get_account_with_user_rights to return account_name with domain
Signed-off-by: Vasu1105 <vasundhara.jagdale@msystechnologies.com>
-rw-r--r-- | lib/chef/resource/windows_user_privilege.rb | 18 | ||||
-rw-r--r-- | lib/chef/win32/security.rb | 22 | ||||
-rw-r--r-- | spec/functional/resource/windows_user_privilege_spec.rb | 6 | ||||
-rw-r--r-- | spec/functional/win32/security_spec.rb | 5 |
4 files changed, 30 insertions, 21 deletions
diff --git a/lib/chef/resource/windows_user_privilege.rb b/lib/chef/resource/windows_user_privilege.rb index 85f1557d09..685354cfb4 100644 --- a/lib/chef/resource/windows_user_privilege.rb +++ b/lib/chef/resource/windows_user_privilege.rb @@ -97,7 +97,7 @@ class Chef action :add do ([*new_resource.privilege] - [*current_resource.privilege]).each do |user_right| - converge_by("adding user privilege #{user_right}") do + converge_by("adding user '#{new_resource.principal}' privilege #{user_right}") do Chef::ReservedNames::Win32::Security.add_account_right(new_resource.principal, user_right) end end @@ -113,7 +113,7 @@ class Chef # Getting users with its domain for comparison new_resource.users.each do |user| user = Chef::ReservedNames::Win32::Security.lookup_account_name(user) - users << user[1].account if user + users << user[1].account_name if user end new_resource.privilege.each do |privilege| @@ -121,14 +121,16 @@ class Chef # comparing the existing accounts for privilege with users unless users == accounts - accounts.each do |account| - converge_by("removing user #{account[1]} from privilege #{privilege}") do - Chef::ReservedNames::Win32::Security.remove_account_right(account[1], privilege) + # Removing only accounts which is not matching with users in new_resource + (accounts - users).each do |account| + converge_by("removing user '#{account}' from privilege #{privilege}") do + Chef::ReservedNames::Win32::Security.remove_account_right(account, privilege) end end - new_resource.users.each do |user| - converge_by("adding user #{user} to privilege #{privilege}") do + # Adding only users which is not already exist + (users - accounts).each do |user| + converge_by("adding user '#{user}' to privilege #{privilege}") do Chef::ReservedNames::Win32::Security.add_account_right(user, privilege) end end @@ -145,7 +147,7 @@ class Chef end (new_resource.privilege - missing_res_privileges).each do |user_right| - converge_by("removing user privilege #{user_right}") do + converge_by("removing user #{new_resource.principal} from privilege #{user_right}") do Chef::ReservedNames::Win32::Security.remove_account_right(new_resource.principal, user_right) end end diff --git a/lib/chef/win32/security.rb b/lib/chef/win32/security.rb index 2879131210..2c0f63684a 100644 --- a/lib/chef/win32/security.rb +++ b/lib/chef/win32/security.rb @@ -227,15 +227,19 @@ class Chef accounts = [] with_lsa_policy(nil) do |policy_handle, sid| result = LsaEnumerateAccountsWithUserRight(policy_handle.read_pointer, privilege_pointer, buffer, count) - win32_error = LsaNtStatusToWinError(result) - return [] if win32_error == 1313 # NO_SUCH_PRIVILEGE - https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--1300-1699- - - test_and_raise_lsa_nt_status(result) - - count.read_ulong.times do |i| - sid = LSA_ENUMERATION_INFORMATION.new(buffer.read_pointer + i * LSA_ENUMERATION_INFORMATION.size) - sid_name = lookup_account_sid(sid[:Sid]) - accounts << sid_name + if result == 0 + win32_error = LsaNtStatusToWinError(result) + return [] if win32_error == 1313 # NO_SUCH_PRIVILEGE - https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--1300-1699- + + test_and_raise_lsa_nt_status(result) + + count.read_ulong.times do |i| + sid = LSA_ENUMERATION_INFORMATION.new(buffer.read_pointer + i * LSA_ENUMERATION_INFORMATION.size) + sid_name = lookup_account_sid(sid[:Sid]) + domain, name, use = sid_name + account_name = (!domain.nil? && domain.length > 0) ? "#{domain}\\#{name}" : name + accounts << account_name + end end result = LsaFreeMemory(buffer.read_pointer) diff --git a/spec/functional/resource/windows_user_privilege_spec.rb b/spec/functional/resource/windows_user_privilege_spec.rb index 9bfb1d731e..6dca54016a 100644 --- a/spec/functional/resource/windows_user_privilege_spec.rb +++ b/spec/functional/resource/windows_user_privilege_spec.rb @@ -51,6 +51,8 @@ describe Chef::Resource::WindowsUserPrivilege, :windows_only do let(:privilege) { "SeCreateSymbolicLinkPrivilege" } it "adds user to privilege" do + # Removing so that add update happens + subject.run_action(:remove) subject.run_action(:add) expect(subject).to be_updated_by_last_action end @@ -64,7 +66,7 @@ describe Chef::Resource::WindowsUserPrivilege, :windows_only do context "when privilege is passed as array" do let(:principal) { "Administrator" } - let(:privilege) { ["SeCreateSymbolicLinkPrivilege", "SeCreatePagefilePrivilege"] } + let(:privilege) { %w{SeCreateSymbolicLinkPrivilege SeCreatePagefilePrivilege} } it "adds user to privilege" do subject.run_action(:add) @@ -125,7 +127,7 @@ describe Chef::Resource::WindowsUserPrivilege, :windows_only do end context "when privilege is passed as array" do - let(:privilege) { ["SeCreateSymbolicLinkPrivilege", "SeCreatePagefilePrivilege"] } + let(:privilege) { %w{SeCreateSymbolicLinkPrivilege SeCreatePagefilePrivilege} } it "remove user from privilege" do subject.run_action(:add) subject.run_action(:remove) diff --git a/spec/functional/win32/security_spec.rb b/spec/functional/win32/security_spec.rb index 8caacffd2c..c01e9be9a3 100644 --- a/spec/functional/win32/security_spec.rb +++ b/spec/functional/win32/security_spec.rb @@ -200,14 +200,15 @@ describe "Chef::Win32::Security", :windows_only do end describe ".get_account_with_user_rights" do + let(:domain) { ENV["COMPUTERNAME"] } let(:username) { ENV["USERNAME"] } context "when given a valid user right" do it "gets all accounts associated with given user right" do Chef::ReservedNames::Win32::Security.add_account_right(username, "SeBatchLogonRight") - expect(Chef::ReservedNames::Win32::Security.get_account_with_user_rights("SeBatchLogonRight").flatten).to include(username) + expect(Chef::ReservedNames::Win32::Security.get_account_with_user_rights("SeBatchLogonRight").flatten).to include("#{domain}\\#{username}") Chef::ReservedNames::Win32::Security.remove_account_right(username, "SeBatchLogonRight") - expect(Chef::ReservedNames::Win32::Security.get_account_with_user_rights("SeBatchLogonRight").flatten).not_to include(username) + expect(Chef::ReservedNames::Win32::Security.get_account_with_user_rights("SeBatchLogonRight").flatten).not_to include("#{domain}\\#{username}") end end |