Merge branch 'master' into instrumentation

mtodd · mtodd · commit e33f774c9423 · 2014-08-29T00:37:15.000-07:00
Conflicts:
	lib/github/ldap.rb
diff --git a/github-ldap.gemspec b/github-ldap.gemspec
@@ -2,7 +2,7 @@
 
 Gem::Specification.new do |spec|
   spec.name          = "github-ldap"
-  spec.version       = "1.3.2"
+  spec.version       = "1.3.3"
   spec.authors       = ["David Calavera"]
   spec.email         = ["david.calavera@gmail.com"]
   spec.description   = %q{Ldap authentication for humans}
diff --git a/lib/github/ldap.rb b/lib/github/ldap.rb
@@ -26,6 +26,12 @@ class Ldap
     # Returns a Net::LDAP::Entry if the operation succeeded.
     def_delegator :@connection, :bind
 
+    # Public - Opens a connection to the server and keeps it open for the
+    # duration of the block.
+    #
+    # Returns the return value of the block.
+    def_delegator :@connection, :open
+
     attr_reader :uid, :search_domains, :virtual_attributes,
                 :instrumentation_service
 
@@ -127,7 +133,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
@@ -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
@@ -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
@@ -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
@@ -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
