[FEATURE] Add TCA 'saltedPassword' eval for type=input 46/57746/5
authorChristian Kuhn <lolli@schwarzbu.ch>
Tue, 31 Jul 2018 14:28:14 +0000 (16:28 +0200)
committerWouter Wolters <typo3@wouterwolters.nl>
Tue, 31 Jul 2018 18:17:27 +0000 (20:17 +0200)
The salted passwords extension did register a custom
field evaluation to hash a given password when editing users
in the backend through FormEngine.
This is now turned into a general eval 'saltedPassword'
and handled in the DataHandler directly. Note the existing
eval 'password' is not extended since 'password' eval
changes the display part to show '****', which should be
kept seperated from the salt evaluation.
The "Evaluation" classes are dropped directly without
substitution since they are totally saltedpasswords internal
and don't make sense to be used externally.

Change-Id: I93306eaf18222d14e141a3d612a419a2b7174341
Resolves: #85698
Releases: master
Reviewed-on: https://review.typo3.org/57746
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Wouter Wolters <typo3@wouterwolters.nl>
Tested-by: Wouter Wolters <typo3@wouterwolters.nl>
13 files changed:
typo3/sysext/core/Classes/DataHandling/DataHandler.php
typo3/sysext/core/Configuration/TCA/be_users.php
typo3/sysext/core/Documentation/Changelog/master/Feature-85698-NewTypeinputEvalSaltedPassword.rst [new file with mode: 0644]
typo3/sysext/core/Documentation/Changelog/master/Important-85683-DoppedSaltedpasswordOptions.rst
typo3/sysext/core/Tests/Unit/DataHandling/DataHandlerTest.php
typo3/sysext/frontend/Configuration/TCA/fe_users.php
typo3/sysext/saltedpasswords/Classes/Evaluation/BackendEvaluator.php [deleted file]
typo3/sysext/saltedpasswords/Classes/Evaluation/Evaluator.php [deleted file]
typo3/sysext/saltedpasswords/Classes/Evaluation/FrontendEvaluator.php [deleted file]
typo3/sysext/saltedpasswords/Configuration/TCA/Overrides/be_users.php [deleted file]
typo3/sysext/saltedpasswords/Configuration/TCA/Overrides/fe_users.php [deleted file]
typo3/sysext/saltedpasswords/Tests/Unit/Evaluation/EvaluatorTest.php [deleted file]
typo3/sysext/saltedpasswords/ext_localconf.php

index 11537a2..5e36bff 100644 (file)
@@ -60,6 +60,7 @@ use TYPO3\CMS\Core\Utility\MathUtility;
 use TYPO3\CMS\Core\Utility\PathUtility;
 use TYPO3\CMS\Core\Utility\StringUtility;
 use TYPO3\CMS\Core\Versioning\VersionState;
+use TYPO3\CMS\Saltedpasswords\Salt\SaltFactory;
 
 /**
  * The main data handler class which takes care of correctly updating and inserting records.
@@ -1420,7 +1421,7 @@ class DataHandler implements LoggerAwareInterface
             return $labelPlaceholder;
         }
         $evalCodesArray = GeneralUtility::trimExplode(',', $GLOBALS['TCA'][$table]['columns'][$labelField]['config']['eval'], true);
-        $transformedLabel = $this->checkValue_input_Eval($labelPlaceholder, $evalCodesArray, '');
+        $transformedLabel = $this->checkValue_input_Eval($labelPlaceholder, $evalCodesArray, '', $table);
         return $transformedLabel['value'] ?? $labelPlaceholder;
     }
 
@@ -1872,7 +1873,7 @@ class DataHandler implements LoggerAwareInterface
                 $this->runtimeCache->set($cacheId, $evalCodesArray);
             }
 
-            $res = $this->checkValue_input_Eval($value, $evalCodesArray, $tcaFieldConf['is_in']);
+            $res = $this->checkValue_input_Eval($value, $evalCodesArray, $tcaFieldConf['is_in'], $table);
             if (isset($tcaFieldConf['dbType']) && isset($res['value']) && !$res['value']) {
                 // set the value to null if we have an empty value for a native field
                 $res['value'] = null;
@@ -2825,9 +2826,10 @@ class DataHandler implements LoggerAwareInterface
      * @param string $value Value to evaluate
      * @param array $evalArray Array of evaluations to traverse.
      * @param string $is_in Is-in string for 'is_in' evaluation
+     * @param string $table Table name the eval is evaluated on
      * @return array Modified $value in key 'value' or empty array
      */
-    public function checkValue_input_Eval($value, $evalArray, $is_in)
+    public function checkValue_input_Eval($value, $evalArray, $is_in, string $table = ''): array
     {
         $res = [];
         $set = true;
@@ -2940,6 +2942,30 @@ class DataHandler implements LoggerAwareInterface
                         $this->checkValue_input_ValidateEmail($value, $set);
                     }
                     break;
+                case 'saltedPassword':
+                    // An incoming value is either the salted password if the user did not change existing password
+                    // when submitting the form, or a plaintext new password that needs to be turned into a salted password now.
+                    // The strategy is to see if a salt instance can be created from the incoming value. If so,
+                    // no new password was submitted and we keep the value. If no salting instance can be created,
+                    // incoming value must be a new plain text value that needs to be hashed.
+                    $hashMethod = substr($value, 0, 2);
+                    // The old scheduler task turned existing non-salted passwords into salted hashes by taking the simple md5
+                    // and using that as 'password' and make a salted md5 from given hash. Those where then prefixed with 'M'.
+                    // SaltFactory::getSaltingInstance($value) only recognizes these salts if we cut off the M again.
+                    $isDeprecatedSaltedHash = $hashMethod === 'M$';
+                    $tempValue = $isDeprecatedSaltedHash ? substr($value, 1) : $value;
+                    $oldSaltInstance = SaltFactory::getSaltingInstance($tempValue);
+                    if (!is_object($oldSaltInstance)) {
+                        // We got no salted password instance, incoming value must be a new plaintext password
+                        // Get an instance of the current configured salted password strategy and hash the value
+                        if ($table === 'fe_users') {
+                            $newSaltInstance = SaltFactory::getSaltingInstance(null, 'FE');
+                        } else {
+                            $newSaltInstance = SaltFactory::getSaltingInstance();
+                        }
+                        $value = $newSaltInstance->getHashedPassword($value);
+                    }
+                    break;
                 default:
                     if (isset($GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['tce']['formevals'][$func])) {
                         if (class_exists($func)) {
index 82189c5..d78ea05 100644 (file)
@@ -55,8 +55,8 @@ return [
             'config' => [
                 'type' => 'input',
                 'size' => 20,
-                'max' => 40,
-                'eval' => 'trim,required,md5,password',
+                'max' => 100,
+                'eval' => 'trim,required,password,saltedPassword',
                 'autocomplete' => false,
             ]
         ],
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Feature-85698-NewTypeinputEvalSaltedPassword.rst b/typo3/sysext/core/Documentation/Changelog/master/Feature-85698-NewTypeinputEvalSaltedPassword.rst
new file mode 100644 (file)
index 0000000..0a0aa1d
--- /dev/null
@@ -0,0 +1,31 @@
+.. include:: ../../Includes.txt
+
+====================================================
+Feature: #85698 - New type=input eval saltedPassword
+====================================================
+
+See :issue:`85698`
+
+Description
+===========
+
+Setting passwords and storing them as salted passwords in the database
+is now supported by adding :php:`saltedPassword` to :php:`TCA` :php:`type=input`
+fields.
+
+Fields having this eval set will get their value evaluated to a salted
+hash before they are stored by the :php:`DataHandler`.
+
+Note the salt configuration for backend (BE) is considered when using this eval
+on tables that is not the :php:`fe_users` table.
+
+
+Impact
+======
+
+The new eval substitutes custom code that has been done within the
+salted passwords extension before. It has no impact on instances
+being upgraded.
+
+
+.. index:: Backend, TCA
\ No newline at end of file
index 08abfc8..004ab8e 100644 (file)
@@ -1,7 +1,7 @@
 .. include:: ../../Includes.txt
 
 ====================================================
-Important: #85026 - Dropped salted passwords options
+Important: #85683 - Dropped salted passwords options
 ====================================================
 
 See :issue:`85683`
@@ -9,10 +9,10 @@ See :issue:`85683`
 Description
 ===========
 
-Some extension configuration of the salted passwords extension have been dropped:
+Some extension configuration of the salted passwords extension has been dropped:
 
 - FE.forceSalted and BE.forceSalted
-  By explicitly setting forceSalted to 1 (default 0) in the saltedpasswords extension configuration, it was possible to
+  By explicitly setting forceSalted to 1 (default 0) in the saltedpasswords extension configuration it was possible to
   deny login of users who had not stored their password as salted password yet. This option has been removed.
   User who have been upgraded to salted passwords using the "Convert user passwords to salted hashes" from
   core version 8 are still able to login and will get their passwords updated to the configured salted password
@@ -22,8 +22,8 @@ Some extension configuration of the salted passwords extension have been dropped
   version 7 or version 8 before upgrading to v9.
 
 - FE.updatePasswd and BE.updatePasswd
-  By explicitly setting updatePasswd to 0 (default 1) in the saltedpasswords extension configuration, it was possible
-  to not update a given hashed password to the currently configured has algorithm, but still allow login. This has
+  By explicitly setting updatePasswd to 0 (default 1) in the saltedpasswords extension configuration it was possible
+  to avoid updating a given hashed password to the currently configured hash algorithm, but still allow login. This option has
   been dropped: A user submitting a valid password using an old salted passwords algorithm that is no longer configured
   as current salted passwords algorithm will always get his password updated and stored using the currently
   configured password salt.
@@ -33,7 +33,7 @@ Some extension configuration of the salted passwords extension have been dropped
   to successfully validate a user. This setting is mostly useless since any different authentication service is usually
   configured to kick in before the native TYPO3 internal authentication service. It does not make sense to have this toggle
   and a search in open extensions revealed no usage. On upgrading to v9, if you are running additional authentication services,
-  please verify those have a higher priority than the default :php:`SaltedPasswordService`, actions are only needed if
+  please verify those have a higher priority than the default :php:`SaltedPasswordService`, action is only needed if
   additionally onlyAuthService has been set to 1 in salted passwords configuration, which is probably never the case.
 
 
index 156da3a..f781b8b 100644 (file)
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types = 1);
 namespace TYPO3\CMS\Core\Tests\Unit\DataHandling;
 
 /*
@@ -149,7 +150,7 @@ class DataHandlerTest extends UnitTestCase
     /**
      * @test
      */
-    public function evalCheckValueDouble2()
+    public function checkValueInputEvalWithEvalDouble2(): void
     {
         $testData = [
             '-0,5' => '-0.50',
@@ -165,7 +166,10 @@ class DataHandlerTest extends UnitTestCase
         }
     }
 
-    public function dataProviderDatetime()
+    /**
+     * @return array
+     */
+    public function checkValueInputEvalWithEvalDatetimeDataProvider(): array
     {
         // Three elements: input, timezone of input, expected output (UTC)
         return [
@@ -180,9 +184,9 @@ class DataHandlerTest extends UnitTestCase
 
     /**
      * @test
-     * @dataProvider dataProviderDatetime
+     * @dataProvider checkValueInputEvalWithEvalDatetimeDataProvider
      */
-    public function evalCheckValueDatetime($input, $serverTimezone, $expectedOutput)
+    public function checkValueInputEvalWithEvalDatetime($input, $serverTimezone, $expectedOutput): void
     {
         $oldTimezone = date_default_timezone_get();
         date_default_timezone_set($serverTimezone);
@@ -196,6 +200,42 @@ class DataHandlerTest extends UnitTestCase
     }
 
     /**
+     * @test
+     */
+    public function checkValueInputEvalWithSaltedPasswordKeepsExistingHash(): void
+    {
+        // Note the involved salted passwords are NOT mocked since the factory is static
+        $subject = new DataHandler();
+        $inputValue = '$1$GNu9HdMt$RwkPb28pce4nXZfnplVZY/';
+        $result = $subject->checkValue_input_Eval($inputValue, ['saltedPassword'], '', 'be_users');
+        $this->assertSame($inputValue, $result['value']);
+    }
+
+    /**
+     * @test
+     */
+    public function checkValueInputEvalWithSaltedPasswordKeepsExistingHashForMd5HashedHash(): void
+    {
+        // Note the involved salted passwords are NOT mocked since the factory is static
+        $subject = new DataHandler();
+        $inputValue = 'M$1$GNu9HdMt$RwkPb28pce4nXZfnplVZY/';
+        $result = $subject->checkValue_input_Eval($inputValue, ['saltedPassword'], '', 'be_users');
+        $this->assertSame($inputValue, $result['value']);
+    }
+
+    /**
+     * @test
+     */
+    public function checkValueInputEvalWithSaltedPasswordReturnsHashForSaltedPassword(): void
+    {
+        // Note the involved salted passwords are NOT mocked since the factory is static
+        $inputValue = 'myPassword';
+        $subject = new DataHandler();
+        $result = $subject->checkValue_input_Eval($inputValue, ['saltedPassword'], '', 'be_users');
+        $this->assertNotSame($inputValue, $result['value']);
+    }
+
+    /**
      * Data provider for inputValueCheckRecognizesStringValuesAsIntegerValuesCorrectly
      *
      * @return array
index 178bd04..effc247 100644 (file)
@@ -38,9 +38,9 @@ return [
             'label' => 'LLL:EXT:frontend/Resources/Private/Language/locallang_tca.xlf:fe_users.password',
             'config' => [
                 'type' => 'input',
-                'size' => 10,
-                'max' => 40,
-                'eval' => 'trim,required,password',
+                'size' => 20,
+                'max' => 100,
+                'eval' => 'trim,required,password,saltedPassword',
                 'autocomplete' => false,
             ]
         ],
diff --git a/typo3/sysext/saltedpasswords/Classes/Evaluation/BackendEvaluator.php b/typo3/sysext/saltedpasswords/Classes/Evaluation/BackendEvaluator.php
deleted file mode 100644 (file)
index d4e03b3..0000000
+++ /dev/null
@@ -1,29 +0,0 @@
-<?php
-namespace TYPO3\CMS\Saltedpasswords\Evaluation;
-
-/*
- * This file is part of the TYPO3 CMS project.
- *
- * It is free software; you can redistribute it and/or modify it under
- * the terms of the GNU General Public License, either version 2
- * of the License, or any later version.
- *
- * For the full copyright and license information, please read the
- * LICENSE.txt file that was distributed with this source code.
- *
- * The TYPO3 project - inspiring people to share!
- */
-
-/**
- * Class implementing salted evaluation methods for BE users.
- */
-class BackendEvaluator extends Evaluator
-{
-    /**
-     * Class constructor.
-     */
-    public function __construct()
-    {
-        $this->mode = 'BE';
-    }
-}
diff --git a/typo3/sysext/saltedpasswords/Classes/Evaluation/Evaluator.php b/typo3/sysext/saltedpasswords/Classes/Evaluation/Evaluator.php
deleted file mode 100644 (file)
index 81f5ad1..0000000
+++ /dev/null
@@ -1,63 +0,0 @@
-<?php
-namespace TYPO3\CMS\Saltedpasswords\Evaluation;
-
-/*
- * This file is part of the TYPO3 CMS project.
- *
- * It is free software; you can redistribute it and/or modify it under
- * the terms of the GNU General Public License, either version 2
- * of the License, or any later version.
- *
- * For the full copyright and license information, please read the
- * LICENSE.txt file that was distributed with this source code.
- *
- * The TYPO3 project - inspiring people to share!
- */
-
-use TYPO3\CMS\Saltedpasswords\Salt\SaltFactory;
-
-/**
- * Class implementing salted evaluation methods.
- */
-class Evaluator
-{
-    /**
-     * This function just return the field value as it is. No transforming,
-     * hashing will be done on server-side.
-     *
-     * @return string JavaScript code for evaluating the
-     */
-    public function returnFieldJS()
-    {
-        return 'return value;';
-    }
-
-    /**
-     * Function uses Portable PHP Hashing Framework to create a proper password string if needed
-     *
-     * @param mixed $value The value that has to be checked.
-     * @param string $is_in Is-In String
-     * @param bool $set Determines if the field can be set (value correct) or not, e.g. if input is required but the value is empty, then $set should be set to FALSE. (PASSED BY REFERENCE!)
-     * @return string The new value of the field
-     */
-    public function evaluateFieldValue($value, $is_in, &$set)
-    {
-        $isMD5 = preg_match('/[0-9abcdef]{32,32}/', $value);
-        $hashingMethod = substr($value, 0, 2);
-        $isDeprecatedSaltedHash = ($hashingMethod === 'C$' || $hashingMethod === 'M$');
-        $objInstanceSaltedPW = SaltFactory::getSaltingInstance();
-        if ($isMD5) {
-            $set = true;
-            $value = 'M' . $objInstanceSaltedPW->getHashedPassword($value);
-        } else {
-            // Determine method used for the (possibly) salted hashed password
-            $tempValue = $isDeprecatedSaltedHash ? substr($value, 1) : $value;
-            $tempObjInstanceSaltedPW = SaltFactory::getSaltingInstance($tempValue);
-            if (!is_object($tempObjInstanceSaltedPW)) {
-                $set = true;
-                $value = $objInstanceSaltedPW->getHashedPassword($value);
-            }
-        }
-        return $value;
-    }
-}
diff --git a/typo3/sysext/saltedpasswords/Classes/Evaluation/FrontendEvaluator.php b/typo3/sysext/saltedpasswords/Classes/Evaluation/FrontendEvaluator.php
deleted file mode 100644 (file)
index 8ce37a8..0000000
+++ /dev/null
@@ -1,29 +0,0 @@
-<?php
-namespace TYPO3\CMS\Saltedpasswords\Evaluation;
-
-/*
- * This file is part of the TYPO3 CMS project.
- *
- * It is free software; you can redistribute it and/or modify it under
- * the terms of the GNU General Public License, either version 2
- * of the License, or any later version.
- *
- * For the full copyright and license information, please read the
- * LICENSE.txt file that was distributed with this source code.
- *
- * The TYPO3 project - inspiring people to share!
- */
-
-/**
- * Class implementing salted evaluation methods for FE users.
- */
-class FrontendEvaluator extends Evaluator
-{
-    /**
-     * Class constructor.
-     */
-    public function __construct()
-    {
-        $this->mode = 'FE';
-    }
-}
diff --git a/typo3/sysext/saltedpasswords/Configuration/TCA/Overrides/be_users.php b/typo3/sysext/saltedpasswords/Configuration/TCA/Overrides/be_users.php
deleted file mode 100644 (file)
index e99b391..0000000
+++ /dev/null
@@ -1,15 +0,0 @@
-<?php
-defined('TYPO3_MODE') or die();
-
-$GLOBALS['TCA']['be_users']['columns']['password']['config']['max'] = 100;
-
-// Backend configuration for saltedpasswords
-// Get eval field operations methods as array keys
-$operations = array_flip(\TYPO3\CMS\Core\Utility\GeneralUtility::trimExplode(',', $GLOBALS['TCA']['be_users']['columns']['password']['config']['eval'], true));
-// Remove md5 and temporary password from the list of evaluated methods
-unset($operations['md5'], $operations['password']);
-// Append new methods to have "password" as last operation.
-$operations['TYPO3\\CMS\\Saltedpasswords\\Evaluation\\BackendEvaluator'] = 1;
-$operations['password'] = 1;
-$GLOBALS['TCA']['be_users']['columns']['password']['config']['eval'] = implode(',', array_keys($operations));
-unset($operations);
diff --git a/typo3/sysext/saltedpasswords/Configuration/TCA/Overrides/fe_users.php b/typo3/sysext/saltedpasswords/Configuration/TCA/Overrides/fe_users.php
deleted file mode 100644 (file)
index 8197085..0000000
+++ /dev/null
@@ -1,13 +0,0 @@
-<?php
-defined('TYPO3_MODE') or die();
-
-$GLOBALS['TCA']['fe_users']['columns']['password']['config']['max'] = 100;
-// Get eval field operations methods as array keys
-$operations = array_flip(\TYPO3\CMS\Core\Utility\GeneralUtility::trimExplode(',', $GLOBALS['TCA']['fe_users']['columns']['password']['config']['eval'], true));
-// Remove md5 and temporary password from the list of evaluated methods
-unset($operations['md5'], $operations['password']);
-// Append new methods to have "password" as last operation.
-$operations[\TYPO3\CMS\Saltedpasswords\Evaluation\FrontendEvaluator::class] = 1;
-$operations['password'] = 1;
-$GLOBALS['TCA']['fe_users']['columns']['password']['config']['eval'] = implode(',', array_keys($operations));
-unset($operations);
diff --git a/typo3/sysext/saltedpasswords/Tests/Unit/Evaluation/EvaluatorTest.php b/typo3/sysext/saltedpasswords/Tests/Unit/Evaluation/EvaluatorTest.php
deleted file mode 100644 (file)
index cafaa1d..0000000
+++ /dev/null
@@ -1,64 +0,0 @@
-<?php
-namespace TYPO3\CMS\Saltedpasswords\Tests\Unit\Evaluation;
-
-/*
- * This file is part of the TYPO3 CMS project.
- *
- * It is free software; you can redistribute it and/or modify it under
- * the terms of the GNU General Public License, either version 2
- * of the License, or any later version.
- *
- * For the full copyright and license information, please read the
- * LICENSE.txt file that was distributed with this source code.
- *
- * The TYPO3 project - inspiring people to share!
- */
-
-use TYPO3\CMS\Core\Utility\GeneralUtility;
-use TYPO3\CMS\Saltedpasswords\Evaluation\Evaluator;
-use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
-
-/**
- * Test case
- */
-class EvaluatorTest extends UnitTestCase
-{
-    /**
-     * @test
-     */
-    public function passwordIsTurnedIntoSaltedString()
-    {
-        $isSet = null;
-        $originalPassword = 'password';
-        $saltedPassword = (new Evaluator())->evaluateFieldValue($originalPassword, '', $isSet);
-        $isSalted = substr($saltedPassword, 0, 1) === '$';
-        $this->assertTrue($isSet);
-        $this->assertNotEquals($originalPassword, $saltedPassword);
-        $this->assertTrue($isSalted);
-    }
-
-    /**
-     * @test
-     */
-    public function md5HashIsUpdatedToTemporarySaltedString()
-    {
-        $isSet = null;
-        $originalPassword = '5f4dcc3b5aa765d61d8327deb882cf99';
-        $saltedPassword = (new Evaluator())->evaluateFieldValue($originalPassword, '', $isSet);
-        $this->assertTrue($isSet);
-        $this->assertNotEquals($originalPassword, $saltedPassword);
-        $this->assertTrue(GeneralUtility::isFirstPartOfStr($saltedPassword, 'M$'));
-    }
-
-    /**
-     * @test
-     */
-    public function temporarySaltedStringIsNotTouched()
-    {
-        $isSet = null;
-        $originalPassword = 'M$P$CibIRipvLfaPlaaeH8ifu9g21BrPjp.';
-        $saltedPassword = (new Evaluator())->evaluateFieldValue($originalPassword, '', $isSet);
-        $this->assertNull($isSet);
-        $this->assertSame($originalPassword, $saltedPassword);
-    }
-}
index 6a548cc..d22bf0c 100644 (file)
@@ -1,11 +1,6 @@
 <?php
 defined('TYPO3_MODE') or die();
 
-// Form evaluation function for fe_users
-$GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['tce']['formevals'][\TYPO3\CMS\Saltedpasswords\Evaluation\FrontendEvaluator::class] = '';
-// Form evaluation function for be_users
-$GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['tce']['formevals'][\TYPO3\CMS\Saltedpasswords\Evaluation\BackendEvaluator::class] = '';
-
 // Hook for processing "forgotPassword" in felogin
 $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['felogin']['password_changed'][] = \TYPO3\CMS\Saltedpasswords\Utility\SaltedPasswordsUtility::class . '->feloginForgotPasswordHook';