From b7423d25ec957e1fb9bad5ed03bb6d9b50d16516 Mon Sep 17 00:00:00 2001 From: Alexander Fisher Date: Fri, 22 Jan 2021 15:24:21 +0000 Subject: [PATCH] Fix downloading when passwords contain spaces This also adds our first acceptance tests to this module. --- .github/workflows/ci.yml | 28 +++++++++++ .sync.yml | 4 +- lib/puppet/provider/archive/curl.rb | 10 +++- lib/puppet/provider/archive/wget.rb | 4 +- spec/acceptance/authentication_spec.rb | 46 +++++++++++++++++++ spec/spec_helper_acceptance.rb | 6 +++ .../unit/puppet/provider/archive/curl_spec.rb | 16 +++++++ 7 files changed, 108 insertions(+), 6 deletions(-) create mode 100644 spec/acceptance/authentication_spec.rb create mode 100644 spec/spec_helper_acceptance.rb diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ea492ae0..b4f47e87 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,3 +45,31 @@ jobs: bundler-cache: true - name: Run tests run: bundle exec rake + + acceptance: + needs: setup_matrix + runs-on: ubuntu-latest + env: + BUNDLE_WITHOUT: development:test:release + strategy: + fail-fast: false + matrix: + setfile: ${{fromJson(needs.setup_matrix.outputs.beaker_setfiles)}} + puppet: ${{fromJson(needs.setup_matrix.outputs.puppet_major_versions)}} + name: ${{ matrix.puppet.name }} - ${{ matrix.setfile.name }} + steps: + - name: Enable IPv6 on docker + run: | + echo '{"ipv6":true,"fixed-cidr-v6":"2001:db8:1::/64"}' | sudo tee /etc/docker/daemon.json + sudo service docker restart + - uses: actions/checkout@v2 + - name: Setup ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: '2.7' + bundler-cache: true + - name: Run tests + run: bundle exec rake beaker + env: + BEAKER_PUPPET_COLLECTION: ${{ matrix.puppet.collection }} + BEAKER_setfile: ${{ matrix.setfile.value }} diff --git a/.sync.yml b/.sync.yml index ff7cdcb5..3f0028e0 100644 --- a/.sync.yml +++ b/.sync.yml @@ -1,5 +1,5 @@ --- -.travis.yml: - secure: "S8pxtUQSCxezeHuMTOa2nmbOC1Ln37evcDBd6rcAPse4DhrwzdJ7Ijme2tE+cPcdyYgO66jL0wWUBNia+10vOoYs1mCMj3mUq2bgySRhML0CQxGIvKYu8R6PbijljSK7LYplD1THQRgFLyHQ+f4Mh7+8yFpjXSmxuAQMdCPfPv8=" spec/spec_helper.rb: unmanaged: true +spec/spec_helper_acceptance.rb: + unmanaged: false diff --git a/lib/puppet/provider/archive/curl.rb b/lib/puppet/provider/archive/curl.rb index 84d9d93c..87d207fc 100644 --- a/lib/puppet/provider/archive/curl.rb +++ b/lib/puppet/provider/archive/curl.rb @@ -7,8 +7,14 @@ def curl_params(params) if resource[:username] - create_netrcfile - params += ['--netrc-file', @netrc_file.path] + if resource[:username] =~ %r{\s} || resource[:password] =~ %r{\s} + Puppet.warning('Username or password contains a space. Unable to use netrc file to hide credentials') + account = [resource[:username], resource[:password]].compact.join(':') + params += optional_switch(account, ['--user', '%s']) + else + create_netrcfile + params += ['--netrc-file', @netrc_file.path] + end end params += optional_switch(resource[:proxy_server], ['--proxy', '%s']) params += ['--insecure'] if resource[:allow_insecure] diff --git a/lib/puppet/provider/archive/wget.rb b/lib/puppet/provider/archive/wget.rb index fc8e0b52..d3734dd9 100644 --- a/lib/puppet/provider/archive/wget.rb +++ b/lib/puppet/provider/archive/wget.rb @@ -2,8 +2,8 @@ commands wget: 'wget' def wget_params(params) - params += optional_switch(resource[:username], ['--user=%s']) - params += optional_switch(resource[:password], ['--password=%s']) + params += optional_switch(Shellwords.shellescape(resource[:username]), ['--user=%s']) + params += optional_switch(Shellwords.shellescape(resource[:password]), ['--password=%s']) params += optional_switch(resource[:cookie], ['--header="Cookie: %s"']) params += optional_switch(resource[:proxy_server], ['-e use_proxy=yes', "-e #{resource[:proxy_type]}_proxy=#{resource[:proxy_server]}"]) params += ['--no-check-certificate'] if resource[:allow_insecure] diff --git a/spec/acceptance/authentication_spec.rb b/spec/acceptance/authentication_spec.rb new file mode 100644 index 00000000..cf7754fb --- /dev/null +++ b/spec/acceptance/authentication_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper_acceptance' +require 'uri' + +context 'authenticated download' do + let(:source) do + URI.escape("http://httpbin.org/basic-auth/user/#{password}") + end + let(:pp) do + <<-EOS + archive { '/tmp/testfile': + source => '#{source.gsub("'") { "\\'" }}', + username => 'user', + password => '#{password.gsub("'") { "\\'" }}', + provider => #{provider}, + } + EOS + end + + %w[curl wget ruby].each do |provider| + context "with provider #{provider}" do + let(:provider) { provider } + + [ + 'hunter2', + 'pass word with spaces', + 'y^%88_', + "passwordwithsinglequote'!", + ].each do |password| + context "with password '#{password}'" do + let(:password) { password } + + it 'applies idempotently with no errors' do + shell('/bin/rm -f /tmp/testfile') + apply_manifest(pp, catch_failures: true) + apply_manifest(pp, catch_changes: true) + end + + describe file('/tmp/testfile') do + it { is_expected.to be_file } + its(:content_as_json) { is_expected.to include('authenticated' => true, 'user' => 'user') } + end + end + end + end + end +end diff --git a/spec/spec_helper_acceptance.rb b/spec/spec_helper_acceptance.rb new file mode 100644 index 00000000..bec34fdd --- /dev/null +++ b/spec/spec_helper_acceptance.rb @@ -0,0 +1,6 @@ +# This file is completely managed via modulesync +require 'voxpupuli/acceptance/spec_helper_acceptance' + +configure_beaker + +Dir['./spec/support/acceptance/**/*.rb'].sort.each { |f| require f } diff --git a/spec/unit/puppet/provider/archive/curl_spec.rb b/spec/unit/puppet/provider/archive/curl_spec.rb index ad74c083..85704f2a 100644 --- a/spec/unit/puppet/provider/archive/curl_spec.rb +++ b/spec/unit/puppet/provider/archive/curl_spec.rb @@ -70,6 +70,22 @@ provider.download(name) expect(File.exist?(netrc_filepath)).to eq(false) end + + context 'with password containing space' do + let(:resource_properties) do + { + name: name, + source: 'http://home.lan/example.zip', + username: 'foo', + password: 'b ar' + } + end + + it 'calls curl with default options and username and password on command line' do + provider.download(name) + expect(provider).to have_received(:curl).with(default_options << '--user' << 'foo:b ar') + end + end end context 'allow_insecure true' do