From 99de4172fe9f0d0cb946ec08e5de4f10c33bbc1c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=B4me=20Chilliet?= <come.chilliet@fusiondirectory.org>
Date: Thu, 10 Oct 2019 11:15:29 +0200
Subject: [PATCH] :ambulance: fix(templates) Fix userPassword handling in
 templates

When hash is set but not the password value (which is the default if you
 leave the template fields empty), applying the template to a user would
 result in changing its password, even if he already uses the same hash
 method.
With this change it will leave its password alone if it already uses the
 method, and if the method changed it should complain and ask for a new
 password.
This should avoid users erasing passwords by mistake.

issue #6035
---
 include/class_template.inc                    | 20 ++++++-------------
 include/simpleplugin/class_simplePlugin.inc   |  7 +++++--
 .../generic/class_UserPasswordAttribute.inc   |  5 +++++
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/include/class_template.inc b/include/class_template.inc
index 17e9616e2..7e82bbce5 100644
--- a/include/class_template.inc
+++ b/include/class_template.inc
@@ -68,22 +68,18 @@ class template
     return $result;
   }
 
-  function __construct ($type, $dn, $targetdn = NULL)
+  function __construct ($type, $dn)
   {
     $this->type = $type;
     $this->dn   = $dn;
     list($this->attrs, $depends) = templateHandling::fetch($this->dn);
     $this->needed     = templateHandling::neededAttrs($this->attrs, $depends);
     $this->needed[]   = 'base';
-    if ($targetdn === NULL) {
-      $this->tabObject  = objects::create($this->type);
-    } else {
-      trigger_error("This should not be used for now");
-      $this->tabObject  = objects::open($this->dn, $this->type);
-    }
-    $tempTabObject    = objects::open($this->dn, $this->type); /* Used to know which tab is activated */
-    $this->attributes = [];
+    $this->tabObject  = objects::create($this->type);
+    /* Used to know which tab is activated */
+    $tempTabObject    = objects::open($this->dn, $this->type);
     $tempTabObject->setActiveTabs($this->tabObject);
+    $this->attributes = [];
     foreach ($this->tabObject->by_object as $class => $tab) {
       if ($tab->is_account || $tab->ignore_account) {
         $this->attributes[$class] = [];
@@ -114,11 +110,7 @@ class template
     $this->tabObject  = objects::create($this->type);
     /* Used to know which tab is activated */
     $tempTabObject    = objects::open($this->dn, $this->type);
-    foreach ($tempTabObject->by_object as $class => $plugin) {
-      if ($plugin->is_account || $plugin->ignore_account) {
-        $this->tabObject->by_object[$class]->is_account = $plugin->is_account;
-      }
-    }
+    $tempTabObject->setActiveTabs($this->tabObject);
     $this->applied = FALSE;
   }
 
diff --git a/include/simpleplugin/class_simplePlugin.inc b/include/simpleplugin/class_simplePlugin.inc
index 85f35b0a7..3adb17807 100644
--- a/include/simpleplugin/class_simplePlugin.inc
+++ b/include/simpleplugin/class_simplePlugin.inc
@@ -1913,7 +1913,7 @@ class simplePlugin implements SimpleTab
     throw new FusionDirectoryException(_('Failed to create a unique DN'));
   }
 
-  /*
+  /*!
    * \brief Adapt from template
    *
    * Adapts fields to the values from a template.
@@ -2010,7 +2010,10 @@ class simplePlugin implements SimpleTab
     return TRUE;
   }
 
-  /* Returns TRUE if this attribute should be asked in the creation by template dialog */
+  /*! \brief Returns TRUE if this attribute should be asked in the creation by template dialog
+   *
+   * \return bool whether this attribute should be asked
+   */
   function showInTemplate (string $attr, array $templateAttrs): bool
   {
     if (isset($templateAttrs[$attr])) {
diff --git a/plugins/personal/generic/class_UserPasswordAttribute.inc b/plugins/personal/generic/class_UserPasswordAttribute.inc
index 7b73efaa9..6a8cdf87f 100644
--- a/plugins/personal/generic/class_UserPasswordAttribute.inc
+++ b/plugins/personal/generic/class_UserPasswordAttribute.inc
@@ -199,6 +199,11 @@ class UserPasswordAttribute extends CompositeAttribute
         $pw_storage = $tmp->get_hash();
         $locked     = $tmp->is_locked('', $value);
       }
+      if ($istemplate && empty($password)) {
+        /* Do not store hash for templates,
+         * we have the password anyway, and this avoids problems with empty passwords */
+        $value = $this->attributes[3]->getValue();
+      }
     }
     return [$pw_storage, $password, $password, $value, ($locked ? 'TRUE' : 'FALSE')];
   }
-- 
GitLab