From 43a31e9db957adcfebd1bb426227a74796663e31 Mon Sep 17 00:00:00 2001 From: Schparky <3172830+Schparky@users.noreply.github.com> Date: Wed, 20 Jan 2021 09:06:34 -0700 Subject: [PATCH 1/4] allow a list of hosts in ldap config Iterate through the list of hosts and hold on to the first one that works so it doesn't have to do that each time `connect` is called, which can be several times in one request. --- .gitignore | 1 + .../common/components/passwordStore/Ldap.php | 58 ++++++++++++++++--- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/.gitignore b/.gitignore index 827f526e..140e0f59 100644 --- a/.gitignore +++ b/.gitignore @@ -23,3 +23,4 @@ application/tests/_support/_generated/ api.html dockercfg +*.crt diff --git a/application/common/components/passwordStore/Ldap.php b/application/common/components/passwordStore/Ldap.php index be2c8421..0fa6cd7d 100644 --- a/application/common/components/passwordStore/Ldap.php +++ b/application/common/components/passwordStore/Ldap.php @@ -11,7 +11,7 @@ class Ldap extends Component implements PasswordStoreInterface /** @var string */ public $baseDn; - /** @var string */ + /** @var string|string[] */ public $host; /** @var integer default=636 */ @@ -83,31 +83,71 @@ class Ldap extends Component implements PasswordStoreInterface */ public function connect() { + // Connection has already been established + if ($this->ldapClient !== null) { + return; + } + if ($this->useSsl && $this->useTls) { // Prefer TLS over SSL $this->useSsl = false; } - /* - * Initialize provider with configuration - */ - $this->ldapClient = new Adldap(); - $this->ldapClient->addProvider([ + // ensure the `host` property is an array + $this->host = is_array($this->host) ? $this->host : [$this->host]; + + // iterate over the list of hosts to find the first one that is good + foreach ($this->host as $host) { + $client = $this->connectHost($host); + if ($client !== null) { + $this->ldapClient = new Adldap(); + return; + } + } + + // Wasn't able to connect to any of the provided LDAP hosts + if ($this->ldapClient === null) { + throw new \Exception( + "failed to connect to " . $this->displayName . " host", + 1611157472 + ); + } + } + + /** + * @param string $host + * @return Adldap|null + */ + private function connectHost(string $host) + { + $client = new Adldap(); + $client->addProvider([ 'base_dn' => $this->baseDn, - 'hosts' => [$this->host], + 'hosts' => [$host], 'port' => $this->port, 'username' => $this->adminUsername, 'password' => $this->adminPassword, 'use_ssl' => $this->useSsl, 'use_tls' => $this->useTls, 'schema' => OpenLDAP::class, + 'timeout' => 3, ]); try { - $this->ldapProvider = $this->ldapClient->connect(); + $this->ldapProvider = $client->connect(); } catch (BindException $e) { - throw new \Exception($e->getDetailedError()); + $err = $e->getDetailedError(); + \Yii::warning([ + 'action' => 'ldap connect host', + 'status' => 'warning', + 'host' => $host, + 'ldap_code' => $err->getErrorCode(), + 'diagnostic' => $err->getDiagnosticMessage(), + 'message' => $err->getErrorMessage(), + ]); + return null; } + return $client; } /** From 5d0fd932b350eae9361ed0d488295c98262f7d10 Mon Sep 17 00:00:00 2001 From: Schparky <3172830+Schparky@users.noreply.github.com> Date: Wed, 20 Jan 2021 09:25:32 -0700 Subject: [PATCH 2/4] need to actually save the client connection! The previous "successful" test probably didn't actually connect to the host on later calls to `connect`. This time we verified the connection actually changed the test user's password. --- application/common/components/passwordStore/Ldap.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/common/components/passwordStore/Ldap.php b/application/common/components/passwordStore/Ldap.php index 0fa6cd7d..3509ebe3 100644 --- a/application/common/components/passwordStore/Ldap.php +++ b/application/common/components/passwordStore/Ldap.php @@ -100,7 +100,7 @@ public function connect() foreach ($this->host as $host) { $client = $this->connectHost($host); if ($client !== null) { - $this->ldapClient = new Adldap(); + $this->ldapClient = $client; return; } } From 9c4ebcf611da0571b557e418eb8221ed36beab49 Mon Sep 17 00:00:00 2001 From: Schparky <3172830+Schparky@users.noreply.github.com> Date: Wed, 20 Jan 2021 09:46:28 -0700 Subject: [PATCH 3/4] document the LDAP connection timeout --skip-ci --- application/common/components/passwordStore/Ldap.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/common/components/passwordStore/Ldap.php b/application/common/components/passwordStore/Ldap.php index 3509ebe3..41e6790c 100644 --- a/application/common/components/passwordStore/Ldap.php +++ b/application/common/components/passwordStore/Ldap.php @@ -130,7 +130,7 @@ private function connectHost(string $host) 'use_ssl' => $this->useSsl, 'use_tls' => $this->useTls, 'schema' => OpenLDAP::class, - 'timeout' => 3, + 'timeout' => 3, // set connection timeout to 3 seconds, default is 5 seconds ]); try { From abd7b151f296eef1fda2eaae2544189189d780df Mon Sep 17 00:00:00 2001 From: Schparky <3172830+Schparky@users.noreply.github.com> Date: Thu, 21 Jan 2021 16:28:03 -0700 Subject: [PATCH 4/4] add 5.4.0 to CHANGELOG --- CHANGELOG.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46232468..7f1f59b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +## [5.4.0] +### Added +- Allow LDAP host name to be a list of hostname strings as well as a single string for backward compatibility + ## [5.3.4] ### Fixed - Improved handling of expired session on login @@ -138,7 +142,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Added - Initial version of Password Manager Backend. -[Unreleased]: https://github.com/silinternational/idp-pw-api/compare/5.3.4...HEAD +[Unreleased]: https://github.com/silinternational/idp-pw-api/compare/5.4.0..HEAD +[5.4.0]: https://github.com/silinternational/idp-pw-api/compare/5.3.4..5.4.0 [5.3.4]: https://github.com/silinternational/idp-pw-api/compare/5.3.3..5.3.4 [5.3.3]: https://github.com/silinternational/idp-pw-api/compare/5.3.2..5.3.3 [5.3.2]: https://github.com/silinternational/idp-pw-api/compare/5.3.1..5.3.2