From 9b826e6e5fbbd649bcec67a4f4ca153b09f98828 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Mon, 9 Nov 2015 13:04:02 +0100 Subject: [PATCH 01/13] Drop class Ldap\Expression and introduce LdapQuery::$nativeFilter I'm about to add support for our Data\Filter implementation, since it cannot parse native LDAP filters and a user may have configured such, we need to differentiate the two types of filter. refs #10370 --- .../Authentication/User/LdapUserBackend.php | 3 +- .../UserGroup/LdapUserGroupBackend.php | 17 +++-- library/Icinga/Protocol/Ldap/Expression.php | 30 --------- library/Icinga/Protocol/Ldap/LdapQuery.php | 62 +++++++++++++------ 4 files changed, 53 insertions(+), 59 deletions(-) delete mode 100644 library/Icinga/Protocol/Ldap/Expression.php diff --git a/library/Icinga/Authentication/User/LdapUserBackend.php b/library/Icinga/Authentication/User/LdapUserBackend.php index b5bd51f45..b26b8aad7 100644 --- a/library/Icinga/Authentication/User/LdapUserBackend.php +++ b/library/Icinga/Authentication/User/LdapUserBackend.php @@ -12,7 +12,6 @@ use Icinga\Exception\ProgrammingError; use Icinga\Repository\LdapRepository; use Icinga\Repository\RepositoryQuery; use Icinga\Protocol\Ldap\LdapException; -use Icinga\Protocol\Ldap\Expression; use Icinga\User; class LdapUserBackend extends LdapRepository implements UserBackendInterface, Inspectable @@ -203,7 +202,7 @@ class LdapUserBackend extends LdapRepository implements UserBackendInterface, In $query = parent::select($columns); $query->getQuery()->setBase($this->baseDn); if ($this->filter) { - $query->getQuery()->where(new Expression($this->filter)); + $query->getQuery()->setNativeFilter($this->filter); } return $query; diff --git a/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php b/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php index 9a4ab53c0..4c11d1a68 100644 --- a/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php +++ b/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php @@ -9,7 +9,6 @@ use Icinga\Application\Logger; use Icinga\Data\ConfigObject; use Icinga\Exception\ConfigurationError; use Icinga\Exception\ProgrammingError; -use Icinga\Protocol\Ldap\Expression; use Icinga\Repository\LdapRepository; use Icinga\Repository\RepositoryQuery; use Icinga\User; @@ -368,11 +367,6 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt { $query = parent::select($columns); $query->getQuery()->setBase($this->groupBaseDn); - if ($this->groupFilter) { - // TODO(jom): This should differentiate between groups and their memberships - $query->getQuery()->where(new Expression($this->groupFilter)); - } - return $query; } @@ -529,7 +523,12 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt public function requireTable($table, RepositoryQuery $query = null) { $table = parent::requireTable($table, $query); - if ($table === 'group' || $table === 'group_membership') { + if ($table === 'group') { + $table = $this->groupClass; + if ($query !== null && $this->groupFilter) { + $query->getQuery()->setNativeFilter($this->groupFilter); + } + } elseif ($table === 'group_memership') { $table = $this->groupClass; } @@ -576,7 +575,7 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt ->setBase($this->userBaseDn) ->setUsePagedResults(false); if ($this->userFilter) { - $userQuery->where(new Expression($this->userFilter)); + $userQuery->setNativeFilter($this->userFilter); } if (($queryValue = $userQuery->fetchDn()) === null) { @@ -590,7 +589,7 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt ->where($this->groupMemberAttribute, $queryValue) ->setBase($this->groupBaseDn); if ($this->groupFilter) { - $groupQuery->where(new Expression($this->groupFilter)); + $groupQuery->setNativeFilter($this->groupFilter); } $groups = array(); diff --git a/library/Icinga/Protocol/Ldap/Expression.php b/library/Icinga/Protocol/Ldap/Expression.php deleted file mode 100644 index 403e1fd04..000000000 --- a/library/Icinga/Protocol/Ldap/Expression.php +++ /dev/null @@ -1,30 +0,0 @@ -value = $value; - } - - public function setValue($value) - { - $this->value = $value; - return $this; - } - - public function getValue() - { - return $this->value; - } - - public function __toString() - { - return (string) $this->getValue(); - } -} diff --git a/library/Icinga/Protocol/Ldap/LdapQuery.php b/library/Icinga/Protocol/Ldap/LdapQuery.php index 2832908ea..3696062ef 100644 --- a/library/Icinga/Protocol/Ldap/LdapQuery.php +++ b/library/Icinga/Protocol/Ldap/LdapQuery.php @@ -42,6 +42,13 @@ class LdapQuery extends SimpleQuery */ protected $unfoldAttribute; + /** + * This query's native LDAP filter + * + * @var string + */ + protected $nativeFilter; + /** * Initialize this query */ @@ -120,6 +127,29 @@ class LdapQuery extends SimpleQuery return $this->unfoldAttribute; } + /** + * Set this query's native LDAP filter + * + * @param string $filter + * + * @return $this + */ + public function setNativeFilter($filter) + { + $this->nativeFilter = $filter; + return $this; + } + + /** + * Return this query's native LDAP filter + * + * @return string + */ + public function getNativeFilter() + { + return $this->nativeFilter; + } + /** * Choose an objectClass and the columns you are interested in * @@ -141,13 +171,7 @@ class LdapQuery extends SimpleQuery */ public function where($condition, $value = null) { - // TODO: Adjust this once support for Icinga\Data\Filter is available - if ($condition instanceof Expression) { - $this->filters[] = $condition; - } else { - $this->filters[$condition] = $value; - } - + $this->filters[$condition] = $value; return $this; } @@ -239,22 +263,24 @@ class LdapQuery extends SimpleQuery $parts = array(); foreach ($this->filters as $key => $value) { - if ($value instanceof Expression) { - $parts[] = (string) $value; - } else { - $parts[] = sprintf( - '%s=%s', - LdapUtils::quoteForSearch($key), - LdapUtils::quoteForSearch($value, true) - ); - } + $parts[] = sprintf( + '%s=%s', + LdapUtils::quoteForSearch($key), + LdapUtils::quoteForSearch($value, true) + ); } if (count($parts) > 1) { - return '(&(' . implode(')(', $parts) . '))'; + $filter = '(&(' . implode(')(', $parts) . '))'; } else { - return '(' . $parts[0] . ')'; + $filter = '(' . $parts[0] . ')'; } + + if ($this->nativeFilter) { + $filter = '(&(' . $this->nativeFilter . ')' . $filter . ')'; + } + + return $filter; } /** From 4341eef4b10a0a6101a1f87b70d5394b2197b5bb Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Mon, 9 Nov 2015 15:59:48 +0100 Subject: [PATCH 02/13] LdapQuery: Add support for Icinga\Data\Filter refs #10370 --- .../Icinga/Protocol/Ldap/LdapConnection.php | 92 ++++++++++++++++++- library/Icinga/Protocol/Ldap/LdapQuery.php | 77 +--------------- 2 files changed, 94 insertions(+), 75 deletions(-) diff --git a/library/Icinga/Protocol/Ldap/LdapConnection.php b/library/Icinga/Protocol/Ldap/LdapConnection.php index ceefc81b8..fe18c100e 100644 --- a/library/Icinga/Protocol/Ldap/LdapConnection.php +++ b/library/Icinga/Protocol/Ldap/LdapConnection.php @@ -7,13 +7,14 @@ use Exception; use ArrayIterator; use Icinga\Application\Config; use Icinga\Application\Logger; -use Icinga\Application\Platform; use Icinga\Data\ConfigObject; use Icinga\Data\Inspectable; use Icinga\Data\Inspection; use Icinga\Data\Selectable; use Icinga\Data\Sortable; -use Icinga\Exception\InspectionException; +use Icinga\Data\Filter\Filter; +use Icinga\Data\Filter\FilterChain; +use Icinga\Data\Filter\FilterExpression; use Icinga\Exception\ProgrammingError; use Icinga\Protocol\Ldap\LdapException; @@ -1184,6 +1185,93 @@ class LdapConnection implements Selectable, Inspectable return $dir; } + /** + * Render and return a valid LDAP filter representation of the given filter + * + * @param Filter $filter + * @param int $level + * + * @return string + */ + public function renderFilter(Filter $filter, $level = 0) + { + if ($filter->isExpression()) { + /** @var $filter FilterExpression */ + return $this->renderFilterExpression($filter); + } + + /** @var $filter FilterChain */ + $parts = array(); + foreach ($filter->filters() as $filterPart) { + $part = $this->renderFilter($filterPart, $level + 1); + if ($part) { + $parts[] = $part; + } + } + + if (empty($parts)) { + return ''; + } + + $format = '%1$s(%2$s)'; + if (count($parts) === 1) { + $format = '%2$s'; + } + if ($level === 0) { + $format = '(' . $format . ')'; + } + + return sprintf($format, $filter->getOperatorSymbol(), implode(')(', $parts)); + } + + /** + * Render and return a valid LDAP filter expression of the given filter + * + * @param FilterExpression $filter + * + * @return string + */ + protected function renderFilterExpression(FilterExpression $filter) + { + $column = $filter->getColumn(); + $sign = $filter->getSign(); + $expression = $filter->getExpression(); + $format = '%1$s%2$s%3$s'; + + if ($expression === null || $expression === true) { + $expression = '*'; + } elseif (is_array($expression)) { + $seqFormat = '|(%s)'; + if ($sign === '!=') { + $seqFormat = '!(' . $seqFormat . ')'; + $sign = '='; + } + + $seqParts = array(); + foreach ($expression as $expressionValue) { + $seqParts[] = sprintf( + $format, + LdapUtils::quoteForSearch($column), + $sign, + LdapUtils::quoteForSearch($expressionValue, true) + ); + } + + return sprintf($seqFormat, implode(')(', $seqParts)); + } + + if ($sign === '!=') { + $format = '!(%1$s=%3$s)'; + } + + return sprintf( + $format, + LdapUtils::quoteForSearch($column), + $sign, + LdapUtils::quoteForSearch($expression, true) + ); + } + /** * Inspect if this LDAP Connection is working as expected * diff --git a/library/Icinga/Protocol/Ldap/LdapQuery.php b/library/Icinga/Protocol/Ldap/LdapQuery.php index 3696062ef..bf37f72dd 100644 --- a/library/Icinga/Protocol/Ldap/LdapQuery.php +++ b/library/Icinga/Protocol/Ldap/LdapQuery.php @@ -4,23 +4,12 @@ namespace Icinga\Protocol\Ldap; use Icinga\Data\SimpleQuery; -use Icinga\Data\Filter\Filter; -use Icinga\Exception\NotImplementedError; /** * LDAP query class */ class LdapQuery extends SimpleQuery { - /** - * This query's filters - * - * Currently just a basic key/value pair based array. Can be removed once Icinga\Data\Filter is supported. - * - * @var array - */ - protected $filters; - /** * The base dn being used for this query * @@ -54,7 +43,6 @@ class LdapQuery extends SimpleQuery */ protected function init() { - $this->filters = array(); $this->usePagedResults = false; } @@ -157,47 +145,10 @@ class LdapQuery extends SimpleQuery */ public function from($target, array $fields = null) { - $this->filters['objectClass'] = $target; + $this->where('objectClass', $target); return parent::from($target, $fields); } - /** - * Add a new filter to the query - * - * @param string $condition Column to search in - * @param mixed $value Value to look for (asterisk wildcards are allowed) - * - * @return $this - */ - public function where($condition, $value = null) - { - $this->filters[$condition] = $value; - return $this; - } - - public function getFilter() - { - throw new NotImplementedError('Support for Icinga\Data\Filter is still missing. Use $this->where() instead'); - } - - public function addFilter(Filter $filter) - { - // TODO: This should be considered a quick fix only. - // Drop this entirely once support for Icinga\Data\Filter is available - if ($filter->isExpression()) { - $this->where($filter->getColumn(), $filter->getExpression()); - } elseif ($filter->isChain()) { - foreach ($filter->filters() as $chainOrExpression) { - $this->addFilter($chainOrExpression); - } - } - } - - public function setFilter(Filter $filter) - { - throw new NotImplementedError('Support for Icinga\Data\Filter is still missing. Use $this->where() instead'); - } - /** * Fetch result as tree * @@ -249,33 +200,13 @@ class LdapQuery extends SimpleQuery } /** - * Return the LDAP filter to be applied on this query + * Render and return this query's filter * * @return string - * - * @throws LdapException In case the objectClass filter does not exist */ - protected function renderFilter() + public function renderFilter() { - if (! isset($this->filters['objectClass'])) { - throw new LdapException('Object class is mandatory'); - } - - $parts = array(); - foreach ($this->filters as $key => $value) { - $parts[] = sprintf( - '%s=%s', - LdapUtils::quoteForSearch($key), - LdapUtils::quoteForSearch($value, true) - ); - } - - if (count($parts) > 1) { - $filter = '(&(' . implode(')(', $parts) . '))'; - } else { - $filter = '(' . $parts[0] . ')'; - } - + $filter = $this->ds->renderFilter($this->filter); if ($this->nativeFilter) { $filter = '(&(' . $this->nativeFilter . ')' . $filter . ')'; } From ffcc2ed56b868462fbc74a9b0ba8608ac0ac0c7e Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Mon, 9 Nov 2015 16:00:24 +0100 Subject: [PATCH 03/13] LdapUserGroupBackend: Fix exception when searching for single chars refs #10370 --- .../UserGroup/LdapUserGroupBackend.php | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php b/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php index 4c11d1a68..9b7aaf691 100644 --- a/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php +++ b/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php @@ -9,6 +9,7 @@ use Icinga\Application\Logger; use Icinga\Data\ConfigObject; use Icinga\Exception\ConfigurationError; use Icinga\Exception\ProgrammingError; +use Icinga\Protocol\Ldap\LdapException; use Icinga\Repository\LdapRepository; use Icinga\Repository\RepositoryQuery; use Icinga\User; @@ -467,24 +468,28 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt */ protected function persistUserName($name) { - $userDn = $this->ds - ->select() - ->from($this->userClass, array()) - ->where($this->userNameAttribute, $name) - ->setUsePagedResults(false) - ->fetchDn(); - if ($userDn) { - return $userDn; - } + try { + $userDn = $this->ds + ->select() + ->from($this->userClass, array()) + ->where($this->userNameAttribute, $name) + ->setUsePagedResults(false) + ->fetchDn(); + if ($userDn) { + return $userDn; + } - $groupDn = $this->ds - ->select() - ->from($this->groupClass, array()) - ->where($this->groupNameAttribute, $name) - ->setUsePagedResults(false) - ->fetchDn(); - if ($groupDn) { - return $groupDn; + $groupDn = $this->ds + ->select() + ->from($this->groupClass, array()) + ->where($this->groupNameAttribute, $name) + ->setUsePagedResults(false) + ->fetchDn(); + if ($groupDn) { + return $groupDn; + } + } catch (LdapException $_) { + // pass } Logger::debug('Unable to persist uid or gid "%s" in repository "%s". No DN found.', $name, $this->getName()); From c416216822e5183f8279673d804e08e3c0fd601e Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Mon, 9 Nov 2015 16:00:55 +0100 Subject: [PATCH 04/13] LdapUserGroupBackend: Fix typo in method requireTable() refs #10370 --- .../Icinga/Authentication/UserGroup/LdapUserGroupBackend.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php b/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php index 9b7aaf691..50365a3ef 100644 --- a/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php +++ b/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php @@ -533,7 +533,7 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt if ($query !== null && $this->groupFilter) { $query->getQuery()->setNativeFilter($this->groupFilter); } - } elseif ($table === 'group_memership') { + } elseif ($table === 'group_membership') { $table = $this->groupClass; } From 505f5902c7b70e3bd8ed464ef94f75e5af65f8e0 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 10 Nov 2015 09:56:27 +0100 Subject: [PATCH 05/13] LdapUserBackend: Utilize $virtualTables --- .../Authentication/User/LdapUserBackend.php | 41 ++++++++----------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/library/Icinga/Authentication/User/LdapUserBackend.php b/library/Icinga/Authentication/User/LdapUserBackend.php index b26b8aad7..930ac4c99 100644 --- a/library/Icinga/Authentication/User/LdapUserBackend.php +++ b/library/Icinga/Authentication/User/LdapUserBackend.php @@ -208,6 +208,24 @@ class LdapUserBackend extends LdapRepository implements UserBackendInterface, In return $query; } + /** + * Initialize this repository's virtual tables + * + * @return array + * + * @throws ProgrammingError In case $this->userClass has not been set yet + */ + protected function initializeVirtualTables() + { + if ($this->userClass === null) { + throw new ProgrammingError('It is required to set the object class where to find users first'); + } + + return array( + 'user' => $this->userClass + ); + } + /** * Initialize this repository's query columns * @@ -317,29 +335,6 @@ class LdapUserBackend extends LdapRepository implements UserBackendInterface, In return ((int) $value) >= $bigBang->diff($now)->days; } - /** - * Validate that the requested table exists - * - * This will return $this->userClass in case $table equals "user". - * - * @param string $table The table to validate - * @param RepositoryQuery $query An optional query to pass as context - * (unused by the base implementation) - * - * @return string - * - * @throws ProgrammingError In case the given table does not exist - */ - public function requireTable($table, RepositoryQuery $query = null) - { - $table = parent::requireTable($table, $query); - if ($table === 'user') { - $table = $this->userClass; - } - - return $table; - } - /** * Validate that the given column is a valid query target and return it or the actual name if it's an alias * From d56056bba79f9412a8d4c6f3e955a64546be5a88 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 10 Nov 2015 09:56:58 +0100 Subject: [PATCH 06/13] LdapUserGroupBackend: Utilize $virtualTables --- .../UserGroup/LdapUserGroupBackend.php | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php b/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php index 50365a3ef..e0e957ee3 100644 --- a/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php +++ b/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php @@ -371,6 +371,25 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt return $query; } + /** + * Initialize this repository's virtual tables + * + * @return array + * + * @throws ProgrammingError In case $this->groupClass has not been set yet + */ + protected function initializeVirtualTables() + { + if ($this->groupClass === null) { + throw new ProgrammingError('It is required to set the object class where to find groups first'); + } + + return array( + 'group' => $this->groupClass, + 'group_membership' => $this->groupClass + ); + } + /** * Initialize this repository's query columns * @@ -515,11 +534,8 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt /** * Validate that the requested table exists * - * This will return $this->groupClass in case $table equals "group" or "group_membership". - * * @param string $table The table to validate * @param RepositoryQuery $query An optional query to pass as context - * (unused by the base implementation) * * @return string * @@ -527,17 +543,11 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt */ public function requireTable($table, RepositoryQuery $query = null) { - $table = parent::requireTable($table, $query); - if ($table === 'group') { - $table = $this->groupClass; - if ($query !== null && $this->groupFilter) { - $query->getQuery()->setNativeFilter($this->groupFilter); - } - } elseif ($table === 'group_membership') { - $table = $this->groupClass; + if ($query !== null && $table === 'group' && $this->groupFilter) { + $query->getQuery()->setNativeFilter($this->groupFilter); } - return $table; + return parent::requireTable($table, $query); } /** From 8d04c8548ad220a309dc5b04b1db278b16981b93 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 10 Nov 2015 11:51:26 +0100 Subject: [PATCH 07/13] Do not hardcode action specific parameters to preserve in the FilterEditor This should only happen for other control parameters or framework specific stuff. This is still subject to improvement, as this solution is rather ugly imho.. refs #10370 --- application/controllers/GroupController.php | 2 +- application/controllers/UserController.php | 3 +- library/Icinga/Web/Controller.php | 36 ++++++++++--------- .../controllers/ListController.php | 7 +++- 4 files changed, 28 insertions(+), 20 deletions(-) diff --git a/application/controllers/GroupController.php b/application/controllers/GroupController.php index 21fba5ebb..949aa8fc6 100644 --- a/application/controllers/GroupController.php +++ b/application/controllers/GroupController.php @@ -97,7 +97,7 @@ class GroupController extends AuthBackendController ->from('group_membership', array('user_name')) ->where('group_name', $groupName); - $this->setupFilterControl($members, null, array('user')); + $this->setupFilterControl($members, null, array('user'), array('group')); $this->setupPaginationControl($members); $this->setupLimitControl(); $this->setupSortControl( diff --git a/application/controllers/UserController.php b/application/controllers/UserController.php index 69d4a71a5..573d85c93 100644 --- a/application/controllers/UserController.php +++ b/application/controllers/UserController.php @@ -99,7 +99,8 @@ class UserController extends AuthBackendController $this->setupFilterControl( $memberships, array('group_name' => t('User Group')), - array('group_name') + array('group_name'), + array('user') ); $this->setupPaginationControl($memberships); $this->setupLimitControl(); diff --git a/library/Icinga/Web/Controller.php b/library/Icinga/Web/Controller.php index ac92e45b6..891b70329 100644 --- a/library/Icinga/Web/Controller.php +++ b/library/Icinga/Web/Controller.php @@ -186,6 +186,7 @@ class Controller extends ModuleActionController * @param Filterable $filterable The filterable to create a filter editor for * @param array $filterColumns The filter columns to offer to the user * @param array $searchColumns The search columns to utilize for quick searches + * @param array $preserveParams The url parameters to preserve * * @return $this * @@ -194,25 +195,26 @@ class Controller extends ModuleActionController protected function setupFilterControl( Filterable $filterable, array $filterColumns = null, - array $searchColumns = null + array $searchColumns = null, + array $preserveParams = null ) { - $editor = Widget::create('filterEditor') + $defaultPreservedParams = array( + 'limit', // setupPaginationControl() + 'sort', // setupSortControl() + 'dir', // setupSortControl() + 'backend', // Framework + '_dev' // Framework + ); + + $editor = Widget::create('filterEditor'); + call_user_func_array( + array($editor, 'preserveParams'), + array_merge($defaultPreservedParams, $preserveParams ?: array()) + ); + + $editor ->setQuery($filterable) - ->preserveParams( - 'limit', - 'sort', - 'dir', - 'format', - 'view', - 'user', - 'group', - 'backend', - 'stateType', - 'addColumns', - 'problems', - '_dev' - ) - ->ignoreParams('page') + ->ignoreParams('page') // setupPaginationControl() ->setColumns($filterColumns) ->setSearchColumns($searchColumns) ->handleRequest($this->getRequest()); diff --git a/modules/monitoring/application/controllers/ListController.php b/modules/monitoring/application/controllers/ListController.php index d08257557..c0ae1133f 100644 --- a/modules/monitoring/application/controllers/ListController.php +++ b/modules/monitoring/application/controllers/ListController.php @@ -574,7 +574,12 @@ class ListController extends Controller */ protected function filterQuery(DataView $dataView) { - $this->setupFilterControl($dataView); + $this->setupFilterControl($dataView, null, null, array( + 'format', // handleFormatRequest() + 'stateType', // hostsAction() and servicesAction() + 'addColumns', // addColumns() + 'problems' // servicegridAction() + )); $this->handleFormatRequest($dataView); return $dataView; } From 72f3ba116191401195c5e355b111f3f31c687998 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 10 Nov 2015 11:52:06 +0100 Subject: [PATCH 08/13] LdapUserGroupBackend: Offer "user_name" as filter column instead of "user" refs #10370 --- .../Icinga/Authentication/UserGroup/LdapUserGroupBackend.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php b/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php index e0e957ee3..fab3d6bea 100644 --- a/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php +++ b/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php @@ -434,7 +434,7 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt protected function initializeFilterColumns() { return array( - t('Username') => 'user', + t('Username') => 'user_name', t('User Group') => 'group_name', t('Created At') => 'created_at', t('Last Modified') => 'last_modified' From 7efefc1975564d606c01b1aa629abe4e794fdcab Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 10 Nov 2015 11:52:54 +0100 Subject: [PATCH 09/13] UserController: Use "group" instead of "group_name" for the membership quicksearch refs #10370 --- application/controllers/UserController.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/application/controllers/UserController.php b/application/controllers/UserController.php index 573d85c93..ae7cf841a 100644 --- a/application/controllers/UserController.php +++ b/application/controllers/UserController.php @@ -99,7 +99,7 @@ class UserController extends AuthBackendController $this->setupFilterControl( $memberships, array('group_name' => t('User Group')), - array('group_name'), + array('group'), array('user') ); $this->setupPaginationControl($memberships); @@ -260,6 +260,7 @@ class UserController extends AuthBackendController $alreadySeen[$groupName] = null; $groups[] = (object) array( 'group_name' => $groupName, + 'group' => $groupName, 'backend' => $backend ); } From 6c07881466804bd9748eecdb6012187879e76e41 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 10 Nov 2015 13:16:40 +0100 Subject: [PATCH 10/13] FilterChain: Fix and document method listFilterColumns() refs #10370 --- library/Icinga/Data/Filter/FilterChain.php | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/library/Icinga/Data/Filter/FilterChain.php b/library/Icinga/Data/Filter/FilterChain.php index 51d511785..c6354e402 100644 --- a/library/Icinga/Data/Filter/FilterChain.php +++ b/library/Icinga/Data/Filter/FilterChain.php @@ -125,17 +125,26 @@ abstract class FilterChain extends Filter return $this; } - public function listFilteredColumns() + /** + * List and return all column names referenced in this filter + * + * @param array $columns The columns listed so far + * + * @return array + */ + public function listFilteredColumns(array $columns = array()) { - $columns = array(); foreach ($this->filters as $filter) { if ($filter instanceof FilterExpression) { - $columns[] = $filter->getColumn(); + $column= $filter->getColumn(); + if (! in_array($column, $columns, true)) { + $columns[] = $column; + } } else { - $columns += $filter->listFilteredColumns(); + $columns = $filter->listFilteredColumns($columns); } } - // array_unique? + return $columns; } From 666e67b405ff065f03148e48f00d4964b93c22a8 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 10 Nov 2015 13:17:30 +0100 Subject: [PATCH 11/13] LdapConnection: Prefer strict checks when utilizing in_array() --- library/Icinga/Protocol/Ldap/LdapConnection.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/library/Icinga/Protocol/Ldap/LdapConnection.php b/library/Icinga/Protocol/Ldap/LdapConnection.php index fe18c100e..95d21b987 100644 --- a/library/Icinga/Protocol/Ldap/LdapConnection.php +++ b/library/Icinga/Protocol/Ldap/LdapConnection.php @@ -695,7 +695,7 @@ class LdapConnection implements Selectable, Inspectable )); } else { foreach ($query->getOrder() as $rule) { - if (! in_array($rule[0], $fields)) { + if (! in_array($rule[0], $fields, true)) { $fields[] = $rule[0]; } } @@ -812,7 +812,7 @@ class LdapConnection implements Selectable, Inspectable $serverSorting = false;//$this->getCapabilities()->hasOid(LdapCapabilities::LDAP_SERVER_SORT_OID); if (! $serverSorting && $query->hasOrder()) { foreach ($query->getOrder() as $rule) { - if (! in_array($rule[0], $fields)) { + if (! in_array($rule[0], $fields, true)) { $fields[] = $rule[0]; } } @@ -858,7 +858,8 @@ class LdapConnection implements Selectable, Inspectable } elseif (ldap_count_entries($ds, $results) === 0) { if (in_array( ldap_errno($ds), - array(static::LDAP_SIZELIMIT_EXCEEDED, static::LDAP_ADMINLIMIT_EXCEEDED) + array(static::LDAP_SIZELIMIT_EXCEEDED, static::LDAP_ADMINLIMIT_EXCEEDED), + true )) { Logger::warning( 'Unable to request more than %u results. Does the server allow paged search requests? (%s)', From e408630e343313741e95b79625c0511c34a1b555 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 10 Nov 2015 13:41:08 +0100 Subject: [PATCH 12/13] LdapConnection: Do not require calling array_flip for method cleanupAttributes() Seems to be a relict of an earlier implementation.. refs #10370 --- .../Icinga/Protocol/Ldap/LdapCapabilities.php | 8 +--- .../Icinga/Protocol/Ldap/LdapConnection.php | 41 +++++++++++-------- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/library/Icinga/Protocol/Ldap/LdapCapabilities.php b/library/Icinga/Protocol/Ldap/LdapCapabilities.php index 1ee64eaba..596618d79 100644 --- a/library/Icinga/Protocol/Ldap/LdapCapabilities.php +++ b/library/Icinga/Protocol/Ldap/LdapCapabilities.php @@ -308,12 +308,8 @@ class LdapCapabilities ldap_error($ds) ); } - $cap = new LdapCapabilities( - $connection->cleanupAttributes( - ldap_get_attributes($ds, $entry), - array_flip($fields) - ) - ); + + $cap = new LdapCapabilities($connection->cleanupAttributes(ldap_get_attributes($ds, $entry), $fields)); return $cap; } diff --git a/library/Icinga/Protocol/Ldap/LdapConnection.php b/library/Icinga/Protocol/Ldap/LdapConnection.php index 95d21b987..aa9a1c07e 100644 --- a/library/Icinga/Protocol/Ldap/LdapConnection.php +++ b/library/Icinga/Protocol/Ldap/LdapConnection.php @@ -731,12 +731,7 @@ class LdapConnection implements Selectable, Inspectable $unfoldAttribute = $query->getUnfoldAttribute(); do { if ($unfoldAttribute) { - $rows = $this->cleanupAttributes( - ldap_get_attributes($ds, $entry), - array_flip($fields), - $unfoldAttribute - ); - + $rows = $this->cleanupAttributes(ldap_get_attributes($ds, $entry), $fields, $unfoldAttribute); if (is_array($rows)) { // TODO: Register the DN the same way as a section name in the ArrayDatasource! foreach ($rows as $row) { @@ -760,7 +755,7 @@ class LdapConnection implements Selectable, Inspectable if (! $serverSorting || $offset === 0 || $offset < $count) { $entries[ldap_get_dn($ds, $entry)] = $this->cleanupAttributes( ldap_get_attributes($ds, $entry), - array_flip($fields) + $fields ); } } @@ -874,12 +869,7 @@ class LdapConnection implements Selectable, Inspectable $entry = ldap_first_entry($ds, $results); do { if ($unfoldAttribute) { - $rows = $this->cleanupAttributes( - ldap_get_attributes($ds, $entry), - array_flip($fields), - $unfoldAttribute - ); - + $rows = $this->cleanupAttributes(ldap_get_attributes($ds, $entry), $fields, $unfoldAttribute); if (is_array($rows)) { // TODO: Register the DN the same way as a section name in the ArrayDatasource! foreach ($rows as $row) { @@ -903,7 +893,7 @@ class LdapConnection implements Selectable, Inspectable if (! $serverSorting || $offset === 0 || $offset < $count) { $entries[ldap_get_dn($ds, $entry)] = $this->cleanupAttributes( ldap_get_attributes($ds, $entry), - array_flip($fields) + $fields ); } } @@ -965,8 +955,17 @@ class LdapConnection implements Selectable, Inspectable // necessary to create another array to map attributes case insensitively to their requested counterparts. // This does also apply the virtual alias handling. (Since an LDAP server does not handle such) $loweredFieldMap = array(); - foreach ($requestedFields as $name => $alias) { - $loweredFieldMap[strtolower($name)] = is_string($alias) ? $alias : $name; + foreach ($requestedFields as $alias => $name) { + $loweredName = strtolower($name); + if (isset($loweredFieldMap[$loweredName])) { + if (! is_array($loweredFieldMap[$loweredName])) { + $loweredFieldMap[$loweredName] = array($loweredFieldMap[$loweredName]); + } + + $loweredFieldMap[$loweredName][] = is_string($alias) ? $alias : $name; + } else { + $loweredFieldMap[$loweredName] = is_string($alias) ? $alias : $name; + } } $cleanedAttributes = array(); @@ -984,12 +983,18 @@ class LdapConnection implements Selectable, Inspectable $requestedAttributeName = isset($loweredFieldMap[strtolower($attribute_name)]) ? $loweredFieldMap[strtolower($attribute_name)] : $attribute_name; - $cleanedAttributes[$requestedAttributeName] = $attribute_value; + if (is_array($requestedAttributeName)) { + foreach ($requestedAttributeName as $requestedName) { + $cleanedAttributes[$requestedName] = $attribute_value; + } + } else { + $cleanedAttributes[$requestedAttributeName] = $attribute_value; + } } // The result may not contain all requested fields, so populate the cleaned // result with the missing fields and their value being set to null - foreach ($requestedFields as $name => $alias) { + foreach ($requestedFields as $alias => $name) { if (! is_string($alias)) { $alias = $name; } From cee639d6892e2ab0d860de8fccc67eaff235f815 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 10 Nov 2015 14:03:08 +0100 Subject: [PATCH 13/13] LdapConnection: Re-apply a query's filter on unfolded rows refs #10370 --- .../Icinga/Protocol/Ldap/LdapConnection.php | 70 ++++++++++++++----- 1 file changed, 54 insertions(+), 16 deletions(-) diff --git a/library/Icinga/Protocol/Ldap/LdapConnection.php b/library/Icinga/Protocol/Ldap/LdapConnection.php index aa9a1c07e..9158dc5a4 100644 --- a/library/Icinga/Protocol/Ldap/LdapConnection.php +++ b/library/Icinga/Protocol/Ldap/LdapConnection.php @@ -702,11 +702,23 @@ class LdapConnection implements Selectable, Inspectable } } + $unfoldAttribute = $query->getUnfoldAttribute(); + if ($unfoldAttribute) { + foreach ($query->getFilter()->listFilteredColumns() as $filterColumn) { + $fieldKey = array_search($filterColumn, $fields, true); + if ($fieldKey === false || is_string($fieldKey)) { + $fields[] = $filterColumn; + } + } + } + $results = @ldap_search( $ds, $query->getBase() ?: $this->rootDn, (string) $query, - array_values($fields), + $unfoldAttribute + ? array_unique(array_values($fields)) + : array_values($fields), 0, // Attributes and values $serverSorting && $limit ? $offset + $limit : 0 ); @@ -728,20 +740,21 @@ class LdapConnection implements Selectable, Inspectable $count = 0; $entries = array(); $entry = ldap_first_entry($ds, $results); - $unfoldAttribute = $query->getUnfoldAttribute(); do { if ($unfoldAttribute) { $rows = $this->cleanupAttributes(ldap_get_attributes($ds, $entry), $fields, $unfoldAttribute); if (is_array($rows)) { // TODO: Register the DN the same way as a section name in the ArrayDatasource! foreach ($rows as $row) { - $count += 1; - if (! $serverSorting || $offset === 0 || $offset < $count) { - $entries[] = $row; - } + if ($query->getFilter()->matches($row)) { + $count += 1; + if (! $serverSorting || $offset === 0 || $offset < $count) { + $entries[] = $row; + } - if ($serverSorting && $limit > 0 && $limit === count($entries)) { - break; + if ($serverSorting && $limit > 0 && $limit === count($entries)) { + break; + } } } } else { @@ -813,10 +826,19 @@ class LdapConnection implements Selectable, Inspectable } } + $unfoldAttribute = $query->getUnfoldAttribute(); + if ($unfoldAttribute) { + foreach ($query->getFilter()->listFilteredColumns() as $filterColumn) { + $fieldKey = array_search($filterColumn, $fields, true); + if ($fieldKey === false || is_string($fieldKey)) { + $fields[] = $filterColumn; + } + } + } + $count = 0; $cookie = ''; $entries = array(); - $unfoldAttribute = $query->getUnfoldAttribute(); do { // Do not request the pagination control as a critical extension, as we want the // server to return results even if the paged search request cannot be satisfied @@ -835,7 +857,9 @@ class LdapConnection implements Selectable, Inspectable $ds, $base, $queryString, - array_values($fields), + $unfoldAttribute + ? array_unique(array_values($fields)) + : array_values($fields), 0, // Attributes and values $serverSorting && $limit ? $offset + $limit : 0 ); @@ -873,13 +897,15 @@ class LdapConnection implements Selectable, Inspectable if (is_array($rows)) { // TODO: Register the DN the same way as a section name in the ArrayDatasource! foreach ($rows as $row) { - $count += 1; - if (! $serverSorting || $offset === 0 || $offset < $count) { - $entries[] = $row; - } + if ($query->getFilter()->matches($row)) { + $count += 1; + if (! $serverSorting || $offset === 0 || $offset < $count) { + $entries[] = $row; + } - if ($serverSorting && $limit > 0 && $limit === count($entries)) { - break; + if ($serverSorting && $limit > 0 && $limit === count($entries)) { + break; + } } } } else { @@ -1010,6 +1036,14 @@ class LdapConnection implements Selectable, Inspectable && isset($cleanedAttributes[$unfoldAttribute]) && is_array($cleanedAttributes[$unfoldAttribute]) ) { + $siblings = array(); + foreach ($loweredFieldMap as $loweredName => $requestedNames) { + if (is_array($requestedNames) && in_array($unfoldAttribute, $requestedNames, true)) { + $siblings = array_diff($requestedNames, array($unfoldAttribute)); + break; + } + } + $values = $cleanedAttributes[$unfoldAttribute]; unset($cleanedAttributes[$unfoldAttribute]); $baseRow = (object) $cleanedAttributes; @@ -1017,6 +1051,10 @@ class LdapConnection implements Selectable, Inspectable foreach ($values as $value) { $row = clone $baseRow; $row->{$unfoldAttribute} = $value; + foreach ($siblings as $sibling) { + $row->{$sibling} = $value; + } + $rows[] = $row; }