Deprecate hook and disable access token parameter by default

This sets the `authenticate_box_url` hook as deprecated and also
    disables the cloud auth middleware from adding an access token
    as a URL parameter by default. An environment variable has been
    added which can be used for re-enabling the access token URL
    parameter behavior if required for some legacy system which does
    not support the authorization header.
This commit is contained in:
Chris Roberts 2021-03-15 14:46:46 -07:00
parent e963774ceb
commit 51382a0d0a
5 changed files with 186 additions and 133 deletions

View File

@ -27,6 +27,10 @@ module Vagrant
DEFAULT_LOCAL_DATA = ".vagrant"
# List of hooks which are deprecated and should log a warning
# when used
DEPRECATED_HOOKS = [:authenticate_box_url].freeze
# The `cwd` that this environment represents
attr_reader :cwd
@ -517,6 +521,12 @@ module Vagrant
# @param [Action::Runner] action_runner A custom action runner for running hooks.
def hook(name, opts=nil)
@logger.info("Running hook: #{name}")
if DEPRECATED_HOOKS.include?(name.to_sym)
@logger.warn("Deprecated hook: #{name}. This hook is deprecated and " \
"will be removed in a future version of Vagrant.")
end
opts ||= {}
opts[:callable] ||= Action::Builder.new
opts[:runner] ||= action_runner

View File

@ -1,5 +1,6 @@
require "cgi"
require "uri"
require "log4r"
require Vagrant.source_root.join("plugins/commands/cloud/client/client")
@ -27,55 +28,64 @@ module VagrantPlugins
def initialize(app, env)
@app = app
@logger = Log4r::Logger.new("vagrant::cloud::auth::authenticate-box-url")
CloudCommand::Plugin.init!
end
def call(env)
client = Client.new(env[:env])
token = client.token
if ENV["VAGRANT_ALLOW_PARAM_AUTH_TOKEN"]
@logger.warn("Adding auth token as GET parameter by user request")
client = Client.new(env[:env])
token = client.token
env[:box_urls].map! do |url|
begin
u = URI.parse(url)
if u.host != TARGET_HOST && REPLACEMENT_HOSTS.include?(u.host)
u.host = TARGET_HOST
u.to_s
else
env[:box_urls].map! do |url|
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
rescue URI::Error
url
end
rescue URI::Error
url
end
end
server_uri = URI.parse(Vagrant.server_url.to_s)
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 token && !server_uri.host.to_s.empty?
env[:box_urls].map! do |url|
begin
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!
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
u.to_s
rescue URI::Error
url
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
u.to_s
end
else
@logger.warn("Authentication token not added as GET parameter.")
end
@app.call(env)
end.freeze
end

View File

@ -7,13 +7,16 @@ 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.
# with Vagrant cloud. It does this by adding Authentication headers to a
# Vagrant::Util::Downloader object.
module VagrantPlugins
module CloudCommand
class AddDownloaderAuthentication < AddAuthentication
@@logger = Log4r::Logger.new("vagrant::clout::add_download_authentication")
def initialize(app, env)
super
@logger = Log4r::Logger.new("vagrant::cloud::auth::add-download-authentication")
end
def call(env)
client = Client.new(env[:env])
@ -41,7 +44,7 @@ module VagrantPlugins
end
if Array(env[:downloader].headers).any? { |h| h.include?("Authorization") }
@@logger.info("Not adding an authentication header, one already found")
@logger.info("Not adding an authentication header, one already found")
else
env[:downloader].headers << "Authorization: Bearer #{token}"
end

View File

@ -14,21 +14,24 @@ describe VagrantPlugins::CloudCommand::AddAuthentication do
let(:iso_env) { isolated_environment.create_vagrant_env }
let(:server_url) { "http://vagrantcloud.com" }
let(:client) { double("client", token: token) }
let(:token) { "TEST_TOKEN" }
subject { described_class.new(app, env) }
before do
allow(Vagrant).to receive(:server_url).and_return(server_url)
allow(ui).to receive(:warn)
allow(VagrantPlugins::CloudCommand::Client).to receive(:new).
with(iso_env).and_return(client)
stub_env("ATLAS_TOKEN" => nil)
end
describe "#call" do
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("foo")
original = ["foo", "#{server_url}/bar"]
original = [token, "#{server_url}/bar"]
env[:box_urls] = original.dup
subject.call(env)
@ -45,125 +48,144 @@ describe VagrantPlugins::CloudCommand::AddAuthentication do
expect(env[:box_urls]).to eq(original)
end
it "appends the access token to the URL of server URLs" do
token = "foobarbaz"
VagrantPlugins::CloudCommand::Client.new(iso_env).store_token(token)
context "with VAGRANT_ALLOW_PARAM_AUTH_TOKEN set" do
original = [
"http://example.com/box.box",
"#{server_url}/foo.box",
"#{server_url}/bar.box?arg=true",
]
before { stub_env("VAGRANT_ALLOW_PARAM_AUTH_TOKEN" => "1") }
expected = original.dup
expected[1] = "#{original[1]}?access_token=#{token}"
expected[2] = "#{original[2]}&access_token=#{token}"
it "appends the access token to the URL of server URLs" do
original = [
"http://example.com/box.box",
"#{server_url}/foo.box",
"#{server_url}/bar.box?arg=true",
]
env[:box_urls] = original.dup
subject.call(env)
expected = original.dup
expected[1] = "#{original[1]}?access_token=#{token}"
expected[2] = "#{original[2]}&access_token=#{token}"
expect(env[:box_urls]).to eq(expected)
end
env[:box_urls] = original.dup
subject.call(env)
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)
expect(env[:box_urls]).to eq(expected)
end
token = "foobarbaz"
VagrantPlugins::CloudCommand::Client.new(iso_env).store_token(token)
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)
original = [
"http://example.com/box.box",
"http://vagrantcloud.com/foo.box",
"http://vagrantcloud.com/bar.box?arg=true",
]
original = [
"http://example.com/box.box",
"http://vagrantcloud.com/foo.box",
"http://vagrantcloud.com/bar.box?arg=true",
]
expected = original.dup
expected = original.dup
env[:box_urls] = original.dup
subject.call(env)
env[:box_urls] = original.dup
subject.call(env)
expect(env[:box_urls]).to eq(expected)
end
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)
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::CloudCommand::Client.new(iso_env).store_token(token)
original = [
"http://example.org/box.box",
"http://vagrantcloud.com/foo.box",
"http://example.com/bar.box",
"http://example.com/foo.box"
]
original = [
"http://example.org/box.box",
"http://vagrantcloud.com/foo.box",
"http://example.com/bar.box",
"http://example.com/foo.box"
]
expected = original.dup
expected[2] = expected[2] + "?access_token=#{token}"
expected[3] = expected[3] + "?access_token=#{token}"
expected = original.dup
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
expect(subject).to receive(:sleep).once
expect(ui).to receive(:warn).once
env[:box_urls] = original.dup
subject.call(env)
env[:box_urls] = original.dup
subject.call(env)
expect(env[:box_urls]).to eq(expected)
end
expect(env[:box_urls]).to eq(expected)
end
it "ignores urls that it cannot parse" do
bad_url = "this is not a valid url"
# Ensure the bad URL does cause an exception
expect{ URI.parse(bad_url) }.to raise_error URI::Error
env[:box_urls] = [bad_url]
subject.call(env)
expect(env[:box_urls].first).to eq(bad_url)
end
it "modifies host URL to target if authorized host" do
originals = VagrantPlugins::CloudCommand::AddAuthentication::
REPLACEMENT_HOSTS.map{ |h| "http://#{h}/box.box" }
expected = "http://#{VagrantPlugins::CloudCommand::AddAuthentication::TARGET_HOST}/box.box"
env[:box_urls] = originals
subject.call(env)
env[:box_urls].each do |url|
expect(url).to eq(expected)
it "does not append multiple access_tokens" do
original = [
"#{server_url}/foo.box?access_token=existing",
"#{server_url}/bar.box?arg=true",
]
env[:box_urls] = original.dup
subject.call(env)
expect(env[:box_urls][0]).to eq("#{server_url}/foo.box?access_token=existing")
expect(env[:box_urls][1]).to eq("#{server_url}/bar.box?arg=true&access_token=#{token}")
end
context "when token is not set" do
let(:token) { nil }
it "modifies host URL to target if authorized host" do
originals = VagrantPlugins::CloudCommand::AddAuthentication::
REPLACEMENT_HOSTS.map{ |h| "http://#{h}/box.box" }
expected = "http://#{VagrantPlugins::CloudCommand::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 "returns original urls when not modified" do
to_persist = "file:////path/to/box.box"
to_change = VagrantPlugins::CloudCommand::AddAuthentication::
REPLACEMENT_HOSTS.map{ |h| "http://#{h}/box.box" }.first
expected = "http://#{VagrantPlugins::CloudCommand::AddAuthentication::TARGET_HOST}/box.box"
env[:box_urls] = [to_persist, to_change]
subject.call(env)
check_persist, check_change = env[:box_urls]
expect(check_change).to eq(expected)
expect(check_persist).to eq(to_persist)
# NOTE: The behavior of URI.parse changes on Ruby 2.5 to produce
# the same string value. To make the test worthwhile in checking
# for the same value, check that the object IDs are still the same.
expect(check_persist.object_id).to eq(to_persist.object_id)
end
end
end
it "ignores urls that it cannot parse" do
bad_url = "this is not a valid url"
# Ensure the bad URL does cause an exception
expect{ URI.parse(bad_url) }.to raise_error URI::Error
env[:box_urls] = [bad_url]
subject.call(env)
expect(env[:box_urls].first).to eq(bad_url)
end
it "returns original urls when not modified" do
to_persist = "file:////path/to/box.box"
to_change = VagrantPlugins::CloudCommand::AddAuthentication::
REPLACEMENT_HOSTS.map{ |h| "http://#{h}/box.box" }.first
expected = "http://#{VagrantPlugins::CloudCommand::AddAuthentication::TARGET_HOST}/box.box"
env[:box_urls] = [to_persist, to_change]
subject.call(env)
check_persist, check_change = env[:box_urls]
expect(check_change).to eq(expected)
expect(check_persist).to eq(to_persist)
# NOTE: The behavior of URI.parse changes on Ruby 2.5 to produce
# the same string value. To make the test worthwhile in checking
# for the same value, check that the object IDs are still the same.
expect(check_persist.object_id).to eq(to_persist.object_id)
end
context "with VAGRANT_ALLOW_PARAM_AUTH_TOKEN unset" do
it "does not append multiple access_tokens" do
token = "foobarbaz"
VagrantPlugins::CloudCommand::Client.new(iso_env).store_token(token)
before { stub_env("VAGRANT_ALLOW_PARAM_AUTH_TOKEN" => nil) }
original = [
"#{server_url}/foo.box?access_token=existing",
"#{server_url}/bar.box?arg=true",
]
it "returns the original urls" do
box1 = "http://vagrantcloud.com/box.box"
box2 = "http://app.vagrantup.com/box.box"
env[:box_urls] = original.dup
subject.call(env)
env = {
box_urls: [
box1.dup,
box2.dup
]
}
subject.call(env)
expect(env[:box_urls][0]).to eq("#{server_url}/foo.box?access_token=existing")
expect(env[:box_urls][1]).to eq("#{server_url}/bar.box?arg=true&access_token=foobarbaz")
expect(env[:box_urls]).to eq([box1, box2])
end
end
end
end

View File

@ -46,6 +46,14 @@ 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