Merge pull request #6534 from mitchellh/sethvargo/less_access_token

Only append access_token the first time

plugins/commands/login/middleware/add_authentication.rb

@@ -1,3 +1,4 @@
+require "cgi"
 require "uri"
 
 require_relative "../client"
@@ -5,6 +6,9 @@ require_relative "../client"
 module VagrantPlugins
   module LoginCommand
     class AddAuthentication
+      VCLOUD = "vagrantcloud.com".freeze
+      ATLAS  = "atlas.hashicorp.com".freeze
+
       def initialize(app, env)
         @app = app
       end
@@ -19,19 +23,26 @@ module VagrantPlugins
           env[:box_urls].map! do |url|
             u = URI.parse(url)
             replace = u.host == server_uri.host
+
             if !replace
               # We need this in here for the transition we made from
               # Vagrant Cloud to Atlas. This preserves access tokens
               # appending to both without leaking access tokens to
               # unsavory URLs.
-              replace = u.host == "vagrantcloud.com" &&
-                server_uri.host == "atlas.hashicorp.com"
+              if u.host == VCLOUD && server_uri.host == ATLAS
+                replace = true
+              end
             end
 
             if replace
-              u.query ||= ""
-              u.query += "&" if u.query != ""
-              u.query += "access_token=#{token}"
+              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

test/unit/plugins/commands/login/middleware/add_authentication_test.rb

@@ -84,5 +84,21 @@ describe VagrantPlugins::LoginCommand::AddAuthentication do
 
       expect(env[:box_urls]).to eq(expected)
     end
+
+    it "does not append multiple access_tokens" do
+      token = "foobarbaz"
+      VagrantPlugins::LoginCommand::Client.new(iso_env).store_token(token)
+
+      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=foobarbaz")
+    end
   end
 end