From 56dd787ce229afa949530b95fcabc79f92904c2b Mon Sep 17 00:00:00 2001
From: David Goodwin <david@codepoets.co.uk>
Date: Fri, 17 May 2024 22:02:01 +0100
Subject: [PATCH] when going through password recovery, only wipe the recovery
 token after the user has updated their password

see https://github.com/postfixadmin/postfixadmin/issues/550
---
 functions.inc.php                |  4 ++--
 model/PFAHandler.php             | 24 ++++++++++++++----------
 public/users/password-change.php |  8 ++++++--
 3 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/functions.inc.php b/functions.inc.php
index c32d927d..034f6134 100644
--- a/functions.inc.php
+++ b/functions.inc.php
@@ -424,7 +424,7 @@ function escape_string($string_or_int)
  *
  * @param string $param parameter name.
  * @param string $default (optional) - default value if key is not set.
- * @return string
+ * @return string - can only return a string
  */
 function safeget($param, $default = "")
 {
@@ -439,7 +439,7 @@ function safeget($param, $default = "")
  * safepost - similar to safeget() but for $_POST
  * @param string $param parameter name
  * @param string $default (optional) default value (defaults to "")
- * @return string - value in $_POST[$param] or $default
+ * @return string - value in $_POST[$param] or $default - we do not support array like value(s) etc
  * @see safeget()
  */
 function safepost($param, $default = "")
diff --git a/model/PFAHandler.php b/model/PFAHandler.php
index 8b473bb9..2f006b77 100644
--- a/model/PFAHandler.php
+++ b/model/PFAHandler.php
@@ -876,25 +876,29 @@ abstract class PFAHandler
         $now = date('Y-m-d H:i:s');
 
         $query = "SELECT token FROM $table WHERE {$this->id_field} = :username AND token <> '' AND active = :active AND token_validity > :now ";
-        $values = array('username' => $username, 'active' => $active, 'now' => $now);
+        $values = ['username' => $username, 'active' => $active, 'now' => $now];
 
         $result = db_query_all($query, $values);
         if (sizeof($result) == 1) {
             $row = $result[0];
-
             $crypt_token = pacrypt($token, $row['token'], $username);
-
-            if ($row['token'] == $crypt_token) {
-                db_update($this->db_table, $this->id_field, $username, array(
-                    'token' => '',
-                    'token_validity' => '2000-01-01 00:00:00',
-                ));
-                return true;
-            }
+            return $row['token'] == $crypt_token;
         }
         return false;
     }
 
+    /**
+     * Blindly wipe someone's password recovery token (even if they don't have one set!)
+     * @param string $username
+     * @return bool
+     * @throws Exception
+     */
+    public function wipePasswordRecoveryCode($username)
+    {
+        db_update($this->db_table, $this->id_field, $username, ['token' => '', 'token_validity' => '2000-01-01 00:00:00']);
+        return true;
+    }
+
     /**************************************************************************
      * functions to read protected variables
      */
diff --git a/public/users/password-change.php b/public/users/password-change.php
index 87af0224..b0ff0053 100644
--- a/public/users/password-change.php
+++ b/public/users/password-change.php
@@ -40,6 +40,9 @@ $CONF = Config::getInstance()->getAll();
 
 $smarty->configureTheme($rel_path);
 
+$tCode = null;
+$tUsername = null;
+
 if ($context === 'admin' && !Config::read('forgotten_admin_password_reset')) {
     die('Password change is disabled by configuration option: forgotten_admin_password_reset or mailbox_postpassword_script');
 }
@@ -82,6 +85,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') {
                 $values['password2'] = $fPassword2;
                 if ($handler->set($values) && $handler->save()) {
                     flash_info(Config::lang_f('pPassword_result_success', $tUsername));
+                    $handler->wipePasswordRecoveryCode($tUsername); // so we only wipe the recovery token if they've managed to change their password.
                     header('Location: main.php');
                     exit(0);
                 } else {
@@ -95,8 +99,8 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') {
 }
 
 $smarty->assign('language_selector', language_selector(), false);
-$smarty->assign('tUsername', @$tUsername);
-$smarty->assign('tCode', @$tCode);
+$smarty->assign('tUsername', $tUsername);
+$smarty->assign('tCode', $tCode);
 $smarty->assign('smarty_template', 'password-change');
 $smarty->display('index.tpl');