From f4da8948695be6637cab4cc10d9192c5fb6347f6 Mon Sep 17 00:00:00 2001 From: Colin Seymour Date: Mon, 11 Apr 2016 16:40:18 +0100 Subject: [PATCH 1/6] Bump net-ldap requirement to 0.14.0 We need this so we can take advantage of the `hosts` option --- github-ldap.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github-ldap.gemspec b/github-ldap.gemspec index af13ce0..eb04367 100644 --- a/github-ldap.gemspec +++ b/github-ldap.gemspec @@ -15,7 +15,7 @@ Gem::Specification.new do |spec| spec.test_files = spec.files.grep(%r{^(test|spec|features)/}) spec.require_paths = ["lib"] - spec.add_dependency 'net-ldap', '~> 0.11.0' + spec.add_dependency 'net-ldap', '~> 0.14.0' spec.add_development_dependency "bundler", "~> 1.3" spec.add_development_dependency 'ladle' From baca799d8f7d7431116a48c5fc45c949934d4964 Mon Sep 17 00:00:00 2001 From: Colin Seymour Date: Mon, 11 Apr 2016 16:41:02 +0100 Subject: [PATCH 2/6] Adds support for the `hosts` option now available in net-ldap --- lib/github/ldap.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/github/ldap.rb b/lib/github/ldap.rb index 0545247..90c6f92 100644 --- a/lib/github/ldap.rb +++ b/lib/github/ldap.rb @@ -50,6 +50,9 @@ class Ldap # # host: required string ldap server host address # port: required string or number ldap server port + # hosts: an enumerable of pairs of hosts and corresponding ports with + # which to attempt opening connections (default [[host, port]]). Overrides + # host and port if set. # encryption: optional string. `ssl` or `tls`. nil by default # admin_user: optional string ldap administrator user dn for authentication # admin_password: optional string ldap administrator user password @@ -72,6 +75,7 @@ def initialize(options = {}) @connection = Net::LDAP.new({ host: options[:host], port: options[:port], + hosts: options[:hosts], instrumentation_service: options[:instrumentation_service] }) From 65e653987f7138b3ce0518fe1a88deb44f70d2c6 Mon Sep 17 00:00:00 2001 From: Colin Seymour Date: Mon, 11 Apr 2016 16:41:39 +0100 Subject: [PATCH 3/6] Add tests for `hosts` option --- test/ldap_test.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/ldap_test.rb b/test/ldap_test.rb index c7b24c4..d00c7a0 100644 --- a/test/ldap_test.rb +++ b/test/ldap_test.rb @@ -9,6 +9,21 @@ def test_connection_with_default_options assert @ldap.test_connection, "Ldap connection expected to succeed" end + def test_connection_with_list_of_hosts_with_one_valid_host + ldap = GitHub::Ldap.new(options.merge(hosts: [["localhost", 3897]])) + assert ldap.test_connection, "Ldap connection expected to succeed" + end + + def test_connection_with_list_of_hosts_with_first_valid + ldap = GitHub::Ldap.new(options.merge(hosts: [["localhost", 3897], ["invalid.local", 3897]])) + assert ldap.test_connection, "Ldap connection expected to succeed" + end + + def test_connection_with_list_of_hosts_with_first_invalid + ldap = GitHub::Ldap.new(options.merge(hosts: [["invalid.local", 3897], ["localhost", 3897]])) + assert ldap.test_connection, "Ldap connection expected to succeed" + end + def test_simple_tls assert_equal :simple_tls, @ldap.check_encryption(:ssl) assert_equal :simple_tls, @ldap.check_encryption('SSL') From a87afd0d562871b741ce6e02968a8b9d9261abcf Mon Sep 17 00:00:00 2001 From: Colin Seymour Date: Mon, 11 Apr 2016 16:53:47 +0100 Subject: [PATCH 4/6] Document `hosts` option in README --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 482f6ea..eb5fb01 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,7 @@ There are a few configuration options required to use this adapter: * host: is the host address where the ldap server lives. * port: is the port where the ldap server lives. +* hosts: (optional) an enumerable of pairs of hosts and corresponding ports with which to attempt opening connections (default [[host, port]]). Overrides host and port if set. * encryption: is the encryption protocol, disabled by default. The valid options are `ssl` and `tls`. * uid: is the field name in the ldap server used to authenticate your users, in ActiveDirectory this is `sAMAccountName`. From 0e695c622af73498c692901051c03160b388e863 Mon Sep 17 00:00:00 2001 From: Colin Seymour Date: Fri, 15 Apr 2016 10:49:55 +0100 Subject: [PATCH 5/6] Bump version --- github-ldap.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github-ldap.gemspec b/github-ldap.gemspec index eb04367..b098983 100644 --- a/github-ldap.gemspec +++ b/github-ldap.gemspec @@ -2,7 +2,7 @@ Gem::Specification.new do |spec| spec.name = "github-ldap" - spec.version = "1.9.0" + spec.version = "1.9.1" spec.authors = ["David Calavera", "Matt Todd"] spec.email = ["david.calavera@gmail.com", "chiology@gmail.com"] spec.description = %q{LDAP authentication for humans} From 9315cee5510b2d2360d745a404411840b154dda8 Mon Sep 17 00:00:00 2001 From: Colin Seymour Date: Fri, 16 Sep 2016 16:28:02 +0100 Subject: [PATCH 6/6] Use port from options We don't care too much about the port in testing. Using the port in the options allows us to test against ApacheDS and OpenLDAP without adding any additional conditionals. --- test/ldap_test.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/ldap_test.rb b/test/ldap_test.rb index 8105fb8..959c0b3 100644 --- a/test/ldap_test.rb +++ b/test/ldap_test.rb @@ -10,17 +10,17 @@ def test_connection_with_default_options end def test_connection_with_list_of_hosts_with_one_valid_host - ldap = GitHub::Ldap.new(options.merge(hosts: [["localhost", 3897]])) + ldap = GitHub::Ldap.new(options.merge(hosts: [["localhost", options[:port]]])) assert ldap.test_connection, "Ldap connection expected to succeed" end def test_connection_with_list_of_hosts_with_first_valid - ldap = GitHub::Ldap.new(options.merge(hosts: [["localhost", 3897], ["invalid.local", 3897]])) + ldap = GitHub::Ldap.new(options.merge(hosts: [["localhost", options[:port]], ["invalid.local", options[:port]]])) assert ldap.test_connection, "Ldap connection expected to succeed" end def test_connection_with_list_of_hosts_with_first_invalid - ldap = GitHub::Ldap.new(options.merge(hosts: [["invalid.local", 3897], ["localhost", 3897]])) + ldap = GitHub::Ldap.new(options.merge(hosts: [["invalid.local", options[:port]], ["localhost", options[:port]]])) assert ldap.test_connection, "Ldap connection expected to succeed" end