From 88652a217641f0967ee81fd4e1867d4a9b7cfd05 Mon Sep 17 00:00:00 2001
From: Remy Blom <remy.blom@hku.nl>
Date: Wed, 21 Jun 2017 16:38:29 +0200
Subject: [PATCH] bugfix: wrapped the building of authsource config with
issets. (#539)
* Adjusted the silent fail to log a warning when $this->getLdap() fails
the silent fail on searchformultiple(...) did not show anything in the log when actually it was the $this->getLdap() that failed.
* Bugfix: Wrapped the building of authsource config with issets
Not doing this gave me errors about ldap.port and ldap.timeout not being an integer (but NULL) from Configuration.php
Dec 23 08:28:10 simplesamlphp WARNING [94b0f44d76] AttributeAddFromLDAP: exception = exception 'Exception' with message 'ldap:AuthProcess: The option 'ldap.port' is not a valid integer value.' in /Users/remy/git/saml-IdP/lib/SimpleSAML/Configuration.php:737
Stack trace:
#0 /Users/remy/git/saml-IdP/modules/ldap/lib/Auth/Process/BaseFilter.php(267): SimpleSAML_Configuration->getInteger('ldap.port', 389)
#1 /Users/remy/git/saml-IdP/modules/ldap/lib/Auth/Process/AttributeAddFromLDAP.php(172): sspmod_ldap_Auth_Process_BaseFilter->getLdap()
...
* removed the @ as thijskh suggested...
* feature: AttributeCopy can take array for 1 attribute
* Revert "feature: AttributeCopy can take array for 1 attribute"
This reverts commit 78ccac061eab0fc4a0680e2aaf9ae07c3b6a29ac.
* BaseFilter.php: fix indent and added more isset checks...
* BaseFilter.php: removed an unneeded if ($authsource['search.enable'] ....
Since I moved this code into an if that already only gets executed when authsource['search.enable'] = true it is no longer needed in this check....
---
.../lib/Auth/Process/AttributeAddFromLDAP.php | 17 +++++-
modules/ldap/lib/Auth/Process/BaseFilter.php | 59 ++++++++++++++-----
2 files changed, 58 insertions(+), 18 deletions(-)
diff --git a/modules/ldap/lib/Auth/Process/AttributeAddFromLDAP.php b/modules/ldap/lib/Auth/Process/AttributeAddFromLDAP.php
index b1c5910cb..f124ecc8b 100644
--- a/modules/ldap/lib/Auth/Process/AttributeAddFromLDAP.php
+++ b/modules/ldap/lib/Auth/Process/AttributeAddFromLDAP.php
@@ -19,14 +19,17 @@
* - Updated the constructor to use the new config method
* - Updated the process method to use the new config variable names
* Updated: 20131119 Yørn de Jong / Jaime Perez
- * - Added support for retrieving multiple values at once from LDAP
- * - Don't crash but fail silently on LDAP errors; the plugin is to complement attributes
+ * - Added support for retrieving multiple values at once from LDAP
+ * - Don't crash but fail silently on LDAP errors; the plugin is to complement attributes
+ * Updated: 20161223 Remy Blom <remy.blom@hku.nl>
+ * - Adjusted the silent fail so it does show a warning in log when $this->getLdap() fails
*
* @author Yørn de Jong
* @author Jaime Perez
* @author Steve Moitozo
* @author JAARS, Inc.
* @author Ryan Panning
+ * @author Remy Blom <remy.blom@hku.nl>
* @package SimpleSAMLphp
*/
class sspmod_ldap_Auth_Process_AttributeAddFromLDAP extends sspmod_ldap_Auth_Process_BaseFilter
@@ -167,9 +170,17 @@ class sspmod_ldap_Auth_Process_AttributeAddFromLDAP extends sspmod_ldap_Auth_Pro
return;
}
+ // getLdap
+ try {
+ $ldap = $this->getLdap();
+ } catch (Exception $e) {
+ // Added this warning in case $this->getLdap() fails
+ SimpleSAML\Logger::warning("AttributeAddFromLDAP: exception = " . $e);
+ return;
+ }
// search for matching entries
try {
- $entries = $this->getLdap()->searchformultiple($this->base_dn, $filter,
+ $entries = $ldap->searchformultiple($this->base_dn, $filter,
array_values($this->search_attributes), true, false);
} catch (Exception $e) {
return; // silent fail, error is still logged by LDAP search
diff --git a/modules/ldap/lib/Auth/Process/BaseFilter.php b/modules/ldap/lib/Auth/Process/BaseFilter.php
index 22aa197bf..41261d244 100644
--- a/modules/ldap/lib/Auth/Process/BaseFilter.php
+++ b/modules/ldap/lib/Auth/Process/BaseFilter.php
@@ -5,7 +5,11 @@
* filter classes direct access to the authsource ldap config
* and connects to the ldap server.
*
+ * Updated: 20161223 Remy Blom
+ * - Wrapped the building of authsource config with issets
+ *
* @author Ryan Panning <panman@traileyes.com>
+ * @author Remy Blom <remy.blom@hku.nl>
* @package SimpleSAMLphp
*/
abstract class sspmod_ldap_Auth_Process_BaseFilter extends SimpleSAML_Auth_ProcessingFilter
@@ -137,21 +141,46 @@ abstract class sspmod_ldap_Auth_Process_BaseFilter extends SimpleSAML_Auth_Proce
// Build the authsource config
$authconfig = array();
- $authconfig['ldap.hostname'] = @$authsource['hostname'];
- $authconfig['ldap.enable_tls'] = @$authsource['enable_tls'];
- $authconfig['ldap.port'] = @$authsource['port'];
- $authconfig['ldap.timeout'] = @$authsource['timeout'];
- $authconfig['ldap.debug'] = @$authsource['debug'];
- $authconfig['ldap.basedn'] = (@$authsource['search.enable'] ? @$authsource['search.base'] : null);
- $authconfig['ldap.username'] = (@$authsource['search.enable'] ? @$authsource['search.username'] : null);
- $authconfig['ldap.password'] = (@$authsource['search.enable'] ? @$authsource['search.password'] : null);
- $authconfig['ldap.username'] = (@$authsource['priv.read'] ? @$authsource['priv.username'] : $authconfig['ldap.username']);
- $authconfig['ldap.password'] = (@$authsource['priv.read'] ? @$authsource['priv.password'] : $authconfig['ldap.password']);
-
- // Only set the username attribute if the authsource specifies one attribute
- if (@$authsource['search.enable'] && is_array(@$authsource['search.attributes'])
- && count($authsource['search.attributes']) == 1) {
- $authconfig['attribute.username'] = reset($authsource['search.attributes']);
+ if (isset($authsource['hostname'])) {
+ $authconfig['ldap.hostname'] = $authsource['hostname'];
+ }
+ if (isset($authsource['enable_tls'])) {
+ $authconfig['ldap.enable_tls'] = $authsource['enable_tls'];
+ }
+ if (isset($authsource['port'])) {
+ $authconfig['ldap.port'] = $authsource['port'];
+ }
+ if (isset($authsource['timeout'])) {
+ $authconfig['ldap.timeout'] = $authsource['timeout'];
+ }
+ if (isset($authsource['debug'])) {
+ $authconfig['ldap.debug'] = $authsource['debug'];
+ }
+ // only set when search.enabled = true
+ if (isset($authsource['search.enable']) && $authsource['search.enable']) {
+ if (isset($authsource['search.base'])) {
+ $authconfig['ldap.basedn'] = $authsource['search.base'];
+ }
+ if (isset($authsource['search.username'])) {
+ $authconfig['ldap.username'] = $authsource['search.username'];
+ }
+ if (isset($authsource['search.password'])) {
+ $authconfig['ldap.password'] = $authsource['search.password'];
+ }
+ // Only set the username attribute if the authsource specifies one attribute
+ if (isset($authsource['search.attributes']) && is_array($authsource['search.attributes'])
+ && count($authsource['search.attributes']) == 1) {
+ $authconfig['attribute.username'] = reset($authsource['search.attributes']);
+ }
+ }
+ // only set when priv.read = true
+ if (isset($authsource['priv.read']) && $authsource['priv.read']) {
+ if (isset($authsource['priv.username'])) {
+ $authconfig['ldap.username'] = $authsource['priv.username'];
+ }
+ if (isset($authsource['priv.password'])) {
+ $authconfig['ldap.password'] = $authsource['priv.password'];
+ }
}
// Merge the authsource config with the filter config,
--
GitLab