Skip to content

Commit

Permalink
feature Sylius#10080 Password hashing - multiple encoders support (pa…
Browse files Browse the repository at this point in the history
…mil)

This PR was merged into the 1.4-dev branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | 1.4
| Bug fix?        | yes
| New feature?    | yes
| BC breaks?      | no
| Deprecations?   | no
| Related tickets | Sylius#9560, Sylius#10008
| License         | MIT

It reverses the BC break made in Sylius#10008 which forced end-users to change their security settings. This PR makes it opt-in for old installations and is enabled by default for new ones.

<!--
 - Bug fixes must be submitted against the 1.2 or 1.3 branch (the lowest possible)
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set
-->

Commits
-------

d694c5f Fix backwards compatibility for security configuration
43aa742 Add encoder name to the User model
983729f Add possibity to setup default encoder for new users
c4872d2 Set up default encoders for Sylius application
  • Loading branch information
pamil authored Jan 10, 2019
2 parents 10101cd + c4872d2 commit 2e28729
Show file tree
Hide file tree
Showing 20 changed files with 260 additions and 34 deletions.
30 changes: 30 additions & 0 deletions app/migrations/Version20190109160409.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php declare(strict_types=1);

namespace Sylius\Migrations;

use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;

/**
* Auto-generated Migration: Please modify to your needs!
*/
final class Version20190109160409 extends AbstractMigration
{
public function up(Schema $schema) : void
{
// this up() migration is auto-generated, please modify it to your needs
$this->abortIf($this->connection->getDatabasePlatform()->getName() !== 'mysql', 'Migration can only be executed safely on \'mysql\'.');

$this->addSql('ALTER TABLE sylius_shop_user ADD encoder_name VARCHAR(255) DEFAULT NULL');
$this->addSql('ALTER TABLE sylius_admin_user ADD encoder_name VARCHAR(255) DEFAULT NULL');
}

public function down(Schema $schema) : void
{
// this down() migration is auto-generated, please modify it to your needs
$this->abortIf($this->connection->getDatabasePlatform()->getName() !== 'mysql', 'Migration can only be executed safely on \'mysql\'.');

$this->addSql('ALTER TABLE sylius_admin_user DROP encoder_name');
$this->addSql('ALTER TABLE sylius_shop_user DROP encoder_name');
}
}
3 changes: 3 additions & 0 deletions config/packages/_sylius.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,6 @@ parameters:
sylius_shop:
product_grid:
include_all_descendants: true

sylius_user:
encoder: argon2i
4 changes: 2 additions & 2 deletions config/packages/security.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ security:
sylius_shop_user_provider:
id: sylius.shop_user_provider.email_or_name_based
encoders:
sylius_password_encoder_sha512:
id: 'sylius.security.user_password_encoder.sha512'
Sylius\Component\User\Model\UserInterface: sha512
argon2i: argon2i
firewalls:
admin:
switch_user: true
Expand Down
3 changes: 1 addition & 2 deletions src/Sylius/Bundle/ReviewBundle/test/app/config/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ security:
password: foo
roles: ROLE_USER
encoders:
sylius_password_encoder_sha512:
id: 'sylius.security.user_password_encoder.sha512'
Sylius\Component\User\Model\UserInterface: sha512
firewalls:
admin:
switch_user: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public function getConfigTreeBuilder(): TreeBuilder
->addDefaultsIfNotSet()
->children()
->scalarNode('driver')->defaultValue(SyliusResourceBundle::DRIVER_DOCTRINE_ORM)->end()
->scalarNode('encoder')->defaultNull()->end()
->end()
;

Expand All @@ -62,6 +63,7 @@ private function addResourcesSection(ArrayNodeDefinition $node): void
->addDefaultsIfNotSet()
->children()
->scalarNode('templates')->defaultValue('SyliusUserBundle:User')->end()
->scalarNode('encoder')->defaultNull()->end()
->variableNode('options')->end()
->arrayNode('resetting')
->addDefaultsIfNotSet()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use Sylius\Bundle\UserBundle\EventListener\UserDeleteListener;
use Sylius\Bundle\UserBundle\EventListener\UserLastLoginSubscriber;
use Sylius\Bundle\UserBundle\EventListener\UserReloaderListener;
use Sylius\Bundle\UserBundle\Factory\UserWithEncoderFactory;
use Sylius\Bundle\UserBundle\Provider\AbstractUserProvider;
use Sylius\Bundle\UserBundle\Provider\EmailProvider;
use Sylius\Bundle\UserBundle\Provider\UsernameOrEmailProvider;
Expand Down Expand Up @@ -49,6 +50,7 @@ public function load(array $config, ContainerBuilder $container): void
$loader->load('services.xml');

$this->createServices($config['resources'], $container);
$this->createFactories($config['encoder'], $config['resources'], $container);
}

private function resolveResources(array $resources, ContainerBuilder $container): array
Expand Down Expand Up @@ -80,6 +82,30 @@ private function createServices(array $resources, ContainerBuilder $container):
}
}

private function createFactories(?string $globalEncoder, array $resources, ContainerBuilder $container): void
{
foreach ($resources as $userType => $config) {
$encoder = $config['user']['encoder'] ?? $globalEncoder;

if (null === $encoder) {
continue;
}

$factoryServiceId = sprintf('sylius.factory.%s_user', $userType);

$factoryDefinition = new Definition(
UserWithEncoderFactory::class,
[
$container->getDefinition($factoryServiceId),
$encoder,
]
);
$factoryDefinition->setPublic(true);

$container->setDefinition($factoryServiceId, $factoryDefinition);
}
}

private function createTokenGenerators(string $userType, array $config, ContainerBuilder $container): void
{
$this->createUniquenessCheckers($userType, $config, $container);
Expand Down
33 changes: 33 additions & 0 deletions src/Sylius/Bundle/UserBundle/Factory/UserWithEncoderFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

declare(strict_types=1);

namespace Sylius\Bundle\UserBundle\Factory;

use Sylius\Component\Resource\Factory\FactoryInterface;
use Sylius\Component\User\Model\UserInterface;

final class UserWithEncoderFactory implements FactoryInterface
{
/** @var FactoryInterface */
private $decoratedUserFactory;

/** @var string */
private $encoderName;

public function __construct(FactoryInterface $decoratedUserFactory, string $encoderName)
{
$this->decoratedUserFactory = $decoratedUserFactory;
$this->encoderName = $encoderName;
}

public function createNew(): UserInterface
{
/** @var UserInterface $user */
$user = $this->decoratedUserFactory->createNew();

$user->setEncoderName($this->encoderName);

return $user;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
<field name="enabled" column="enabled" type="boolean" nullable="false" />
<field name="salt" column="salt" type="string" nullable="false" />
<field name="password" column="password" type="string" nullable="false" />
<field name="encoderName" column="encoder_name" type="string" nullable="true" />

<field name="lastLogin" column="last_login" type="datetime" nullable="true" />
<field name="passwordResetToken" column="password_reset_token" type="string" nullable="true" />
Expand Down
6 changes: 0 additions & 6 deletions src/Sylius/Bundle/UserBundle/Resources/config/services.xml
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,6 @@
<argument type="service" id="event_dispatcher" />
</service>

<service class="Symfony\Component\Security\Core\Encoder\MessageDigestPasswordEncoder"
id="sylius.security.user_password_encoder.sha512"
>
<argument type="string">sha512</argument>
</service>

<!-- Listeners -->
<service id="sylius.listener.password_updater" class="Sylius\Bundle\UserBundle\EventListener\PasswordUpdaterListener">
<argument type="service" id="sylius.security.password_updater" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
<?php

declare(strict_types=1);

namespace Sylius\Bundle\UserBundle\Tests\DependencyInjection;

use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractExtensionTestCase;
use PHPUnit\Framework\Assert;
use Sylius\Bundle\UserBundle\DependencyInjection\SyliusUserExtension;
use Sylius\Bundle\UserBundle\Factory\UserWithEncoderFactory;
use Sylius\Component\Resource\Factory\Factory;

final class SyliusUserExtensionTest extends AbstractExtensionTestCase
{
/** @test */
public function it_creates_default_resource_factory_by_default(): void
{
$this->load([
'resources' => [
'admin' => [
'user' => [],
],
],
]);

$factoryDefinition = $this->container->getDefinition('sylius.factory.admin_user');

Assert::assertSame(Factory::class, $factoryDefinition->getClass());
}

/** @test */
public function it_decorates_user_factory_if_its_configuration_has_encoder_specified(): void
{
$this->load([
'resources' => [
'admin' => [
'user' => [
'encoder' => 'customencoder',
],
],
],
]);

$factoryDefinition = $this->container->getDefinition('sylius.factory.admin_user');

Assert::assertSame(UserWithEncoderFactory::class, $factoryDefinition->getClass());
Assert::assertSame('customencoder', $factoryDefinition->getArgument(1));
}

/** @test */
public function it_decorates_user_factory_if_there_is_a_global_encoder_specified_in_the_configuration(): void
{
$this->load([
'encoder' => 'customencoder',
'resources' => [
'admin' => [
'user' => [],
],
],
]);

$factoryDefinition = $this->container->getDefinition('sylius.factory.admin_user');

Assert::assertSame(UserWithEncoderFactory::class, $factoryDefinition->getClass());
Assert::assertSame('customencoder', $factoryDefinition->getArgument(1));
}

/** @test */
public function it_decorates_user_factory_using_the_most_specific_encoder_configured(): void
{
$this->load([
'encoder' => 'customencoder',
'resources' => [
'admin' => [
'user' => [
'encoder' => 'evenmorecustomencoder',
],
],
],
]);

$factoryDefinition = $this->container->getDefinition('sylius.factory.admin_user');

Assert::assertSame(UserWithEncoderFactory::class, $factoryDefinition->getClass());
Assert::assertSame('evenmorecustomencoder', $factoryDefinition->getArgument(1));
}

protected function getContainerExtensions(): iterable
{
return [
new SyliusUserExtension(),
];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ security:
password: foo
roles: ROLE_USER
encoders:
sylius_password_encoder_sha512:
id: 'sylius.security.user_password_encoder.sha512'
Sylius\Component\User\Model\UserInterface: sha512
firewalls:
admin:
switch_user: true
Expand Down
3 changes: 2 additions & 1 deletion src/Sylius/Bundle/UserBundle/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@
},
"require-dev": {
"hwi/oauth-bundle": "^0.6",
"matthiasnoback/symfony-dependency-injection-test": "^2.0",
"php-http/guzzle6-adapter": "^1.1",
"phpspec/phpspec": "^4.0",
"phpspec/phpspec": "^5.0",
"phpunit/phpunit": "^6.5",
"symfony/dependency-injection": "^3.4|^4.1.1",
"symfony/security-bundle": "^3.4|^4.1.1",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

declare(strict_types=1);

namespace spec\Sylius\Bundle\UserBundle\Factory;

use PhpSpec\ObjectBehavior;
use Sylius\Component\Resource\Factory\FactoryInterface;
use Sylius\Component\User\Model\UserInterface;

final class UserWithEncoderFactorySpec extends ObjectBehavior
{
function let(FactoryInterface $decoratedUserFactory)
{
$this->beConstructedWith($decoratedUserFactory, 'encodername');
}

function it_is_a_factory(): void
{
$this->shouldHaveType(FactoryInterface::class);
}

function it_sets_the_given_encoder_name_on_created_user(FactoryInterface $decoratedUserFactory, UserInterface $user): void
{
$decoratedUserFactory->createNew()->willReturn($user);

$user->setEncoderName('encodername')->shouldBeCalled();

$this->createNew()->shouldReturn($user);
}
}
8 changes: 0 additions & 8 deletions src/Sylius/Component/Core/Model/AdminUser.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,4 @@ public function setLocaleCode(?string $code): void
{
$this->localeCode = $code;
}

/**
* {@inheritdoc}
*/
public function getEncoderName(): string
{
return 'sylius_password_encoder_sha512';
}
}
3 changes: 1 addition & 2 deletions src/Sylius/Component/Core/Model/AdminUserInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@
namespace Sylius\Component\Core\Model;

use Sylius\Component\User\Model\UserInterface as BaseUserInterface;
use Symfony\Component\Security\Core\Encoder\EncoderAwareInterface;

interface AdminUserInterface extends BaseUserInterface, EncoderAwareInterface
interface AdminUserInterface extends BaseUserInterface
{
public const DEFAULT_ADMIN_ROLE = 'ROLE_ADMINISTRATION_ACCESS';

Expand Down
8 changes: 0 additions & 8 deletions src/Sylius/Component/Core/Model/ShopUser.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,4 @@ public function setEmailCanonical(?string $emailCanonical): void

$this->customer->setEmailCanonical($emailCanonical);
}

/**
* {@inheritdoc}
*/
public function getEncoderName(): string
{
return 'sylius_password_encoder_sha512';
}
}
3 changes: 1 addition & 2 deletions src/Sylius/Component/Core/Model/ShopUserInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@

use Sylius\Component\Customer\Model\CustomerAwareInterface;
use Sylius\Component\User\Model\UserInterface as BaseUserInterface;
use Symfony\Component\Security\Core\Encoder\EncoderAwareInterface;

interface ShopUserInterface extends BaseUserInterface, CustomerAwareInterface, EncoderAwareInterface
interface ShopUserInterface extends BaseUserInterface, CustomerAwareInterface
{
}
Loading

0 comments on commit 2e28729

Please sign in to comment.