Skip to content

Commit 76bcefa

Browse files
wouterjwouterj
committed
[make:user] Use password_hashers instead of encoders
And also improve the position of this config when "enable_authenticator_manager" is the first option.
1 parent 702558c commit 76bcefa

27 files changed

+228
-17
lines changed

‎src/Maker/MakeUser.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
use Symfony\Component\Console\Input\InputArgument;
3232
use Symfony\Component\Console\Input\InputInterface;
3333
use Symfony\Component\Console\Input\InputOption;
34+
use Symfony\Component\PasswordHasher\Hasher\NativePasswordHasher;
3435
use Symfony\Component\Security\Core\Encoder\Argon2iPasswordEncoder;
3536
use Symfony\Component\Security\Core\Encoder\NativePasswordEncoder;
3637
use Symfony\Component\Security\Core\Exception\UserNotFoundException;
@@ -116,6 +117,11 @@ class_exists(DoctrineBundle::class)
116117
$userWillHavePassword = $io->confirm('Does this app need to hash/check user passwords?');
117118
$input->setOption('with-password', $userWillHavePassword);
118119

120+
$symfonyGte53 = class_exists(NativePasswordHasher::class);
121+
if ($symfonyGte53) {
122+
return;
123+
}
124+
119125
if ($userWillHavePassword && !class_exists(NativePasswordEncoder::class) && Argon2iPasswordEncoder::isSupported()) {
120126
$io->writeln('The newer <comment>Argon2i</comment> password hasher requires PHP 7.2, libsodium or paragonie/sodium_compat. Your system DOES support this algorithm.');
121127
$io->writeln('You should use <comment>Argon2i</comment> unless your production system will not support it.');

‎src/Resources/skeleton/security/UserProvider.tpl.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,13 @@ public function supportsClass($class)
7272
<?php if ($password_upgrader): ?>
7373

7474
/**
75-
* Upgrades the encoded password of a user, typically for using a better hash algorithm.
75+
* Upgrades the hashed password of a user, typically for using a better hash algorithm.
7676
*/
77-
public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
77+
public function upgradePassword(UserInterface $user, string $newHashedPassword): void
7878
{
79-
// TODO: when encoded passwords are in use, this method should:
79+
// TODO: when hashed passwords are in use, this method should:
8080
// 1. persist the new password in the user storage
81-
// 2. update the $user object with $user->setPassword($newEncodedPassword);
81+
// 2. update the $user object with $user->setPassword($newHashedPassword);
8282
}
8383
<?php endif ?>
8484
}

‎src/Security/SecurityConfigUpdater.php

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Symfony\Bundle\MakerBundle\Util\YamlSourceManipulator;
1515
use Symfony\Component\HttpKernel\Log\Logger;
16+
use Symfony\Component\PasswordHasher\Hasher\NativePasswordHasher;
1617
use Symfony\Component\Security\Core\Encoder\NativePasswordEncoder;
1718

1819
/**
@@ -50,7 +51,8 @@ public function updateForUserClass(string $yamlSource, UserClassConfiguration $u
5051
$this->updateProviders($userConfig, $userClass);
5152

5253
if ($userConfig->hasPassword()) {
53-
$this->updateEncoders($userConfig, $userClass);
54+
$symfonyGte53 = class_exists(NativePasswordHasher::class);
55+
$this->updatePasswordHashers($userConfig, $userClass, $symfonyGte53 ? 'password_hashers' : 'encoders');
5456
}
5557

5658
$contents = $this->manipulator->getContents();
@@ -72,6 +74,10 @@ public function updateForAuthenticator(string $yamlSource, string $firewallName,
7274
$newData = $this->manipulator->getData();
7375

7476
if (!isset($newData['security']['firewalls'])) {
77+
if ($newData['security']) {
78+
$newData['security']['_firewalls'] = $this->manipulator->createEmptyLine();
79+
}
80+
7581
$newData['security']['firewalls'] = [];
7682
}
7783

@@ -153,6 +159,10 @@ private function updateProviders(UserClassConfiguration $userConfig, string $use
153159
$this->removeMemoryProviderIfIsSingleConfigured();
154160

155161
$newData = $this->manipulator->getData();
162+
if ($newData['security'] && !\array_key_exists('providers', $newData['security'])) {
163+
$newData['security']['_providers'] = $this->manipulator->createEmptyLine();
164+
}
165+
156166
$newData['security']['providers']['__'] = $this->manipulator->createCommentLine(
157167
' used to reload user from session & other features (e.g. switch_user)'
158168
);
@@ -175,18 +185,34 @@ private function updateProviders(UserClassConfiguration $userConfig, string $use
175185
$this->manipulator->setData($newData);
176186
}
177187

178-
private function updateEncoders(UserClassConfiguration $userConfig, string $userClass)
188+
private function updatePasswordHashers(UserClassConfiguration $userConfig, string $userClass, string $keyName = 'password_hashers')
179189
{
180190
$newData = $this->manipulator->getData();
181-
if (!isset($newData['security']['encoders'])) {
182-
// encoders is usually the first key, by convention
183-
$newData['security'] = ['encoders' => []] + $newData['security'];
191+
if ('password_hashers' === $keyName && isset($newData['security']['encoders'])) {
192+
// fallback to "encoders" if the user already defined encoder config
193+
$this->updatePasswordHashers($userClass, $userClass, 'encoders');
194+
195+
return;
196+
}
197+
198+
if (!isset($newData['security'][$keyName])) {
199+
// by convention, password_hashers are put before the user provider option
200+
$providersIndex = array_search('providers', array_keys($newData['security']));
201+
if (false === $providersIndex) {
202+
$newData['security'] = [$keyName => []] + $newData['security'];
203+
} else {
204+
$newData['security'] = array_merge(
205+
\array_slice($newData['security'], 0, $providersIndex),
206+
[$keyName => []],
207+
\array_slice($newData['security'], $providersIndex)
208+
);
209+
}
184210
}
185211

186-
$newData['security']['encoders'][$userClass] = [
187-
'algorithm' => $userConfig->shouldUseArgon2() ? 'argon2i' : (class_exists(NativePasswordEncoder::class) ? 'auto' : 'bcrypt'),
212+
$newData['security'][$keyName][$userClass] = [
213+
'algorithm' => $userConfig->shouldUseArgon2() ? 'argon2i' : ((class_exists(NativePasswordHasher::class) || class_exists(NativePasswordEncoder::class)) ? 'auto' : 'bcrypt'),
188214
];
189-
$newData['security']['encoders']['_'] = $this->manipulator->createEmptyLine();
215+
$newData['security'][$keyName]['_'] = $this->manipulator->createEmptyLine();
190216

191217
$this->manipulator->setData($newData);
192218
}

‎tests/Security/SecurityConfigUpdaterTest.php

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Bundle\MakerBundle\Security\SecurityConfigUpdater;
1717
use Symfony\Bundle\MakerBundle\Security\UserClassConfiguration;
1818
use Symfony\Component\HttpKernel\Log\Logger;
19+
use Symfony\Component\PasswordHasher\Hasher\NativePasswordHasher;
1920
use Symfony\Component\Security\Core\Encoder\NativePasswordEncoder;
2021

2122
class SecurityConfigUpdaterTest extends TestCase
@@ -48,9 +49,10 @@ public function testUpdateForUserClass(UserClassConfiguration $userConfig, strin
4849
$updater = new SecurityConfigUpdater($this->ysmLogger);
4950
$source = file_get_contents(__DIR__.'/yaml_fixtures/source/'.$startingSourceFilename);
5051
$actualSource = $updater->updateForUserClass($source, $userConfig, $userClass);
51-
$expectedSource = file_get_contents(__DIR__.'/yaml_fixtures/expected_user_class/'.$expectedSourceFilename);
52+
$symfonyVersion = class_exists(NativePasswordHasher::class) ? '5.3' : 'legacy';
53+
$expectedSource = file_get_contents(__DIR__.'/yaml_fixtures/expected_user_class/'.$symfonyVersion.'/'.$expectedSourceFilename);
5254

53-
$bcryptOrAuto = class_exists(NativePasswordEncoder::class) ? 'auto' : 'bcrypt';
55+
$bcryptOrAuto = ('5.3' === $symfonyVersion || class_exists(NativePasswordEncoder::class)) ? 'auto' : 'bcrypt';
5456
$expectedSource = str_replace('{BCRYPT_OR_AUTO}', $bcryptOrAuto, $expectedSource);
5557

5658
$this->assertSame($expectedSource, $actualSource);
@@ -95,6 +97,18 @@ public function getUserClassTests()
9597
'simple_security_with_single_memory_provider_configured.yaml',
9698
'simple_security_with_single_memory_provider_configured.yaml',
9799
];
100+
101+
yield 'security_52_empty_security' => [
102+
new UserClassConfiguration(true, 'email', true),
103+
'security_52_entity_email_with_password.yaml',
104+
'security_52_empty_security.yaml',
105+
];
106+
107+
yield 'security_52_with_firewalls_and_logout' => [
108+
new UserClassConfiguration(true, 'email', true),
109+
'security_52_complex_entity_email_with_password.yaml',
110+
'security_52_with_firewalls_and_logout.yaml',
111+
];
98112
}
99113

100114
/**

‎tests/Security/yaml_fixtures/expected_authenticator/empty_source.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@ security:
33
main:
44
anonymous: lazy
55
guard:
6-
authenticators: [App\Security\AppCustomAuthenticator]
6+
authenticators: [App\Security\AppCustomAuthenticator]

‎tests/Security/yaml_fixtures/expected_authenticator/security_52_empty_source.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
security:
22
enable_authenticator_manager: true
3+
34
firewalls:
45
main:
56
lazy: true

‎tests/Security/yaml_fixtures/expected_authenticator/security_52_with_multiple_authenticators.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
security:
22
enable_authenticator_manager: true
3+
34
firewalls:
45
main:
56
lazy: true

‎tests/Security/yaml_fixtures/expected_user_class/5.3/empty_source_model_email_with_password.yaml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
security:
2+
password_hashers:
3+
App\Security\User:
4+
algorithm: {BCRYPT_OR_AUTO}
5+
6+
providers:
7+
# used to reload user from session & other features (e.g. switch_user)
8+
app_user_provider:
9+
id: App\Security\UserProvider

‎tests/Security/yaml_fixtures/expected_user_class/5.3/entity_email_with_password.yaml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
security:
2+
password_hashers:
3+
App\Entity\User:
4+
algorithm: {BCRYPT_OR_AUTO}
5+
6+
# https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers
7+
providers:
8+
# used to reload user from session & other features (e.g. switch_user)
9+
app_user_provider:
10+
entity:
11+
class: App\Entity\User
12+
property: email
13+
14+
firewalls:
15+
dev: ~
16+
main: ~

‎tests/Security/yaml_fixtures/expected_user_class/entity_username_no_password.yaml renamed to ‎tests/Security/yaml_fixtures/expected_user_class/5.3/entity_username_no_password.yaml

File renamed without changes.

‎tests/Security/yaml_fixtures/expected_user_class/5.3/model_email_password_existing_providers.yaml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
security:
2+
password_hashers:
3+
App\Security\User:
4+
algorithm: {BCRYPT_OR_AUTO}
5+
6+
# https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers
7+
providers:
8+
in_memory: { memory: ~ }
9+
other_provider: { foo: true }
10+
# used to reload user from session & other features (e.g. switch_user)
11+
app_user_provider:
12+
id: App\Security\UserProvider
13+
14+
firewalls:
15+
dev: ~
16+
main: ~

‎tests/Security/yaml_fixtures/expected_user_class/5.3/model_email_with_password.yaml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
security:
2+
password_hashers:
3+
App\Security\User:
4+
algorithm: {BCRYPT_OR_AUTO}
5+
6+
# https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers
7+
providers:
8+
# used to reload user from session & other features (e.g. switch_user)
9+
app_user_provider:
10+
id: App\Security\UserProvider
11+
12+
firewalls:
13+
dev: ~
14+
main: ~

‎tests/Security/yaml_fixtures/expected_user_class/model_username_no_password.yaml renamed to ‎tests/Security/yaml_fixtures/expected_user_class/5.3/model_username_no_password.yaml

File renamed without changes.

‎tests/Security/yaml_fixtures/expected_user_class/5.3/security_52_complex_entity_email_with_password.yaml

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
security:
2+
enable_authenticator_manager: true
3+
password_hashers:
4+
App\Entity\User:
5+
algorithm: {BCRYPT_OR_AUTO}
6+
7+
# https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers
8+
providers:
9+
# used to reload user from session & other features (e.g. switch_user)
10+
app_user_provider:
11+
entity:
12+
class: App\Entity\User
13+
property: email
14+
15+
firewalls:
16+
dev:
17+
pattern: ^/(_(profiler|wdt)|css|images|js)/
18+
security: false
19+
main:
20+
lazy: true

‎tests/Security/yaml_fixtures/expected_user_class/5.3/security_52_entity_email_with_password.yaml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
security:
2+
enable_authenticator_manager: true
3+
4+
password_hashers:
5+
App\Entity\User:
6+
algorithm: {BCRYPT_OR_AUTO}
7+
8+
providers:
9+
# used to reload user from session & other features (e.g. switch_user)
10+
app_user_provider:
11+
entity:
12+
class: App\Entity\User
13+
property: email

‎tests/Security/yaml_fixtures/expected_user_class/5.3/simple_security_with_single_memory_provider_configured.yaml

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
security:
2+
password_hashers:
3+
App\Entity\User:
4+
algorithm: {BCRYPT_OR_AUTO}
5+
6+
# https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers
7+
providers:
8+
# used to reload user from session & other features (e.g. switch_user)
9+
app_user_provider:
10+
entity:
11+
class: App\Entity\User
12+
property: email
13+
14+
firewalls:
15+
dev: ~
16+
main:
17+
anonymous: lazy
18+
provider: app_user_provider

‎tests/Security/yaml_fixtures/expected_user_class/empty_source_model_email_with_password.yaml renamed to ‎tests/Security/yaml_fixtures/expected_user_class/legacy/empty_source_model_email_with_password.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@ security:
66
providers:
77
# used to reload user from session & other features (e.g. switch_user)
88
app_user_provider:
9-
id: App\Security\UserProvider
9+
id: App\Security\UserProvider

‎tests/Security/yaml_fixtures/expected_user_class/entity_email_with_password.yaml renamed to ‎tests/Security/yaml_fixtures/expected_user_class/legacy/entity_email_with_password.yaml

File renamed without changes.

‎tests/Security/yaml_fixtures/expected_user_class/legacy/entity_username_no_password.yaml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
security:
2+
# https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers
3+
providers:
4+
# used to reload user from session & other features (e.g. switch_user)
5+
app_user_provider:
6+
entity:
7+
class: App\Entity\User
8+
property: username
9+
10+
firewalls:
11+
dev: ~
12+
main: ~

‎tests/Security/yaml_fixtures/expected_user_class/model_email_password_existing_providers.yaml renamed to ‎tests/Security/yaml_fixtures/expected_user_class/legacy/model_email_password_existing_providers.yaml

File renamed without changes.

‎tests/Security/yaml_fixtures/expected_user_class/model_email_with_password.yaml renamed to ‎tests/Security/yaml_fixtures/expected_user_class/legacy/model_email_with_password.yaml

File renamed without changes.

‎tests/Security/yaml_fixtures/expected_user_class/legacy/model_username_no_password.yaml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
security:
2+
# https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers
3+
providers:
4+
# used to reload user from session & other features (e.g. switch_user)
5+
app_user_provider:
6+
id: App\Security\UserProvider
7+
8+
firewalls:
9+
dev: ~
10+
main: ~

‎tests/Security/yaml_fixtures/expected_user_class/legacy/security_52_complex_entity_email_with_password.yaml

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
security:
2+
enable_authenticator_manager: true
3+
4+
encoders:
5+
App\Entity\User:
6+
algorithm: {BCRYPT_OR_AUTO}
7+
8+
# https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers
9+
providers:
10+
# used to reload user from session & other features (e.g. switch_user)
11+
app_user_provider:
12+
entity:
13+
class: App\Entity\User
14+
property: email
15+
16+
firewalls:
17+
dev:
18+
pattern: ^/(_(profiler|wdt)|css|images|js)/
19+
security: false
20+
main:
21+
lazy: true

‎tests/Security/yaml_fixtures/expected_user_class/legacy/security_52_entity_email_with_password.yaml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
security:
2+
enable_authenticator_manager: true
3+
4+
encoders:
5+
App\Entity\User:
6+
algorithm: {BCRYPT_OR_AUTO}
7+
8+
providers:
9+
# used to reload user from session & other features (e.g. switch_user)
10+
app_user_provider:
11+
entity:
12+
class: App\Entity\User
13+
property: email

‎tests/Security/yaml_fixtures/expected_user_class/simple_security_with_single_memory_provider_configured.yaml renamed to ‎tests/Security/yaml_fixtures/expected_user_class/legacy/simple_security_with_single_memory_provider_configured.yaml

File renamed without changes.

‎tests/Security/yaml_fixtures/source/empty_security.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
security: {}
1+
security: {}

‎tests/Security/yaml_fixtures/source/security_52_with_multiple_authenticators.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
security:
22
enable_authenticator_manager: true
3+
34
firewalls:
45
main:
56
lazy: true

0 commit comments

Comments
 (0)