Comment inline. -- Jacob Helwig
On Tue, 28 Sep 2010 18:57:05 -0400, Nick Lewis wrote: > > This adds a new feature to user providers "manages_password_age", along > with properties password_min_age and password_max_age to the user type. > These represent password min and max age in days. The useradd and > user_role_add providers now support these new properties. > > Signed-off-by: Nick Lewis <[email protected]> > --- > lib/puppet/provider/nameservice.rb | 1 + > lib/puppet/provider/nameservice/objectadd.rb | 3 +- > lib/puppet/provider/user/hpux.rb | 1 - > lib/puppet/provider/user/user_role_add.rb | 23 ++++++++++++-- > lib/puppet/provider/user/useradd.rb | 35 ++++++++++++++++++++- > lib/puppet/type/user.rb | 41 > +++++++++++++++++++++++++ > spec/unit/provider/user/user_role_add_spec.rb | 12 +++++++- > spec/unit/provider/user/useradd_spec.rb | 9 +++++ > spec/unit/type/user_spec.rb | 6 +++- > 9 files changed, 122 insertions(+), 9 deletions(-) > > diff --git a/lib/puppet/provider/nameservice.rb > b/lib/puppet/provider/nameservice.rb > index 7339b64..9830fab 100644 > --- a/lib/puppet/provider/nameservice.rb > +++ b/lib/puppet/provider/nameservice.rb > @@ -165,6 +165,7 @@ class Puppet::Provider::NameService < Puppet::Provider > > begin > execute(self.addcmd) > + execute(self.passcmd) if self.feature? :manages_password_age > rescue Puppet::ExecutionFailure => detail > raise Puppet::Error, "Could not create #[email protected]} > #[email protected]}: #{detail}" > end > diff --git a/lib/puppet/provider/nameservice/objectadd.rb > b/lib/puppet/provider/nameservice/objectadd.rb > index 80c1429..dbb9f30 100644 > --- a/lib/puppet/provider/nameservice/objectadd.rb > +++ b/lib/puppet/provider/nameservice/objectadd.rb > @@ -13,7 +13,8 @@ class ObjectAdd < Puppet::Provider::NameService > end > > def modifycmd(param, value) > - cmd = [command(:modify), flag(param), value] > + cmd = [command(param.to_s =~ /password_.+_age/ ? :password : :modify)] > + cmd << flag(param) << value > if @resource.allowdupe? && ((param == :uid) || (param == :gid and > self.class.name == :groupadd)) > cmd << "-o" > end > diff --git a/lib/puppet/provider/user/hpux.rb > b/lib/puppet/provider/user/hpux.rb > index 50506c4..9839709 100644 > --- a/lib/puppet/provider/user/hpux.rb > +++ b/lib/puppet/provider/user/hpux.rb > @@ -26,5 +26,4 @@ Puppet::Type.type(:user).provide :hpuxuseradd, :parent => > :useradd do > def modifycmd(param,value) > super.insert(1,"-F") > end > - > end > diff --git a/lib/puppet/provider/user/user_role_add.rb > b/lib/puppet/provider/user/user_role_add.rb > index c131259..dd91de3 100644 > --- a/lib/puppet/provider/user/user_role_add.rb > +++ b/lib/puppet/provider/user/user_role_add.rb > @@ -6,13 +6,15 @@ Puppet::Type.type(:user).provide :user_role_add, :parent => > :useradd, :source => > > defaultfor :operatingsystem => :solaris > > - commands :add => "useradd", :delete => "userdel", :modify => "usermod", > :role_add => "roleadd", :role_delete => "roledel", :role_modify => "rolemod" > + commands :add => "useradd", :delete => "userdel", :modify => "usermod", > :password => "chage", :role_add => "roleadd", :role_delete => "roledel", > :role_modify => "rolemod" > options :home, :flag => "-d", :method => :dir > options :comment, :method => :gecos > options :groups, :flag => "-G" > options :roles, :flag => "-R" > options :auths, :flag => "-A" > options :profiles, :flag => "-P" > + options :password_min_age, :flag => "-m" > + options :password_max_age, :flag => "-M" > > verify :gid, "GID must be an integer" do |value| > value.is_a? Integer > @@ -22,14 +24,14 @@ Puppet::Type.type(:user).provide :user_role_add, :parent > => :useradd, :source => > value !~ /\s/ > end > > - has_features :manages_homedir, :allows_duplicates, :manages_solaris_rbac, > :manages_passwords > + has_features :manages_homedir, :allows_duplicates, :manages_solaris_rbac, > :manages_passwords, :manages_password_age > > #must override this to hand the keyvalue pairs > def add_properties > cmd = [] > Puppet::Type.type(:user).validproperties.each do |property| > #skip the password because we can't create it with the solaris useradd > - next if [:ensure, :password].include?(property) > + next if [:ensure, :password, :password_min_age, > :password_max_age].include?(property) > # 1680 Now you can set the hashed passwords on > solaris:lib/puppet/provider/user/user_role_add.rb > # the value needs to be quoted, mostly because -c might > # have spaces in it > @@ -79,6 +81,7 @@ Puppet::Type.type(:user).provide :user_role_add, :parent => > :useradd, :source => > run(transition("normal"), "transition role to") > else > run(addcmd, "create") > + run(passcmd, "change password policy for") > end > # added to handle case when password is specified > self.password = @resource[:password] if @resource[:password] > @@ -150,6 +153,20 @@ Puppet::Type.type(:user).provide :user_role_add, :parent > => :useradd, :source => > pass > end > > + def min_age > + if ary = File.readlines("/etc/shadow").reject { |r| r =~ > /^[^\w]/}.collect { |l| l.split(':')[0..3] }.find { |user, _, _, minage| user > == @resource[:name] } > + minage = ary[3] > + end > + :absent > + end > + > + def max_age > + if ary = File.readlines("/etc/shadow").reject { |r| r =~ > /^[^\w]/}.collect { |l| l.split(':')[0..3] }.find { |user, _, _, _, maxage| > user == @resource[:name] } > + maxage = ary[4] This doesn't look like it would actually work? Wouldn't the "[0..3]" need to be "[0..4]" here, since you're looking for "ary[4]"? Which leads me to believe that this line isn't actually being tested, unless I'm completely misunderstanding it. Also: Might be worth putting a space before the closing } on the reject, since it gets lost in the line there, and I had to scan a couple of times to find it. > + end > + :absent > + end > + > #Read in /etc/shadow, find the line for our used and rewrite it with the > new pw > #Smooth like 80 grit > def password=(cryptopw) > diff --git a/lib/puppet/provider/user/useradd.rb > b/lib/puppet/provider/user/useradd.rb > index 7ef217d..a305fb5 100644 > --- a/lib/puppet/provider/user/useradd.rb > +++ b/lib/puppet/provider/user/useradd.rb > @@ -3,11 +3,13 @@ require 'puppet/provider/nameservice/objectadd' > Puppet::Type.type(:user).provide :useradd, :parent => > Puppet::Provider::NameService::ObjectAdd do > desc "User management via `useradd` and its ilk. Note that you will need > to install the `Shadow Password` Ruby library often known as ruby-libshadow > to manage user passwords." > > - commands :add => "useradd", :delete => "userdel", :modify => "usermod" > + commands :add => "useradd", :delete => "userdel", :modify => "usermod", > :password => "chage" > > options :home, :flag => "-d", :method => :dir > options :comment, :method => :gecos > options :groups, :flag => "-G" > + options :password_min_age, :flag => "-m" > + options :password_max_age, :flag => "-M" > > verify :gid, "GID must be an integer" do |value| > value.is_a? Integer > @@ -19,7 +21,7 @@ Puppet::Type.type(:user).provide :useradd, :parent => > Puppet::Provider::NameServ > > has_features :manages_homedir, :allows_duplicates > > - has_feature :manages_passwords if Puppet.features.libshadow? > + has_features :manages_passwords, :manages_password_age if > Puppet.features.libshadow? > > def check_allow_dup > @resource.allowdupe? ? ["-o"] : [] > @@ -39,6 +41,7 @@ Puppet::Type.type(:user).provide :useradd, :parent => > Puppet::Provider::NameServ > cmd = [] > Puppet::Type.type(:user).validproperties.each do |property| > next if property == :ensure > + next if property.to_s =~ /password_.+_age/ > # the value needs to be quoted, mostly because -c might > # have spaces in it > if value = @resource.should(property) and value != "" > @@ -56,6 +59,34 @@ Puppet::Type.type(:user).provide :useradd, :parent => > Puppet::Provider::NameServ > cmd << @resource[:name] > end > > + def passcmd > + cmd = [command(:password)] > + [:password_min_age, :password_max_age].each do |property| > + if value = @resource.should(property) > + cmd << flag(property) << value > + end > + end > + cmd << @resource[:name] > + end > + > + def min_age > + if Puppet.features.libshadow? > + if ent = Shadow::Passwd.getspnam(@resource.name) > + return ent.sp_min > + end > + end > + :absent > + end > + > + def max_age > + if Puppet.features.libshadow? > + if ent = Shadow::Passwd.getspnam(@resource.name) > + return ent.sp_max > + end > + end > + :absent > + end > + > # Retrieve the password using the Shadow Password library > def password > if Puppet.features.libshadow? > diff --git a/lib/puppet/type/user.rb b/lib/puppet/type/user.rb > index 007b760..7b50d92 100755 > --- a/lib/puppet/type/user.rb > +++ b/lib/puppet/type/user.rb > @@ -24,6 +24,10 @@ module Puppet > "The provider can modify user passwords, by accepting a password > hash." > > + feature :manages_password_age, > + "The provider can set age requirements and restrictions for > + passwords." > + > feature :manages_solaris_rbac, > "The provider can manage roles and normal users" > > @@ -157,6 +161,43 @@ module Puppet > end > end > > + newproperty(:password_min_age, :required_features => > :manages_password_age) do > + desc "The minimum amount of time in days a password must be used > before it may be changed" > + > + munge do |value| > + case value > + when String > + Integer(value) > + else > + value > + end > + end > + > + validate do |value| > + if value.to_s !~ /^\d+$/ > + raise ArgumentError, "Password minimum age must be provided as a > number" > + end > + end > + end > + > + newproperty(:password_max_age, :required_features => > :manages_password_age) do > + desc "The maximum amount of time in days a password may be used before > it must be changed" > + > + munge do |value| > + case value > + when String > + Integer(value) > + else > + value > + end > + end > + > + validate do |value| > + if value.to_s !~ /^\d+$/ > + raise ArgumentError, "Password maximum age must be provided as a > number" > + end > + end > + end > > newproperty(:groups, :parent => Puppet::Property::List) do > desc "The groups of which the user is a member. The primary > diff --git a/spec/unit/provider/user/user_role_add_spec.rb > b/spec/unit/provider/user/user_role_add_spec.rb > index 211f426..298ac22 100644 > --- a/spec/unit/provider/user/user_role_add_spec.rb > +++ b/spec/unit/provider/user/user_role_add_spec.rb > @@ -56,7 +56,7 @@ describe provider_class do > it "should use the add command when the user is not a role" do > @provider.stubs(:is_role?).returns(false) > @provider.expects(:addcmd).returns("useradd") > - @provider.expects(:run) > + @provider.expects(:run).at_least_once > @provider.create > end > > @@ -66,6 +66,15 @@ describe provider_class do > @provider.expects(:run) > @provider.create > end > + > + it "should set password age rules" do > + @resource = Puppet::Type.type(:user).new :name => "myuser", > :password_min_age => 5, :password_max_age => 10 > + @provider = provider_class.new(@resource) > + @provider.stubs(:user_attributes) > + @provider.stubs(:execute) > + @provider.expects(:execute).with { |cmd, *args| args == ["-m", 5, > "-M", 10, "myuser"] } > + @provider.create > + end > end > > describe "when calling destroy" do > @@ -107,6 +116,7 @@ describe provider_class do > before do > @resource.expects(:allowdupe?).returns true > @provider.stubs(:is_role?).returns(false) > + @provider.stubs(:execute) > @provider.expects(:execute).with { |args| args.include?("-o") } > end > > diff --git a/spec/unit/provider/user/useradd_spec.rb > b/spec/unit/provider/user/useradd_spec.rb > index 6eb9717..c796cdc 100755 > --- a/spec/unit/provider/user/useradd_spec.rb > +++ b/spec/unit/provider/user/useradd_spec.rb > @@ -15,6 +15,7 @@ describe provider_class do > # #1360 > it "should add -o when allowdupe is enabled and the user is being created" > do > @resource.expects(:allowdupe?).returns true > + @provider.stubs(:execute) > @provider.expects(:execute).with { |args| args.include?("-o") } > @provider.create > end > @@ -26,6 +27,14 @@ describe provider_class do > @provider.uid = 150 > end > > + it "should set password age rules" do > + @resource = Puppet::Type.type(:user).new :name => "myuser", > :password_min_age => 5, :password_max_age => 10 > + @provider = provider_class.new(@resource) > + @provider.stubs(:execute) > + @provider.expects(:execute).with { |cmd, *args| args == ["-m", 5, "-M", > 10, "myuser"] } > + @provider.create > + end > + > describe "when checking to add allow dup" do > it "should check allow dup" do > @resource.expects(:allowdupe?) > diff --git a/spec/unit/type/user_spec.rb b/spec/unit/type/user_spec.rb > index 4c6eb11..f71b39a 100755 > --- a/spec/unit/type/user_spec.rb > +++ b/spec/unit/type/user_spec.rb > @@ -35,6 +35,10 @@ describe user do > user.provider_feature(:manages_solaris_rbac).should_not be_nil > end > > + it "should have a manages_password_age feature" do > + user.provider_feature(:manages_password_age).should_not be_nil > + end > + > describe "instances" do > it "should have a valid provider" do > user.new(:name => "foo").provider.class.ancestors.should > be_include(Puppet::Provider) > @@ -47,7 +51,7 @@ describe user do > end > end > > - properties = [:ensure, :uid, :gid, :home, :comment, :shell, :password, > :groups, :roles, :auths, :profiles, :project, :keys] > + properties = [:ensure, :uid, :gid, :home, :comment, :shell, :password, > :password_min_age, :password_max_age, :groups, :roles, :auths, :profiles, > :project, :keys] > > properties.each do |property| > it "should have a #{property} property" do > -- > 1.7.2.1 >
signature.asc
Description: Digital signature
