Merge pull request #399 from alexjfisher/issue_397

Use curl netrc file instead of `--user`

lib/puppet/provider/archive/curl.rb

@@ -1,10 +1,15 @@
+require 'uri'
+require 'tempfile'
+
 Puppet::Type.type(:archive).provide(:curl, parent: :ruby) do
   commands curl: 'curl'
   defaultfor feature: :posix
 
   def curl_params(params)
-    account = [resource[:username], resource[:password]].compact.join(':') if resource[:username]
-    params += optional_switch(account, ['--user', '%s'])
+    if resource[:username]
+      create_netrcfile
+      params += ['--netrc-file', @netrc_file.path]
+    end
     params += optional_switch(resource[:proxy_server], ['--proxy', '%s'])
     params += ['--insecure'] if resource[:allow_insecure]
     params += resource[:download_options] if resource[:download_options]
@@ -13,6 +18,20 @@ def curl_params(params)
     params
   end
 
+  def create_netrcfile
+    @netrc_file = Tempfile.new('.puppet_archive_curl')
+    machine = URI.parse(resource[:source]).host
+    @netrc_file.write("machine #{machine}\nlogin #{resource[:username]}\npassword #{resource[:password]}\n")
+    @netrc_file.close
+  end
+
+  def delete_netrcfile
+    return if @netrc_file.nil?
+
+    @netrc_file.unlink
+    @netrc_file = nil
+  end
+
   def download(filepath)
     params = curl_params(
       [
@@ -25,7 +44,11 @@ def download(filepath)
       ]
     )
 
-    curl(params)
+    begin
+      curl(params)
+    ensure
+      delete_netrcfile
+    end
   end
 
   def remote_checksum
@@ -38,6 +61,10 @@ def remote_checksum
       ]
     )
 
-    curl(params)[%r{\b[\da-f]{32,128}\b}i]
+    begin
+      curl(params)[%r{\b[\da-f]{32,128}\b}i]
+    ensure
+      delete_netrcfile
+    end
   end
 end

spec/unit/puppet/provider/archive/curl_spec.rb

@@ -9,6 +9,7 @@
     let(:name)      { '/tmp/example.zip' }
     let(:resource)  { Puppet::Type::Archive.new(resource_properties) }
     let(:provider)  { curl_provider.new(resource) }
+    let(:tempfile)  { Tempfile.new('mock') }
 
     let(:default_options) do
       [
@@ -24,6 +25,7 @@
     before do
       allow(FileUtils).to receive(:mv)
       allow(provider).to receive(:curl)
+      allow(Tempfile).to receive(:new).with('.puppet_archive_curl').and_return(tempfile)
     end
 
     context 'no extra properties specified' do
@@ -40,34 +42,33 @@
       end
     end
 
-    context 'username specified' do
+    context 'username and password specified' do
       let(:resource_properties) do
         {
           name: name,
           source: 'http://home.lan/example.zip',
-          username: 'foo'
+          username: 'foo',
+          password: 'bar'
         }
       end
 
-      it 'calls curl with default options and username' do
+      it 'populates temp netrc file with credentials' do
+        allow(provider).to receive(:delete_netrcfile) # Don't delete the file or we won't be able to examine its contents.
         provider.download(name)
-        expect(provider).to have_received(:curl).with(default_options << '--user' << 'foo')
+        nettc_content = File.open(tempfile.path).read
+        expect(nettc_content).to eq("machine home.lan\nlogin foo\npassword bar\n")
       end
-    end
 
-    context 'username and password specified' do
-      let(:resource_properties) do
-        {
-          name: name,
-          source: 'http://home.lan/example.zip',
-          username: 'foo',
-          password: 'bar'
-        }
+      it 'calls curl with default options and path to netrc file' do
+        netrc_filepath = tempfile.path
+        provider.download(name)
+        expect(provider).to have_received(:curl).with(default_options << '--netrc-file' << netrc_filepath)
       end
 
-      it 'calls curl with default options and password' do
+      it 'deletes netrc file' do
+        netrc_filepath = tempfile.path
         provider.download(name)
-        expect(provider).to have_received(:curl).with(default_options << '--user' << 'foo:bar')
+        expect(File.exist?(netrc_filepath)).to eq(false)
       end
     end