allow the use of SIDs in windows securable resourcessid

Signed-off-by: mwrock <matt@mattwrock.com>

2 files changed, 115 insertions, 30 deletions

diff --git a/lib/chef/file_access_control/windows.rb b/lib/chef/file_access_control/windows.rb
index 118eae179c..9e5f07428c 100644
--- a/lib/chef/file_access_control/windows.rb
+++ b/lib/chef/file_access_control/windows.rb
@@ -112,7 +112,11 @@ class Chef
 
       def get_sid(value)
         if value.is_a?(String)
-          SID.from_account(value)
+          begin
+            Security.convert_string_sid_to_sid(value)
+          rescue Chef::Exceptions::Win32APIError
+            SID.from_account(value)
+          end
         elsif value.is_a?(SID)
           value
         else
diff --git a/spec/support/shared/functional/securable_resource.rb b/spec/support/shared/functional/securable_resource.rb
index 26f2b1cc5a..f933237583 100644
--- a/spec/support/shared/functional/securable_resource.rb
+++ b/spec/support/shared/functional/securable_resource.rb
@@ -242,48 +242,100 @@ shared_examples_for "a securable resource with existing target" do
     include_context "use Windows permissions"
 
     describe "when setting owner" do
-      before do
-        resource.owner(SID.admin_account_name)
-        resource.run_action(:create)
-      end
-
-      it "should set the owner" do
-        expect(descriptor.owner).to eq(SID.Administrator)
+      context "with user name" do
+        before do
+          resource.owner(SID.admin_account_name)
+          resource.run_action(:create)
+        end
+  
+        it "should set the owner" do
+          expect(descriptor.owner).to eq(SID.Administrator)
+        end
+  
+        it "is marked as updated only if changes are made" do
+          expect(resource.updated_by_last_action?).to eq(expect_updated?)
+        end
       end
 
-      it "is marked as updated only if changes are made" do
-        expect(resource.updated_by_last_action?).to eq(expect_updated?)
+      context "with SID" do
+        before do
+          resource.owner(SID.Administrator.to_s)
+          resource.run_action(:create)
+        end
+  
+        it "should set the owner" do
+          expect(descriptor.owner).to eq(SID.Administrator)
+        end
+  
+        it "is marked as updated only if changes are made" do
+          expect(resource.updated_by_last_action?).to eq(expect_updated?)
+        end
       end
     end
 
     describe "when setting group" do
-      before do
-        resource.group("Administrators")
-        resource.run_action(:create)
-      end
-
-      it "should set the group" do
-        expect(descriptor.group).to eq(SID.Administrators)
+      context "with group name" do
+        before do
+          resource.group("Administrators")
+          resource.run_action(:create)
+        end
+  
+        it "should set the group" do
+          expect(descriptor.group).to eq(SID.Administrators)
+        end
+  
+        it "is marked as updated only if changes are made" do
+          expect(resource.updated_by_last_action?).to eq(expect_updated?)
+        end
       end
 
-      it "is marked as updated only if changes are made" do
-        expect(resource.updated_by_last_action?).to eq(expect_updated?)
+      context "with group SID" do
+        before do
+          resource.group(SID.Administrators.to_s)
+          resource.run_action(:create)
+        end
+  
+        it "should set the group" do
+          expect(descriptor.group).to eq(SID.Administrators)
+        end
+  
+        it "is marked as updated only if changes are made" do
+          expect(resource.updated_by_last_action?).to eq(expect_updated?)
+        end
       end
     end
 
     describe "when setting rights and deny_rights" do
-      before do
-        resource.deny_rights(:modify, "Guest")
-        resource.rights(:read, "Guest")
-        resource.run_action(:create)
-      end
+      context "with user name" do
+        before do
+          resource.deny_rights(:modify, "Guest")
+          resource.rights(:read, "Guest")
+          resource.run_action(:create)
+        end
 
-      it "should set the rights and deny_rights" do
-        expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_modify_perms) + allowed_acl(SID.Guest, expected_read_perms))
+        it "should set the rights and deny_rights" do
+          expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_modify_perms) + allowed_acl(SID.Guest, expected_read_perms))
+        end
+
+        it "is marked as updated only if changes are made" do
+          expect(resource.updated_by_last_action?).to eq(expect_updated?)
+        end
       end
 
-      it "is marked as updated only if changes are made" do
-        expect(resource.updated_by_last_action?).to eq(expect_updated?)
+      context "with SID" do
+        before do
+          resource.deny_rights(:modify, SID.Guest.to_s)
+          resource.rights(:read, SID.Guest.to_s)
+          resource.run_action(:create)
+        end
+
+        it "should set the rights and deny_rights" do
+          expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_modify_perms) + allowed_acl(SID.Guest, expected_read_perms))
+        end
+
+        it "is marked as updated only if changes are made" do
+          expect(resource.updated_by_last_action?).to eq(expect_updated?)
+        end
       end
     end
   end
@@ -302,12 +354,18 @@ shared_examples_for "a securable resource without existing target" do
       expect(descriptor.owner).to eq(SID.default_security_object_owner)
     end
 
-    it "sets owner when owner is specified" do
+    it "sets owner when owner is specified by name" do
       resource.owner "Guest"
       resource.run_action(:create)
       expect(descriptor.owner).to eq(SID.Guest)
     end
 
+    it "sets owner when owner is specified by SID" do
+      resource.owner SID.Guest.to_s
+      resource.run_action(:create)
+      expect(descriptor.owner).to eq(SID.Guest)
+    end
+
     it "fails to set owner when owner has invalid characters" do
       expect { resource.owner 'Lance "The Nose" Glindenberry III' }.to raise_error(Chef::Exceptions::ValidationFailed)
     end
@@ -339,12 +397,18 @@ shared_examples_for "a securable resource without existing target" do
       expect(descriptor.group).to eq(SID.default_security_object_group)
     end
 
-    it "sets group when group is specified" do
+    it "sets group when group is specified by name" do
       resource.group "Everyone"
       resource.run_action(:create)
       expect(descriptor.group).to eq(SID.Everyone)
     end
 
+    it "sets group when group is specified by SID" do
+      resource.group SID.Everyone.to_s
+      resource.run_action(:create)
+      expect(descriptor.group).to eq(SID.Everyone)
+    end
+
     it "fails to set group when group has invalid characters" do
       expect { resource.group 'Lance "The Nose" Glindenberry III' }.to raise_error(Chef::Exceptions::ValidationFailed)
     end
@@ -406,6 +470,17 @@ shared_examples_for "a securable resource without existing target" do
             allowed_acl(SID.Guest, expected_modify_perms)
           )
         end
+
+        it "multiple rights with SID" do
+          resource.rights(:read, SID.Everyone.to_s)
+          resource.rights(:modify, SID.Guest.to_s)
+          resource.run_action(:create)
+
+          expect(explicit_aces).to eq(
+            allowed_acl(SID.Everyone, expected_read_perms) +
+            allowed_acl(SID.Guest, expected_modify_perms)
+          )
+        end
       end
     end
 
@@ -443,6 +518,12 @@ shared_examples_for "a securable resource without existing target" do
           expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_full_control_perms))
         end
 
+        it "using SID" do
+          resource.deny_rights(:full_control, SID.Guest.to_s)
+          resource.run_action(:create)
+          expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_full_control_perms))
+        end
+
         it "deny_rights ahead of rights" do
           resource.rights(:read, "Everyone")
           resource.deny_rights(:modify, "Guest")