From b5f6df9ec04b7a42e4a93961a38b667adbdcf06d Mon Sep 17 00:00:00 2001 From: sophia Date: Mon, 17 Aug 2020 18:01:35 -0500 Subject: [PATCH 1/5] Download a box by setting auth headers --- lib/vagrant/action/builtin/box_add.rb | 47 ++++--------------- lib/vagrant/box.rb | 9 +++- lib/vagrant/box_collection.rb | 8 +--- lib/vagrant/util/downloader.rb | 1 + .../auth/middleware/add_authentication.rb | 46 +++++++----------- 5 files changed, 34 insertions(+), 77 deletions(-) diff --git a/lib/vagrant/action/builtin/box_add.rb b/lib/vagrant/action/builtin/box_add.rb index a635d77ce..53cb05ee8 100644 --- a/lib/vagrant/action/builtin/box_add.rb +++ b/lib/vagrant/action/builtin/box_add.rb @@ -76,18 +76,8 @@ module Vagrant end end - # Call the hook to transform URLs into authenticated URLs. - # In the case we don't have a plugin that does this, then it - # will just return the same URLs. - hook_env = env[:hook].call( - :authenticate_box_url, box_urls: url.dup) - authed_urls = hook_env[:box_urls] - if !authed_urls || authed_urls.length != url.length - raise "Bad box authentication hook, did not generate proper results." - end - # Test if any of our URLs point to metadata - is_metadata_results = authed_urls.map do |u| + is_metadata_results = url.map do |u| begin metadata_url?(u, env) rescue Errors::DownloaderError => e @@ -115,8 +105,7 @@ module Vagrant end if is_metadata - url = [url.first, authed_urls.first] - add_from_metadata(url, env, expanded) + add_from_metadata(url.first, env, expanded) else add_direct(url, env) end @@ -158,10 +147,7 @@ module Vagrant # Adds a box given that the URL is a metadata document. # - # @param [String | Array] url The URL of the metadata for - # the box to add. If this is an array, then it must be a two-element - # array where the first element is the original URL and the second - # element is an authenticated URL. + # @param [String] url The URL of the metadata for the box to add. # @param [Hash] env # @param [Bool] expanded True if the metadata URL was expanded with # a Atlas server URL. @@ -170,15 +156,6 @@ module Vagrant provider = env[:box_provider] provider = Array(provider) if provider version = env[:box_version] - - authenticated_url = url - if url.is_a?(Array) - # We have both a normal URL and "authenticated" URL. Split - # them up. - authenticated_url = url[1] - url = url[0] - end - display_original_url = Util::CredentialScrubber.scrub(Array(original_url).first) display_url = Util::CredentialScrubber.scrub(url) @@ -193,7 +170,7 @@ module Vagrant metadata = nil begin metadata_path = download( - authenticated_url, env, json: true, ui: false) + url, env, json: true, ui: false) return if @download_interrupted File.open(metadata_path) do |f| @@ -271,16 +248,6 @@ module Vagrant end provider_url = metadata_provider.url - if provider_url != authenticated_url - # Authenticate the provider URL since we're using auth - hook_env = env[:hook].call(:authenticate_box_url, box_urls: [provider_url]) - authed_urls = hook_env[:box_urls] - if !authed_urls || authed_urls.length != 1 - raise "Bad box authentication hook, did not generate proper results." - end - provider_url = authed_urls[0] - end - box_add( [[provider_url, metadata_provider.url]], metadata.name, @@ -436,13 +403,16 @@ module Vagrant downloader_options[:location_trusted] = env[:box_download_location_trusted] downloader_options[:box_extra_download_options] = env[:box_extra_download_options] - Util::Downloader.new(url, temp_path, downloader_options) + d = Util::Downloader.new(url, temp_path, downloader_options) + env[:hook].call(:authenticate_box_url, downloader: d) + d end def download(url, env, **opts) opts[:ui] = true if !opts.key?(:ui) d = downloader(url, env, **opts) + env[:hook].call(:authenticate_box_url, downloader: d) # Download the box to a temporary path. We store the temporary # path as an instance variable so that the `#recover` method can @@ -486,6 +456,7 @@ module Vagrant # @return [Boolean] true if metadata def metadata_url?(url, env) d = downloader(url, env, json: true, ui: false) + env[:hook].call(:authenticate_box_url, downloader: d) # If we're downloading a file, cURL just returns no # content-type (makes sense), so we just test if it is JSON diff --git a/lib/vagrant/box.rb b/lib/vagrant/box.rb index 0ee5d29f3..0faf570e9 100644 --- a/lib/vagrant/box.rb +++ b/lib/vagrant/box.rb @@ -58,12 +58,13 @@ module Vagrant # @param [Pathname] directory The directory where this box exists on # disk. # @param [String] metadata_url Metadata URL for box - def initialize(name, provider, version, directory, metadata_url: nil) + def initialize(name, provider, version, directory, metadata_url: nil, hook: nil) @name = name @version = version @provider = provider @directory = directory @metadata_url = metadata_url + @hook = hook metadata_file = directory.join("metadata.json") raise Errors::BoxMetadataFileNotFound, name: @name if !metadata_file.file? @@ -133,7 +134,11 @@ module Vagrant end opts = { headers: ["Accept: application/json"] }.merge(download_options) - Util::Downloader.new(url, tf.path, opts).download! + d = Util::Downloader.new(url, tf.path, opts) + if @hook + @hook.call(:authenticate_box_url, downloader: d) + end + d.download! BoxMetadata.new(File.open(tf.path, "r")) rescue Errors::DownloaderError => e raise Errors::BoxMetadataDownloadError, diff --git a/lib/vagrant/box_collection.rb b/lib/vagrant/box_collection.rb index 91423c39b..7bf9f6f5b 100644 --- a/lib/vagrant/box_collection.rb +++ b/lib/vagrant/box_collection.rb @@ -317,15 +317,9 @@ module Vagrant metadata_url_file = box_directory.join("metadata_url") metadata_url = metadata_url_file.read if metadata_url_file.file? - if metadata_url && @hook - hook_env = @hook.call( - :authenticate_box_url, box_urls: [metadata_url]) - metadata_url = hook_env[:box_urls].first - end - return Box.new( name, provider, version_dir_map[v.to_s], provider_dir, - metadata_url: metadata_url, + metadata_url: metadata_url, hook: @hook ) end end diff --git a/lib/vagrant/util/downloader.rb b/lib/vagrant/util/downloader.rb index bf966ecaf..b79a8734e 100644 --- a/lib/vagrant/util/downloader.rb +++ b/lib/vagrant/util/downloader.rb @@ -31,6 +31,7 @@ module Vagrant attr_reader :source attr_reader :destination + attr_accessor :headers def initialize(source, destination, options=nil) options ||= {} diff --git a/plugins/commands/cloud/auth/middleware/add_authentication.rb b/plugins/commands/cloud/auth/middleware/add_authentication.rb index 9d826617e..ff9b8ad84 100644 --- a/plugins/commands/cloud/auth/middleware/add_authentication.rb +++ b/plugins/commands/cloud/auth/middleware/add_authentication.rb @@ -33,47 +33,33 @@ module VagrantPlugins def call(env) client = Client.new(env[:env]) token = client.token + target_url = URI.parse(env[:downloader].source) - env[:box_urls].map! do |url| + if target_url.host != TARGET_HOST && REPLACEMENT_HOSTS.include?(target_url.host) begin - u = URI.parse(url) - if u.host != TARGET_HOST && REPLACEMENT_HOSTS.include?(u.host) - u.host = TARGET_HOST - u.to_s - else - url - end + target_url.host = TARGET_HOST + target_url = target_url.to_s rescue URI::Error - url + # if there is an error, use current target_url end end server_uri = URI.parse(Vagrant.server_url.to_s) - if token && !server_uri.host.to_s.empty? - env[:box_urls].map! do |url| - u = URI.parse(url) - - if u.host == server_uri.host - if server_uri.host != TARGET_HOST && !self.class.custom_host_notified? - env[:ui].warn(I18n.t("cloud_command.middleware.authentication.different_target", - custom_host: server_uri.host, known_host: TARGET_HOST) + "\n") - sleep CUSTOM_HOST_NOTIFY_WAIT - self.class.custom_host_notified! - end - - q = CGI.parse(u.query || "") - - current = q["access_token"] - if current && current.empty? - q["access_token"] = token - end - - u.query = URI.encode_www_form(q) + if target_url.host == server_uri.host + if server_uri.host != TARGET_HOST && !self.class.custom_host_notified? + env[:ui].warn(I18n.t("cloud_command.middleware.authentication.different_target", + custom_host: server_uri.host, known_host: TARGET_HOST) + "\n") + sleep CUSTOM_HOST_NOTIFY_WAIT + self.class.custom_host_notified! end - u.to_s + if env[:downloader].headers && !env[:downloader].headers.any? { |h| h.include?("Authorization") } + env[:downloader].headers << "Authorization: Bearer #{token}" + end end + + env[:downloader] end @app.call(env) From f440012b3017bd59a135f65f80eb81fb55822035 Mon Sep 17 00:00:00 2001 From: sophia Date: Tue, 18 Aug 2020 10:51:42 -0500 Subject: [PATCH 2/5] Scrub token from logs --- plugins/commands/cloud/auth/middleware/add_authentication.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugins/commands/cloud/auth/middleware/add_authentication.rb b/plugins/commands/cloud/auth/middleware/add_authentication.rb index ff9b8ad84..09e6ac116 100644 --- a/plugins/commands/cloud/auth/middleware/add_authentication.rb +++ b/plugins/commands/cloud/auth/middleware/add_authentication.rb @@ -1,6 +1,8 @@ require "cgi" require "uri" +require "vagrant/util/credential_scrubber" + require Vagrant.source_root.join("plugins/commands/cloud/client/client") module VagrantPlugins @@ -34,6 +36,7 @@ module VagrantPlugins client = Client.new(env[:env]) token = client.token target_url = URI.parse(env[:downloader].source) + Vagrant::Util::CredentialScrubber.sensitive(token) if target_url.host != TARGET_HOST && REPLACEMENT_HOSTS.include?(target_url.host) begin From 0d1635303f021134060ebf2af468272fa540fbdf Mon Sep 17 00:00:00 2001 From: sophia Date: Tue, 18 Aug 2020 17:38:49 -0500 Subject: [PATCH 3/5] Sperate hook for authenticating url and adding headers for authentication --- lib/vagrant/action/builtin/box_add.rb | 47 ++++++++++++++--- lib/vagrant/box.rb | 2 +- lib/vagrant/box_collection.rb | 6 +++ .../auth/middleware/add_authentication.rb | 49 +++++++++++------- .../add_downloader_authentication.rb | 50 +++++++++++++++++++ plugins/commands/cloud/plugin.rb | 5 ++ 6 files changed, 132 insertions(+), 27 deletions(-) create mode 100644 plugins/commands/cloud/auth/middleware/add_downloader_authentication.rb diff --git a/lib/vagrant/action/builtin/box_add.rb b/lib/vagrant/action/builtin/box_add.rb index 53cb05ee8..1a53918a9 100644 --- a/lib/vagrant/action/builtin/box_add.rb +++ b/lib/vagrant/action/builtin/box_add.rb @@ -76,8 +76,18 @@ module Vagrant end end + # Call the hook to transform URLs into authenticated URLs. + # In the case we don't have a plugin that does this, then it + # will just return the same URLs. + hook_env = env[:hook].call( + :authenticate_box_url, box_urls: url.dup) + authed_urls = hook_env[:box_urls] + if !authed_urls || authed_urls.length != url.length + raise "Bad box authentication hook, did not generate proper results." + end + # Test if any of our URLs point to metadata - is_metadata_results = url.map do |u| + is_metadata_results = authed_urls.map do |u| begin metadata_url?(u, env) rescue Errors::DownloaderError => e @@ -105,7 +115,8 @@ module Vagrant end if is_metadata - add_from_metadata(url.first, env, expanded) + url = [url.first, authed_urls.first] + add_from_metadata(url, env, expanded) else add_direct(url, env) end @@ -147,7 +158,10 @@ module Vagrant # Adds a box given that the URL is a metadata document. # - # @param [String] url The URL of the metadata for the box to add. + # @param [String | Array] url The URL of the metadata for + # the box to add. If this is an array, then it must be a two-element + # array where the first element is the original URL and the second + # element is an authenticated URL. # @param [Hash] env # @param [Bool] expanded True if the metadata URL was expanded with # a Atlas server URL. @@ -156,6 +170,15 @@ module Vagrant provider = env[:box_provider] provider = Array(provider) if provider version = env[:box_version] + + authenticated_url = url + if url.is_a?(Array) + # We have both a normal URL and "authenticated" URL. Split + # them up. + authenticated_url = url[1] + url = url[0] + end + display_original_url = Util::CredentialScrubber.scrub(Array(original_url).first) display_url = Util::CredentialScrubber.scrub(url) @@ -170,7 +193,7 @@ module Vagrant metadata = nil begin metadata_path = download( - url, env, json: true, ui: false) + authenticated_url, env, json: true, ui: false) return if @download_interrupted File.open(metadata_path) do |f| @@ -248,6 +271,16 @@ module Vagrant end provider_url = metadata_provider.url + if provider_url != authenticated_url + # Authenticate the provider URL since we're using auth + hook_env = env[:hook].call(:authenticate_box_url, box_urls: [provider_url]) + authed_urls = hook_env[:box_urls] + if !authed_urls || authed_urls.length != 1 + raise "Bad box authentication hook, did not generate proper results." + end + provider_url = authed_urls[0] + end + box_add( [[provider_url, metadata_provider.url]], metadata.name, @@ -404,7 +437,7 @@ module Vagrant downloader_options[:box_extra_download_options] = env[:box_extra_download_options] d = Util::Downloader.new(url, temp_path, downloader_options) - env[:hook].call(:authenticate_box_url, downloader: d) + env[:hook].call(:authenticate_box_downloader, downloader: d) d end @@ -412,7 +445,7 @@ module Vagrant opts[:ui] = true if !opts.key?(:ui) d = downloader(url, env, **opts) - env[:hook].call(:authenticate_box_url, downloader: d) + env[:hook].call(:authenticate_box_downloader, downloader: d) # Download the box to a temporary path. We store the temporary # path as an instance variable so that the `#recover` method can @@ -456,7 +489,7 @@ module Vagrant # @return [Boolean] true if metadata def metadata_url?(url, env) d = downloader(url, env, json: true, ui: false) - env[:hook].call(:authenticate_box_url, downloader: d) + env[:hook].call(:authenticate_box_downloader, downloader: d) # If we're downloading a file, cURL just returns no # content-type (makes sense), so we just test if it is JSON diff --git a/lib/vagrant/box.rb b/lib/vagrant/box.rb index 0faf570e9..e00fac2d9 100644 --- a/lib/vagrant/box.rb +++ b/lib/vagrant/box.rb @@ -136,7 +136,7 @@ module Vagrant opts = { headers: ["Accept: application/json"] }.merge(download_options) d = Util::Downloader.new(url, tf.path, opts) if @hook - @hook.call(:authenticate_box_url, downloader: d) + @hook.call(:authenticate_box_downloader, downloader: d) end d.download! BoxMetadata.new(File.open(tf.path, "r")) diff --git a/lib/vagrant/box_collection.rb b/lib/vagrant/box_collection.rb index 7bf9f6f5b..9e0471f5b 100644 --- a/lib/vagrant/box_collection.rb +++ b/lib/vagrant/box_collection.rb @@ -317,6 +317,12 @@ module Vagrant metadata_url_file = box_directory.join("metadata_url") metadata_url = metadata_url_file.read if metadata_url_file.file? + if metadata_url && @hook + hook_env = @hook.call( + :authenticate_box_url, box_urls: [metadata_url]) + metadata_url = hook_env[:box_urls].first + end + return Box.new( name, provider, version_dir_map[v.to_s], provider_dir, metadata_url: metadata_url, hook: @hook diff --git a/plugins/commands/cloud/auth/middleware/add_authentication.rb b/plugins/commands/cloud/auth/middleware/add_authentication.rb index 09e6ac116..9d826617e 100644 --- a/plugins/commands/cloud/auth/middleware/add_authentication.rb +++ b/plugins/commands/cloud/auth/middleware/add_authentication.rb @@ -1,8 +1,6 @@ require "cgi" require "uri" -require "vagrant/util/credential_scrubber" - require Vagrant.source_root.join("plugins/commands/cloud/client/client") module VagrantPlugins @@ -35,34 +33,47 @@ module VagrantPlugins def call(env) client = Client.new(env[:env]) token = client.token - target_url = URI.parse(env[:downloader].source) - Vagrant::Util::CredentialScrubber.sensitive(token) - if target_url.host != TARGET_HOST && REPLACEMENT_HOSTS.include?(target_url.host) + env[:box_urls].map! do |url| begin - target_url.host = TARGET_HOST - target_url = target_url.to_s + u = URI.parse(url) + if u.host != TARGET_HOST && REPLACEMENT_HOSTS.include?(u.host) + u.host = TARGET_HOST + u.to_s + else + url + end rescue URI::Error - # if there is an error, use current target_url + url end end server_uri = URI.parse(Vagrant.server_url.to_s) + if token && !server_uri.host.to_s.empty? - if target_url.host == server_uri.host - if server_uri.host != TARGET_HOST && !self.class.custom_host_notified? - env[:ui].warn(I18n.t("cloud_command.middleware.authentication.different_target", - custom_host: server_uri.host, known_host: TARGET_HOST) + "\n") - sleep CUSTOM_HOST_NOTIFY_WAIT - self.class.custom_host_notified! + env[:box_urls].map! do |url| + u = URI.parse(url) + + if u.host == server_uri.host + if server_uri.host != TARGET_HOST && !self.class.custom_host_notified? + env[:ui].warn(I18n.t("cloud_command.middleware.authentication.different_target", + custom_host: server_uri.host, known_host: TARGET_HOST) + "\n") + sleep CUSTOM_HOST_NOTIFY_WAIT + self.class.custom_host_notified! + end + + q = CGI.parse(u.query || "") + + current = q["access_token"] + if current && current.empty? + q["access_token"] = token + end + + u.query = URI.encode_www_form(q) end - if env[:downloader].headers && !env[:downloader].headers.any? { |h| h.include?("Authorization") } - env[:downloader].headers << "Authorization: Bearer #{token}" - end + u.to_s end - - env[:downloader] end @app.call(env) diff --git a/plugins/commands/cloud/auth/middleware/add_downloader_authentication.rb b/plugins/commands/cloud/auth/middleware/add_downloader_authentication.rb new file mode 100644 index 000000000..b99a01916 --- /dev/null +++ b/plugins/commands/cloud/auth/middleware/add_downloader_authentication.rb @@ -0,0 +1,50 @@ +require "cgi" +require "uri" + +require "vagrant/util/credential_scrubber" +require_relative "./add_authentication" + +require Vagrant.source_root.join("plugins/commands/cloud/client/client") + +module VagrantPlugins + module CloudCommand + class AddDownloaderAuthentication < AddAuthentication + + def call(env) + client = Client.new(env[:env]) + token = client.token + target_url = URI.parse(env[:downloader].source) + Vagrant::Util::CredentialScrubber.sensitive(token) + + if target_url.host != TARGET_HOST && REPLACEMENT_HOSTS.include?(target_url.host) + begin + target_url.host = TARGET_HOST + target_url = target_url.to_s + rescue URI::Error + # if there is an error, use current target_url + end + end + + server_uri = URI.parse(Vagrant.server_url.to_s) + if token && !server_uri.host.to_s.empty? + if target_url.host == server_uri.host + if server_uri.host != TARGET_HOST && !self.class.custom_host_notified? + env[:ui].warn(I18n.t("cloud_command.middleware.authentication.different_target", + custom_host: server_uri.host, known_host: TARGET_HOST) + "\n") + sleep CUSTOM_HOST_NOTIFY_WAIT + self.class.custom_host_notified! + end + + if env[:downloader].headers && !env[:downloader].headers.any? { |h| h.include?("Authorization") } + env[:downloader].headers << "Authorization: Bearer #{token}" + end + end + + env[:downloader] + end + + @app.call(env) + end.freeze + end + end +end diff --git a/plugins/commands/cloud/plugin.rb b/plugins/commands/cloud/plugin.rb index b5fc8cb75..cce50a01c 100644 --- a/plugins/commands/cloud/plugin.rb +++ b/plugins/commands/cloud/plugin.rb @@ -22,6 +22,11 @@ module VagrantPlugins hook.prepend(AddAuthentication) end + action_hook(:cloud_authenticated_boxes, :authenticate_box_downloader) do |hook| + require_relative "auth/middleware/add_downloader_authentication" + hook.prepend(AddDownloaderAuthentication) + end + protected def self.init! From fffc555faf1c2cde2f3289f97137653d533c9fd9 Mon Sep 17 00:00:00 2001 From: sophia Date: Thu, 20 Aug 2020 15:55:39 -0500 Subject: [PATCH 4/5] Add tests for new hook --- lib/vagrant/util/downloader.rb | 2 +- .../add_downloader_authentication.rb | 20 ++- .../add_downloader_authentication_test.rb | 155 ++++++++++++++++++ .../vagrant/action/builtin/box_add_test.rb | 6 +- test/unit/vagrant/box_test.rb | 18 ++ 5 files changed, 190 insertions(+), 11 deletions(-) create mode 100644 test/unit/plugins/commands/cloud/auth/middleware/add_downloader_authentication_test.rb diff --git a/lib/vagrant/util/downloader.rb b/lib/vagrant/util/downloader.rb index b79a8734e..a8a0968c4 100644 --- a/lib/vagrant/util/downloader.rb +++ b/lib/vagrant/util/downloader.rb @@ -29,7 +29,7 @@ module Vagrant "vagrantup.com".freeze ].freeze - attr_reader :source + attr_accessor :source attr_reader :destination attr_accessor :headers diff --git a/plugins/commands/cloud/auth/middleware/add_downloader_authentication.rb b/plugins/commands/cloud/auth/middleware/add_downloader_authentication.rb index b99a01916..2c7237c36 100644 --- a/plugins/commands/cloud/auth/middleware/add_downloader_authentication.rb +++ b/plugins/commands/cloud/auth/middleware/add_downloader_authentication.rb @@ -6,6 +6,9 @@ require_relative "./add_authentication" require Vagrant.source_root.join("plugins/commands/cloud/client/client") +# Similar to AddAuthentication this middleware will add authentication for interacting +# with Vagrant cloud. It does this by adding Authentication headers to a +# Vagrant::Util::Downloader object. module VagrantPlugins module CloudCommand class AddDownloaderAuthentication < AddAuthentication @@ -13,16 +16,16 @@ module VagrantPlugins def call(env) client = Client.new(env[:env]) token = client.token - target_url = URI.parse(env[:downloader].source) Vagrant::Util::CredentialScrubber.sensitive(token) - if target_url.host != TARGET_HOST && REPLACEMENT_HOSTS.include?(target_url.host) - begin - target_url.host = TARGET_HOST - target_url = target_url.to_s - rescue URI::Error - # if there is an error, use current target_url + begin + target_url = URI.parse(env[:downloader].source) + if target_url.host != TARGET_HOST && REPLACEMENT_HOSTS.include?(target_url.host) + target_url.host = TARGET_HOST + env[:downloader].source = target_url.to_s end + rescue URI::Error + # if there is an error, use current target_url end server_uri = URI.parse(Vagrant.server_url.to_s) @@ -35,7 +38,8 @@ module VagrantPlugins self.class.custom_host_notified! end - if env[:downloader].headers && !env[:downloader].headers.any? { |h| h.include?("Authorization") } + if !Array(env[:downloader].headers).any? { |h| h.include?("Authorization") } + env[:downloader].headers ||= [] env[:downloader].headers << "Authorization: Bearer #{token}" end end diff --git a/test/unit/plugins/commands/cloud/auth/middleware/add_downloader_authentication_test.rb b/test/unit/plugins/commands/cloud/auth/middleware/add_downloader_authentication_test.rb new file mode 100644 index 000000000..9def49ff1 --- /dev/null +++ b/test/unit/plugins/commands/cloud/auth/middleware/add_downloader_authentication_test.rb @@ -0,0 +1,155 @@ +require File.expand_path("../../../../../../base", __FILE__) + +require Vagrant.source_root.join("plugins/commands/cloud/auth/middleware/add_downloader_authentication") +require "vagrant/util/downloader" + +describe VagrantPlugins::CloudCommand::AddDownloaderAuthentication do + include_context "unit" + + let(:app) { lambda { |env| } } + let(:ui) { double("ui") } + let(:env) { { + env: iso_env, + ui: ui + } } + + let(:iso_env) { isolated_environment.create_vagrant_env } + let(:server_url) { "http://vagrantcloud.com/box.box" } + let(:dwnloader) { Vagrant::Util::Downloader.new(server_url, "/some/path", {}) } + + subject { described_class.new(app, env) } + + before do + allow(Vagrant).to receive(:server_url).and_return(server_url) + allow(ui).to receive(:warn) + stub_env("ATLAS_TOKEN" => nil) + end + + describe "#call" do + context "non full paths" do + let(:server_url) { "http://vagrantcloud.com" } + let(:dwnloader) { Vagrant::Util::Downloader.new(server_url, "/some/path", {}) } + + it "does nothing if we have no server set" do + allow(Vagrant).to receive(:server_url).and_return(nil) + VagrantPlugins::CloudCommand::Client.new(iso_env).store_token("fooboohoo") + + env[:downloader] = dwnloader + subject.call(env) + expect(env[:downloader].headers.nil?).to eq(true) + end + + it "does nothing if we aren't logged in" do + env[:downloader] = dwnloader + subject.call(env) + expect(env[:downloader].headers.nil?).to eq(true) + end + end + + context "custom server" do + let(:server_url) { "http://surprise.com/box.box" } + let(:dwnloader) { Vagrant::Util::Downloader.new(server_url, "/some/path", {}) } + + it "warns when adding token to custom server" do + server_url = "https://surprise.com" + allow(Vagrant).to receive(:server_url).and_return(server_url) + + token = "foobarbaz" + VagrantPlugins::CloudCommand::Client.new(iso_env).store_token(token) + + expect(subject).to receive(:sleep).once + expect(ui).to receive(:warn).once + + env[:downloader] = dwnloader + subject.call(env) + + expect(env[:downloader].headers).to eq(["Authorization: Bearer #{token}"]) + end + end + + context "replacement hosts" do + let(:dwnloader) { Vagrant::Util::Downloader.new("https://app.vagrantup.com", "/some/path", {}) } + + it "modifies host URL to target if authorized host" do + token = "foobarbaz" + VagrantPlugins::CloudCommand::Client.new(iso_env).store_token(token) + env[:downloader] = dwnloader + subject.call(env) + expect(env[:downloader].headers).to eq(["Authorization: Bearer #{token}"]) + expect(URI.parse(env[:downloader].source).host).to eq(VagrantPlugins::CloudCommand::AddDownloaderAuthentication::TARGET_HOST) + end + end + + context "malformed url" do + let(:bad_url) { "this is not a valid url" } + let(:dwnloader) { Vagrant::Util::Downloader.new(bad_url, "/some/path", {}) } + + it "ignores urls that it cannot parse" do + # Ensure the bad URL does cause an exception + expect{ URI.parse(bad_url) }.to raise_error URI::Error + env[:downloader] = dwnloader + subject.call(env) + expect(env[:downloader].source).to eq(bad_url) + end + end + + context "with an headers already added" do + let(:auth_header) { "Authorization Bearer: token" } + let(:other_header) {"some: thing"} + let(:dwnloader) { Vagrant::Util::Downloader.new(server_url, "/some/path", {headers: [other_header]}) } + + it "appends the auth header" do + token = "foobarbaz" + VagrantPlugins::CloudCommand::Client.new(iso_env).store_token(token) + + env[:downloader] = dwnloader + subject.call(env) + + expect(env[:downloader].headers).to eq([other_header, "Authorization: Bearer #{token}"]) + end + + context "with local file path" do + let(:file_path) { "file:////path/to/box.box" } + let(:dwnloader) { Vagrant::Util::Downloader.new(file_path, "/some/path", {}) } + + it "returns original urls when not modified" do + env[:downloader] = dwnloader + subject.call(env) + + expect(env[:downloader].source).to eq(file_path) + expect(env[:downloader].headers).to eq(nil) + end + end + + it "does not append multiple access_tokens" do + dwnloader.headers << auth_header + token = "foobarbaz" + VagrantPlugins::CloudCommand::Client.new(iso_env).store_token(token) + + env[:downloader] = dwnloader + subject.call(env) + + expect(env[:downloader].headers).to eq([other_header, auth_header]) + end + end + + it "adds a token to the headers" do + token = "foobarbaz" + VagrantPlugins::CloudCommand::Client.new(iso_env).store_token(token) + env[:downloader] = dwnloader + subject.call(env) + expect(env[:downloader].headers).to eq(["Authorization: Bearer #{token}"]) + end + + it "does not append the access token to vagrantcloud.com URLs if Atlas" do + server_url = "https://atlas.hashicorp.com" + allow(Vagrant).to receive(:server_url).and_return(server_url) + allow(subject).to receive(:sleep) + token = "foobarbaz" + VagrantPlugins::CloudCommand::Client.new(iso_env).store_token(token) + env[:downloader] = dwnloader + subject.call(env) + expect(env[:downloader].headers).to eq(nil) + end + end +end diff --git a/test/unit/vagrant/action/builtin/box_add_test.rb b/test/unit/vagrant/action/builtin/box_add_test.rb index 48406c2cf..e7bdb8b39 100644 --- a/test/unit/vagrant/action/builtin/box_add_test.rb +++ b/test/unit/vagrant/action/builtin/box_add_test.rb @@ -595,8 +595,10 @@ describe Vagrant::Action::Builtin::BoxAdd, :skip_windows, :bsdtar do env[:box_url] = "foo" env[:hook] = double("hook") - allow(env[:hook]).to receive(:call) do |name, opts| - expect(name).to eq(:authenticate_box_url) + + expect(env[:hook]).to receive(:call).with(:authenticate_box_downloader, any_args).at_least(:once) + + allow(env[:hook]).to receive(:call).with(:authenticate_box_url, any_args).at_least(:once) do |name, opts| if opts[:box_urls] == ["foo"] next { box_urls: [real_url] } elsif opts[:box_urls] == ["bar"] diff --git a/test/unit/vagrant/box_test.rb b/test/unit/vagrant/box_test.rb index fcb45780a..d045005cc 100644 --- a/test/unit/vagrant/box_test.rb +++ b/test/unit/vagrant/box_test.rb @@ -285,6 +285,24 @@ describe Vagrant::Box, :skip_windows do expect { subject.load_metadata }. to raise_error(Vagrant::Errors::BoxMetadataDownloadError) end + + context "box has a hook for adding authentication" do + + let(:hook){ double("hook") } + + subject do + described_class.new( + name, provider, version, directory, + metadata_url: metadata_url.path, hook: hook) + end + + it "add authentication headers to the url" do + expect(hook).to receive(:call).with(:authenticate_box_downloader, any_args) + result = subject.load_metadata + expect(result.name).to eq("foo") + expect(result.description).to eq("bar") + end + end end describe "destroying" do From d6a88f666ff99d5c152c6cab87782383e4e24a63 Mon Sep 17 00:00:00 2001 From: sophia Date: Tue, 15 Sep 2020 11:45:07 -0500 Subject: [PATCH 5/5] Add some docstrings and logging --- lib/vagrant/box.rb | 1 + lib/vagrant/util/downloader.rb | 2 +- .../auth/middleware/add_downloader_authentication.rb | 7 +++++-- .../auth/middleware/add_downloader_authentication_test.rb | 8 ++++---- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/vagrant/box.rb b/lib/vagrant/box.rb index e00fac2d9..41b63b292 100644 --- a/lib/vagrant/box.rb +++ b/lib/vagrant/box.rb @@ -58,6 +58,7 @@ module Vagrant # @param [Pathname] directory The directory where this box exists on # disk. # @param [String] metadata_url Metadata URL for box + # @param [Hook] hook A hook to apply to the box downloader, for example, for authentication def initialize(name, provider, version, directory, metadata_url: nil, hook: nil) @name = name @version = version diff --git a/lib/vagrant/util/downloader.rb b/lib/vagrant/util/downloader.rb index a8a0968c4..eee9744e7 100644 --- a/lib/vagrant/util/downloader.rb +++ b/lib/vagrant/util/downloader.rb @@ -59,7 +59,7 @@ module Vagrant @ca_cert = options[:ca_cert] @ca_path = options[:ca_path] @continue = options[:continue] - @headers = options[:headers] + @headers = Array(options[:headers]) @insecure = options[:insecure] @ui = options[:ui] @client_cert = options[:client_cert] diff --git a/plugins/commands/cloud/auth/middleware/add_downloader_authentication.rb b/plugins/commands/cloud/auth/middleware/add_downloader_authentication.rb index 2c7237c36..ae00302a7 100644 --- a/plugins/commands/cloud/auth/middleware/add_downloader_authentication.rb +++ b/plugins/commands/cloud/auth/middleware/add_downloader_authentication.rb @@ -13,6 +13,8 @@ module VagrantPlugins module CloudCommand class AddDownloaderAuthentication < AddAuthentication + @@logger = Log4r::Logger.new("vagrant::clout::add_download_authentication") + def call(env) client = Client.new(env[:env]) token = client.token @@ -38,8 +40,9 @@ module VagrantPlugins self.class.custom_host_notified! end - if !Array(env[:downloader].headers).any? { |h| h.include?("Authorization") } - env[:downloader].headers ||= [] + if Array(env[:downloader].headers).any? { |h| h.include?("Authorization") } + @@logger.info("Not adding an authentication header, one already found") + else env[:downloader].headers << "Authorization: Bearer #{token}" end end diff --git a/test/unit/plugins/commands/cloud/auth/middleware/add_downloader_authentication_test.rb b/test/unit/plugins/commands/cloud/auth/middleware/add_downloader_authentication_test.rb index 9def49ff1..517d789c6 100644 --- a/test/unit/plugins/commands/cloud/auth/middleware/add_downloader_authentication_test.rb +++ b/test/unit/plugins/commands/cloud/auth/middleware/add_downloader_authentication_test.rb @@ -36,13 +36,13 @@ describe VagrantPlugins::CloudCommand::AddDownloaderAuthentication do env[:downloader] = dwnloader subject.call(env) - expect(env[:downloader].headers.nil?).to eq(true) + expect(env[:downloader].headers.empty?).to eq(true) end it "does nothing if we aren't logged in" do env[:downloader] = dwnloader subject.call(env) - expect(env[:downloader].headers.nil?).to eq(true) + expect(env[:downloader].headers.empty?).to eq(true) end end @@ -117,7 +117,7 @@ describe VagrantPlugins::CloudCommand::AddDownloaderAuthentication do subject.call(env) expect(env[:downloader].source).to eq(file_path) - expect(env[:downloader].headers).to eq(nil) + expect(env[:downloader].headers.empty?).to eq(true) end end @@ -149,7 +149,7 @@ describe VagrantPlugins::CloudCommand::AddDownloaderAuthentication do VagrantPlugins::CloudCommand::Client.new(iso_env).store_token(token) env[:downloader] = dwnloader subject.call(env) - expect(env[:downloader].headers).to eq(nil) + expect(env[:downloader].headers.empty?).to eq(true) end end end