* AD: fix account-selection type "groupmembers" caused high load on AD

also caching groups now on instance level instead of session
This commit is contained in:
Ralf Becker 2021-05-19 18:26:40 +02:00
parent aad2a77666
commit bd95db7609
2 changed files with 159 additions and 115 deletions

View File

@ -246,7 +246,21 @@ class Accounts
$account_search = &self::$cache['account_search'];
$serial = self::cacheKey($param, $serial_unlimited);
if (isset($account_search[$serial]))
// cache list of all groups on instance level (not session)
if ($serial_unlimited === self::cacheKey(['type'=>'groups','active'=>true]))
{
$result = Cache::getCache($this->config['install_id'], __CLASS__, 'groups', function() use ($param)
{
return $this->backend->search($param);
}, [], self::READ_CACHE_TIMEOUT);
$this->total = count($result);
if (!empty($param['offset']))
{
return array_slice($result, $param['start'], $param['offset']);
}
return $result;
}
elseif (isset($account_search[$serial]))
{
$this->total = $account_search[$serial]['total'];
}
@ -840,7 +854,7 @@ class Accounts
self::setup_cache();
$name_list = &self::$cache['name_list'];
if(@isset($name_list[$which][$name]) && $name_list[$which][$name])
if (isset($name_list[$which][$name]))
{
return $name_list[$which][$name];
}
@ -1353,22 +1367,30 @@ class Accounts
{
//error_log(__METHOD__.'('.array2string($account_ids).')');
// instance-wide cache
if ($account_ids)
{
$instance = self::getInstance();
// instance-wide cache
$invalidate_groups = !$account_ids;
if ($account_ids)
{
foreach((array)$account_ids as $account_id)
{
Cache::unsetCache($instance->config['install_id'], __CLASS__, 'account-'.$account_id);
unset(self::$request_cache[$account_id]);
if ($account_id < 0) $invalidate_groups = true;
}
}
else
{
self::$request_cache = array();
}
// invalidate instance-wide all-groups cache
if ($invalidate_groups)
{
Cache::unsetCache($instance->config['install_id'], __CLASS__, 'groups');
}
// session-cache
if (self::$cache) self::$cache = array();
@ -1397,10 +1419,11 @@ class Accounts
*
* @param int $account_id
* @param boolean $need_active =false true = 'members-active' required
* @return array
* @param boolean $return_not_cached =false true: return null if nothing is cached
* @return array or null if nothing is cached
* @throws Exception\WrongParameter if no integer was passed as $account_id
*/
static function cache_read($account_id, $need_active=false)
static function cache_read($account_id, bool $need_active=false, bool $return_not_cached=false)
{
if (!is_numeric($account_id)) throw new Exception\WrongParameter('Not an integer!');
@ -1414,6 +1437,10 @@ class Accounts
if (!isset($account)) // not in instance cache --> read from backend
{
if ($return_not_cached)
{
return null;
}
if (($account = $instance->backend->read($account_id)))
{
if ($instance->get_type($account_id) == 'u')
@ -1424,7 +1451,7 @@ class Accounts
{
if (!isset($account['members'])) $account['members'] = $instance->backend->members($account_id);
}
Cache::setCache($instance->config['install_id'], __CLASS__, 'account-'.$account_id, $account, self::READ_CACHE_TIMEOUT);
$instance->cache_data($account_id, $account);
}
//error_log(__METHOD__."($account_id) read from backend ".array2string($account));
}
@ -1445,6 +1472,20 @@ class Accounts
return $account;
}
/**
* Cache account read by backend (incl. members or memberships!)
*
* Can be used by backends too, to inject accounts into the cache.
*
* @param int $account_id
* @param array $account
* @throws Exception\WrongParameter
*/
public function cache_data(int $account_id, array $account)
{
Cache::setCache($this->config['install_id'], __CLASS__, 'account-'.$account_id, $account, self::READ_CACHE_TIMEOUT);
}
/**
* Internal functions not meant to use outside this class!!!
*/

View File

@ -881,8 +881,6 @@ class Ads
/**
* Searches / lists accounts: users and/or groups
*
* @todo sort and limit query on AD, PHP5.4 and AD support it
*
* @param array with the following keys:
* @param $param['type'] string/int 'accounts', 'groups', 'owngroups' (groups the user is a member of), 'both'
* or integer group-id for a list of members of that group
@ -904,29 +902,10 @@ class Ads
function search($param)
{
//error_log(__METHOD__.'('.json_encode($param).') '.function_backtrace());
$account_search = []; // disabled, we have sorted&limited queries now &$this->cache['account_search'];
// check if the query is cached
$serial = Api\Accounts::cacheKey($param, $unl_serial);
if (isset($account_search[$serial]))
{
$this->total = $account_search[$serial]['total'];
return $account_search[$serial]['data'];
}
// if it's a limited query, check if the unlimited query is cached
$start = $param['start'];
if (!($maxmatchs = $GLOBALS['egw_info']['user']['preferences']['common']['maxmatchs'])) $maxmatchs = 15;
if (!($offset = $param['offset'])) $offset = $maxmatchs;
unset($param['start']);
unset($param['offset']);
if (isset($account_search[$unl_serial]))
{
$this->total = $account_search[$unl_serial]['total'];
$sortedAccounts = $account_search[$unl_serial]['data'];
}
else // we need to run the unlimited query
{
$this->total = null;
$query = Api\Ldap::quote(strtolower($param['query']));
@ -1008,18 +987,14 @@ class Ads
}
$sortedAccounts = $accounts;
uasort($sortedAccounts,array($this,'_sort_callback'));
$account_search[$unl_serial]['data'] = $sortedAccounts;
$account_search[$unl_serial]['total'] = $this->total = $this->total ?? count($accounts);
}
$this->total = $this->total ?? count($accounts);
// return only the wanted accounts
reset($sortedAccounts);
if(is_numeric($start) && is_numeric($offset))
{
$account_search[$serial]['data'] = array_slice($sortedAccounts, $start, $offset);
$account_search[$serial]['total'] = $this->total;
//error_log(__METHOD__.'('.array2string($param).") returning $offset/$this->total entries from $start ".array2string($account_search[$serial]['data']));
return $account_search[$serial]['data'];
return array_slice($sortedAccounts, $start, $offset);
}
//error_log(__METHOD__.'('.array2string($param).') returning all '.array2string($sortedAccounts));
return $sortedAccounts;
@ -1093,15 +1068,39 @@ class Ads
isset($GLOBALS['egw_info']['user']['account_id']) && // only use groupmembers, if we have a user context (stalls login otherwise!)
(!isset($GLOBALS['egw_info']['user']['apps']['admin'])))
{
$type_filter .= '(|';
foreach($GLOBALS['egw']->accounts->memberships($GLOBALS['egw_info']['user']['account_id'],true) as $group_id)
// generate groupmembers filter once per session without any external calls (but groups) to NOT generate a deep recursion!
$type_filter .= Api\Cache::getSession(__CLASS__, 'groupmembers_filter', function()
{
if (!in_array($group_id, $this->ignore_membership) && ($dn = Api\Accounts::id2name($group_id, 'account_dn')))
$account_id = $GLOBALS['egw_info']['user']['account_id'];
$account = $this->filter(['objectSid' => $this->get_sid($account_id)],
false, ['memberOf','primaryGroupID','objectSid','samAccountType'])[$account_id];
$filter = '(|';
$primary_ignored = in_array(-$account['primarygroupid'][0], $this->ignore_membership);
foreach($account['memberof'] as $key => $dn)
{
$type_filter .= '(memberOf='.$dn.')(primaryGroupID='.abs($group_id).')';
if ($key === 'count') continue;
// if the primary group of the user is ignored (=Domain Users), we assume it's the case for everyone
if ($primary_ignored)
{
$filter .= '(memberOf='.$dn.')';
continue;
}
if (!isset($groups))
{
$groups = $this->frontend->search(['type' => 'groups']);
}
foreach($groups as $gid => $group)
{
if (($gid == -$account['primarygroupid'][0] || $dn === $group['account_dn']) &&
!in_array($gid, $this->ignore_membership))
{
$filter .= '(memberOf='.$group['account_dn'].')(primaryGroupID='.(-$gid).')';
break;
}
}
$type_filter .= ')';
}
return $filter.')';
});
}
$type_filter .= ')';
if ($account_type === 'u') break;
@ -1167,7 +1166,7 @@ class Ads
* All reading ADS queries are done throught this methods.
*
* @param string|array $attr_filter array with attribute => value pairs or filter string or empty
* @param string $account_type u = user, g = group, default null = try both
* @param string|false $account_type u = user, g = group, default null = try both, false: no type_filter!
* @param array $attrs =null default return account_lid, else return raw values from ldap-query
* @param array $accounts =array() array to add filtered accounts too, default empty array
* @param bool $filter_expired =false true: filter out expired users
@ -1222,7 +1221,11 @@ class Ads
$filter .= '('.$attr.'='.$this->adldap->utilities()->ldapSlashes($value).')';
}
}
$filter .= $this->type_filter($account_type).')';
if ($account_type !== false)
{
$filter .= $this->type_filter($account_type);
}
$filter .= ')';
}
if (!($sri = ldap_search($ds=$this->ldap_connection(), $context=$this->ads_context(), $filter,
$attrs ? $attrs : self::$default_attributes, null, null, null, null, $control)))
@ -1305,9 +1308,9 @@ class Ads
*
* Calls frontend which uses (cached) read method to fetch all data by account_id.
*
* @param int $account_id numerica account_id
* @param int $account_id numeric account_id
* @param string $which ='account_lid' type to convert to: account_lid (default), account_email, ...
* @return string/false converted value or false on error ($account_id not found)
* @return string|false converted value or false on error ($account_id not found)
*/
public function id2name($account_id, $which='account_lid')
{