[macos] fix mac_user platform constraintsryan/fix-macos-build

Signed-off-by: Ryan Cragun <ryan@chef.io>

4 files changed, 45 insertions, 26 deletions

diff --git a/lib/chef/provider/user/mac.rb b/lib/chef/provider/user/mac.rb
index 414445cfa1..7b12eaec3c 100644
--- a/lib/chef/provider/user/mac.rb
+++ b/lib/chef/provider/user/mac.rb
@@ -134,8 +134,8 @@ class Chef
 
         def create_user
           cmd = [-"-addUser", new_resource.username]
-          cmd += ["-fullName", new_resource.comment] if new_resource.property_is_set?(:comment)
-          cmd += ["-UID", new_resource.uid]          if new_resource.property_is_set?(:uid)
+          cmd += ["-fullName", new_resource.comment] if prop_is_set?(:comment)
+          cmd += ["-UID", new_resource.uid]          if prop_is_set?(:uid)
           cmd += ["-shell", new_resource.shell]
           cmd += ["-home", new_resource.home]
           cmd += ["-admin"] if new_resource.admin
@@ -143,7 +143,7 @@ class Chef
           # We can technically create a new user without the admin credentials
           # but without them the user cannot enable SecureToken, thus they cannot
           # create other secure users or enable FileVault full disk encryption.
-          if new_resource.property_is_set?(:admin_username) && new_resource.property_is_set?(:admin_password)
+          if prop_is_set?(:admin_username) && prop_is_set?(:admin_password)
             cmd += ["-adminUser", new_resource.admin_username]
             cmd += ["-adminPassword", new_resource.admin_password]
           end
@@ -165,7 +165,7 @@ class Chef
           reload_user_plist
           reload_admin_group_plist
 
-          if new_resource.property_is_set?(:password)
+          if prop_is_set?(:password)
             converge_by("set password") { set_password }
           end
 
@@ -181,7 +181,7 @@ class Chef
             end
           end
 
-          if new_resource.property_is_set?(:gid)
+          if prop_is_set?(:gid)
             # NOTE: Here we're managing the primary group of the user which is
             # a departure from previous behavior. We could just set the
             # PrimaryGroupID for the user and move on if we decide that actual
@@ -282,7 +282,7 @@ class Chef
         def remove_user
           cmd = ["-deleteUser", new_resource.username]
           cmd << new_resource.manage_home ? "-secure" : "-keepHome"
-          if new_resource.property_is_set?(:admin_username) && new_resource.property_is_set?(:admin_password)
+          if %i{admin_username admin_password}.all? { |p| prop_is_set?(p) }
             cmd += ["-adminUser", new_resource.admin_username]
             cmd += ["-adminPassword", new_resource.admin_password]
           end
@@ -344,7 +344,7 @@ class Chef
           else
             # Other fields are have been set on current resource so just compare
             # them.
-            new_resource.property_is_set?(prop) && (new_resource.send(prop) != current_resource.send(prop))
+            !new_resource.send(prop).nil? && (new_resource.send(prop) != current_resource.send(prop))
           end
         end
 
@@ -384,14 +384,8 @@ class Chef
         def toggle_secure_token
           # Check for this lazily as we only need to validate for these credentials
           # if we're toggling secure token.
-          unless new_resource.property_is_set?(:admin_username) &&
-              new_resource.property_is_set?(:admin_password) &&
-              # property_is_set? can't handle a default inherited from password
-              # when not using shadow hash data. Hence, we'll just have to
-              # make sure some valid string is there.
-              new_resource.secure_token_password &&
-              new_resource.secure_token_password != ""
-            raise Chef::Exceptions::User, "secure_token_password, admin_user and admin_password properties are required to modify SecureToken"
+          unless %i{admin_username admin_password secure_token_password}.all? { |p| prop_is_set?(p) }
+            raise Chef::Exceptions::User, "secure_token_password, admin_username and admin_password properties are required to modify SecureToken"
           end
 
           cmd = (new_resource.secure_token ? %w{-secureTokenOn} : %w{-secureTokenOff})
@@ -414,11 +408,11 @@ class Chef
           # Therefore, if we're configuring a user based upon existing shadow
           # hash data we'll have to set the password again so that future runs
           # of the client don't show password drift.
-          set_password if new_resource.property_is_set?(:salt)
+          set_password if prop_is_set?(:salt)
         end
 
         def user_group_diverged?
-          return false unless new_resource.property_is_set?(:gid)
+          return false unless prop_is_set?(:gid)
 
           group_name, group_id = user_group_info
 
@@ -439,11 +433,11 @@ class Chef
           #   * Not configuring it
 
           # Check for no desired password configuration
-          return false unless new_resource.property_is_set?(:password)
+          return false unless prop_is_set?(:password)
 
           # Check for ShadowHashData divergence by comparing the entropy,
           # salt, and iterations.
-          if new_resource.property_is_set?(:salt)
+          if prop_is_set?(:salt)
             return true if %i{salt iterations}.any? { |prop| diverged?(prop) }
 
             return new_resource.password != current_resource.password
@@ -473,7 +467,7 @@ class Chef
         end
 
         def set_password
-          if new_resource.property_is_set?(:salt)
+          if prop_is_set?(:salt)
             entropy = StringIO.new(convert_to_binary(new_resource.password))
             salt = StringIO.new(convert_to_binary(new_resource.salt))
           else
@@ -586,6 +580,12 @@ class Chef
           result.stdout
         end
 
+        def prop_is_set?(prop)
+          v = new_resource.send(prop.to_sym)
+
+          !v.nil? && v != ""
+        end
+
         class Plist
           DSCL_PROPERTY_MAP = {
               uid: "dsAttrTypeStandard:UniqueID",
diff --git a/lib/chef/resource/user/dscl_user.rb b/lib/chef/resource/user/dscl_user.rb
index a3f6661c22..5ba9d3d099 100644
--- a/lib/chef/resource/user/dscl_user.rb
+++ b/lib/chef/resource/user/dscl_user.rb
@@ -24,7 +24,7 @@ class Chef
         resource_name :dscl_user
 
         provides :dscl_user
-        provides :user, os: "darwin", platform_version: "<= 10.13"
+        provides :user, platform: "mac_os_x", platform_version: "< 10.14"
 
         property :iterations, Integer,
           description: "macOS platform only. The number of iterations for a password with a SALTED-SHA512-PBKDF2 shadow hash.",
diff --git a/lib/chef/resource/user/mac_user.rb b/lib/chef/resource/user/mac_user.rb
index ab5cc12947..0892dea077 100644
--- a/lib/chef/resource/user/mac_user.rb
+++ b/lib/chef/resource/user/mac_user.rb
@@ -61,7 +61,7 @@ class Chef
         resource_name :mac_user
 
         provides :mac_user
-        provides :user, os: "darwin", platform_version: ">= 10.14"
+        provides :user, platform: "mac_os_x", platform_version: ">= 10.14"
 
         introduced "15.3"
 
diff --git a/spec/functional/resource/user/mac_user_spec.rb b/spec/functional/resource/user/mac_user_spec.rb
index d01b08616e..102c2824bf 100644
--- a/spec/functional/resource/user/mac_user_spec.rb
+++ b/spec/functional/resource/user/mac_user_spec.rb
@@ -32,6 +32,11 @@ describe "Chef::Resource::User with Chef::Provider::User::MacUser provider", met
       # Raised when the user is already cleaned
   end
 
+  def ensure_file_cache_path_exists
+    path = Chef::Config["file_cache_path"]
+    FileUtils.mkdir_p(path) unless File.directory?(path)
+  end
+
   def user_should_exist
     expect(shell_out("/usr/bin/dscl . -read /Users/#{username}").error?).to be(false)
   end
@@ -89,6 +94,7 @@ describe "Chef::Resource::User with Chef::Provider::User::MacUser provider", met
 
   before do
     clean_user
+    ensure_file_cache_path_exists
   end
 
   after(:each) do
@@ -148,10 +154,23 @@ c5adbbac718b7eb99463a7b679571e0f\
       end
 
       describe "when password is updated" do
-        it "should update the password of the user" do
-          user_resource.password("mykitchen")
-          user_resource.run_action(:create)
-          check_password("mykitchen")
+        describe "without salt" do
+          let(:salt) { nil }
+
+          it "it sets the password" do
+            user_resource.password("mykitchen")
+            user_resource.run_action(:create)
+            check_password("mykitchen")
+          end
+        end
+
+        describe "with salt and plaintext password" do
+          it "raises Chef::Exceptions::User" do
+            expect do
+              user_resource.password("notasha512")
+              user_resource.run_action(:create)
+            end.to raise_error(Chef::Exceptions::User)
+          end
         end
       end
     end