From 5a7e00c5b1deffa3329c43c9c22186cd6d49991e Mon Sep 17 00:00:00 2001
From: Ray Ruvinskiy <ray.ruvinskiy@arcticwolf.com>
Date: Sun, 7 Sep 2014 23:45:32 -0400
Subject: [PATCH] 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
---
 .../action/builtin/box_check_outdated.rb      |  12 +-
 lib/vagrant/box.rb                            |   9 +-
 plugins/commands/box/command/add.rb           |  23 +--
 .../commands/box/command/download_mixins.rb   |  29 +++
 plugins/commands/box/command/outdated.rb      |  15 +-
 plugins/commands/box/command/update.rb        |  35 +++-
 .../commands/box/command/update_test.rb       | 184 ++++++++++++++----
 .../action/builtin/box_check_outdated_test.rb |  37 +++-
 8 files changed, 275 insertions(+), 69 deletions(-)
 create mode 100644 plugins/commands/box/command/download_mixins.rb

diff --git a/lib/vagrant/action/builtin/box_check_outdated.rb b/lib/vagrant/action/builtin/box_check_outdated.rb
index cee8c6877..d39afd272 100644
--- a/lib/vagrant/action/builtin/box_check_outdated.rb
+++ b/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",
diff --git a/lib/vagrant/box.rb b/lib/vagrant/box.rb
index 537c94e2a..045b22fd3 100644
--- a/lib/vagrant/box.rb
+++ b/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
 
diff --git a/plugins/commands/box/command/add.rb b/plugins/commands/box/command/add.rb
index 0356ee298..e9d429433 100644
--- a/plugins/commands/box/command/add.rb
+++ b/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"),
           })
diff --git a/plugins/commands/box/command/download_mixins.rb b/plugins/commands/box/command/download_mixins.rb
new file mode 100644
index 000000000..eda31b6bf
--- /dev/null
+++ b/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
diff --git a/plugins/commands/box/command/outdated.rb b/plugins/commands/box/command/outdated.rb
index a686fa496..cde932e1a 100644
--- a/plugins/commands/box/command/outdated.rb
+++ b/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",
diff --git a/plugins/commands/box/command/update.rb b/plugins/commands/box/command/update.rb
index 5633b0aaf..67d07bcc0 100644
--- a/plugins/commands/box/command/update.rb
+++ b/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
diff --git a/test/unit/plugins/commands/box/command/update_test.rb b/test/unit/plugins/commands/box/command/update_test.rb
index 4477d0aa5..067d0dd35 100644
--- a/test/unit/plugins/commands/box/command/update_test.rb
+++ b/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
diff --git a/test/unit/vagrant/action/builtin/box_check_outdated_test.rb b/test/unit/vagrant/action/builtin/box_check_outdated_test.rb
index bcb313d1c..0bb175c59 100644
--- a/test/unit/vagrant/action/builtin/box_check_outdated_test.rb
+++ b/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