8000 feature #15141 [DX] [Security] Renamed Token#getKey() to getSecret() … · symfony/symfony@fc6ed5b · GitHub
[go: up one dir, main page]
Skip to content

Commit fc6ed5b

Browse files
fabpotfabpot
committed
feature #15141 [DX] [Security] Renamed Token#getKey() to getSecret() (WouterJ)
This PR was squashed before being merged into the 2.8 branch (closes #15141). Discussion ---------- [DX] [Security] Renamed Token#getKey() to getSecret() There are 2 very vague parameter names in the authentication process: `$providerKey` and `$key`. Some tokens/providers have the first one, some tokens/providers the second one and some both. An overview: | Token | `providerKey` | `key` | --- | --- | --- | `AnonymousToken` | - | yes | `PreAuth...Token` | yes | - | `RememberMeToken` | yes | yes | `UsernamePasswordToken` | yes | - Both names are extremely general and their PHPdocs contains pure no-shit-sherlock-descriptions :squirrel: (like "The key."). This made me and @iltar think it's just an inconsistency and they have the same meaning. ...until we dived deeper into the code and came to the conclusion that `$key` has a Security task (while `$providerKey` doesn't really). If it takes people connected to Symfony internals 30+ minutes to find this out, it should be considered for an improvement imo. So here is our suggestion: **Rename `$key` to `$secret`**. This explains much better what the value of the string has to be (for instance, it's important that the string is not easily guessable and cannot be found out, according to the Spring docs). It also explains the usage better (it's used as a replacement for credentials and to hash the RememberMeToken). **Tl;dr**: `$key` and `$providerKey` are too general names, let's improve DX by renaming them. This PR tackles `$key` by renaming it to `$secret`. | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - *My excuse for the completely unrelated branch name* Commits ------- 24e0eb6 [DX] [Security] Renamed Token#getKey() to getSecret()
2 parents f59a0ba + 24e0eb6 commit fc6ed5b

28 files changed

+250
-92
lines changed

‎UPGRADE-3.0.md

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,86 @@ UPGRADE FROM 2.x to 3.0
593593

594594
* The `Resources/` directory was moved to `Core/Resources/`
595595

596+
* The `key` settings of `anonymous` and `remember_me` are renamed to `secret`.
597+
598+
Before:
599+
600+
```yaml
601+
security:
602+
# ...
603+
firewalls:
604+
default:
605+
# ...
606+
anonymous: { key: "%secret%" }
607+
remember_me:
608+
key: "%secret%"
609+
```
610+
611+
```xml
612+
<!-- ... -->
613+
<config>
614+
<!-- ... -->
615+
616+
<firewall>
617+
<!-- ... -->
618+
619+
<anonymous key="%secret%"/>
620+
<remember-me key="%secret%"/>
621+
</firewall>
622+
</config>
623+
```
624+
625+
```php
626+
// ...
627+
$container->loadFromExtension('security', array(
628+
// ...
629+
'firewalls' => array(
630+
// ...
631+
'anonymous' => array('key' => '%secret%'),
632+
'remember_me' => array('key' => '%secret%'),
633+
),
634+
));
635+
```
636+
637+
After:
638+
639+
```yaml
640+
security:
641+
# ...
642+
firewalls:
643+
default:
644+
# ...
645+
anonymous: { secret: "%secret%" }
646+
remember_me:
647+
secret: "%secret%"
648+
```
649+
650+
```xml
651+
<!-- ... -->
652+
<config>
653+
<!-- ... -->
654+
655+
<firewall>
656+
<!-- ... -->
657+
658+
<anonymous secret="%secret%"/>
659+
<remember-me secret="%secret%"/>
660+
</firewall>
661+
</config>
662+
```
663+
664+
```php
665+
// ...
666+
$container->loadFromExtension('security', array(
667+
// ...
668+
'firewalls' => array(
669+
// ...
670+
'anonymous' => array('secret' => '%secret%'),
671+
'remember_me' => array('secret' => '%secret%'),
672+
),
673+
));
674+
```
675+
596676
### Translator
597677

598678
* The `Translator::setFallbackLocale()` method has been removed in favor of

‎src/Symfony/Bundle/SecurityBundle/CHANGELOG.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,18 @@
11
CHANGELOG
22
=========
33

4+
2.8.0
5+
-----
6+
7+
* deprecated the `key` setting of `anonymous` and `remember_me` in favor of the
8+
`secret` setting.
9+
410
2.6.0
511
-----
612

713
* Added the possibility to override the default success/failure handler
814
to get the provider key and the options injected
9-
* Deprecated the `security.context` service for the `security.token_storage` and
15+
* Deprecated the `security.context` service for the `security.token_storage` and
1016
`security.authorization_checker` services.
1117

1218
2.4.0

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,8 +285,22 @@ private function addFirewallsSection(ArrayNodeDefinition $rootNode, array $facto
285285
->end()
286286
->arrayNode('anonymous')
287287
->canBeUnset()
288+
->beforeNormalization()
289+
->ifTrue(function ($v) { return isset($v['key']); })
290+
->then(function ($v) {
291+
if (isset($v['secret'])) {
292+
throw new \LogicException('Cannot set both key and secret options for security.firewall.anonymous, use only secret instead.');
293+
}
294+
295+
@trigger_error('security.firewall.anonymous.key is deprecated since version 2.8 and will be removed in 3.0. Use security.firewall.anonymous.secret instead.', E_USER_DEPRECATED);
296+
297+
$v['secret'] = $v['key'];
298+
299+
unset($v['key']);
300+
})
301+
->end()
288302
->children()
289-
->scalarNode('key')->defaultValue(uniqid())->end()
303+
->scalarNode('secret')->defaultValue(uniqid())->end()
290304
->end()
291305
->end()
292306
->arrayNode('switch_user')

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RememberMeFactory.php

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public function create(ContainerBuilder $container, $id, $config, $userProvider,
3535
$authProviderId = 'security.authentication.provider.rememberme.'.$id;
3636
$container
3737
->setDefinition($authProviderId, new DefinitionDecorator('security.authentication.provider.rememberme'))
38-
->addArgument($config['key'])
38+
->addArgument($config['secret'])
3939
->addArgument($id)
4040
;
4141

@@ -56,7 +56,7 @@ public function create(ContainerBuilder $container, $id, $config, $userProvider,
5656
}
5757

5858
$rememberMeServices = $container->setDefinition($rememberMeServicesId, new DefinitionDecorator($templateId));
59-
$rememberMeServices->replaceArgument(1, $config['key']);
59+
$rememberMeServices->replaceArgument(1, $config['secret']);
6060
$rememberMeServices->replaceArgument(2, $id);
6161

6262
if (isset($config['token_provider'])) {
@@ -120,10 +120,25 @@ public function getKey()
120120
public function addConfiguration(NodeDefinition $node)
121121
{
122122
$node->fixXmlConfig('user_provider');
123-
$builder = $node->children();
123+
$builder = $node
124+
->beforeNormalization()
125+
->ifTrue(function ($v) { return isset($v['key']); })
126+
->then(function ($v) {
127+
if (isset($v['secret'])) {
128+
throw new \LogicException('Cannot set both key and secret options for remember_me, use only secret instead.');
129+
}
130+
131+
@trigger_error('remember_me.key is deprecated since version 2.8 and will be removed in 3.0. Use remember_me.secret instead.', E_USER_DEPRECATED);
132+
133+
$v['secret'] = $v['key'];
134+
135+
unset($v['key']);
136+
})
137+
->end()
138+
->children();
124139

125140
$builder
126-
->scalarNode('key')->isRequired()->cannotBeEmpty()->end()
141+
->scalarNode('secret')->isRequired()->cannotBeEmpty()->end()
127142
->scalarNode('token_provider')->end()
128143
->arrayNode('user_providers')
129144
->beforeNormalization()

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -410,15 +410,15 @@ private function createAuthenticationListeners($container, $id, $firewall, &$aut
410410
$listenerId = 'security.authentication.listener.anonymous.'.$id;
411411
$container
412412
->setDefinition($listenerId, new DefinitionDecorator('security.authentication.listener.anonymous'))
413-
->replaceArgument(1, $firewall['anonymous']['key'])
413+
->replaceArgument(1, $firewall['anonymous']['secret'])
414414
;
415415

416416
$listeners[] = new Reference($listenerId);
417417

418418
$providerId = 'security.authentication.provider.anonymous.'.$id;
419419
$container
420420
->setDefinition($providerId, new DefinitionDecorator('security.authentication.provider.anonymous'))
421-
->replaceArgument(0, $firewall['anonymous']['key'])
421+
->replaceArgument(0, $firewall['anonymous']['secret'])
422422
;
423423

424424
$authenticationProviders[] = $providerId;

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/container1.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@
7171
'x509' => true,
7272
'remote_user' => true,
7373
'logout' => true,
74-
'remember_me' => array('key' => 'TheKey'),
74+
'remember_me' => array('secret' => 'TheSecret'),
7575
),
7676
'host' => array(
7777
'pattern' => '/test',

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/remember_me_options.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
$container->loadFromExtension('security', array(
34
'providers' => array(
45
'default' => array('id' => 'foo'),
@@ -8,7 +9,7 @@
89
'main' => array(
910
'form_login' => true,
1011
'remember_me' => array(
11-
'key' => 'TheyKey',
12+
'secret' => 'TheSecret',
1213
'catch_exceptions' => false,
1314
'token_provider' => 'token_provider_id',
1415
),

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/container1.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
<x509 />
5757
<remote-user />
5858
<logout />
59-
<remember-me key="TheyKey"/>
59+
<remember-me secret="TheSecret"/>
6060
</firewall>
6161

6262
<firewall name="host" pattern="/test" host="foo\.example\.org" methods="GET,POST">

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/remember_me_options.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
</sec:providers>
1212
<sec:firewall name="main">
1313
<sec:form-login/>
14-
<sec:remember-me key="TheKey" catch-exceptions="false" token-provider="token_provider_id" />
14+
<sec:remember-me secret="TheSecret" catch-exceptions="false" token-provider="token_provider_id" />
1515
</sec:firewall>
1616
</sec:config>
1717

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/container1.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ security:
5555
remote_user: true
5656
logout: true
5757
remember_me:
58-
key: TheKey
58+
secret: TheSecret
5959
host:
6060
pattern: /test
6161
host: foo\.example\.org

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/remember_me_options.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@ security:
77
main:
88
form_login: true
99
remember_me:
10-
key: TheKey
10+
secret: TheSecret
1111
catch_exceptions: false
1212
token_provider: token_provider_id

‎src/Symfony/Component/Security/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
CHANGELOG
22
=========
33

4+
2.8.0
5+
-----
6+
7+
* deprecated `getKey()` of the `AnonymousToken`, `RememberMeToken` and `AbstractRememberMeServices` classes
8+
in favor of `getSecret()`.
9+
410
2.7.0
511
-----
612

‎src/Symfony/Component/Security/Core/Authentication/Provider/AnonymousAuthenticationProvider.php

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,22 @@
2222
*/
2323
class AnonymousAuthenticationProvider implements AuthenticationProviderInterface
2424
{
25-
private $key;
25+
/**
26+
* Used to determine if the token is created by the application
27+
* instead of a malicious client.
28+
*
29+
* @var string
30+
*/
31+
private $secret;
2632

2733
/**
2834
* Constructor.
2935
*
30-
* @param string $key The key shared with the authentication token
36+
* @param string $secret The secret shared with the AnonymousToken
3137
*/
32-
public function __construct($key)
38+
public function __construct($secret)
3339
{
34-
$this->key = $key;
40+
$this->secret = $secret;
3541
}
3642

3743
/**
@@ -43,7 +49,7 @@ public function authenticate(TokenInterface $token)
4349
return;
4450
}
4551

46-
if ($this->key !== $token->getKey()) {
52+
if ($this->secret !== $token->getSecret()) {
4753
throw new BadCredentialsException('The Token does not contain the expected key.');
4854
}
4955

‎src/Symfony/Component/Security/Core/Authentication/Provider/RememberMeAuthenticationProvider.php

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,20 @@
1919
class RememberMeAuthenticationProvider implements AuthenticationProviderInterface
2020
{
2121
private $userChecker;
22-
private $key;
22+
private $secret;
2323
private $providerKey;
2424

2525
/**
2626
* Constructor.
2727
*
2828
* @param UserCheckerInterface $userChecker An UserCheckerInterface interface
29-
* @param string $key A key
30-
* @param string $providerKey A provider key
29+
* @param string $secret A secret
30+
* @param string $providerKey A provider secret
3131
*/
32-
public function __construct(UserCheckerInterface $userChecker, $key, $providerKey)
32+
public function __construct(UserCheckerInterface $userChecker, $secret, $providerKey)
3333
{
3434
$this->userChecker = $userChecker;
35-
$this->key = $key;
35+
$this->secret = $secret;
3636
$this->providerKey = $providerKey;
3737
}
3838

@@ -45,14 +45,14 @@ public function authenticate(TokenInterface $token)
4545
return;
4646
}
4747

48-
if ($this->key !== $token->getKey()) {
49-
throw new BadCredentialsException('The presented key does not match.');
48+
if ($this->secret !== $token->getSecret()) {
49+
throw new BadCredentialsException('The presented secret does not match.');
5050
}
5151

5252
$user = $token->getUser();
5353
$this->userChecker->checkPreAuth($user);
5454

55-
$authenticatedToken = new RememberMeToken($user, $this->providerKey, $this->key);
55+
$authenticatedToken = new RememberMeToken($user, $this->providerKey, $this->secret);
5656
$authenticatedToken->setAttributes($token->getAttributes());
5757

5858
return $authenticatedToken;

0 commit comments

Comments
 (0)
0