Add HTTPS download options to `box update` and `box outdated`

Vagrant::Box.load_metadata did not provide a way to specify the HTTPS
download options that could be specified when downloading boxes
(ca cert, ca path, client cert, insecure). As a result, while it was
possible to add a box whose metadata file needed to be downloaded with one of
those options specified, it was impossible to check for updates. The following
changes have been made to address the situation:

1. Create a DownloadMixins module to provide the --insecure, --cacert, --capth,
   and --cert command line options to all of `vagrant box add`,
   `vagrant box update`, and `vagrant box outdated`.
2. Extend `Vagrant::Box.has_update?` and `Vagrant::Box.load_metadata` to accept
   said download options.
3. Extend `box outdated` and `box update` commands to pass download options
   down.
4. Extend `Vagrant::Builtin::Action::BoxCheckOutdated` to honour download
   options.
5. Options specified on the command line take precedence over options specified
   in the machine configuration, if any.
6. Fix bug in `vagrant box add` where client cert was being passed down using
   the wrong environment key.
7. Unit test coverage in update_test and box_check_outdated_test.

Resolves #4420

lib/vagrant/action/builtin/box_check_outdated.rb

@@ -37,13 +37,23 @@ module Vagrant
           end
 
           constraints = machine.config.vm.box_version
+          # Have download options specified in the environment override
+          # options specified for the machine.
+          download_options = {
+            ca_cert: env[:ca_cert] || machine.config.vm.box_download_ca_cert,
+            ca_path: env[:ca_path] || machine.config.vm.box_download_ca_path,
+            client_cert: env[:client_cert] ||
+                           machine.config.vm.box_download_client_cert,
+            insecure: !env[:insecure].nil? ?
+                        env[:insecure] : machine.config.vm.box_download_insecure
+          }
 
           env[:ui].output(I18n.t(
             "vagrant.box_outdated_checking_with_refresh",
             name: box.name))
           update = nil
           begin
-            update = box.has_update?(constraints)
+            update = box.has_update?(constraints, download_options: download_options)
           rescue Errors::BoxMetadataDownloadError => e
             env[:ui].warn(I18n.t(
               "vagrant.box_outdated_metadata_download_error",

lib/vagrant/box.rb

@@ -115,8 +115,9 @@ module Vagrant
     # Loads the metadata URL and returns the latest metadata associated
     # with this box.
     #
+    # @param [Hash] download_options Options to pass to the downloader.
     # @return [BoxMetadata]
-    def load_metadata
+    def load_metadata(**download_options)
       tf = Tempfile.new("vagrant")
       tf.close
 
@@ -127,7 +128,7 @@ module Vagrant
         url = "file:#{url}"
       end
 
-      opts = { headers: ["Accept: application/json"] }
+      opts = { headers: ["Accept: application/json"] }.merge(download_options)
       Util::Downloader.new(url, tf.path, **opts).download!
       BoxMetadata.new(File.open(tf.path, "r"))
     rescue Errors::DownloaderError => e
@@ -148,7 +149,7 @@ module Vagrant
     #   satisfy. If nil, the version constrain defaults to being a
     #   larger version than this box.
     # @return [Array]
-    def has_update?(version=nil)
+    def has_update?(version=nil, download_options: {})
       if !@metadata_url
         raise Errors::BoxUpdateNoMetadata, name: @name
       end
@@ -156,7 +157,7 @@ module Vagrant
       version += ", " if version
       version ||= ""
       version += "> #{@version}"
-      md      = self.load_metadata
+      md      = self.load_metadata(download_options)
       newer   = md.version(version, provider: @provider)
       return nil if !newer
 

plugins/commands/box/command/add.rb

@@ -1,9 +1,13 @@
 require 'optparse'
 
+require_relative 'download_mixins'
+
 module VagrantPlugins
   module CommandBox
     module Command
       class Add < Vagrant.plugin("2", :command)
+        include DownloadMixins
+
         def execute
           options = {}
 
@@ -21,22 +25,7 @@ module VagrantPlugins
               options[:force] = f
             end
 
-            o.on("--insecure", "Do not validate SSL certificates") do |i|
-              options[:insecure] = i
-            end
-
-            o.on("--cacert FILE", String, "CA certificate for SSL download") do |c|
-              options[:ca_cert] = c
-            end
-
-            o.on("--capath DIR", String, "CA certificate directory for SSL download") do |c|
-              options[:ca_path] = c
-            end
-
-            o.on("--cert FILE", String,
-                 "A client SSL cert, if needed") do |c|
-              options[:client_cert] = c
-            end
+            build_download_options(o, options)
 
             o.on("--provider PROVIDER", String, "Provider the box should satisfy") do |p|
               options[:provider] = p
@@ -93,7 +82,7 @@ module VagrantPlugins
             box_force: options[:force],
             box_download_ca_cert: options[:ca_cert],
             box_download_ca_path: options[:ca_path],
-            box_download_client_cert: options[:client_cert],
+            box_client_cert: options[:client_cert],
             box_download_insecure: options[:insecure],
             ui: Vagrant::UI::Prefixed.new(@env.ui, "box"),
           })

plugins/commands/box/command/download_mixins.rb

@@ -0,0 +1,29 @@
+module VagrantPlugins
+  module CommandBox
+    module DownloadMixins
+      # This adds common download command line flags to the given
+      # OptionParser, storing the result in the `options` dictionary.
+      #
+      # @param [OptionParser] parser
+      # @param [Hash] options
+      def build_download_options(parser, options)
+        # Add the options
+        parser.on("--insecure", "Do not validate SSL certificates") do |i|
+          options[:insecure] = i
+        end
+
+        parser.on("--cacert FILE", String, "CA certificate for SSL download") do |c|
+          options[:ca_cert] = c
+        end
+
+        parser.on("--capath DIR", String, "CA certificate directory for SSL download") do |c|
+          options[:ca_path] = c
+        end
+
+        parser.on("--cert FILE", String, "A client SSL cert, if needed") do |c|
+          options[:client_cert] = c
+        end
+      end
+    end
+  end
+end

plugins/commands/box/command/outdated.rb

@@ -1,11 +1,16 @@
 require 'optparse'
 
+require_relative 'download_mixins'
+
 module VagrantPlugins
   module CommandBox
     module Command
       class Outdated < Vagrant.plugin("2", :command)
+        include DownloadMixins
+
         def execute
           options = {}
+          download_options = {}
 
           opts = OptionParser.new do |o|
             o.banner = "Usage: vagrant box outdated [options]"
@@ -20,6 +25,8 @@ module VagrantPlugins
             o.on("--global", "Check all boxes installed") do |g|
               options[:global] = g
             end
+
+            build_download_options(o, download_options)
           end
 
           argv = parse_options(opts)
@@ -27,7 +34,7 @@ module VagrantPlugins
 
           # If we're checking the boxes globally, then do that.
           if options[:global]
-            outdated_global
+            outdated_global(download_options)
             return 0
           end
 
@@ -37,11 +44,11 @@ module VagrantPlugins
               box_outdated_refresh: true,
               box_outdated_success_ui: true,
               machine: machine,
-            })
+            }.merge(download_options))
           end
         end
 
-        def outdated_global
+        def outdated_global(download_options)
           boxes = {}
           @env.boxes.all.reverse.each do |name, version, provider|
             next if boxes[name]
@@ -58,7 +65,7 @@ module VagrantPlugins
 
             md = nil
             begin
-              md = box.load_metadata
+              md = box.load_metadata(download_options)
             rescue Vagrant::Errors::DownloaderError => e
               @env.ui.error(I18n.t(
                 "vagrant.box_outdated_metadata_error",

plugins/commands/box/command/update.rb

@@ -1,11 +1,16 @@
 require 'optparse'
 
+require_relative 'download_mixins'
+
 module VagrantPlugins
   module CommandBox
     module Command
       class Update < Vagrant.plugin("2", :command)
+        include DownloadMixins
+
         def execute
           options = {}
+          download_options = {}
 
           opts = OptionParser.new do |o|
             o.banner = "Usage: vagrant box update [options]"
@@ -27,21 +32,23 @@ module VagrantPlugins
             o.on("--provider PROVIDER", String, "Update box with specific provider") do |p|
               options[:provider] = p.to_sym
             end
+
+            build_download_options(o, download_options)
           end
 
           argv = parse_options(opts)
           return if !argv
 
           if options[:box]
-            update_specific(options[:box], options[:provider])
+            update_specific(options[:box], options[:provider], download_options)
           else
-            update_vms(argv, options[:provider])
+            update_vms(argv, options[:provider], download_options)
           end
 
           0
         end
 
-        def update_specific(name, provider)
+        def update_specific(name, provider, download_options)
           boxes = {}
           @env.boxes.all.each do |n, v, p|
             boxes[n] ||= {}
@@ -74,11 +81,11 @@ module VagrantPlugins
 
           to_update.each do |n, p, v|
             box = @env.boxes.find(n, p, v)
-            box_update(box, "> #{v}", @env.ui)
+            box_update(box, "> #{v}", @env.ui, download_options)
           end
         end
 
-        def update_vms(argv, provider)
+        def update_vms(argv, provider, download_options)
           with_target_vms(argv, provider: provider) do |machine|
             if !machine.config.vm.box
               machine.ui.output(I18n.t(
@@ -95,17 +102,25 @@ module VagrantPlugins
 
             box = machine.box
             version = machine.config.vm.box_version
-            box_update(box, version, machine.ui)
+            # Get download options from machine configuration if not specified
+            # on the command line.
+            download_options[:ca_cert] ||= machine.config.vm.box_download_ca_cert
+            download_options[:ca_path] ||= machine.config.vm.box_download_ca_path
+            download_options[:client_cert] ||= machine.config.vm.box_download_client_cert
+            if download_options[:insecure].nil?
+              download_options[:insecure] = machine.config.vm.box_download_insecure
+            end
+            box_update(box, version, machine.ui, download_options)
           end
         end
 
-        def box_update(box, version, ui)
+        def box_update(box, version, ui, download_options)
           ui.output(I18n.t("vagrant.box_update_checking", name: box.name))
           ui.detail("Latest installed version: #{box.version}")
           ui.detail("Version constraints: #{version}")
           ui.detail("Provider: #{box.provider}")
 
-          update = box.has_update?(version)
+          update = box.has_update?(version, download_options: download_options)
           if !update
             ui.success(I18n.t(
               "vagrant.box_up_to_date_single",
@@ -124,6 +139,10 @@ module VagrantPlugins
             box_provider: update[2].name,
             box_version: update[1].version,
             ui: ui,
+            box_client_cert: download_options[:client_cert],
+            box_download_ca_cert: download_options[:ca_cert],
+            box_download_ca_path: download_options[:ca_path],
+            box_download_insecure: download_options[:insecure]
           })
         end
       end

test/unit/plugins/commands/box/command/update_test.rb

@@ -19,6 +19,11 @@ describe VagrantPlugins::CommandBox::Command::Update do
   let(:action_runner) { double("action_runner") }
   let(:machine) { iso_env.machine(iso_env.machine_names[0], :dummy) }
 
+  let(:download_options) { ["--insecure",
+                            "--cacert", "foo",
+                            "--capath", "bar",
+                            "--cert", "baz"] }
+
   subject { described_class.new(argv, iso_env) }
 
   before do
@@ -86,6 +91,10 @@ describe VagrantPlugins::CommandBox::Command::Update do
             expect(opts[:box_url]).to eq(metadata_url.to_s)
             expect(opts[:box_provider]).to eq("virtualbox")
             expect(opts[:box_version]).to eq("1.1")
+            expect(opts[:box_download_ca_path]).to be_nil
+            expect(opts[:box_download_ca_cert]).to be_nil
+            expect(opts[:box_client_cert]).to be_nil
+            expect(opts[:box_download_insecure]).to be_nil
           end
 
           opts
@@ -158,6 +167,50 @@ describe VagrantPlugins::CommandBox::Command::Update do
         end
       end
 
+      context "download options are specified" do
+        let(:argv) { ["--box", "foo" ].concat(download_options) }
+
+        it "passes down download options" do
+          metadata_url.open("w") do |f|
+            f.write(<<-RAW)
+        {
+          "name": "foo",
+          "versions": [
+            {
+              "version": "1.0"
+            },
+            {
+              "version": "1.1",
+              "providers": [
+                {
+                  "name": "virtualbox",
+                  "url": "bar"
+                }
+              ]
+            }
+          ]
+        }
+            RAW
+          end
+
+          action_called = false
+          allow(action_runner).to receive(:run) do |action, opts|
+            if opts[:box_provider]
+              action_called = true
+              expect(opts[:box_download_ca_cert]).to eq("foo")
+              expect(opts[:box_download_ca_path]).to eq("bar")
+              expect(opts[:box_client_cert]).to eq("baz")
+              expect(opts[:box_download_insecure]).to be_true
+            end
+
+            opts
+          end
+
+          subject.execute
+          expect(action_called).to be_true
+        end
+      end
+
       context "with a box that doesn't exist" do
         let(:argv) { ["--box", "nope"] }
 
@@ -192,7 +245,10 @@ describe VagrantPlugins::CommandBox::Command::Update do
       it "doesn't update boxes if they're up-to-date" do
         machine.stub(box: box)
         expect(box).to receive(:has_update?).
-          with(machine.config.vm.box_version).
+          with(machine.config.vm.box_version,
+               {download_options:
+                 {ca_cert: nil, ca_path: nil, client_cert: nil,
+                  insecure: false}}).
           and_return(nil)
 
         expect(action_runner).to receive(:run).never
@@ -200,41 +256,101 @@ describe VagrantPlugins::CommandBox::Command::Update do
         subject.execute
       end
 
-      it "updates boxes if they have an update" do
-        md = Vagrant::BoxMetadata.new(StringIO.new(<<-RAW))
-      {
-        "name": "foo",
-        "versions": [
-          {
-            "version": "1.0"
-          },
-          {
-            "version": "1.1",
-            "providers": [
-              {
-                "name": "virtualbox",
-                "url": "bar"
-              }
-            ]
-          }
-        ]
-      }
-        RAW
-
-        machine.stub(box: box)
-        expect(box).to receive(:has_update?).
-          with(machine.config.vm.box_version).
-          and_return([md, md.version("1.1"), md.version("1.1").provider("virtualbox")])
-
-        expect(action_runner).to receive(:run).with { |action, opts|
-          expect(opts[:box_url]).to eq(box.metadata_url)
-          expect(opts[:box_provider]).to eq("virtualbox")
-          expect(opts[:box_version]).to eq("1.1")
-          expect(opts[:ui]).to equal(machine.ui)
-          true
+      context "boxes have an update" do
+        let(:md) {
+          md = Vagrant::BoxMetadata.new(StringIO.new(<<-RAW))
+        {
+          "name": "foo",
+          "versions": [
+            {
+              "version": "1.0"
+            },
+            {
+              "version": "1.1",
+              "providers": [
+                {
+                  "name": "virtualbox",
+                  "url": "bar"
+                }
+              ]
+            }
+          ]
+        }
+          RAW
         }
 
-        subject.execute
+        before { machine.stub(box: box) }
+
+        it "updates boxes" do
+          expect(box).to receive(:has_update?).
+            with(machine.config.vm.box_version,
+                 {download_options:
+                   {ca_cert: nil, ca_path: nil, client_cert: nil,
+                    insecure: false}}).
+            and_return([md, md.version("1.1"), md.version("1.1").provider("virtualbox")])
+
+          expect(action_runner).to receive(:run).with { |action, opts|
+            expect(opts[:box_url]).to eq(box.metadata_url)
+            expect(opts[:box_provider]).to eq("virtualbox")
+            expect(opts[:box_version]).to eq("1.1")
+            expect(opts[:ui]).to equal(machine.ui)
+            true
+          }
+
+          subject.execute
+        end
+
+        context "machine has download options" do
+          before do
+            machine.config.vm.box_download_ca_cert = "oof"
+            machine.config.vm.box_download_ca_path = "rab"
+            machine.config.vm.box_download_client_cert = "zab"
+            machine.config.vm.box_download_insecure = false
+          end
+
+          it "uses download options from machine" do
+            expect(box).to receive(:has_update?).
+              with(machine.config.vm.box_version,
+                   {download_options:
+                     {ca_cert: "oof", ca_path: "rab", client_cert: "zab",
+                      insecure: false}}).
+              and_return([md, md.version("1.1"), md.version("1.1").provider("virtualbox")])
+
+            expect(action_runner).to receive(:run).with { |action, opts|
+              expect(opts[:box_download_ca_cert]).to eq("oof")
+              expect(opts[:box_download_ca_path]).to eq("rab")
+              expect(opts[:box_client_cert]).to eq("zab")
+              expect(opts[:box_download_insecure]).to be_false
+              true
+            }
+
+            subject.execute
+          end
+
+          context "download options are specified on the command line" do
+            let(:argv) { download_options }
+
+            it "overrides download options from machine with options from CLI" do
+              expect(box).to receive(:has_update?).
+                with(machine.config.vm.box_version,
+                     {download_options:
+                       {ca_cert: "foo", ca_path: "bar", client_cert: "baz",
+                        insecure: true}}).
+                and_return([md, md.version("1.1"),
+                            md.version("1.1").provider("virtualbox")])
+
+              expect(action_runner).to receive(:run).with { |action, opts|
+                expect(opts[:box_download_ca_cert]).to eq("foo")
+                expect(opts[:box_download_ca_path]).to eq("bar")
+                expect(opts[:box_client_cert]).to eq("baz")
+                expect(opts[:box_download_insecure]).to be_true
+                true
+              }
+
+              subject.execute
+            end
+          end
+        end
       end
     end
   end

test/unit/vagrant/action/builtin/box_check_outdated_test.rb

@@ -117,7 +117,9 @@ describe Vagrant::Action::Builtin::BoxCheckOutdated do
       }
       RAW
 
-      expect(box).to receive(:has_update?).with(machine.config.vm.box_version).
+      expect(box).to receive(:has_update?).with(machine.config.vm.box_version,
+          {download_options:
+            {ca_cert: nil, ca_path: nil, client_cert: nil, insecure: false}}).
         and_return([md, md.version("1.1"), md.version("1.1").provider("virtualbox")])
 
       expect(app).to receive(:call).with(env).once
@@ -180,5 +182,38 @@ describe Vagrant::Action::Builtin::BoxCheckOutdated do
 
       expect { subject.call(env) }.to_not raise_error
     end
+
+    context "when machine download options are specified" do
+      before do
+        machine.config.vm.box_download_ca_cert = "foo"
+        machine.config.vm.box_download_ca_path = "bar"
+        machine.config.vm.box_download_client_cert = "baz"
+        machine.config.vm.box_download_insecure = true
+      end
+
+      it "uses download options from machine" do
+        expect(box).to receive(:has_update?).with(machine.config.vm.box_version,
+          {download_options:
+            {ca_cert: "foo", ca_path: "bar", client_cert: "baz", insecure: true}})
+
+        expect(app).to receive(:call).with(env).once
+
+        subject.call(env)
+      end
+
+      it "overrides download options from machine with options from env" do
+        expect(box).to receive(:has_update?).with(machine.config.vm.box_version,
+          {download_options:
+            {ca_cert: "oof", ca_path: "rab", client_cert: "zab", insecure: false}})
+
+        env[:ca_cert] = "oof"
+        env[:ca_path] = "rab"
+        env[:client_cert] = "zab"
+        env[:insecure] = false
+        expect(app).to receive(:call).with(env).once
+
+        subject.call(env)
+      end
+    end
   end
 end