store client-secret as hash only

composer.json

@@ -21,11 +21,12 @@
         "league/oauth2-server": "^9.2",
         "nyholm/psr7": "^1.4",
         "psr/http-factory": "^1.0",
-        "symfony/deprecation-contracts": "^3",
+        "symfony/deprecation-contracts": "^3.0",
         "symfony/event-dispatcher": "^6.4|^7.0|^8.0",
         "symfony/filesystem": "^6.4|^7.0|^8.0",
         "symfony/framework-bundle": "^6.4|^7.0|^8.0",
         "symfony/psr-http-message-bridge": "^6.4|^7.0|^8.0",
+        "symfony/password-hasher": "^6.4|^7.0|^8.0",
         "symfony/security-bundle": "^6.4|^7.0|^8.0"
     },
     "require-dev": {

config/services.php

@@ -13,6 +13,7 @@
 use League\Bundle\OAuth2ServerBundle\Command\DeleteClientCommand;
 use League\Bundle\OAuth2ServerBundle\Command\GenerateKeyPairCommand;
 use League\Bundle\OAuth2ServerBundle\Command\ListClientsCommand;
+use League\Bundle\OAuth2ServerBundle\Command\RehashClientSecretsCommand;
 use League\Bundle\OAuth2ServerBundle\Command\UpdateClientCommand;
 use League\Bundle\OAuth2ServerBundle\Controller\AuthorizationController;
 use League\Bundle\OAuth2ServerBundle\Controller\TokenController;
@@ -59,6 +60,7 @@
 use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\PasswordHasher\Hasher\NativePasswordHasher;
 
 return static function (ContainerConfigurator $container): void {
     $container->services()
@@ -67,6 +69,7 @@
         ->set('league.oauth2_server.repository.client', ClientRepository::class)
             ->args([
                 service(ClientManagerInterface::class),
+                service('league.oauth2_server.password_hasher'),
             ])
         ->alias(ClientRepositoryInterface::class, 'league.oauth2_server.repository.client')
         ->alias(ClientRepository::class, 'league.oauth2_server.repository.client')
@@ -233,6 +236,7 @@
             ->args([
                 service(ClientManagerInterface::class),
                 null,
+                service('league.oauth2_server.password_hasher'),
             ])
             ->tag('console.command', ['command' => 'league:oauth2-server:create-client'])
         ->alias(CreateClientCommand::class, 'league.oauth2_server.command.create_client')
@@ -277,6 +281,14 @@
             ->tag('console.command', ['command' => 'league:oauth2-server:generate-keypair'])
         ->alias(GenerateKeyPairCommand::class, 'league.oauth2_server.command.generate_keypair')
 
+        ->set('league.oauth2_server.command.rehash_client_secrets', RehashClientSecretsCommand::class)
+            ->args([
+                service(ClientManagerInterface::class),
+                service('league.oauth2_server.password_hasher'),
+            ])
+            ->tag('console.command', ['command' => 'league:oauth2-server:rehash-client-secrets'])
+        ->alias(RehashClientSecretsCommand::class, 'league.oauth2_server.command.rehash_client_secrets')
+
         // Utility services
         ->set('league.oauth2_server.converter.user', UserConverter::class)
         ->alias(UserConverterInterface::class, 'league.oauth2_server.converter.user')
@@ -321,5 +333,8 @@
             ])
 
         ->set('league.oauth2_server.factory.http_foundation', HttpFoundationFactory::class)
+
+        // Password hasher for client secrets
+        ->set('league.oauth2_server.password_hasher', NativePasswordHasher::class)
     ;
 };

src/Command/CreateClientCommand.php

@@ -17,6 +17,7 @@
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Style\SymfonyStyle;
+use Symfony\Component\PasswordHasher\PasswordHasherInterface;
 
 #[AsCommand(name: 'league:oauth2-server:create-client', description: 'Creates a new OAuth2 client')]
 final class CreateClientCommand extends Command
@@ -31,12 +32,19 @@ final class CreateClientCommand extends Command
      */
     private $clientFqcn;
 
-    public function __construct(ClientManagerInterface $clientManager, string $clientFqcn)
+    private ?PasswordHasherInterface $passwordHasher = null;
+
+    public function __construct(ClientManagerInterface $clientManager, string $clientFqcn, ?PasswordHasherInterface $passwordHasher = null)
     {
         parent::__construct();
 
         $this->clientManager = $clientManager;
         $this->clientFqcn = $clientFqcn;
+        $this->passwordHasher = $passwordHasher;
+
+        if (null === $this->passwordHasher) {
+            trigger_deprecation('league/oauth2-server-bundle', '1.2', 'Not passing a "%s" to "%s" is deprecated since version 1.2 and will be required in 2.0.', PasswordHasherInterface::class, __CLASS__);
+        }
     }
 
     protected function configure(): void
@@ -99,7 +107,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $io = new SymfonyStyle($input, $output);
 
         try {
-            $client = $this->buildClientFromInput($input);
+            $plainSecret = $this->resolvePlainSecret($input);
+            $client = $this->buildClientFromInput($input, $plainSecret);
         } catch (\InvalidArgumentException $exception) {
             $io->error($exception->getMessage());
 
@@ -111,27 +120,36 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 
         $headers = ['Identifier', 'Secret'];
         $rows = [
-            [$client->getIdentifier(), $client->getSecret()],
+            [$client->getIdentifier(), $plainSecret],
         ];
         $io->table($headers, $rows);
 
         return 0;
     }
 
-    private function buildClientFromInput(InputInterface $input): ClientInterface
+    private function resolvePlainSecret(InputInterface $input): ?string
     {
-        $name = $input->getArgument('name');
-        $identifier = (string) $input->getArgument('identifier') ?: hash('md5', random_bytes(16));
         $isPublic = $input->getOption('public');
 
         if ($isPublic && null !== $input->getArgument('secret')) {
             throw new \InvalidArgumentException('The client cannot have a secret and be public.');
         }
 
-        $secret = $isPublic ? null : $input->getArgument('secret') ?? hash('sha512', random_bytes(32));
+        return $isPublic ? null : $input->getArgument('secret') ?? bin2hex(random_bytes(32));
+    }
+
+    private function buildClientFromInput(InputInterface $input, ?string $plainSecret): ClientInterface
+    {
+        $name = $input->getArgument('name');
+        $identifier = (string) $input->getArgument('identifier') ?: hash('md5', random_bytes(16));
+
+        $hashedSecret = $plainSecret;
+        if ($this->passwordHasher && \is_string($plainSecret)) {
+            $hashedSecret = $this->passwordHasher->hash($plainSecret);
+        }
 
         /** @var AbstractClient $client */
-        $client = new $this->clientFqcn($name, $identifier, $secret);
+        $client = new $this->clientFqcn($name, $identifier, $hashedSecret);
         $client->setActive(true);
         $client->setAllowPlainTextPkce($input->getOption('allow-plain-text-pkce'));
 

src/Command/ListClientsCommand.php

@@ -66,6 +66,12 @@ protected function configure(): void
                 'Finds by allowed scope for client. Use this option multiple times to find by multiple scopes.',
                 []
             )
+            ->addOption(
+                'reveal-secret',
+                null,
+                InputOption::VALUE_NONE,
+                'Reveal the client secret in the output.'
+            )
         ;
     }
 
@@ -107,7 +113,7 @@ private function drawTable(InputInterface $input, OutputInterface $output, array
     {
         $io = new SymfonyStyle($input, $output);
         $columns = $this->getColumns($input);
-        $rows = $this->getRows($clients, $columns);
+        $rows = $this->getRows($clients, $columns, $input->getOption('reveal-secret'));
         $io->table($columns, $rows);
     }
 
@@ -117,13 +123,13 @@ private function drawTable(InputInterface $input, OutputInterface $output, array
      *
      * @return array<array<string>>
      */
-    private function getRows(array $clients, array $columns): array
+    private function getRows(array $clients, array $columns, bool $revealSecret): array
     {
-        return array_map(static function (ClientInterface $client) use ($columns): array {
+        return array_map(static function (ClientInterface $client) use ($columns, $revealSecret): array {
             $values = [
                 'name' => $client->getName(),
                 'identifier' => $client->getIdentifier(),
-                'secret' => $client->getSecret(),
+                'secret' => $revealSecret ? $client->getSecret() : ($client->isConfidential() ? '****' : '(public)'),
                 'scope' => implode(', ', $client->getScopes()),
                 'redirect uri' => implode(', ', $client->getRedirectUris()),
                 'grant type' => implode(', ', $client->getGrants()),

src/Command/RehashClientSecretsCommand.php

@@ -0,0 +1,61 @@
+<?php
+
+declare(strict_types=1);
+
+namespace League\Bundle\OAuth2ServerBundle\Command;
+
+use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface;
+use Symfony\Component\Console\Attribute\AsCommand;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+use Symfony\Component\Console\Style\SymfonyStyle;
+use Symfony\Component\PasswordHasher\PasswordHasherInterface;
+
+#[AsCommand(name: 'league:oauth2-server:rehash-client-secrets', description: 'Rehashes existing client secrets using the configured password hasher.')]
+final class RehashClientSecretsCommand extends Command
+{
+    private ClientManagerInterface $clientManager;
+    private PasswordHasherInterface $hasher;
+
+    public function __construct(ClientManagerInterface $clientManager, PasswordHasherInterface $hasher)
+    {
+        parent::__construct();
+
+        $this->clientManager = $clientManager;
+        $this->hasher = $hasher;
+    }
+
+    protected function execute(InputInterface $input, OutputInterface $output): int
+    {
+        $io = new SymfonyStyle($input, $output);
+        $migrated = $alreadyHashed = $public = 0;
+
+        foreach ($this->clientManager->list(null) as $client) {
+            if (!$client->isConfidential()) {
+                ++$public;
+                continue;
+            }
+
+            $secret = $client->getSecret() ?? '';
+
+            if (!$this->hasher->needsRehash($secret)) {
+                ++$alreadyHashed;
+                continue;
+            }
+
+            $client->setSecret($this->hasher->hash($secret));
+            $this->clientManager->save($client);
+            ++$migrated;
+        }
+
+        $io->success(\sprintf(
+            'Migration complete: %d secret(s) rehashed, %d already hashed, %d public client(s) skipped.',
+            $migrated,
+            $alreadyHashed,
+            $public,
+        ));
+
+        return Command::SUCCESS;
+    }
+}

src/DependencyInjection/Configuration.php

@@ -277,6 +277,10 @@ private function createClientNode(): NodeDefinition
                         ->thenInvalid(\sprintf('%%s must be a %s', AbstractClient::class))
                     ->end()
                 ->end()
+                ->booleanNode('allow_plaintext_secrets')
+                    ->info('Whether to allow plaintext client secrets.')
+                    ->defaultTrue()
+                ->end()
             ->end()
         ;
 

src/DependencyInjection/LeagueOAuth2ServerExtension.php

@@ -45,6 +45,8 @@
 use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface;
 use Symfony\Component\DependencyInjection\Loader\PhpFileLoader;
 use Symfony\Component\DependencyInjection\Reference;
+use Symfony\Component\PasswordHasher\Hasher\MigratingPasswordHasher;
+use Symfony\Component\PasswordHasher\Hasher\PlaintextPasswordHasher;
 
 final class LeagueOAuth2ServerExtension extends Extension implements PrependExtensionInterface, CompilerPassInterface
 {
@@ -75,6 +77,18 @@ public function load(array $configs, ContainerBuilder $container): void
             ->replaceArgument(1, $config['client']['classname'])
         ;
 
+        if ($config['client']['allow_plaintext_secrets']) {
+            trigger_deprecation('league/oauth2-server-bundle', '1.2', 'Setting "client.allow_plaintext_secrets" config option to "true" is deprecated. Use the `league:oauth2-server:rehash-client-secrets` command to rehash existing client secrets and set this option to "false" afterwards.');
+            $container->register('league.oauth2_server.password_hasher.plaintext', PlaintextPasswordHasher::class);
+            $container->register('league.oauth2_server.password_hasher.migrating', MigratingPasswordHasher::class)
+                ->setDecoratedService('league.oauth2_server.password_hasher')
+                ->setArguments([
+                    new Reference('league.oauth2_server.password_hasher.migrating.inner'),
+                    new Reference('league.oauth2_server.password_hasher.plaintext'),
+                ])
+            ;
+        }
+
         $container
             ->findDefinition(GenerateKeyPairCommand::class)
             ->replaceArgument(1, $config['authorization_server']['private_key'])

src/Model/AbstractClient.php

@@ -62,6 +62,11 @@ public function getSecret(): ?string
         return $this->secret;
     }
 
+    public function setSecret(string $secret): void
+    {
+        $this->secret = $secret;
+    }
+
     /**
      * @return list<RedirectUri>
      */

src/Model/ClientInterface.php

@@ -10,6 +10,7 @@
 
 /**
  * @method string getName()
+ * @method void setSecret(string $secret)
  */
 interface ClientInterface
 {
@@ -20,6 +21,8 @@ public function getIdentifier(): string;
 
     public function getSecret(): ?string;
 
+    // public function setSecret(string $secret): void;
+
     /**
      * @return list<RedirectUri>
      */

src/Persistence/Mapping/Driver.php

@@ -97,7 +97,7 @@ private function buildAbstractClientMetadata(ORMClassMetadata $metadata): void
         (new ClassMetadataBuilder($metadata))
             ->setMappedSuperClass()
             ->createField('name', 'string')->length(128)->build()
-            ->createField('secret', 'string')->length(128)->nullable(true)->build()
+            ->createField('secret', 'string')->length(255)->nullable(true)->build()
             ->createField('redirectUris', 'oauth2_redirect_uri')->nullable(true)->build()
             ->createField('grants', 'oauth2_grant')->nullable(true)->build()
             ->createField('scopes', 'oauth2_scope')->nullable(true)->build()

src/Repository/ClientRepository.php

@@ -9,6 +9,7 @@
 use League\Bundle\OAuth2ServerBundle\Model\ClientInterface;
 use League\OAuth2\Server\Entities\ClientEntityInterface;
 use League\OAuth2\Server\Repositories\ClientRepositoryInterface;
+use Symfony\Component\PasswordHasher\PasswordHasherInterface;
 
 final class ClientRepository implements ClientRepositoryInterface
 {
@@ -17,9 +18,16 @@ final class ClientRepository implements ClientRepositoryInterface
      */
     private $clientManager;
 
-    public function __construct(ClientManagerInterface $clientManager)
+    private ?PasswordHasherInterface $passwordHasher;
+
+    public function __construct(ClientManagerInterface $clientManager, ?PasswordHasherInterface $passwordHasher = null)
     {
         $this->clientManager = $clientManager;
+        $this->passwordHasher = $passwordHasher;
+
+        if (null === $this->passwordHasher) {
+            trigger_deprecation('league/oauth2-server-bundle', '1.2', 'Not passing a "%s" to "%s" is deprecated since version 1.2 and will be required in 2.0.', PasswordHasherInterface::class, __CLASS__);
+        }
     }
 
     public function getClientEntity(string $clientIdentifier): ?ClientEntityInterface
@@ -33,7 +41,7 @@ public function getClientEntity(string $clientIdentifier): ?ClientEntityInterfac
         return $this->buildClientEntity($client);
     }
 
-    public function validateClient(string $clientIdentifier, ?string $clientSecret, ?string $grantType): bool
+    public function validateClient(string $clientIdentifier, #[\SensitiveParameter] ?string $clientSecret, ?string $grantType): bool
     {
         $client = $this->clientManager->find($clientIdentifier);
 
@@ -49,11 +57,29 @@ public function validateClient(string $clientIdentifier, ?string $clientSecret,
             return false;
         }
 
-        if (!$client->isConfidential() || hash_equals((string) $client->getSecret(), (string) $clientSecret)) {
+        if (!$client->isConfidential()) {
             return true;
         }
 
-        return false;
+        $storedSecret = (string) $client->getSecret();
+        $inputSecret = (string) $clientSecret;
+
+        if (null === $this->passwordHasher) {
+            return hash_equals($storedSecret, $inputSecret);
+        }
+
+        $secretIsValid = $this->passwordHasher->verify($storedSecret, $inputSecret);
+
+        if ($secretIsValid && $this->passwordHasher->needsRehash($storedSecret)) {
+            if (!method_exists($client, 'setSecret')) {
+                trigger_deprecation('league/oauth2-server-bundle', '1.2', 'Not implementing method "setSecret()" in client "%s" is deprecated. This method will be required in 2.0.', $client::class);
+            } else {
+                $client->setSecret($this->passwordHasher->hash($inputSecret));
+                $this->clientManager->save($client);
+            }
+        }
+
+        return $secretIsValid;
     }
 
     private function buildClientEntity(ClientInterface $client): ClientEntity