From 218f8323fb6fe6639517f3b09a4e880066b17c48 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Fri, 11 Jun 2021 16:59:35 -0700 Subject: [PATCH] Return original when access token is not removed When scrubbing box urls of access token parameters, only return the processed URL if the access token was removed. If it was not removed, return the original URL string. This prevents issues with local file URLs being parsed and replaced with invalid paths. Fixes: #12340 #12350 #12320 --- .../auth/middleware/add_authentication.rb | 9 +++++---- .../middleware/add_authentication_test.rb | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/plugins/commands/cloud/auth/middleware/add_authentication.rb b/plugins/commands/cloud/auth/middleware/add_authentication.rb index 1754db14b..5e7e7a95e 100644 --- a/plugins/commands/cloud/auth/middleware/add_authentication.rb +++ b/plugins/commands/cloud/auth/middleware/add_authentication.rb @@ -88,7 +88,7 @@ module VagrantPlugins begin u = URI.parse(url) q = CGI.parse(u.query || "") - if q["access_token"] + if !q["access_token"].empty? @logger.warn("Removing access token from URL parameter.") q.delete("access_token") if q.empty? @@ -96,14 +96,15 @@ module VagrantPlugins else u.query = URI.encode_www_form(q) end + u.to_s + else + @logger.warn("Authentication token not found as GET parameter.") + url end - - u.to_s rescue URI::Error url end end - @logger.warn("Authentication token not added as GET parameter.") end @app.call(env) end.freeze 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 c20d8a806..e0ec87a2e 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,6 +48,25 @@ describe VagrantPlugins::CloudCommand::AddAuthentication do expect(env[:box_urls]).to eq(original) end + context "when urls are set" do + it "does not modify urls" do + original = ["https://example.com/boxes/test.box", + "file://C:/my/box/path/local.box"] + env[:box_urls] = original.dup + subject.call(env) + expect(env[:box_urls]).to eq(original) + end + + it "should remove access_token parameters when found" do + env[:box_urls] = ["https://example.com/boxes/test.box?access_token=TEST", + "file://C:/my/box/path/local.box"] + subject.call(env) + expect(env[:box_urls]).to eq([ + "https://example.com/boxes/test.box", + "file://C:/my/box/path/local.box"]) + end + end + context "with VAGRANT_SERVER_ACCESS_TOKEN_BY_URL set" do before { stub_env("VAGRANT_SERVER_ACCESS_TOKEN_BY_URL" => "1") }