CHEF-3012: Use SID instead of username for group membership checks

3 files changed, 22 insertions, 8 deletions

diff --git a/lib/chef/provider/group/windows.rb b/lib/chef/provider/group/windows.rb
index cb0ab76927..de12fb4a7f 100644
--- a/lib/chef/provider/group/windows.rb
+++ b/lib/chef/provider/group/windows.rb
@@ -59,7 +59,9 @@ class Chef
           if @new_resource.append
             members_to_be_added = [ ]
             @new_resource.members.each do |member|
-              members_to_be_added << member if !@current_resource.members.include?(member)
+              member = "#{ENV['COMPUTERNAME']}\\#{member}" if ! member.include?("\\")
+              member_sid = Chef::ReservedNames::Win32::Security.lookup_account_name(member)[1]
+              members_to_be_added << member if !@current_resource.members.include?(member_sid.to_s)
             end
 
             # local_add_members will raise ERROR_MEMBER_IN_ALIAS if a
diff --git a/lib/chef/util/windows/net_group.rb b/lib/chef/util/windows/net_group.rb
index e5d78614db..924bd392f9 100644
--- a/lib/chef/util/windows/net_group.rb
+++ b/lib/chef/util/windows/net_group.rb
@@ -55,19 +55,20 @@ class Chef::Util::Windows::NetGroup < Chef::Util::Windows
       nread = 0.chr * PTR_SIZE
       total = 0.chr * PTR_SIZE
 
-      rc = NetLocalGroupGetMembers.call(nil, @name, 1, ptr, -1,
+      rc = NetLocalGroupGetMembers.call(nil, @name, 0, ptr, -1,
                                         nread, total, handle)
       if (rc == NERR_Success) || (rc == ERROR_MORE_DATA)
         ptr = ptr.unpack('L')[0]
         nread = nread.unpack('i')[0]
-        members = 0.chr * (nread * (PTR_SIZE * 3)) #nread * sizeof(LOCALGROUP_MEMBERS_INFO_1)
+        members = 0.chr * (nread * PTR_SIZE ) #nread * sizeof(LOCALGROUP_MEMBERS_INFO_0)
         memcpy(members, ptr, members.size)
 
-        # 3 pointer fields in LOCALGROUP_MEMBERS_INFO_1, offset 2*PTR_SIZE is lgrmi1_name
+        # 1 pointer field in LOCALGROUP_MEMBERS_INFO_0, offset 0 is lgrmi0_sid
         nread.times do |i|
-          offset = (i * 3) + 2
-          member = lpwstr_to_s(members, offset)
-          group_members << member
+          sid_address = members[i * PTR_SIZE, PTR_SIZE].unpack('L')[0]
+          sid_ptr = FFI::Pointer.new(sid_address)
+          member_sid = Chef::ReservedNames::Win32::Security::SID.new(sid_ptr)
+          group_members << member_sid.to_s
         end
         NetApiBufferFree(ptr)
       else
diff --git a/spec/functional/resource/group_spec.rb b/spec/functional/resource/group_spec.rb
index b8369d087b..67fa204b9d 100644
--- a/spec/functional/resource/group_spec.rb
+++ b/spec/functional/resource/group_spec.rb
@@ -38,7 +38,8 @@ describe Chef::Resource::Group, :requires_root_or_running_windows, :not_supporte
   def user_exist_in_group?(user)
     case ohai[:platform_family]
     when "windows"
-      Chef::Util::Windows::NetGroup.new(group_name).local_get_members.include?(user)
+      user_sid = sid_string_from_user(user)
+      user_sid.nil? ? false : Chef::Util::Windows::NetGroup.new(group_name).local_get_members.include?(user_sid)
     else
       Etc::getgrnam(group_name).mem.include?(user)
     end
@@ -57,6 +58,16 @@ describe Chef::Resource::Group, :requires_root_or_running_windows, :not_supporte
     return resource.gid == Etc::getgrnam(resource.name).gid if unix?
   end
 
+  def sid_string_from_user(user)
+    begin
+      sid = Chef::ReservedNames::Win32::Security.lookup_account_name(user)
+    rescue Chef::Exceptions::Win32APIError
+      sid = nil
+    end
+
+    sid.nil? ? nil : sid[1].to_s
+  end
+
   def user(username)
     usr = Chef::Resource::User.new("#{username}", run_context)
     if ohai[:platform_family] == "windows"