From 7de787be3e792a58285f089c31e2b58b995fde08 Mon Sep 17 00:00:00 2001 From: Lamont Granquist Date: Wed, 21 Dec 2016 15:24:40 -0800 Subject: user provider cleanup * removes more run_command * converts to shell_out_compact * some other misc style cleanup Signed-off-by: Lamont Granquist --- lib/chef/provider/user.rb | 37 +++++------ lib/chef/provider/user/aix.rb | 47 +++++++------- lib/chef/provider/user/dscl.rb | 109 ++++++++++++++++--------------- lib/chef/provider/user/linux.rb | 29 ++++----- lib/chef/provider/user/pw.rb | 56 ++++++++-------- lib/chef/provider/user/solaris.rb | 23 ++++--- lib/chef/provider/user/useradd.rb | 42 ++++++------ lib/chef/provider/user/windows.rb | 41 ++++++------ spec/unit/provider/user/dscl_spec.rb | 111 ++++++++++++++++---------------- spec/unit/provider/user/linux_spec.rb | 10 +-- spec/unit/provider/user/pw_spec.rb | 48 +++++++------- spec/unit/provider/user/windows_spec.rb | 8 +-- spec/unit/provider/user_spec.rb | 40 ++++++------ 13 files changed, 293 insertions(+), 308 deletions(-) diff --git a/lib/chef/provider/user.rb b/lib/chef/provider/user.rb index 4b05ac8f5e..de94871a87 100644 --- a/lib/chef/provider/user.rb +++ b/lib/chef/provider/user.rb @@ -39,7 +39,7 @@ class Chef if @new_resource.gid.is_a? String @new_resource.gid(Etc.getgrnam(@new_resource.gid).gid) end - rescue ArgumentError => e + rescue ArgumentError @group_name_resolved = false end @@ -53,7 +53,7 @@ class Chef begin user_info = Etc.getpwnam(@new_resource.username) - rescue ArgumentError => e + rescue ArgumentError @user_exists = false Chef::Log.debug("#{@new_resource} user does not exist") user_info = nil @@ -97,7 +97,7 @@ class Chef requirements.assert(:all_actions) do |a| a.assertion { @shadow_lib_ok } a.failure_message Chef::Exceptions::MissingLibrary, "You must have ruby-shadow installed for password support!" - a.whyrun "ruby-shadow is not installed. Attempts to set user password will cause failure. Assuming that this gem will have been previously installed." + + a.whyrun "ruby-shadow is not installed. Attempts to set user password will cause failure. Assuming that this gem will have been previously installed." \ "Note that user update converge may report false-positive on the basis of mismatched password. " end requirements.assert(:modify, :lock, :unlock) do |a| @@ -137,34 +137,31 @@ class Chef end def action_remove - if @user_exists - converge_by("remove user #{@new_resource.username}") do - remove_user - Chef::Log.info("#{@new_resource} removed") - end + return unless @user_exists + converge_by("remove user #{@new_resource.username}") do + remove_user + Chef::Log.info("#{@new_resource} removed") end end def action_manage - if @user_exists && compare_user - converge_by("manage user #{@new_resource.username}") do - manage_user - Chef::Log.info("#{@new_resource} managed") - end + return unless @user_exists && compare_user + converge_by("manage user #{@new_resource.username}") do + manage_user + Chef::Log.info("#{@new_resource} managed") end end def action_modify - if compare_user - converge_by("modify user #{@new_resource.username}") do - manage_user - Chef::Log.info("#{@new_resource} modified") - end + return unless compare_user + converge_by("modify user #{@new_resource.username}") do + manage_user + Chef::Log.info("#{@new_resource} modified") end end def action_lock - if check_lock() == false + if check_lock == false converge_by("lock the user #{@new_resource.username}") do lock_user Chef::Log.info("#{@new_resource} locked") @@ -175,7 +172,7 @@ class Chef end def action_unlock - if check_lock() == true + if check_lock == true converge_by("unlock user #{@new_resource.username}") do unlock_user Chef::Log.info("#{@new_resource} unlocked") diff --git a/lib/chef/provider/user/aix.rb b/lib/chef/provider/user/aix.rb index 8ac229ae4d..0e81c76bbc 100644 --- a/lib/chef/provider/user/aix.rb +++ b/lib/chef/provider/user/aix.rb @@ -23,7 +23,7 @@ class Chef provides :user, os: "aix" provides :aix_user - UNIVERSAL_OPTIONS = [[:comment, "-c"], [:gid, "-g"], [:shell, "-s"], [:uid, "-u"]] + UNIVERSAL_OPTIONS = [[:comment, "-c"], [:gid, "-g"], [:shell, "-s"], [:uid, "-u"]].freeze def create_user super @@ -44,53 +44,52 @@ class Chef end def check_lock - lock_info = shell_out!("lsuser -a account_locked #{new_resource.username}") + lock_info = shell_out_compact!("lsuser", "-a", "account_locked", new_resource.username) if whyrun_mode? && passwd_s.stdout.empty? && lock_info.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 lock_info.stdout.empty? + raise Chef::Exceptions::User, "Cannot determine if #{new_resource} is locked!" if lock_info.stdout.empty? status = /\S+\s+account_locked=(\S+)/.match(lock_info.stdout) - if status && status[1] == "true" - @locked = true - else - @locked = false - end + @locked = + if status && status[1] == "true" + true + else + false + end @locked end def lock_user - shell_out!("chuser account_locked=true #{new_resource.username}") + shell_out_compact!("chuser", "account_locked=true", new_resource.username) end def unlock_user - shell_out!("chuser account_locked=false #{new_resource.username}") + shell_out_compact!("chuser", "account_locked=false", new_resource.username) end private def add_password - if @current_resource.password != @new_resource.password && @new_resource.password - Chef::Log.debug("#{@new_resource.username} setting password to #{@new_resource.password}") - command = "echo '#{@new_resource.username}:#{@new_resource.password}' | chpasswd -e" - shell_out!(command) - end + return unless current_resource.password != new_resource.password && new_resource.password + Chef::Log.debug("#{new_resource.username} setting password to #{new_resource.password}") + command = "echo '#{new_resource.username}:#{new_resource.password}' | chpasswd -e" + shell_out!(command) end # Aix specific handling to update users home directory. def manage_home + return unless updating_home? && managing_home_dir? # -m option does not work on aix, so move dir. - if updating_home? && managing_home_dir? - universal_options.delete("-m") - if ::File.directory?(@current_resource.home) - Chef::Log.debug("Changing users home directory from #{@current_resource.home} to #{new_resource.home}") - shell_out!("mv #{@current_resource.home} #{new_resource.home}") - else - Chef::Log.debug("Creating users home directory #{new_resource.home}") - shell_out!("mkdir -p #{new_resource.home}") - end + universal_options.delete("-m") + if ::File.directory?(current_resource.home) + Chef::Log.debug("Changing users home directory from #{current_resource.home} to #{new_resource.home}") + FileUtils.mv current_resource.home, new_resource.home + else + Chef::Log.debug("Creating users home directory #{new_resource.home}") + FileUtils.mkdir_p new_resource.home end end diff --git a/lib/chef/provider/user/dscl.rb b/lib/chef/provider/user/dscl.rb index 16d60ba116..2302a874e2 100644 --- a/lib/chef/provider/user/dscl.rb +++ b/lib/chef/provider/user/dscl.rb @@ -65,12 +65,12 @@ class Chef end requirements.assert(:all_actions) do |a| - a.assertion { ::File.exists?("/usr/bin/dscl") } + a.assertion { ::File.exist?("/usr/bin/dscl") } a.failure_message(Chef::Exceptions::User, "Cannot find binary '/usr/bin/dscl' on the system for #{new_resource}!") end requirements.assert(:all_actions) do |a| - a.assertion { ::File.exists?("/usr/bin/plutil") } + a.assertion { ::File.exist?("/usr/bin/plutil") } a.failure_message(Chef::Exceptions::User, "Cannot find binary '/usr/bin/plutil' on the system for #{new_resource}!") end @@ -199,7 +199,7 @@ user password using shadow hash.") # Create a user using dscl # def dscl_create_user - run_dscl("create /Users/#{new_resource.username}") + run_dscl("create", "/Users/#{new_resource.username}") end # @@ -208,7 +208,7 @@ user password using shadow hash.") # def dscl_create_comment comment = new_resource.comment || new_resource.username - run_dscl("create /Users/#{new_resource.username} RealName '#{comment}'") + run_dscl("create", "/Users/#{new_resource.username}", "RealName", comment) end # @@ -224,7 +224,7 @@ user password using shadow hash.") raise(Chef::Exceptions::RequestedUIDUnavailable, "uid #{new_resource.uid} is already in use") end - run_dscl("create /Users/#{new_resource.username} UniqueID #{new_resource.uid}") + run_dscl("create", "/Users/#{new_resource.username}", "UniqueID", new_resource.uid) end # @@ -235,7 +235,7 @@ user password using shadow hash.") uid = nil base_uid = new_resource.system ? 200 : 500 next_uid_guess = base_uid - users_uids = run_dscl("list /Users uid") + users_uids = run_dscl("list", "/Users", "uid") while next_uid_guess < search_limit + base_uid if users_uids =~ Regexp.new("#{Regexp.escape(next_uid_guess.to_s)}\n") next_uid_guess += 1 @@ -244,7 +244,7 @@ user password using shadow hash.") break end end - return uid || raise("uid not found. Exhausted. Searched #{search_limit} times") + uid || raise("uid not found. Exhausted. Searched #{search_limit} times") end # @@ -252,18 +252,18 @@ user password using shadow hash.") # def uid_used?(uid) return false unless uid - users_uids = run_dscl("list /Users uid").split("\n") - uid_map = users_uids.inject({}) do |tmap, tuid| + users_uids = run_dscl("list", "/Users", "uid").split("\n") + uid_map = users_uids.each_with_object({}) do |tuid, tmap| x = tuid.split tmap[x[1]] = x[0] tmap end if uid_map[uid.to_s] - unless uid_map[uid.to_s] == new_resource.username.to_s + unless uid_map[uid.to_s] == new_resource.username return true end end - return false + false end # @@ -277,14 +277,14 @@ user password using shadow hash.") new_resource.gid(STAFF_GROUP_ID) elsif !new_resource.gid.to_s.match(/^\d+$/) begin - possible_gid = run_dscl("read /Groups/#{new_resource.gid} PrimaryGroupID").split(" ").last - rescue Chef::Exceptions::DsclCommandFailed => e - raise Chef::Exceptions::GroupIDNotFound.new("Group not found for #{new_resource.gid} when creating user #{new_resource.username}") + possible_gid = run_dscl("read", "/Groups/#{new_resource.gid}", "PrimaryGroupID").split(" ").last + rescue Chef::Exceptions::DsclCommandFailed + raise Chef::Exceptions::GroupIDNotFound, "Group not found for #{new_resource.gid} when creating user #{new_resource.username}" end # XXX: mutates the new resource new_resource.gid(possible_gid) if possible_gid && possible_gid.match(/^\d+$/) end - run_dscl("create /Users/#{new_resource.username} PrimaryGroupID '#{new_resource.gid}'") + run_dscl("create", "/Users/#{new_resource.username}", "PrimaryGroupID", new_resource.gid) end # @@ -293,7 +293,7 @@ user password using shadow hash.") # def dscl_set_home if new_resource.home.nil? || new_resource.home.empty? - run_dscl("delete /Users/#{new_resource.username} NFSHomeDirectory") + run_dscl("delete", "/Users/#{new_resource.username}", "NFSHomeDirectory") return end @@ -308,7 +308,7 @@ user password using shadow hash.") move_home end end - run_dscl("create /Users/#{new_resource.username} NFSHomeDirectory '#{new_resource.home}'") + run_dscl("create", "/Users/#{new_resource.username}", "NFSHomeDirectory", new_resource.home) end def validate_home_dir_specification! @@ -318,17 +318,17 @@ user password using shadow hash.") end def current_home_exists? - ::File.exist?("#{current_resource.home}") + ::File.exist?(current_resource.home) end def new_home_exists? - ::File.exist?("#{new_resource.home}") + ::File.exist?(new_resource.home) end def ditto_home skel = "/System/Library/User Template/English.lproj" - raise(Chef::Exceptions::User, "can't find skel at: #{skel}") unless ::File.exists?(skel) - shell_out! "ditto '#{skel}' '#{new_resource.home}'" + raise(Chef::Exceptions::User, "can't find skel at: #{skel}") unless ::File.exist?(skel) + shell_out_compact!("ditto", skel, new_resource.home) ::FileUtils.chown_R(new_resource.username, new_resource.gid.to_s, new_resource.home) end @@ -338,7 +338,7 @@ user password using shadow hash.") src = current_resource.home FileUtils.mkdir_p(new_resource.home) files = ::Dir.glob("#{Chef::Util::PathHelper.escape_glob_dir(src)}/*", ::File::FNM_DOTMATCH) - ["#{src}/.", "#{src}/.."] - ::FileUtils.mv(files, new_resource.home, :force => true) + ::FileUtils.mv(files, new_resource.home, force: true) ::FileUtils.rmdir(src) ::FileUtils.chown_R(new_resource.username, new_resource.gid.to_s, new_resource.home) end @@ -347,10 +347,10 @@ user password using shadow hash.") # Sets the shell for the user using dscl. # def dscl_set_shell - if new_resource.shell || ::File.exists?("#{new_resource.shell}") - run_dscl("create /Users/#{new_resource.username} UserShell '#{new_resource.shell}'") + if new_resource.shell + run_dscl("create", "/Users/#{new_resource.username}", "UserShell", new_resource.shell) else - run_dscl("create /Users/#{new_resource.username} UserShell '/usr/bin/false'") + run_dscl("create", "/Users/#{new_resource.username}", "UserShell", "/usr/bin/false") end end @@ -367,9 +367,8 @@ user password using shadow hash.") # Shadow info is saved as binary plist. Convert the info to binary plist. shadow_info_binary = StringIO.new - command = Mixlib::ShellOut.new("plutil -convert binary1 -o - -", - :input => shadow_info.to_plist, :live_stream => shadow_info_binary) - command.run_command + shell_out_compact("plutil", "-convert", "binary1", "-o", "-", "-", + input: shadow_info.to_plist, live_stream: shadow_info_binary) if user_info.nil? # User is just created. read_user_info() will read the fresh information @@ -401,7 +400,7 @@ user password using shadow hash.") # Create a random 4 byte salt salt = OpenSSL::Random.random_bytes(4) encoded_password = OpenSSL::Digest::SHA512.hexdigest(salt + new_resource.password) - hash_value = salt.unpack("H*").first + encoded_password + salt.unpack("H*").first + encoded_password end shadow_info["SALTED-SHA512"] = StringIO.new @@ -449,21 +448,21 @@ user password using shadow hash.") end # Remove the user from its groups - run_dscl("list /Groups").each_line do |group| + run_dscl("list", "/Groups").each_line do |group| if member_of_group?(group.chomp) - run_dscl("delete /Groups/#{group.chomp} GroupMembership '#{new_resource.username}'") + run_dscl("delete", "/Groups/#{group.chomp}", "GroupMembership", new_resource.username) end end # Remove user account - run_dscl("delete /Users/#{new_resource.username}") + run_dscl("delete", "/Users/#{new_resource.username}") end # # Locks the user. # def lock_user - run_dscl("append /Users/#{new_resource.username} AuthenticationAuthority ';DisabledUser;'") + run_dscl("append", "/Users/#{new_resource.username}", "AuthenticationAuthority", ";DisabledUser;") end # @@ -471,7 +470,7 @@ user password using shadow hash.") # def unlock_user auth_string = authentication_authority.gsub(/AuthenticationAuthority: /, "").gsub(/;DisabledUser;/, "").strip - run_dscl("create /Users/#{new_resource.username} AuthenticationAuthority '#{auth_string}'") + run_dscl("create", "/Users/#{new_resource.username}", "AuthenticationAuthority", auth_string) end # @@ -489,7 +488,7 @@ user password using shadow hash.") # This is the interface base User provider requires to provide idempotency. # def check_lock - return @locked = locked? + @locked = locked? end # @@ -501,11 +500,11 @@ user password using shadow hash.") # given attribute. # def diverged?(parameter) - parameter_updated?(parameter) && (not new_resource.send(parameter).nil?) + parameter_updated?(parameter) && !new_resource.send(parameter).nil? end def parameter_updated?(parameter) - not (new_resource.send(parameter) == current_resource.send(parameter)) + !(new_resource.send(parameter) == current_resource.send(parameter)) end # @@ -551,7 +550,7 @@ user password using shadow hash.") def member_of_group?(group_name) membership_info = "" begin - membership_info = run_dscl("read /Groups/#{group_name}") + membership_info = run_dscl("read", "/Groups/#{group_name}") rescue Chef::Exceptions::DsclCommandFailed # Raised if the group doesn't contain any members end @@ -568,14 +567,14 @@ user password using shadow hash.") # A simple map of Chef's terms to DSCL's terms. DSCL_PROPERTY_MAP = { - :uid => "uid", - :gid => "gid", - :home => "home", - :shell => "shell", - :comment => "realname", - :password => "passwd", - :auth_authority => "authentication_authority", - :shadow_hash => "ShadowHashData", + uid: "uid", + gid: "gid", + home: "home", + shell: "shell", + comment: "realname", + password: "passwd", + auth_authority: "authentication_authority", + shadow_hash: "ShadowHashData", }.freeze # Directory where the user plist files are stored for versions 10.7 and above @@ -590,11 +589,11 @@ user password using shadow hash.") # We flush the cache here in order to make sure that we read fresh information # for the user. - shell_out("dscacheutil '-flushcache'") + shell_out_compact("dscacheutil", "-flushcache") # FIXME: this is MacOS version dependent begin user_plist_file = "#{USER_PLIST_DIRECTORY}/#{new_resource.username}.plist" - user_plist_info = run_plutil("convert xml1 -o - #{user_plist_file}") + user_plist_info = run_plutil("convert", "xml1", "-o", "-", user_plist_file) user_info = Plist.parse_xml(user_plist_info) rescue Chef::Exceptions::PlistUtilCommandFailed end @@ -609,7 +608,7 @@ user password using shadow hash.") def save_user_info(user_info) user_plist_file = "#{USER_PLIST_DIRECTORY}/#{new_resource.username}.plist" Plist::Emit.save_plist(user_info, user_plist_file) - run_plutil("convert binary1 #{user_plist_file}") + run_plutil("convert", "binary1", user_plist_file) end # @@ -658,7 +657,9 @@ user password using shadow hash.") end def run_dscl(*args) - result = shell_out("dscl . -#{args.join(' ')}") + argdup = args.dup + cmd = argdup.shift + result = shell_out_compact("dscl", ".", "-#{cmd}", argdup) return "" if ( args.first =~ /^delete/ ) && ( result.exitstatus != 0 ) raise(Chef::Exceptions::DsclCommandFailed, "dscl error: #{result.inspect}") unless result.exitstatus == 0 raise(Chef::Exceptions::DsclCommandFailed, "dscl error: #{result.inspect}") if result.stdout =~ /No such key: / @@ -666,17 +667,19 @@ user password using shadow hash.") end def run_plutil(*args) - result = shell_out("plutil -#{args.join(' ')}") + argdup = args.dup + cmd = argdup.shift + result = shell_out_compact("plutil", "-#{cmd}", argdup) raise(Chef::Exceptions::PlistUtilCommandFailed, "plutil error: #{result.inspect}") unless result.exitstatus == 0 if result.stdout.encoding == Encoding::ASCII_8BIT - result.stdout.encode("utf-8", "binary", :undef => :replace, :invalid => :replace, :replace => "?") + result.stdout.encode("utf-8", "binary", undef: :replace, invalid: :replace, replace: "?") else result.stdout end end def convert_binary_plist_to_xml(binary_plist_string) - Mixlib::ShellOut.new("plutil -convert xml1 -o - -", :input => binary_plist_string).run_command.stdout + shell_out_compact("plutil", "-convert", "xml1", "-o", "-", "-", input: binary_plist_string).stdout end def convert_to_binary(string) diff --git a/lib/chef/provider/user/linux.rb b/lib/chef/provider/user/linux.rb index 968cf771e4..445421ad38 100644 --- a/lib/chef/provider/user/linux.rb +++ b/lib/chef/provider/user/linux.rb @@ -24,23 +24,23 @@ class Chef provides :user, os: "linux" def create_user - shell_out!(*clean_array("useradd", universal_options, useradd_options, new_resource.username)) + shell_out_compact!("useradd", universal_options, useradd_options, new_resource.username) end def manage_user - shell_out!(*clean_array("usermod", universal_options, usermod_options, new_resource.username)) + shell_out_compact!("usermod", universal_options, usermod_options, new_resource.username) end def remove_user - shell_out!(*clean_array("userdel", userdel_options, new_resource.username)) + shell_out_compact!("userdel", userdel_options, new_resource.username) end def lock_user - shell_out!(*clean_array("usermod", "-L", new_resource.username)) + shell_out_compact!("usermod", "-L", new_resource.username) end def unlock_user - shell_out!(*clean_array("usermod", "-U", new_resource.username)) + shell_out_compact!("usermod", "-U", new_resource.username) end # common to usermod and useradd @@ -69,11 +69,11 @@ class Chef def useradd_options opts = [] opts << "-r" if new_resource.system - if managing_home_dir? - opts << "-m" - else - opts << "-M" - end + opts << if managing_home_dir? + "-m" + else + "-M" + end opts end @@ -97,15 +97,12 @@ class Chef def check_lock # 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 ]) + passwd_s = shell_out_compact("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 + return false if whyrun_mode? + raise Chef::Exceptions::User, "User #{new_resource.username} does not exist when checking lock status for #{new_resource}" end # now raise if we didn't get a 0 or 1 (see above) diff --git a/lib/chef/provider/user/pw.rb b/lib/chef/provider/user/pw.rb index b210374eb9..42d862a983 100644 --- a/lib/chef/provider/user/pw.rb +++ b/lib/chef/provider/user/pw.rb @@ -27,49 +27,45 @@ class Chef def load_current_resource super - raise Chef::Exceptions::User, "Could not find binary /usr/sbin/pw for #{@new_resource}" unless ::File.exists?("/usr/sbin/pw") + raise Chef::Exceptions::User, "Could not find binary /usr/sbin/pw for #{new_resource}" unless ::File.exist?("/usr/sbin/pw") end def create_user - command = "pw useradd" - command << set_options - run_command(:command => command) + shell_out_compact!("pw", "useradd", set_options) modify_password end def manage_user - command = "pw usermod" - command << set_options - run_command(:command => command) + shell_out_compact!("pw", "usermod", set_options) modify_password end def remove_user - command = "pw userdel #{@new_resource.username}" - command << " -r" if managing_home_dir? - run_command(:command => command) + command = [ "pw", "userdel", new_resource.username ] + command << "-r" if managing_home_dir? + shell_out_compact!(command) end def check_lock - case @current_resource.password - when /^\*LOCKED\*/ - @locked = true - else - @locked = false - end + @locked = case current_resource.password + when /^\*LOCKED\*/ + true + else + false + end @locked end def lock_user - run_command(:command => "pw lock #{@new_resource.username}") + shell_out_compact!("pw", "lock", new_resource.username) end def unlock_user - run_command(:command => "pw unlock #{@new_resource.username}") + shell_out_compact!("pw", "unlock", new_resource.username) end def set_options - opts = " #{@new_resource.username}" + opts = [ new_resource.username ] field_list = { "comment" => "-c", @@ -80,26 +76,26 @@ class Chef } field_list.sort { |a, b| a[0] <=> b[0] }.each do |field, option| field_symbol = field.to_sym - if @current_resource.send(field_symbol) != @new_resource.send(field_symbol) - if @new_resource.send(field_symbol) - Chef::Log.debug("#{@new_resource} setting #{field} to #{@new_resource.send(field_symbol)}") - opts << " #{option} '#{@new_resource.send(field_symbol)}'" - end + next unless current_resource.send(field_symbol) != new_resource.send(field_symbol) + if new_resource.send(field_symbol) + Chef::Log.debug("#{new_resource} setting #{field} to #{new_resource.send(field_symbol)}") + opts << option + opts << new_resource.send(field_symbol) end end if managing_home_dir? - Chef::Log.debug("#{@new_resource} is managing the users home directory") - opts << " -m" + Chef::Log.debug("#{new_resource} is managing the users home directory") + opts << "-m" end opts end def modify_password - if (not @new_resource.password.nil?) && (@current_resource.password != @new_resource.password) + if !new_resource.password.nil? && (current_resource.password != new_resource.password) Chef::Log.debug("#{new_resource} updating password") - command = "pw usermod #{@new_resource.username} -H 0" - status = popen4(command, :waitlast => true) do |pid, stdin, stdout, stderr| - stdin.puts "#{@new_resource.password}" + command = "pw usermod #{new_resource.username} -H 0" + status = popen4(command, waitlast: true) do |pid, stdin, stdout, stderr| + stdin.puts new_resource.password.to_s end unless status.exitstatus == 0 diff --git a/lib/chef/provider/user/solaris.rb b/lib/chef/provider/user/solaris.rb index 7aa0ceb93a..4e772312ae 100644 --- a/lib/chef/provider/user/solaris.rb +++ b/lib/chef/provider/user/solaris.rb @@ -26,7 +26,7 @@ class Chef class Solaris < Chef::Provider::User::Useradd provides :solaris_user provides :user, os: %w{omnios solaris2} - UNIVERSAL_OPTIONS = [[:comment, "-c"], [:gid, "-g"], [:shell, "-s"], [:uid, "-u"]] + UNIVERSAL_OPTIONS = [[:comment, "-c"], [:gid, "-g"], [:shell, "-s"], [:uid, "-u"]].freeze attr_writer :password_file @@ -46,22 +46,22 @@ class Chef end def check_lock - user = IO.read(@password_file).match(/^#{Regexp.escape(@new_resource.username)}:([^:]*):/) + user = IO.read(@password_file).match(/^#{Regexp.escape(new_resource.username)}:([^:]*):/) # If we're in whyrun mode, and the user is not created, we assume it will be return false if whyrun_mode? && user.nil? - raise Chef::Exceptions::User, "Cannot determine if #{@new_resource} is locked!" if user.nil? + raise Chef::Exceptions::User, "Cannot determine if #{new_resource} is locked!" if user.nil? @locked = user[1].start_with?("*LK*") end def lock_user - shell_out!("passwd", "-l", new_resource.username) + shell_out_compact!("passwd", "-l", new_resource.username) end def unlock_user - shell_out!("passwd", "-u", new_resource.username) + shell_out_compact!("passwd", "-u", new_resource.username) end private @@ -82,10 +82,9 @@ class Chef end def manage_password - if @current_resource.password != @new_resource.password && @new_resource.password - Chef::Log.debug("#{@new_resource} setting password to #{@new_resource.password}") - write_shadow_file - end + return unless current_resource.password != new_resource.password && new_resource.password + Chef::Log.debug("#{new_resource} setting password to #{new_resource.password}") + write_shadow_file end def write_shadow_file @@ -93,7 +92,7 @@ class Chef ::File.open(@password_file) do |shadow_file| shadow_file.each do |entry| user = entry.split(":").first - if user == @new_resource.username + if user == new_resource.username buffer.write(updated_password(entry)) else buffer.write(entry) @@ -104,7 +103,7 @@ class Chef # FIXME: mostly duplicates code with file provider deploying a file s = ::File.stat(@password_file) - mode = s.mode & 07777 + mode = s.mode & 0o7777 uid = s.uid gid = s.gid @@ -116,7 +115,7 @@ class Chef def updated_password(entry) fields = entry.split(":") - fields[1] = @new_resource.password + fields[1] = new_resource.password fields[2] = days_since_epoch fields.join(":") end diff --git a/lib/chef/provider/user/useradd.rb b/lib/chef/provider/user/useradd.rb index 35a106b0b6..cf6f4e727f 100644 --- a/lib/chef/provider/user/useradd.rb +++ b/lib/chef/provider/user/useradd.rb @@ -23,25 +23,25 @@ class Chef class Provider class User class Useradd < Chef::Provider::User - # MAJOR XXX: this should become the base class of all Useradd providers instead of the linux implementation + # the linux version of this has been forked off, this is the base class now of solaris and AIX and should be abandoned + # and those provider should be rewritten like the linux version. - UNIVERSAL_OPTIONS = [[:comment, "-c"], [:gid, "-g"], [:password, "-p"], [:shell, "-s"], [:uid, "-u"]] + UNIVERSAL_OPTIONS = [[:comment, "-c"], [:gid, "-g"], [:password, "-p"], [:shell, "-s"], [:uid, "-u"]].freeze def create_user command = compile_command("useradd") do |useradd| useradd.concat(universal_options) useradd.concat(useradd_options) end - shell_out!(*command) + shell_out_compact!(command) end def manage_user - unless universal_options.empty? - command = compile_command("usermod") do |u| - u.concat(universal_options) - end - shell_out!(*command) + return if universal_options.empty? + command = compile_command("usermod") do |u| + u.concat(universal_options) end + shell_out_compact!(command) end def remove_user @@ -49,19 +49,19 @@ class Chef command << "-r" if managing_home_dir? command << "-f" if new_resource.force command << new_resource.username - shell_out!(*command) + shell_out_compact!(command) end 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]) + passwd_s = shell_out_compact!("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? + 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] @@ -76,7 +76,7 @@ class Chef 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_check = shell_out_compact!("rpm", "-q", "passwd") passwd_version = passwd_version_check.stdout.chomp unless passwd_version == "passwd-0.73-1" @@ -93,11 +93,11 @@ class Chef end def lock_user - shell_out!("usermod", "-L", new_resource.username) + shell_out_compact!("usermod", "-L", new_resource.username) end def unlock_user - shell_out!("usermod", "-U", new_resource.username) + shell_out_compact!("usermod", "-U", new_resource.username) end def compile_command(base_command) @@ -130,12 +130,10 @@ class Chef end def update_options(field, option, opts) - if @current_resource.send(field).to_s != new_resource.send(field).to_s - if new_resource.send(field) - Chef::Log.debug("#{new_resource} setting #{field} to #{new_resource.send(field)}") - opts << option << new_resource.send(field).to_s - end - end + return unless current_resource.send(field).to_s != new_resource.send(field).to_s + return unless new_resource.send(field) + Chef::Log.debug("#{new_resource} setting #{field} to #{new_resource.send(field)}") + opts << option << new_resource.send(field).to_s end def useradd_options @@ -150,8 +148,8 @@ class Chef # Pathname#cleanpath does a better job than ::File::expand_path (on both unix and windows) # ::File.expand_path("///tmp") == ::File.expand_path("/tmp") => false # ::File.expand_path("\\tmp") => "C:/tmp" - return true if @current_resource.home.nil? && new_resource.home - new_resource.home && Pathname.new(@current_resource.home).cleanpath != Pathname.new(new_resource.home).cleanpath + return true if current_resource.home.nil? && new_resource.home + new_resource.home && Pathname.new(current_resource.home).cleanpath != Pathname.new(new_resource.home).cleanpath end end diff --git a/lib/chef/provider/user/windows.rb b/lib/chef/provider/user/windows.rb index b086a1e32b..0afa8fa14a 100644 --- a/lib/chef/provider/user/windows.rb +++ b/lib/chef/provider/user/windows.rb @@ -31,31 +31,30 @@ class Chef def initialize(new_resource, run_context) super - @net_user = Chef::Util::Windows::NetUser.new(@new_resource.username) + @net_user = Chef::Util::Windows::NetUser.new(new_resource.username) end def load_current_resource - if @new_resource.gid + if new_resource.gid Chef::Log.warn("The 'gid' attribute is not implemented by the Windows platform. Please use the 'group' resource to assign a user to a group.") end - @current_resource = Chef::Resource::User.new(@new_resource.name) - @current_resource.username(@new_resource.username) - user_info = nil + @current_resource = Chef::Resource::User.new(new_resource.name) + current_resource.username(new_resource.username) begin user_info = @net_user.get_info - @current_resource.uid(user_info[:user_id]) - @current_resource.comment(user_info[:full_name]) - @current_resource.home(user_info[:home_dir]) - @current_resource.shell(user_info[:script_path]) + current_resource.uid(user_info[:user_id]) + current_resource.comment(user_info[:full_name]) + current_resource.home(user_info[:home_dir]) + current_resource.shell(user_info[:script_path]) rescue Chef::Exceptions::UserIDNotFound => e # e.message should be "The user name could not be found" but checking for that could cause a localization bug @user_exists = false - Chef::Log.debug("#{@new_resource} does not exist (#{e.message})") + Chef::Log.debug("#{new_resource} does not exist (#{e.message})") end - @current_resource + current_resource end # Check to see if the user needs any changes @@ -64,12 +63,12 @@ class Chef # :: If a change is required # :: If the users are identical def compare_user - unless @net_user.validate_credentials(@new_resource.password) - Chef::Log.debug("#{@new_resource} password has changed") + unless @net_user.validate_credentials(new_resource.password) + Chef::Log.debug("#{new_resource} password has changed") return true end [ :uid, :comment, :home, :shell ].any? do |user_attrib| - !@new_resource.send(user_attrib).nil? && @new_resource.send(user_attrib) != @current_resource.send(user_attrib) + !new_resource.send(user_attrib).nil? && new_resource.send(user_attrib) != current_resource.send(user_attrib) end end @@ -98,7 +97,7 @@ class Chef end def set_options - opts = { :name => @new_resource.username } + opts = { name: new_resource.username } field_list = { "comment" => "full_name", @@ -110,14 +109,12 @@ class Chef field_list.sort { |a, b| a[0] <=> b[0] }.each do |field, option| field_symbol = field.to_sym - if @current_resource.send(field_symbol) != @new_resource.send(field_symbol) - if @new_resource.send(field_symbol) - unless field_symbol == :password - Chef::Log.debug("#{@new_resource} setting #{field} to #{@new_resource.send(field_symbol)}") - end - opts[option.to_sym] = @new_resource.send(field_symbol) - end + next unless current_resource.send(field_symbol) != new_resource.send(field_symbol) + next unless new_resource.send(field_symbol) + unless field_symbol == :password + Chef::Log.debug("#{new_resource} setting #{field} to #{new_resource.send(field_symbol)}") end + opts[option.to_sym] = new_resource.send(field_symbol) end opts end diff --git a/spec/unit/provider/user/dscl_spec.rb b/spec/unit/provider/user/dscl_spec.rb index 7b8be02f3a..f59709e717 100644 --- a/spec/unit/provider/user/dscl_spec.rb +++ b/spec/unit/provider/user/dscl_spec.rb @@ -116,38 +116,38 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30" describe "when shelling out to dscl" do it "should run dscl with the supplied cmd /Path args" do shell_return = shellcmdresult.new("stdout", "err", 0) - expect(provider).to receive(:shell_out).with("dscl . -cmd /Path args").and_return(shell_return) - expect(provider.run_dscl("cmd /Path args")).to eq("stdout") + expect(provider).to receive(:shell_out).with("dscl", ".", "-cmd", "/Path", "args").and_return(shell_return) + expect(provider.run_dscl("cmd", "/Path", "args")).to eq("stdout") end it "returns an empty string from delete commands" do shell_return = shellcmdresult.new("out", "err", 23) - expect(provider).to receive(:shell_out).with("dscl . -delete /Path args").and_return(shell_return) - expect(provider.run_dscl("delete /Path args")).to eq("") + expect(provider).to receive(:shell_out).with("dscl", ".", "-delete", "/Path", "args").and_return(shell_return) + expect(provider.run_dscl("delete", "/Path", "args")).to eq("") end it "should raise an exception for any other command" do shell_return = shellcmdresult.new("out", "err", 23) - expect(provider).to receive(:shell_out).with("dscl . -cmd /Path arguments").and_return(shell_return) - expect { provider.run_dscl("cmd /Path arguments") }.to raise_error(Chef::Exceptions::DsclCommandFailed) + expect(provider).to receive(:shell_out).with("dscl", ".", "-cmd", "/Path", "arguments").and_return(shell_return) + expect { provider.run_dscl("cmd", "/Path", "arguments") }.to raise_error(Chef::Exceptions::DsclCommandFailed) end it "raises an exception when dscl reports 'no such key'" do shell_return = shellcmdresult.new("No such key: ", "err", 23) - expect(provider).to receive(:shell_out).with("dscl . -cmd /Path args").and_return(shell_return) - expect { provider.run_dscl("cmd /Path args") }.to raise_error(Chef::Exceptions::DsclCommandFailed) + expect(provider).to receive(:shell_out).with("dscl", ".", "-cmd", "/Path", "args").and_return(shell_return) + expect { provider.run_dscl("cmd", "/Path", "args") }.to raise_error(Chef::Exceptions::DsclCommandFailed) end it "raises an exception when dscl reports 'eDSRecordNotFound'" do shell_return = shellcmdresult.new(" DS Error: -14136 (eDSRecordNotFound)", "err", -14136) - expect(provider).to receive(:shell_out).with("dscl . -cmd /Path args").and_return(shell_return) - expect { provider.run_dscl("cmd /Path args") }.to raise_error(Chef::Exceptions::DsclCommandFailed) + expect(provider).to receive(:shell_out).with("dscl", ".", "-cmd", "/Path", "args").and_return(shell_return) + expect { provider.run_dscl("cmd", "/Path", "args") }.to raise_error(Chef::Exceptions::DsclCommandFailed) end end describe "get_free_uid" do before do - expect(provider).to receive(:run_dscl).with("list /Users uid").and_return("\nwheel 200\nstaff 201\nbrahms 500\nchopin 501\n") + expect(provider).to receive(:run_dscl).with("list", "/Users", "uid").and_return("\nwheel 200\nstaff 201\nbrahms 500\nchopin 501\n") end describe "when resource is configured as system" do @@ -177,7 +177,7 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30" describe "when called with a user id" do before do - expect(provider).to receive(:run_dscl).with("list /Users uid").and_return("\naj 500\n") + expect(provider).to receive(:run_dscl).with("list", "/Users", "uid").and_return("\naj 500\n") end it "should return true for a used uid number" do @@ -198,8 +198,8 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30" end it "finds a valid, unused uid when none is specified" do - expect(provider).to receive(:run_dscl).with("list /Users uid").and_return("") - expect(provider).to receive(:run_dscl).with("create /Users/toor UniqueID 501") + expect(provider).to receive(:run_dscl).with("list", "/Users", "uid").and_return("") + expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "UniqueID", 501) expect(provider).to receive(:get_free_uid).and_return(501) provider.dscl_set_uid expect(new_resource.uid).to eq(501) @@ -207,8 +207,8 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30" it "sets the uid specified in the resource" do new_resource.uid(1000) - expect(provider).to receive(:run_dscl).with("create /Users/toor UniqueID 1000").and_return(true) - expect(provider).to receive(:run_dscl).with("list /Users uid").and_return("") + expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "UniqueID", 1000).and_return(true) + expect(provider).to receive(:run_dscl).with("list", "/Users", "uid").and_return("") provider.dscl_set_uid end end @@ -219,9 +219,7 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30" end before do - Chef::Config[:treat_deprecation_warnings_as_errors] = false - Chef::Config[:treat_deprecation_warnings_as_errors] = false - new_resource.supports({ :manage_home => true }) + new_resource.manage_home true new_resource.home("/Users/toor") provider.current_resource = current_resource @@ -229,7 +227,7 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30" it "deletes the home directory when resource#home is nil" do new_resource.instance_variable_set(:@home, nil) - expect(provider).to receive(:run_dscl).with("delete /Users/toor NFSHomeDirectory").and_return(true) + expect(provider).to receive(:run_dscl).with("delete", "/Users/toor", "NFSHomeDirectory").and_return(true) provider.dscl_set_home end @@ -239,40 +237,40 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30" end it "moves the users home to the new location if it exists and the target location is different" do - Chef::Config[:treat_deprecation_warnings_as_errors] = false - new_resource.supports(:manage_home => true) + new_resource.manage_home true current_home = CHEF_SPEC_DATA + "/old_home_dir" current_home_files = [current_home + "/my-dot-emacs", current_home + "/my-dot-vim"] current_resource.home(current_home) new_resource.gid(23) - allow(::File).to receive(:exists?).with("/old/home/toor").and_return(true) - allow(::File).to receive(:exists?).with("/Users/toor").and_return(true) + allow(::File).to receive(:exist?).with("/old/home/toor").and_return(true) + allow(::File).to receive(:exist?).with("/Users/toor").and_return(true) + allow(::File).to receive(:exist?).with(current_home).and_return(true) expect(FileUtils).to receive(:mkdir_p).with("/Users/toor").and_return(true) expect(FileUtils).to receive(:rmdir).with(current_home) expect(::Dir).to receive(:glob).with("#{CHEF_SPEC_DATA}/old_home_dir/*", ::File::FNM_DOTMATCH).and_return(current_home_files) - expect(FileUtils).to receive(:mv).with(current_home_files, "/Users/toor", :force => true) + expect(FileUtils).to receive(:mv).with(current_home_files, "/Users/toor", force: true) expect(FileUtils).to receive(:chown_R).with("toor", "23", "/Users/toor") - expect(provider).to receive(:run_dscl).with("create /Users/toor NFSHomeDirectory '/Users/toor'") + expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "NFSHomeDirectory", "/Users/toor") provider.dscl_set_home end it "should raise an exception when the systems user template dir (skel) cannot be found" do - allow(::File).to receive(:exists?).and_return(false, false, false) + allow(::File).to receive(:exist?).and_return(false, false, false) expect { provider.dscl_set_home }.to raise_error(Chef::Exceptions::User) end it "should run ditto to copy any missing files from skel to the new home dir" do - expect(::File).to receive(:exists?).with("/System/Library/User\ Template/English.lproj").and_return(true) + expect(::File).to receive(:exist?).with("/System/Library/User\ Template/English.lproj").and_return(true) expect(FileUtils).to receive(:chown_R).with("toor", "", "/Users/toor") - expect(provider).to receive(:shell_out!).with("ditto '/System/Library/User Template/English.lproj' '/Users/toor'") + expect(provider).to receive(:shell_out!).with("ditto", "/System/Library/User Template/English.lproj", "/Users/toor") provider.ditto_home end it "creates the user's NFSHomeDirectory and home directory" do - expect(provider).to receive(:run_dscl).with("create /Users/toor NFSHomeDirectory '/Users/toor'").and_return(true) + expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "NFSHomeDirectory", "/Users/toor").and_return(true) expect(provider).to receive(:ditto_home) provider.dscl_set_home end @@ -283,8 +281,8 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30" let(:plutil_exists) { true } before do - allow(::File).to receive(:exists?).with("/usr/bin/dscl").and_return(dscl_exists) - allow(::File).to receive(:exists?).with("/usr/bin/plutil").and_return(plutil_exists) + allow(::File).to receive(:exist?).with("/usr/bin/dscl").and_return(dscl_exists) + allow(::File).to receive(:exist?).with("/usr/bin/plutil").and_return(plutil_exists) end def run_requirements @@ -382,8 +380,8 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30" let(:user_plist_file) { nil } before do - expect(provider).to receive(:shell_out).with("dscacheutil '-flushcache'") - expect(provider).to receive(:shell_out).with("plutil -convert xml1 -o - /var/db/dslocal/nodes/Default/users/toor.plist") do + expect(provider).to receive(:shell_out).with("dscacheutil", "-flushcache") + expect(provider).to receive(:shell_out).with("plutil", "-convert", "xml1", "-o", "-", "/var/db/dslocal/nodes/Default/users/toor.plist") do if user_plist_file.nil? shellcmdresult.new("Can not find the file", "Sorry!!", 1) else @@ -391,7 +389,7 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30" end end - if !user_plist_file.nil? + unless user_plist_file.nil? expect(provider).to receive(:convert_binary_plist_to_xml).and_return(File.read(File.join(CHEF_SPEC_DATA, "mac_users/#{user_plist_file}.shadow.xml"))) end end @@ -726,7 +724,7 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30") expect(provider).to receive(:prepare_password_shadow_info).and_return({}) mock_shellout = double("Mock::Shellout") allow(mock_shellout).to receive(:run_command) - expect(Mixlib::ShellOut).to receive(:new).and_return(mock_shellout) + expect(provider).to receive(:shell_out).and_return(mock_shellout) expect(provider).to receive(:read_user_info) expect(provider).to receive(:dscl_set) expect(provider).to receive(:sleep).with(3) @@ -754,29 +752,29 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30") end it "creates the user and sets the comment field" do - expect(provider).to receive(:run_dscl).with("create /Users/toor").and_return(true) + expect(provider).to receive(:run_dscl).with("create", "/Users/toor").and_return(true) provider.dscl_create_user end it "sets the comment field" do - expect(provider).to receive(:run_dscl).with("create /Users/toor RealName '#mockssuck'").and_return(true) + expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "RealName", "#mockssuck").and_return(true) provider.dscl_create_comment end it "sets the comment field to username" do new_resource.comment nil - expect(provider).to receive(:run_dscl).with("create /Users/toor RealName '#mockssuck'").and_return(true) + expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "RealName", "#mockssuck").and_return(true) provider.dscl_create_comment expect(new_resource.comment).to eq("#mockssuck") end it "should run run_dscl with create /Users/user PrimaryGroupID to set the users primary group" do - expect(provider).to receive(:run_dscl).with("create /Users/toor PrimaryGroupID '1001'").and_return(true) + expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "PrimaryGroupID", 1001).and_return(true) provider.dscl_set_gid end it "should run run_dscl with create /Users/user UserShell to set the users login shell" do - expect(provider).to receive(:run_dscl).with("create /Users/toor UserShell '/usr/bin/false'").and_return(true) + expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "UserShell", "/usr/bin/false").and_return(true) provider.dscl_set_shell end end @@ -788,21 +786,21 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30") end it "should map the group name to a numeric ID when the group exists" do - expect(provider).to receive(:run_dscl).with("read /Groups/newgroup PrimaryGroupID").ordered.and_return("PrimaryGroupID: 1001\n") - expect(provider).to receive(:run_dscl).with("create /Users/toor PrimaryGroupID '1001'").ordered.and_return(true) + expect(provider).to receive(:run_dscl).with("read", "/Groups/newgroup", "PrimaryGroupID").ordered.and_return("PrimaryGroupID: 1001\n") + expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "PrimaryGroupID", "1001").ordered.and_return(true) provider.dscl_set_gid end it "should raise an exception when the group does not exist" do shell_return = shellcmdresult.new(" DS Error: -14136 (eDSRecordNotFound)", "err", -14136) - expect(provider).to receive(:shell_out).with("dscl . -read /Groups/newgroup PrimaryGroupID").and_return(shell_return) + expect(provider).to receive(:shell_out).with("dscl", ".", "-read", "/Groups/newgroup", "PrimaryGroupID").and_return(shell_return) expect { provider.dscl_set_gid }.to raise_error(Chef::Exceptions::GroupIDNotFound) end end it "should set group ID to 20 if it's not specified" do new_resource.gid nil - expect(provider).to receive(:run_dscl).with("create /Users/toor PrimaryGroupID '20'").ordered.and_return(true) + expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "PrimaryGroupID", 20).ordered.and_return(true) provider.dscl_set_gid expect(new_resource.gid).to eq(20) end @@ -850,8 +848,8 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30") describe "when the user exists" do before do - expect(provider).to receive(:shell_out).with("dscacheutil '-flushcache'") - expect(provider).to receive(:shell_out).with("plutil -convert xml1 -o - /var/db/dslocal/nodes/Default/users/toor.plist") do + expect(provider).to receive(:shell_out).with("dscacheutil", "-flushcache") + expect(provider).to receive(:shell_out).with("plutil", "-convert", "xml1", "-o", "-", "/var/db/dslocal/nodes/Default/users/toor.plist") do shellcmdresult.new(File.read(File.join(CHEF_SPEC_DATA, "mac_users/10.9.plist.xml")), "", 0) end provider.load_current_resource @@ -859,14 +857,13 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30") describe "when Chef is removing the user" do it "removes the user from the groups and deletes home directory when the resource is configured to manage home" do - Chef::Config[:treat_deprecation_warnings_as_errors] = false - new_resource.supports({ :manage_home => true }) - expect(provider).to receive(:run_dscl).with("list /Groups").and_return("my_group\nyour_group\nreal_group\n") - expect(provider).to receive(:run_dscl).with("read /Groups/my_group").and_raise(Chef::Exceptions::DsclCommandFailed) # Empty group - expect(provider).to receive(:run_dscl).with("read /Groups/your_group").and_return("GroupMembership: not_you") - expect(provider).to receive(:run_dscl).with("read /Groups/real_group").and_return("GroupMembership: toor") - expect(provider).to receive(:run_dscl).with("delete /Groups/real_group GroupMembership 'toor'") - expect(provider).to receive(:run_dscl).with("delete /Users/toor") + new_resource.manage_home true + expect(provider).to receive(:run_dscl).with("list", "/Groups").and_return("my_group\nyour_group\nreal_group\n") + expect(provider).to receive(:run_dscl).with("read", "/Groups/my_group").and_raise(Chef::Exceptions::DsclCommandFailed) # Empty group + expect(provider).to receive(:run_dscl).with("read", "/Groups/your_group").and_return("GroupMembership: not_you") + expect(provider).to receive(:run_dscl).with("read", "/Groups/real_group").and_return("GroupMembership: toor") + expect(provider).to receive(:run_dscl).with("delete", "/Groups/real_group", "GroupMembership", "toor") + expect(provider).to receive(:run_dscl).with("delete", "/Users/toor") expect(FileUtils).to receive(:rm_rf).with("/Users/vagrant") provider.remove_user end @@ -889,7 +886,7 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30") end it "can unlock the user" do - expect(provider).to receive(:run_dscl).with("create /Users/toor AuthenticationAuthority ';ShadowHash;HASHLIST:'") + expect(provider).to receive(:run_dscl).with("create", "/Users/toor", "AuthenticationAuthority", ";ShadowHash;HASHLIST:") provider.unlock_user end end @@ -897,7 +894,7 @@ ea18e18b720e358e7fbe3cfbeaa561456f6ba008937a30") describe "when locking the user" do it "should run run_dscl with append /Users/user AuthenticationAuthority ;DisabledUser; to lock the user account" do - expect(provider).to receive(:run_dscl).with("append /Users/toor AuthenticationAuthority ';DisabledUser;'") + expect(provider).to receive(:run_dscl).with("append", "/Users/toor", "AuthenticationAuthority", ";DisabledUser;") provider.lock_user end end diff --git a/spec/unit/provider/user/linux_spec.rb b/spec/unit/provider/user/linux_spec.rb index ce9b84ec92..b04ada2511 100644 --- a/spec/unit/provider/user/linux_spec.rb +++ b/spec/unit/provider/user/linux_spec.rb @@ -55,7 +55,7 @@ describe Chef::Provider::User::Linux do # supports is a method on the superclass so can't totally be removed, but we should aggressively NOP it to decisively break it it "disables the supports API", chef: ">= 13" do - @new_resource.supports( { manage_home: true } ) + @new_resource.supports( manage_home: true ) expect( @new_resource.supports.key?(:manage_home) ).to be false end @@ -70,13 +70,13 @@ describe Chef::Provider::User::Linux do it "throws a deprecation warning on setting supports[:non_unique]" do Chef::Config[:treat_deprecation_warnings_as_errors] = false expect(Chef).to receive(:deprecated).with(:supports_property, "supports { non_unique: true } on the user resource is deprecated and will be removed in Chef 13, set non_unique true instead") - @new_resource.supports( { :non_unique => true } ) + @new_resource.supports( non_unique: true ) end it "throws a deprecation warning on setting supports[:manage_home]" do Chef::Config[:treat_deprecation_warnings_as_errors] = false expect(Chef).to receive(:deprecated).with(:supports_property, "supports { manage_home: true } on the user resource is deprecated and will be removed in Chef 13, set manage_home true instead") - @new_resource.supports( { :manage_home => true } ) + @new_resource.supports( manage_home: true ) end it "defaults manage_home to false" do @@ -85,13 +85,13 @@ describe Chef::Provider::User::Linux do it "supports[:manage_home] (incorectly) acts like manage_home" do Chef::Config[:treat_deprecation_warnings_as_errors] = false - @new_resource.supports({ manage_home: true }) + @new_resource.supports(manage_home: true) expect( provider.useradd_options ).to eql(["-m"]) end it "supports[:manage_home] does not change behavior of manage_home: false", chef: ">= 13" do Chef::Config[:treat_deprecation_warnings_as_errors] = false - @new_resource.supports({ manage_home: true }) + @new_resource.supports(manage_home: true) expect( provider.useradd_options ).to eql(["-M"]) end diff --git a/spec/unit/provider/user/pw_spec.rb b/spec/unit/provider/user/pw_spec.rb index fb7c9211a1..2f44d6f3e3 100644 --- a/spec/unit/provider/user/pw_spec.rb +++ b/spec/unit/provider/user/pw_spec.rb @@ -34,7 +34,7 @@ describe Chef::Provider::User::Pw do # XXX: rip out in Chef-13 Chef::Config[:treat_deprecation_warnings_as_errors] = false - @new_resource.supports :manage_home => true + @new_resource.supports manage_home: true @current_resource = Chef::Resource::User::PwUser.new("adam") @current_resource.comment "Adam Jacob" @@ -65,35 +65,36 @@ describe Chef::Provider::User::Pw do it "should set the option for #{attribute} if the new resources #{attribute} is not null" do allow(@new_resource).to receive(attribute).and_return("hola") - expect(@provider.set_options).to eql(" #{@new_resource.username} #{option} '#{@new_resource.send(attribute)}' -m") + expect(@provider.set_options).to eql([ @new_resource.username, option, @new_resource.send(attribute), "-m"]) end it "should set the option for #{attribute} if the new resources #{attribute} is not null, without homedir management" do - allow(@new_resource).to receive(:supports).and_return({ :manage_home => false }) + allow(@new_resource).to receive(:supports).and_return(manage_home: false) allow(@new_resource).to receive(attribute).and_return("hola") - expect(@provider.set_options).to eql(" #{@new_resource.username} #{option} '#{@new_resource.send(attribute)}'") + expect(@provider.set_options).to eql([@new_resource.username, option, @new_resource.send(attribute)]) end end it "should combine all the possible options" do - match_string = " adam" + match_array = [ "adam" ] field_list.sort { |a, b| a[0] <=> b[0] }.each do |attribute, option| allow(@new_resource).to receive(attribute).and_return("hola") - match_string << " #{option} 'hola'" + match_array << option + match_array << "hola" end - match_string << " -m" - expect(@provider.set_options).to eql(match_string) + match_array << "-m" + expect(@provider.set_options).to eql(match_array) end end describe "create_user" do before(:each) do - allow(@provider).to receive(:run_command).and_return(true) + allow(@provider).to receive(:shell_out!).and_return(true) allow(@provider).to receive(:modify_password).and_return(true) end it "should run pw useradd with the return of set_options" do - expect(@provider).to receive(:run_command).with({ :command => "pw useradd adam -m" }).and_return(true) + expect(@provider).to receive(:shell_out!).with("pw", "useradd", "adam", "-m").and_return(true) @provider.create_user end @@ -105,12 +106,12 @@ describe Chef::Provider::User::Pw do describe "manage_user" do before(:each) do - allow(@provider).to receive(:run_command).and_return(true) + allow(@provider).to receive(:shell_out!).and_return(true) allow(@provider).to receive(:modify_password).and_return(true) end it "should run pw usermod with the return of set_options" do - expect(@provider).to receive(:run_command).with({ :command => "pw usermod adam -m" }).and_return(true) + expect(@provider).to receive(:shell_out!).with("pw", "usermod", "adam", "-m").and_return(true) @provider.manage_user end @@ -122,13 +123,13 @@ describe Chef::Provider::User::Pw do describe "remove_user" do it "should run pw userdel with the new resources user name" do - @new_resource.supports :manage_home => false - expect(@provider).to receive(:run_command).with({ :command => "pw userdel #{@new_resource.username}" }).and_return(true) + @new_resource.supports manage_home: false + expect(@provider).to receive(:shell_out!).with("pw", "userdel", @new_resource.username).and_return(true) @provider.remove_user end it "should run pw userdel with the new resources user name and -r if manage_home is true" do - expect(@provider).to receive(:run_command).with({ :command => "pw userdel #{@new_resource.username} -r" }).and_return(true) + expect(@provider).to receive(:shell_out!).with("pw", "userdel", @new_resource.username, "-r").and_return(true) @provider.remove_user end end @@ -147,23 +148,26 @@ describe Chef::Provider::User::Pw do describe "when locking the user" do it "should run pw lock with the new resources username" do - expect(@provider).to receive(:run_command).with({ :command => "pw lock #{@new_resource.username}" }) + expect(@provider).to receive(:shell_out!).with("pw", "lock", @new_resource.username) @provider.lock_user end end describe "when unlocking the user" do it "should run pw unlock with the new resources username" do - expect(@provider).to receive(:run_command).with({ :command => "pw unlock #{@new_resource.username}" }) + expect(@provider).to receive(:shell_out!).with("pw", "unlock", @new_resource.username) @provider.unlock_user end end describe "when modifying the password" do before(:each) do - @status = double("Status", :exitstatus => 0) + @status = double("Status", exitstatus: 0) allow(@provider).to receive(:popen4).and_return(@status) - @pid, @stdin, @stdout, @stderr = nil, nil, nil, nil + @pid = nil + @stdin = nil + @stdout = nil + @stderr = nil end describe "and the new password has not been specified" do @@ -210,7 +214,7 @@ describe Chef::Provider::User::Pw do end it "should run pw usermod with the username and the option -H 0" do - expect(@provider).to receive(:popen4).with("pw usermod adam -H 0", :waitlast => true).and_return(@status) + expect(@provider).to receive(:popen4).with("pw usermod adam -H 0", waitlast: true).and_return(@status) @provider.modify_password end @@ -239,12 +243,12 @@ describe Chef::Provider::User::Pw do end it "should raise an error if the required binary /usr/sbin/pw doesn't exist" do - expect(File).to receive(:exists?).with("/usr/sbin/pw").and_return(false) + expect(File).to receive(:exist?).with("/usr/sbin/pw").and_return(false) expect { @provider.load_current_resource }.to raise_error(Chef::Exceptions::User) end it "shouldn't raise an error if /usr/sbin/pw exists" do - allow(File).to receive(:exists?).and_return(true) + allow(File).to receive(:exist?).and_return(true) expect { @provider.load_current_resource }.not_to raise_error end end diff --git a/spec/unit/provider/user/windows_spec.rb b/spec/unit/provider/user/windows_spec.rb index 324b5f503f..82f3c1ab2a 100644 --- a/spec/unit/provider/user/windows_spec.rb +++ b/spec/unit/provider/user/windows_spec.rb @@ -127,19 +127,19 @@ describe Chef::Provider::User::Windows do describe "when creating the user" do it "should call @net_user.add with the return of set_options" do - allow(@provider).to receive(:set_options).and_return(:name => "monkey") - expect(@net_user).to receive(:add).with(:name => "monkey") + allow(@provider).to receive(:set_options).and_return(name: "monkey") + expect(@net_user).to receive(:add).with(name: "monkey") @provider.create_user end end describe "manage_user" do before(:each) do - allow(@provider).to receive(:set_options).and_return(:name => "monkey") + allow(@provider).to receive(:set_options).and_return(name: "monkey") end it "should call @net_user.update with the return of set_options" do - expect(@net_user).to receive(:update).with(:name => "monkey") + expect(@net_user).to receive(:update).with(name: "monkey") @provider.manage_user end end diff --git a/spec/unit/provider/user_spec.rb b/spec/unit/provider/user_spec.rb index 719dc8d492..489597445f 100644 --- a/spec/unit/provider/user_spec.rb +++ b/spec/unit/provider/user_spec.rb @@ -58,7 +58,7 @@ describe Chef::Provider::User do describe "executing load_current_resource" do before(:each) do @node = Chef::Node.new - #@new_resource = double("Chef::Resource::User", + # @new_resource = double("Chef::Resource::User", # :null_object => true, # :username => "adam", # :comment => "Adam Jacob", @@ -68,7 +68,7 @@ describe Chef::Provider::User do # :shell => "/usr/bin/zsh", # :password => nil, # :updated => nil - #) + # ) allow(Chef::Resource::User).to receive(:new).and_return(@current_resource) @pw_user = EtcPwnamIsh.new @pw_user.name = "adam" @@ -110,11 +110,11 @@ describe Chef::Provider::User do # The mapping between the Chef::Resource::User and Getpwnam struct user_attrib_map = { - :uid => :uid, - :gid => :gid, - :comment => :gecos, - :home => :dir, - :shell => :shell, + uid: :uid, + gid: :gid, + comment: :gecos, + home: :dir, + shell: :shell, } user_attrib_map.each do |user_attrib, getpwnam_attrib| it "should set the current resources #{user_attrib} based on getpwnam #{getpwnam_attrib}" do @@ -140,18 +140,16 @@ describe Chef::Provider::User do describe "and running assertions" do def self.shadow_lib_unavail? - begin - require "rubygems" - require "shadow" - rescue LoadError - skip "ruby-shadow gem not installed for dynamic load test" - true - else - false - end + require "rubygems" + require "shadow" + rescue LoadError + skip "ruby-shadow gem not installed for dynamic load test" + true + else + false end - before (:each) do + before(:each) do user = @pw_user.dup user.name = "root" user.passwd = "x" @@ -161,15 +159,15 @@ describe Chef::Provider::User do unless shadow_lib_unavail? context "and we have the ruby-shadow gem" do - skip "and we are not root (rerun this again as root)", :requires_unprivileged_user => true + skip "and we are not root (rerun this again as root)", requires_unprivileged_user: true - context "and we are root", :requires_root => true do + context "and we are root", requires_root: true do it "should pass assertions when ruby-shadow can be loaded" do @provider.action = "create" original_method = @provider.method(:require) expect(@provider).to receive(:require) { |*args| original_method.call(*args) } - passwd_info = Struct::PasswdEntry.new(:sp_namp => "adm ", :sp_pwdp => "$1$T0N0Q.lc$nyG6pFI3Dpqa5cxUz/57j0", :sp_lstchg => 14861, :sp_min => 0, :sp_max => 99999, - :sp_warn => 7, :sp_inact => -1, :sp_expire => -1, :sp_flag => -1) + passwd_info = Struct::PasswdEntry.new(sp_namp: "adm ", sp_pwdp: "$1$T0N0Q.lc$nyG6pFI3Dpqa5cxUz/57j0", sp_lstchg: 14861, sp_min: 0, sp_max: 99999, + sp_warn: 7, sp_inact: -1, sp_expire: -1, sp_flag: -1) expect(Shadow::Passwd).to receive(:getspnam).with("adam").and_return(passwd_info) @provider.load_current_resource @provider.define_resource_requirements -- cgit v1.2.1