Fixing review comments

Signed-off-by: nimisha <nimisha.sharad@msystechnologies.com>

4 files changed, 65 insertions, 135 deletions

diff --git a/lib/chef/mixin/user_identity.rb b/lib/chef/mixin/user_identity.rb
deleted file mode 100644
index bc8626ac65..0000000000
--- a/lib/chef/mixin/user_identity.rb
+++ /dev/null
@@ -1,90 +0,0 @@
-#
-# Author:: Adam Edwards (<adamed@chef.io>)
-# Copyright:: Copyright (c) 2016 Chef Software, Inc.
-# License:: Apache License, Version 2.0
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-
-class Chef
-  module Mixin
-    module UserIdentity
-
-      def validate_identity(specified_user, password = nil, specified_domain = nil)
-        validate_identity_platform(specified_user, password, specified_domain)
-        validate_identity_syntax(specified_user, password, specified_domain)
-      end
-
-      def validate_identity_platform(specified_user, password = nil, specified_domain = nil)
-        if ! Chef::Platform.windows?
-          if password || specified_domain
-            raise Exceptions::UnsupportedPlatform, "Values for `domain` and `password` are only supported on the Windows platform"
-          end
-        else
-          if specified_user && password.nil?
-            raise ArgumentError, "A value for `password` must be specified when a value for `user` is specified on the Windows platform"
-          end
-        end
-      end
-
-      def validate_identity_syntax(specified_user, password = nil, specified_domain = nil)
-        identity = qualify_user(specified_user, specified_domain)
-
-        if ( password || identity[:domain] ) && identity[:user].nil?
-          raise ArgumentError, "A value for `password` or `domain` was specified without specification of a value for `user`"
-        end
-      end
-
-      def qualify_user(specified_user, specified_domain = nil)
-        domain = specified_domain
-        user = specified_user
-
-        if specified_user.nil? && ! specified_domain.nil?
-          raise ArgumentError, "The domain `#{specified_domain}` was specified, but no user name was given"
-        end
-
-        # if domain is provided in both username and domain
-        if specified_user && ((specified_user.include? '\\') || (specified_user.include? "@")) && specified_domain
-          raise ArgumentError, "The domain is provided twice. Username: `#{specified_user}`, Domain: `#{specified_domain}`. Please specify domain only once."
-        end
-
-        if ! specified_user.nil? && specified_domain.nil?
-          # Splitting username of format: Domain\Username
-          domain_and_user = user.split('\\')
-
-          if domain_and_user.length == 2
-            domain = domain_and_user[0]
-            user = domain_and_user[1]
-          elsif domain_and_user.length == 1
-            # Splitting username of format: Username@Domain
-            domain_and_user = user.split("@")
-            if domain_and_user.length == 2
-              domain = domain_and_user[1]
-              user = domain_and_user[0]
-            elsif domain_and_user.length != 1
-              raise ArgumentError, "The specified user name `#{user}` is not a syntactically valid user name"
-            end
-          end
-        end
-
-        { domain: domain, user: user }
-      end
-
-      private(:validate_identity)
-      private(:validate_identity_platform)
-      private(:validate_identity_syntax)
-      private(:qualify_user)
-
-    end
-  end
-end
diff --git a/lib/chef/provider/execute.rb b/lib/chef/provider/execute.rb
index 5494405a02..a605d9f7ec 100644
--- a/lib/chef/provider/execute.rb
+++ b/lib/chef/provider/execute.rb
@@ -19,15 +19,12 @@
 require "chef/log"
 require "chef/provider"
 require "forwardable"
-require "chef/mixin/user_identity"
 
 class Chef
   class Provider
     class Execute < Chef::Provider
       extend Forwardable
 
-      include Chef::Mixin::UserIdentity
-
       provides :execute
 
       def_delegators :@new_resource, :command, :returns, :environment, :user, :domain, :password, :group, :cwd, :umask, :creates
@@ -43,10 +40,6 @@ class Chef
 
       def define_resource_requirements
         # @todo: this should change to raise in some appropriate major version bump.
-        requirements.assert(:all_actions) do |a|
-          a.assertion { validate_identity(new_resource.user, new_resource.password, new_resource.domain) }
-        end
-
         if creates && creates_relative? && !cwd
           Chef::Log.warn "Providing a relative path for the creates attribute without the cwd is deprecated and will be changed to fail in the future (CHEF-3819)"
         end
@@ -59,11 +52,6 @@ class Chef
       end
 
       def action_run
-        # parse username if it's in the following format: domain/username or username@domain
-        identity = qualify_user(new_resource.user, new_resource.domain)
-        new_resource.user identity[:user]
-        new_resource.domain identity[:domain]
-
         if creates && sentinel_file.exist?
           Chef::Log.debug("#{new_resource} sentinel file #{sentinel_file} exists - nothing to do")
           return false
diff --git a/lib/chef/provider/script.rb b/lib/chef/provider/script.rb
index 5df31786bd..fca9cea467 100644
--- a/lib/chef/provider/script.rb
+++ b/lib/chef/provider/script.rb
@@ -67,14 +67,14 @@ class Chef
       end
 
       def set_owner_and_group
-        if ! Chef::Platform.windows?
+        if Chef::Platform.windows?
+          # And on Windows also this is a no-op if there is no user specified.
+          grant_alternate_user_read_access
+        else
           # FileUtils itself implements a no-op if +user+ or +group+ are nil
           # You can prove this by running FileUtils.chown(nil,nil,'/tmp/file')
           # as an unprivileged user.
           FileUtils.chown(new_resource.user, new_resource.group, script_file.path)
-        else
-          # And on Windows also this is a no-op if there is no user specified.
-          grant_alternate_user_read_access
         end
       end
 
diff --git a/lib/chef/resource/execute.rb b/lib/chef/resource/execute.rb
index d0e4d958f6..cf81a24cec 100644
--- a/lib/chef/resource/execute.rb
+++ b/lib/chef/resource/execute.rb
@@ -135,37 +135,11 @@ class Chef
         )
       end
 
-      def user(arg = nil)
-        set_or_return(
-          :user,
-          arg,
-          :kind_of => [ String, Integer ]
-        )
-      end
-
-      def domain(arg = nil)
-        set_or_return(
-          :domain,
-          arg,
-          :kind_of => [ String ]
-        )
-      end
+      property :user, [ String, Integer ]
 
-      def password(arg = nil)
-        set_or_return(
-          :password,
-          arg,
-          :kind_of => [ String ]
-        )
-      end
+      property :domain, String
 
-      def sensitive(args = nil)
-        if ! password.nil?
-          true
-        else
-          super
-        end
-      end
+      property :password, String, sensitive: true
 
       def self.set_guard_inherited_attributes(*inherited_attributes)
         @class_inherited_attributes = inherited_attributes
@@ -183,6 +157,64 @@ class Chef
         ancestor_attributes.concat(@class_inherited_attributes ? @class_inherited_attributes : []).uniq
       end
 
+      def after_created
+        validate_identity_platform(user, password, domain)
+        identity = qualify_user(user, password, domain)
+        domain(identity[:domain])
+        user(identity[:user])
+      end
+
+      def validate_identity_platform(specified_user, password = nil, specified_domain = nil)
+        if Chef::Platform.windows?
+          if specified_user && password.nil?
+            raise ArgumentError, "A value for `password` must be specified when a value for `user` is specified on the Windows platform"
+          end
+        else
+          if password || specified_domain
+            raise Exceptions::UnsupportedPlatform, "Values for `domain` and `password` are only supported on the Windows platform"
+          end
+        end
+      end
+
+      def qualify_user(specified_user, password = nil, specified_domain = nil)
+        domain = specified_domain
+        user = specified_user
+
+        if specified_user.nil? && ! specified_domain.nil?
+          raise ArgumentError, "The domain `#{specified_domain}` was specified, but no user name was given"
+        end
+
+        # if domain is provided in both username and domain
+        if specified_user && ((specified_user.include? '\\') || (specified_user.include? "@")) && specified_domain
+          raise ArgumentError, "The domain is provided twice. Username: `#{specified_user}`, Domain: `#{specified_domain}`. Please specify domain only once."
+        end
+
+        if ! specified_user.nil? && specified_domain.nil?
+          # Splitting username of format: Domain\Username
+          domain_and_user = user.split('\\')
+
+          if domain_and_user.length == 2
+            domain = domain_and_user[0]
+            user = domain_and_user[1]
+          elsif domain_and_user.length == 1
+            # Splitting username of format: Username@Domain
+            domain_and_user = user.split("@")
+            if domain_and_user.length == 2
+              domain = domain_and_user[1]
+              user = domain_and_user[0]
+            elsif domain_and_user.length != 1
+              raise ArgumentError, "The specified user name `#{user}` is not a syntactically valid user name"
+            end
+          end
+        end
+
+        if ( password || domain ) && user.nil?
+          raise ArgumentError, "A value for `password` or `domain` was specified without specification of a value for `user`"
+        end
+
+        { domain: domain, user: user }
+      end
+
       set_guard_inherited_attributes(
         :cwd,
         :environment,