improve password handling (#4135)

* update password handling to Symfony. fixes #2842
* raise default minimum length of passwords to 8 characters. allow setting as low as 5 by admin.
* optionally add NonCompromisedPassword validator
* add new password strength meter
* add batch force password change for various groups
* add simple password generator

CHANGELOG-3.0.md

@@ -76,7 +76,9 @@
     - The corresponding Twig function is similarly renamed.
   - `Zikula\ExtensionsModule\Event\ModuleStateEvent` is renamed to `Zikula\ExtensionsModule\Event\ExtensionStateEvent`.
     - Its methods also renamed: `getModule` -> `getExtension` and `getModInfo` -> `getInfo`.
-  - All the Events in `Zikula\ExtensionsModule\ExtensionEvents` are changed - both the name and the ConstantName. 
+  - All the Events in `Zikula\ExtensionsModule\ExtensionEvents` are changed - both the name and the ConstantName.
+  - `Zikula\ZAuthModule\Api\PasswordApi` & `Zikula\ZAuthModule\Api\ApiInterface\PasswordApiInterface` are deprecated and will be removed in Core-4.0.0
+    - Use `Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface` or `bin2hex(random_bytes(8))`
   - Dropped vendors:
     - Removed afarkas/html5shiv
     - Removed afarkas/webshim (#3925)
@@ -164,6 +166,7 @@
   - Refactored page title handling, introducing a new `\Zikula\Bundle\CoreBundle\Site\SiteDefinitionInterface` (#3969).
   - Fixed creating new ZAuth users as admin without setting a password.
   - Start page controllers now get properly set the `_route` request argument (#3955).
+  - Default minimum length for passwords is now raised to 8. Absolute minimum length is still 5 (#2842).
 
 - Features:
   - Utilise autowiring and autoconfiguring functionality from Symfony (#3940).
@@ -198,6 +201,11 @@
   - Start page can now be defined much easier (a dropdown allows to choose a route/controller combination) (#3955).
   - Start page arguments can now be defined more flexible (GET parameters and request attributes) (#3955).
   - Start page can now be configured for each available language (#3955).
+  - Passwords in the ZAuth module are now always hashed with the the most up-to-date algorithm available (via Symfony security component) and automatically updated on login (#2842).
+  - Passwords can optionally be validated with Symfony's NonCompromisedPassword validator (see https://symfony.com/doc/current/reference/constraints/NotCompromisedPassword.html) (#2842).
+  - A new password strength meter is implemented. See https://github.com/ablanco/jquery.pwstrength.bootstrap (#2842).
+  - Added a simple password generator in all places where a new password might be needed (#2842).
+  - Added ability to force a group of users to change their password on next login (#2842).
   - Extensions module automatically contributes admin menu item to display Markdown docs for other extensions. Help UI can be configured to use either a modal window or a fixed sidebar (#3739).
 
 - Vendor updates:

config/packages/security.yaml

@@ -1,6 +1,8 @@
 security:
     encoders:
         Symfony\Component\Security\Core\User\User: plaintext
+        Zikula\ZAuthModule\Entity\AuthenticationMappingEntity:
+            algorithm: auto
 
     role_hierarchy:
         ROLE_ADMIN:       ROLE_USER

src/Zikula/CoreInstallerBundle/Helper/StageHelper.php

@@ -138,8 +138,8 @@ public function executeStage(string $stageName): bool
                 return $this->coreInstallerExtensionHelper->install('ZikulaRoutesModule');
             case 'menu':
                 return $this->coreInstallerExtensionHelper->install('ZikulaMenuModule');
-            case 'updateadmin':
-                return $this->superUserHelper->updateAdmin();
+            case 'createadmin':
+                return $this->superUserHelper->createAdmin();
             case 'loginadmin':
                 return $this->superUserHelper->loginAdmin();
             case 'activateextensions':

src/Zikula/CoreInstallerBundle/Helper/SuperUserHelper.php

@@ -16,10 +16,10 @@
 use DateTime;
 use Doctrine\ORM\EntityManagerInterface;
 use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface;
 use Zikula\UsersModule\Entity\RepositoryInterface\UserRepositoryInterface;
 use Zikula\UsersModule\Entity\UserEntity;
 use Zikula\UsersModule\Helper\AccessHelper;
-use Zikula\ZAuthModule\Api\ApiInterface\PasswordApiInterface;
 use Zikula\ZAuthModule\Entity\AuthenticationMappingEntity;
 use Zikula\ZAuthModule\ZAuthConstant;
 
@@ -51,30 +51,30 @@ class SuperUserHelper
     private $accessHelper;
 
     /**
-     * @var PasswordApiInterface
+     * @var EncoderFactoryInterface
      */
-    private $passwordApi;
+    private $encoderFactory;
 
     public function __construct(
         UserRepositoryInterface $userRepository,
         EntityManagerInterface $entityManager,
         ParameterHelper $parameterHelper,
         RequestStack $requestStack,
         AccessHelper $accessHelper,
-        PasswordApiInterface $passwordApi
+        EncoderFactoryInterface $encoderFactory
     ) {
         $this->userRepository = $userRepository;
         $this->entityManager = $entityManager;
         $this->parameterHelper = $parameterHelper;
         $this->requestStack = $requestStack;
         $this->accessHelper = $accessHelper;
-        $this->passwordApi = $passwordApi;
+        $this->encoderFactory = $encoderFactory;
     }
 
     /**
      * This inserts the admin's user data
      */
-    public function updateAdmin(): bool
+    public function createAdmin(): bool
     {
         $params = $this->parameterHelper->decodeParameters($this->parameterHelper->getYamlHelper()->getParameters());
         /** @var UserEntity $userEntity */
@@ -91,7 +91,7 @@ public function updateAdmin(): bool
         $mapping->setUname($userEntity->getUname());
         $mapping->setEmail($userEntity->getEmail());
         $mapping->setVerifiedEmail(true);
-        $mapping->setPass($this->passwordApi->getHashedPassword($params['password']));
+        $mapping->setPass($this->encoderFactory->getEncoder($mapping)->encodePassword($params['password'], null));
         $mapping->setMethod(ZAuthConstant::AUTHENTICATION_METHOD_UNAME);
         $this->entityManager->persist($mapping);
 

src/Zikula/CoreInstallerBundle/Stage/Install/AjaxInstallerStage.php

@@ -188,7 +188,7 @@ public function getTemplateParams(): array
                 AjaxStageInterface::FAIL => $this->trans('There was an error creating default blocks')
             ],
             21 => [
-                AjaxStageInterface::NAME => 'updateadmin',
+                AjaxStageInterface::NAME => 'createadmin',
                 AjaxStageInterface::PRE => $this->trans('Create admin account'),
                 AjaxStageInterface::DURING => $this->trans('Creating admin account'),
                 AjaxStageInterface::SUCCESS => $this->trans('Admin account created'),

src/Zikula/CoreInstallerBundle/Validator/Constraints/AuthenticateAdminLoginValidator.php

@@ -15,12 +15,14 @@
 
 use Doctrine\DBAL\Connection;
 use Exception;
+use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface;
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\ConstraintValidator;
 use Symfony\Contracts\Translation\TranslatorInterface;
 use Zikula\Bundle\CoreBundle\Translation\TranslatorTrait;
 use Zikula\PermissionsModule\Api\ApiInterface\PermissionApiInterface;
 use Zikula\ZAuthModule\Api\ApiInterface\PasswordApiInterface;
+use Zikula\ZAuthModule\Entity\AuthenticationMappingEntity;
 
 class AuthenticateAdminLoginValidator extends ConstraintValidator
 {
@@ -36,6 +38,11 @@ class AuthenticateAdminLoginValidator extends ConstraintValidator
      */
     private $databaseConnection;
 
+    /**
+     * @var EncoderFactoryInterface
+     */
+    private $encoderFactory;
+
     /**
      * @var PasswordApiInterface
      */
@@ -45,11 +52,13 @@ public function __construct(
         PermissionApiInterface $permissionApi,
         Connection $connection,
         TranslatorInterface $translator,
+        EncoderFactoryInterface $encoderFactory,
         PasswordApiInterface $passwordApi
     ) {
         $this->permissionApi = $permissionApi;
         $this->databaseConnection = $connection;
         $this->setTranslator($translator);
+        $this->encoderFactory = $encoderFactory;
         $this->passwordApi = $passwordApi;
     }
 
@@ -63,23 +72,52 @@ public function validate($object, Constraint $constraint)
             ', [$object['username']]);
         } catch (Exception $exception) {
             $this->context->buildViolation($this->trans('Error! There was a problem with the database connection.'))
-                ->addViolation()
-            ;
+                ->addViolation();
         }
 
-        if (empty($user) || $user['uid'] <= 1 || !$this->passwordApi->passwordsMatch($object['password'], $user['pass'])) {
+        $passwordEncoder = $this->encoderFactory->getEncoder(AuthenticationMappingEntity::class);
+
+        if (empty($user) || $user['uid'] <= 1) { // || !$passwordEncoder->isPasswordValid($user['pass'], $object['password'], null)) {
             $this->context
-                ->buildViolation($this->trans('Error! Could not login with provided credentials. Please try again.'))
-                ->addViolation()
-            ;
+                ->buildViolation($this->trans('Error! Could not login because the user could not be found. Please try again.'))
+                ->addViolation();
         } else {
+            $validPassword = false;
+            if ($this->passwordApi->passwordsMatch($object['password'], $user['pass'])) {
+                // old way - remove in Core-4.0.0
+                $validPassword = true;
+                // convert old encoding to new
+                $this->setPassword((int) $user['uid'], $object['password']);
+            } elseif (
+                // new way
+                $passwordEncoder->isPasswordValid($user['pass'], $object['password'], NULL)) {
+                $validPassword = true;
+                if ($passwordEncoder->needsRehash($user['pass'])) { // check to update hash to newer algo
+                    $this->setPassword((int) $user['uid'], $object['password']);
+                }
+            }
+            if (!$validPassword) {
+                $this->context
+                    ->buildViolation($this->trans('Error! Could not login with the provided credentials. Please try again.'))
+                    ->addViolation();
+            }
+
             $granted = $this->permissionApi->hasPermission('.*', '.*', ACCESS_ADMIN, (int) $user['uid']);
             if (!$granted) {
                 $this->context
                     ->buildViolation($this->trans('Error! You logged in to an account without Admin permissions'))
-                    ->addViolation()
-                ;
+                    ->addViolation();
             }
         }
     }
+
+    private function setPassword(int $uid, string $unHashedPassword)
+    {
+        $passwordEncoder = $this->encoderFactory->getEncoder(AuthenticationMappingEntity::class);
+        $hashedPassword = $passwordEncoder->encodePassword($unHashedPassword, null);
+        $this->databaseConnection->executeUpdate('UPDATE zauth_authentication_mapping SET pass = ? WHERE uid = ?', [
+            $hashedPassword,
+            $uid
+        ]);
+    }
 }

src/system/UsersModule/Entity/Repository/UserRepository.php

@@ -37,7 +37,12 @@ public function __construct(ManagerRegistry $registry)
 
     public function findByUids(array $userIds = []): array
     {
-        return $this->findBy(['uid' => $userIds]);
+        // using queryBuilder allows to index collection by the uid
+        $qb = $this->createQueryBuilder('u', 'u.uid')
+            ->where('u.uid IN (:uids)')
+            ->setParameter('uids', $userIds);
+
+        return $qb->getQuery()->getResult();
     }
 
     public function persistAndFlush(UserEntity $user): void

src/system/UsersModule/UsersModuleInstaller.php

@@ -293,7 +293,6 @@ private function migrateModVarsToZAuth(): void
     private function getMigratedModVarNames(): array
     {
         return [
-            ZAuthConstant::MODVAR_HASH_METHOD,
             ZAuthConstant::MODVAR_PASSWORD_MINIMUM_LENGTH,
             ZAuthConstant::MODVAR_PASSWORD_STRENGTH_METER_ENABLED, // convert to bool
             ZAuthConstant::MODVAR_REGISTRATION_ANTISPAM_QUESTION,

src/system/ZAuthModule/Api/ApiInterface/PasswordApiInterface.php

@@ -15,6 +15,9 @@
 
 use InvalidArgumentException;
 
+/**
+ * @deprecated at Core-3.0.0 to be removed Core-4.0.0
+ */
 interface PasswordApiInterface
 {
     public const SALT_DELIM = '$';

src/system/ZAuthModule/Api/PasswordApi.php

@@ -17,6 +17,12 @@
 use RandomLib\Factory as RandomLibFactory;
 use Zikula\ZAuthModule\Api\ApiInterface\PasswordApiInterface;
 
+/**
+ * Class PasswordApi
+ * @deprecated at Core-3.0.0 to be removed in Core-4.0.0
+ * Use \Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface
+ * Use `bin2hex(random_bytes(8))` for random string generation suitable for passwords
+ */
 class PasswordApi implements PasswordApiInterface
 {
     /**
@@ -48,6 +54,11 @@ class PasswordApi implements PasswordApiInterface
      */
     private $randomStringCharacters = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ~@#$%^*()_+-={}|][';
 
+    /**
+     * @internal
+     * Do not use this method publicly.
+     * It only remains to allow testing of `passwordsMatch()`
+     */
     public function getHashedPassword(
         string $unhashedPassword,
         int $hashMethodCode = self::DEFAULT_HASH_METHOD_CODE
@@ -58,16 +69,12 @@ public function getHashedPassword(
         return $this->getSaltedHash($unhashedPassword, $hashAlgorithmName, $this->methods);
     }
 
+    /**
+     * This function no longer is used or useful
+     */
     public function generatePassword(int $length = self::MIN_LENGTH): string
     {
-        if ($length < self::MIN_LENGTH) {
-            $length = self::MIN_LENGTH;
-        }
-        $factory = new RandomLibFactory();
-        $generator = $factory->getMediumStrengthGenerator();
-        $chars = str_replace($this->passwordIncompatibleCharacters, '', $this->randomStringCharacters);
-
-        return $generator->generateString($length, $chars);
+        return '';
     }
 
     public function passwordsMatch(string $unhashedPassword, string $hashedPassword): bool
@@ -76,7 +83,10 @@ public function passwordsMatch(string $unhashedPassword, string $hashedPassword)
             || false === mb_strpos($hashedPassword, self::SALT_DELIM)
             || 2 !== mb_substr_count($hashedPassword, self::SALT_DELIM)
         ) {
-            throw new InvalidArgumentException();
+            // passwords encoded with the new Symfony encoder (since Core3) will fail here.
+            // returning false here allows it to pass-through to the next password validation check
+            // see \Zikula\ZAuthModule\AuthenticationMethod\AbstractNativeAuthenticationMethod::authenticateByField
+            return false;
         }
 
         return $this->checkSaltedHash($unhashedPassword, $hashedPassword);
@@ -168,7 +178,7 @@ protected function checkSaltedHash(
         $hashMethodCodeToName = array_flip($this->methods);
 
         if (1 === mb_strlen($saltDelimiter) && false !== mb_strpos($saltedHash, $saltDelimiter)) {
-            list($hashMethod, $saltStr, $correctHash) = explode($saltDelimiter, $saltedHash);
+            [$hashMethod, $saltStr, $correctHash] = explode($saltDelimiter, $saltedHash);
 
             if (is_numeric($hashMethod) && ((int)$hashMethod === $hashMethod)) {
                 $hashMethod = (int)$hashMethod;

src/system/ZAuthModule/AuthenticationMethod/AbstractNativeAuthenticationMethod.php

@@ -15,6 +15,7 @@
 
 use Exception;
 use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface;
 use Symfony\Component\Validator\Validator\ValidatorInterface;
 use Symfony\Contracts\Translation\TranslatorInterface;
 use Zikula\ExtensionsModule\Api\ApiInterface\VariableApiInterface;
@@ -57,20 +58,27 @@ abstract class AbstractNativeAuthenticationMethod implements NonReEntrantAuthent
      */
     private $passwordApi;
 
+    /**
+     * @var EncoderFactoryInterface
+     */
+    private $encoderFactory;
+
     public function __construct(
         AuthenticationMappingRepositoryInterface $mappingRepository,
         RequestStack $requestStack,
         TranslatorInterface $translator,
         VariableApiInterface $variableApi,
         ValidatorInterface $validator,
-        PasswordApiInterface $passwordApi
+        PasswordApiInterface $passwordApi,
+        EncoderFactoryInterface $encoderFactory
     ) {
         $this->mappingRepository = $mappingRepository;
         $this->requestStack = $requestStack;
         $this->translator = $translator;
         $this->variableApi = $variableApi;
         $this->validator = $validator;
         $this->passwordApi = $passwordApi;
+        $this->encoderFactory = $encoderFactory;
     }
 
     public function getRegistrationFormClassName(): string
@@ -93,9 +101,20 @@ protected function authenticateByField(array $data, string $field = 'uname'): ?i
         if (!$mapping->getPass()) {
             return null;
         }
+        $passwordEncoder = $this->encoderFactory->getEncoder($mapping);
 
         if ($mapping && $this->passwordApi->passwordsMatch($data['pass'], $mapping->getPass())) {
-            // is this the place to update the hash method? #2842
+            // old way - remove in Core-4.0.0
+            // convert old encoding to new
+            $this->updatePassword($mapping, $data['pass']);
+
+            return $mapping->getUid();
+        } elseif ($mapping && $passwordEncoder->isPasswordValid($mapping->getPass(), $data['pass'], null)) {
+            // new way
+            if ($passwordEncoder->needsRehash($mapping->getPass())) { // check to update hash to newer algo
+                $this->updatePassword($mapping, $data['pass']);
+            }
+
             return $mapping->getUid();
         }
 
@@ -107,6 +126,12 @@ protected function authenticateByField(array $data, string $field = 'uname'): ?i
         return null;
     }
 
+    private function updatePassword(AuthenticationMappingEntity $mapping, string $unHashedPassword)
+    {
+        $mapping->setPass($this->encoderFactory->getEncoder($mapping)->encodePassword($unHashedPassword, null));
+        $this->mappingRepository->persistAndFlush($mapping);
+    }
+
     /**
      * Get a AuthenticationMappingEntity if it exists. If not, check for existing UserEntity and
      * migrate data from UserEntity to AuthenticationMappingEntity and return that.
@@ -152,7 +177,7 @@ public function register(array $data = []): bool
         if (empty($data['pass'])) {
             $mapping->setPass('');
         } else {
-            $mapping->setPass($this->passwordApi->getHashedPassword($data['pass']));
+            $mapping->setPass($this->encoderFactory->getEncoder($mapping)->encodePassword($data['pass'], null));
         }
 
         $mapping->setMethod($this->getAlias());