Merge pull request #5678 from Klaas-/Klaas-patch1

(PUP-7292) Change to allow set of password_warn_days

lib/puppet/provider/user/aix.rb

@@ -72,6 +72,7 @@ def self.attribute_ignore
                                 :from => :expiry_from_attr },
     {:aix_attr => :maxage,     :puppet_prop => :password_max_age},
     {:aix_attr => :minage,     :puppet_prop => :password_min_age},
+    {:aix_attr => :pwdwarntime, :puppet_prop => :password_warn_days},
     {:aix_attr => :attributes, :puppet_prop => :attributes},
     { :aix_attr => :gecos,      :puppet_prop => :comment },
   ]

lib/puppet/provider/user/user_role_add.rb

@@ -18,6 +18,7 @@
   options :profiles, :flag => "-P"
   options :password_min_age, :flag => "-n"
   options :password_max_age, :flag => "-x"
+  options :password_warn_days, :flag => "-w"
 
   verify :gid, "GID must be an integer" do |value|
     value.is_a? Integer
@@ -48,7 +49,7 @@ 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, :password_min_age, :password_max_age].include?(property)
+      next if [:ensure, :password, :password_min_age, :password_max_age, :password_warn_days].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
@@ -196,6 +197,11 @@ def password_max_age
     shadow_entry[4].empty? ? -1 : shadow_entry[4]
   end
 
+  def password_warn_days
+    return :absent unless shadow_entry
+    shadow_entry[5].empty? ? -1 : shadow_entry[5]
+  end
+
   # Read in /etc/shadow, find the line for our used and rewrite it with the
   # new pw.  Smooth like 80 grit sandpaper.
   #

lib/puppet/provider/user/useradd.rb

@@ -16,6 +16,7 @@
   options :groups, :flag => "-G"
   options :password_min_age, :flag => "-m", :method => :sp_min
   options :password_max_age, :flag => "-M", :method => :sp_max
+  options :password_warn_days, :flag => "-W", :method => :sp_warn
   options :password, :method => :sp_pwdp
   options :expiry, :method => :sp_expire,
     :munge => proc { |value|
@@ -192,15 +193,15 @@ def deletecmd
   end
 
   def passcmd
-    age_limits = [:password_min_age, :password_max_age].select { |property| @resource.should(property) }
+    age_limits = [:password_min_age, :password_max_age, :password_warn_days].select { |property| @resource.should(property) }
     if age_limits.empty?
       nil
     else
       [command(:password),age_limits.collect { |property| [flag(property), @resource.should(property)]}, @resource[:name]].flatten
     end
   end
 
-  [:expiry, :password_min_age, :password_max_age, :password].each do |shadow_property|
+  [:expiry, :password_min_age, :password_max_age, :password_warn_days, :password].each do |shadow_property|
     define_method(shadow_property) do
       if Puppet.features.libshadow?
         if ent = Shadow::Passwd.getspnam(@canonical_name)

lib/puppet/type/user.rb

@@ -279,6 +279,25 @@ def should_to_s( newvalue )
       end
     end
 
+    newproperty(:password_warn_days, :required_features => :manages_password_age) do
+      desc "The number of days before a password is going to expire (see the maximum password age) during which the user should be warned."
+
+      munge do |value|
+        case value
+        when String
+          Integer(value)
+        else
+          value
+        end
+      end
+
+      validate do |value|
+        if value.to_s !~ /^-?\d+$/
+          raise ArgumentError, "Password warning days must be provided as a number."
+        end
+      end
+    end
+
     newproperty(:groups, :parent => Puppet::Property::List) do
       desc "The groups to which the user belongs.  The primary group should
         not be listed, and groups should be identified by name rather than by

spec/unit/provider/user/user_role_add_spec.rb

@@ -70,11 +70,11 @@
     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 => :user_role_add
+      resource = Puppet::Type.type(:user).new :name => "myuser", :password_min_age => 5, :password_max_age => 10, :password_warn_days => 15, :provider => :user_role_add
       provider = described_class.new(resource)
       provider.stubs(:user_attributes)
       provider.stubs(:execute)
-      provider.expects(:execute).with { |cmd, *args| args == ["-n", 5, "-x", 10, "myuser"] }
+      provider.expects(:execute).with { |cmd, *args| args == ["-n", 5, "-x", 10, '-w', 15, "myuser"] }
       provider.create
     end
   end
@@ -354,4 +354,21 @@ def write_fixture(content)
       expect(provider.password_min_age).to eq(-1)
     end
   end
+
+  describe "#password_warn_days" do
+    it "should return a warn days number" do
+      File.stubs(:readlines).returns(["fakeval:NP:12345:10:50:30:::\n"])
+      expect(provider.password_warn_days).to eq("30")
+    end
+
+    it "should return -1 for no warn days" do
+      File.stubs(:readlines).returns(["fakeval:NP:12345::::::\n"])
+      expect(provider.password_warn_days).to eq(-1)
+    end
+
+    it "should return -1 for no warn days when failed attempts are present" do
+      File.stubs(:readlines).returns(["fakeval:NP:12345::::::3\n"])
+      expect(provider.password_warn_days).to eq(-1)
+    end
+  end
 end

spec/unit/provider/user/useradd_spec.rb

@@ -94,8 +94,9 @@
       described_class.has_feature :manages_password_age
       resource[:password_min_age] = 5
       resource[:password_max_age] = 10
+      resource[:password_warn_days] = 15
       provider.expects(:execute).with(includes('/usr/sbin/useradd'), kind_of(Hash))
-      provider.expects(:execute).with(['/usr/bin/chage', '-m', 5, '-M', 10, 'myuser'])
+      provider.expects(:execute).with(['/usr/bin/chage', '-m', 5, '-M', 10, '-W', 15, 'myuser'])
       provider.create
     end
 
@@ -340,9 +341,10 @@
   end
 
   {
-    :password_min_age => 10,
-    :password_max_age => 20,
-    :password         => '$6$FvW8Ib8h$qQMI/CR9m.QzIicZKutLpBgCBBdrch1IX0rTnxuI32K1pD9.RXZrmeKQlaC.RzODNuoUtPPIyQDufunvLOQWF0'
+    :password_min_age   => 10,
+    :password_max_age   => 20,
+    :password_warn_days => 30,
+    :password           => '$6$FvW8Ib8h$qQMI/CR9m.QzIicZKutLpBgCBBdrch1IX0rTnxuI32K1pD9.RXZrmeKQlaC.RzODNuoUtPPIyQDufunvLOQWF0'
   }.each_pair do |property, expected_value|
     describe "##{property}" do
       before :each do
@@ -437,11 +439,23 @@
       expect(provider.passcmd).to eq(['/usr/bin/chage','-M',999,'myuser'])
     end
 
+    it "should return a chage command array with -W <value> if password_warn_days is set" do
+      resource[:password_warn_days] = 999
+      expect(provider.passcmd).to eq(['/usr/bin/chage','-W',999,'myuser'])
+    end
+
     it "should return a chage command array with -M <value> -m <value> if both password_min_age and password_max_age are set" do
       resource[:password_min_age] = 123
       resource[:password_max_age] = 999
       expect(provider.passcmd).to eq(['/usr/bin/chage','-m',123,'-M',999,'myuser'])
     end
+
+    it "should return a chage command array with -M <value> -m <value> -W <value> if password_min_age, password_max_age and password_warn_days are set" do
+      resource[:password_min_age] = 123
+      resource[:password_max_age] = 999
+      resource[:password_warn_days] = 555
+      expect(provider.passcmd).to eq(['/usr/bin/chage','-m',123,'-M',999,'-W',555,'myuser'])
+    end
   end
 
   describe "#check_valid_shell" do

spec/unit/type/user_spec.rb

@@ -86,7 +86,7 @@ def self.instances; []; end
     end
   end
 
-  properties = [:ensure, :uid, :gid, :home, :comment, :shell, :password, :password_min_age, :password_max_age, :groups, :roles, :auths, :profiles, :project, :keys, :expiry]
+  properties = [:ensure, :uid, :gid, :home, :comment, :shell, :password, :password_min_age, :password_max_age, :password_warn_days, :groups, :roles, :auths, :profiles, :project, :keys, :expiry]
 
   properties.each do |property|
     it "should have a #{property} property" do
@@ -314,6 +314,16 @@ def self.instances; []; end
     end
   end
 
+  describe "when managing warning password days" do
+    it "should accept a negative warning days" do
+      expect { described_class.new(:name => 'foo', :password_warn_days => '-1') }.to_not raise_error
+    end
+
+    it "should fail with an empty warning days" do
+      expect { described_class.new(:name => 'foo', :password_warn_days => '') }.to raise_error(Puppet::Error, /warning days must be provided as a number/)
+    end
+  end
+
   describe "when managing passwords" do
     before do
       @password = described_class.new(:name => 'foo', :password => 'mypass').parameter(:password)