Greetings!
Please review the pull request #120: (#9326) Support plaintext passwords in Windows 'user' provider opened by (ChaseMotorman)
Some more information about the pull request:
- Opened: Fri Sep 16 17:47:44 UTC 2011
- Based on: puppetlabs:2.7.x (5210f8d7748158d47bb83aaeb658fa621587e111)
- Requested merge: ChaseMotorman:ticket/2.7.x/9326-windows-cleartext-passwords (1551ecf7384b9fdea2f1435d2a0320f7fa423046)
Description:
This commit adds password support to the Windows 'user' provider. When setting the password, the resource value is passed unmolested to the underlying ADSI 'SetPassword' method. During a get, the password is checked using the Windows 'LoginUser' function with the current user name and password. If the call succeeds, the password is returned; if unsuccessful, ':absent' is returned.
The spec test has been updated to reflect these changes.
Thanks!
The Pull Request Bot
Diff follows:
diff --git a/lib/puppet/provider/group/windows_adsi.rb b/lib/puppet/provider/group/windows_adsi.rb
index 4468d00..2ca8522 100644
--- a/lib/puppet/provider/group/windows_adsi.rb
+++ b/lib/puppet/provider/group/windows_adsi.rb
@@ -34,6 +34,11 @@ Puppet::Type.type(:group).provide :windows_adsi do
Puppet::Util::ADSI::Group.delete(@resource[:name])
end
+ # Only flush if we created or modified a group, not deleted
+ def flush
+ @group.commit if @group
+ end
+
def gid
nil
end
diff --git a/lib/puppet/provider/user/windows_adsi.rb b/lib/puppet/provider/user/windows_adsi.rb
index 9250def..8d629ed 100644
--- a/lib/puppet/provider/user/windows_adsi.rb
+++ b/lib/puppet/provider/user/windows_adsi.rb
@@ -7,7 +7,7 @@ Puppet::Type.type(:user).provide :windows_adsi do
confine :operatingsystem => :windows
confine :feature => :microsoft_windows
- has_features :manages_homedir
+ has_features :manages_homedir, :manages_passwords
def user
@user ||= Puppet::Util::ADSI::User.new(@resource[:name])
@@ -23,6 +23,7 @@ Puppet::Type.type(:user).provide :windows_adsi do
def create
@user = Puppet::Util::ADSI::User.create(@resource[:name])
+ @user.commit if @user #9459: ensure the state is set before enumerating groups
[:comment, :home, :groups].each do |prop|
send("#{prop}=", @resource[prop]) if @resource[prop]
end
@@ -57,6 +58,14 @@ Puppet::Type.type(:user).provide :windows_adsi do
user['HomeDirectory'] = value
end
+ def password
+ user.password_is?( @resource[:password] ) ? @resource[:password] : :absent
+ end
+
+ def password=(value)
+ user.password = value
+ end
+
[:uid, :gid, :shell].each do |prop|
define_method(prop) { nil }
diff --git a/lib/puppet/util/adsi.rb b/lib/puppet/util/adsi.rb
index f865743..dfa8a04 100644
--- a/lib/puppet/util/adsi.rb
+++ b/lib/puppet/util/adsi.rb
@@ -127,7 +127,7 @@ module Puppet::Util::ADSI
def groups
# WIN32OLE objects aren't enumerable, so no map
groups = []
- native_user.Groups.each {|g| groups << g.Name}
+ native_user.Groups.each {|g| groups << g.Name} rescue nil
groups
end
@@ -163,6 +163,8 @@ module Puppet::Util::ADSI
end
def self.create(name)
+ # Windows error 1379: The specified local group already exists.
+ raise Puppet::Error.new( "Cannot create user if group '#{name}' exists." ) if Puppet::Util::ADSI::Group.exists? name
new(name, Puppet::Util::ADSI.create(name, 'user'))
end
@@ -253,6 +255,8 @@ module Puppet::Util::ADSI
end
def self.create(name)
+ # Windows error 2224: The account already exists.
+ raise Puppet::Error.new( "Cannot create group if user '#{name}' exists." ) if Puppet::Util::ADSI::User.exists? name
new(name, Puppet::Util::ADSI.create(name, 'group'))
end
diff --git a/spec/unit/provider/group/windows_adsi_spec.rb b/spec/unit/provider/group/windows_adsi_spec.rb
index 7faaa1a..2186fa5 100644
--- a/spec/unit/provider/group/windows_adsi_spec.rb
+++ b/spec/unit/provider/group/windows_adsi_spec.rb
@@ -47,20 +47,36 @@ describe Puppet::Type.type(:group).provider(:windows_adsi) do
end
end
- it "should be able to create a group" do
- resource[:members] = ['user1', 'user2']
+ describe 'when creating groups' do
- group = stub 'group'
- Puppet::Util::ADSI::Group.expects(:create).with('testers').returns group
+ it "should be able to create a group" do
+ resource[:members] = ['user1', 'user2']
- group.expects(:set_members).with(['user1', 'user2'])
+ group = stub 'group'
+ Puppet::Util::ADSI::Group.expects(:create).with('testers').returns group
+
+ group.expects(:set_members).with(['user1', 'user2'])
+
+ provider.create
+ end
+
+ it 'should not create a group if a user by the same name exists' do
+ Puppet::Util::ADSI::Group.expects(:create).with('testers').raises( Puppet::Error.new("Cannot create group if user 'testers' exists.") )
+ expect{ provider.create }.to raise_error( Puppet::Error,
+ /Cannot create group if user 'testers' exists./ )
+ end
+
+ it 'should commit a newly created group' do
+ provider.group.expects( :commit )
+
+ provider.flush
+ end
- provider.create
end
it "should be able to test whether a group exists" do
Puppet::Util::ADSI.stubs(:connect).returns stub('connection')
- provider.should be_exists
+ provider.should be_exists if Puppet.features.microsoft_windows?
Puppet::Util::ADSI.stubs(:connect).returns nil
provider.should_not be_exists
@@ -77,3 +93,4 @@ describe Puppet::Type.type(:group).provider(:windows_adsi) do
provider.send(:gid=, 500)
end
end
+
diff --git a/spec/unit/provider/user/windows_adsi_spec.rb b/spec/unit/provider/user/windows_adsi_spec.rb
old mode 100644
new mode 100755
index 073a3d3..520efbe
--- a/spec/unit/provider/user/windows_adsi_spec.rb
+++ b/spec/unit/provider/user/windows_adsi_spec.rb
@@ -62,6 +62,7 @@ describe Puppet::Type.type(:user).provider(:windows_adsi) do
end
describe "when creating a user" do
+
it "should create the user on the system and set its other properties" do
resource[:groups] = ['group1', 'group2']
resource[:membership] = :inclusive
@@ -76,14 +77,43 @@ describe Puppet::Type.type(:user).provider(:windows_adsi) do
user.expects(:set_groups).with('group1,group2', false)
user.expects(:[]=).with('Description', 'a test user')
user.expects(:[]=).with('HomeDirectory', 'C:\Users\testuser')
+ user.expects(:commit) #9459: ensure the state is set before enumerating groups
provider.create
end
+
+ it 'should not create a user if a group by the same name exists' do
+ Puppet::Util::ADSI::User.expects(:create).with('testuser').raises( Puppet::Error.new("Cannot create user if group 'testuser' exists.") )
+ expect{ provider.create }.to raise_error( Puppet::Error,
+ /Cannot create user if group 'testuser' exists./ )
+ end
+
+ it "should set a user's password" do
+ provider.user.expects(:password=).with('plaintextbad')
+
+ provider.password = "plaintextbad"
+ end
+
+ it "should test a valid user password" do
+ resource[:password] = 'plaintext'
+ provider.user.expects(:password_is?).with('plaintext').returns true
+
+ provider.password.should == 'plaintext'
+
+ end
+
+ it "should test a bad user password" do
+ resource[:password] = 'plaintext'
+ provider.user.expects(:password_is?).with('plaintext').returns false
+
+ provider.password.should == :absent
+ end
+
end
it 'should be able to test whether a user exists' do
Puppet::Util::ADSI.stubs(:connect).returns stub('connection')
- provider.should be_exists
+ provider.should be_exists if Puppet.features.microsoft_windows?
Puppet::Util::ADSI.stubs(:connect).returns nil
provider.should_not be_exists
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
