From 1a74c4d1ee6758bdf52c2118daa5a65ab720d7f3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=B4me=20Chilliet?= <come@opensides.be>
Date: Tue, 16 Jul 2019 11:11:11 +0200
Subject: [PATCH] :ambulance: fix(core) Refactor department cache handling
This allows to avoid ldap reads from pluglist::load, and should fix a
crash happenning in index.php when using encrypted password in config
file.
issue #6015
---
html/autocomplete.php | 5 +-
include/class_baseSelector.inc | 14 ++--
include/class_config.inc | 75 ++++++++++++++-----
include/class_listing.inc | 7 +-
include/class_objects.inc | 5 +-
include/class_pluglist.inc | 7 +-
include/class_userinfo.inc | 21 ++++--
include/functions.inc | 3 +-
.../management/class_managementListing.inc | 3 +-
include/simpleplugin/class_Attribute.inc | 2 +-
include/simpleplugin/class_simplePlugin.inc | 12 +--
.../class_departmentManagement.inc | 3 +-
plugins/admin/departments/tabs_department.inc | 3 +-
plugins/config/class_configInLdap.inc | 2 +-
setup/class_setupStepMigrate.inc | 14 ++--
15 files changed, 112 insertions(+), 64 deletions(-)
diff --git a/html/autocomplete.php b/html/autocomplete.php
index 0ee0e8774..4606ac15e 100644
--- a/html/autocomplete.php
+++ b/html/autocomplete.php
@@ -44,8 +44,9 @@ if (isset($_GET['type']) && $_GET['type'] == "base") {
$pathMapping = session::get("pathMapping");
$search = preg_replace('/"/', '"', current($_POST));
- $config = session::global_get('config');
- foreach ($config->department_info as $dn => $info) {
+ $config = session::global_get('config');
+ $departmentInfo = $config->getDepartmentInfo();
+ foreach ($departmentInfo as $dn => $info) {
if (!isset($pathMapping[$dn])) {
continue;
}
diff --git a/include/class_baseSelector.inc b/include/class_baseSelector.inc
index a70ca5acd..29fcd50a7 100644
--- a/include/class_baseSelector.inc
+++ b/include/class_baseSelector.inc
@@ -214,6 +214,8 @@ class baseSelector
$last_indent = 2;
$last_base = '';
+ $departmentInfo = $config->getDepartmentInfo();
+
foreach ($this->pathMapping as $base => $dummy) {
// Skip root for tree
if ($base == $config->current['BASE']) {
@@ -233,10 +235,10 @@ class baseSelector
$this->tree .= "</li>\n";
$this->tree .= '<li><a title="'.$parent_base.'">'.
'<img class="center" '.
- 'src="'.htmlentities($config->department_info[$parent_base]['img'], ENT_COMPAT, 'UTF-8').'" '.
+ 'src="'.htmlentities($departmentInfo[$parent_base]['img'], ENT_COMPAT, 'UTF-8').'" '.
'alt=""/> '.
- $this->escape($config->department_info[$parent_base]['name']).
- (($config->department_info[$parent_base]['description'] == '') ? '' : ' <span class="informal">['.$this->escape($config->department_info[$parent_base]['description']).']</span>').
+ $this->escape($departmentInfo[$parent_base]['name']).
+ (($departmentInfo[$parent_base]['description'] == '') ? '' : ' <span class="informal">['.$this->escape($departmentInfo[$parent_base]['description']).']</span>').
'</a>';
}
$this->tree .= "<ul>\n";
@@ -250,10 +252,10 @@ class baseSelector
$link = "onclick=\"\$('bs_rebase_".$this->pid."').value='".base64_encode($base)."';$('submit_tree_base_".$this->pid."').click();\"";
$this->tree .= '<li><a'.$selected.' '.$link.' title="'.$base.'">'.
'<img class="center" '.
- 'src="'.htmlentities($config->department_info[$base]['img'], ENT_COMPAT, 'UTF-8').'" '.
+ 'src="'.htmlentities($departmentInfo[$base]['img'], ENT_COMPAT, 'UTF-8').'" '.
'alt=""/> '.
- $this->escape($config->department_info[$base]['name']).
- (($config->department_info[$base]['description'] == '') ? '' : ' <span class="informal">['.$this->escape($config->department_info[$base]['description']).']</span>').
+ $this->escape($departmentInfo[$base]['name']).
+ (($departmentInfo[$base]['description'] == '') ? '' : ' <span class="informal">['.$this->escape($departmentInfo[$base]['description']).']</span>').
'</a>';
$last_indent = $indent;
diff --git a/include/class_config.inc b/include/class_config.inc
index a01e7e825..49759d975 100644
--- a/include/class_config.inc
+++ b/include/class_config.inc
@@ -59,9 +59,10 @@ class config
var $basedir = '';
/* Keep a copy of the current department list */
- var $departments = [];
- var $idepartments = [];
- var $department_info = [];
+ protected $departmentList;
+ protected $departmentTree;
+ protected $departmentInfo;
+
var $filename = '';
var $last_modified = 0;
@@ -628,15 +629,46 @@ class config
}
}
+ function getDepartmentList (): array
+ {
+ if (!isset($this->departmentList)) {
+ $this->storeDepartmentList();
+ }
+ return $this->departmentList;
+ }
+
+ function getDepartmentTree (): array
+ {
+ if (!isset($this->departmentTree)) {
+ $this->storeDepartmentTree();
+ }
+ return $this->departmentTree;
+ }
+
+ function getDepartmentInfo (): array
+ {
+ if (!isset($this->departmentInfo)) {
+ $this->storeDepartmentList();
+ }
+ return $this->departmentInfo;
+ }
+
+ function resetDepartmentCache ()
+ {
+ unset($this->departmentList);
+ unset($this->departmentTree);
+ unset($this->departmentInfo);
+ }
+
/*!
- * \brief Store the departments from ldap in $this->departments
+ * \brief Store the departments from ldap in $this->departmentList
*/
- function get_departments ()
+ protected function storeDepartmentList ()
{
/* Initialize result hash */
- $result = [];
+ $result = ['/' => $this->current['BASE']];
- $result['/'] = $this->current['BASE'];
+ $this->departmentInfo = [];
/* Get all department types from department Management, to be able detect the department type.
-It is possible that different department types have the same name,
@@ -678,7 +710,7 @@ class config
$dn = $attrs['dn'];
$infos = objects::infos($oc);
- $this->department_info[$dn] = [
+ $this->departmentInfo[$dn] = [
'img' => $infos['icon'],
'description' => (isset($attrs['description'][0]) ? $attrs['description'][0] : ''),
'name' => $attrs[$infos['mainAttr']][0]
@@ -691,20 +723,25 @@ class config
}
}
- $this->departments = $result;
+ $this->departmentList = $result;
}
- function make_idepartments ($max_size = 28)
+ /*!
+ * \brief Store the tree render for departments in $this->departmentTree
+ */
+ protected function storeDepartmentTree ()
{
+ if (!isset($this->departmentList)) {
+ $this->storeDepartmentList();
+ }
+
$base = $this->current['BASE'];
$qbase = preg_quote($base, '/');
$arr = [];
- $this->idepartments = [];
-
/* Create multidimensional array, with all departments. */
- foreach ($this->departments as $key => $val) {
+ foreach ($this->departmentList as $val) {
/* Split dn into single department pieces */
$elements = array_reverse(explode(',', preg_replace("/$qbase$/", '', $val)));
@@ -737,9 +774,13 @@ class config
}
/* Add base entry */
- $ret['/']['ENTRY'] = $base;
- $ret['/']['SUB'] = $arr;
- $this->idepartments = $this->generateDepartmentArray($ret, -1, $max_size);
+ $ret = [
+ '/' => [
+ 'ENTRY' => $base,
+ 'SUB' => $arr,
+ ]
+ ];
+ $this->departmentTree = $this->generateDepartmentArray($ret, -1, 28);
}
/*
@@ -751,7 +792,7 @@ class config
*
* \param int $max_size initialized at 256
*/
- function generateDepartmentArray ($arr, $depth = -1, $max_size = 256)
+ protected function generateDepartmentArray ($arr, $depth, $max_size)
{
$ret = [];
$depth++;
diff --git a/include/class_listing.inc b/include/class_listing.inc
index ce37f7daf..281a5505a 100644
--- a/include/class_listing.inc
+++ b/include/class_listing.inc
@@ -1525,9 +1525,10 @@ class listing
$ui = get_userinfo();
// Fill internal bases list
- $this->bases = [];
- $deps = $ui->get_module_departments($this->categories);
- foreach ($config->idepartments as $key => $dep) {
+ $this->bases = [];
+ $deps = $ui->get_module_departments($this->categories);
+ $departmentTree = $config->getDepartmentTree();
+ foreach ($departmentTree as $key => $dep) {
if (in_array_ics($key, $deps)) {
$this->bases[$key] = $dep;
}
diff --git a/include/class_objects.inc b/include/class_objects.inc
index 89cb37d5e..a9615e413 100644
--- a/include/class_objects.inc
+++ b/include/class_objects.inc
@@ -489,8 +489,9 @@ class objects
$infos = static::infos($type);
- $templates = [];
- foreach ($config->departments as $key => $value) {
+ $templates = [];
+ $departments = $config->getDepartmentList();
+ foreach ($departments as $key => $value) {
// Search all templates from the current dn.
try {
$ldap = static::search($type, ['cn'], $infos['ou'].$value, $filter, FALSE, 'subtree', TRUE);
diff --git a/include/class_pluglist.inc b/include/class_pluglist.inc
index 33bc7de10..801c9670c 100644
--- a/include/class_pluglist.inc
+++ b/include/class_pluglist.inc
@@ -518,7 +518,7 @@ class pluglist
/*!
* \brief Loads plist and load it in config object
*/
- static function load ($ldap_available = TRUE)
+ static function load ()
{
global $config, $plist;
if (!session::global_is_set('plist')) {
@@ -528,10 +528,7 @@ class pluglist
$plist = new pluglist();
session::global_set('plist', $plist);
$config->loadPlist($plist);
- if ($ldap_available) {
- $config->get_departments();
- $config->make_idepartments();
- }
+ $config->resetDepartmentCache();
} else {
$plist = session::global_get('plist');
}
diff --git a/include/class_userinfo.inc b/include/class_userinfo.inc
index e8d37e1fa..101473a8d 100644
--- a/include/class_userinfo.inc
+++ b/include/class_userinfo.inc
@@ -513,12 +513,14 @@ class userinfo
$path = explode(',', $dn);
$path = array_reverse($path);
+ $departmentInfo = $config->getDepartmentInfo();
+
/* Walk along the path to evaluate the acl */
$cpath = '';
foreach ($path as $element) {
/* Clean potential ACLs for each level */
- if (isset($config->idepartments[$cpath])) {
+ if (isset($departmentInfo[$cpath])) {
$acl = $this->cleanACL($acl);
}
@@ -626,7 +628,7 @@ class userinfo
/* If the requested ACL is for a container object, then alter
ACLs by applying cleanACL a last time.
*/
- if (isset($config->idepartments[$dn])) {
+ if (isset($departmentInfo[$dn])) {
$acl = $this->cleanACL($acl);
}
@@ -667,7 +669,7 @@ class userinfo
then return all departments as valid.
*/
if ($this->ignore_acl_for_current_user()) {
- return array_keys($config->idepartments);
+ return array_values($config->getDepartmentList());
}
/* Use cached results if possilbe */
@@ -677,6 +679,8 @@ class userinfo
$module = [$module];
}
+ $departmentInfo = $config->getDepartmentInfo();
+
$res = [];
foreach ($module as $mod) {
if (isset($ACL_CACHE['MODULE_DEPARTMENTS'][$mod])) {
@@ -701,19 +705,20 @@ class userinfo
}
}
- if ($found && !isset($config->idepartments[$dn])) {
- while (!isset($config->idepartments[$dn]) && strpos($dn, ",")) {
+ if ($found && !isset($departmentInfo[$dn])) {
+ while (!isset($departmentInfo[$dn]) && strpos($dn, ',')) {
$dn = preg_replace("/^[^,]+,/", "", $dn);
}
- if (isset($config->idepartments[$dn])) {
+ if (isset($departmentInfo[$dn])) {
$deps[$dn] = $dn;
}
}
}
}
- /* For all gosaDepartments */
- foreach ($config->departments as $dn) {
+ /* For all departments */
+ $departments = $config->getDepartmentList();
+ foreach ($departments as $dn) {
if (isset($deps[$dn])) {
continue;
}
diff --git a/include/functions.inc b/include/functions.inc
index 0d8768596..27c58f871 100644
--- a/include/functions.inc
+++ b/include/functions.inc
@@ -839,7 +839,8 @@ function get_base_from_people ($dn)
$base = preg_replace($pattern, '', $dn);
/* Set to base, if we're not on a correct subtree */
- if (!isset($config->idepartments[$base])) {
+ $departmentInfo = $config->getDepartmentInfo();
+ if (!isset($departmentInfo[$base])) {
$base = $config->current['BASE'];
}
diff --git a/include/management/class_managementListing.inc b/include/management/class_managementListing.inc
index 47394cb49..63eddaf67 100644
--- a/include/management/class_managementListing.inc
+++ b/include/management/class_managementListing.inc
@@ -557,7 +557,8 @@ class managementListing
}
$deps = $ui->get_module_departments(array_values($categories));
- foreach ($config->idepartments as $key => $dep) {
+ $departmentTree = $config->getDepartmentTree();
+ foreach ($departmentTree as $key => $dep) {
if (in_array_ics($key, $deps)) {
$this->bases[$key] = $dep;
}
diff --git a/include/simpleplugin/class_Attribute.inc b/include/simpleplugin/class_Attribute.inc
index 4e2bc60ac..65161d69a 100644
--- a/include/simpleplugin/class_Attribute.inc
+++ b/include/simpleplugin/class_Attribute.inc
@@ -562,7 +562,7 @@ class Attribute
break;
}
}
- if (!in_array($dn_base, $config->departments)) {
+ if (!in_array($dn_base, $config->getDepartmentList())) {
continue;
}
} else {
diff --git a/include/simpleplugin/class_simplePlugin.inc b/include/simpleplugin/class_simplePlugin.inc
index 841ab9a2c..34e89abd3 100644
--- a/include/simpleplugin/class_simplePlugin.inc
+++ b/include/simpleplugin/class_simplePlugin.inc
@@ -545,7 +545,8 @@ class simplePlugin implements SimpleTab
$deps = [];
/* Is this a new object ? Or just an edited existing object */
- foreach ($config->idepartments as $dn => $name) {
+ $departmentTree = $config->getDepartmentTree();
+ foreach ($departmentTree as $dn => $name) {
if (
(!$this->initially_was_account && $this->acl_is_createable($dn)) ||
($this->initially_was_account && $this->acl_is_moveable($dn))
@@ -555,8 +556,8 @@ class simplePlugin implements SimpleTab
}
/* Add current base */
- if (isset($this->base) && isset($config->idepartments[$this->base])) {
- $deps[$this->base] = $config->idepartments[$this->base];
+ if (isset($this->base) && isset($departmentTree[$this->base])) {
+ $deps[$this->base] = $departmentTree[$this->base];
} elseif (strtolower($this->dn) != strtolower($config->current['BASE'])) {
trigger_error('Cannot return list of departments, no default base found in class '.get_class($this).'. (base is "'.$this->base.'")');
}
@@ -620,12 +621,11 @@ class simplePlugin implements SimpleTab
$ui->dn = $ui_dn;
}
- /* Check if departments were moved. If so, force the reload of config->departments */
+ /* Check if departments were moved. If so, force the reload of $config departments cache */
$ldap->cd($dst_dn);
$ldap->search('(objectClass=gosaDepartment)', ['dn']);
if ($ldap->count()) {
- $config->get_departments();
- $config->make_idepartments();
+ $config->resetDepartmentCache();
$ui->reset_acl_cache();
}
diff --git a/plugins/admin/departments/class_departmentManagement.inc b/plugins/admin/departments/class_departmentManagement.inc
index f0258116d..ab8a1f571 100644
--- a/plugins/admin/departments/class_departmentManagement.inc
+++ b/plugins/admin/departments/class_departmentManagement.inc
@@ -52,8 +52,7 @@ class departmentManagement extends management
function refreshDeps ()
{
global $config, $ui;
- $config->get_departments();
- $config->make_idepartments();
+ $config->resetDepartmentsCache();
$ui->reset_acl_cache();
$this->listing->refreshBasesList();
}
diff --git a/plugins/admin/departments/tabs_department.inc b/plugins/admin/departments/tabs_department.inc
index 148557902..2f1d728c1 100644
--- a/plugins/admin/departments/tabs_department.inc
+++ b/plugins/admin/departments/tabs_department.inc
@@ -27,8 +27,7 @@ class deptabs extends simpleTabs
/* Update department cache */
global $config;
- $config->get_departments();
- $config->make_idepartments();
+ $config->resetDepartmentCache();
return $errors;
}
diff --git a/plugins/config/class_configInLdap.inc b/plugins/config/class_configInLdap.inc
index 4d964f400..bb403ba6a 100644
--- a/plugins/config/class_configInLdap.inc
+++ b/plugins/config/class_configInLdap.inc
@@ -111,7 +111,7 @@ class configInLdap extends simplePlugin
'ou=snapshots,'.$config->current['BASE']
),
new BooleanAttribute(
- _('Wildcard foreign keys'), _('Enables wildcard searches like member=* when moving a whole departement. This will open all existing groups and roles to make sure foreign keys are respected. Slow on big trees.'),
+ _('Wildcard foreign keys'), _('Enables wildcard searches like member=* when moving a whole department. This will open all existing groups and roles to make sure foreign keys are respected. Slow on big trees.'),
'fdWildcardForeignKeys', FALSE,
TRUE
),
diff --git a/setup/class_setupStepMigrate.inc b/setup/class_setupStepMigrate.inc
index 6c491496e..63c3d2b12 100644
--- a/setup/class_setupStepMigrate.inc
+++ b/setup/class_setupStepMigrate.inc
@@ -252,7 +252,7 @@ class setupStepMigrate extends setupStep
function initialize_checks ()
{
global $config;
- $config->get_departments();
+ $config->resetDepartmentCache();
$checks = [
'baseOC' => new StepMigrateCheck($this, 'baseOC', _('Inspecting object classes in root object')),
@@ -1010,7 +1010,7 @@ class setupStepMigrate extends setupStep
* and verify that he is in a valid department
*/
if (!preg_match('/dc=addressbook,/', $people_db_base) &&
- !in_array($people_db_base, $config->departments)) {
+ !in_array($people_db_base, $config->getDepartmentList())) {
$attrs['checked'] = FALSE;
$attrs['ldif'] = '';
$this->outsideUsers_toMigrate[base64_encode($attrs['dn'])] = $attrs;
@@ -1036,7 +1036,7 @@ class setupStepMigrate extends setupStep
[
'title' => _('Move users into configured user tree'),
'outside' => TRUE,
- 'ous' => $config->departments,
+ 'ous' => $config->getDepartmentList(),
'destination' => (isset($_POST['destination']) ? $_POST['destination'] : ''),
]
);
@@ -1050,7 +1050,7 @@ class setupStepMigrate extends setupStep
[
'title' => _('Move users into configured user tree'),
'outside' => TRUE,
- 'ous' => $config->departments,
+ 'ous' => $config->getDepartmentList(),
'destination' => (isset($_POST['destination']) ? $_POST['destination'] : ''),
]
);
@@ -1128,7 +1128,7 @@ class setupStepMigrate extends setupStep
* and verify that he is in a valid department
*/
if (!preg_match('/'.preg_quote('dc=addressbook,', '/').'/', $group_db_base) &&
- !in_array($group_db_base, $config->departments)
+ !in_array($group_db_base, $config->getDepartmentList())
) {
$attrs['checked'] = FALSE;
$attrs['ldif'] = '';
@@ -1155,7 +1155,7 @@ class setupStepMigrate extends setupStep
[
'title' => _('Move groups into configured groups tree'),
'outside' => TRUE,
- 'ous' => $config->departments,
+ 'ous' => $config->getDepartmentList(),
'destination' => (isset($_POST['destination']) ? $_POST['destination'] : ''),
]
);
@@ -1169,7 +1169,7 @@ class setupStepMigrate extends setupStep
[
'title' => _('Move groups into configured groups tree'),
'outside' => TRUE,
- 'ous' => $config->departments,
+ 'ous' => $config->getDepartmentList(),
'destination' => (isset($_POST['destination']) ? $_POST['destination'] : ''),
]
);
--
GitLab