Enhance groups resource with members property (#3029)

* implement members property
* flatten groups entry, extract flatten helper
* lints
* more idiomatic spec, add example of members testing

Signed-off-by: Tor Magnus Rakvåg <tm@intility.no>

lib/resources/groups.rb

@@ -52,6 +52,7 @@ module Inspec::Resources
           .add(:names,     field: 'name')
           .add(:gids,      field: 'gid')
           .add(:domains,   field: 'domain')
+          .add(:members,   field: 'members')
           .add(:exists?) { |x| !x.entries.empty? }
     filter.connect(self, :collect_group_details)
 
@@ -90,6 +91,10 @@ module Inspec::Resources
         it { should exist }
         its('gid') { should eq 0 }
       end
+
+      describe group('Administrators') do
+        its('members') { should include 'Administrator' }
+      end
     "
 
     def initialize(groupname)
@@ -106,15 +111,7 @@ module Inspec::Resources
     end
 
     def gid
-      gids = group_info.gids
+      flatten_entry(group_info, 'gid')
-      if gids.empty?
-        nil
-      # the default case should be one group
-      elsif gids.size == 1
-        gids.entries[0]
-      else
-        raise 'found more than one group with the same name, please use `groups` resource'
-      end
     end
 
     # implements rspec has matcher, to be compatible with serverspec
@@ -122,6 +119,11 @@ module Inspec::Resources
       gid == compare_gid
     end
 
+    def members
+      return unless inspec.os.windows?
+      flatten_entry(group_info, 'members')
+    end
+
     def local
       # at this point the implementation only returns local groups
       true
@@ -133,6 +135,17 @@ module Inspec::Resources
 
     private
 
+    def flatten_entry(group_info, prop)
+      entries = group_info.entries
+      if entries.empty?
+        nil
+      elsif entries.size == 1
+        entries.first.send(prop)
+      else
+        raise 'found more than one group with the same name, please use `groups` resource'
+      end
+    end
+
     def group_info
       # we need a local copy for the block
       group = @group.dup
@@ -183,18 +196,22 @@ module Inspec::Resources
   class WindowsGroup < GroupInfo
     # returns all local groups
     def groups
-      script = <<~EOH
+      script = <<-EOH
         Function ConvertTo-SID { Param([byte[]]$BinarySID)
           (New-Object System.Security.Principal.SecurityIdentifier($BinarySID,0)).Value
         }
-
         $Computername = $Env:Computername
         $adsi = [ADSI]"WinNT://$Computername"
         $groups = $adsi.Children | where {$_.SchemaClassName -eq 'group'} | ForEach {
           $name = $_.Name[0]
           $sid = ConvertTo-SID -BinarySID $_.ObjectSID[0]
           $group =[ADSI]$_.Path
-          new-object psobject -property @{name = $group.Name[0]; gid = $sid; domain=$Computername}
+          $members = $_.Members() | Foreach-Object { $_.GetType().InvokeMember('Name', 'GetProperty', $null, $_, $null) }
+          # An empty collection of these objects isn't properly converted to an empty array by ConvertTo-Json
+          if(-not [bool]$members) {
+            $members = @()
+          }
+          new-object psobject -property @{name = $group.Name[0]; gid = $sid; domain = $Computername; members = $members}
         }
         $groups | ConvertTo-Json -Depth 3
       EOH

test/helper.rb

@@ -325,7 +325,7 @@ class MockLoader
       # user info for windows (winrm 1.6.0, 1.6.1)
       '27c6cda89fa5d196506251c0ed0d20468b378c5689711981dc1e1e683c7b02c1' => cmd.call('adsiusers'),
       # group info for windows
-      'd8d5b3e3355650399e23857a526ee100b4e49e5c2404a0a5dbb7d85d7f4de5cc' => cmd.call('adsigroups'),
+      '4020573097e910a573e22e8863c4faa434f52910a45714606cad1fb8b060d9e9' => cmd.call('adsigroups'),
       # group info for Darwin
       'dscacheutil -q group' => cmd.call('dscacheutil-query-group'),
       # network interface

test/integration/default/controls/group_spec.rb

@@ -56,4 +56,16 @@ elsif os.solaris?
     it { should_not exist }
     its('gid') { should eq nil }
   end
+elsif os[:family] == 'windows'
+  describe group('Administrators') do
+    it { should exist }
+    its('gid') { should eq 'S-1-5-32-544' }
+    its('members') { should include 'Administrator' }
+  end
+
+  describe group('Power Users') do
+    it { should exist }
+    its('gid') { should eq 'S-1-5-32-547' }
+    its('members') { should eq [] }
+  end
 end

test/unit/mock/cmd/adsigroups

@@ -2,21 +2,36 @@
     {
         "gid":  "S-1-5-32-544",
         "domain":  "WIN-CIV7VMLVHLD",
-        "name":  "Administrators"
+        "name":  "Administrators",
+        "members": [
+            "Administrators",
+            "Domain Admins"
+        ]
     },
     {
         "gid":  "S-1-5-32-546",
         "domain":  "WIN-CIV7VMLVHLD",
-        "name":  "Guests"
+        "name":  "Guests",
+        "members": [
+            "Guest"
+        ]
     },
     {
         "gid":  "S-1-5-32-547",
         "domain":  "WIN-CIV7VMLVHLD",
-        "name":  "Power Users"
+        "name":  "Power Users",
+        "members": [
+
+        ]
     },
     {
         "gid":  "S-1-5-32-545",
         "domain":  "WIN-CIV7VMLVHLD",
-        "name":  "Users"
+        "name":  "Users",
+        "members":  [
+                "INTERACTIVE",
+                "Authenticated Users",
+                "Domain Users"
+            ]
     }
 ]

test/unit/resources/group_test.rb

@@ -47,13 +47,20 @@ describe 'Inspec::Resources::Group' do
   end
 
   # windows with local group
-  it 'verify group on windows' do
+  it 'verify administrator group on windows' do
     resource = MockLoader.new(:windows).load_resource('group', 'Administrators')
     _(resource.exists?).must_equal true
     _(resource.gid).must_equal 'S-1-5-32-544'
     _(resource.has_gid?(0)).must_equal false
-    _(resource.local).must_equal true
+    _(resource.members).must_equal ['Administrators', 'Domain Admins']
-    _(resource.to_s).must_equal 'Group Administrators'
+  end
+
+  it 'verify power users group on windows' do
+    resource = MockLoader.new(:windows).load_resource('group', 'Power Users')
+    _(resource.exists?).must_equal true
+    _(resource.gid).must_equal 'S-1-5-32-547'
+    _(resource.has_gid?(0)).must_equal false
+    _(resource.members).must_equal []
   end
 
   # windows non-existent group
@@ -62,6 +69,7 @@ describe 'Inspec::Resources::Group' do
     _(resource.exists?).must_equal false
     _(resource.gid).must_be_nil
     _(resource.has_gid?(0)).must_equal false
+    _(resource.members).must_equal nil
   end
 
   # undefined