From 7d6a6cd2639d01b9f4887a378a1333cbbedb57de Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Wed, 10 Dec 2014 15:08:43 -0800 Subject: [PATCH 1/3] Read the ATLAS_TOKEN in vagrant-login --- plugins/commands/login/client.rb | 16 ++- .../plugins/commands/login/client_test.rb | 100 +++++++++++------- 2 files changed, 76 insertions(+), 40 deletions(-) diff --git a/plugins/commands/login/client.rb b/plugins/commands/login/client.rb index f06772be9..1e8381fe4 100644 --- a/plugins/commands/login/client.rb +++ b/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 diff --git a/test/unit/plugins/commands/login/client_test.rb b/test/unit/plugins/commands/login/client_test.rb index 7c9480442..fcdf1b902 100644 --- a/test/unit/plugins/commands/login/client_test.rb +++ b/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", - } + 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 - 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)) + 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 - expect(subject).to be_logged_in - 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 - 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") From 0506e177786408dbc6e5e55090a935e53a7a9c0a Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Wed, 10 Dec 2014 15:11:04 -0800 Subject: [PATCH 2/3] Update Atlas Push to use new vagrant-login API --- plugins/pushes/atlas/config.rb | 2 +- test/unit/plugins/pushes/atlas/config_test.rb | 32 +------------------ 2 files changed, 2 insertions(+), 32 deletions(-) diff --git a/plugins/pushes/atlas/config.rb b/plugins/pushes/atlas/config.rb index d4d2da2c5..55543fae6 100644 --- a/plugins/pushes/atlas/config.rb +++ b/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 diff --git a/test/unit/plugins/pushes/atlas/config_test.rb b/test/unit/plugins/pushes/atlas/config_test.rb index 1b2695ba9..580a39ab0 100644 --- a/test/unit/plugins/pushes/atlas/config_test.rb +++ b/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 From 234adaae637e8565fff5490fb554742ed5deb431 Mon Sep 17 00:00:00 2001 From: Max Lincoln Date: Thu, 11 Dec 2014 13:55:49 -0500 Subject: [PATCH 3/3] WinRM SSL support --- plugins/communicators/winrm/communicator.rb | 7 +--- plugins/communicators/winrm/config.rb | 32 +++++++++----- plugins/communicators/winrm/shell.rb | 42 +++++++++++-------- plugins/kernel_v2/config/vm.rb | 11 +++-- .../communicators/winrm/communicator_test.rb | 2 +- .../plugins/communicators/winrm/shell_test.rb | 34 ++++++++++++--- test/unit/plugins/kernel_v2/config/vm_test.rb | 37 ++++++++++++---- vagrant.gemspec | 2 +- 8 files changed, 114 insertions(+), 53 deletions(-) diff --git a/plugins/communicators/winrm/communicator.rb b/plugins/communicators/winrm/communicator.rb index 7609f16b3..4bd6e47d4 100644 --- a/plugins/communicators/winrm/communicator.rb +++ b/plugins/communicators/winrm/communicator.rb @@ -106,11 +106,8 @@ module VagrantPlugins WinRMShell.new( winrm_info[:host], - @machine.config.winrm.username, - @machine.config.winrm.password, - port: winrm_info[:port], - timeout_in_seconds: @machine.config.winrm.timeout, - max_tries: @machine.config.winrm.max_tries, + winrm_info[:port], + @machine.config.winrm ) end diff --git a/plugins/communicators/winrm/config.rb b/plugins/communicators/winrm/config.rb index 155ac0567..6e4a44338 100644 --- a/plugins/communicators/winrm/config.rb +++ b/plugins/communicators/winrm/config.rb @@ -8,25 +8,32 @@ module VagrantPlugins attr_accessor :guest_port attr_accessor :max_tries attr_accessor :timeout + attr_accessor :transport + attr_accessor :ssl_peer_verification def initialize - @username = UNSET_VALUE - @password = UNSET_VALUE - @host = UNSET_VALUE - @port = UNSET_VALUE - @guest_port = UNSET_VALUE - @max_tries = UNSET_VALUE - @timeout = UNSET_VALUE + @username = UNSET_VALUE + @password = UNSET_VALUE + @host = UNSET_VALUE + @port = UNSET_VALUE + @guest_port = UNSET_VALUE + @max_tries = UNSET_VALUE + @timeout = UNSET_VALUE + @transport = UNSET_VALUE + @ssl_peer_verification = UNSET_VALUE end def finalize! - @username = "vagrant" if @username == UNSET_VALUE - @password = "vagrant" if @password == UNSET_VALUE + @username = "vagrant" if @username == UNSET_VALUE + @password = "vagrant" if @password == UNSET_VALUE + @transport = :plaintext if @transport == UNSET_VALUE @host = nil if @host == UNSET_VALUE - @port = 5985 if @port == UNSET_VALUE - @guest_port = 5985 if @guest_port == UNSET_VALUE + is_ssl = @transport == :ssl + @port = (is_ssl ? 5986 : 5985) if @port == UNSET_VALUE + @guest_port = (is_ssl ? 5986 : 5985) if @guest_port == UNSET_VALUE @max_tries = 20 if @max_tries == UNSET_VALUE @timeout = 1800 if @timeout == UNSET_VALUE + @ssl_peer_verification = true if @ssl_peer_verification == UNSET_VALUE end def validate(machine) @@ -38,6 +45,9 @@ module VagrantPlugins errors << "winrm.guest_port cannot be nil." if @guest_port.nil? errors << "winrm.max_tries cannot be nil." if @max_tries.nil? errors << "winrm.timeout cannot be nil." if @timeout.nil? + unless @ssl_peer_verification == true || @ssl_peer_verification == false + errors << "winrm.ssl_peer_verification must be a boolean." + end { "WinRM" => errors } end diff --git a/plugins/communicators/winrm/shell.rb b/plugins/communicators/winrm/shell.rb index 353c0e84d..1f9e6796c 100644 --- a/plugins/communicators/winrm/shell.rb +++ b/plugins/communicators/winrm/shell.rb @@ -32,23 +32,21 @@ module VagrantPlugins ] attr_reader :logger - attr_reader :username - attr_reader :password attr_reader :host attr_reader :port - attr_reader :timeout_in_seconds - attr_reader :max_tries + attr_reader :username + attr_reader :password + attr_reader :config - def initialize(host, username, password, options = {}) + def initialize(host, port, config) @logger = Log4r::Logger.new("vagrant::communication::winrmshell") @logger.debug("initializing WinRMShell") - @host = host - @port = options[:port] || 5985 - @username = username - @password = password - @timeout_in_seconds = options[:timeout_in_seconds] || 60 - @max_tries = options[:max_tries] || 20 + @host = host + @port = port + @username = config.username + @password = config.password + @config = config end def powershell(command, &block) @@ -87,7 +85,7 @@ module VagrantPlugins end def execute_shell_with_retry(command, shell, &block) - retryable(tries: @max_tries, on: @@exceptions_to_retry_on, sleep: 10) do + retryable(tries: @config.max_tries, on: @@exceptions_to_retry_on, sleep: 10) do @logger.debug("#{shell} executing:\n#{command}") output = session.send(shell, command) do |out, err| block.call(:stdout, out) if block_given? && out @@ -118,10 +116,11 @@ module VagrantPlugins @logger.info("Attempting to connect to WinRM...") @logger.info(" - Host: #{@host}") @logger.info(" - Port: #{@port}") - @logger.info(" - Username: #{@username}") + @logger.info(" - Username: #{@config.username}") + @logger.info(" - Transport: #{@config.transport}") - client = ::WinRM::WinRMWebService.new(endpoint, :plaintext, endpoint_options) - client.set_timeout(@timeout_in_seconds) + client = ::WinRM::WinRMWebService.new(endpoint, @config.transport.to_sym, endpoint_options) + client.set_timeout(@config.timeout) client.toggle_nori_type_casting(:off) #we don't want coersion of types client end @@ -131,7 +130,14 @@ module VagrantPlugins end def endpoint - "http://#{@host}:#{@port}/wsman" + case @config.transport.to_sym + when :ssl + "https://#{@host}:#{@port}/wsman" + when :plaintext + "http://#{@host}:#{@port}/wsman" + else + raise Errors::WinRMInvalidTransport, transport: @config.transport + end end def endpoint_options @@ -139,8 +145,8 @@ module VagrantPlugins pass: @password, host: @host, port: @port, - operation_timeout: @timeout_in_seconds, - basic_auth_only: true } + basic_auth_only: true, + no_ssl_peer_verification: !@config.ssl_peer_verification } end end #WinShell class end diff --git a/plugins/kernel_v2/config/vm.rb b/plugins/kernel_v2/config/vm.rb index da2440c52..fd08f436a 100644 --- a/plugins/kernel_v2/config/vm.rb +++ b/plugins/kernel_v2/config/vm.rb @@ -399,10 +399,15 @@ module VagrantPlugins host_ip: "127.0.0.1", id: "winrm", auto_correct: true - end - end - if !@__networks["forwarded_port-ssh"] + network :forwarded_port, + guest: 5986, + host: 55986, + host_ip: "127.0.0.1", + id: "winrm-ssl", + auto_correct: true + end + elsif !@__networks["forwarded_port-ssh"] network :forwarded_port, guest: 22, host: 2222, diff --git a/test/unit/plugins/communicators/winrm/communicator_test.rb b/test/unit/plugins/communicators/winrm/communicator_test.rb index 1433f4317..c73106900 100644 --- a/test/unit/plugins/communicators/winrm/communicator_test.rb +++ b/test/unit/plugins/communicators/winrm/communicator_test.rb @@ -101,6 +101,6 @@ describe VagrantPlugins::CommunicatorWinRM::Communicator do expect(shell).to receive(:download).with("from", "to") subject.download("from", "to") end - end + end end diff --git a/test/unit/plugins/communicators/winrm/shell_test.rb b/test/unit/plugins/communicators/winrm/shell_test.rb index 477108b97..95fbb5c69 100644 --- a/test/unit/plugins/communicators/winrm/shell_test.rb +++ b/test/unit/plugins/communicators/winrm/shell_test.rb @@ -1,14 +1,23 @@ require File.expand_path("../../../../base", __FILE__) require Vagrant.source_root.join("plugins/communicators/winrm/shell") +require Vagrant.source_root.join("plugins/communicators/winrm/config") describe VagrantPlugins::CommunicatorWinRM::WinRMShell do include_context "unit" let(:session) { double("winrm_session") } + let(:port) { config.transport == :ssl ? 5986 : 5985 } + let(:config) { + VagrantPlugins::CommunicatorWinRM::Config.new.tap do |c| + c.username = 'username' + c.password = 'password' + c.finalize! + end + } subject do - described_class.new('localhost', 'username', 'password').tap do |comm| + described_class.new('localhost', port, config).tap do |comm| allow(comm).to receive(:new_session).and_return(session) end end @@ -20,7 +29,8 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do end it "should raise auth error when exception message contains 401" do - expect(session).to receive(:powershell).with(/^dir.+/).and_raise( + config.winrm.max_tries = 2 + expect(session).to receive(:powershell).with(/^dir.+/).twice.and_raise( StandardError.new("Oh no! a 401 SOAP error!")) expect { subject.powershell("dir") }.to raise_error( VagrantPlugins::CommunicatorWinRM::Errors::AuthError) @@ -42,8 +52,22 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do end describe ".endpoint" do - it "should create winrm endpoint address" do - expect(subject.send(:endpoint)).to eq("http://localhost:5985/wsman") + context 'when transport is :ssl' do + let(:config) { + VagrantPlugins::CommunicatorWinRM::Config.new.tap do |c| + c.transport = :ssl + c.finalize! + end + } + it "should create winrm endpoint address using https" do + expect(subject.send(:endpoint)).to eq("https://localhost:5986/wsman") + end + end + + context "when transport is :plaintext" do + it "should create winrm endpoint address using http" do + expect(subject.send(:endpoint)).to eq("http://localhost:5985/wsman") + end end end @@ -51,7 +75,7 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do it "should create endpoint options" do expect(subject.send(:endpoint_options)).to eq( { user: "username", pass: "password", host: "localhost", port: 5985, - operation_timeout: 60, basic_auth_only: true }) + basic_auth_only: true, no_ssl_peer_verification: false }) end end diff --git a/test/unit/plugins/kernel_v2/config/vm_test.rb b/test/unit/plugins/kernel_v2/config/vm_test.rb index 828753ad0..14f43205d 100644 --- a/test/unit/plugins/kernel_v2/config/vm_test.rb +++ b/test/unit/plugins/kernel_v2/config/vm_test.rb @@ -23,6 +23,13 @@ describe VagrantPlugins::Kernel_V2::VMConfig do end end + def find_network(name) + network_definitions = subject.networks.map do |n| + n[1] + end + network_definitions.find {|n| n[:id] == name} + end + before do env = double("env") env.stub(root_path: nil) @@ -183,6 +190,7 @@ describe VagrantPlugins::Kernel_V2::VMConfig do subject.finalize! n = subject.networks expect(n.length).to eq(2) + expect(n[0][0]).to eq(:forwarded_port) expect(n[0][1][:guest]).to eq(5985) expect(n[0][1][:host]).to eq(55985) @@ -190,9 +198,10 @@ describe VagrantPlugins::Kernel_V2::VMConfig do expect(n[0][1][:id]).to eq("winrm") expect(n[1][0]).to eq(:forwarded_port) - expect(n[1][1][:guest]).to eq(22) - expect(n[1][1][:host]).to eq(2222) - expect(n[1][1][:id]).to eq("ssh") + expect(n[1][1][:guest]).to eq(5986) + expect(n[1][1][:host]).to eq(55986) + expect(n[1][1][:host_ip]).to eq("127.0.0.1") + expect(n[1][1][:id]).to eq("winrm-ssl") end it "allows overriding SSH" do @@ -214,12 +223,22 @@ describe VagrantPlugins::Kernel_V2::VMConfig do guest: 22, host: 14100, id: "winrm" subject.finalize! - n = subject.networks - expect(n.length).to eq(2) - expect(n[0][0]).to eq(:forwarded_port) - expect(n[0][1][:guest]).to eq(22) - expect(n[0][1][:host]).to eq(14100) - expect(n[0][1][:id]).to eq("winrm") + winrm_network = find_network 'winrm' + expect(winrm_network[:guest]).to eq(22) + expect(winrm_network[:host]).to eq(14100) + expect(winrm_network[:id]).to eq("winrm") + end + + it "allows overriding WinRM SSL" do + subject.communicator = :winrmssl + subject.network "forwarded_port", + guest: 22, host: 14100, id: "winrmssl" + subject.finalize! + + winrmssl_network = find_network 'winrmssl' + expect(winrmssl_network[:guest]).to eq(22) + expect(winrmssl_network[:host]).to eq(14100) + expect(winrmssl_network[:id]).to eq("winrmssl") end it "turns all forwarded port ports to ints" do diff --git a/vagrant.gemspec b/vagrant.gemspec index 0afe7e766..b032b8f73 100644 --- a/vagrant.gemspec +++ b/vagrant.gemspec @@ -28,7 +28,7 @@ Gem::Specification.new do |s| s.add_dependency "rb-kqueue", "~> 0.2.0" s.add_dependency "rest-client", ">= 1.6.0", "< 2.0" s.add_dependency "wdm", "~> 0.1.0" - s.add_dependency "winrm", "~> 1.1.3" + s.add_dependency "winrm", "= 1.3.0.dev.2" # We lock this down to avoid compilation issues. s.add_dependency "nokogiri", "= 1.6.3.1"