Merge pull request #6612 from MsysTechnologiesllc/harry/allow_setting_environment_variables_at_the_user

[MSYS-726] Allow setting environment variables at the user level

13 files changed, 453 insertions, 407 deletions

diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb
index 5b470f574e..1ed71d2a55 100644
--- a/lib/chef/exceptions.rb
+++ b/lib/chef/exceptions.rb
@@ -45,7 +45,7 @@ class Chef
     class SigInt < RuntimeError; end
     class SigTerm < RuntimeError; end
     class Cron < RuntimeError; end
-    class Env < RuntimeError; end
+    class WindowsEnv < RuntimeError; end
     class Exec < RuntimeError; end
     class Execute < RuntimeError; end
     class ErlCall < RuntimeError; end
diff --git a/lib/chef/provider/env/windows.rb b/lib/chef/provider/env/windows.rb
deleted file mode 100644
index e813025c81..0000000000
--- a/lib/chef/provider/env/windows.rb
+++ /dev/null
@@ -1,76 +0,0 @@
-#
-# Author:: Doug MacEachern (<dougm@vmware.com>)
-# Copyright:: Copyright 2010-2016, VMware, Inc.
-# License:: Apache License, Version 2.0
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-
-require "chef/mixin/windows_env_helper"
-
-class Chef
-  class Provider
-    class Env
-      class Windows < Chef::Provider::Env
-        include Chef::Mixin::WindowsEnvHelper
-
-        provides :env, os: "windows"
-
-        def whyrun_supported?
-          false
-        end
-
-        def create_env
-          obj = env_obj(@new_resource.key_name)
-          unless obj
-            obj = WIN32OLE.connect("winmgmts://").get("Win32_Environment").spawninstance_
-            obj.name = @new_resource.key_name
-            obj.username = "<System>"
-          end
-          obj.variablevalue = @new_resource.value
-          obj.put_
-          value = @new_resource.value
-          value = expand_path(value) if @new_resource.key_name.casecmp("PATH") == 0
-          ENV[@new_resource.key_name] = value
-          broadcast_env_change
-        end
-
-        def delete_env
-          obj = env_obj(@new_resource.key_name)
-          if obj
-            obj.delete_
-            broadcast_env_change
-          end
-          if ENV[@new_resource.key_name]
-            ENV.delete(@new_resource.key_name)
-          end
-        end
-
-        def env_value(key_name)
-          obj = env_obj(key_name)
-          obj ? obj.variablevalue : ENV[key_name]
-        end
-
-        def env_obj(key_name)
-          wmi = WmiLite::Wmi.new
-          # Note that by design this query is case insensitive with regard to key_name
-          environment_variables = wmi.query("select * from Win32_Environment where name = '#{key_name}'")
-          if environment_variables && environment_variables.length > 0
-            environment_variables[0].wmi_ole_object
-          end
-        end
-
-      end
-    end
-  end
-end
diff --git a/lib/chef/provider/env.rb b/lib/chef/provider/windows_env.rb
index 490fa31146..b5b06666f3 100644
--- a/lib/chef/provider/env.rb
+++ b/lib/chef/provider/windows_env.rb
@@ -17,14 +17,17 @@
 #
 
 require "chef/provider"
-require "chef/resource/env"
+require "chef/resource/windows_env"
+require "chef/mixin/windows_env_helper"
 
 class Chef
   class Provider
-    class Env < Chef::Provider
+    class WindowsEnv < Chef::Provider
+      include Chef::Mixin::WindowsEnvHelper
       attr_accessor :key_exists
 
-      provides :env, os: "!windows"
+      provides :env, os: "windows"
+      provides :windows_env, os: "windows"
 
       def whyrun_supported?
         false
@@ -36,7 +39,7 @@ class Chef
       end
 
       def load_current_resource
-        @current_resource = Chef::Resource::Env.new(new_resource.name)
+        @current_resource = Chef::Resource::WindowsEnv.new(new_resource.name)
         current_resource.key_name(new_resource.key_name)
 
         if env_key_exists(new_resource.key_name)
@@ -49,10 +52,6 @@ class Chef
         current_resource
       end
 
-      def env_value(key_name)
-        raise Chef::Exceptions::Env, "#{self} provider does not implement env_value!"
-      end
-
       def env_key_exists(key_name)
         env_value(key_name) ? true : false
       end
@@ -123,7 +122,7 @@ class Chef
       end
 
       def action_delete
-        if @key_exists && !delete_element
+        if ( ENV[new_resource.key_name] || @key_exists ) && !delete_element
           delete_env
           Chef::Log.info("#{new_resource} deleted")
           new_resource.updated_by_last_action(true)
@@ -138,16 +137,34 @@ class Chef
             new_resource.updated_by_last_action(true)
           end
         else
-          raise Chef::Exceptions::Env, "Cannot modify #{new_resource} - key does not exist!"
+          raise Chef::Exceptions::WindowsEnv, "Cannot modify #{new_resource} - key does not exist!"
         end
       end
 
       def create_env
-        raise Chef::Exceptions::UnsupportedAction, "#{self} does not support :#{new_resource.action}"
+        obj = env_obj(@new_resource.key_name)
+        unless obj
+          obj = WIN32OLE.connect("winmgmts://").get("Win32_Environment").spawninstance_
+          obj.name = @new_resource.key_name
+          obj.username = new_resource.user
+        end
+        obj.variablevalue = @new_resource.value
+        obj.put_
+        value = @new_resource.value
+        value = expand_path(value) if @new_resource.key_name.casecmp("PATH") == 0
+        ENV[@new_resource.key_name] = value
+        broadcast_env_change
       end
 
       def delete_env
-        raise Chef::Exceptions::UnsupportedAction, "#{self} does not support :delete"
+        obj = env_obj(@new_resource.key_name)
+        if obj
+          obj.delete_
+          broadcast_env_change
+        end
+        if ENV[@new_resource.key_name]
+          ENV.delete(@new_resource.key_name)
+        end
       end
 
       def modify_env
@@ -166,6 +183,25 @@ class Chef
       def new_values
         @new_values ||= new_resource.value.split(new_resource.delim)
       end
+
+      def env_value(key_name)
+        obj = env_obj(key_name)
+        obj.variablevalue if obj
+      end
+
+      def env_obj(key_name)
+        return @env_obj if @env_obj
+        wmi = WmiLite::Wmi.new
+        # Note that by design this query is case insensitive with regard to key_name
+        environment_variables = wmi.query("select * from Win32_Environment where name = '#{key_name}'")
+        if environment_variables && environment_variables.length > 0
+          environment_variables.each do |env|
+            @env_obj = env.wmi_ole_object
+            return @env_obj if @env_obj.username.split('\\').last.casecmp(new_resource.user) == 0
+          end
+        end
+        @env_obj = nil
+      end
     end
   end
 end
diff --git a/lib/chef/providers.rb b/lib/chef/providers.rb
index 507203fd28..8b07e1b405 100644
--- a/lib/chef/providers.rb
+++ b/lib/chef/providers.rb
@@ -27,7 +27,6 @@ require "chef/provider/cron/aix"
 require "chef/provider/directory"
 require "chef/provider/dsc_script"
 require "chef/provider/dsc_resource"
-require "chef/provider/env"
 require "chef/provider/execute"
 require "chef/provider/file"
 require "chef/provider/git"
@@ -56,13 +55,12 @@ require "chef/provider/systemd_unit"
 require "chef/provider/template"
 require "chef/provider/user"
 require "chef/provider/whyrun_safe_ruby_block"
+require "chef/provider/windows_env"
 require "chef/provider/yum_repository"
 require "chef/provider/windows_task"
 require "chef/provider/zypper_repository"
 require "chef/provider/windows_path"
 
-require "chef/provider/env/windows"
-
 require "chef/provider/package/apt"
 require "chef/provider/package/chocolatey"
 require "chef/provider/package/dpkg"
diff --git a/lib/chef/resource/env.rb b/lib/chef/resource/windows_env.rb
index 12133f4368..d25fe9326b 100644
--- a/lib/chef/resource/env.rb
+++ b/lib/chef/resource/windows_env.rb
@@ -21,8 +21,9 @@ class Chef
   class Resource
     # Use the env resource to manage environment keys in Microsoft Windows. After an environment key is set, Microsoft
     # Windows must be restarted before the environment key will be available to the Task Scheduler.
-    class Env < Chef::Resource
-      resource_name :env
+    class WindowsEnv < Chef::Resource
+      resource_name :windows_env
+      provides :windows_env, os: "windows"
       provides :env, os: "windows"
 
       default_action :create
@@ -31,6 +32,7 @@ class Chef
       property :key_name, String, identity: true, name_property: true
       property :value, String, required: true
       property :delim, [ String, nil, false ], desired_state: false
+      property :user, String, default: "<System>"
     end
   end
 end
diff --git a/lib/chef/resources.rb b/lib/chef/resources.rb
index 9fee978432..b580a01c29 100644
--- a/lib/chef/resources.rb
+++ b/lib/chef/resources.rb
@@ -33,7 +33,6 @@ require "chef/resource/dpkg_package"
 require "chef/resource/dnf_package"
 require "chef/resource/dsc_script"
 require "chef/resource/dsc_resource"
-require "chef/resource/env"
 require "chef/resource/execute"
 require "chef/resource/file"
 require "chef/resource/freebsd_package"
@@ -89,6 +88,7 @@ require "chef/resource/user/pw_user"
 require "chef/resource/user/solaris_user"
 require "chef/resource/user/windows_user"
 require "chef/resource/whyrun_safe_ruby_block"
+require "chef/resource/windows_env"
 require "chef/resource/windows_package"
 require "chef/resource/yum_package"
 require "chef/resource/yum_repository"
diff --git a/spec/functional/resource/env_spec.rb b/spec/functional/resource/env_spec.rb
deleted file mode 100755
index 4b0ff70c0b..0000000000
--- a/spec/functional/resource/env_spec.rb
+++ /dev/null
@@ -1,192 +0,0 @@
-#
-# Author:: Adam Edwards (<adamed@chef.io>)
-# Copyright:: Copyright 2014-2016, Chef Software Inc.
-# License:: Apache License, Version 2.0
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-
-require "spec_helper"
-
-describe Chef::Resource::Env, :windows_only do
-  context "when running on Windows" do
-    let(:chef_env_test_lower_case) { "chefenvtest" }
-    let(:chef_env_test_mixed_case) { "chefENVtest" }
-    let(:env_dne_key) { "env_dne_key" }
-    let(:env_value1) { "value1" }
-    let(:env_value2) { "value2" }
-
-    let(:env_value_expandable) { "%SystemRoot%" }
-    let(:test_run_context) do
-      node = Chef::Node.new
-      node.default["os"] = "windows"
-      node.default["platform"] = "windows"
-      node.default["platform_version"] = "6.1"
-      empty_events = Chef::EventDispatch::Dispatcher.new
-      Chef::RunContext.new(node, {}, empty_events)
-    end
-    let(:test_resource) do
-      Chef::Resource::Env.new("unknown", test_run_context)
-    end
-
-    before(:each) do
-      resource_lower = Chef::Resource::Env.new(chef_env_test_lower_case, test_run_context)
-      resource_lower.run_action(:delete)
-      resource_mixed = Chef::Resource::Env.new(chef_env_test_mixed_case, test_run_context)
-      resource_mixed.run_action(:delete)
-    end
-
-    context "when the create action is invoked" do
-      it "should create an environment variable for action create" do
-        expect(ENV[chef_env_test_lower_case]).to eq(nil)
-        test_resource.key_name(chef_env_test_lower_case)
-        test_resource.value(env_value1)
-        test_resource.run_action(:create)
-        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
-      end
-
-      it "should modify an existing variable's value to a new value" do
-        test_resource.key_name(chef_env_test_lower_case)
-        test_resource.value(env_value1)
-        test_resource.run_action(:create)
-        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
-        test_resource.value(env_value2)
-        test_resource.run_action(:create)
-        expect(ENV[chef_env_test_lower_case]).to eq(env_value2)
-      end
-
-      it "should modify an existing variable's value to a new value if the variable name case differs from the existing variable" do
-        test_resource.key_name(chef_env_test_lower_case)
-        test_resource.value(env_value1)
-        test_resource.run_action(:create)
-        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
-        test_resource.key_name(chef_env_test_mixed_case)
-        test_resource.value(env_value2)
-        test_resource.run_action(:create)
-        expect(ENV[chef_env_test_lower_case]).to eq(env_value2)
-      end
-
-      it "should not expand environment variables if the variable is not PATH" do
-        expect(ENV[chef_env_test_lower_case]).to eq(nil)
-        test_resource.key_name(chef_env_test_lower_case)
-        test_resource.value(env_value_expandable)
-        test_resource.run_action(:create)
-        expect(ENV[chef_env_test_lower_case]).to eq(env_value_expandable)
-      end
-    end
-
-    context "when the modify action is invoked" do
-      it "should raise an exception for modify if the variable doesn't exist" do
-        expect(ENV[chef_env_test_lower_case]).to eq(nil)
-        test_resource.key_name(chef_env_test_lower_case)
-        test_resource.value(env_value1)
-        expect { test_resource.run_action(:modify) }.to raise_error(Chef::Exceptions::Env)
-      end
-
-      it "should modify an existing variable's value to a new value" do
-        test_resource.key_name(chef_env_test_lower_case)
-        test_resource.value(env_value1)
-        test_resource.run_action(:create)
-        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
-        test_resource.value(env_value2)
-        test_resource.run_action(:modify)
-        expect(ENV[chef_env_test_lower_case]).to eq(env_value2)
-      end
-
-      # This examlpe covers Chef Issue #1754
-      it "should modify an existing variable's value to a new value if the variable name case differs from the existing variable" do
-        test_resource.key_name(chef_env_test_lower_case)
-        test_resource.value(env_value1)
-        test_resource.run_action(:create)
-        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
-        test_resource.key_name(chef_env_test_mixed_case)
-        test_resource.value(env_value2)
-        test_resource.run_action(:modify)
-        expect(ENV[chef_env_test_lower_case]).to eq(env_value2)
-      end
-
-      it "should not expand environment variables if the variable is not PATH" do
-        test_resource.key_name(chef_env_test_lower_case)
-        test_resource.value(env_value1)
-        test_resource.run_action(:create)
-        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
-        test_resource.value(env_value_expandable)
-        test_resource.run_action(:modify)
-        expect(ENV[chef_env_test_lower_case]).to eq(env_value_expandable)
-      end
-
-      context "when using PATH" do
-        let(:random_name) { Time.now.to_i }
-        let(:env_val) { "#{env_value_expandable}_#{random_name}" }
-        let!(:path_before) { test_resource.provider_for_action(test_resource.action).env_value("PATH") || "" }
-        let!(:env_path_before) { ENV["PATH"] }
-
-        it "should expand PATH" do
-          expect(path_before).not_to include(env_val)
-          test_resource.key_name("PATH")
-          test_resource.value("#{path_before};#{env_val}")
-          test_resource.run_action(:create)
-          expect(ENV["PATH"]).not_to include(env_val)
-          expect(ENV["PATH"]).to include("#{random_name}")
-        end
-
-        after(:each) do
-          # cleanup so we don't flood the path
-          test_resource.key_name("PATH")
-          test_resource.value(path_before)
-          test_resource.run_action(:create)
-          ENV["PATH"] = env_path_before
-        end
-      end
-
-    end
-
-    context "when the delete action is invoked" do
-      it "should delete an environment variable" do
-        test_resource.key_name(chef_env_test_lower_case)
-        test_resource.value(env_value1)
-        test_resource.run_action(:create)
-        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
-        test_resource.run_action(:delete)
-        expect(ENV[chef_env_test_lower_case]).to eq(nil)
-      end
-
-      it "should not raise an exception when a non-existent environment variable is deleted" do
-        expect(ENV[chef_env_test_lower_case]).to eq(nil)
-        test_resource.key_name(chef_env_test_lower_case)
-        test_resource.value(env_value1)
-        expect { test_resource.run_action(:delete) }.not_to raise_error
-        expect(ENV[chef_env_test_lower_case]).to eq(nil)
-      end
-
-      it "should delete an existing variable's value to a new value if the specified variable name case differs from the existing variable" do
-        test_resource.key_name(chef_env_test_lower_case)
-        test_resource.value(env_value1)
-        test_resource.run_action(:create)
-        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
-        test_resource.key_name(chef_env_test_mixed_case)
-        test_resource.run_action(:delete)
-        expect(ENV[chef_env_test_lower_case]).to eq(nil)
-        expect(ENV[chef_env_test_mixed_case]).to eq(nil)
-      end
-
-      it "should delete a value from the current process even if it is not in the registry" do
-        expect(ENV[env_dne_key]).to eq(nil)
-        ENV[env_dne_key] = env_value1
-        test_resource.key_name(env_dne_key)
-        test_resource.run_action(:delete)
-        expect(ENV[env_dne_key]).to eq(nil)
-      end
-    end
-  end
-end
diff --git a/spec/functional/resource/windows_env_spec.rb b/spec/functional/resource/windows_env_spec.rb
new file mode 100644
index 0000000000..a6c6b39970
--- /dev/null
+++ b/spec/functional/resource/windows_env_spec.rb
@@ -0,0 +1,285 @@
+#
+# Author:: Adam Edwards (<adamed@chef.io>)
+# Copyright:: Copyright 2014-2016, Chef Software Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require "spec_helper"
+
+describe Chef::Resource::WindowsEnv, :windows_only do
+  context "when running on Windows" do
+    let(:chef_env_test_lower_case) { "chefenvtest" }
+    let(:chef_env_test_mixed_case) { "chefENVtest" }
+    let(:chef_env_with_delim) { "chef_env_with_delim" }
+    let(:chef_env_delim) { ";" }
+    let(:chef_env_test_delim) { "#{value1};#{value2}" }
+    let(:env_dne_key) { "env_dne_key" }
+    let(:env_value1) { "value1" }
+    let(:env_value2) { "value2" }
+    let(:delim_value) { "#{env_value1};#{env_value2}" }
+    let(:env_user) { ENV["USERNAME"].upcase }
+    let(:default_env_user) { "<SYSTEM>" }
+
+    let(:env_obj) do
+      wmi = WmiLite::Wmi.new
+      environment_variables = wmi.query("select * from Win32_Environment where name = '#{test_resource.key_name}'")
+      if environment_variables && environment_variables.length > 0
+        environment_variables.each do |env|
+          env_obj = env.wmi_ole_object
+          return env_obj if env_obj.username.split('\\').last.casecmp(test_resource.user) == 0
+        end
+      end
+      nil
+    end
+
+    let(:env_value_expandable) { "%SystemRoot%" }
+    let(:test_run_context) do
+      node = Chef::Node.new
+      node.default["os"] = "windows"
+      node.default["platform"] = "windows"
+      node.default["platform_version"] = "6.1"
+      empty_events = Chef::EventDispatch::Dispatcher.new
+      Chef::RunContext.new(node, {}, empty_events)
+    end
+    let(:test_resource) do
+      Chef::Resource::WindowsEnv.new("unknown", test_run_context)
+    end
+
+    before(:each) do
+      resource_lower = Chef::Resource::WindowsEnv.new(chef_env_test_lower_case, test_run_context)
+      resource_lower.run_action(:delete)
+      resource_lower = Chef::Resource::WindowsEnv.new(chef_env_test_lower_case, test_run_context)
+      resource_lower.user(env_user)
+      resource_lower.run_action(:delete)
+      resource_mixed = Chef::Resource::WindowsEnv.new(chef_env_test_mixed_case, test_run_context)
+      resource_mixed.run_action(:delete)
+      resource_mixed = Chef::Resource::WindowsEnv.new(chef_env_test_mixed_case, test_run_context)
+      resource_lower.user(env_user)
+      resource_mixed.run_action(:delete)
+    end
+
+    context "when the create action is invoked" do
+      it "should create an environment variable for action create" do
+        expect(ENV[chef_env_test_lower_case]).to eq(nil)
+        test_resource.key_name(chef_env_test_lower_case)
+        test_resource.value(env_value1)
+        test_resource.run_action(:create)
+        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+      end
+
+      it "should create an environment variable with default user System for action create" do
+        expect(ENV[chef_env_test_lower_case]).to eq(nil)
+        test_resource.key_name(chef_env_test_lower_case)
+        test_resource.value(env_value1)
+        test_resource.run_action(:create)
+        expect(env_obj.username.upcase).to eq(default_env_user)
+      end
+
+      it "should create an environment variable with user for action create" do
+        expect(ENV[chef_env_test_lower_case]).to eq(nil)
+        test_resource.key_name(chef_env_test_lower_case)
+        test_resource.value(env_value1)
+        test_resource.user(env_user)
+        test_resource.run_action(:create)
+        expect(env_obj.username.split('\\').last.upcase).to eq(env_user)
+      end
+
+      context "when env variable exist with same name" do
+        before(:each) do
+          test_resource.key_name(chef_env_test_lower_case)
+          test_resource.value(env_value1)
+          test_resource.run_action(:create)
+        end
+        it "should modify an existing variable's value to a new value" do
+          expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+          test_resource.value(env_value2)
+          test_resource.run_action(:create)
+          expect(ENV[chef_env_test_lower_case]).to eq(env_value2)
+        end
+
+        it "should not modify an existing variable's value to a new value if the users are different" do
+          expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+          test_resource.value(env_value2)
+          test_resource.user(env_user)
+          test_resource.run_action(:create)
+          test_resource.key_name(chef_env_test_lower_case)
+          test_resource.user(default_env_user)
+          expect(env_obj.variablevalue).to eq(env_value1)
+        end
+
+        it "should modify an existing variable's value to a new value if the variable name case differs from the existing variable" do
+          expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+          test_resource.key_name(chef_env_test_mixed_case)
+          test_resource.value(env_value2)
+          test_resource.run_action(:create)
+          expect(ENV[chef_env_test_lower_case]).to eq(env_value2)
+        end
+      end
+
+      it "should not expand environment variables if the variable is not PATH" do
+        expect(ENV[chef_env_test_lower_case]).to eq(nil)
+        test_resource.key_name(chef_env_test_lower_case)
+        test_resource.value(env_value_expandable)
+        test_resource.run_action(:create)
+        expect(ENV[chef_env_test_lower_case]).to eq(env_value_expandable)
+      end
+    end
+
+    context "when the modify action is invoked" do
+      it "should raise an exception for modify if the variable doesn't exist" do
+        expect(ENV[chef_env_test_lower_case]).to eq(nil)
+        test_resource.key_name(chef_env_test_lower_case)
+        test_resource.value(env_value1)
+        expect { test_resource.run_action(:modify) }.to raise_error(Chef::Exceptions::WindowsEnv)
+      end
+
+      context "when env variable exist with same name" do
+        before(:each) do
+          test_resource.key_name(chef_env_test_lower_case)
+          test_resource.value(env_value1)
+          test_resource.run_action(:create)
+        end
+
+        it "should modify an existing variable's value to a new value" do
+          expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+          test_resource.value(env_value2)
+          test_resource.run_action(:modify)
+          expect(ENV[chef_env_test_lower_case]).to eq(env_value2)
+        end
+
+        # This examlpe covers Chef Issue #1754
+        it "should modify an existing variable's value to a new value if the variable name case differs from the existing variable" do
+          expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+          test_resource.key_name(chef_env_test_mixed_case)
+          test_resource.value(env_value2)
+          test_resource.run_action(:modify)
+          expect(ENV[chef_env_test_lower_case]).to eq(env_value2)
+        end
+
+        it "should not expand environment variables if the variable is not PATH" do
+          expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+          test_resource.value(env_value_expandable)
+          test_resource.run_action(:modify)
+          expect(ENV[chef_env_test_lower_case]).to eq(env_value_expandable)
+        end
+      end
+
+      context "when using PATH" do
+        let(:random_name) { Time.now.to_i }
+        let(:env_val) { "#{env_value_expandable}_#{random_name}" }
+        let!(:path_before) { test_resource.provider_for_action(test_resource.action).env_value("PATH") || "" }
+        let!(:env_path_before) { ENV["PATH"] }
+
+        it "should expand PATH" do
+          expect(path_before).not_to include(env_val)
+          test_resource.key_name("PATH")
+          test_resource.value("#{path_before};#{env_val}")
+          test_resource.run_action(:create)
+          expect(ENV["PATH"]).not_to include(env_val)
+          expect(ENV["PATH"]).to include("#{random_name}")
+        end
+
+        after(:each) do
+          # cleanup so we don't flood the path
+          test_resource.key_name("PATH")
+          test_resource.value(path_before)
+          test_resource.run_action(:create)
+          ENV["PATH"] = env_path_before
+        end
+      end
+
+    end
+
+    context "when the delete action is invoked" do
+      context "when env variable exist with same name" do
+        before(:each) do
+          test_resource.key_name(chef_env_test_lower_case)
+          test_resource.value(env_value1)
+          test_resource.run_action(:create)
+        end
+
+        it "should delete a System environment variable" do
+          expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+          test_resource.run_action(:delete)
+          expect(ENV[chef_env_test_lower_case]).to eq(nil)
+        end
+
+        it "should not delete an System environment variable if user are passed" do
+          expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+          test_resource.user(env_user)
+          test_resource.run_action(:delete)
+          test_resource.user(default_env_user)
+          expect(env_obj).not_to be_nil
+        end
+      end
+
+      context "when env variable exist with same name" do
+        before(:each) do
+          test_resource.key_name(chef_env_test_lower_case)
+          test_resource.value(env_value1)
+          test_resource.user(env_user)
+          test_resource.run_action(:create)
+        end
+
+        it "should delete a user environment variable" do
+          expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+          test_resource.run_action(:delete)
+          expect(env_obj).to eq(nil)
+        end
+
+        it "should not delete an user environment variable if user is not passed" do
+          expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+          test_resource.user(default_env_user)
+          test_resource.run_action(:delete)
+          test_resource.user(env_user)
+          expect(env_obj).not_to be_nil
+        end
+      end
+
+      context "when env variable exist with same name" do
+        before(:each) do
+          test_resource.key_name(chef_env_with_delim)
+          test_resource.delim(chef_env_delim)
+          test_resource.value(delim_value)
+          test_resource.run_action(:create)
+        end
+
+        it "should not delete variable when a delim present" do
+          expect(ENV[chef_env_with_delim]).to eq(delim_value)
+          test_resource.value(env_value1)
+          test_resource.run_action(:delete)
+          expect(ENV[chef_env_with_delim]).to eq(env_value2)
+        end
+      end
+
+      it "should not raise an exception when a non-existent environment variable is deleted" do
+        expect(ENV[chef_env_test_lower_case]).to eq(nil)
+        test_resource.key_name(chef_env_test_lower_case)
+        test_resource.value(env_value1)
+        expect { test_resource.run_action(:delete) }.not_to raise_error
+        expect(ENV[chef_env_test_lower_case]).to eq(nil)
+      end
+
+      it "should delete a value from the current process even if it is not in the registry" do
+        expect(ENV[env_dne_key]).to eq(nil)
+        ENV[env_dne_key] = env_value1
+        test_resource.key_name(env_dne_key)
+        test_resource.run_action(:delete)
+        expect(ENV[env_dne_key]).to eq(nil)
+      end
+
+    end
+  end
+end
diff --git a/spec/unit/exceptions_spec.rb b/spec/unit/exceptions_spec.rb
index e952a5448a..c892b24f28 100644
--- a/spec/unit/exceptions_spec.rb
+++ b/spec/unit/exceptions_spec.rb
@@ -25,7 +25,7 @@ describe Chef::Exceptions do
   exception_to_super_class = {
     Chef::Exceptions::Application => RuntimeError,
     Chef::Exceptions::Cron => RuntimeError,
-    Chef::Exceptions::Env => RuntimeError,
+    Chef::Exceptions::WindowsEnv => RuntimeError,
     Chef::Exceptions::Exec => RuntimeError,
     Chef::Exceptions::FileNotFound => RuntimeError,
     Chef::Exceptions::Package => RuntimeError,
diff --git a/spec/unit/provider/env/windows_spec.rb b/spec/unit/provider/env/windows_spec.rb
deleted file mode 100644
index 5ddc1d6f91..0000000000
--- a/spec/unit/provider/env/windows_spec.rb
+++ /dev/null
@@ -1,103 +0,0 @@
-#
-# Author:: Sander van Harmelen <svanharmelen@schubergphilis.com>
-# Copyright:: Copyright 2014-2016, Chef Software, Inc.
-# License:: Apache License, Version 2.0
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-
-require "spec_helper"
-
-describe Chef::Provider::Env::Windows, :windows_only do
-  let(:node) { Chef::Node.new }
-  let(:events) { Chef::EventDispatch::Dispatcher.new }
-  let(:run_context) { Chef::RunContext.new(node, {}, events) }
-
-  context "when environment variable is not PATH" do
-    let(:new_resource) do
-      new_resource = Chef::Resource::Env.new("CHEF_WINDOWS_ENV_TEST")
-      new_resource.value("foo")
-      new_resource
-    end
-    let(:provider) do
-      provider = Chef::Provider::Env::Windows.new(new_resource, run_context)
-      allow(provider).to receive(:env_obj).and_return(double("null object").as_null_object)
-      provider
-    end
-
-    describe "action_create" do
-      before do
-        ENV.delete("CHEF_WINDOWS_ENV_TEST")
-        provider.key_exists = false
-      end
-
-      it "should update the ruby ENV object when it creates the key" do
-        provider.action_create
-        expect(ENV["CHEF_WINDOWS_ENV_TEST"]).to eql("foo")
-      end
-    end
-
-    describe "action_modify" do
-      before do
-        ENV["CHEF_WINDOWS_ENV_TEST"] = "foo"
-      end
-
-      it "should update the ruby ENV object when it updates the value" do
-        expect(provider).to receive(:requires_modify_or_create?).and_return(true)
-        new_resource.value("foobar")
-        provider.action_modify
-        expect(ENV["CHEF_WINDOWS_ENV_TEST"]).to eql("foobar")
-      end
-
-      describe "action_delete" do
-        before do
-          ENV["CHEF_WINDOWS_ENV_TEST"] = "foo"
-        end
-
-        it "should update the ruby ENV object when it deletes the key" do
-          provider.action_delete
-          expect(ENV["CHEF_WINDOWS_ENV_TEST"]).to eql(nil)
-        end
-      end
-    end
-  end
-
-  context "when environment is PATH" do
-    describe "for PATH" do
-      let(:system_root) { "%SystemRoot%" }
-      let(:system_root_value) { 'D:\Windows' }
-      let(:new_resource) do
-        new_resource = Chef::Resource::Env.new("PATH")
-        new_resource.value(system_root)
-        new_resource
-      end
-      let(:provider) do
-        provider = Chef::Provider::Env::Windows.new(new_resource, run_context)
-        allow(provider).to receive(:env_obj).and_return(double("null object").as_null_object)
-        provider
-      end
-
-      before do
-        stub_const("ENV", { "PATH" => "" })
-      end
-
-      it "replaces Windows system variables" do
-        expect(provider).to receive(:requires_modify_or_create?).and_return(true)
-        expect(provider).to receive(:expand_path).with(system_root).and_return(system_root_value)
-        provider.action_modify
-        expect(ENV["PATH"]).to eql(system_root_value)
-      end
-    end
-
-  end
-end
diff --git a/spec/unit/provider/env_spec.rb b/spec/unit/provider/windows_env_spec.rb
index fd52c5a8ed..d648314dc1 100644
--- a/spec/unit/provider/env_spec.rb
+++ b/spec/unit/provider/windows_env_spec.rb
@@ -18,15 +18,16 @@
 
 require "spec_helper"
 
-describe Chef::Provider::Env do
+describe Chef::Provider::WindowsEnv do
 
   before do
     @node = Chef::Node.new
     @events = Chef::EventDispatch::Dispatcher.new
     @run_context = Chef::RunContext.new(@node, {}, @events)
-    @new_resource = Chef::Resource::Env.new("FOO")
+    @new_resource = Chef::Resource::WindowsEnv.new("FOO")
     @new_resource.value("bar")
-    @provider = Chef::Provider::Env.new(@new_resource, @run_context)
+    @new_resource.user("<System>")
+    @provider = Chef::Provider::WindowsEnv.new(@new_resource, @run_context)
   end
 
   it "assumes the key_name exists by default" do
@@ -47,12 +48,17 @@ describe Chef::Provider::Env do
       expect(@provider.new_resource.name).to eq("FOO")
     end
 
+    it "should create a current resource with the same user as the new resource" do
+      @provider.load_current_resource
+      expect(@provider.new_resource.user).to eq("<System>")
+    end
+
     it "should set the key_name to the key name of the new resource" do
       @provider.load_current_resource
       expect(@provider.current_resource.key_name).to eq("FOO")
     end
 
-    it "should check if the key_name exists" do
+    it "should check if the key_name and user exist" do
       expect(@provider).to receive(:env_key_exists).with("FOO").and_return(true)
       @provider.load_current_resource
       expect(@provider.key_exists).to be_truthy
@@ -65,7 +71,7 @@ describe Chef::Provider::Env do
     end
 
     it "should return the current resource" do
-      expect(@provider.load_current_resource).to be_a_kind_of(Chef::Resource::Env)
+      expect(@provider.load_current_resource).to be_a_kind_of(Chef::Resource::WindowsEnv)
     end
   end
 
@@ -76,7 +82,7 @@ describe Chef::Provider::Env do
       allow(@provider).to receive(:modify_env).and_return(true)
     end
 
-    it "should call create_env if the key does not exist" do
+    it "should call create_env if the key does not exist with user" do
       expect(@provider).to receive(:create_env).and_return(true)
       @provider.action_create
     end
@@ -92,7 +98,7 @@ describe Chef::Provider::Env do
       @provider.action_create
     end
 
-    it "should call modify_env if the key exists and values are not equal" do
+    it "should call modify_env if the key exists with provided user and values are not equal" do
       @provider.key_exists = true
       allow(@provider).to receive(:requires_modify_or_create?).and_return(true)
       expect(@provider).to receive(:modify_env).and_return(true)
@@ -152,6 +158,12 @@ describe Chef::Provider::Env do
       @provider.action_modify
     end
 
+    it "should call modify_group if the key exists and users are not equal" do
+      expect(@provider).to receive(:requires_modify_or_create?).and_return(true)
+      expect(@provider).to receive(:modify_env).and_return(true)
+      @provider.action_modify
+    end
+
     it "should set the new resources updated flag to true if modify_env is called" do
       allow(@provider).to receive(:requires_modify_or_create?).and_return(true)
       allow(@provider).to receive(:modify_env).and_return(true)
@@ -159,25 +171,26 @@ describe Chef::Provider::Env do
       expect(@new_resource).to be_updated
     end
 
-    it "should not call modify_env if the key exists but the values are equal" do
+    it "should not call modify_env if the key exists with user but the values are equal" do
       expect(@provider).to receive(:requires_modify_or_create?).and_return(false)
       expect(@provider).not_to receive(:modify_env)
       @provider.action_modify
     end
 
-    it "should raise a Chef::Exceptions::Env if the key doesn't exist" do
+    it "should raise a Chef::Exceptions::WindowsEnv if the key doesn't exist" do
       @provider.key_exists = false
-      expect { @provider.action_modify }.to raise_error(Chef::Exceptions::Env)
+      expect { @provider.action_modify }.to raise_error(Chef::Exceptions::WindowsEnv)
     end
   end
 
   describe "delete_element" do
     before(:each) do
-      @current_resource = Chef::Resource::Env.new("FOO")
+      @current_resource = Chef::Resource::WindowsEnv.new("FOO")
 
       @new_resource.delim ";"
       @new_resource.value "C:/bar/bin"
 
+      @current_resource.user "<System>"
       @current_resource.value "C:/foo/bin;C:/bar/bin"
       @provider.current_resource = @current_resource
     end
@@ -280,7 +293,7 @@ describe Chef::Provider::Env do
       allow(@provider).to receive(:create_env).and_return(true)
       @new_resource.delim ";"
 
-      @current_resource = Chef::Resource::Env.new("FOO")
+      @current_resource = Chef::Resource::WindowsEnv.new("FOO")
       @current_resource.value "C:/foo/bin"
       @provider.current_resource = @current_resource
     end
@@ -307,4 +320,81 @@ describe Chef::Provider::Env do
       expect(@new_resource.value).to eq("C:/foo;C:/bar;C:/baz;C:/foo/bar")
     end
   end
+
+  context "when environment variable is not PATH" do
+    let(:new_resource) do
+      new_resource = Chef::Resource::WindowsEnv.new("CHEF_WINDOWS_ENV_TEST")
+      new_resource.value("foo")
+      new_resource
+    end
+    let(:provider) do
+      provider = Chef::Provider::WindowsEnv.new(new_resource, run_context)
+      allow(provider).to receive(:env_obj).and_return(double("null object").as_null_object)
+      provider
+    end
+
+    describe "action_create" do
+      before do
+        ENV.delete("CHEF_WINDOWS_ENV_TEST")
+        provider.key_exists = false
+      end
+
+      it "should update the ruby ENV object when it creates the key" do
+        provider.action_create
+        expect(ENV["CHEF_WINDOWS_ENV_TEST"]).to eql("foo")
+      end
+    end
+
+    describe "action_modify" do
+      before do
+        ENV["CHEF_WINDOWS_ENV_TEST"] = "foo"
+      end
+
+      it "should update the ruby ENV object when it updates the value" do
+        expect(provider).to receive(:requires_modify_or_create?).and_return(true)
+        new_resource.value("foobar")
+        provider.action_modify
+        expect(ENV["CHEF_WINDOWS_ENV_TEST"]).to eql("foobar")
+      end
+
+      describe "action_delete" do
+        before do
+          ENV["CHEF_WINDOWS_ENV_TEST"] = "foo"
+        end
+
+        it "should update the ruby ENV object when it deletes the key" do
+          provider.action_delete
+          expect(ENV["CHEF_WINDOWS_ENV_TEST"]).to eql(nil)
+        end
+      end
+    end
+  end
+
+  context "when environment is PATH" do
+    describe "for PATH" do
+      let(:system_root) { "%SystemRoot%" }
+      let(:system_root_value) { 'D:\Windows' }
+      let(:new_resource) do
+        new_resource = Chef::Resource::WindowsEnv.new("PATH")
+        new_resource.value(system_root)
+        new_resource
+      end
+      let(:provider) do
+        provider = Chef::Provider::WindowsEnv.new(new_resource, run_context)
+        allow(provider).to receive(:env_obj).and_return(double("null object").as_null_object)
+        provider
+      end
+
+      before do
+        stub_const("ENV", { "PATH" => "" })
+      end
+
+      it "replaces Windows system variables" do
+        expect(provider).to receive(:requires_modify_or_create?).and_return(true)
+        expect(provider).to receive(:expand_path).with(system_root).and_return(system_root_value)
+        provider.action_modify
+        expect(ENV["PATH"]).to eql(system_root_value)
+      end
+    end
+  end
 end
diff --git a/spec/unit/provider_resolver_spec.rb b/spec/unit/provider_resolver_spec.rb
index a331093055..ac641de43a 100644
--- a/spec/unit/provider_resolver_spec.rb
+++ b/spec/unit/provider_resolver_spec.rb
@@ -762,7 +762,7 @@ describe Chef::ProviderResolver do
         "windows" => {
           batch:             [ Chef::Resource::Batch, Chef::Provider::Batch ],
           dsc_script:        [ Chef::Resource::DscScript, Chef::Provider::DscScript ],
-          env:               [ Chef::Resource::Env, Chef::Provider::Env::Windows ],
+          windows_env:       [ Chef::Resource::WindowsEnv, Chef::Provider::WindowsEnv ],
           group:             [ Chef::Resource::Group, Chef::Provider::Group::Windows ],
           mount:             [ Chef::Resource::Mount, Chef::Provider::Mount::Windows ],
           package:           [ Chef::Resource::WindowsPackage, Chef::Provider::Package::Windows ],
diff --git a/spec/unit/resource/env_spec.rb b/spec/unit/resource/windows_env_spec.rb
index 61c40e33e8..776e4f7cd5 100644
--- a/spec/unit/resource/env_spec.rb
+++ b/spec/unit/resource/windows_env_spec.rb
@@ -19,10 +19,16 @@
 
 require "spec_helper"
 
-describe Chef::Resource::Env do
-  let(:resource) { Chef::Resource::Env.new("FOO") }
+describe Chef::Resource::WindowsEnv do
 
-  it "has a name property" do
+  let(:resource) { Chef::Resource::WindowsEnv.new("FOO") }
+
+  it "creates a new Chef::Resource::WindowsEnv" do
+    expect(resource).to be_a_kind_of(Chef::Resource)
+    expect(resource).to be_a_kind_of(Chef::Resource::WindowsEnv)
+  end
+
+  it "has a name" do
     expect(resource.name).to eql("FOO")
   end