diff --git a/lib/github/ldap.rb b/lib/github/ldap.rb index 5918670..a0f6fe4 100644 --- a/lib/github/ldap.rb +++ b/lib/github/ldap.rb @@ -112,7 +112,7 @@ def group(base_name) 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) diff --git a/lib/github/ldap/group.rb b/lib/github/ldap/group.rb index f77196d..3d53066 100644 --- a/lib/github/ldap/group.rb +++ b/lib/github/ldap/group.rb @@ -72,9 +72,12 @@ def member_names # 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. diff --git a/test/domain_test.rb b/test/domain_test.rb index f7f9056..470e00d 100644 --- a/test/domain_test.rb +++ b/test/domain_test.rb @@ -158,6 +158,13 @@ def test_membership_in_subgroups 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 diff --git a/test/fixtures/github-with-subgroups.ldif b/test/fixtures/github-with-subgroups.ldif index f9d22be..00dc929 100644 --- a/test/fixtures/github-with-subgroups.ldif +++ b/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 diff --git a/test/group_test.rb b/test/group_test.rb index bfdc3e8..a6fbf1f 100644 --- a/test/group_test.rb +++ b/test/group_test.rb @@ -14,6 +14,12 @@ def setup @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 @@ def test_members_from_subgroups 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