Merge branch 'CHEF-3557-unix'

21 files changed, 613 insertions, 180 deletions

diff --git a/lib/chef/mixin/securable.rb b/lib/chef/mixin/securable.rb
index 47c388b239..4ab343df8f 100644
--- a/lib/chef/mixin/securable.rb
+++ b/lib/chef/mixin/securable.rb
@@ -59,9 +59,17 @@ class Chef
         )
       end
 
-      # TODO should this be separated into different files?
-      if RUBY_PLATFORM =~ /mswin|mingw|windows/
 
+      #==WindowsMacros
+      # Defines methods for adding attributes to a chef resource to describe
+      # Windows file security metadata.
+      #
+      # This module is meant to be used to extend a class (instead of
+      # `include`-ing). A class is automatically extended with this module when
+      # it includes WindowsSecurableAttributes.
+      # --
+      # TODO should this be separated into different files?
+      module WindowsMacros
         # === rights_attribute
         # "meta-method" for dynamically creating rights attributes on resources.
         #
@@ -99,7 +107,7 @@ class Chef
         # * `:applies_to_self` (optional): Boolean
         # * `:one_level_deep` (optional): Boolean
         #
-        def self.rights_attribute(name)
+        def rights_attribute(name)
 
           # equivalent to something like:
           # def rights(permissions=nil, principals=nil, args_hash=nil)
@@ -160,10 +168,24 @@ class Chef
             )
           end
         end
+      end
 
-        # create a default 'rights' attribute
-        rights_attribute(:rights)
-        rights_attribute(:deny_rights)
+      #==WindowsSecurableAttributes
+      # Defines methods #rights, #deny_rights, and #inherits to describe
+      # Windows file security ACLs on the including class. The including class
+      # is also extended with WindowsMacros, so more rights attributes may be
+      # defined.
+      module WindowsSecurableAttributes
+
+        # Callback that fires when included; will extend the including class
+        # with WindowsMacros and define #rights and #deny_rights on it.
+        def self.included(including_class)
+          including_class.extend(WindowsMacros)
+
+          # create a default 'rights' attribute
+          including_class.rights_attribute(:rights)
+          including_class.rights_attribute(:deny_rights)
+        end
 
         def inherits(arg=nil)
           set_or_return(
@@ -172,7 +194,10 @@ class Chef
             :kind_of => [ TrueClass, FalseClass ]
           )
         end
+      end
 
+      if RUBY_PLATFORM =~ /mswin|mingw|windows/
+        include WindowsSecurableAttributes
       end # Windows-specific
 
     end
diff --git a/lib/chef/provider/cookbook_file.rb b/lib/chef/provider/cookbook_file.rb
index c9e9f5fa56..46a4935844 100644
--- a/lib/chef/provider/cookbook_file.rb
+++ b/lib/chef/provider/cookbook_file.rb
@@ -48,6 +48,7 @@ class Chef
               tempfile.close
               FileUtils.cp(file_cache_location, tempfile.path)
             end
+            update_new_file_state
             Chef::Log.info("#{@new_resource} created file #{@new_resource.path}")
           end
         else
diff --git a/lib/chef/provider/directory.rb b/lib/chef/provider/directory.rb
index 37dc03c9fb..8fdc070c77 100644
--- a/lib/chef/provider/directory.rb
+++ b/lib/chef/provider/directory.rb
@@ -36,7 +36,6 @@ class Chef
       def load_current_resource
         @current_resource = Chef::Resource::Directory.new(@new_resource.name)
         @current_resource.path(@new_resource.path)
-        load_current_resource_attrs
         setup_acl
 
         @current_resource
@@ -111,6 +110,7 @@ class Chef
           end 
         end
         set_all_access_controls
+        update_new_file_state
       end
 
       def action_delete
diff --git a/lib/chef/provider/file.rb b/lib/chef/provider/file.rb
index 2928a36f5c..26c1618b60 100644
--- a/lib/chef/provider/file.rb
+++ b/lib/chef/provider/file.rb
@@ -138,41 +138,15 @@ class Chef
             end
           end
         end
-        load_current_resource_attrs
         setup_acl
-        
+
         @current_resource
       end
 
-      def load_current_resource_attrs
-        if Chef::Platform.windows?
-          # TODO: To work around CHEF-3554, add support for Windows
-          # equivalent, or implicit resource reporting won't work for
-          # Windows.
-          return
-        end
-
-        if ::File.exist?(@new_resource.path)
-          stat = ::File.stat(@new_resource.path)
-          @current_resource.owner(stat.uid)
-          @current_resource.mode(stat.mode & 07777)
-          @current_resource.group(stat.gid)
-
-          if @new_resource.group.nil?
-            @new_resource.group(@current_resource.group)
-          end 
-          if @new_resource.owner.nil?
-            @new_resource.owner(@current_resource.owner)
-          end
-          if @new_resource.mode.nil?
-            @new_resource.mode(@current_resource.mode)
-          end
-        end
-      end
-      
       def setup_acl
-        @acl_scanner = ScanAccessControl.new(@new_resource, @current_resource)
-        @acl_scanner.set_all!
+        return if Chef::Platform.windows?
+        acl_scanner = ScanAccessControl.new(@new_resource, @current_resource)
+        acl_scanner.set_all!
       end
 
       def define_resource_requirements
@@ -236,10 +210,8 @@ class Chef
           return
         end
 
-        stat = ::File.stat(path)
-        @new_resource.owner(stat.uid)
-        @new_resource.mode(stat.mode & 07777)
-        @new_resource.group(stat.gid)
+        acl_scanner = ScanAccessControl.new(@new_resource, @new_resource)
+        acl_scanner.set_all!
       end
 
       def action_create
@@ -249,7 +221,7 @@ class Chef
           desc << " with content checksum #{short_cksum(new_resource_content_checksum)}" if new_resource.content
           description << desc
           description << diff_current_from_content(@new_resource.content) 
-          
+
           converge_by(description) do
             Chef::Log.info("entered create")
             ::File.open(@new_resource.path, "w+") {|f| f.write @new_resource.content }
diff --git a/lib/chef/provider/remote_file.rb b/lib/chef/provider/remote_file.rb
index 62db2cd7dd..f985682441 100644
--- a/lib/chef/provider/remote_file.rb
+++ b/lib/chef/provider/remote_file.rb
@@ -73,6 +73,7 @@ class Chef
           end
         end
         set_all_access_controls
+        update_new_file_state
       end
 
       def current_resource_matches_target_checksum?
diff --git a/lib/chef/provider/template.rb b/lib/chef/provider/template.rb
index 4dcf23cb4e..fbd5c600b9 100644
--- a/lib/chef/provider/template.rb
+++ b/lib/chef/provider/template.rb
@@ -51,14 +51,17 @@ class Chef
       def action_create
         render_with_context(template_location) do |rendered_template|
           rendered(rendered_template)
-          update = ::File.exist?(@new_resource.path)
-          if update && content_matches?
+          if file_already_exists? && content_matches?
             Chef::Log.debug("#{@new_resource} content has not changed.")
             set_all_access_controls
+            update_new_file_state(@new_resource.path)
           else
-            description = [] 
-            action_message = update ? "update #{@current_resource} from #{short_cksum(@current_resource.checksum)} to #{short_cksum(@new_resource.checksum)}" :
+            description = []
+            action_message = if file_already_exists?
+              "update #{@current_resource} from #{short_cksum(@current_resource.checksum)} to #{short_cksum(@new_resource.checksum)}"
+            else
               "create #{@new_resource}"
+            end
             description << action_message
             description << diff_current(rendered_template.path)
             converge_by(description) do
@@ -66,17 +69,10 @@ class Chef
               FileUtils.mv(rendered_template.path, @new_resource.path)
               Chef::Log.info("#{@new_resource} updated content")
               access_controls.set_all!
-              stat = ::File.stat(@new_resource.path)
-
-              # template depends on the checksum not changing, and updates it
-              # itself later in the code, so we cannot set it here, as we do with
-              # all other < File child provider classes
-              @new_resource.owner(stat.uid)
-              @new_resource.mode(stat.mode & 07777)
-              @new_resource.group(stat.gid)
+              update_new_file_state(@new_resource.path)
             end
           end
-        end  
+        end
       end
 
       def template_finder
@@ -107,6 +103,10 @@ class Chef
 
       private
 
+      def file_already_exists?
+        ::File.exist?(@new_resource.path)
+      end
+
       def render_with_context(template_location, &block)
         context = {}
         context.merge!(@new_resource.variables)
diff --git a/lib/chef/resource/remote_directory.rb b/lib/chef/resource/remote_directory.rb
index 490e3c6ba7..0f7e0eb5de 100644
--- a/lib/chef/resource/remote_directory.rb
+++ b/lib/chef/resource/remote_directory.rb
@@ -51,6 +51,12 @@ class Chef
         @provider = Chef::Provider::RemoteDirectory
       end
 
+      if Chef::Platform.windows?
+        # create a second instance of the 'rights' attribute
+        rights_attribute(:files_rights)
+      end
+
+
       def source(args=nil)
         set_or_return(
           :source,
@@ -83,11 +89,6 @@ class Chef
         )
       end
 
-      if Chef::Platform.windows?
-        # create a second instance of the 'rights' attribute
-        Chef::Mixin::Securable.rights_attribute(:files_rights)
-      end
-
       def files_mode(arg=nil)
         set_or_return(
           :files_mode,
diff --git a/lib/chef/scan_access_control.rb b/lib/chef/scan_access_control.rb
index 5863a8c7c4..1384656a8b 100644
--- a/lib/chef/scan_access_control.rb
+++ b/lib/chef/scan_access_control.rb
@@ -118,10 +118,8 @@ class Chef
 
     def current_mode
       case new_resource.mode
-      when String, nil
-        (stat.mode & 007777).to_s(8)
-      when Integer
-        stat.mode & 007777
+      when String, Integer, nil
+        "0#{(stat.mode & 07777).to_s(8)}"
       else
         Chef::Log.error("The `mode` parameter of the #@new_resource resource is set to an invalid value (#{new_resource.mode.inspect})")
         raise ArgumentError, "Invalid value #{new_resource.mode.inspect} for `mode` on resource #@new_resource"
diff --git a/spec/functional/resource/cookbook_file_spec.rb b/spec/functional/resource/cookbook_file_spec.rb
index b2bee5d95b..d88dafec0c 100644
--- a/spec/functional/resource/cookbook_file_spec.rb
+++ b/spec/functional/resource/cookbook_file_spec.rb
@@ -32,6 +32,10 @@ describe Chef::Resource::CookbookFile do
     content
   end
 
+  let(:default_mode) { "600" }
+
+  it_behaves_like "a securable resource with reporting"
+
   def create_resource
     # set up cookbook collection for this run to use, based on our
     # spec data.
diff --git a/spec/functional/resource/directory_spec.rb b/spec/functional/resource/directory_spec.rb
index 5777a114fc..9ae0503336 100644
--- a/spec/functional/resource/directory_spec.rb
+++ b/spec/functional/resource/directory_spec.rb
@@ -23,6 +23,8 @@ describe Chef::Resource::Directory do
 
   let(:directory_base) { "directory_spec" }
 
+  let(:default_mode) { "755" }
+
   def create_resource
     events = Chef::EventDispatch::Dispatcher.new
     node = Chef::Node.new
@@ -36,4 +38,6 @@ describe Chef::Resource::Directory do
 
   it_behaves_like "a directory resource"
 
+  it_behaves_like "a securable resource with reporting"
+
 end
diff --git a/spec/functional/resource/file_spec.rb b/spec/functional/resource/file_spec.rb
index 8e481d9fc4..2c18d07520 100644
--- a/spec/functional/resource/file_spec.rb
+++ b/spec/functional/resource/file_spec.rb
@@ -46,8 +46,18 @@ describe Chef::Resource::File do
     "This is file content that is not managed by chef"
   end
 
+  let(:current_resource) do
+    provider = resource.provider_for_action(resource.action)
+    provider.load_current_resource
+    provider.current_resource
+  end
+
+  let(:default_mode) { ((0100666 - File.umask) & 07777).to_s(8) }
+
   it_behaves_like "a file resource"
 
+  it_behaves_like "a securable resource with reporting"
+
   describe "when running action :touch" do
     context "and the target file does not exist" do
       before do
diff --git a/spec/functional/resource/remote_directory_spec.rb b/spec/functional/resource/remote_directory_spec.rb
index 49e84ce12d..b4e26a59b2 100644
--- a/spec/functional/resource/remote_directory_spec.rb
+++ b/spec/functional/resource/remote_directory_spec.rb
@@ -22,6 +22,7 @@ describe Chef::Resource::RemoteDirectory do
   include_context Chef::Resource::Directory
 
   let(:directory_base) { "directory_spec" }
+  let(:default_mode) { "755" }
 
   def create_resource
     cookbook_repo = File.expand_path(File.join(CHEF_SPEC_DATA, "cookbooks"))
@@ -69,6 +70,8 @@ describe Chef::Resource::RemoteDirectory do
 
   it_behaves_like "a directory resource"
 
+  it_behaves_like "a securable resource with reporting"
+
   context "when creating the remote directory with purging disabled" do
 
     context "and the directory does not yet exist" do
diff --git a/spec/functional/resource/remote_file_spec.rb b/spec/functional/resource/remote_file_spec.rb
index 998255e720..619283d632 100644
--- a/spec/functional/resource/remote_file_spec.rb
+++ b/spec/functional/resource/remote_file_spec.rb
@@ -45,6 +45,8 @@ describe Chef::Resource::RemoteFile do
     create_resource
   end
 
+  let(:default_mode) { "600" }
+
   before(:all) do
     @server = TinyServer::Manager.new
     @server.start
@@ -62,4 +64,6 @@ describe Chef::Resource::RemoteFile do
   end
 
   it_behaves_like "a file resource"
+
+  it_behaves_like "a securable resource with reporting"
 end
diff --git a/spec/functional/resource/template_spec.rb b/spec/functional/resource/template_spec.rb
index 0dfdb17324..87a8a3db66 100644
--- a/spec/functional/resource/template_spec.rb
+++ b/spec/functional/resource/template_spec.rb
@@ -49,8 +49,12 @@ describe Chef::Resource::Template do
     create_resource
   end
 
+  let(:default_mode) { "600" }
+
   it_behaves_like "a file resource"
 
+  it_behaves_like "a securable resource with reporting"
+
   context "when the target file does not exist" do
     it "creates the template with the rendered content using the variable attribute when the :create action is run" do
       resource.source('openldap_variable_stuff.conf.erb')
diff --git a/spec/support/shared/functional/file_resource.rb b/spec/support/shared/functional/file_resource.rb
index 0d94329bfd..a0342cb6d7 100644
--- a/spec/support/shared/functional/file_resource.rb
+++ b/spec/support/shared/functional/file_resource.rb
@@ -143,6 +143,9 @@ shared_examples_for "a file with the correct content" do
 end
 
 shared_examples_for "a file resource" do
+  before do
+    Chef::Log.level = :info
+  end
    # note the stripping of the drive letter from the tmpdir on windows
   let(:backup_glob) { File.join(CHEF_SPEC_BACKUP_PATH, Dir.tmpdir.sub(/^([A-Za-z]:)/, ""), "#{file_base}*") }
 
diff --git a/spec/support/shared/functional/securable_resource_with_reporting.rb b/spec/support/shared/functional/securable_resource_with_reporting.rb
new file mode 100644
index 0000000000..60ab1e77db
--- /dev/null
+++ b/spec/support/shared/functional/securable_resource_with_reporting.rb
@@ -0,0 +1,371 @@
+
+shared_examples_for "a securable resource with reporting" do
+
+  let(:current_resource) do
+    provider = resource.provider_for_action(resource.action)
+    provider.load_current_resource
+    provider.current_resource
+  end
+
+  # Default mode varies based on implementation. Providers that use a tempfile
+  # will default to 0600. Providers that use File.open will default to 0666 -
+  # umask
+  # let(:default_mode) { ((0100666 - File.umask) & 07777).to_s(8) }
+
+  describe "reading file security metadata for reporting on unix", :unix_only => true do
+    context "when the target file doesn't exist" do
+      before do
+        resource.action(:create)
+      end
+
+      it "has empty values for file metadata in 'current_resource'" do
+        current_resource.owner.should be_nil
+        current_resource.group.should be_nil
+        current_resource.mode.should be_nil
+      end
+
+      context "and no security metadata is specified in new_resource" do
+        it "sets the metadata values on the new_resource as strings after creating" do
+          resource.run_action(:create)
+          # TODO: most stable way to specify?
+          resource.owner.should == Etc.getpwuid(Process.uid).name
+          resource.group.should == Etc.getgrgid(Process.gid).name
+          resource.mode.should == "0#{default_mode}"
+        end
+      end
+
+      context "and owner is specified with a String (username) in new_resource", :requires_root => true do
+
+        # TODO/bug: duplicated from the "securable resource" tests
+        let(:expected_user_name) { 'nobody' }
+
+        before do
+          resource.owner(expected_user_name)
+          resource.run_action(:create)
+        end
+
+        it "sets the owner on new_resource to the username (String) of the desired owner" do
+          resource.owner.should == expected_user_name
+        end
+
+      end
+
+      context "and owner is specified with an Integer (uid) in new_resource", :requires_root => true do
+
+        # TODO: duplicated from "securable resource"
+        let(:expected_user_name) { 'nobody' }
+        let(:expected_uid) { Etc.getpwnam(expected_user_name).uid }
+        let(:desired_gid) { 1337 }
+        let(:expected_gid) { 1337 }
+
+        before do
+          resource.owner(expected_uid)
+          resource.run_action(:create)
+        end
+
+        it "sets the owner on new_resource to the uid (Integer) of the desired owner" do
+          resource.owner.should == expected_uid
+        end
+      end
+
+      context "and group is specified with a String (group name)", :requires_root => true do
+
+        let(:expected_group_name) { Etc.getgrent.name }
+
+        before do
+          resource.group(expected_group_name)
+          resource.run_action(:create)
+        end
+
+        it "sets the group on new_resource to the group name (String) of the group" do
+          resource.group.should == expected_group_name
+        end
+
+      end
+
+      context "and group is specified with an Integer (gid)", :requires_root => true do
+        let(:expected_gid) { Etc.getgrent.gid }
+
+        before do
+          resource.group(expected_gid)
+          resource.run_action(:create)
+        end
+
+        it "sets the group on new_resource to the gid (Integer)" do
+          resource.group.should == expected_gid
+        end
+
+      end
+
+      context "and mode is specified as a String" do
+        # Need full permission for owner here or else remote directory gets
+        # into trouble trying to manage nested directories
+        let(:set_mode) { "0740" }
+        let(:expected_mode) { "0740" }
+
+        before do
+          resource.mode(set_mode)
+          resource.run_action(:create)
+        end
+
+        it "sets mode on the new_resource as a String" do
+          resource.mode.should == expected_mode
+        end
+      end
+
+      context "and mode is specified as an Integer" do
+        let(:set_mode) { 00740 }
+
+        let(:expected_mode) { "0740" }
+        before do
+          resource.mode(set_mode)
+          resource.run_action(:create)
+        end
+
+        it "sets mode on the new resource as a String" do
+          resource.mode.should == expected_mode
+        end
+      end
+    end
+
+    context "when the target file exists" do
+      before do
+        FileUtils.touch(resource.path)
+        resource.action(:create)
+      end
+
+      context "and no security metadata is specified in new_resource" do
+        it "sets the current values on current resource as strings" do
+          # TODO: most stable way to specify?
+          current_resource.owner.should == Etc.getpwuid(Process.uid).name
+          current_resource.group.should == Etc.getgrgid(Process.gid).name
+          current_resource.mode.should == "0#{((0100666 - File.umask) & 07777).to_s(8)}"
+        end
+      end
+
+      context "and owner is specified with a String (username) in new_resource" do
+
+        let(:expected_user_name) { Etc.getpwuid(Process.uid).name }
+
+        before do
+          resource.owner(expected_user_name)
+        end
+
+        it "sets the owner on new_resource to the username (String) of the desired owner" do
+          current_resource.owner.should == expected_user_name
+        end
+
+      end
+
+      context "and owner is specified with an Integer (uid) in new_resource" do
+
+        let(:expected_uid) { Process.uid }
+
+        before do
+          resource.owner(expected_uid)
+        end
+
+        it "sets the owner on new_resource to the uid (Integer) of the desired owner" do
+          current_resource.owner.should == expected_uid
+        end
+      end
+
+      context "and group is specified with a String (group name)" do
+
+        let(:expected_group_name) { Etc.getgrgid(Process.gid).name }
+
+        before do
+          resource.group(expected_group_name)
+        end
+
+        it "sets the group on new_resource to the group name (String) of the group" do
+          current_resource.group.should == expected_group_name
+        end
+
+      end
+
+      context "and group is specified with an Integer (gid)" do
+        let(:expected_gid) { Process.gid }
+
+        before do
+          resource.group(expected_gid)
+        end
+
+        it "sets the group on new_resource to the gid (Integer)" do
+          current_resource.group.should == expected_gid
+        end
+
+      end
+
+      context "and mode is specified as a String" do
+        let(:default_create_mode) { (0100666 - File.umask) }
+        let(:expected_mode) { "0#{(default_create_mode & 07777).to_s(8)}" }
+
+        before do
+          resource.mode(expected_mode)
+        end
+
+        it "sets mode on the new_resource as a String" do
+          current_resource.mode.should == expected_mode
+        end
+      end
+
+      context "and mode is specified as an Integer" do
+        let(:set_mode) { (0100666 - File.umask) & 07777 }
+        let(:expected_mode) { "0#{set_mode.to_s(8)}" }
+
+        before do
+          resource.mode(set_mode)
+        end
+
+        it "sets mode on the new resource as a String" do
+          current_resource.mode.should == expected_mode
+        end
+      end
+    end
+  end
+
+  describe "reading file security metadata for reporting on windows",:windows_only => true, :focus => true do
+
+    ALL_EXPANDED_PERMISSIONS = ["generic read",
+                                "generic write",
+                                "generic execute",
+                                "generic all",
+                                "delete",
+                                "read permissions",
+                                "change permissions",
+                                "take ownership",
+                                "synchronize",
+                                "access system security",
+                                "read data / list directory",
+                                "write data / add file",
+                                "append data / add subdirectory",
+                                "read extended attributes",
+                                "write extended attributes",
+                                "execute / traverse",
+                                "delete child",
+                                "read attributes",
+                                "write attributes"]
+
+
+    context "when the target file doesn't exist" do
+
+      # Windows reporting data should look like this (+/- ish):
+      # { "owner" => "bob", "checksum" => "ffff", "access control" => { "bob" => { "permissions" => ["perm1", "perm2", ...], "flags" => [] }}}
+
+
+      before do
+        resource.action(:create)
+      end
+
+      it "has empty values for file metadata in 'current_resource'" do
+        current_resource.owner.should be_nil
+        current_resource.expanded_rights.should be_nil
+      end
+
+      context "and no security metadata is specified in new_resource" do
+        it "sets the metadata values on the new_resource as strings after creating" do
+          resource.run_action(:create)
+          # TODO: most stable way to specify?
+          resource.owner.should == etc.getpwuid(process.uid).name
+          resource.state[:expanded_rights].should == { "CURRENTUSER" => { "permissions" => ALL_EXPANDED_PERMISSIONS, "flags" => [] }}
+          resource.state[:expanded_deny_rights].should == {}
+          resource.state[:inherits].should be_true
+        end
+      end
+
+
+      context "and owner is specified with a string (username) in new_resource" do
+
+        # TODO/bug: duplicated from the "securable resource" tests
+        let(:expected_user_name) { 'Guest' }
+
+        before do
+          resource.owner(expected_user_name)
+          resource.run_action(:create)
+        end
+
+        it "sets the owner on new_resource to the username (string) of the desired owner" do
+          resource.owner.should == expected_user_name
+        end
+
+      end
+
+      context "and owner is specified with a fully qualified domain user" do
+
+        # TODO: duplicated from "securable resource"
+        let(:expected_user_name) { 'domain\user' }
+
+        before do
+          resource.owner(expected_user_name)
+          resource.run_action(:create)
+        end
+
+        it "sets the owner on new_resource to the fully qualified name of the desired owner" do
+          resource.owner.should == expected_user_name
+        end
+      end
+
+    end
+
+    context "when the target file exists" do
+      before do
+        FileUtils.touch(resource.path)
+        resource.action(:create)
+      end
+
+      context "and no security metadata is specified in new_resource" do
+        it "sets the current values on current resource as strings" do
+          # TODO: most stable way to specify?
+          current_resource.owner.should == etc.getpwuid(process.uid).name
+          current_resource.expanded_rights.should == { "CURRENTUSER" => ALL_EXPANDED_PERMISSIONS }
+        end
+      end
+
+      context "and owner is specified with a string (username) in new_resource" do
+
+        let(:expected_user_name) { etc.getpwuid(process.uid).name }
+
+        before do
+          resource.owner(expected_user_name)
+        end
+
+        it "sets the owner on current_resource to the username (string) of the desired owner" do
+          current_resource.owner.should == expected_user_name
+        end
+
+      end
+
+      context "and owner is specified as a fully qualified 'domain\\user' in new_resource" do
+
+        let(:expected_user_name) { 'domain\user' }
+
+        before do
+          resource.owner(expected_user_name)
+        end
+
+        it "sets the owner on current_resource to the fully qualified name of the desired owner" do
+          current_resource.owner.should == expected_uid
+        end
+      end
+
+      context "and access rights are specified on the new_resource" do
+        # TODO: before do blah
+
+        it "sets the expanded_rights on the current resource" do
+          pending
+        end
+      end
+
+      context "and no access rights are specified on the current resource" do
+        # TODO: before do blah
+
+        it "sets the expanded rights on the current resource" do
+          pending
+        end
+      end
+
+
+    end
+  end
+end
diff --git a/spec/unit/provider/cookbook_file_spec.rb b/spec/unit/provider/cookbook_file_spec.rb
index c70a7d852c..6761eba692 100644
--- a/spec/unit/provider/cookbook_file_spec.rb
+++ b/spec/unit/provider/cookbook_file_spec.rb
@@ -85,7 +85,7 @@ EXPECTED
       @provider.current_resource = @current_resource
     end
 
-    after { ::File.exist?(@install_to) && FileUtils.rm(@install_to) }
+    after { ::File.exist?(File.dirname(@install_to)) && FileUtils.rm_rf(@install_to) }
 
     it "loads the current file state" do
       @provider.load_current_resource
@@ -97,12 +97,6 @@ EXPECTED
       @provider.file_cache_location.should == expected
     end
 
-    it "stages the cookbook to a temporary file" do
-      @new_resource.path(@install_to)
-      @provider.should_receive(:deploy_tempfile)
-      @provider.run_action(:create)
-    end
-
     it "installs the file from the cookbook cache" do
       @new_resource.path(@install_to)
       @provider.should_receive(:backup_new_resource)
diff --git a/spec/unit/provider/directory_spec.rb b/spec/unit/provider/directory_spec.rb
index 835be07bd6..722a451a7b 100644
--- a/spec/unit/provider/directory_spec.rb
+++ b/spec/unit/provider/directory_spec.rb
@@ -36,80 +36,98 @@ describe Chef::Provider::Directory do
     @directory = Chef::Provider::Directory.new(@new_resource, @run_context)
   end
 
-  context "load_current_resource_attrs", :unix_only do
-    it "should load the current resource based on the new resource" do
-      File.stub!(:exist?).and_return(true)
+
+  describe "scanning file security metadata on windows" do
+    before do
+    end
+
+    it "describes the directory's access rights" do
+      pending
+    end
+  end
+
+  describe "scanning file security metadata on unix" do
+    let(:mock_stat) do
       cstats = mock("stats")
       cstats.stub!(:uid).and_return(500)
       cstats.stub!(:gid).and_return(500)
       cstats.stub!(:mode).and_return(0755)
-      File.should_receive(:stat).twice.and_return(cstats)
-      @directory.load_current_resource
-      @directory.current_resource.path.should eql(@new_resource.path)
-      @directory.current_resource.owner.should eql(500)
-      @directory.current_resource.group.should eql(500)
-      @directory.current_resource.mode.should == 00755
+      cstats
     end
-  
-    it "should create a new directory on create, setting updated to true" do
-      @new_resource.path "/tmp/foo"
-  
-      File.should_receive(:exist?).exactly(3).and_return(false)
-      Dir.should_receive(:mkdir).with(@new_resource.path).once.and_return(true)
-  
-      @directory.should_receive(:set_all_access_controls)
-      @directory.stub!(:update_new_file_state)
-      @directory.run_action(:create)
-      @directory.new_resource.should be_updated
-    end
-  
-    it "should raise an exception if the parent directory does not exist and recursive is false" do 
-      @new_resource.path "/tmp/some/dir"
-      @new_resource.recursive false
-      lambda { @directory.run_action(:create) }.should raise_error(Chef::Exceptions::EnclosingDirectoryDoesNotExist) 
-    end
-  
-    it "should create a new directory when parent directory does not exist if recursive is true and permissions are correct" do
-      @new_resource.path "/path/to/dir"
-      @new_resource.recursive true
-      File.should_receive(:exist?).with(@new_resource.path).ordered.and_return(false)
-      File.should_receive(:exist?).with(@new_resource.path).ordered.and_return(false)
-  
-      File.should_receive(:exist?).with('/path/to').ordered.and_return(false)
-      File.should_receive(:exist?).with('/path').ordered.and_return(true)
-      File.should_receive(:writable?).with('/path').ordered.and_return(true)
-      File.should_receive(:exist?).with(@new_resource.path).ordered.and_return(false)
-   
-      FileUtils.should_receive(:mkdir_p).with(@new_resource.path).and_return(true) 
-      @directory.should_receive(:set_all_access_controls)
-      @directory.stub!(:update_new_file_state)
-      @directory.run_action(:create)
-      @new_resource.should be_updated
+
+    it "describes the access mode as a String of octal integers" do
+      File.stub!(:exist?).and_return(true)
+      File.should_receive(:stat).and_return(mock_stat)
+      @directory.load_current_resource
+      @directory.current_resource.mode.should == "0755"
     end
-   
-    # it "should raise an error when creating a directory recursively and permissions do not allow creation" do
-      
-    # end
-  
-    it "should raise an error when creating a directory when parent directory is a file" do
-      File.should_receive(:directory?).and_return(false)
-      Dir.should_not_receive(:mkdir).with(@new_resource.path)
-      lambda { @directory.run_action(:create) }.should raise_error(Chef::Exceptions::EnclosingDirectoryDoesNotExist)
-      @directory.new_resource.should_not be_updated
+
+    context "when user and group are specified with UID/GID" do
+      it "describes the current owner and group as UID and GID" do
+        File.stub!(:exist?).and_return(true)
+        File.should_receive(:stat).and_return(mock_stat)
+        @directory.load_current_resource
+        @directory.current_resource.path.should eql(@new_resource.path)
+        @directory.current_resource.owner.should eql(500)
+        @directory.current_resource.group.should eql(500)
+      end
     end
-    
-    it "should not create the directory if it already exists" do
-      stub_file_cstats
-      @new_resource.path "/tmp/foo"
-      File.should_receive(:exist?).exactly(3).and_return(true)
-      Dir.should_not_receive(:mkdir).with(@new_resource.path)
-      @directory.should_receive(:set_all_access_controls)
-      @directory.run_action(:create)
+
+    context "when user/group are specified with user/group names" do
     end
   end
 
-  context "load_current_resource_attrs", :windows_only do
-    pending "CHEF-3557: Fix implicit resource change collection on Windows"
+  it "should create a new directory on create, setting updated to true" do
+    @new_resource.path "/tmp/foo"
+
+    File.should_receive(:exist?).exactly(2).and_return(false)
+    Dir.should_receive(:mkdir).with(@new_resource.path).once.and_return(true)
+
+    @directory.should_receive(:set_all_access_controls)
+    @directory.stub!(:update_new_file_state)
+    @directory.run_action(:create)
+    @directory.new_resource.should be_updated
+  end
+
+  it "should raise an exception if the parent directory does not exist and recursive is false" do 
+    @new_resource.path "/tmp/some/dir"
+    @new_resource.recursive false
+    lambda { @directory.run_action(:create) }.should raise_error(Chef::Exceptions::EnclosingDirectoryDoesNotExist) 
+  end
+
+  it "should create a new directory when parent directory does not exist if recursive is true and permissions are correct" do
+    @new_resource.path "/path/to/dir"
+    @new_resource.recursive true
+    File.should_receive(:exist?).with(@new_resource.path).ordered.and_return(false)
+
+    File.should_receive(:exist?).with('/path/to').ordered.and_return(false)
+    File.should_receive(:exist?).with('/path').ordered.and_return(true)
+    File.should_receive(:writable?).with('/path').ordered.and_return(true)
+    File.should_receive(:exist?).with(@new_resource.path).ordered.and_return(false)
+
+    FileUtils.should_receive(:mkdir_p).with(@new_resource.path).and_return(true) 
+    @directory.should_receive(:set_all_access_controls)
+    @directory.stub!(:update_new_file_state)
+    @directory.run_action(:create)
+    @new_resource.should be_updated
+  end
+
+
+  it "should raise an error when creating a directory when parent directory is a file" do
+    File.should_receive(:directory?).and_return(false)
+    Dir.should_not_receive(:mkdir).with(@new_resource.path)
+    lambda { @directory.run_action(:create) }.should raise_error(Chef::Exceptions::EnclosingDirectoryDoesNotExist)
+    @directory.new_resource.should_not be_updated
+  end
+
+  it "should not create the directory if it already exists" do
+    stub_file_cstats
+    @new_resource.path "/tmp/foo"
+    File.should_receive(:directory?).twice.and_return(true)
+    File.should_receive(:exist?).exactly(3).and_return(true)
+    Dir.should_not_receive(:mkdir).with(@new_resource.path)
+    @directory.should_receive(:set_all_access_controls)
+    @directory.run_action(:create)
   end
 
   it "should delete the directory if it exists, and is writable with action_delete" do
diff --git a/spec/unit/provider/file_spec.rb b/spec/unit/provider/file_spec.rb
index 8c575e815e..9dc3908bad 100644
--- a/spec/unit/provider/file_spec.rb
+++ b/spec/unit/provider/file_spec.rb
@@ -53,25 +53,30 @@ describe Chef::Provider::File do
     @provider.current_resource.content.should eql(nil)
   end
 
-  context "load_current_resource_attrs", :unix_only do
+  describe "examining file security metadata on Unix" do
     it "should collect the current state of the file on the filesystem and populate current_resource" do
       # test setup
       stat_struct = mock("::File.stat", :mode => 0600, :uid => 0, :gid => 0, :mtime => 10000)
-      ::File.should_receive(:stat).exactly(2).times.with(@resource.path).and_return(stat_struct)
+      ::File.should_receive(:stat).exactly(1).times.with(@resource.path).and_return(stat_struct)
 
       # test execution
+
+      Etc.should_receive(:getgrgid).with(0).and_return(mock("Group Ent", :name => "wheel"))
+      Etc.should_receive(:getpwuid).with(0).and_return(mock("User Ent", :name => "root"))
+
+      # test execution 
       @provider.load_current_resource
 
       # post-condition checks
-      @provider.current_resource.mode.should == 0600
-      @provider.current_resource.owner.should == 0
-      @provider.current_resource.group.should == 0
+      @provider.current_resource.mode.should == "0600"
+      @provider.current_resource.owner.should == "root"
+      @provider.current_resource.group.should == "wheel"
     end
 
     it "should NOT update the new_resource state with the current_resourse state if new_resource state is already specified" do
       # test setup
       stat_struct = mock("::File.stat", :mode => 0600, :uid => 0, :gid => 0, :mtime => 10000)
-      ::File.should_receive(:stat).exactly(2).times.with(@resource.path).and_return(stat_struct)
+      ::File.should_receive(:stat).exactly(1).times.with(@resource.path).and_return(stat_struct)
 
       @provider.new_resource.group(1)
       @provider.new_resource.owner(1)
@@ -86,47 +91,45 @@ describe Chef::Provider::File do
       @provider.new_resource.mode.should == 0644
     end
 
-    it "should update the new_resource state with the current_resource state if the new_resource state is not specified." do
-      # test setup
-      stat_struct = mock("::File.stat", :mode => 0600, :uid => 0, :gid => 0, :mtime => 10000)
-      ::File.should_receive(:stat).exactly(2).times.with(@resource.path).and_return(stat_struct)
+    context "when the new_resource does not specify the desired access control" do
+      it "records access control information in the new resource after modifying the file" do
+        # test setup
+        stat_struct = mock("::File.stat", :mode => 0600, :uid => 0, :gid => 0, :mtime => 10000) 
+        # called once in update_new_file_state and once in checksum
+        ::File.should_receive(:stat).once.with(@provider.new_resource.path).and_return(stat_struct)  
+        ::File.should_receive(:directory?).once.with(@provider.new_resource.path).and_return(false)
 
-      @provider.new_resource.group(nil)
-      @provider.new_resource.owner(nil)
-      @provider.new_resource.mode(nil)
+        Etc.should_receive(:getpwuid).with(0).and_return(mock("User Ent", :name => "root"))
+        Etc.should_receive(:getgrgid).with(0).and_return(mock("Group Ent", :name => "wheel"))
 
-      # test execution
-      @provider.load_current_resource
+        @provider.new_resource.group(nil)
+        @provider.new_resource.owner(nil)
+        @provider.new_resource.mode(nil)
 
-      # post-condition checks
-      @provider.new_resource.group.should eql(@provider.current_resource.group)
-      @provider.new_resource.owner.should eql(@provider.current_resource.owner)
-      @provider.new_resource.mode.should eql(@provider.current_resource.mode)
-    end
+        # test exectution
+        @provider.update_new_file_state
 
-    it "should update the new_resource when attempting to set the new state" do
-      # test setup
-      stat_struct = mock("::File.stat", :mode => 0600, :uid => 0, :gid => 0, :mtime => 10000)
-      # called once in update_new_file_state and once in checksum
-      ::File.should_receive(:stat).with(@provider.new_resource.path).and_return(stat_struct)
-      ::File.should_receive(:directory?).once.with(@provider.new_resource.path).and_return(false)
+        # post-condition checks
+        @provider.new_resource.group.should == "wheel"
+        @provider.new_resource.owner.should == "root"
+        @provider.new_resource.mode.should == "0600"
+      end
+    end
+  end
 
-      @provider.new_resource.group(nil)
-      @provider.new_resource.owner(nil)
-      @provider.new_resource.mode(nil)
+  describe "when reporting security metadata on windows" do
 
-      # test exectution
-      @provider.update_new_file_state
+    it "records the file owner" do
+      pending
+    end
 
-      # post-condition checks
-      @provider.new_resource.group.should == 0
-      @provider.new_resource.owner.should == 0
-      @provider.new_resource.mode.should == 0600
+    it "records rights for each user in the ACL" do
+      pending
     end
-  end
 
-  context "load_current_resource_attrs", :windows_only do
-    pending "CHEF-3557: Fix implicit resource change collection on Windows"
+    it "records deny_rights for each user in the ACL" do
+      pending
+    end
   end
 
   it "should load a mostly blank current resource if the file specified in new_resource doesn't exist/isn't readable" do
diff --git a/spec/unit/provider/template_spec.rb b/spec/unit/provider/template_spec.rb
index f5d30f0219..5f59f8d50e 100644
--- a/spec/unit/provider/template_spec.rb
+++ b/spec/unit/provider/template_spec.rb
@@ -47,16 +47,16 @@ describe Chef::Provider::Template do
                     else
                       Struct::Passwd.new("root", "x", 0, 0, "root", "/root", "/bin/bash")
                     end
-    group_struct = OpenStruct.new(:name => "root", :passwd => "x", :gid => 0)
+    group_struct = mock("Group Ent", :name => "root", :passwd => "x", :gid => 0)
     Etc.stub!(:getpwuid).and_return(passwd_struct)
     Etc.stub!(:getgrgid).and_return(group_struct)
   end
 
   describe "when creating the template" do
 
-    before do 
-
+    before do
     end
+
     after do
       FileUtils.rm(@rendered_file_location) if ::File.exist?(@rendered_file_location)
     end
@@ -115,6 +115,23 @@ describe Chef::Provider::Template do
         IO.read(@rendered_file_location).should == "slappiness is happiness"
         @resource.should be_updated_by_last_action
       end
+
+      context "and no access control settings are set on the resource" do
+        it "sets access control metadata on the new resource" do
+          @access_controls.stub!(:requires_changes?).and_return(false)
+          @access_controls.should_receive(:set_all!)
+          @node.normal[:slappiness] = "happiness"
+          @provider.should_receive(:backup)
+          @provider.run_action(:create)
+          IO.read(@rendered_file_location).should == "slappiness is happiness"
+          @resource.should be_updated_by_last_action
+
+          # Veracity of actual data checked in functional tests
+          @resource.owner.should be_a_kind_of(String)
+          @resource.group.should be_a_kind_of(String)
+          @resource.mode.should be_a_kind_of(String)
+        end
+      end
     end
 
     describe "when the target file has the wrong content" do
diff --git a/spec/unit/scan_access_control_spec.rb b/spec/unit/scan_access_control_spec.rb
index 00e2e2669a..8fcda0edb1 100644
--- a/spec/unit/scan_access_control_spec.rb
+++ b/spec/unit/scan_access_control_spec.rb
@@ -60,7 +60,7 @@ describe Chef::ScanAccessControl do
       end
 
       it "sets the mode of the current resource to the current mode as a String" do
-        @current_resource.mode.should == "644"
+        @current_resource.mode.should == "0644"
       end
 
       context "on unix", :unix_only do
@@ -91,7 +91,7 @@ describe Chef::ScanAccessControl do
       end
 
       it "sets the mode of the current resource to the file's current mode as a string" do
-        @current_resource.mode.should == "644"
+        @current_resource.mode.should == "0644"
       end
     end
 
@@ -101,8 +101,8 @@ describe Chef::ScanAccessControl do
         @scanner.set_all!
       end
 
-      it "sets the mode of the current resource to the current mode as an integer" do
-        @current_resource.mode.should == 00644
+      it "sets the mode of the current resource to the current mode as a String" do
+        @current_resource.mode.should == "0644"
       end
 
     end