feature #20677 [DX][SecurityBundle] UserPasswordEncoderCommand: ask u…

…ser class choice question (ogizanagi)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[DX][SecurityBundle] UserPasswordEncoderCommand: ask user class choice question

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | N/A
| License       | MIT
| Doc PR        | N/A

Typing the user class is quite annoying so I'd suggest to ask the user to select it using a choice question.

This changes however requires to inject configured encoders' user classes. Making the command a service and providing it in the constructor from the extension is probably the cleanest way, but it deprecates:

-  registering the command by convention (registered as a service now, so potential commands extending this one should do the same)
-  relying on the fact the command extends `ContainerAwareCommand` and implements `ContainerAwareInterface` (will not extends/implements it anymore in 4.0 because it's not required anymore)

Commits
-------

366aefd [SecurityBundle] UserPasswordEncoderCommand: ask user class choice question

UPGRADE-3.3.md

@@ -96,6 +96,14 @@ SecurityBundle
 
  * The `FirewallMap::$map` and `$container` properties have been deprecated and will be removed in 4.0.
 
+ * The `UserPasswordEncoderCommand` command expects to be registered as a service and its
+   constructor arguments fully provided.
+   Registering by convention the command or commands extending it is deprecated and will
+   not be allowed anymore in 4.0.
+
+ * `UserPasswordEncoderCommand::getContainer()` is deprecated, and this class won't 
+    extend `ContainerAwareCommand` nor implement `ContainerAwareInterface` anymore in 4.0.
+
 TwigBridge
 ----------
 

UPGRADE-4.0.md

@@ -437,6 +437,13 @@ Ldap
 
  * The `RenameEntryInterface` has been deprecated, and merged with `EntryManagerInterface`
 
+SecurityBundle
+--------------
+
+ * The `UserPasswordEncoderCommand` class does not allow `null` as the first argument anymore.
+
+ * `UserPasswordEncoderCommand` does not implement `ContainerAwareInterface` anymore.
+
 Workflow
 --------
 

src/Symfony/Bundle/SecurityBundle/CHANGELOG.md

@@ -4,6 +4,10 @@ CHANGELOG
 3.3.0
 -----
 
+ * Deprecated instantiating `UserPasswordEncoderCommand` without its constructor
+   arguments fully provided.
+ * Deprecated `UserPasswordEncoderCommand::getContainer()` and relying on the
+  `ContainerAwareInterface` interface for this command.
  * Deprecated the `FirewallMap::$map` and `$container` properties.
 
 3.2.0

src/Symfony/Bundle/SecurityBundle/Command/UserPasswordEncoderCommand.php

@@ -18,7 +18,10 @@
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Question\Question;
 use Symfony\Component\Console\Style\SymfonyStyle;
+use Symfony\Component\DependencyInjection\ContainerAwareInterface;
 use Symfony\Component\Security\Core\Encoder\BCryptPasswordEncoder;
+use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface;
+use Symfony\Component\Security\Core\User\User;
 
 /**
  * Encode a user's password.
@@ -27,6 +30,31 @@
  */
 class UserPasswordEncoderCommand extends ContainerAwareCommand
 {
+    private $encoderFactory;
+    private $userClasses;
+
+    public function __construct(EncoderFactoryInterface $encoderFactory = null, array $userClasses = array())
+    {
+        if (null === $encoderFactory) {
+            @trigger_error(sprintf('Passing null as the first argument of "%s" is deprecated since version 3.3 and will be removed in 4.0. If the command was registered by convention, make it a service instead.', __METHOD__), E_USER_DEPRECATED);
+        }
+
+        $this->encoderFactory = $encoderFactory;
+        $this->userClasses = $userClasses;
+
+        parent::__construct();
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function getContainer()
+    {
+        @trigger_error(sprintf('Method "%s" is deprecated since version 3.3 and "%s" won\'t implement "%s" anymore in 4.0.', __METHOD__, __CLASS__, ContainerAwareInterface::class), E_USER_DEPRECATED);
+
+        return parent::getContainer();
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -36,7 +64,7 @@ protected function configure()
             ->setName('security:encode-password')
             ->setDescription('Encodes a password.')
             ->addArgument('password', InputArgument::OPTIONAL, 'The plain password to encode.')
-            ->addArgument('user-class', InputArgument::OPTIONAL, 'The User entity class path associated with the encoder used to encode the password.', 'Symfony\Component\Security\Core\User\User')
+            ->addArgument('user-class', InputArgument::OPTIONAL, 'The User entity class path associated with the encoder used to encode the password.')
             ->addOption('empty-salt', null, InputOption::VALUE_NONE, 'Do not generate a salt or let the encoder generate one.')
             ->setHelp(<<<EOF
 
@@ -55,8 +83,9 @@ protected function configure()
         AppBundle\Entity\User: bcrypt
 </comment>
 
-If you execute the command non-interactively, the default Symfony User class
-is used and a random salt is generated to encode the password:
+If you execute the command non-interactively, the first available configured
+user class under the <comment>security.encoders</comment> key is used and a random salt is
+generated to encode the password:
 
   <info>php %command.full_name% --no-interaction [password]</info>
 
@@ -89,10 +118,11 @@ protected function execute(InputInterface $input, OutputInterface $output)
         $input->isInteractive() ? $io->title('Symfony Password Encoder Utility') : $io->newLine();
 
         $password = $input->getArgument('password');
-        $userClass = $input->getArgument('user-class');
+        $userClass = $this->getUserClass($input, $io);
         $emptySalt = $input->getOption('empty-salt');
 
-        $encoder = $this->getContainer()->get('security.encoder_factory')->getEncoder($userClass);
+        $encoderFactory = $this->encoderFactory ?: parent::getContainer()->get('security.encoder_factory');
+        $encoder = $encoderFactory->getEncoder($userClass);
         $bcryptWithoutEmptySalt = !$emptySalt && $encoder instanceof BCryptPasswordEncoder;
 
         if ($bcryptWithoutEmptySalt) {
@@ -166,4 +196,30 @@ private function generateSalt()
     {
         return base64_encode(random_bytes(30));
     }
+
+    private function getUserClass(InputInterface $input, SymfonyStyle $io)
+    {
+        if (null !== $userClass = $input->getArgument('user-class')) {
+            return $userClass;
+        }
+
+        if (empty($this->userClasses)) {
+            if (null === $this->encoderFactory) {
+                // BC to be removed and simply keep the exception whenever there is no configured user classes in 4.0
+                return User::class;
+            }
+
+            throw new \RuntimeException('There are no configured encoders for the "security" extension.');
+        }
+
+        if (!$input->isInteractive() || 1 === count($this->userClasses)) {
+            return reset($this->userClasses);
+        }
+
+        $userClasses = $this->userClasses;
+        natcasesort($userClasses);
+        $userClasses = array_values($userClasses);
+
+        return $io->choice('For which user class would you like to encode a password?', $userClasses, reset($userClasses));
+    }
 }

src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php

@@ -14,6 +14,7 @@
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\SecurityFactoryInterface;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\UserProvider\UserProviderFactoryInterface;
 use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
+use Symfony\Component\Console\Application;
 use Symfony\Component\DependencyInjection\Alias;
 use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
 use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
@@ -96,6 +97,11 @@ public function load(array $configs, ContainerBuilder $container)
 
         if ($config['encoders']) {
             $this->createEncoders($config['encoders'], $container);
+
+            if (class_exists(Application::class)) {
+                $loader->load('console.xml');
+                $container->getDefinition('security.console.user_password_encoder_command')->replaceArgument(1, array_keys($config['encoders']));
+            }
         }
 
         // load ACL

src/Symfony/Bundle/SecurityBundle/Resources/config/console.xml

@@ -0,0 +1,14 @@
+<?xml version="1.0" ?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
+
+    <services>
+        <service id="security.console.user_password_encoder_command" class="Symfony\Bundle\SecurityBundle\Command\UserPasswordEncoderCommand" public="false">
+            <argument type="service" id="security.encoder_factory"/>
+            <argument type="collection" /> <!-- encoders' user classes -->
+            <tag name="console.command" />
+        </service>
+    </services>
+</container>

src/Symfony/Bundle/SecurityBundle/Tests/Functional/UserPasswordEncoderCommandTest.php

@@ -13,8 +13,10 @@
 
 use Symfony\Bundle\FrameworkBundle\Console\Application;
 use Symfony\Bundle\SecurityBundle\Command\UserPasswordEncoderCommand;
+use Symfony\Component\Console\Application as ConsoleApplication;
 use Symfony\Component\Console\Tester\CommandTester;
 use Symfony\Component\Security\Core\Encoder\BCryptPasswordEncoder;
+use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface;
 use Symfony\Component\Security\Core\Encoder\Pbkdf2PasswordEncoder;
 
 /**
@@ -24,6 +26,7 @@
  */
 class UserPasswordEncoderCommandTest extends WebTestCase
 {
+    /** @var CommandTester */
     private $passwordEncoderCommandTester;
 
     public function testEncodePasswordEmptySalt()
@@ -105,6 +108,7 @@ public function testEncodePasswordEmptySaltOutput()
             array(
                 'command' => 'security:encode-password',
                 'password' => 'p@ssw0rd',
+                'user-class' => 'Symfony\Component\Security\Core\User\User',
                 '--empty-salt' => true,
             )
         );
@@ -138,6 +142,74 @@ public function testEncodePasswordNoConfigForGivenUserClass()
         ), array('interactive' => false));
     }
 
+    public function testEncodePasswordAsksNonProvidedUserClass()
+    {
+        $this->passwordEncoderCommandTester->setInputs(array('Custom\Class\Pbkdf2\User', "\n"));
+        $this->passwordEncoderCommandTester->execute(array(
+            'command' => 'security:encode-password',
+            'password' => 'password',
+        ), array('decorated' => false));
+
+        $this->assertContains(<<<EOTXT
+ For which user class would you like to encode a password? [Custom\Class\Bcrypt\User]:
+  [0] Custom\Class\Bcrypt\User
+  [1] Custom\Class\Pbkdf2\User
+  [2] Custom\Class\Test\User
+  [3] Symfony\Component\Security\Core\User\User
+EOTXT
+        , $this->passwordEncoderCommandTester->getDisplay(true));
+    }
+
+    public function testNonInteractiveEncodePasswordUsesFirstUserClass()
+    {
+        $this->passwordEncoderCommandTester->execute(array(
+            'command' => 'security:encode-password',
+            'password' => 'password',
+        ), array('interactive' => false));
+
+        $this->assertContains('Encoder used       Symfony\Component\Security\Core\Encoder\PlaintextPasswordEncoder', $this->passwordEncoderCommandTester->getDisplay());
+    }
+
+    /**
+     * @expectedException \RuntimeException
+     * @expectedExceptionMessage There are no configured encoders for the "security" extension.
+     */
+    public function testThrowsExceptionOnNoConfiguredEncoders()
+    {
+        $application = new ConsoleApplication();
+        $application->add(new UserPasswordEncoderCommand($this->createMock(EncoderFactoryInterface::class), array()));
+
+        $passwordEncoderCommand = $application->find('security:encode-password');
+
+        $tester = new CommandTester($passwordEncoderCommand);
+        $tester->execute(array(
+            'command' => 'security:encode-password',
+            'password' => 'password',
+        ), array('interactive' => false));
+    }
+
+    /**
+     * @group legacy
+     * @expectedDeprecation Passing null as the first argument of "Symfony\Bundle\SecurityBundle\Command\UserPasswordEncoderCommand::__construct" is deprecated since version 3.3 and will be removed in 4.0. If the command was registered by convention, make it a service instead.
+     */
+    public function testLegacy()
+    {
+        $application = new ConsoleApplication();
+        $application->add(new UserPasswordEncoderCommand());
+
+        $passwordEncoderCommand = $application->find('security:encode-password');
+        self::bootKernel(array('test_case' => 'PasswordEncode'));
+        $passwordEncoderCommand->setContainer(self::$kernel->getContainer());
+
+        $tester = new CommandTester($passwordEncoderCommand);
+        $tester->execute(array(
+            'command' => 'security:encode-password',
+            'password' => 'password',
+        ), array('interactive' => false));
+
+        $this->assertContains('Encoder used       Symfony\Component\Security\Core\Encoder\PlaintextPasswordEncoder', $tester->getDisplay());
+    }
+
     protected function setUp()
     {
         putenv('COLUMNS='.(119 + strlen(PHP_EOL)));
@@ -146,8 +218,7 @@ protected function setUp()
 
         $application = new Application($kernel);
 
-        $application->add(new UserPasswordEncoderCommand());
-        $passwordEncoderCommand = $application->find('security:encode-password');
+        $passwordEncoderCommand = $application->get('security:encode-password');
 
         $this->passwordEncoderCommandTester = new CommandTester($passwordEncoderCommand);
     }

src/Symfony/Bundle/SecurityBundle/composer.json

@@ -25,7 +25,7 @@
     "require-dev": {
         "symfony/asset": "~2.8|~3.0",
         "symfony/browser-kit": "~2.8|~3.0",
-        "symfony/console": "~2.8|~3.0",
+        "symfony/console": "~3.2",
         "symfony/css-selector": "~2.8|~3.0",
         "symfony/dom-crawler": "~2.8|~3.0",
         "symfony/form": "~2.8|~3.0",