rewrite linu_user provider check_lock

this removes the kinda brittle kinda insane parsing of the installed
rpm version.  now we're just less strict about the exit 1 code and
fail on the output not being what is expected.

this also likely fixes a bug in rhel in why-run mode where shell_out!
would have failed on the user-not-found and raised an exception that
would have ended the run.  the code before certainly looks buggy.

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

2 files changed, 35 insertions, 69 deletions

diff --git a/lib/chef/provider/user/linux.rb b/lib/chef/provider/user/linux.rb
index ff1bc9c4c8..f09d86e98c 100644
--- a/lib/chef/provider/user/linux.rb
+++ b/lib/chef/provider/user/linux.rb
@@ -94,44 +94,32 @@ class Chef
           new_resource.home && Pathname.new(current_resource.home).cleanpath != Pathname.new(new_resource.home).cleanpath
         end
 
-        # FIXME: see if we can clean this up
         def check_lock
-          # we can get an exit code of 1 even when it's successful on
-          # rhel/centos (redhat bug 578534). See additional error checks below.
-          passwd_s = shell_out!("passwd", "-S", new_resource.username, :returns => [0, 1])
-          if whyrun_mode? && passwd_s.stdout.empty? && passwd_s.stderr.match(/does not exist/)
-            # if we're in whyrun mode and the user is not yet created we assume it would be
-            return false
+          # there's an old bug in rhel (https://bugzilla.redhat.com/show_bug.cgi?id=578534)
+          # which means that both 0 and 1 can be success.
+          passwd_s = shell_out("passwd", "-S", new_resource.username, returns: [ 0, 1 ])
+
+          # checking "does not exist" has to come before exit code handling since centos and ubuntu differ in exit codes
+          if passwd_s.stderr =~ /does not exist/
+            if whyrun_mode?
+              return false
+            else
+              raise Chef::Exceptions::User, "User #{new_resource.username} does not exist when checking lock status for #{new_resource}"
+            end
           end
 
-          raise Chef::Exceptions::User, "Cannot determine if #{@new_resource} is locked!" if passwd_s.stdout.empty?
+          # now raise if we didn't get a 0 or 1 (see above)
+          passwd_s.error!
 
+          # now the actual output parsing
+          @locked = nil
           status_line = passwd_s.stdout.split(" ")
-          case status_line[1]
-          when /^P/
-            @locked = false
-          when /^N/
-            @locked = false
-          when /^L/
-            @locked = true
-          end
-
-          unless passwd_s.exitstatus == 0
-            raise_lock_error = false
-            if %w{redhat centos}.include?(node[:platform])
-              passwd_version_check = shell_out!("rpm -q passwd")
-              passwd_version = passwd_version_check.stdout.chomp
+          @locked = false if status_line[1] =~ /^[PN]/
+          @locked = true if status_line[1] =~ /^L/
 
-              unless passwd_version == "passwd-0.73-1"
-                raise_lock_error = true
-              end
-            else
-              raise_lock_error = true
-            end
-
-            raise Chef::Exceptions::User, "Cannot determine if #{new_resource} is locked!" if raise_lock_error
-          end
+          raise Chef::Exceptions::User, "Cannot determine if user #{new_resource.username} is locked for #{new_resource}" if @locked.nil?
 
+          # FIXME: should probably go on the current_resource
           @locked
         end
       end
diff --git a/spec/support/shared/unit/provider/useradd_based_user_provider.rb b/spec/support/shared/unit/provider/useradd_based_user_provider.rb
index fc12b2d5b6..9392c95e95 100644
--- a/spec/support/shared/unit/provider/useradd_based_user_provider.rb
+++ b/spec/support/shared/unit/provider/useradd_based_user_provider.rb
@@ -257,7 +257,7 @@ shared_examples_for "a useradd-based user provider" do |supported_useradd_option
   describe "when checking the lock" do
     # lazy initialize so we can modify stdout and stderr strings
     let(:passwd_s_status) do
-      double("Mixlib::ShellOut command", :exitstatus => 0, :stdout => @stdout, :stderr => @stderr)
+      double("Mixlib::ShellOut command", :exitstatus => 0, :stdout => @stdout, :stderr => @stderr, :error! => nil)
     end
 
     before(:each) do
@@ -272,7 +272,7 @@ shared_examples_for "a useradd-based user provider" do |supported_useradd_option
     end
 
     it "should return false if status begins with P" do
-      expect(provider).to receive(:shell_out!).
+      expect(provider).to receive(:shell_out).
         with("passwd", "-S", @new_resource.username, { :returns => [0, 1] }).
         and_return(passwd_s_status)
       expect(provider.check_lock).to eql(false)
@@ -280,7 +280,7 @@ shared_examples_for "a useradd-based user provider" do |supported_useradd_option
 
     it "should return false if status begins with N" do
       @stdout = "root N"
-      expect(provider).to receive(:shell_out!).
+      expect(provider).to receive(:shell_out).
         with("passwd", "-S", @new_resource.username, { :returns => [0, 1] }).
         and_return(passwd_s_status)
       expect(provider.check_lock).to eql(false)
@@ -288,55 +288,33 @@ shared_examples_for "a useradd-based user provider" do |supported_useradd_option
 
     it "should return true if status begins with L" do
       @stdout = "root L"
-      expect(provider).to receive(:shell_out!).
+      expect(provider).to receive(:shell_out).
         with("passwd", "-S", @new_resource.username, { :returns => [0, 1] }).
         and_return(passwd_s_status)
       expect(provider.check_lock).to eql(true)
     end
 
-    it "should raise a Chef::Exceptions::User if passwd -S fails on anything other than redhat/centos" do
-      @node.automatic_attrs[:platform] = "ubuntu"
-      expect(provider).to receive(:shell_out!).
+    it "should raise a ShellCommandFailed exception if passwd -S exits with something other than 0 or 1" do
+      expect(passwd_s_status).to receive(:error!).and_raise(Mixlib::ShellOut::ShellCommandFailed)
+      expect(provider).to receive(:shell_out).
         with("passwd", "-S", @new_resource.username, { :returns => [0, 1] }).
         and_return(passwd_s_status)
-      expect(passwd_s_status).to receive(:exitstatus).and_return(1)
-      expect { provider.check_lock }.to raise_error(Chef::Exceptions::User)
+      expect { provider.check_lock }.to raise_error(Mixlib::ShellOut::ShellCommandFailed)
     end
 
-    %w{redhat centos}.each do |os|
-      it "should not raise a Chef::Exceptions::User if passwd -S exits with 1 on #{os} and the passwd package is version 0.73-1" do
-        @node.automatic_attrs[:platform] = os
-        expect(passwd_s_status).to receive(:exitstatus).and_return(1)
-        expect(provider).to receive(:shell_out!).
-          with("passwd", "-S", @new_resource.username, { :returns => [0, 1] }).
-          and_return(passwd_s_status)
-        rpm_status = double("Mixlib::ShellOut command", :exitstatus => 0, :stdout => "passwd-0.73-1\n", :stderr => "")
-        expect(provider).to receive(:shell_out!).with("rpm -q passwd").and_return(rpm_status)
-        expect { provider.check_lock }.not_to raise_error
-      end
-
-      it "should raise a Chef::Exceptions::User if passwd -S exits with 1 on #{os} and the passwd package is not version 0.73-1" do
-        @node.automatic_attrs[:platform] = os
-        expect(passwd_s_status).to receive(:exitstatus).and_return(1)
-        expect(provider).to receive(:shell_out!).
-          with("passwd", "-S", @new_resource.username, { :returns => [0, 1] }).
-          and_return(passwd_s_status)
-        rpm_status = double("Mixlib::ShellOut command", :exitstatus => 0, :stdout => "passwd-0.73-2\n", :stderr => "")
-        expect(provider).to receive(:shell_out!).with("rpm -q passwd").and_return(rpm_status)
-        expect { provider.check_lock }.to raise_error(Chef::Exceptions::User)
-      end
-
-      it "should raise a ShellCommandFailed exception if passwd -S exits with something other than 0 or 1 on #{os}" do
-        @node.automatic_attrs[:platform] = os
-        expect(provider).to receive(:shell_out!).and_raise(Mixlib::ShellOut::ShellCommandFailed)
-        expect { provider.check_lock }.to raise_error(Mixlib::ShellOut::ShellCommandFailed)
-      end
+    it "should raise an error if the output isn't parsable" do
+      expect(passwd_s_status).to receive(:stdout).and_return("")
+      expect(passwd_s_status).to receive(:stderr).and_return("")
+      expect(provider).to receive(:shell_out).
+        with("passwd", "-S", @new_resource.username, { :returns => [0, 1] }).
+        and_return(passwd_s_status)
+      expect { provider.check_lock }.to raise_error(Chef::Exceptions::User)
     end
 
     context "when in why run mode" do
       before do
         passwd_status = double("Mixlib::ShellOut command", :exitstatus => 0, :stdout => "", :stderr => "passwd: user 'chef-test' does not exist\n")
-        expect(provider).to receive(:shell_out!).
+        expect(provider).to receive(:shell_out).
           with("passwd", "-S", @new_resource.username, { :returns => [0, 1] }).
           and_return(passwd_status)
         Chef::Config[:why_run] = true