Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion lib/puppet/provider/user/windows_adsi.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,11 @@ def password
end

def password=(value)
# TODO: emit Puppet debug message here about setting a property that we can't verify when this is nil or ''
if value.nil?
Puppet.warning("Puppet does not support setting a nil password on Windows.")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not actually sure we'll ever get nil supplied for value here.

elsif value == ''
Puppet.warning("Blank (zero length) passwords are not recommended. Blank passwords may be allowed on your system based on policy, but Puppet cannot verify a blank password with the operating system. Because of this a password change event will be executed by Puppet on every run.")
end
user.password = value
end

Expand Down
7 changes: 2 additions & 5 deletions lib/puppet/util/windows/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,9 @@ def password_is?(name, password)
# Logon failure: user account restriction. Possible reasons are blank passwords
# not allowed, logon hour restrictions, or a policy restriction has been enforced.
if password.nil? || password == ''
raise Puppet::Util::Windows::Error.new(
"Windows does not have the ability to verify NULL or empty passwords"
)
Puppet.warning("Windows does not have the ability to verify NULL or empty passwords")
return false
end
# TODO: maybe do this instead? - return false if password.nil? || password == ''

begin
logon_user(name, password) { |token| }
rescue Puppet::Util::Windows::Error
Expand Down
9 changes: 7 additions & 2 deletions spec/integration/util/windows/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,13 @@ def expect_logon_failure_error(&block)
expect(Puppet::Util::Windows::User.password_is?(username, bad_password)).to be_falsey
end

it "should raise an error given a nil password" do
expect { Puppet::Util::Windows::User.password_is?(username, nil) }.to raise_error(Puppet::Util::Windows::Error)
it "should return false given a nil password" do
expect(Puppet::Util::Windows::User.password_is?(username, nil)).to be_falsey
end

it "should warn that empty/nil passwords cannot be verified given a nil password" do
Puppet::Util::Windows::User.password_is?(username, nil)
expect(@logs).to have_matching_log(/does not have the ability to verify NULL or empty passwords/)
end

it "should return false given a nil username and an incorrect password" do
Expand Down
23 changes: 16 additions & 7 deletions spec/unit/provider/user/windows_adsi_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
stub_users = names.map{|n| stub(:name => n)}
connection.stubs(:execquery).with('select name from win32_useraccount where localaccount = "TRUE"').returns(stub_users)

#TODO: should never call logon_user
expect(described_class.instances.map(&:name)).to match(names)
end
end
Expand All @@ -35,6 +34,16 @@
expect(provider.user).to be_a(Puppet::Util::Windows::ADSI::User)
end

describe "when retrieving the password property" do
context "when the resource has a nil password" do
it "should never issue a logon attempt" do
resource.stubs(:[]).with(any_of(:name, :password)).returns(nil)
Puppet::Util::Windows::User.expects(:logon_user).never
provider.password
end
end
end

describe "when managing groups" do
it 'should return the list of groups as an array of strings' do
provider.user.stubs(:groups).returns nil
Expand Down Expand Up @@ -228,18 +237,18 @@
end

it "should generate a warning with an empty password" do
resource[:password] = ''

# expect
provider.user.expects(:password=).with('')
provider.password = ''
expect(@logs).to have_matching_log(/Puppet cannot verify a blank password with the operating system/)
end

it "should generate a warning with a nil password" do
# TODO: can we use error code 1327 to identify that a user in fact has an empty password
# i.e. is the error code returned when logging in unsuccessfully with a valid empty password
# any different than trying to login with an invalid non-empty password?
resource[:password] = ''

# expect
provider.user.expects(:password=).with(nil)
provider.password = nil
expect(@logs).to have_matching_log(/Puppet does not support setting a nil password on Windows/)
end

it 'should not create a user if a group by the same name exists' do
Expand Down