diff --git a/lib/puppet/util/windows/sid.rb b/lib/puppet/util/windows/sid.rb index d648b080226..89e7b48e20c 100644 --- a/lib/puppet/util/windows/sid.rb +++ b/lib/puppet/util/windows/sid.rb @@ -75,7 +75,9 @@ def name_to_principal(name, allow_unresolved = false) raw_sid_bytes = sid_ptr.read_array_of_uchar(get_length_sid(sid_ptr)) end rescue => e - Puppet.debug("Could not retrieve raw SID bytes from '#{name}': #{e.message}") + # Avoid debug logs pollution with valid account names + # https://docs.microsoft.com/en-us/windows/win32/api/sddl/nf-sddl-convertstringsidtosidw#return-value + Puppet.debug("Could not retrieve raw SID bytes from '#{name}': #{e.message}") unless e.code == ERROR_INVALID_SID_STRUCTURE end raw_sid_bytes ? Principal.lookup_account_sid(raw_sid_bytes) : Principal.lookup_account_name(name) diff --git a/spec/unit/util/windows/sid_spec.rb b/spec/unit/util/windows/sid_spec.rb index 5b366ada36a..24f9d9f5145 100644 --- a/spec/unit/util/windows/sid_spec.rb +++ b/spec/unit/util/windows/sid_spec.rb @@ -131,38 +131,73 @@ expect(subject.name_to_principal(unknown_name)).to be_nil end + it "should print a debug message if the account does not exist" do + expect(Puppet).to receive(:debug).with(/No mapping between account names and security IDs was done/) + subject.name_to_principal(unknown_name) + end + it "should return a Puppet::Util::Windows::SID::Principal instance for any valid sid" do expect(subject.name_to_principal(sid)).to be_an_instance_of(Puppet::Util::Windows::SID::Principal) end + it "should not print debug messages for valid sid" do + expect(Puppet).not_to receive(:debug).with(/Could not retrieve raw SID bytes from/) + expect(Puppet).not_to receive(:debug).with(/No mapping between account names and security IDs was done/) + subject.name_to_principal(sid) + end + + it "should print a debug message for invalid sid" do + expect(Puppet).not_to receive(:debug).with(/Could not retrieve raw SID bytes from/) + expect(Puppet).to receive(:debug).with(/No mapping between account names and security IDs was done/) + subject.name_to_principal('S-1-5-21-INVALID-SID') + end + it "should accept unqualified account name" do # NOTE: lookup by name works in localized environments only for a few instances # this works in French Windows, even though the account is really Syst\u00E8me expect(subject.name_to_principal('SYSTEM').sid).to eq(sid) end + it "should not print debug messages for unqualified account name" do + expect(Puppet).not_to receive(:debug).with(/Could not retrieve raw SID bytes from/) + expect(Puppet).not_to receive(:debug).with(/No mapping between account names and security IDs was done/) + subject.name_to_principal('SYSTEM') + end + it "should be case-insensitive" do # NOTE: lookup by name works in localized environments only for a few instances # this works in French Windows, even though the account is really Syst\u00E8me expect(subject.name_to_principal('SYSTEM')).to eq(subject.name_to_principal('system')) end + it "should not print debug messages for wrongly cased account name" do + expect(Puppet).not_to receive(:debug).with(/Could not retrieve raw SID bytes from/) + expect(Puppet).not_to receive(:debug).with(/No mapping between account names and security IDs was done/) + subject.name_to_principal('system') + end + it "should be leading and trailing whitespace-insensitive" do # NOTE: lookup by name works in localized environments only for a few instances # this works in French Windows, even though the account is really Syst\u00E8me expect(subject.name_to_principal('SYSTEM')).to eq(subject.name_to_principal(' SYSTEM ')) end + it "should not print debug messages for account name with leading and trailing whitespace" do + expect(Puppet).not_to receive(:debug).with(/Could not retrieve raw SID bytes from/) + expect(Puppet).not_to receive(:debug).with(/No mapping between account names and security IDs was done/) + subject.name_to_principal(' SYSTEM ') + end + it "should accept domain qualified account names" do # NOTE: lookup by name works in localized environments only for a few instances # this works in French Windows, even though the account is really AUTORITE NT\\Syst\u00E8me expect(subject.name_to_principal('NT AUTHORITY\SYSTEM').sid).to eq(sid) end - it "should print a debug message on failures" do - expect(Puppet).to receive(:debug).with(/Could not retrieve raw SID bytes from 'NonExistingUser'/) - expect(Puppet).to receive(:debug).with(/No mapping between account names and security IDs was done/) - subject.name_to_principal('NonExistingUser') + it "should not print debug messages for domain qualified account names" do + expect(Puppet).not_to receive(:debug).with(/Could not retrieve raw SID bytes from/) + expect(Puppet).not_to receive(:debug).with(/No mapping between account names and security IDs was done/) + subject.name_to_principal('NT AUTHORITY\SYSTEM') end end