clean up everything

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

6 files changed, 165 insertions, 47 deletions

diff --git a/lib/chef/mixin/shell_out.rb b/lib/chef/mixin/shell_out.rb
index 1f7deb21d2..a258a91075 100644
--- a/lib/chef/mixin/shell_out.rb
+++ b/lib/chef/mixin/shell_out.rb
@@ -78,6 +78,36 @@ class Chef
         return my_command_args
       end
 
+      # Helper for sublcasses to convert an array of string args into a string.  It
+      # will compact nil or empty strings in the array and will join the array elements
+      # with spaces, without introducing any double spaces for nil/empty elements.
+      #
+      # @param args [String] variable number of string arguments
+      # @return [String] nicely concatenated string or empty string
+      def a_to_s(*args)
+        clean_array(*args).join(" ")
+      end
+
+      # Helper for sublcasses to reject nil and empty strings out of an array.  It allows
+      # using the array form of shell_out (which avoids the need to surround arguments with
+      # quote marks to deal with shells).
+      #
+      # Usage:
+      #   shell_out!(*clean_array("useradd", universal_options, useradd_options, new_resource.username))
+      #
+      # universal_options and useradd_options can be nil, empty array, empty string, strings or arrays
+      # and the result makes sense.
+      #
+      # keeping this separate from shell_out!() makes it a bit easier to write expectations against the
+      # shell_out args and be able to omit nils and such in the tests (and to test that the nils are
+      # being rejected correctly).
+      #
+      # @param args [String] variable number of string arguments
+      # @return [Array] array of strings with nil and null string rejection
+      def clean_array(*args)
+        args.flatten.reject { |i| i.nil? || i == "" }.map(&:to_s)
+      end
+
       private
 
       def shell_out_command(*command_args)
diff --git a/lib/chef/provider/package.rb b/lib/chef/provider/package.rb
index 82b85fb1e4..3f641145e6 100644
--- a/lib/chef/provider/package.rb
+++ b/lib/chef/provider/package.rb
@@ -588,16 +588,6 @@ class Chef
         end
         args
       end
-
-      # Helper for sublcasses to convert an array of string args into a string.  It
-      # will compact nil or empty strings in the array and will join the array elements
-      # with spaces, without introducing any double spaces for nil/empty elements.
-      #
-      # @param args [String] variable number of string arguments
-      # @return [String] nicely concatenated string or empty string
-      def a_to_s(*args)
-        args.flatten.reject { |i| i.nil? || i == "" }.join(" ")
-      end
     end
   end
 end
diff --git a/lib/chef/provider/user.rb b/lib/chef/provider/user.rb
index 85bd674d8d..2bc4cc10bc 100644
--- a/lib/chef/provider/user.rb
+++ b/lib/chef/provider/user.rb
@@ -147,10 +147,6 @@ class Chef
         end
       end
 
-      def remove_user
-        raise NotImplementedError
-      end
-
       def action_manage
         if @user_exists && compare_user
           converge_by("manage user #{@new_resource.username}") do
@@ -160,10 +156,6 @@ class Chef
         end
       end
 
-      def manage_user
-        raise NotImplementedError
-      end
-
       def action_modify
         if compare_user
           converge_by("modify user #{@new_resource.username}") do
@@ -184,14 +176,6 @@ class Chef
         end
       end
 
-      def check_lock
-        raise NotImplementedError
-      end
-
-      def lock_user
-        raise NotImplementedError
-      end
-
       def action_unlock
         if check_lock() == true
           converge_by("unlock user #{@new_resource.username}") do
@@ -203,9 +187,29 @@ class Chef
         end
       end
 
+      def create_user
+        raise NotImplementedError
+      end
+
+      def remove_user
+        raise NotImplementedError
+      end
+
+      def manage_user
+        raise NotImplementedError
+      end
+
+      def lock_user
+        raise NotImplementedError
+      end
+
       def unlock_user
         raise NotImplementedError
       end
+
+      def check_lock
+        raise NotImplementedError
+      end
     end
   end
 end
diff --git a/lib/chef/provider/user/linux.rb b/lib/chef/provider/user/linux.rb
index ca331311f1..792e05e729 100644
--- a/lib/chef/provider/user/linux.rb
+++ b/lib/chef/provider/user/linux.rb
@@ -14,18 +14,122 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-require "chef/provider/user/useradd"
+require "chef/provider/user"
 
 class Chef
   class Provider
     class User
-      class Linux < Chef::Provider::User::Useradd
-        # MAJOR XXX: the implementation of "linux" is the base class and all needs to be moved here
+      class Linux < Chef::Provider::User
         provides :linux_user
         provides :user, os: "linux"
 
-        def managing_home_dir?
-          new_resource.manage_home # linux always 'supports' manage_home
+        def create_user
+          shell_out!(*clean_array("useradd", universal_options, useradd_options, new_resource.username))
+        end
+
+        def manage_user
+          shell_out!(*clean_array("usermod", universal_options, new_resource.username))
+        end
+
+        def remove_user
+          shell_out!(*clean_array("userdel", userdel_options, new_resource.username))
+        end
+
+        def lock_user
+          shell_out!(*clean_array("usermod", "-L", new_resource.username))
+        end
+
+        def unlock_user
+          shell_out!(*clean_array("usermod", "-U", new_resource.username))
+        end
+
+        # common to usermod and useradd
+        def universal_options
+          opts = []
+          opts << "-c" << new_resource.comment if should_set?(:comment)
+          opts << "-g" << new_resource.gid if should_set?(:gid)
+          opts << "-p" << new_resource.password if should_set?(:password)
+          opts << "-s" << new_resource.shell if should_set?(:shell)
+          opts << "-u" << new_resource.uid if should_set?(:uid)
+          if updating_home?
+            opts << "-d" << new_resource.home
+            if new_resource.manage_home
+              opts << "-m"
+            else
+              opts << "-M"
+            end
+          end
+          opts << "-o" if new_resource.non_unique
+          opts
+        end
+
+        def useradd_options
+          opts = []
+          opts << "-r" if new_resource.system
+          if new_resource.manage_home
+            opts << "-m"
+          else
+            opts << "-M"
+          end
+          opts
+        end
+
+        def userdel_options
+          opts = []
+          opts << "-r" if new_resource.manage_home
+          opts << "-f" if new_resource.force
+          opts
+        end
+
+        def should_set?(sym)
+          current_resource.send(sym).to_s != new_resource.send(sym).to_s && new_resource.send(sym)
+        end
+
+        def updating_home?
+          return false unless new_resource.home
+          return true unless current_resource.home
+          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
+          end
+
+          raise Chef::Exceptions::User, "Cannot determine if #{@new_resource} is locked!" if passwd_s.stdout.empty?
+
+          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
+
+              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
+
+          @locked
         end
       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 de851c4ad4..007441dc6c 100644
--- a/spec/support/shared/unit/provider/useradd_based_user_provider.rb
+++ b/spec/support/shared/unit/provider/useradd_based_user_provider.rb
@@ -100,7 +100,7 @@ shared_examples_for "a useradd-based user provider" do |supported_useradd_option
 
       it "should set useradd -r" do
         @new_resource.system(true)
-        expect(provider.useradd_options).to eq([ "-r" ])
+        expect(provider.useradd_options).to eq([ "-r", "-m" ])
       end
     end
 
@@ -112,7 +112,7 @@ shared_examples_for "a useradd-based user provider" do |supported_useradd_option
 
       it "should set -m -d /homedir" do
         expect(provider.universal_options).to eq(%w{-d /wowaweea -m})
-        expect(provider.useradd_options).to eq([])
+        expect(provider.useradd_options).to eq([ "-m" ])  # FIXME: repeated -m
       end
     end
 
@@ -125,7 +125,7 @@ shared_examples_for "a useradd-based user provider" do |supported_useradd_option
 
       it "should set -m -d /homedir" do
         expect(provider.universal_options).to eql(%w{-d /wowaweea -m})
-        expect(provider.useradd_options).to eq([])
+        expect(provider.useradd_options).to eq([ "-m" ])  # FIXME: repeated -m
       end
     end
 
@@ -158,7 +158,7 @@ shared_examples_for "a useradd-based user provider" do |supported_useradd_option
       command.concat([ "-s", "/usr/bin/zsh",
                        "-u", "1000",
                        "-d", "/Users/mud",
-                       "-m",
+                       "-m", "-m", # FIXME: repeated -m
                        "adam" ])
       expect(provider).to receive(:shell_out!).with(*command).and_return(true)
       provider.create_user
@@ -179,7 +179,7 @@ shared_examples_for "a useradd-based user provider" do |supported_useradd_option
         command.concat(["-p", "abracadabra"]) if supported_useradd_options.key?("password")
         command.concat([ "-s", "/usr/bin/zsh",
                          "-u", "1000",
-                         "-r",
+                         "-r", "-m",
                          "adam" ])
         expect(provider).to receive(:shell_out!).with(*command).and_return(true)
         provider.create_user
diff --git a/spec/unit/provider/user/linux_spec.rb b/spec/unit/provider/user/linux_spec.rb
index ef12cec234..eb76b2672e 100644
--- a/spec/unit/provider/user/linux_spec.rb
+++ b/spec/unit/provider/user/linux_spec.rb
@@ -39,16 +39,6 @@ describe Chef::Provider::User::Linux do
 
   include_examples "a useradd-based user provider", supported_useradd_options
 
-  describe "manage_user" do
-    # CHEF-5247: Chef::Provider::User::Solaris subclasses Chef::Provider::User::Useradd, but does not use usermod to change passwords.
-    # Thus, a call to Solaris#manage_user calls Solaris#manage_password and Useradd#manage_user, but the latter should be a no-op.
-    it "should not run the command if universal_options is an empty array" do
-      allow(provider).to receive(:universal_options).and_return([])
-      expect(provider).not_to receive(:shell_out!)
-      provider.manage_user
-    end
-  end
-
   describe "manage_home behavior" do
     before(:each) do
       @new_resource = Chef::Resource::User::LinuxUser.new("adam", @run_context)
@@ -70,7 +60,7 @@ describe Chef::Provider::User::Linux do
     end
 
     it "by default manage_home is true and we do not -M" do
-      expect( provider.useradd_options ).to eql([])
+      expect( provider.useradd_options ).to eql(["-m"])
     end
 
     it "setting manage_home to false includes -M" do