use group name instead of sid for `current_resource` in the group resource on windowsjdm/3411

Using the SID makes things look not so nice from a reporting perspective

2 files changed, 21 insertions, 3 deletions

diff --git a/lib/chef/provider/group/windows.rb b/lib/chef/provider/group/windows.rb
index 46d8afc7f6..bd4c064462 100644
--- a/lib/chef/provider/group/windows.rb
+++ b/lib/chef/provider/group/windows.rb
@@ -39,7 +39,9 @@ class Chef
 
           members = nil
           begin
-            members = @net_group.local_get_members
+            members = current_group_user_sids.map do |sid|
+              sid.account_name
+            end
           rescue => e
             @group_exists = false
             Chef::Log.debug("#{@new_resource} group does not exist")
@@ -81,7 +83,7 @@ class Chef
 
         def has_current_group_member?(member)
           member_sid = local_group_name_to_sid(member)
-          @current_resource.members.include?(member_sid)
+          current_group_user_sids.any?{|s| s.to_s == member_sid}
         end
 
         def remove_group
@@ -92,6 +94,12 @@ class Chef
           locally_qualified_name = group_name.include?("\\") ? group_name : "#{ENV['COMPUTERNAME']}\\#{group_name}"
           Chef::ReservedNames::Win32::Security.lookup_account_name(locally_qualified_name)[1].to_s
         end
+
+        def current_group_user_sids
+          @sids ||= @net_group.local_get_members.map do |member|
+            Chef::ReservedNames::Win32::Security::SID.from_string_sid(member)
+          end
+        end
       end
     end
   end
diff --git a/spec/unit/provider/group/windows_spec.rb b/spec/unit/provider/group/windows_spec.rb
index 23dfa8315f..b88d3d81a4 100644
--- a/spec/unit/provider/group/windows_spec.rb
+++ b/spec/unit/provider/group/windows_spec.rb
@@ -50,15 +50,16 @@ describe Chef::Provider::Group::Windows do
     before do
       @new_resource.members([ "us" ])
       @current_resource = Chef::Resource::Group.new("staff")
-      @current_resource.members [ "all", "your", "base" ]
 
       allow(Chef::Util::Windows::NetGroup).to receive(:new).and_return(@net_group)
       allow(@net_group).to receive(:local_add_members)
       allow(@net_group).to receive(:local_set_members)
       allow(@provider).to receive(:local_group_name_to_sid)
+      allow(@provider).to receive(:current_group_user_sids)
       @provider.current_resource = @current_resource
     end
 
+    let(:members) { [ "all", "your", "base" ] }
     it "should call @net_group.local_set_members" do
       allow(@new_resource).to receive(:append).and_return(false)
       expect(@net_group).to receive(:local_set_members).with(@new_resource.members)
@@ -68,7 +69,16 @@ describe Chef::Provider::Group::Windows do
     it "should call @net_group.local_add_members" do
       allow(@new_resource).to receive(:append).and_return(true)
       expect(@net_group).to receive(:local_add_members).with(@new_resource.members)
+      expect(@provider).to receive(:current_group_user_sids).and_return(
+        members.map do |member|
+          instance_double("SID", :to_s => "S-1-#{member}", :account_name => member)
+        end
+      )
       @provider.manage_group
+      
+      members.each do |member|
+        expect(@provider.current_resource.members.include?(member))
+      end
     end
 
   end