From f2bf18e56be36ed4bfbfe42fe20ab9147910ab3f Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Wed, 21 Feb 2018 17:00:58 -0800 Subject: [PATCH 1/2] Update behavior of the authentication middleware Always remap old hosts to target host when encountered. When custom vagrant server is defined, warn when tokens may be attached and allow time for user to cancel. Fixes #9442 --- lib/vagrant/shared_helpers.rb | 2 +- plugins/commands/login/locales/en.yml | 12 +++++ .../login/middleware/add_authentication.rb | 40 +++++++++------ .../middleware/add_authentication_test.rb | 50 ++++++++++++++++--- 4 files changed, 80 insertions(+), 24 deletions(-) diff --git a/lib/vagrant/shared_helpers.rb b/lib/vagrant/shared_helpers.rb index e2b18ca88..1dfbcd5d5 100644 --- a/lib/vagrant/shared_helpers.rb +++ b/lib/vagrant/shared_helpers.rb @@ -10,7 +10,7 @@ module Vagrant # of Vagrant that may require remote access. # # @return [String] - DEFAULT_SERVER_URL = "https://vagrantcloud.com" + DEFAULT_SERVER_URL = "https://vagrantcloud.com".freeze # Max number of seconds to wait for joining an active thread. # diff --git a/plugins/commands/login/locales/en.yml b/plugins/commands/login/locales/en.yml index 76fa01281..3f41a1067 100644 --- a/plugins/commands/login/locales/en.yml +++ b/plugins/commands/login/locales/en.yml @@ -1,5 +1,17 @@ en: login_command: + middleware: + authentication: + different_target: |- + Vagrant has detected a custom Vagrant server in use for downloading + box files. An authentication token is currently set which will be + added to the box request. If the custom Vagrant server should not + be receiving the authentication token, please unset it. + + Known Vagrant server: %{known_host} + Custom Vagrant server: %{custom_host} + + Press ctrl-c to cancel... errors: server_error: |- The Vagrant Cloud server responded with a not-OK response: diff --git a/plugins/commands/login/middleware/add_authentication.rb b/plugins/commands/login/middleware/add_authentication.rb index 3056a88f9..6cd5bee07 100644 --- a/plugins/commands/login/middleware/add_authentication.rb +++ b/plugins/commands/login/middleware/add_authentication.rb @@ -6,35 +6,43 @@ require_relative "../client" module VagrantPlugins module LoginCommand class AddAuthentication - ALLOWED_AUTHENTICATION_HOSTS = %w[ - app.vagrantup.com - atlas.hashicorp.com - vagrantcloud.com + REPLACEMENT_HOSTS = [ + "app.vagrantup.com".freeze, + "atlas.hashicorp.com".freeze ].freeze + TARGET_HOST = "vagrantcloud.com".freeze + CUSTOM_HOST_NOTIFY_WAIT = 5 def initialize(app, env) @app = app + LoginCommand::Plugin.init! end def call(env) client = Client.new(env[:env]) token = client.token - if token && Vagrant.server_url - server_uri = URI.parse(Vagrant.server_url) + env[:box_urls].map! do |url| + u = URI.parse(url) + if u.host != TARGET_HOST && REPLACEMENT_HOSTS.include?(u.host) + u.host = TARGET_HOST + end + u.to_s + end + + server_uri = URI.parse(Vagrant.server_url.to_s) + + if token && !server_uri.host.to_s.empty? + if server_uri.host != TARGET_HOST + env[:ui].warn(I18n.t("login_command.middleware.authentication.different_target", + custom_host: server_uri.host, known_host: TARGET_HOST) + "\n") + sleep CUSTOM_HOST_NOTIFY_WAIT + end env[:box_urls].map! do |url| u = URI.parse(url) - replace = u.host == server_uri.host - if !replace - if ALLOWED_AUTHENTICATION_HOSTS.include?(u.host) && - ALLOWED_AUTHENTICATION_HOSTS.include?(server_uri.host) - replace = true - end - end - - if replace + if u.host == server_uri.host q = CGI.parse(u.query || "") current = q["access_token"] @@ -50,7 +58,7 @@ module VagrantPlugins end @app.call(env) - end + end.freeze end end end diff --git a/test/unit/plugins/commands/login/middleware/add_authentication_test.rb b/test/unit/plugins/commands/login/middleware/add_authentication_test.rb index 8f1d1c6d9..72a193b9d 100644 --- a/test/unit/plugins/commands/login/middleware/add_authentication_test.rb +++ b/test/unit/plugins/commands/login/middleware/add_authentication_test.rb @@ -6,17 +6,20 @@ describe VagrantPlugins::LoginCommand::AddAuthentication 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://foo.com" } + let(:server_url) { "http://vagrantcloud.com" } 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 @@ -62,24 +65,46 @@ describe VagrantPlugins::LoginCommand::AddAuthentication do expect(env[:box_urls]).to eq(expected) end - it "appends the access token to vagrantcloud.com URLs if Atlas" do + 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::LoginCommand::Client.new(iso_env).store_token(token) + + original = [ + "http://google.com/box.box", + "http://vagrantcloud.com/foo.box", + "http://vagrantcloud.com/bar.box?arg=true", + ] + + expected = original.dup + + env[:box_urls] = original.dup + subject.call(env) + + expect(env[:box_urls]).to eq(expected) + end + + it "warns when adding token to custom server" do + server_url = "https://example.com" + allow(Vagrant).to receive(:server_url).and_return(server_url) token = "foobarbaz" VagrantPlugins::LoginCommand::Client.new(iso_env).store_token(token) original = [ "http://google.com/box.box", - "http://app.vagrantup.com/foo.box", "http://vagrantcloud.com/foo.box", - "http://vagrantcloud.com/bar.box?arg=true", + "http://example.com/bar.box" ] expected = original.dup - expected[1] = "#{original[1]}?access_token=#{token}" - expected[2] = "#{original[2]}?access_token=#{token}" - expected[3] = "#{original[3]}&access_token=#{token}" + expected.last.replace(expected.last + "?access_token=#{token}") + + expect(subject).to receive(:sleep).once + expect(ui).to receive(:warn).once env[:box_urls] = original.dup subject.call(env) @@ -87,6 +112,17 @@ describe VagrantPlugins::LoginCommand::AddAuthentication do expect(env[:box_urls]).to eq(expected) end + it "modifies host URL to target if authorized host" do + originals = VagrantPlugins::LoginCommand::AddAuthentication:: + REPLACEMENT_HOSTS.map{ |h| "http://#{h}/box.box" } + expected = "http://#{VagrantPlugins::LoginCommand::AddAuthentication::TARGET_HOST}/box.box" + env[:box_urls] = originals + subject.call(env) + env[:box_urls].each do |url| + expect(url).to eq(expected) + end + end + it "does not append multiple access_tokens" do token = "foobarbaz" VagrantPlugins::LoginCommand::Client.new(iso_env).store_token(token) From fc5ceafbcff36fab2d2d759bb4896fc0b4987c86 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 22 Feb 2018 14:49:44 -0800 Subject: [PATCH 2/2] Only generate notice once --- .../login/middleware/add_authentication.rb | 25 ++++++++++++++----- .../middleware/add_authentication_test.rb | 6 +++-- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/plugins/commands/login/middleware/add_authentication.rb b/plugins/commands/login/middleware/add_authentication.rb index 6cd5bee07..060ecf544 100644 --- a/plugins/commands/login/middleware/add_authentication.rb +++ b/plugins/commands/login/middleware/add_authentication.rb @@ -13,6 +13,18 @@ module VagrantPlugins TARGET_HOST = "vagrantcloud.com".freeze CUSTOM_HOST_NOTIFY_WAIT = 5 + def self.custom_host_notified! + @_host_notify = true + end + + def self.custom_host_notified? + if defined?(@_host_notify) + @_host_notify + else + false + end + end + def initialize(app, env) @app = app LoginCommand::Plugin.init! @@ -33,16 +45,17 @@ module VagrantPlugins server_uri = URI.parse(Vagrant.server_url.to_s) if token && !server_uri.host.to_s.empty? - if server_uri.host != TARGET_HOST - env[:ui].warn(I18n.t("login_command.middleware.authentication.different_target", - custom_host: server_uri.host, known_host: TARGET_HOST) + "\n") - sleep CUSTOM_HOST_NOTIFY_WAIT - end - 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("login_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"] diff --git a/test/unit/plugins/commands/login/middleware/add_authentication_test.rb b/test/unit/plugins/commands/login/middleware/add_authentication_test.rb index 72a193b9d..07fc9fcfe 100644 --- a/test/unit/plugins/commands/login/middleware/add_authentication_test.rb +++ b/test/unit/plugins/commands/login/middleware/add_authentication_test.rb @@ -97,11 +97,13 @@ describe VagrantPlugins::LoginCommand::AddAuthentication do original = [ "http://google.com/box.box", "http://vagrantcloud.com/foo.box", - "http://example.com/bar.box" + "http://example.com/bar.box", + "http://example.com/foo.box" ] expected = original.dup - expected.last.replace(expected.last + "?access_token=#{token}") + expected[2] = expected[2] + "?access_token=#{token}" + expected[3] = expected[3] + "?access_token=#{token}" expect(subject).to receive(:sleep).once expect(ui).to receive(:warn).once