Merge pull request #4934 from mitchellh/sethvargo/login_token

Read ATLAS_TOKEN in vagrant-login

plugins/commands/login/client.rb

@@ -58,13 +58,21 @@ module VagrantPlugins
         nil
       end
 
-      # Reads the access token if there is one, or returns nil otherwise.
+      # Reads the access token if there is one. This will first read the
+      # `ATLAS_TOKEN` environment variable and then fallback to the stored
+      # access token on disk.
       #
       # @return [String]
       def token
-        token_path.read
-      rescue Errno::ENOENT
-        return nil
+        if ENV["ATLAS_TOKEN"] && !ENV["ATLAS_TOKEN"].empty?
+          return ENV["ATLAS_TOKEN"]
+        end
+
+        if token_path.exist?
+          return token_path.read.strip
+        end
+
+        nil
       end
 
       protected

plugins/pushes/atlas/config.rb

@@ -85,7 +85,7 @@ module VagrantPlugins
         errors = _detected_errors
 
         if missing?(@token)
-          token = token_from_vagrant_login(machine.env) || ENV["ATLAS_TOKEN"]
+          token = token_from_vagrant_login(machine.env)
           if missing?(token)
             errors << I18n.t("atlas_push.errors.missing_token")
           else

test/unit/plugins/commands/login/client_test.rb

@@ -5,49 +5,59 @@ require Vagrant.source_root.join("plugins/commands/login/command")
 describe VagrantPlugins::LoginCommand::Client do
   include_context "unit"
 
+  def stub_env(key, value = nil)
+    allow(ENV).to receive(:[]).and_call_original
+    allow(ENV).to receive(:[])
+      .with(key)
+      .and_return(value)
+  end
+
   let(:env) { isolated_environment.create_vagrant_env }
 
   subject { described_class.new(env) }
 
+  before do
+    stub_env("ATLAS_TOKEN", nil)
+    subject.clear_token
+  end
+
   describe "#logged_in?" do
-    it "quickly returns false if no token is set" do
-      expect(subject).to_not be_logged_in
+    let(:url) { "#{Vagrant.server_url}/api/v1/authenticate?access_token=#{token}" }
+    let(:headers) { { "Content-Type" => "application/json" } }
+
+    before { allow(subject).to receive(:token).and_return(token) }
+
+    context "when there is no token" do
+      let(:token) { nil }
+
+      it "returns false" do
+        expect(subject.logged_in?).to be(false)
+      end
     end
 
-    it "returns true if the endpoint returns 200" do
-      subject.store_token("foo")
+    context "when there is a token" do
+      let(:token) { "ABCD1234" }
 
-      response = {
-        "token" => "baz",
-      }
-
-      headers = { "Content-Type" => "application/json" }
-      url = "#{Vagrant.server_url}/api/v1/authenticate?access_token=foo"
-      stub_request(:get, url).
-        with(headers: headers).
-        to_return(status: 200, body: JSON.dump(response))
-
-      expect(subject).to be_logged_in
+      it "returns true if the endpoint returns a 200" do
+        stub_request(:get, url)
+          .with(headers: headers)
+          .to_return(body: JSON.pretty_generate("token" => token))
+        expect(subject.logged_in?).to be(true)
       end
 
-    it "returns false if 401 is returned" do
-      subject.store_token("foo")
-
-      url = "#{Vagrant.server_url}/api/v1/authenticate?access_token=foo"
-      stub_request(:get, url).
-        to_return(status: 401, body: "")
-
-      expect(subject).to_not be_logged_in
+      it "returns false if the endpoint returns a non-200" do
+        stub_request(:get, url)
+          .with(headers: headers)
+          .to_return(body: JSON.pretty_generate("bad" => true), status: 401)
+        expect(subject.logged_in?).to be(false)
       end
 
-    it "raises an exception if it can't reach the sever" do
-      subject.store_token("foo")
-
-      url = "#{Vagrant.server_url}/api/v1/authenticate?access_token=foo"
-      stub_request(:get, url).to_raise(SocketError)
-
-      expect { subject.logged_in? }.
-        to raise_error(VagrantPlugins::LoginCommand::Errors::ServerUnreachable)
+      it "raises an exception if the server cannot be found" do
+        stub_request(:get, url)
+          .to_raise(SocketError)
+        expect { subject.logged_in? }
+          .to raise_error(VagrantPlugins::LoginCommand::Errors::ServerUnreachable)
+      end
     end
   end
 
@@ -89,11 +99,29 @@ describe VagrantPlugins::LoginCommand::Client do
     end
   end
 
-  describe "#token, #store_token, #clear_token" do
-    it "returns nil if there is no token" do
-      expect(subject.token).to be_nil
+  describe "#token" do
+    it "reads ATLAS_TOKEN" do
+      stub_env("ATLAS_TOKEN", "ABCD1234")
+      expect(subject.token).to eq("ABCD1234")
     end
 
+    it "reads the stored file" do
+      subject.store_token("EFGH5678")
+      expect(subject.token).to eq("EFGH5678")
+    end
+
+    it "prefers the environment variable" do
+      stub_env("ATLAS_TOKEN", "ABCD1234")
+      subject.store_token("EFGH5678")
+      expect(subject.token).to eq("ABCD1234")
+    end
+
+    it "returns nil if there's no token set" do
+      expect(subject.token).to be(nil)
+    end
+  end
+
+  describe "#store_token, #clear_token" do
     it "stores the token and can re-access it" do
       subject.store_token("foo")
       expect(subject.token).to eq("foo")

test/unit/plugins/pushes/atlas/config_test.rb

@@ -73,13 +73,9 @@ describe VagrantPlugins::AtlasPush::Config do
         before do
           allow(subject).to receive(:token_from_vagrant_login)
             .and_return("token_from_vagrant_login")
-
-          allow(ENV).to receive(:[]).and_call_original
-          allow(ENV).to receive(:[])
-            .with("ATLAS_TOKEN").and_return("token_from_env")
         end
 
-        it "uses the token in the Vagrantfile" do
+        it "uses the token from vagrant-login" do
           subject.token = ""
           subject.finalize!
           expect(errors).to be_empty
@@ -87,32 +83,10 @@ describe VagrantPlugins::AtlasPush::Config do
         end
       end
 
-      context "when ATLAS_TOKEN is set in the environment" do
-        before do
-          allow(subject).to receive(:token_from_vagrant_login)
-            .and_return(nil)
-
-          allow(ENV).to receive(:[]).and_call_original
-          allow(ENV).to receive(:[])
-            .with("ATLAS_TOKEN").and_return("token_from_env")
-        end
-
-        it "uses the token in the environment" do
-          subject.token = ""
-          subject.finalize!
-          expect(errors).to be_empty
-          expect(subject.token).to eq("token_from_env")
-        end
-      end
-
       context "when a token is given in the Vagrantfile" do
         before do
           allow(subject).to receive(:token_from_vagrant_login)
             .and_return("token_from_vagrant_login")
-
-          allow(ENV).to receive(:[]).and_call_original
-          allow(ENV).to receive(:[])
-            .with("ATLAS_TOKEN").and_return("token_from_env")
         end
 
         it "uses the token in the Vagrantfile" do
@@ -127,10 +101,6 @@ describe VagrantPlugins::AtlasPush::Config do
         before do
           allow(subject).to receive(:token_from_vagrant_login)
             .and_return(nil)
-
-          allow(ENV).to receive(:[]).and_call_original
-          allow(ENV).to receive(:[])
-            .with("ATLAS_TOKEN").and_return(nil)
         end
 
         it "returns an error" do