[TASK] Refactor SaltFactory 47/57847/11
authorChristian Kuhn <lolli@schwarzbu.ch>
Thu, 9 Aug 2018 15:47:30 +0000 (17:47 +0200)
committerAndreas Fernandez <a.fernandez@scripting-base.de>
Thu, 9 Aug 2018 18:39:03 +0000 (20:39 +0200)
The patch deprecates static SaltFactory::getSaltingInstance()
and replaces it with the two new non-static methods.
The ->get() method returns a hash instance to check a given password
against a given hash, and ->getDefaultHashInstance() which returns
an instance of the configured default hash method to calculate
a hash for a new password.

The new methods are now strict, non-static and throw exceptions if
something goes wrong. This simplifies mocking in tests and sanitizes
password hash handling in the core.

Change-Id: I186576593202cb6d052bc7c1ca6f81314eddbaf2
Resolves: #85796
Releases: master
Reviewed-on: https://review.typo3.org/57847
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Tested-by: Markus Klein <markus.klein@typo3.org>
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: Andreas Fernandez <a.fernandez@scripting-base.de>
20 files changed:
typo3/sysext/core/Classes/Authentication/AuthenticationService.php
typo3/sysext/core/Classes/Authentication/CommandLineUserAuthentication.php
typo3/sysext/core/Classes/DataHandling/DataHandler.php
typo3/sysext/core/Documentation/Changelog/master/Deprecation-85796-SaltedPasswordsCleanups.rst [new file with mode: 0644]
typo3/sysext/felogin/Classes/Controller/FrontendLoginController.php
typo3/sysext/install/Classes/Authentication/AuthenticationService.php
typo3/sysext/install/Classes/Controller/InstallerController.php
typo3/sysext/install/Classes/Controller/MaintenanceController.php
typo3/sysext/install/Classes/Controller/SettingsController.php
typo3/sysext/install/Classes/Http/RequestHandler.php
typo3/sysext/install/Classes/Report/SecurityStatusReport.php
typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallStaticMatcher.php
typo3/sysext/reports/Classes/Report/Status/SecurityStatus.php
typo3/sysext/saltedpasswords/Classes/Salt/SaltFactory.php
typo3/sysext/saltedpasswords/Classes/SaltedPasswordService.php
typo3/sysext/saltedpasswords/Classes/Utility/SaltedPasswordsUtility.php
typo3/sysext/saltedpasswords/Documentation/DevelopersGuide/Index.rst
typo3/sysext/saltedpasswords/Tests/Unit/Salt/SaltFactoryTest.php
typo3/sysext/saltedpasswords/Tests/UnitDeprecated/Salt/SaltFactoryTest.php [new file with mode: 0644]
typo3/sysext/setup/Classes/Controller/SetupModuleController.php

index e9c6a15..c085418 100644 (file)
@@ -19,6 +19,7 @@ use TYPO3\CMS\Core\Database\ConnectionPool;
 use TYPO3\CMS\Core\Database\Query\Restriction\HiddenRestriction;
 use TYPO3\CMS\Core\TimeTracker\TimeTracker;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Saltedpasswords\Exception\InvalidSaltException;
 use TYPO3\CMS\Saltedpasswords\Salt\SaltFactory;
 use TYPO3\CMS\Saltedpasswords\Salt\SaltInterface;
 
@@ -117,18 +118,26 @@ class AuthenticationService extends AbstractAuthenticationService
         $isReHashNeeded = false;
         $isDomainLockMet = false;
 
+        $saltFactory = GeneralUtility::makeInstance(SaltFactory::class);
+
         // Get a hashed password instance for the hash stored in db of this user
-        $saltedPasswordInstance = SaltFactory::getSaltingInstance($passwordHashInDatabase);
+        try {
+            $hashInstance = $saltFactory->get($passwordHashInDatabase);
+        } catch (InvalidSaltException $e) {
+            // This can be refactored if the 'else' part below is gone in v10: Log and return 100 here
+            $hashInstance = null;
+        }
         // An instance of the currently configured salted password mechanism
-        $currentConfiguredSaltedPasswordInstance = SaltFactory::getSaltingInstance(null);
+        // Don't catch InvalidSaltException here: Only install tool should handle those configuration failures
+        $defaultHashInstance = $saltFactory->getDefaultHashInstance(TYPO3_MODE);
 
-        if ($saltedPasswordInstance instanceof SaltInterface) {
+        if ($hashInstance instanceof SaltInterface) {
             // We found a hash class that can handle this type of hash
             $isSaltedPassword = true;
-            $isValidPassword = $saltedPasswordInstance->checkPassword($submittedPassword, $passwordHashInDatabase);
+            $isValidPassword = $hashInstance->checkPassword($submittedPassword, $passwordHashInDatabase);
             if ($isValidPassword) {
-                if ($saltedPasswordInstance->isHashUpdateNeeded($passwordHashInDatabase)
-                    || $currentConfiguredSaltedPasswordInstance != $saltedPasswordInstance
+                if ($hashInstance->isHashUpdateNeeded($passwordHashInDatabase)
+                    || $defaultHashInstance != $hashInstance
                 ) {
                     // Lax object comparison intended: Rehash if old and new salt objects are not
                     // instances of the same class.
@@ -148,10 +157,10 @@ class AuthenticationService extends AbstractAuthenticationService
                 // If the stored db password starts with M$, it may be a md5 password that has been
                 // upgraded to a salted md5 using the old salted passwords scheduler task.
                 // See if a salt instance is returned if we cut off the M, so Md5Salt kicks in
-                $saltedPasswordInstance = SaltFactory::getSaltingInstance(substr($passwordHashInDatabase, 1));
-                if ($saltedPasswordInstance instanceof SaltInterface) {
+                try {
+                    $hashInstance = $saltFactory->get(substr($passwordHashInDatabase, 1));
                     $isSaltedPassword = true;
-                    $isValidPassword = $saltedPasswordInstance->checkPassword(md5($submittedPassword), substr($passwordHashInDatabase, 1));
+                    $isValidPassword = $hashInstance->checkPassword(md5($submittedPassword), substr($passwordHashInDatabase, 1));
                     if ($isValidPassword) {
                         // Upgrade this password to a sane mechanism now
                         $isReHashNeeded = true;
@@ -163,6 +172,8 @@ class AuthenticationService extends AbstractAuthenticationService
                             $isDomainLockMet = true;
                         }
                     }
+                } catch (InvalidSaltException $e) {
+                    // Still no instance found: $isSaltedPasswords is NOT set to true, logging and return done below
                 }
             }
         }
@@ -204,7 +215,7 @@ class AuthenticationService extends AbstractAuthenticationService
             $this->updatePasswordHashInDatabase(
                 $userDatabaseTable,
                 (int)$user['uid'],
-                $currentConfiguredSaltedPasswordInstance->getHashedPassword($submittedPassword)
+                $defaultHashInstance->getHashedPassword($submittedPassword)
             );
         }
 
index 77d5dba..94ca947 100644 (file)
@@ -148,7 +148,7 @@ class CommandLineUserAuthentication extends BackendUserAuthentication
     {
         $cryptoService = GeneralUtility::makeInstance(Random::class);
         $password = $cryptoService->generateRandomBytes(20);
-        $saltFactory = SaltFactory::getSaltingInstance(null, 'BE');
-        return $saltFactory->getHashedPassword($password);
+        $hashInstance = GeneralUtility::makeInstance(SaltFactory::class)->getDefaultHashInstance('BE');
+        return $hashInstance->getHashedPassword($password);
     }
 }
index 819ea0a..3d1cba2 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\Exception\InvalidSaltException;
 use TYPO3\CMS\Saltedpasswords\Salt\SaltFactory;
 
 /**
@@ -2953,19 +2954,23 @@ class DataHandler implements LoggerAwareInterface
                     $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.
+                    // SaltFactory->get($value) only recognizes these salts if we cut off the M again.
+                    // @todo @deprecated: $isDeprecatedSaltedHash should be removed in v10.0 as dedicated breaking patch, similar
+                    // @todo to authUser() of AuthenticationService::class
                     $isDeprecatedSaltedHash = $hashMethod === 'M$';
                     $tempValue = $isDeprecatedSaltedHash ? substr($value, 1) : $value;
-                    $oldSaltInstance = SaltFactory::getSaltingInstance($tempValue);
-                    if (!is_object($oldSaltInstance)) {
+                    $hashFactory = GeneralUtility::makeInstance(SaltFactory::class);
+                    try {
+                        $hashFactory->get($tempValue);
+                    } catch (InvalidSaltException $e) {
                         // 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');
+                            $newHashInstance = $hashFactory->getDefaultHashInstance('FE');
                         } else {
-                            $newSaltInstance = SaltFactory::getSaltingInstance();
+                            $newHashInstance = $hashFactory->getDefaultHashInstance('BE');
                         }
-                        $value = $newSaltInstance->getHashedPassword($value);
+                        $value = $newHashInstance->getHashedPassword($value);
                     }
                     break;
                 default:
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Deprecation-85796-SaltedPasswordsCleanups.rst b/typo3/sysext/core/Documentation/Changelog/master/Deprecation-85796-SaltedPasswordsCleanups.rst
new file mode 100644 (file)
index 0000000..15de81e
--- /dev/null
@@ -0,0 +1,49 @@
+.. include:: ../../Includes.txt
+
+===============================================
+Deprecation: #85796 - Salted passwords cleanups
+===============================================
+
+See :issue:`85796`
+
+Description
+===========
+
+These methods have been deprecated:
+
+* :php:`TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::getSaltingInstance()` - Use :php:`SaltFactory->get()` to
+  retrieve a hash instance of for a given password hash. Use :php:`SaltFactory->getDefaultHashInstance()`
+  to retrieve an instance of the configured default hash algorithm for a given context. See the method comments
+  for usage details.
+* :php:`TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::determineSaltingHashingMethod()` - Use
+  :php:`SaltFactory->getDefaultHashInstance()` instead
+* :php:`TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::setPreferredHashingMethod()` - This method has only been used
+  for unit testing has been deprecated without substitution since object instances of :php:`SaltFactory` can
+  now be properly mocked. Use :php:`Prophecy` to do that in unit tests that have :php:`SaltFactory` as dependency.
+* :php:`TYPO3\CMS\Saltedpasswords\Utility\SaltedPasswordsUtility->getNumberOfBackendUsersWithInsecurePassword()` -
+  This internal method is unused and there is no new implementation to substitute it.
+
+
+Impact
+======
+
+Calling one of the above methods will log a deprecation log entry and will trigger
+a fatal PHP error in core v10.
+
+
+Affected Installations
+======================
+
+Most instances are not affected by this change if they don't have custom authentication
+services loaded that add magic with stored local password hashes, and if they don't use
+the :php:`SaltFactory` in own extension which is a seldom use case.
+
+The extension scanner will find usages in extensions.
+
+
+Migration
+=========
+
+Use the new factory methods as outlined in the description section.
+
+.. index:: PHP-API, FullyScanned, ext:saltedpasswords
index 935404c..33d2332 100644 (file)
@@ -25,6 +25,7 @@ use TYPO3\CMS\Core\Database\Query\Restriction\FrontendRestrictionContainer;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
 use TYPO3\CMS\Frontend\Plugin\AbstractPlugin;
+use TYPO3\CMS\Saltedpasswords\Salt\SaltFactory;
 
 /**
  * Plugin 'Website User Login' for the 'felogin' extension.
@@ -352,8 +353,8 @@ class FrontendLoginController extends AbstractPlugin implements LoggerAwareInter
                         );
                     } else {
                         // Hash password using configured salted passwords hash mechanism for FE
-                        $objInstanceSaltedPW = \TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::getSaltingInstance();
-                        $newPass = $objInstanceSaltedPW->getHashedPassword($postData['password1']);
+                        $hashInstance = GeneralUtility::makeInstance(SaltFactory::class)->getDefaultHashInstance('FE');
+                        $newPass = $hashInstance->getHashedPassword($postData['password1']);
 
                         // Call a hook for further password processing
                         if ($GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['felogin']['password_changed']) {
index 87a8d63..b7bd761 100644 (file)
@@ -19,6 +19,7 @@ use TYPO3\CMS\Core\Configuration\ConfigurationManager;
 use TYPO3\CMS\Core\Mail\MailMessage;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Install\Service\SessionService;
+use TYPO3\CMS\Saltedpasswords\Exception\InvalidSaltException;
 use TYPO3\CMS\Saltedpasswords\Salt\SaltFactory;
 
 /**
@@ -50,18 +51,24 @@ class AuthenticationService
         $validPassword = false;
         if ($password !== null && $password !== '') {
             $installToolPassword = $GLOBALS['TYPO3_CONF_VARS']['BE']['installToolPassword'];
-            $saltFactory = SaltFactory::getSaltingInstance($installToolPassword);
-            if (is_object($saltFactory)) {
-                $validPassword = $saltFactory->checkPassword($password, $installToolPassword);
-            } elseif (md5($password) === $installToolPassword) {
-                // Update install tool password if it is still "MD5"
-                $saltFactory = SaltFactory::getSaltingInstance(null, 'BE');
-                $configurationManager = GeneralUtility::makeInstance(ConfigurationManager::class);
-                $configurationManager->setLocalConfigurationValueByPath(
-                    'BE/installToolPassword',
-                    $saltFactory->getHashedPassword($password)
-                );
-                $validPassword = true;
+            $hashFactory = GeneralUtility::makeInstance(SaltFactory::class);
+            try {
+                $hashInstance = $hashFactory->get($installToolPassword);
+                $validPassword = $hashInstance->checkPassword($password, $installToolPassword);
+            } catch (InvalidSaltException $e) {
+                // Given hash in global configuration is not a valid salted password
+                if (md5($password) === $installToolPassword) {
+                    // Update configured install tool hash if it is still "MD5" and password matches
+                    // @todo: This should be removed in v10 with a dedicated breaking patch
+                    // @todo: Additionally, this code should check required hash updates and update the hash if needed
+                    $hashInstance = $hashFactory->getDefaultHashInstance('BE');
+                    $configurationManager = GeneralUtility::makeInstance(ConfigurationManager::class);
+                    $configurationManager->setLocalConfigurationValueByPath(
+                        'BE/installToolPassword',
+                        $hashInstance->getHashedPassword($password)
+                    );
+                    $validPassword = true;
+                }
             }
         }
         if ($validPassword) {
index 60efa98..67638a9 100644 (file)
@@ -1092,15 +1092,14 @@ For each website you need a TypoScript template on the main page of your website
     }
 
     /**
-     * This function returns a salted hashed key.
+     * This function returns a salted hashed key for new backend user password and install tool password
      *
      * @param string $password
      * @return string
      */
     protected function getHashedPassword($password)
     {
-        $saltFactory = SaltFactory::getSaltingInstance(null, 'BE');
-        return $saltFactory->getHashedPassword($password);
+        return GeneralUtility::makeInstance(SaltFactory::class)->getDefaultHashInstance('BE')->getHashedPassword($password);
     }
 
     /**
index 5020732..8767a39 100644 (file)
@@ -478,8 +478,8 @@ class MaintenanceController extends AbstractController
                     FlashMessage::ERROR
                 ));
             } else {
-                $saltFactory = SaltFactory::getSaltingInstance(null, 'BE');
-                $hashedPassword = $saltFactory->getHashedPassword($password);
+                $hashInstance = GeneralUtility::makeInstance(SaltFactory::class)->getDefaultHashInstance('BE');
+                $hashedPassword = $hashInstance->getHashedPassword($password);
                 $adminUserFields = [
                     'username' => $username,
                     'password' => $hashedPassword,
index 553b0cc..a1055bb 100644 (file)
@@ -100,11 +100,11 @@ class SettingsController extends AbstractController
                 FlashMessage::ERROR
             ));
         } else {
-            $saltFactory = SaltFactory::getSaltingInstance(null, 'BE');
+            $hashInstance = GeneralUtility::makeInstance(SaltFactory::class)->getDefaultHashInstance('BE');
             $configurationManager = GeneralUtility::makeInstance(ConfigurationManager::class);
             $configurationManager->setLocalConfigurationValueByPath(
                 'BE/installToolPassword',
-                $saltFactory->getHashedPassword($password)
+                $hashInstance->getHashedPassword($password)
             );
             $messageQueue->enqueue(new FlashMessage('Install tool password changed'));
         }
index 8c7c10c..bd756f0 100644 (file)
@@ -138,8 +138,8 @@ class RequestHandler implements RequestHandlerInterface
                         new FlashMessage('Please enter the install tool password', '', FlashMessage::ERROR)
                     );
                 } else {
-                    $saltFactory = SaltFactory::getSaltingInstance(null, 'BE');
-                    $hashedPassword = $saltFactory->getHashedPassword($password);
+                    $hashInstance = GeneralUtility::makeInstance(SaltFactory::class)->getDefaultHashInstance('BE');
+                    $hashedPassword = $hashInstance->getHashedPassword($password);
                     $messageQueue = (new FlashMessageQueue('install'))->enqueue(
                         new FlashMessage(
                             'Given password does not match the install tool login password. Calculated hash: ' . $hashedPassword,
index 06a5af0..d7e06de 100644 (file)
@@ -18,6 +18,8 @@ use TYPO3\CMS\Core\Core\Environment;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Install\Service\EnableFileService;
 use TYPO3\CMS\Reports\Status;
+use TYPO3\CMS\Saltedpasswords\Exception\InvalidSaltException;
+use TYPO3\CMS\Saltedpasswords\Salt\SaltFactory;
 
 /**
  * Provides an status report of the security of the install tool
@@ -45,14 +47,21 @@ class SecurityStatusReport implements \TYPO3\CMS\Reports\StatusProviderInterface
      */
     protected function getInstallToolPasswordStatus()
     {
+        // @todo @deprecated: This should be removed in v10 when install tool allows proper hashes only
         $value = $GLOBALS['LANG']->getLL('status_ok');
         $message = '';
         $severity = Status::OK;
         $validPassword = true;
         $installToolPassword = $GLOBALS['TYPO3_CONF_VARS']['BE']['installToolPassword'];
-        $saltFactory = \TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::getSaltingInstance($installToolPassword);
-        if ($installToolPassword !== '' && is_object($saltFactory)) {
-            $validPassword = !$saltFactory->checkPassword('joh316', $installToolPassword);
+        $hashInstance = null;
+        $hashFactory = GeneralUtility::makeInstance(SaltFactory::class);
+        try {
+            $hashInstance = $hashFactory->get($installToolPassword);
+        } catch (InvalidSaltException $e) {
+            // $hashInstance stays null
+        }
+        if ($installToolPassword !== '' && $hashInstance === null) {
+            $validPassword = !$hashFactory->checkPassword('joh316', $installToolPassword);
         } elseif ($installToolPassword === md5('joh316')) {
             $validPassword = false;
         }
index b2caf46..4229b96 100644 (file)
@@ -624,4 +624,32 @@ return [
             'Deprecation-85760-DeprecateGeneralUtilityunQuoteFilenames.rst',
         ],
     ],
+    'TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::getSaltingInstance' => [
+        'numberOfMandatoryArguments' => 0,
+        'maximumNumberOfArguments' => 2,
+        'restFiles' => [
+            'Deprecation-85796-SaltedPasswordsCleanups.rst',
+        ],
+    ],
+    'TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::determineSaltingHashingMethod' => [
+        'numberOfMandatoryArguments' => 1,
+        'maximumNumberOfArguments' => 2,
+        'restFiles' => [
+            'Deprecation-85796-SaltedPasswordsCleanups.rst',
+        ],
+    ],
+    'TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::setPreferredHashingMethod' => [
+        'numberOfMandatoryArguments' => 1,
+        'maximumNumberOfArguments' => 1,
+        'restFiles' => [
+            'Deprecation-85796-SaltedPasswordsCleanups.rst',
+        ],
+    ],
+    'TYPO3\CMS\Saltedpasswords\Utility\SaltedPasswordsUtility::getNumberOfBackendUsersWithInsecurePassword' => [
+        'numberOfMandatoryArguments' => 0,
+        'maximumNumberOfArguments' => 0,
+        'restFiles' => [
+            'Deprecation-85796-SaltedPasswordsCleanups.rst',
+        ],
+    ],
 ];
index ca0d8ab..9f00a8d 100644 (file)
@@ -16,12 +16,14 @@ namespace TYPO3\CMS\Reports\Report\Status;
  */
 
 use Psr\Http\Message\ServerRequestInterface;
+use TYPO3\CMS\Backend\Routing\UriBuilder;
 use TYPO3\CMS\Core\Database\ConnectionPool;
 use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction;
 use TYPO3\CMS\Core\Localization\LanguageService;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Reports\RequestAwareStatusProviderInterface;
 use TYPO3\CMS\Reports\Status as ReportStatus;
+use TYPO3\CMS\Saltedpasswords\Exception\InvalidSaltException;
 use TYPO3\CMS\Saltedpasswords\Salt\SaltFactory;
 
 /**
@@ -159,35 +161,29 @@ class SecurityStatus implements RequestAwareStatusProviderInterface
             ->fetch();
 
         if (!empty($row)) {
-            $secure = true;
-            /** @var \TYPO3\CMS\Saltedpasswords\Salt\SaltInterface $saltingObject */
-            $saltingObject = SaltFactory::getSaltingInstance($row['password']);
-            if (is_object($saltingObject)) {
-                if ($saltingObject->checkPassword('password', $row['password'])) {
-                    $secure = false;
+            try {
+                $hashInstance = GeneralUtility::makeInstance(SaltFactory::class)->get($row['password']);
+                if ($hashInstance->checkPassword('password', $row['password'])) {
+                    // If the password for 'admin' user is 'password': bad idea!
+                    // We're checking since the (very) old installer created instances like this in dark old times.
+                    $uriBuilder = GeneralUtility::makeInstance(UriBuilder::class);
+                    $value = $this->getLanguageService()->getLL('status_insecure');
+                    $severity = ReportStatus::ERROR;
+                    $editUserAccountUrl = (string)$uriBuilder->buildUriFromRoute(
+                        'record_edit',
+                        [
+                            'edit[be_users][' . $row['uid'] . ']' => 'edit',
+                            'returnUrl' => (string)$uriBuilder->buildUriFromRoute('system_reports')
+                        ]
+                    );
+                    $message = sprintf(
+                        $this->getLanguageService()->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:warning.backend_admin'),
+                        '<a href="' . htmlspecialchars($editUserAccountUrl) . '">',
+                        '</a>'
+                    );
                 }
-            }
-            // Check against plain MD5
-            if ($row['password'] === '5f4dcc3b5aa765d61d8327deb882cf99') {
-                $secure = false;
-            }
-            if (!$secure) {
-                /** @var \TYPO3\CMS\Backend\Routing\UriBuilder $uriBuilder */
-                $uriBuilder = GeneralUtility::makeInstance(\TYPO3\CMS\Backend\Routing\UriBuilder::class);
-                $value = $this->getLanguageService()->getLL('status_insecure');
-                $severity = ReportStatus::ERROR;
-                $editUserAccountUrl = (string)$uriBuilder->buildUriFromRoute(
-                    'record_edit',
-                    [
-                        'edit[be_users][' . $row['uid'] . ']' => 'edit',
-                        'returnUrl' => (string)$uriBuilder->buildUriFromRoute('system_reports')
-                    ]
-                );
-                $message = sprintf(
-                    $this->getLanguageService()->sL('LLL:EXT:core/Resources/Private/Language/locallang_core.xlf:warning.backend_admin'),
-                    '<a href="' . htmlspecialchars($editUserAccountUrl) . '">',
-                    '</a>'
-                );
+            } catch (InvalidSaltException $e) {
+                // No hash class handling for current hash could be found. Not good, but ok in this case.
             }
         }
 
index 9a850a6..3a7da98 100644 (file)
@@ -16,11 +16,12 @@ namespace TYPO3\CMS\Saltedpasswords\Salt;
  */
 
 use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Saltedpasswords\Exception\InvalidSaltException;
 use TYPO3\CMS\Saltedpasswords\Utility\SaltedPasswordsUtility;
 
 /**
- * Class that implements Blowfish salted hashing based on PHP's
- * crypt() function.
+ * Factory class to find and return hash instances of given hashed passwords
+ * and to find and return default hash instances to hash new passwords.
  */
 class SaltFactory
 {
@@ -29,10 +30,77 @@ class SaltFactory
      * This member is set in the getSaltingInstance() function.
      *
      * @var SaltInterface
+     * @deprecated since TYPO3 v9, will be removed in TYPO3 v10
      */
     protected static $instance;
 
     /**
+     * Find a hash class that handles given hash and return an instance of it.
+     *
+     * @param string $hash Given hash to find instance for
+     * @return SaltInterface Object that can handle given hash
+     * @throws \LogicException If a registered hash class does not implement SaltInterface
+     * @throws InvalidSaltException If no class was found that handles given hash
+     */
+    public function get(string $hash): SaltInterface
+    {
+        // @todo: Refactor $registeredHashClasses when implementing 'preset' and moving config options
+        $registeredHashClasses = static::getRegisteredSaltedHashingMethods();
+
+        foreach ($registeredHashClasses as $className) {
+            $hashInstance = GeneralUtility::makeInstance($className);
+            if (!$hashInstance instanceof SaltInterface) {
+                throw new \LogicException('Class ' . $className . ' does not implement SaltInterface', 1533818569);
+            }
+            if ($hashInstance->isAvailable() && $hashInstance->isValidSaltedPW($hash)) {
+                return $hashInstance;
+            }
+        }
+        // Do not add the hash to the exception to prevent information disclosure
+        throw new InvalidSaltException('No implementation found that handles given hash.', 1533818591);
+    }
+
+    /**
+     * Determine configured default hash method and return an instance of the class representing it.
+     *
+     * @param string $mode 'FE' for frontend users, 'BE' for backend users
+     * @return SaltInterface Class instance that is configured as default hash method
+     * @throws \InvalidArgumentException If configured default hash class does not implement SaltInterface
+     * @throws InvalidSaltException If configuration is broken
+     */
+    public function getDefaultHashInstance(string $mode): SaltInterface
+    {
+        if ($mode !== 'FE' && $mode !== 'BE') {
+            throw new \InvalidArgumentException('Mode must be either \'FE\' or \'BE\', ' . $mode . ' given.', 1533820041);
+        }
+        $defaultHashClassName = SaltedPasswordsUtility::getDefaultSaltingHashingMethod($mode);
+
+        // @todo: Refactor $availableHashClasses when implementing 'preset' and moving config options
+        $availableHashClasses = static::getRegisteredSaltedHashingMethods();
+
+        if (!isset($availableHashClasses[$defaultHashClassName])) {
+            throw new InvalidSaltException(
+                'Configured default hash method ' . $defaultHashClassName . ' is not registered',
+                1533820194
+            );
+        }
+        $hashInstance =  GeneralUtility::makeInstance($defaultHashClassName);
+        if (!$hashInstance instanceof SaltInterface) {
+            throw new \RuntimeException(
+                'Configured default hash method ' . $defaultHashClassName . ' is not an instance of SaltInterface',
+                1533820281
+            );
+        }
+        if (!$hashInstance->isAvailable()) {
+            throw new InvalidSaltException(
+                'Configured default hash method ' . $defaultHashClassName . ' is not available, missing php requirement?',
+                1533822084
+            );
+        }
+        return $hashInstance;
+    }
+
+    /**
      * Returns list of all registered hashing methods. Used eg. in
      * extension configuration to select the default hashing method.
      *
@@ -40,7 +108,14 @@ class SaltFactory
      */
     public static function getRegisteredSaltedHashingMethods(): array
     {
-        $saltMethods = static::getDefaultSaltMethods();
+        $saltMethods = [
+            Md5Salt::class => Md5Salt::class,
+            BlowfishSalt::class => BlowfishSalt::class,
+            PhpassSalt::class => PhpassSalt::class,
+            Pbkdf2Salt::class => Pbkdf2Salt::class,
+            BcryptSalt::class => BcryptSalt::class,
+            Argon2iSalt::class => Argon2iSalt::class,
+        ];
         if (isset($GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['ext/saltedpasswords']['saltMethods'])) {
             $configuredMethods = (array)$GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['ext/saltedpasswords']['saltMethods'];
             if (!empty($configuredMethods)) {
@@ -58,23 +133,6 @@ class SaltFactory
     }
 
     /**
-     * Returns an array with default salt method class names.
-     *
-     * @return array
-     */
-    protected static function getDefaultSaltMethods(): array
-    {
-        return [
-            Md5Salt::class => Md5Salt::class,
-            BlowfishSalt::class => BlowfishSalt::class,
-            PhpassSalt::class => PhpassSalt::class,
-            Pbkdf2Salt::class => Pbkdf2Salt::class,
-            BcryptSalt::class => BcryptSalt::class,
-            Argon2iSalt::class => Argon2iSalt::class,
-        ];
-    }
-
-    /**
      * Obtains a salting hashing method instance.
      *
      * This function will return an instance of a class that implements
@@ -85,9 +143,14 @@ class SaltFactory
      * @param string|null $saltedHash Salted hashed password to determine the type of used method from or NULL to reset to the default type
      * @param string $mode The TYPO3 mode (FE or BE) saltedpasswords shall be used for
      * @return SaltInterface|null An instance of salting hash method class or null if given hash is not supported
+     * @deprecated since TYPO3 v9, will be removed in TYPO3 v10
      */
     public static function getSaltingInstance($saltedHash = '', $mode = TYPO3_MODE)
     {
+        trigger_error(
+            'This method is obsolete and will be removed in TYPO3 v10. Use get() and getDefaultHashInstance() instead.',
+            E_USER_DEPRECATED
+        );
         // Creating new instance when
         // * no instance existing
         // * a salted hash given to determine salted hashing method from
@@ -101,6 +164,7 @@ class SaltFactory
                 }
             } else {
                 $classNameToUse = SaltedPasswordsUtility::getDefaultSaltingHashingMethod($mode);
+                // Calls deprecated determineSaltingHashingMethod - ok, since getSaltingInstance() is deprecated, too
                 $availableClasses = static::getRegisteredSaltedHashingMethods();
                 self::$instance = GeneralUtility::makeInstance($availableClasses[$classNameToUse]);
             }
@@ -116,9 +180,14 @@ class SaltFactory
      * @param string $saltedHash
      * @param string $mode (optional) The TYPO3 mode (FE or BE) saltedpasswords shall be used for
      * @return bool TRUE, if salting hashing method has been found, otherwise FALSE
+     * @deprecated since TYPO3 v9, will be removed in TYPO3 v10
      */
     public static function determineSaltingHashingMethod(string $saltedHash, $mode = TYPO3_MODE): bool
     {
+        trigger_error(
+            'This method is obsolete and will be removed in TYPO3 v10.',
+            E_USER_DEPRECATED
+        );
         $registeredMethods = static::getRegisteredSaltedHashingMethods();
         $defaultClassName = SaltedPasswordsUtility::getDefaultSaltingHashingMethod($mode);
         $defaultReference = $registeredMethods[$defaultClassName];
@@ -144,9 +213,11 @@ class SaltFactory
      *
      * @param string $resource Object resource to use (e.g. \TYPO3\CMS\Saltedpasswords\Salt\BlowfishSalt::class)
      * @return SaltInterface|null An instance of salting hashing method object or null
+     * @deprecated since TYPO3 v9, will be removed in TYPO3 v10
      */
     public static function setPreferredHashingMethod(string $resource)
     {
+        trigger_error('This method is obsolete and will be removed in TYPO3 v10.', E_USER_DEPRECATED);
         self::$instance = null;
         $objectInstance = GeneralUtility::makeInstance($resource);
         if ($objectInstance instanceof SaltInterface) {
index 1b70ee8..83091cc 100644 (file)
@@ -100,6 +100,7 @@ class SaltedPasswordService extends AbstractAuthenticationService
         $validPasswd = false;
         $password = $loginData['uident_text'];
         // Determine method used for given salted hashed password
+        // This calls deprecated getSaltingInstance(). This is "ok" since this SaltedPasswordsService in itself is deprecated.
         $this->objInstanceSaltedPW = SaltFactory::getSaltingInstance($user['password']);
         // Existing record is in format of Salted Hash password
         if (is_object($this->objInstanceSaltedPW)) {
@@ -114,6 +115,7 @@ class SaltedPasswordService extends AbstractAuthenticationService
             // Test for wrong salted hashing method (only if current method is not related to default method)
             if ($validPasswd && get_class($this->objInstanceSaltedPW) !== $defaultHashingClassName && !is_subclass_of($this->objInstanceSaltedPW, $defaultHashingClassName)) {
                 // Instantiate default method class
+                // This calls deprecated getSaltingInstance(). This is "ok" since this SaltedPasswordsService in itself is deprecated.
                 $this->objInstanceSaltedPW = SaltFactory::getSaltingInstance(null);
                 $this->updatePassword((int)$user['uid'], ['password' => $this->objInstanceSaltedPW->getHashedPassword($password)]);
             }
@@ -125,6 +127,7 @@ class SaltedPasswordService extends AbstractAuthenticationService
             $hashingMethod = substr($user['password'], 0, 2);
             if ($hashingMethod === 'M$') {
                 // Instantiate default method class
+                // This calls deprecated getSaltingInstance(). This is "ok" since this SaltedPasswordsService in itself is deprecated.
                 $this->objInstanceSaltedPW = SaltFactory::getSaltingInstance(substr($user['password'], 1));
                 // md5 passwords that have been upgraded to salted passwords using old scheduler task
                 // @todo: The entire 'else' should be dropped in v10, admins had to upgrade users to salted passwords with v8 latest since the
@@ -139,6 +142,7 @@ class SaltedPasswordService extends AbstractAuthenticationService
             // Upgrade to a sane salt mechanism if password was correct
             if ($validPasswd) {
                 // Instantiate default method class
+                // This calls deprecated getSaltingInstance(). This is "ok" since this SaltedPasswordsService in itself is deprecated.
                 $this->objInstanceSaltedPW = SaltFactory::getSaltingInstance(null);
                 $this->updatePassword((int)$user['uid'], ['password' => $this->objInstanceSaltedPW->getHashedPassword($password)]);
             }
index 1b578a8..af86fa0 100644 (file)
@@ -32,9 +32,11 @@ class SaltedPasswordsUtility
      * Calculates number of backend users, who have no saltedpasswords protection.
      *
      * @return int
+     * @deprecated since TYPO3 v9, will be removed in TYPO3 v10
      */
     public static function getNumberOfBackendUsersWithInsecurePassword()
     {
+        trigger_error('This method is obsolete and will be removed in TYPO3 v10.', E_USER_DEPRECATED);
         $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable('be_users');
         $queryBuilder->getRestrictions()->removeAll();
 
index e6915b3..935e0c4 100644 (file)
@@ -25,22 +25,18 @@ Creating a hash
 When you want to create a new salted user password hash from a given
 plain-text password, these are the steps to be done:
 
-- let the factory deliver an instance of the default hashing class
-
-- create the salted user password hash
+* let the factory deliver an instance of the default hashing class with given context `FE` or `BE`
+* create the salted user password hash
 
 Example implementation for TYPO3 frontend:
 
 ::
 
-   // plain-text password
+   // Given plain text password
    $password = 'XXX';
-   $saltedPassword = '';
 
-   $objSalt = \TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::getSaltingInstance(NULL);
-   if (is_object($objSalt)) {
-       $saltedPassword = $objSalt->getHashedPassword($password);
-   }
+   $hashInstance = GeneralUtility::makeInstance(SaltFactory::class)->getDefaultHashInstance('FE');
+   $hashedPassword = $hashInstance->getHashedPassword($password);
 
 
 .. _checking-a-password:
@@ -51,9 +47,8 @@ Checking a password
 When you want to check a plain-text password against a salted user
 password hash, these are the steps to be done:
 
-- let the factory deliver an instance of the according hashing class
-
-- compare plain-text password with salted user password hash
+* let the factory deliver an instance of the according hashing class
+* compare plain-text password with salted user password hash
 
 Example implementation for TYPO3 frontend:
 
@@ -61,14 +56,11 @@ Example implementation for TYPO3 frontend:
 
    // plain-text password
    $password = 'XXX';
-   // salted user password hash
-   $saltedPassword = 'YYY';
-   // keeps status if plain-text password matches given salted user password hash
-   $success = FALSE;
-   $objSalt = \TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::getSaltingInstance($saltedPassword);
-   if (is_object($objSalt)) {
-       $success = $objSalt->checkPassword($password, $saltedPassword);
-   }
+   // stored password hash
+   $passwordHash = 'YYY';
+   $success = GeneralUtility::makeInstance(SaltFactory::class)
+       ->get($saltedPassword)
+       ->checkPassword($password, $passwordHash);
 
 
 .. _adding-a-new-salting-method:
index 029bb8d..3164cc8 100644 (file)
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types = 1);
 namespace TYPO3\CMS\Saltedpasswords\Tests\Unit\Salt;
 
 /*
@@ -14,7 +15,11 @@ namespace TYPO3\CMS\Saltedpasswords\Tests\Unit\Salt;
  * The TYPO3 project - inspiring people to share!
  */
 
-use TYPO3\CMS\Core\Crypto\Random;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Saltedpasswords\Exception\InvalidSaltException;
+use TYPO3\CMS\Saltedpasswords\Salt\Argon2iSalt;
+use TYPO3\CMS\Saltedpasswords\Salt\PhpassSalt;
+use TYPO3\CMS\Saltedpasswords\Salt\SaltFactory;
 use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
 
 /**
@@ -23,145 +28,123 @@ use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
 class SaltFactoryTest extends UnitTestCase
 {
     /**
-     * Keeps instance of object to test.
-     *
-     * @var \TYPO3\CMS\Saltedpasswords\Salt\SaltInterface
-     */
-    protected $objectInstance;
-
-    /**
-     * Sets up the fixtures for this testcase.
-     */
-    protected function setUp()
-    {
-        $GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS']['saltedpasswords'] = [
-            'BE' => [
-                'saltedPWHashingMethod' => \TYPO3\CMS\Saltedpasswords\Salt\Pbkdf2Salt::class,
-            ],
-            'FE' => [
-                'saltedPWHashingMethod' => \TYPO3\CMS\Saltedpasswords\Salt\Pbkdf2Salt::class,
-            ],
-        ];
-        $this->objectInstance = \TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::getSaltingInstance();
-    }
-
-    /**
      * @test
      */
-    public function objectInstanceNotNull()
+    public function getThrowsExceptionIfARegisteredHashDoesNotImplementSaltInterface(): void
     {
-        $this->assertNotNull($this->objectInstance);
+        $GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['ext/saltedpasswords']['saltMethods'] = [ \stdClass::class ];
+        $this->expectException(\LogicException::class);
+        $this->expectExceptionCode(1533818569);
+        (new SaltFactory())->get('ThisIsNotAValidHash');
     }
 
     /**
      * @test
      */
-    public function objectInstanceImplementsInterface()
+    public function getThrowsExceptionIfNoClassIsFoundThatHandlesGivenHash(): void
     {
-        $this->assertInstanceOf(\TYPO3\CMS\Saltedpasswords\Salt\SaltInterface::class, $this->objectInstance);
+        $this->expectException(InvalidSaltException::class);
+        $this->expectExceptionCode(1533818591);
+        (new SaltFactory())->get('ThisIsNotAValidHash');
     }
 
     /**
      * @test
      */
-    public function abstractComposedSaltBase64EncodeReturnsProperLength()
+    public function getThrowsExceptionIfClassThatHandlesAHashIsNotAvailable(): void
     {
-        // set up an instance that extends AbstractComposedSalt first
-        $saltPbkdf2 = '$pbkdf2-sha256$6400$0ZrzXitFSGltTQnBWOsdAw$Y11AchqV4b0sUisdZd0Xr97KWoymNE0LNNrnEgY4H9M';
-        $this->objectInstance = \TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::getSaltingInstance($saltPbkdf2);
-
-        // 3 Bytes should result in a 6 char length base64 encoded string
-        // used for MD5 and PHPass salted hashing
-        $byteLength = 3;
-        $reqLengthBase64 = (int)ceil($byteLength * 8 / 6);
-        $randomBytes = (new Random())->generateRandomBytes($byteLength);
-        $this->assertTrue(strlen($this->objectInstance->base64Encode($randomBytes, $byteLength)) == $reqLengthBase64);
-        // 16 Bytes should result in a 22 char length base64 encoded string
-        // used for Blowfish salted hashing
-        $byteLength = 16;
-        $reqLengthBase64 = (int)ceil($byteLength * 8 / 6);
-        $randomBytes = (new Random())->generateRandomBytes($byteLength);
-        $this->assertTrue(strlen($this->objectInstance->base64Encode($randomBytes, $byteLength)) == $reqLengthBase64);
+        $phpassProphecy = $this->prophesize(PhpassSalt::class);
+        GeneralUtility::addInstance(PhpassSalt::class, $phpassProphecy->reveal());
+        $phpassProphecy->isAvailable()->shouldBeCalled()->willReturn(false);
+        $this->expectException(InvalidSaltException::class);
+        $this->expectExceptionCode(1533818591);
+        (new SaltFactory())->get('$P$C7u7E10SBEie/Jbdz0jDtUcWhzgOPF.');
     }
 
     /**
      * @test
      */
-    public function objectInstanceForMD5Salts()
+    public function getThrowsExceptionIfClassThatHandlesAHashSaysNoToHash(): void
     {
-        $saltMD5 = '$1$rasmusle$rISCgZzpwk3UhDidwXvin0';
-        $this->objectInstance = \TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::getSaltingInstance($saltMD5);
-        $this->assertTrue(get_class($this->objectInstance) == \TYPO3\CMS\Saltedpasswords\Salt\Md5Salt::class || is_subclass_of($this->objectInstance, \TYPO3\CMS\Saltedpasswords\Salt\Md5Salt::class));
-        $this->assertInstanceOf(\TYPO3\CMS\Saltedpasswords\Salt\AbstractComposedSalt::class, $this->objectInstance);
+        $phpassProphecy = $this->prophesize(PhpassSalt::class);
+        GeneralUtility::addInstance(PhpassSalt::class, $phpassProphecy->reveal());
+        $phpassProphecy->isAvailable()->shouldBeCalled()->willReturn(true);
+        $hash = '$P$C7u7E10SBEie/Jbdz0jDtUcWhzgOPF.';
+        $phpassProphecy->isValidSaltedPW($hash)->shouldBeCalled()->willReturn(false);
+        $this->expectException(InvalidSaltException::class);
+        $this->expectExceptionCode(1533818591);
+        (new SaltFactory())->get($hash);
     }
 
     /**
      * @test
      */
-    public function objectInstanceForBlowfishSalts()
+    public function getReturnsInstanceOfHashClassThatHandlesHash(): void
     {
-        $saltBlowfish = '$2a$07$abcdefghijklmnopqrstuuIdQV69PAxWYTgmnoGpe0Sk47GNS/9ZW';
-        $this->objectInstance = \TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::getSaltingInstance($saltBlowfish);
-        $this->assertTrue(get_class($this->objectInstance) == \TYPO3\CMS\Saltedpasswords\Salt\BlowfishSalt::class || is_subclass_of($this->objectInstance, \TYPO3\CMS\Saltedpasswords\Salt\BlowfishSalt::class));
-        $this->assertInstanceOf(\TYPO3\CMS\Saltedpasswords\Salt\AbstractComposedSalt::class, $this->objectInstance);
+        $phpassProphecy = $this->prophesize(PhpassSalt::class);
+        $phpassRevelation = $phpassProphecy->reveal();
+        GeneralUtility::addInstance(PhpassSalt::class, $phpassRevelation);
+        $phpassProphecy->isAvailable()->shouldBeCalled()->willReturn(true);
+        $hash = '$P$C7u7E10SBEie/Jbdz0jDtUcWhzgOPF.';
+        $phpassProphecy->isValidSaltedPW($hash)->shouldBeCalled()->willReturn(true);
+        $this->assertSame($phpassRevelation, (new SaltFactory())->get($hash));
     }
 
     /**
      * @test
      */
-    public function objectInstanceForPhpassSalts()
+    public function getDefaultHashInstanceThrowsExceptionIfModeIsNotBeOrFe(): void
     {
-        $saltPhpass = '$P$CWF13LlG/0UcAQFUjnnS4LOqyRW43c.';
-        $this->objectInstance = \TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::getSaltingInstance($saltPhpass);
-        $this->assertTrue(get_class($this->objectInstance) == \TYPO3\CMS\Saltedpasswords\Salt\PhpassSalt::class || is_subclass_of($this->objectInstance, \TYPO3\CMS\Saltedpasswords\Salt\PhpassSalt::class));
-        $this->assertInstanceOf(\TYPO3\CMS\Saltedpasswords\Salt\AbstractComposedSalt::class, $this->objectInstance);
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionCode(1533820041);
+        (new SaltFactory())->getDefaultHashInstance('foo');
     }
 
     /**
      * @test
      */
-    public function objectInstanceForPbkdf2Salts()
+    public function getDefaultHashReturnsInstanceOfConfiguredDefaultFeMethod(): void
     {
-        $saltPbkdf2 = '$pbkdf2-sha256$6400$0ZrzXitFSGltTQnBWOsdAw$Y11AchqV4b0sUisdZd0Xr97KWoymNE0LNNrnEgY4H9M';
-        $this->objectInstance = \TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::getSaltingInstance($saltPbkdf2);
-        $this->assertTrue(get_class($this->objectInstance) == \TYPO3\CMS\Saltedpasswords\Salt\Pbkdf2Salt::class || is_subclass_of($this->objectInstance, \TYPO3\CMS\Saltedpasswords\Salt\Pbkdf2Salt::class));
-        $this->assertInstanceOf(\TYPO3\CMS\Saltedpasswords\Salt\AbstractComposedSalt::class, $this->objectInstance);
+        $GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS']['saltedpasswords']['FE']['saltedPWHashingMethod'] = Argon2iSalt::class;
+        $hashInstance = (new SaltFactory())->getDefaultHashInstance('FE');
+        $this->assertInstanceOf(Argon2iSalt::class, $hashInstance);
     }
 
     /**
      * @test
      */
-    public function objectInstanceForPhpPasswordHashBcryptSalts()
+    public function getDefaultHashReturnsInstanceOfConfiguredDefaultBeMethod(): void
     {
-        $saltBcrypt = '$2y$12$Tz.al0seuEgRt61u0bzqAOWu67PgG2ThG25oATJJ0oS5KLCPCgBOe';
-        $this->objectInstance = \TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::getSaltingInstance($saltBcrypt);
-        $this->assertInstanceOf(\TYPO3\CMS\Saltedpasswords\Salt\BcryptSalt::class, $this->objectInstance);
+        $GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS']['saltedpasswords']['BE']['saltedPWHashingMethod'] = Argon2iSalt::class;
+        $hashInstance = (new SaltFactory())->getDefaultHashInstance('BE');
+        $this->assertInstanceOf(Argon2iSalt::class, $hashInstance);
     }
 
     /**
      * @test
+     * @todo: have a test for exception 1533820194 after utility class is no longer used
      */
-    public function objectInstanceForPhpPasswordHashArgon2iSalts()
+    public function getDefaultHashThrowsExceptionIfDefaultHashMethodDoesNotImplementSaltInterface(): void
     {
-        $saltArgon2i = '$argon2i$v=19$m=8,t=1,p=1$djZiNkdEa3lOZm1SSmZsdQ$9iiRjpLZAT7kfHwS1xU9cqSU7+nXy275qpB/eKjI1ig';
-        $this->objectInstance = \TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::getSaltingInstance($saltArgon2i);
-        $this->assertInstanceOf(\TYPO3\CMS\Saltedpasswords\Salt\Argon2iSalt::class, $this->objectInstance);
+        $GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS']['saltedpasswords']['BE']['saltedPWHashingMethod'] = \stdClass::class;
+        $GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['ext/saltedpasswords']['saltMethods'] = [ \stdClass::class ];
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionCode(1533820281);
+        (new SaltFactory())->getDefaultHashInstance('BE');
     }
 
     /**
      * @test
      */
-    public function resettingFactoryInstanceSucceeds()
+    public function getDefaultHashThrowsExceptionIfDefaultHashMethodIsNotAvailable(): void
     {
-        $defaultClassNameToUse = \TYPO3\CMS\Saltedpasswords\Utility\SaltedPasswordsUtility::getDefaultSaltingHashingMethod();
-        if ($defaultClassNameToUse == \TYPO3\CMS\Saltedpasswords\Salt\Md5Salt::class) {
-            $saltedPW = '$P$CWF13LlG/0UcAQFUjnnS4LOqyRW43c.';
-        } else {
-            $saltedPW = '$1$rasmusle$rISCgZzpwk3UhDidwXvin0';
-        }
-        $this->objectInstance = \TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::getSaltingInstance($saltedPW);
-        // resetting
-        $this->objectInstance = \TYPO3\CMS\Saltedpasswords\Salt\SaltFactory::getSaltingInstance(null);
-        $this->assertTrue(get_class($this->objectInstance) == $defaultClassNameToUse || is_subclass_of($this->objectInstance, $defaultClassNameToUse));
+        $GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS']['saltedpasswords']['BE']['saltedPWHashingMethod'] = Argon2iSalt::class;
+        $GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['ext/saltedpasswords']['saltMethods'] = [ \stdClass::class ];
+        $argonProphecy = $this->prophesize(Argon2iSalt::class);
+        GeneralUtility::addInstance(Argon2iSalt::class, $argonProphecy->reveal());
+        $argonProphecy->isAvailable()->shouldBeCalled()->willReturn(false);
+        $this->expectException(InvalidSaltException::class);
+        $this->expectExceptionCode(1533822084);
+        (new SaltFactory())->getDefaultHashInstance('BE');
     }
 }
diff --git a/typo3/sysext/saltedpasswords/Tests/UnitDeprecated/Salt/SaltFactoryTest.php b/typo3/sysext/saltedpasswords/Tests/UnitDeprecated/Salt/SaltFactoryTest.php
new file mode 100644 (file)
index 0000000..69328d1
--- /dev/null
@@ -0,0 +1,148 @@
+<?php
+declare(strict_types = 1);
+namespace TYPO3\CMS\Saltedpasswords\Tests\UnitDeprecated\Salt;
+
+/*
+ * 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\Crypto\Random;
+use TYPO3\CMS\Saltedpasswords\Salt\SaltFactory;
+use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
+
+/**
+ * Test case
+ */
+class SaltFactoryTest extends UnitTestCase
+{
+    /**
+     * @test
+     */
+    public function abstractComposedSaltBase64EncodeReturnsProperLength()
+    {
+        $GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS']['saltedpasswords'] = [
+            'BE' => [
+                'saltedPWHashingMethod' => \TYPO3\CMS\Saltedpasswords\Salt\Pbkdf2Salt::class,
+            ],
+            'FE' => [
+                'saltedPWHashingMethod' => \TYPO3\CMS\Saltedpasswords\Salt\Pbkdf2Salt::class,
+            ],
+        ];
+
+        // set up an instance that extends AbstractComposedSalt first
+        $saltPbkdf2 = '$pbkdf2-sha256$6400$0ZrzXitFSGltTQnBWOsdAw$Y11AchqV4b0sUisdZd0Xr97KWoymNE0LNNrnEgY4H9M';
+        $objectInstance = SaltFactory::getSaltingInstance($saltPbkdf2);
+
+        // 3 Bytes should result in a 6 char length base64 encoded string
+        // used for MD5 and PHPass salted hashing
+        $byteLength = 3;
+        $reqLengthBase64 = (int)ceil($byteLength * 8 / 6);
+        $randomBytes = (new Random())->generateRandomBytes($byteLength);
+        $this->assertTrue(strlen($objectInstance->base64Encode($randomBytes, $byteLength)) == $reqLengthBase64);
+        // 16 Bytes should result in a 22 char length base64 encoded string
+        // used for Blowfish salted hashing
+        $byteLength = 16;
+        $reqLengthBase64 = (int)ceil($byteLength * 8 / 6);
+        $randomBytes = (new Random())->generateRandomBytes($byteLength);
+        $this->assertTrue(strlen($objectInstance->base64Encode($randomBytes, $byteLength)) == $reqLengthBase64);
+    }
+
+    /**
+     * @test
+     */
+    public function objectInstanceForMD5Salts()
+    {
+        $saltMD5 = '$1$rasmusle$rISCgZzpwk3UhDidwXvin0';
+        $objectInstance = SaltFactory::getSaltingInstance($saltMD5);
+        $this->assertTrue(get_class($objectInstance) == \TYPO3\CMS\Saltedpasswords\Salt\Md5Salt::class || is_subclass_of($objectInstance, \TYPO3\CMS\Saltedpasswords\Salt\Md5Salt::class));
+        $this->assertInstanceOf(\TYPO3\CMS\Saltedpasswords\Salt\AbstractComposedSalt::class, $objectInstance);
+    }
+
+    /**
+     * @test
+     */
+    public function objectInstanceForBlowfishSalts()
+    {
+        $saltBlowfish = '$2a$07$abcdefghijklmnopqrstuuIdQV69PAxWYTgmnoGpe0Sk47GNS/9ZW';
+        $objectInstance = SaltFactory::getSaltingInstance($saltBlowfish);
+        $this->assertTrue(get_class($objectInstance) == \TYPO3\CMS\Saltedpasswords\Salt\BlowfishSalt::class || is_subclass_of($objectInstance, \TYPO3\CMS\Saltedpasswords\Salt\BlowfishSalt::class));
+        $this->assertInstanceOf(\TYPO3\CMS\Saltedpasswords\Salt\AbstractComposedSalt::class, $objectInstance);
+    }
+
+    /**
+     * @test
+     */
+    public function objectInstanceForPhpassSalts()
+    {
+        $saltPhpass = '$P$CWF13LlG/0UcAQFUjnnS4LOqyRW43c.';
+        $objectInstance = SaltFactory::getSaltingInstance($saltPhpass);
+        $this->assertTrue(get_class($objectInstance) == \TYPO3\CMS\Saltedpasswords\Salt\PhpassSalt::class || is_subclass_of($objectInstance, \TYPO3\CMS\Saltedpasswords\Salt\PhpassSalt::class));
+        $this->assertInstanceOf(\TYPO3\CMS\Saltedpasswords\Salt\AbstractComposedSalt::class, $objectInstance);
+    }
+
+    /**
+     * @test
+     */
+    public function objectInstanceForPbkdf2Salts()
+    {
+        $saltPbkdf2 = '$pbkdf2-sha256$6400$0ZrzXitFSGltTQnBWOsdAw$Y11AchqV4b0sUisdZd0Xr97KWoymNE0LNNrnEgY4H9M';
+        $objectInstance = SaltFactory::getSaltingInstance($saltPbkdf2);
+        $this->assertTrue(get_class($objectInstance) == \TYPO3\CMS\Saltedpasswords\Salt\Pbkdf2Salt::class || is_subclass_of($objectInstance, \TYPO3\CMS\Saltedpasswords\Salt\Pbkdf2Salt::class));
+        $this->assertInstanceOf(\TYPO3\CMS\Saltedpasswords\Salt\AbstractComposedSalt::class, $objectInstance);
+    }
+
+    /**
+     * @test
+     */
+    public function objectInstanceForPhpPasswordHashBcryptSalts()
+    {
+        $saltBcrypt = '$2y$12$Tz.al0seuEgRt61u0bzqAOWu67PgG2ThG25oATJJ0oS5KLCPCgBOe';
+        $objectInstance = SaltFactory::getSaltingInstance($saltBcrypt);
+        $this->assertInstanceOf(\TYPO3\CMS\Saltedpasswords\Salt\BcryptSalt::class, $objectInstance);
+    }
+
+    /**
+     * @test
+     */
+    public function objectInstanceForPhpPasswordHashArgon2iSalts()
+    {
+        $saltArgon2i = '$argon2i$v=19$m=8,t=1,p=1$djZiNkdEa3lOZm1SSmZsdQ$9iiRjpLZAT7kfHwS1xU9cqSU7+nXy275qpB/eKjI1ig';
+        $objectInstance = SaltFactory::getSaltingInstance($saltArgon2i);
+        $this->assertInstanceOf(\TYPO3\CMS\Saltedpasswords\Salt\Argon2iSalt::class, $objectInstance);
+    }
+
+    /**
+     * @test
+     */
+    public function resettingFactoryInstanceSucceeds()
+    {
+        $GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS']['saltedpasswords'] = [
+            'BE' => [
+                'saltedPWHashingMethod' => \TYPO3\CMS\Saltedpasswords\Salt\Pbkdf2Salt::class,
+            ],
+            'FE' => [
+                'saltedPWHashingMethod' => \TYPO3\CMS\Saltedpasswords\Salt\Pbkdf2Salt::class,
+            ],
+        ];
+
+        $defaultClassNameToUse = \TYPO3\CMS\Saltedpasswords\Utility\SaltedPasswordsUtility::getDefaultSaltingHashingMethod();
+        if ($defaultClassNameToUse == \TYPO3\CMS\Saltedpasswords\Salt\Md5Salt::class) {
+            $saltedPW = '$P$CWF13LlG/0UcAQFUjnnS4LOqyRW43c.';
+        } else {
+            $saltedPW = '$1$rasmusle$rISCgZzpwk3UhDidwXvin0';
+        }
+        $objectInstance = SaltFactory::getSaltingInstance($saltedPW);
+        // resetting
+        $objectInstance = SaltFactory::getSaltingInstance(null);
+        $this->assertTrue(get_class($objectInstance) == $defaultClassNameToUse || is_subclass_of($objectInstance, $defaultClassNameToUse));
+    }
+}
index c0fc53f..c1c2d64 100644 (file)
@@ -36,6 +36,7 @@ use TYPO3\CMS\Core\Messaging\FlashMessageService;
 use TYPO3\CMS\Core\Resource\Exception\FileDoesNotExistException;
 use TYPO3\CMS\Core\Resource\ResourceFactory;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Saltedpasswords\Exception\InvalidSaltException;
 use TYPO3\CMS\Saltedpasswords\Salt\SaltFactory;
 
 /**
@@ -288,8 +289,15 @@ class SetupModuleController
                         $passwordOk = true;
                     } else {
                         $currentPasswordHashed = $GLOBALS['BE_USER']->user['password'];
-                        $saltFactory = SaltFactory::getSaltingInstance($currentPasswordHashed);
-                        $passwordOk = $saltFactory->checkPassword($be_user_data['passwordCurrent'], $currentPasswordHashed);
+                        $passwordOk = false;
+                        $saltFactory = GeneralUtility::makeInstance(SaltFactory::class);
+                        try {
+                            $hashInstance = $saltFactory->get($currentPasswordHashed);
+                            $passwordOk = $hashInstance->checkPassword($be_user_data['passwordCurrent'], $currentPasswordHashed);
+                        } catch (InvalidSaltException $e) {
+                            // Could not find hash class responsible for existing password. This is a
+                            // misconfiguration and user can not change its password.
+                        }
                     }
                     if ($passwordOk) {
                         $this->passwordIsUpdated = self::PASSWORD_UPDATED;