From f64f2936dfb8e65125f83d288d723f32c56593fa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=B4me=20Chilliet?= <come@opensides.be>
Date: Wed, 17 Jul 2019 15:52:34 +0200
Subject: [PATCH] :tractor: feat(core) Big refactor to get rid of acl_base

ACL base should be computed from current object DN or base.

issue #5039
---
 include/class_CopyPasteHandler.inc            |  7 ++-
 include/class_objects.inc                     |  1 -
 include/management/class_management.inc       | 14 +++---
 .../snapshot/class_SnapshotCreateDialog.inc   | 12 ++++-
 .../snapshot/class_SnapshotRestoreDialog.inc  | 10 ++--
 .../class_BaseSelectorAttribute.inc           |  6 ---
 .../simpleplugin/class_dialogAttributes.inc   |  2 +-
 include/simpleplugin/class_multiPlugin.inc    | 10 ----
 .../simpleplugin/class_simpleManagement.inc   | 16 +++----
 include/simpleplugin/class_simplePlugin.inc   | 47 ++++++++++---------
 include/simpleplugin/class_simpleTabs.inc     | 18 ++-----
 include/simpleplugin/interface_SimpleTab.inc  |  6 +--
 plugins/admin/acl/class_aclAssignment.inc     |  2 +-
 plugins/admin/acl/class_aclManagement.inc     |  2 +-
 14 files changed, 66 insertions(+), 87 deletions(-)

diff --git a/include/class_CopyPasteHandler.inc b/include/class_CopyPasteHandler.inc
index a7c529a20..40b074e68 100644
--- a/include/class_CopyPasteHandler.inc
+++ b/include/class_CopyPasteHandler.inc
@@ -134,7 +134,7 @@ class CopyPasteHandler
   /*!
    * \brief Paste one entry from LDAP
    */
-  protected function load_entry_from_ldap ($entry, $base)
+  protected function load_entry_from_ldap ($entry)
   {
     @DEBUG(DEBUG_TRACE, __LINE__, __FUNCTION__, __FILE__, $entry['dn'], 'load_entry_from_ldap');
     if (!isset($entry['tab_class']) && !isset($entry['type'])) {
@@ -143,7 +143,6 @@ class CopyPasteHandler
 
     $entry['object'] = objects::open($entry['dn'], $entry['type']);
 
-    $entry['object']->set_acl_base($base);
     if ($entry['parent'] !== NULL) {
       $entry['object']->parent = $entry['parent'];
     }
@@ -161,7 +160,7 @@ class CopyPasteHandler
    * \brief Displays a dialog which allows the user to fix all dependencies of this object.
    *      Create unique names, ids, or what ever
    */
-  function execute ($base)
+  function execute ()
   {
     $ui = get_userinfo();
 
@@ -178,7 +177,7 @@ class CopyPasteHandler
 
         /* Update entries on demand */
         if (!isset($entry['object'])) {
-          $entry = $this->load_entry_from_ldap($entry, $base);
+          $entry = $this->load_entry_from_ldap($entry);
           $this->queue[$key] = $entry;
         }
 
diff --git a/include/class_objects.inc b/include/class_objects.inc
index a9615e413..aaf4945b0 100644
--- a/include/class_objects.inc
+++ b/include/class_objects.inc
@@ -334,7 +334,6 @@ class objects
     $tabClass = $infos['tabClass'];
 
     $tabObject = new $tabClass($type, $dn);
-    $tabObject->set_acl_base();
     @DEBUG(DEBUG_TRACE, __LINE__, __FUNCTION__, __FILE__, $dn, "Openned as $type object");
 
     return $tabObject;
diff --git a/include/management/class_management.inc b/include/management/class_management.inc
index d6e896599..462043749 100644
--- a/include/management/class_management.inc
+++ b/include/management/class_management.inc
@@ -581,10 +581,9 @@ class management
     return print_header($this->icon, $this->title, get_object_info());
   }
 
-  function openTabObject ($object, $base)
+  function openTabObject ($object)
   {
     $this->tabObject = $object;
-    $this->tabObject->set_acl_base($base);
     $this->tabObject->parent = &$this;
   }
 
@@ -708,7 +707,7 @@ class management
     set_object_info($this->currentDn);
 
     // Open object
-    $this->openTabObject(objects::create($type), $this->listing->getBase());
+    $this->openTabObject(objects::create($type));
     @DEBUG(DEBUG_TRACE, __LINE__, __FUNCTION__, __FILE__, $this->currentDn, 'Create entry initiated');
   }
 
@@ -828,7 +827,7 @@ class management
     add_lock($this->currentDn, $ui->dn);
 
     // Open object
-    $this->openTabObject(objects::open($this->currentDn, $entry->getTemplatedType()), $this->currentDn);
+    $this->openTabObject(objects::open($this->currentDn, $entry->getTemplatedType()));
     @DEBUG(DEBUG_TRACE, __LINE__, __FUNCTION__, __FILE__, $this->currentDn, 'Edit entry initiated');
     if (isset($action['subaction'])
       && ($this->handleSubAction($action) === FALSE)) {
@@ -978,7 +977,7 @@ class management
       if ($entry->checkAcl('d')) {
         // Delete the object
         $this->currentDn = $dn;
-        $this->openTabObject(objects::open($this->currentDn, $entry->getTemplatedType()), $this->currentDn);
+        $this->openTabObject(objects::open($this->currentDn, $entry->getTemplatedType()));
         $this->tabObject->delete();
 
         // Remove the lock for the current object.
@@ -1039,7 +1038,7 @@ class management
 
     // Display any c&p dialogs, eg. object modifications required before pasting.
     if ($this->cpPastingStarted && $this->cpHandler->entries_queued()) {
-      $data = $this->cpHandler->execute($this->listing->getBase());
+      $data = $this->cpHandler->execute();
       if (!empty($data)) {
         return $data;
       }
@@ -1069,7 +1068,6 @@ class management
     $entry = $this->listing->getEntry($this->currentDn);
     if ($entry->snapshotCreationAllowed()) {
       $this->dialogObject = new SnapshotCreateDialog($this->currentDn, $this, '');
-      $this->dialogObject->set_acl_base($this->currentDn);
     } else {
       msg_dialog::display(_('Permission'), sprintf(_('You are not allowed to create a snapshot for %s.'), $this->currentDn),
           ERROR_DIALOG);
@@ -1215,7 +1213,7 @@ class management
         add_lock($this->currentDn, $ui->dn);
 
         // Open object
-        $this->openTabObject(objects::open($this->currentDn, $entry->getTemplatedType()), $this->currentDn);
+        $this->openTabObject(objects::open($this->currentDn, $entry->getTemplatedType()));
         $this->saveChanges();
       }
     } else {
diff --git a/include/management/snapshot/class_SnapshotCreateDialog.inc b/include/management/snapshot/class_SnapshotCreateDialog.inc
index f44f36fde..d8bdb79bc 100644
--- a/include/management/snapshot/class_SnapshotCreateDialog.inc
+++ b/include/management/snapshot/class_SnapshotCreateDialog.inc
@@ -67,6 +67,14 @@ class SnapshotCreateDialog extends ManagementDialog
     $this->aclCategory  = $aclCategory;
   }
 
+  /*!
+   * \brief Get LDAP base to use for ACL checks
+   */
+  function getAclBase(): string
+  {
+    return $this->object_dn;
+  }
+
   /*! \brief Check if logged in user have enough right to write this attribute value
    *
    * \param mixed $attr Attribute object or name (in this case it will be fetched from attributesAccess)
@@ -79,7 +87,7 @@ class SnapshotCreateDialog extends ManagementDialog
       $attr = $this->attributesAccess[$attr];
     }
     if ($attr->getLdapName() == 'description') {
-      return in_array('c', $ui->get_snapshot_permissions($this->acl_base, $this->aclCategory));
+      return in_array('c', $ui->get_snapshot_permissions($this->object_dn, $this->aclCategory));
     } else {
       return parent::attrIsWriteable($attr);
     }
@@ -90,7 +98,7 @@ class SnapshotCreateDialog extends ManagementDialog
     global $ui;
     $smarty = get_smarty();
 
-    $permissions = $ui->get_snapshot_permissions($this->acl_base, $this->aclCategory);
+    $permissions = $ui->get_snapshot_permissions($this->object_dn, $this->aclCategory);
     $acl = '';
     if (in_array('c', $permissions)) {
       $acl .= 'crw';
diff --git a/include/management/snapshot/class_SnapshotRestoreDialog.inc b/include/management/snapshot/class_SnapshotRestoreDialog.inc
index 0f45b3a65..0747ae4f7 100644
--- a/include/management/snapshot/class_SnapshotRestoreDialog.inc
+++ b/include/management/snapshot/class_SnapshotRestoreDialog.inc
@@ -65,7 +65,6 @@ class SnapshotRestoreDialog extends ManagementDialog
     $this->global       = $global;
     parent::__construct(NULL, NULL, $parent);
     $this->object_dn    = $dn;
-    $this->acl_base     = $dn;
     $this->aclCategory  = $aclCategories;
     if ($this->global) {
       $this->attributesAccess['object_dn']->setVisible(FALSE);
@@ -73,6 +72,11 @@ class SnapshotRestoreDialog extends ManagementDialog
     $this->updateList();
   }
 
+  function getAclBase (): string
+  {
+    return $this->object_dn;
+  }
+
   function updateList ()
   {
     if ($this->global) {
@@ -115,7 +119,7 @@ class SnapshotRestoreDialog extends ManagementDialog
       $str = $smarty->fetch(get_template_path('restore-confirm.tpl'));
     } else {
       $smarty = get_smarty();
-      $permissions = $ui->get_snapshot_permissions($this->acl_base, $this->aclCategory);
+      $permissions = $ui->get_snapshot_permissions($this->getAclBase(), $this->aclCategory);
       $acl = '';
       if (in_array('r', $permissions)) {
         $acl .= 'r';
@@ -144,7 +148,7 @@ class SnapshotRestoreDialog extends ManagementDialog
       $attr = $this->attributesAccess[$attr];
     }
     if ($attr->getLdapName() == 'snapshots') {
-      return in_array(($this->global ? 'restore_deleted' : 'restore_over'), $ui->get_snapshot_permissions($this->acl_base, $this->aclCategory));
+      return in_array(($this->global ? 'restore_deleted' : 'restore_over'), $ui->get_snapshot_permissions($this->getAclBase(), $this->aclCategory));
     } else {
       return parent::attrIsWriteable($attr);
     }
diff --git a/include/simpleplugin/attributes/class_BaseSelectorAttribute.inc b/include/simpleplugin/attributes/class_BaseSelectorAttribute.inc
index c883ead9c..d2bbb6b73 100644
--- a/include/simpleplugin/attributes/class_BaseSelectorAttribute.inc
+++ b/include/simpleplugin/attributes/class_BaseSelectorAttribute.inc
@@ -130,12 +130,6 @@ class BaseSelectorAttribute extends Attribute
   {
     parent::setValue($value);
     if (is_object($this->plugin)) {
-      /* Set the new acl base */
-      if ($this->plugin->dn == 'new') {
-        $this->plugin->set_acl_base($this->value);
-        $this->plugin->parent->set_acl_base();
-      }
-
       if (($this->baseSelector !== NULL) && ($this->baseSelector->getBase() !== $this->value)) {
         $this->baseSelector->setBase($this->value);
       }
diff --git a/include/simpleplugin/class_dialogAttributes.inc b/include/simpleplugin/class_dialogAttributes.inc
index 759464f3b..45877e45d 100644
--- a/include/simpleplugin/class_dialogAttributes.inc
+++ b/include/simpleplugin/class_dialogAttributes.inc
@@ -836,7 +836,7 @@ class GenericSimplePluginDialog extends GenericDialog
     if ($base == 'new') {
       $base = $simplePlugin->base;
     }
-    $this->dialog->set_acl_base($base);
+    $this->dialog->base = $base;
     $this->dialog->set_acl_category($simplePlugin->acl_category);
     if (!empty($value)) {
       $this->initialDialogValue = $value;
diff --git a/include/simpleplugin/class_multiPlugin.inc b/include/simpleplugin/class_multiPlugin.inc
index 4079789a4..1bac4c59d 100644
--- a/include/simpleplugin/class_multiPlugin.inc
+++ b/include/simpleplugin/class_multiPlugin.inc
@@ -56,7 +56,6 @@ class multiPlugin extends simplePlugin
           these settings will be overloaded in main.inc,
           if we are editing ourself */
       $this->plugin[$name]->set_acl_category($plInfos['plCategory'][0]);
-      $this->plugin[$name]->set_acl_base($this->dn);
     }
   }
 
@@ -134,15 +133,6 @@ class multiPlugin extends simplePlugin
     unset($plug);
   }
 
-  function set_acl_base (string $base)
-  {
-    parent::set_acl_base($base);
-    foreach ($this->plugin as &$plug) {
-      $plug->set_acl_base($base);
-    }
-    unset($plug);
-  }
-
   public function setNeedEditMode (bool $bool)
   {
     parent::setNeedEditMode($bool);
diff --git a/include/simpleplugin/class_simpleManagement.inc b/include/simpleplugin/class_simpleManagement.inc
index 938a349ad..81f1205eb 100644
--- a/include/simpleplugin/class_simpleManagement.inc
+++ b/include/simpleplugin/class_simpleManagement.inc
@@ -75,7 +75,7 @@ class templateDialog
     }
     if (is_object($this->template)) {
       if ($this->target !== NULL) {
-        $this->simpleManagement->openTabObject($this->template->apply($this->target), $this->template->getBase());
+        $this->simpleManagement->openTabObject($this->template->apply($this->target));
         $this->simpleManagement->handleTemplateApply();
         return FALSE;
       } else {
@@ -112,7 +112,7 @@ class templateDialog
   function handle_finish ()
   {
     $this->simpleManagement->closeDialogs();
-    $this->simpleManagement->openTabObject($this->template->apply(), $this->template->getBase());
+    $this->simpleManagement->openTabObject($this->template->apply());
     return FALSE;
   }
 
@@ -676,7 +676,7 @@ class simpleManagement
     set_object_info($this->dn);
 
     // Open object
-    $this->openTabObject(objects::open($this->dn, $type), $this->headpage->getBase());
+    $this->openTabObject(objects::open($this->dn, $type));
     @DEBUG(DEBUG_TRACE, __LINE__, __FUNCTION__, __FILE__, $this->dn, "Create new entry initiated!");
   }
 
@@ -877,7 +877,7 @@ class simpleManagement
       add_lock($this->dn, $ui->dn);
 
       // Open object
-      $this->openTabObject(objects::open($this->dn, $type), $this->dn);
+      $this->openTabObject(objects::open($this->dn, $type));
       @DEBUG(DEBUG_TRACE, __LINE__, __FUNCTION__, __FILE__, $this->dn, "Edit entry initiated!");
       if (isset($all['subaction']) && !$this->handleSubAction($all)) {
         trigger_error('Was not able to handle subaction: '.$all['subaction']);
@@ -944,10 +944,9 @@ class simpleManagement
     return FALSE;
   }
 
-  function openTabObject ($object, $base)
+  function openTabObject ($object)
   {
     $this->tabObject = $object;
-    $this->tabObject->set_acl_base($base);
     $this->tabObject->parent = &$this;
   }
 
@@ -1081,7 +1080,7 @@ class simpleManagement
       if (preg_match("/d/", $acl)) {
         // Delete the object
         $this->dn = $dn;
-        $this->openTabObject(objects::open($this->dn, $type), $this->dn);
+        $this->openTabObject(objects::open($this->dn, $type));
         $this->tabObject->delete();
 
         // Remove the lock for the current object.
@@ -1117,7 +1116,6 @@ class simpleManagement
       $aclCategory = $config->data['OBJECTS'][$this->getType($this->dn)]['aclCategory'];
       if ($ui->allow_snapshot_create($this->dn, $aclCategory)) {
         $this->dialogObject = new SnapshotCreateDialog($this->dn, $this, $aclCategory);
-        $this->dialogObject->set_acl_base($this->dn);
       } else {
         msg_dialog::display(_('Permission'), sprintf(_('You are not allowed to create a snapshot for %s.'), $this->dn),
             ERROR_DIALOG);
@@ -1216,7 +1214,7 @@ class simpleManagement
 
     // Display any c&p dialogs, eg. object modifications required before pasting.
     if ($this->cpPastingStarted && $this->cpHandler->entries_queued()) {
-      $data = $this->cpHandler->execute($this->headpage->getBase());
+      $data = $this->cpHandler->execute();
       if (!empty($data)) {
         return $data;
       }
diff --git a/include/simpleplugin/class_simplePlugin.inc b/include/simpleplugin/class_simplePlugin.inc
index 34e89abd3..b0732ce32 100644
--- a/include/simpleplugin/class_simplePlugin.inc
+++ b/include/simpleplugin/class_simplePlugin.inc
@@ -53,7 +53,6 @@ class simplePlugin implements SimpleTab
   public $initially_was_account = FALSE;
   public $ignore_account        = FALSE;
 
-  public $acl_base      = '';
   public $acl_category  = '';
 
   /*! \brief dn of the opened object */
@@ -220,9 +219,6 @@ class simplePlugin implements SimpleTab
           $this->read_only = TRUE;
         }
       }
-
-      /* Save current dn as acl_base */
-      $this->acl_base = $this->dn;
     }
 
     /* Load LDAP data */
@@ -564,16 +560,6 @@ class simplePlugin implements SimpleTab
     return $deps;
   }
 
-  /*!
-   * \brief Set acl base
-   *
-   * \param string $base
-   */
-  function set_acl_base (string $base)
-  {
-    $this->acl_base = $base;
-  }
-
   /*!
    * \brief Set acl category
    *
@@ -817,13 +803,37 @@ class simplePlugin implements SimpleTab
     return $this->acl_is_writeable($attr->getAcl(), $this->acl_skip_write());
   }
 
+  /*!
+   * \brief Get LDAP base to use for ACL checks
+   */
+  function getAclBase(): string
+  {
+    global $config;
+
+    if ($this->mainTab) {
+      if (isset($this->dn) && ($this->dn != 'new')) {
+        return $this->dn;
+      }
+      if (isset($this->base)) {
+        return 'new,'.$this->base;
+      }
+    } elseif ($this->parent instanceof simpleTabs) {
+      return $this->parent->getAclBase();
+    }
+    if (isset($this->base)) {
+      return 'new,'.$this->base;
+    }
+
+    return $config->current['BASE'];
+  }
+
   function renderAttributes (bool $readOnly = FALSE)
   {
     global $ui;
     $smarty = get_smarty();
 
     if ($this->is_template) {
-      $smarty->assign('template_cnACL', $ui->get_permissions($this->acl_base, $this->acl_category.'template', 'template_cn', $this->acl_skip_write()));
+      $smarty->assign('template_cnACL', $ui->get_permissions($this->getAclBase(), $this->acl_category.'template', 'template_cn', $this->acl_skip_write()));
     }
 
     /* Handle rights to modify the base */
@@ -969,7 +979,7 @@ class simplePlugin implements SimpleTab
     $ui         = get_userinfo();
     $skipWrite  |= $this->readOnly();
     if ($base === NULL) {
-      $base = $this->acl_base;
+      $base = $this->getAclBase();
     }
     return $ui->get_permissions($base, $this->acl_category.get_class($this), $attribute, $skipWrite);
   }
@@ -2118,11 +2128,6 @@ class simplePlugin implements SimpleTab
         if (!$tabs) {
           $tabObject->current = $classname;
         }
-        if (($entry_dn != '') && ($entry_dn != 'new')) {
-          $tabObject->set_acl_base($entry_dn);
-        } else {
-          $tabObject->set_acl_base($config->current['BASE']);
-        }
         session::set($classname, $tabObject);
       }
       $tabObject = session::get($classname);
diff --git a/include/simpleplugin/class_simpleTabs.inc b/include/simpleplugin/class_simpleTabs.inc
index 2aa51956a..61355d49b 100644
--- a/include/simpleplugin/class_simpleTabs.inc
+++ b/include/simpleplugin/class_simpleTabs.inc
@@ -483,23 +483,11 @@ class simpleTabs
   }
 
   /*!
-   * \brief Set acl base
-   *
-   * \param string $base The new acl base
+   * \brief Get LDAP base to use for ACL checks
    */
-  function set_acl_base ($base = "")
+  function getAclBase(): string
   {
-    /* Update reference, transfer variables */
-    $first = ($base == "");
-    foreach ($this->by_object as &$obj) {
-      if ($first) {
-        $first  = FALSE;
-        $base   = $obj->acl_base;
-      } else {
-        $obj->set_acl_base($base);
-      }
-    }
-    unset($obj);
+    return $this->getBaseObject()->getAclBase();
   }
 
   function setTemplateMode ($cn)
diff --git a/include/simpleplugin/interface_SimpleTab.inc b/include/simpleplugin/interface_SimpleTab.inc
index 7a6070f7c..85b307214 100644
--- a/include/simpleplugin/interface_SimpleTab.inc
+++ b/include/simpleplugin/interface_SimpleTab.inc
@@ -38,13 +38,13 @@ interface SimpleTab
    * string $_template_cn (only for main tab of templates)
    * array $attributesAccess (only for main tab of templates)
    * simpleTabs $parent
-   * string $acl_base
    */
 
   /*
    * Public methods needed in some cases:
    * compute_dn (): string (only for main tab)
    * move (string $src_dn, string $dst_dn): TRUE|string (only for main tab)
+   * getAclBase (): string (only for main tab)
    */
 
   public static function plInfo (): array;
@@ -73,10 +73,6 @@ interface SimpleTab
    */
   public function resetCopyInfos ();
 
-  /*! \brief    Forward plugin acls
-   */
-  public function set_acl_base (string $base);
-
   /*! \brief Sets ACL category provided by simpleTabs
    */
   public function set_acl_category (string $category);
diff --git a/plugins/admin/acl/class_aclAssignment.inc b/plugins/admin/acl/class_aclAssignment.inc
index a9590029d..e33fe17db 100644
--- a/plugins/admin/acl/class_aclAssignment.inc
+++ b/plugins/admin/acl/class_aclAssignment.inc
@@ -143,7 +143,7 @@ class ACLsAssignmentDialog extends GenericDialog
     }
     $this->attribute        = $attribute;
     $this->dialog           = new $this->dialogClass($acl, $isContainer);
-    $this->dialog->set_acl_base($simplePlugin->acl_base);
+    $this->dialog->base     = $simplePlugin->getAclBase();
     $this->initialAclValue  = $acl;
   }
 
diff --git a/plugins/admin/acl/class_aclManagement.inc b/plugins/admin/acl/class_aclManagement.inc
index bf07bc378..889270f98 100644
--- a/plugins/admin/acl/class_aclManagement.inc
+++ b/plugins/admin/acl/class_aclManagement.inc
@@ -189,7 +189,7 @@ class aclManagement extends management
 
     set_object_info($this->currentDn);
 
-    $this->openTabObject($tabObject, $this->listing->getBase());
+    $this->openTabObject($tabObject);
     @DEBUG(DEBUG_TRACE, __LINE__, __FUNCTION__, __FILE__, $this->currentDn, 'Creating new ACLĂ‚ assignment');
   }
 
-- 
GitLab