From cf51c18ad8955b814e7f3871fcced5a51dad6137 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Wed, 24 Mar 2021 17:24:47 -0700 Subject: [PATCH] Update authentication middleware access token handling Replace the `VAGRANT_ALLOW_PARAM_AUTH_TOKEN` environment variable with `VAGRANT_SERVER_ACCESS_TOKEN_BY_URL` and update the behavior when the environment variable is set to add the access token as a query parameter and disable the addition of the authentication header. Fixes #12080 --- .../auth/middleware/add_authentication.rb | 4 +- .../add_downloader_authentication.rb | 56 ++++++++++--------- .../middleware/add_authentication_test.rb | 8 +-- .../add_downloader_authentication_test.rb | 15 +++++ .../docs/other/environmental-variables.mdx | 25 ++++++--- 5 files changed, 68 insertions(+), 40 deletions(-) diff --git a/plugins/commands/cloud/auth/middleware/add_authentication.rb b/plugins/commands/cloud/auth/middleware/add_authentication.rb index 704a07967..1754db14b 100644 --- a/plugins/commands/cloud/auth/middleware/add_authentication.rb +++ b/plugins/commands/cloud/auth/middleware/add_authentication.rb @@ -33,8 +33,8 @@ module VagrantPlugins end def call(env) - if ENV["VAGRANT_ALLOW_PARAM_AUTH_TOKEN"] - @logger.warn("Adding auth token as GET parameter by user request") + if ENV["VAGRANT_SERVER_ACCESS_TOKEN_BY_URL"] + @logger.warn("Adding access token as GET parameter by user request") client = Client.new(env[:env]) token = client.token diff --git a/plugins/commands/cloud/auth/middleware/add_downloader_authentication.rb b/plugins/commands/cloud/auth/middleware/add_downloader_authentication.rb index 1ee1da7a0..1690580f0 100644 --- a/plugins/commands/cloud/auth/middleware/add_downloader_authentication.rb +++ b/plugins/commands/cloud/auth/middleware/add_downloader_authentication.rb @@ -19,38 +19,42 @@ module VagrantPlugins end def call(env) - client = Client.new(env[:env]) - token = client.token - Vagrant::Util::CredentialScrubber.sensitive(token) + if ENV["VAGRANT_SERVER_ACCESS_TOKEN_BY_URL"] + @logger.warn("Authentication header not added due to user requested access token URL parameter") + else + client = Client.new(env[:env]) + token = client.token + Vagrant::Util::CredentialScrubber.sensitive(token) - begin - target_url = URI.parse(env[:downloader].source) - if target_url.host != TARGET_HOST && REPLACEMENT_HOSTS.include?(target_url.host) + 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) - 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 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 + rescue URI::Error + # if there is an error, use current target_url end - env[:downloader] + 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 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 + + env[:downloader] + end end @app.call(env) diff --git a/test/unit/plugins/commands/cloud/auth/middleware/add_authentication_test.rb b/test/unit/plugins/commands/cloud/auth/middleware/add_authentication_test.rb index bb72329a1..c20d8a806 100644 --- a/test/unit/plugins/commands/cloud/auth/middleware/add_authentication_test.rb +++ b/test/unit/plugins/commands/cloud/auth/middleware/add_authentication_test.rb @@ -48,9 +48,9 @@ describe VagrantPlugins::CloudCommand::AddAuthentication do expect(env[:box_urls]).to eq(original) end - context "with VAGRANT_ALLOW_PARAM_AUTH_TOKEN set" do + context "with VAGRANT_SERVER_ACCESS_TOKEN_BY_URL set" do - before { stub_env("VAGRANT_ALLOW_PARAM_AUTH_TOKEN" => "1") } + before { stub_env("VAGRANT_SERVER_ACCESS_TOKEN_BY_URL" => "1") } it "appends the access token to the URL of server URLs" do original = [ @@ -168,9 +168,9 @@ describe VagrantPlugins::CloudCommand::AddAuthentication do end - context "with VAGRANT_ALLOW_PARAM_AUTH_TOKEN unset" do + context "with VAGRANT_SERVER_ACCESS_TOKEN_BY_URL unset" do - before { stub_env("VAGRANT_ALLOW_PARAM_AUTH_TOKEN" => nil) } + before { stub_env("VAGRANT_SERVER_ACCESS_TOKEN_BY_URL" => nil) } it "returns the original urls" do box1 = "http://vagrantcloud.com/box.box" 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 0ddb463de..e274f7a36 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 @@ -23,6 +23,7 @@ describe VagrantPlugins::CloudCommand::AddDownloaderAuthentication do allow(Vagrant).to receive(:server_url).and_return(server_url) allow(ui).to receive(:warn) stub_env("ATLAS_TOKEN" => nil) + stub_env("VAGRANT_SERVER_ACCESS_TOKEN_BY_URL" => nil) end describe "#call" do @@ -151,5 +152,19 @@ describe VagrantPlugins::CloudCommand::AddDownloaderAuthentication do subject.call(env) expect(env[:downloader].headers.empty?).to eq(true) end + + context "with VAGRANT_SERVER_ACCESS_TOKEN_BY_URL environment variable set" do + before do + stub_env("VAGRANT_SERVER_ACCESS_TOKEN_BY_URL" => "1") + end + + it "does not add 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([]) + end + end end end diff --git a/website/content/docs/other/environmental-variables.mdx b/website/content/docs/other/environmental-variables.mdx index e73c6c6f6..acf794c83 100644 --- a/website/content/docs/other/environmental-variables.mdx +++ b/website/content/docs/other/environmental-variables.mdx @@ -46,14 +46,6 @@ the Vagrant installation. `VAGRANT_ALIAS_FILE` can be set to change the file where Vagrant aliases are defined. By default, this is set to `~/.vagrant.d/aliases`. -## `VAGRANT_ALLOW_PARAM_AUTH_TOKEN` - -If this is set to any value Vagrant will append an access token to a Vagrant -box URL as a GET parameter. By default Vagrant will set the access token -with an authorization header of the request. Setting the access token as -a parameter on the URL is deprecated behavior, however this environment -variable allows it to be enabled for legacy implementations. - ## `VAGRANT_ALLOW_PLUGIN_SOURCE_ERRORS` If this is set to any value, then Vagrant will not error when a configured @@ -298,6 +290,23 @@ Vagrant will default to using a system provided `ssh` on Windows. This environment variable can also be used to disable that behavior to force Vagrant to use the embedded `ssh` executable by setting it to `0`. +## `VAGRANT_SERVER_URL` + +This configures the remote server which Vagrant will connect to for fetching +Vagrant boxes. By default this is configured for Vagrant Cloud (https://vagrantcloud.com) + +## `VAGRANT_SERVER_ACCESS_TOKEN_BY_URL` + +If this is set Vagrant will change the way it authenticates with the configured +Vagrant server. When set, the authentication behavior will be reverted to the +deprecated authentication behavior of: + +1. not adding an authentication header to the request +2. setting the configured access token as a query parameter on URLs + +This behavior can be useful for third party servers which do not accept the +authentication header currently used with Vagrant Cloud. + ## `VAGRANT_SKIP_SUBPROCESS_JAILBREAK` As of Vagrant 1.7.3, Vagrant tries to intelligently detect if it is running in