feat: Use roadiz/random interfaces and declare them as services

roadiz · Dec 2, 2024 · 4a9b073 · 4a9b073
1 parent 70dab29
commit 4a9b073
Show file tree

Hide file tree

Showing 11 changed files with 36 additions and 59 deletions.
diff --git a/lib/OpenId/src/OAuth2LinkGenerator.php b/lib/OpenId/src/OAuth2LinkGenerator.php
@@ -5,7 +5,7 @@
 namespace RZ\Roadiz\OpenId;
 
 use RZ\Roadiz\OpenId\Exception\DiscoveryNotAvailableException;
-use RZ\Roadiz\Random\TokenGenerator;
+use RZ\Roadiz\Random\TokenGeneratorInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
 
@@ -17,6 +17,7 @@ class OAuth2LinkGenerator
     public function __construct(
         protected readonly ?Discovery $discovery,
         protected readonly CsrfTokenManagerInterface $csrfTokenManager,
+        protected readonly TokenGeneratorInterface $tokenGenerator,
         protected readonly ?string $openIdHostedDomain,
         protected readonly ?string $oauthClientId,
         ?array $openIdScopes,
@@ -71,7 +72,7 @@ public function generate(
             'state' => http_build_query(array_merge($state, [
                 'token' => $stateToken->getValue(),
             ])),
-            'nonce' => (new TokenGenerator())->generateToken(),
+            'nonce' => $this->tokenGenerator->generateToken(),
             'login_hint' => $request->get('email', null),
             'scope' => implode(' ', $customScopes),
             'client_id' => $this->oauthClientId,

diff --git a/lib/Random/src/PasswordGenerator.php b/lib/Random/src/PasswordGenerator.php
@@ -11,20 +11,18 @@ class PasswordGenerator extends RandomGenerator implements PasswordGeneratorInte
      * one uppercase letter, one digit, and one special character. The remaining characters
      * in the password are chosen at random from those four sets.
      *
-     * The available characters in each set are user friendly - there are no ambiguous
+     * The available characters in each set are user-friendly - there are no ambiguous
      * characters such as i, l, 1, o, 0, etc.
      *
-     * @return string
-     *
      * @see https://gist.github.com/tylerhall/521810
      */
-    public function generatePassword(int $length = 12)
+    public function generatePassword(int $length = 16): string
     {
         $sets = [];
         $sets[] = 'abcdefghjkmnpqrstuvwxyz';
         $sets[] = 'ABCDEFGHJKMNPQRSTUVWXYZ';
         $sets[] = '23456789';
-        $sets[] = '!@#$%&*?';
+        $sets[] = '!@#$%&*?-';
 
         $all = '';
         $password = '';
@@ -38,8 +36,6 @@ public function generatePassword(int $length = 12)
             $password .= $all[array_rand($all)];
         }
 
-        $password = str_shuffle($password);
-
-        return $password;
+        return str_shuffle($password);
     }
 }
diff --git a/lib/Random/src/PasswordGeneratorInterface.php b/lib/Random/src/PasswordGeneratorInterface.php
@@ -6,8 +6,5 @@
 
 interface PasswordGeneratorInterface
 {
-    /**
-     * @return string
-     */
-    public function generatePassword(int $length = 12);
+    public function generatePassword(int $length = 16): string;
 }
diff --git a/lib/Random/src/RandomGenerator.php b/lib/Random/src/RandomGenerator.php
@@ -8,40 +8,22 @@
 
 class RandomGenerator
 {
-    protected ?LoggerInterface $logger;
-    protected bool $useOpenSsl;
-
-    public function __construct(?LoggerInterface $logger = null)
+    public function __construct(protected readonly LoggerInterface $logger)
     {
-        $this->logger = $logger;
-        // determine whether to use OpenSSL
-        if (defined('PHP_WINDOWS_VERSION_BUILD') && version_compare(PHP_VERSION, '5.3.4', '<')) {
-            $this->useOpenSsl = false;
-        } elseif (!function_exists('openssl_random_pseudo_bytes')) {
-            if (null !== $this->logger) {
-                $this->logger->notice('It is recommended that you enable the "openssl" extension for random number generation.');
-            }
-            $this->useOpenSsl = false;
-        } else {
-            $this->useOpenSsl = true;
+        if (!function_exists('openssl_random_pseudo_bytes')) {
+            throw new \RuntimeException('You must enable the "openssl" extension for secure random number generation.');
         }
     }
 
     public function getRandomNumber(int $nbBytes = 32): string
     {
         // try OpenSSL
-        if ($this->useOpenSsl) {
-            $bytes = \openssl_random_pseudo_bytes($nbBytes, $strong);
-
-            if (false !== $bytes && true === $strong) {
-                return $bytes;
-            }
+        $bytes = \openssl_random_pseudo_bytes($nbBytes, $strong);
 
-            if (null !== $this->logger) {
-                $this->logger->info('OpenSSL did not produce a secure random number.');
-            }
+        if (false !== $bytes && true === $strong) {
+            return $bytes;
         }
 
-        return hash('sha256', uniqid((string) mt_rand(), true), true);
+        throw new \RuntimeException('Unable to generate a secure random number.');
     }
 }
diff --git a/lib/Random/src/SaltGenerator.php b/lib/Random/src/SaltGenerator.php
@@ -6,10 +6,7 @@
 
 class SaltGenerator extends RandomGenerator implements SaltGeneratorInterface
 {
-    /**
-     * @return string
-     */
-    public function generateSalt()
+    public function generateSalt(): string
     {
         return strtr(base64_encode($this->getRandomNumber(24)), '{}', '-_');
     }

diff --git a/lib/Random/src/SaltGeneratorInterface.php b/lib/Random/src/SaltGeneratorInterface.php
@@ -6,8 +6,5 @@
 
 interface SaltGeneratorInterface
 {
-    /**
-     * @return string
-     */
-    public function generateSalt();
+    public function generateSalt(): string;
 }
diff --git a/lib/Random/src/TokenGenerator.php b/lib/Random/src/TokenGenerator.php
@@ -6,10 +6,7 @@
 
 class TokenGenerator extends RandomGenerator implements TokenGeneratorInterface
 {
-    /**
-     * @return string
-     */
-    public function generateToken()
+    public function generateToken(): string
     {
         return rtrim(strtr(base64_encode($this->getRandomNumber()), '+/', '-_'), '=');
     }

diff --git a/lib/Random/src/TokenGeneratorInterface.php b/lib/Random/src/TokenGeneratorInterface.php
@@ -6,8 +6,5 @@
 
 interface TokenGeneratorInterface
 {
-    /**
-     * @return string
-     */
-    public function generateToken();
+    public function generateToken(): string;
 }
diff --git a/lib/RoadizCoreBundle/config/services.yaml b/lib/RoadizCoreBundle/config/services.yaml
@@ -441,7 +441,12 @@ services:
         arguments: ['%kernel.project_dir%']
         public: true
 
-    RZ\Roadiz\Random\PasswordGenerator: ~
+    RZ\Roadiz\Random\PasswordGeneratorInterface:
+        class: RZ\Roadiz\Random\PasswordGenerator
+    RZ\Roadiz\Random\SaltGeneratorInterface:
+        class: RZ\Roadiz\Random\SaltGenerator
+    RZ\Roadiz\Random\TokenGeneratorInterface:
+        class: RZ\Roadiz\Random\TokenGenerator
 
     JMS\Serializer\Construction\ObjectConstructorInterface:
         alias: RZ\Roadiz\CoreBundle\Serializer\ObjectConstructor\ObjectConstructor

diff --git a/lib/RoadizCoreBundle/src/Console/UsersPasswordCommand.php b/lib/RoadizCoreBundle/src/Console/UsersPasswordCommand.php
@@ -6,7 +6,7 @@
 
 use Doctrine\Persistence\ManagerRegistry;
 use RZ\Roadiz\CoreBundle\Entity\User;
-use RZ\Roadiz\Random\PasswordGenerator;
+use RZ\Roadiz\Random\PasswordGeneratorInterface;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
@@ -16,7 +16,7 @@
 final class UsersPasswordCommand extends UsersCommand
 {
     public function __construct(
-        private readonly PasswordGenerator $passwordGenerator,
+        private readonly PasswordGeneratorInterface $passwordGenerator,
         ManagerRegistry $managerRegistry,
         ?string $name = null,
     ) {
@@ -31,6 +31,11 @@ protected function configure(): void
                 'username',
                 InputArgument::REQUIRED,
                 'Username'
+            )->addOption(
+                'length',
+                'l',
+                InputArgument::OPTIONAL,
+                default: 16,
             );
     }
 
@@ -49,7 +54,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int
                 $confirmation
             )
         ) {
-            $user->setPlainPassword($this->passwordGenerator->generatePassword(12));
+            $user->setPlainPassword($this->passwordGenerator->generatePassword(
+                (int) $input->getOption('length')
+            ));
             $this->managerRegistry->getManagerForClass(User::class)->flush();
             $io->success('A new password was regenerated for '.$name.': '.$user->getPlainPassword());
 

diff --git a/lib/RoadizRozierBundle/config/services.yaml b/lib/RoadizRozierBundle/config/services.yaml
@@ -91,6 +91,7 @@ services:
         arguments:
             - '@?roadiz_rozier.open_id.discovery'
             - '@security.csrf.token_manager'
+            - '@RZ\Roadiz\Random\TokenGeneratorInterface'
             - '%roadiz_rozier.open_id.hosted_domain%'
             - '%roadiz_rozier.open_id.oauth_client_id%'
             - '%roadiz_rozier.open_id.scopes%'