store client-secret as hash only

UPGRADE.md

@@ -8,6 +8,7 @@ FROM 1.x to 2.0
  * Change config option `authorization_server.enable_password_grant` default value to `false`
  * Change config option `authorization_server.enable_implicit_grant` default value to `false`
  * Add `EventDispatcherInterface` argument to `AccessTokenRepository::__construct()`
+ * The `client.allow_plaintext_secrets` option value is now ignored and plaintext client secrets are no longer supported
 
 
 FROM 1.1 to 1.2
@@ -17,3 +18,5 @@ FROM 1.1 to 1.2
  * Deprecate not setting the `authorization_server.enable_password_grant` config option. It will default to `false` in 2.0
  * Deprecate not setting the `authorization_server.enable_implicit_grant` config option. It will default to `false` in 2.0
  * Deprecate not passing an `EventDispatcherInterface` instance to `AccessTokenRepository::__construct()`
+ * Client secrets are now stored hashed. The client `secret` column length was increased from 128 to 255 to fit the hashed value; with the Doctrine persistence, generate and run a migration to apply this schema change before rehashing your client secrets
+ * Add option `client.allow_plaintext_secrets` (default `true`) controlling whether plaintext client secrets are allowed. Setting it to `true` is deprecated: existing plaintext secrets keep working and are rehashed transparently on first successful authentication, but you should run the `league:oauth2-server:rehash-client-secrets` command to rehash them all and then set this option to `false`. The option value will be ignored in 2.0

composer.json

@@ -25,6 +25,7 @@
         "symfony/event-dispatcher": "^6.4|^7.4|^8.0",
         "symfony/filesystem": "^6.4|^7.4|^8.0",
         "symfony/framework-bundle": "^6.4|^7.4|^8.0",
+        "symfony/password-hasher": "^6.4|^7.4|^8.0",
         "symfony/psr-http-message-bridge": "^6.4|^7.4|^8.0",
         "symfony/security-bundle": "^6.4|^7.4|^8.0"
     },

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\DeviceCodeController;
@@ -64,6 +65,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()
@@ -72,6 +74,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')
@@ -268,6 +271,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')
@@ -313,6 +317,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')
@@ -357,5 +369,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, #[\SensitiveParameter] ?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 stored client secret in the output. For clients whose secret is hashed, this is the hash, not the original secret.'
+            )
         ;
     }
 
@@ -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,9 +123,9 @@ 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 {
             if (!method_exists($client, 'getName')) {
                 trigger_deprecation('league/oauth2-server-bundle', '1.2', 'Not implementing method "getName()" in client "%s" is deprecated. This method will be required in 2.0.', $client::class);
                 $name = $client->getIdentifier();
@@ -129,7 +135,7 @@ private function getRows(array $clients, array $columns): array
             $values = [
                 'name' => $name,
                 '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

@@ -321,6 +321,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

@@ -48,6 +48,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
 {
@@ -78,6 +80,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
 {
@@ -22,6 +23,8 @@ public function getIdentifier(): string;
 
     public function getSecret(): ?string;
 
+    // public function setSecret(string $secret): void;
+
     /**
      * @return list<RedirectUri>
      */

src/Persistence/Mapping/Driver.php

@@ -103,7 +103,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()