Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 16, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions UPGRADE-3.3.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
----------

Expand Down
7 changes: 7 additions & 0 deletions UPGRADE-4.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
--------

Expand Down
4 changes: 4 additions & 0 deletions src/Symfony/Bundle/SecurityBundle/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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}
*/
Expand All @@ -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

Expand All @@ -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>

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$userClasses = array_values($this->userClasses);
natcasesort($userClasses);

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

natcasesort preserves keys. Hence the array_values call afterwards :)

natcasesort($userClasses);
$userClasses = array_values($userClasses);

return $io->choice('For which user class would you like to encode a password?', $userClasses, reset($userClasses));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions src/Symfony/Bundle/SecurityBundle/Resources/config/console.xml
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -24,6 +26,7 @@
*/
class UserPasswordEncoderCommandTest extends WebTestCase
{
/** @var CommandTester */
private $passwordEncoderCommandTester;

public function testEncodePasswordEmptySalt()
Expand Down Expand Up @@ -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,
)
);
Expand Down Expand Up @@ -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)));
Expand All @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/SecurityBundle/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"symfony/css-selector": "~2.8|~3.0",
"symfony/dom-crawler": "~2.8|~3.0",
"symfony/form": "~2.8|~3.0",
Expand Down