Access remote_file resource source files as alternate user on Windows

Conflicts:
	spec/support/shared/functional/execute_resource.rb

8 files changed, 242 insertions, 30 deletions

diff --git a/lib/chef/mixin/user_context.rb b/lib/chef/mixin/user_context.rb
index 5305678f2b..b29afa5087 100644
--- a/lib/chef/mixin/user_context.rb
+++ b/lib/chef/mixin/user_context.rb
@@ -49,7 +49,7 @@ class Chef
             logon_session.open
             logon_session.set_user_context
           end
-          block.call
+          yield
         ensure
           logon_session.close if logon_session
         end
diff --git a/lib/chef/mixin/user_identity.rb b/lib/chef/mixin/user_identity.rb
index 6218b44ac2..c037adb441 100644
--- a/lib/chef/mixin/user_identity.rb
+++ b/lib/chef/mixin/user_identity.rb
@@ -57,7 +57,7 @@ class Chef
           domain_and_user = user.split('\\')
 
           if domain_and_user.length == 1
-            domain_and_user = user.split('@')
+            domain_and_user = user.split("@")
           end
 
           if domain_and_user.length == 2
diff --git a/lib/chef/provider/remote_file/network_file.rb b/lib/chef/provider/remote_file/network_file.rb
index 44046132a9..2bfcf38dbd 100644
--- a/lib/chef/provider/remote_file/network_file.rb
+++ b/lib/chef/provider/remote_file/network_file.rb
@@ -19,14 +19,21 @@
 require "uri"
 require "tempfile"
 require "chef/provider/remote_file"
+require "chef/mixin/user_identity"
+require "chef/mixin/user_context"
 
 class Chef
   class Provider
     class RemoteFile
       class NetworkFile
 
+        include Chef::Mixin::UserIdentity
+        include Chef::Mixin::UserContext
+
         attr_reader :new_resource
 
+        TRANSFER_CHUNK_SIZE = 1048576
+
         def initialize(source, new_resource, current_resource)
           @new_resource = new_resource
           @source = source
@@ -35,13 +42,23 @@ class Chef
         # Fetches the file on a network share, returning a Tempfile-like File handle
         # windows only
         def fetch
-          tempfile = Chef::FileContentManagement::Tempfile.new(new_resource).tempfile
-          Chef::Log.debug("#{new_resource} staging #{@source} to #{tempfile.path}")
-          FileUtils.cp(@source, tempfile.path)
-          tempfile.close if tempfile
+          validate_identity(new_resource.remote_user, new_resource.remote_user_password, new_resource.remote_user_domain)
+          begin
+            tempfile = Chef::FileContentManagement::Tempfile.new(new_resource).tempfile
+            Chef::Log.debug("#{new_resource} staging #{@source} to #{tempfile.path}")
+
+            with_user_context(new_resource.remote_user, new_resource.remote_user_password, new_resource.remote_user_domain) do
+              ::File.open(@source, "rb") do |remote_file|
+                while data = remote_file.read(TRANSFER_CHUNK_SIZE)
+                  tempfile.write(data)
+                end
+              end
+            end
+          ensure
+            tempfile.close if tempfile
+          end
           tempfile
         end
-
       end
     end
   end
diff --git a/lib/chef/resource/remote_file.rb b/lib/chef/resource/remote_file.rb
index 4a1d1c6cff..f631b4e579 100644
--- a/lib/chef/resource/remote_file.rb
+++ b/lib/chef/resource/remote_file.rb
@@ -131,6 +131,38 @@ class Chef
         )
       end
 
+      def remote_user(args = nil)
+        set_or_return(
+          :remote_user,
+          args,
+          :kind_of => String
+        )
+      end
+
+      def remote_user_domain(args = nil)
+        set_or_return(
+          :remote_user_domain,
+          args,
+          :kind_of => String
+        )
+      end
+
+      def remote_user_password(args = nil)
+        set_or_return(
+          :remote_user_password,
+          args,
+          :kind_of => String
+        )
+      end
+
+      def sensitive(args = nil)
+        if ! remote_user_password.nil?
+          true
+        else
+          super
+        end
+      end
+
       private
 
       include Chef::Mixin::Uris
diff --git a/spec/functional/resource/remote_file_spec.rb b/spec/functional/resource/remote_file_spec.rb
index 1f92a567f3..60c2f87bb6 100644
--- a/spec/functional/resource/remote_file_spec.rb
+++ b/spec/functional/resource/remote_file_spec.rb
@@ -123,6 +123,164 @@ describe Chef::Resource::RemoteFile do
 
   end
 
+  context "when running on Windows", :windows_only do
+    describe "when fetching files over SMB" do
+      include Chef::Mixin::ShellOut
+      let(:smb_share_root_directory) { directory = File.join(Dir.tmpdir, make_tmpname("windows_script_test")); Dir.mkdir(directory); directory }
+      let(:smb_file_local_file_name) { "smb_file.txt" }
+      let(:smb_file_local_path) { File.join( smb_share_root_directory, smb_file_local_file_name ) }
+      let(:smb_share_name) { "chef_smb_test" }
+      let(:smb_remote_path) { File.join("//#{ENV['COMPUTERNAME']}", smb_share_name, smb_file_local_file_name).gsub(/\//, "\\") }
+      let(:smb_file_content) { "hellofun" }
+      let(:local_destination_path) { File.join(Dir.tmpdir, make_tmpname("chef_remote_file")) }
+      let(:windows_current_user) { ENV["USERNAME"] }
+      let(:windows_current_user_domain) { ENV["USERDOMAIN"] || ENV["COMPUTERNAME"] }
+      let(:windows_current_user_qualified) { "#{windows_current_user_domain}\\#{windows_current_user}" }
+
+      let(:remote_domain) { nil }
+      let(:remote_user) { nil }
+      let(:remote_password) { nil }
+
+      let(:resource) do
+        node = Chef::Node.new
+        events = Chef::EventDispatch::Dispatcher.new
+        run_context = Chef::RunContext.new(node, {}, events)
+        resource = Chef::Resource::RemoteFile.new(path, run_context)
+      end
+
+      before do
+        shell_out("net.exe share #{smb_share_name} /delete")
+        File.write(smb_file_local_path, smb_file_content )
+        shell_out!("net.exe share #{smb_share_name}=\"#{smb_share_root_directory.gsub(/\//, '\\')}\" /grant:\"authenticated users\",read")
+      end
+
+      after do
+        shell_out("net.exe share #{smb_share_name} /delete")
+        File.delete(smb_file_local_path) if File.exist?(smb_file_local_path)
+        File.delete(local_destination_path) if File.exist?(local_destination_path)
+        Dir.rmdir(smb_share_root_directory)
+      end
+
+      context "when configuring the Windows identity used to access the remote file" do
+        before do
+          resource.path(local_destination_path)
+          resource.source(smb_remote_path)
+          resource.remote_user_domain(remote_domain)
+          resource.remote_user(remote_user)
+          resource.remote_user_password(remote_password)
+        end
+
+        shared_examples_for "a remote_file resource accessing a remote file to which the specified user has access" do
+          it "has the same content as the original file" do
+            expect { resource.run_action(:create) }.not_to raise_error
+            expect(::File.read(local_destination_path).chomp).to eq smb_file_content
+          end
+        end
+
+        shared_examples_for "a remote_file resource accessing a remote file to which the specified user does not have access" do
+          it "causes an error to be raised" do
+            expect { resource.run_action(:create) }.to raise_error(Errno::EACCES)
+          end
+        end
+
+        context "when the the file is accessible to non-admin users only as the current identity" do
+          before do
+            shell_out!("icacls #{smb_file_local_path} /grant:r \"authenticated users:(W)\" /grant \"#{windows_current_user_qualified}:(R)\" /inheritance:r")
+          end
+
+          context "when the resource is accessed using the current user's identity" do
+            let(:remote_user) { nil }
+            let(:remote_domain) { nil }
+            let(:remote_password) { nil }
+
+            it_behaves_like "a remote_file resource accessing a remote file to which the specified user has access"
+
+            describe "uses the ::Chef::Provider::RemoteFile::NetworkFile::TRANSFER_CHUNK_SIZE constant to chunk the file" do
+              let(:invalid_chunk_size) { -1 }
+              before do
+                stub_const("::Chef::Provider::RemoteFile::NetworkFile::TRANSFER_CHUNK_SIZE", invalid_chunk_size)
+              end
+
+              it "raises an ArgumentError when the chunk size is negative" do
+                expect(::Chef::Provider::RemoteFile::NetworkFile::TRANSFER_CHUNK_SIZE).to eq(invalid_chunk_size)
+                expect { resource.run_action(:create) }.to raise_error(ArgumentError)
+              end
+            end
+
+            context "when the file must be transferred in more than one chunk" do
+              before do
+                stub_const("::Chef::Provider::RemoteFile::NetworkFile::TRANSFER_CHUNK_SIZE", 3)
+              end
+              it_behaves_like "a remote_file resource accessing a remote file to which the specified user has access"
+            end
+          end
+
+          context "when the resource is accessed using an alternate user's identity with no access to the file" do
+            let (:windows_nonadmin_user) { "chefremfile1" }
+            let (:windows_nonadmin_user_password) { "j82ajfxK3;2Xe1" }
+            include_context "a non-admin Windows user"
+
+            let(:remote_user) { windows_nonadmin_user }
+            let(:remote_domain) { nil }
+            let(:remote_password) { windows_nonadmin_user_password }
+
+            it_behaves_like "a remote_file resource accessing a remote file to which the specified user does not have access"
+          end
+        end
+
+        context "when the the file is only accessible as a specific alternate identity" do
+          let (:windows_nonadmin_user) { "chefremfile2" }
+          let (:windows_nonadmin_user_password) { "j82ajfxK3;2Xe2" }
+          include_context "a non-admin Windows user"
+
+          before do
+            shell_out!("icacls #{smb_file_local_path} /grant:r \"authenticated users:(W)\" /grant \"#{windows_nonadmin_user_qualified}:(R)\" /deny #{windows_current_user_qualified}:(R) /inheritance:r")
+          end
+
+          context "when the resource is accessed using the specific non-qualified alternate user identity with access" do
+            let(:remote_user) { windows_nonadmin_user }
+            let(:remote_domain) { nil }
+            let(:remote_password) { windows_nonadmin_user_password }
+
+            it_behaves_like "a remote_file resource accessing a remote file to which the specified user has access"
+          end
+
+          context "when the resource is accessed using the specific alternate user identity with access and the domain is specified" do
+            let(:remote_user) { windows_nonadmin_user }
+            let(:remote_domain) { windows_nonadmin_user_domain }
+            let(:remote_password) { windows_nonadmin_user_password }
+
+            it_behaves_like "a remote_file resource accessing a remote file to which the specified user has access"
+          end
+
+          context "when the resource is accessed using the specific qualified alternate user identity with access" do
+            let(:remote_user) { windows_nonadmin_user_qualified }
+            let(:remote_domain) { nil }
+            let(:remote_password) { windows_nonadmin_user_password }
+
+            it_behaves_like "a remote_file resource accessing a remote file to which the specified user has access"
+          end
+
+          context "when the resource is accessed using the current user's identity" do
+            it_behaves_like "a remote_file resource accessing a remote file to which the specified user does not have access"
+          end
+
+          context "when the resource is accessed using an alternate user's identity with no access to the file" do
+            let (:windows_nonadmin_user) { "chefremfile3" }
+            let (:windows_nonadmin_user_password) { "j82ajfxK3;2Xe3" }
+            include_context "a non-admin Windows user"
+
+            let(:remote_user) { windows_nonadmin_user_qualified }
+            let(:remote_domain) { nil }
+            let(:remote_password) { windows_nonadmin_user_password }
+
+            it_behaves_like "a remote_file resource accessing a remote file to which the specified user does not have access"
+          end
+        end
+      end
+    end
+  end
+
   context "when dealing with content length checking" do
     before(:each) do
       start_tiny_server
diff --git a/spec/support/shared/functional/execute_resource.rb b/spec/support/shared/functional/execute_resource.rb
index 4f7cea1cd1..29b9a5fb9f 100644
--- a/spec/support/shared/functional/execute_resource.rb
+++ b/spec/support/shared/functional/execute_resource.rb
@@ -147,4 +147,4 @@ shared_examples_for "a resource with a guard specifying an alternate user identi
       end
     end
   end
-end
+end
\ No newline at end of file
diff --git a/spec/unit/mixin/user_identity_spec.rb b/spec/unit/mixin/user_identity_spec.rb
index f75092f660..799260b25a 100644
--- a/spec/unit/mixin/user_identity_spec.rb
+++ b/spec/unit/mixin/user_identity_spec.rb
@@ -16,13 +16,13 @@
 # limitations under the License.
 #
 
-require 'spec_helper'
-require 'chef/mixin/user_identity'
+require "spec_helper"
+require "chef/mixin/user_identity"
 
 shared_examples_for "it received valid credentials" do
   describe "the validation method" do
     it "should not raise an error" do
-      expect {instance_with_identity.validate(username, password, domain)}.not_to raise_error
+      expect { instance_with_identity.validate(username, password, domain) }.not_to raise_error
     end
   end
 
@@ -39,7 +39,7 @@ end
 shared_examples_for "it received invalid credentials" do
   describe "the validation method" do
     it "should raise an error" do
-      expect { instance_with_identity.validate(username, password, domain)}.to raise_error(ArgumentError)
+      expect { instance_with_identity.validate(username, password, domain) }.to raise_error(ArgumentError)
     end
   end
 end
@@ -47,7 +47,7 @@ end
 shared_examples_for "it received credentials that are not valid on the platform" do
   describe "the validation method" do
     it "should raise an error" do
-      expect { instance_with_identity.validate(username, password, domain)}.to raise_error(Chef::Exceptions::UnsupportedPlatform)
+      expect { instance_with_identity.validate(username, password, domain) }.to raise_error(Chef::Exceptions::UnsupportedPlatform)
     end
   end
 end
@@ -66,9 +66,9 @@ shared_examples_for "a consumer of the ::Chef::Mixin::UserIdentity mixin" do
     end
 
     context "when a valid username is specified" do
-      let(:username) { 'starchild' }
+      let(:username) { "starchild" }
       context "when a valid domain is specified" do
-        let(:domain) { 'mothership' }
+        let(:domain) { "mothership" }
 
         context "when the password is not specified" do
           let(:password) { nil }
@@ -76,7 +76,7 @@ shared_examples_for "a consumer of the ::Chef::Mixin::UserIdentity mixin" do
         end
 
         context "when the password is specified" do
-          let(:password) { 'we.funk!' }
+          let(:password) { "we.funk!" }
           it_behaves_like "it received valid credentials"
         end
       end
@@ -90,7 +90,7 @@ shared_examples_for "a consumer of the ::Chef::Mixin::UserIdentity mixin" do
         end
 
         context "when the password is specified" do
-          let(:password) { 'we.funk!' }
+          let(:password) { "we.funk!" }
           it_behaves_like "it received valid credentials"
         end
       end
@@ -100,20 +100,20 @@ shared_examples_for "a consumer of the ::Chef::Mixin::UserIdentity mixin" do
       let(:username) { nil }
 
       context "when the password is specified and the domain is not" do
-        let(:password) { 'we.funk!' }
+        let(:password) { "we.funk!" }
         let(:domain) { nil }
         it_behaves_like "it received invalid credentials"
       end
 
       context "when the domain is specified and the password is not" do
-        let(:domain) { 'mothership' }
+        let(:domain) { "mothership" }
         let(:password) { nil }
         it_behaves_like "it received invalid credentials"
       end
 
       context "when the domain and password are specified" do
-        let(:domain) { 'mothership' }
-        let(:password) { 'we.funk!' }
+        let(:domain) { "mothership" }
+        let(:password) { "we.funk!" }
         it_behaves_like "it received invalid credentials"
       end
     end
@@ -132,26 +132,26 @@ shared_examples_for "a consumer of the ::Chef::Mixin::UserIdentity mixin" do
     end
 
     context "when the user is specified and the domain and password are not" do
-      let(:username) { 'starchild' }
+      let(:username) { "starchild" }
       let(:domain) { nil }
       let(:password) { nil }
       it_behaves_like "it received valid credentials"
 
       context "when the password is specified and the domain is not" do
-        let(:password) { 'we.funk!' }
+        let(:password) { "we.funk!" }
         let(:domain) { nil }
         it_behaves_like "it received credentials that are not valid on the platform"
       end
 
       context "when the domain is specified and the password is not" do
-        let(:domain) { 'mothership' }
+        let(:domain) { "mothership" }
         let(:password) { nil }
         it_behaves_like "it received credentials that are not valid on the platform"
       end
 
       context "when the domain and password are specified" do
-        let(:domain) { 'mothership' }
-        let(:password) { 'we.funk!' }
+        let(:domain) { "mothership" }
+        let(:password) { "we.funk!" }
         it_behaves_like "it received credentials that are not valid on the platform"
       end
     end
@@ -159,9 +159,9 @@ shared_examples_for "a consumer of the ::Chef::Mixin::UserIdentity mixin" do
     context "when the user is not specified" do
       let(:username) { nil }
       context "when the domain is specified" do
-        let(:domain) { 'mothership' }
+        let(:domain) { "mothership" }
         context "when the password is specified" do
-          let(:password) { 'we.funk!' }
+          let(:password) { "we.funk!" }
           it_behaves_like "it received credentials that are not valid on the platform"
         end
 
@@ -174,7 +174,7 @@ shared_examples_for "a consumer of the ::Chef::Mixin::UserIdentity mixin" do
       context "when the domain is not specified" do
         let(:domain) { nil }
         context "when the password is specified" do
-          let(:password) { 'we.funk!' }
+          let(:password) { "we.funk!" }
           it_behaves_like "it received credentials that are not valid on the platform"
         end
       end
diff --git a/spec/unit/provider/remote_file/network_file_spec.rb b/spec/unit/provider/remote_file/network_file_spec.rb
index de065c83e2..52743c0830 100644
--- a/spec/unit/provider/remote_file/network_file_spec.rb
+++ b/spec/unit/provider/remote_file/network_file_spec.rb
@@ -1,3 +1,4 @@
+
 #
 # Author:: Jay Mundrawala (<jdm@chef.io>)
 # Copyright:: Copyright 2015-2016, Chef Software
@@ -19,6 +20,9 @@
 require "spec_helper"
 
 describe Chef::Provider::RemoteFile::NetworkFile do
+  before do
+    allow(::Chef::Platform).to receive(:windows?).and_return(true)
+  end
 
   let(:source) { "\\\\foohost\\fooshare\\Foo.tar.gz" }
 
@@ -30,10 +34,11 @@ describe Chef::Provider::RemoteFile::NetworkFile do
 
     let(:tempfile) { double("Tempfile", :path => "/tmp/foo/bar/Foo.tar.gz", :close => nil) }
     let(:chef_tempfile) { double("Chef::FileContentManagement::Tempfile", :tempfile => tempfile) }
+    let(:source_file) { double("::File", :read => nil) }
 
     it "stages the local file to a temporary file" do
       expect(Chef::FileContentManagement::Tempfile).to receive(:new).with(new_resource).and_return(chef_tempfile)
-      expect(::FileUtils).to receive(:cp).with(source, tempfile.path)
+      expect(::File).to receive(:open).with(source, "rb").and_return(source_file)
       expect(tempfile).to receive(:close)
 
       result = fetcher.fetch