Merge pull request #40 from github/deeply-nested-membership-checks

Check memberships recursively

lib/github/ldap.rb

@@ -112,7 +112,7 @@ module GitHub
     def load_group(group_entry)
       if @virtual_attributes.enabled?
         VirtualGroup.new(self, group_entry)
-      elsif PosixGroup.valid?(group_entry)
+      elsif posix_support_enabled? && PosixGroup.valid?(group_entry)
         PosixGroup.new(self, group_entry)
       else
         Group.new(self, group_entry)

lib/github/ldap/group.rb

@@ -72,9 +72,12 @@ module GitHub
       # Internal - Check if an object class includes the member names
       # Use `&` rathen than `include?` because both are arrays.
       #
+      # NOTE: object classes are downcased by default in Net::LDAP, so this
+      # will fail to match correctly unless we also downcase our group classes.
+      #
       # Returns true if the object class includes one of the group class names.
       def group?(object_class)
-        !(GROUP_CLASS_NAMES & object_class).empty?
+        !(GROUP_CLASS_NAMES.map(&:downcase) & object_class.map(&:downcase)).empty?
       end
 
       # Internal - Generate a hash with all the group DNs for caching purposes.

test/domain_test.rb

@@ -158,6 +158,13 @@ class GitHubLdapDomainNestedGroupsTest < GitHub::Ldap::Test
     assert @domain.is_member?(user, %w(enterprise-ops)),
       "Expected `enterprise-ops` to include the member `#{user.dn}`"
   end
+
+  def test_membership_in_deeply_nested_subgroups
+    assert user = @ldap.domain('uid=user1.1.1.1,ou=users,dc=github,dc=com').bind
+
+    assert @domain.is_member?(user, %w(group1)),
+      "Expected `group1` to include the member `#{user.dn}` via deep recursion"
+  end
 end
 
 class GitHubLdapPosixGroupsWithRecursionFallbackTest < GitHub::Ldap::Test

test/fixtures/github-with-subgroups.ldif

@@ -43,6 +43,29 @@ objectClass: groupOfNames
 member: uid=calavera,ou=users,dc=github,dc=com
 member: uid=rubiojr,ou=users,dc=github,dc=com
 
+dn: cn=group1,ou=groups,dc=github,dc=com
+cn: group1
+objectClass: groupOfNames
+member: uid=user1,ou=users,dc=github,dc=com
+member: cn=group1.1,ou=groups,dc=github,dc=com
+
+dn: cn=group1.1,ou=groups,dc=github,dc=com
+cn: group1
+objectClass: groupOfNames
+member: uid=user1.1,ou=users,dc=github,dc=com
+member: cn=group1.1.1,ou=groups,dc=github,dc=com
+
+dn: cn=group1.1.1,ou=groups,dc=github,dc=com
+cn: group1
+objectClass: groupOfNames
+member: uid=user1.1.1,ou=users,dc=github,dc=com
+member: cn=group1.1.1.1,ou=groups,dc=github,dc=com
+
+dn: cn=group1.1.1.1,ou=groups,dc=github,dc=com
+cn: group1
+objectClass: groupOfNames
+member: uid=user1.1.1.1,ou=users,dc=github,dc=com
+
 # Users
 
 dn: ou=users,dc=github,dc=com
@@ -89,3 +112,35 @@ uid: mtodd
 userPassword: passworD1
 mail: mtodd@github.com
 objectClass: inetOrgPerson
+
+dn: uid=user1,ou=users,dc=github,dc=com
+uid: user1
+sn: user1
+cn: user1
+userPassword: passworD1
+mail: user1@github.com
+objectClass: inetOrgPerson
+
+dn: uid=user1.1,ou=users,dc=github,dc=com
+uid: user1.1
+sn: user1.1
+cn: user1.1
+userPassword: passworD1
+mail: user1.1@github.com
+objectClass: inetOrgPerson
+
+dn: uid=user1.1.1,ou=users,dc=github,dc=com
+uid: user1.1.1
+sn: user1.1.1
+cn: user1.1.1
+userPassword: passworD1
+mail: user1.1.1@github.com
+objectClass: inetOrgPerson
+
+dn: uid=user1.1.1.1,ou=users,dc=github,dc=com
+uid: user1.1.1.1
+sn: user1.1.1.1
+cn: user1.1.1.1
+userPassword: passworD1
+mail: user1.1.1.1@github.com
+objectClass: inetOrgPerson

test/group_test.rb

@@ -14,6 +14,12 @@ class GitHubLdapGroupTest < GitHub::Ldap::Test
     @group = @ldap.group("cn=enterprise,ou=groups,dc=github,dc=com")
   end
 
+  def test_group?
+    object_classes = %w(groupOfNames)
+    assert @group.group?(object_classes)
+    assert @group.group?(object_classes.map(&:downcase))
+  end
+
   def test_subgroups
     assert_equal 3, @group.subgroups.size
   end
@@ -24,7 +30,7 @@ class GitHubLdapGroupTest < GitHub::Ldap::Test
 
   def test_all_domain_groups
     groups = groups_domain.all_groups
-    assert_equal 4, groups.size
+    assert_equal 8, groups.size
   end
 
   def test_filter_domain_groups