From be83e88cd76cefd3864adc452aa6a2f43908da95 Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Wed, 27 Aug 2014 16:16:18 -0700 Subject: [PATCH 1/6] Add failing test case for deeply nested membership checks --- test/domain_test.rb | 7 +++ test/fixtures/github-with-subgroups.ldif | 55 ++++++++++++++++++++++++ 2 files changed, 62 insertions(+) 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..9e089d2 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: cn=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 From fc34566c5df9479a14d28a85d7af5277c348246f Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Wed, 27 Aug 2014 17:00:34 -0700 Subject: [PATCH 2/6] Adjust group count --- test/group_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/group_test.rb b/test/group_test.rb index bfdc3e8..b3dc7fe 100644 --- a/test/group_test.rb +++ b/test/group_test.rb @@ -24,7 +24,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 From a9db2f76622e519d50215df5b2aead29f212237f Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Wed, 27 Aug 2014 17:09:15 -0700 Subject: [PATCH 3/6] Fix LDIF entry --- test/fixtures/github-with-subgroups.ldif | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fixtures/github-with-subgroups.ldif b/test/fixtures/github-with-subgroups.ldif index 9e089d2..00dc929 100644 --- a/test/fixtures/github-with-subgroups.ldif +++ b/test/fixtures/github-with-subgroups.ldif @@ -64,7 +64,7 @@ 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: cn=user1.1.1.1,ou=users,dc=github,dc=com +member: uid=user1.1.1.1,ou=users,dc=github,dc=com # Users From 68b245144bb78919fd5f98f202ad2409d04f0a38 Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Wed, 27 Aug 2014 18:42:45 -0700 Subject: [PATCH 4/6] Disable PosixGroup validation unless enabled --- lib/github/ldap.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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) From d29f991689a8fa11b94c076f4662fbf9d34b5b6c Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Wed, 27 Aug 2014 18:45:46 -0700 Subject: [PATCH 5/6] Downcase group classes to match Net::LDAP behavior --- lib/github/ldap/group.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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. From 055ac02410dfa744fe2aaf7f334819b36c825b01 Mon Sep 17 00:00:00 2001 From: Matt Todd Date: Wed, 27 Aug 2014 19:39:41 -0700 Subject: [PATCH 6/6] Test Group#group? checking --- test/group_test.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/group_test.rb b/test/group_test.rb index b3dc7fe..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