summaryrefslogtreecommitdiffstats
path: root/Http
diff options
context:
space:
mode:
authorFabien Potencier <fabien.potencier@gmail.com>2015-07-02 01:04:20 +0200
committerFabien Potencier <fabien.potencier@gmail.com>2015-07-02 01:04:20 +0200
commit2a1a7a58fbecdb00d8a2546252e5e276f2a1d53c (patch)
treef9621f4460fedef469f8d37e90f6d20dbef7371a /Http
parent93bccca789d9afdc88ec04bfae28a12e7ded3837 (diff)
parentcba12eee6f9d829e7c2307982513d01b99830b83 (diff)
downloadsymfony-security-2a1a7a58fbecdb00d8a2546252e5e276f2a1d53c.zip
symfony-security-2a1a7a58fbecdb00d8a2546252e5e276f2a1d53c.tar.gz
symfony-security-2a1a7a58fbecdb00d8a2546252e5e276f2a1d53c.tar.bz2
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()
Diffstat (limited to 'Http')
-rw-r--r--Http/RememberMe/AbstractRememberMeServices.php28
-rw-r--r--Http/RememberMe/PersistentTokenBasedRememberMeServices.php6
-rw-r--r--Http/RememberMe/TokenBasedRememberMeServices.php2
-rw-r--r--Http/Tests/Firewall/AnonymousAuthenticationListenerTest.php10
-rw-r--r--Http/Tests/RememberMe/AbstractRememberMeServicesTest.php8
-rw-r--r--Http/Tests/RememberMe/PersistentTokenBasedRememberMeServicesTest.php4
-rw-r--r--Http/Tests/RememberMe/TokenBasedRememberMeServicesTest.php4
-rw-r--r--Http/composer.json2
8 files changed, 37 insertions, 27 deletions
diff --git a/Http/RememberMe/AbstractRememberMeServices.php b/Http/RememberMe/AbstractRememberMeServices.php
index 3673ff1..16810bd 100644
--- a/Http/RememberMe/AbstractRememberMeServices.php
+++ b/Http/RememberMe/AbstractRememberMeServices.php
@@ -36,24 +36,24 @@ abstract class AbstractRememberMeServices implements RememberMeServicesInterface
protected $logger;
protected $options;
private $providerKey;
- private $key;
+ private $secret;
private $userProviders;
/**
* Constructor.
*
* @param array $userProviders
- * @param string $key
+ * @param string $secret
* @param string $providerKey
* @param array $options
* @param LoggerInterface $logger
*
* @throws \InvalidArgumentException
*/
- public function __construct(array $userProviders, $key, $providerKey, array $options = array(), LoggerInterface $logger = null)
+ public function __construct(array $userProviders, $secret, $providerKey, array $options = array(), LoggerInterface $logger = null)
{
- if (empty($key)) {
- throw new \InvalidArgumentException('$key must not be empty.');
+ if (empty($secret)) {
+ throw new \InvalidArgumentException('$secret must not be empty.');
}
if (empty($providerKey)) {
throw new \InvalidArgumentException('$providerKey must not be empty.');
@@ -63,7 +63,7 @@ abstract class AbstractRememberMeServices implements RememberMeServicesInterface
}
$this->userProviders = $userProviders;
- $this->key = $key;
+ $this->secret = $secret;
$this->providerKey = $providerKey;
$this->options = $options;
$this->logger = $logger;
@@ -81,11 +81,21 @@ abstract class AbstractRememberMeServices implements RememberMeServicesInterface
}
/**
- * @return string
+ * @deprecated Since version 2.8, to be removed in 3.0. Use getSecret() instead.
*/
public function getKey()
{
- return $this->key;
+ @trigger_error(__method__.'() is deprecated since version 2.8 and will be removed in 3.0. Use getSecret() instead.', E_USER_DEPRECATED);
+
+ return $this->getSecret();
+ }
+
+ /**
+ * @return string
+ */
+ public function getSecret()
+ {
+ return $this->secret;
}
/**
@@ -122,7 +132,7 @@ abstract class AbstractRememberMeServices implements RememberMeServicesInterface
$this->logger->info('Remember-me cookie accepted.');
}
- return new RememberMeToken($user, $this->providerKey, $this->key);
+ return new RememberMeToken($user, $this->providerKey, $this->secret);
} catch (CookieTheftException $e) {
$this->cancelCookie($request);
diff --git a/Http/RememberMe/PersistentTokenBasedRememberMeServices.php b/Http/RememberMe/PersistentTokenBasedRememberMeServices.php
index 4fb7e09..3e465d6 100644
--- a/Http/RememberMe/PersistentTokenBasedRememberMeServices.php
+++ b/Http/RememberMe/PersistentTokenBasedRememberMeServices.php
@@ -38,15 +38,15 @@ class PersistentTokenBasedRememberMeServices extends AbstractRememberMeServices
* Constructor.
*
* @param array $userProviders
- * @param string $key
+ * @param string $secret
* @param string $providerKey
* @param array $options
* @param LoggerInterface $logger
* @param SecureRandomInterface $secureRandom
*/
- public function __construct(array $userProviders, $key, $providerKey, array $options = array(), LoggerInterface $logger = null, SecureRandomInterface $secureRandom)
+ public function __construct(array $userProviders, $secret, $providerKey, array $options = array(), LoggerInterface $logger = null, SecureRandomInterface $secureRandom)
{
- parent::__construct($userProviders, $key, $providerKey, $options, $logger);
+ parent::__construct($userProviders, $secret, $providerKey, $options, $logger);
$this->secureRandom = $secureRandom;
}
diff --git a/Http/RememberMe/TokenBasedRememberMeServices.php b/Http/RememberMe/TokenBasedRememberMeServices.php
index d68ada5..f6107ec 100644
--- a/Http/RememberMe/TokenBasedRememberMeServices.php
+++ b/Http/RememberMe/TokenBasedRememberMeServices.php
@@ -121,6 +121,6 @@ class TokenBasedRememberMeServices extends AbstractRememberMeServices
*/
protected function generateCookieHash($class, $username, $expires, $password)
{
- return hash_hmac('sha256', $class.$username.$expires.$password, $this->getKey());
+ return hash_hmac('sha256', $class.$username.$expires.$password, $this->getSecret());
}
}
diff --git a/Http/Tests/Firewall/AnonymousAuthenticationListenerTest.php b/Http/Tests/Firewall/AnonymousAuthenticationListenerTest.php
index dcd672b..6188962 100644
--- a/Http/Tests/Firewall/AnonymousAuthenticationListenerTest.php
+++ b/Http/Tests/Firewall/AnonymousAuthenticationListenerTest.php
@@ -35,7 +35,7 @@ class AnonymousAuthenticationListenerTest extends \PHPUnit_Framework_TestCase
->method('authenticate')
;
- $listener = new AnonymousAuthenticationListener($tokenStorage, 'TheKey', null, $authenticationManager);
+ $listener = new AnonymousAuthenticationListener($tokenStorage, 'TheSecret', null, $authenticationManager);
$listener->handle($this->getMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false));
}
@@ -48,7 +48,7 @@ class AnonymousAuthenticationListenerTest extends \PHPUnit_Framework_TestCase
->will($this->returnValue(null))
;
- $anonymousToken = new AnonymousToken('TheKey', 'anon.', array());
+ $anonymousToken = new AnonymousToken('TheSecret', 'anon.', array());
$authenticationManager = $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface');
$authenticationManager
@@ -56,7 +56,7 @@ class AnonymousAuthenticationListenerTest extends \PHPUnit_Framework_TestCase
->method('authenticate')
->with(self::logicalAnd(
$this->isInstanceOf('Symfony\Component\Security\Core\Authentication\Token\AnonymousToken'),
- $this->attributeEqualTo('key', 'TheKey')
+ $this->attributeEqualTo('secret', 'TheSecret')
))
->will($this->returnValue($anonymousToken))
;
@@ -67,7 +67,7 @@ class AnonymousAuthenticationListenerTest extends \PHPUnit_Framework_TestCase
->with($anonymousToken)
;
- $listener = new AnonymousAuthenticationListener($tokenStorage, 'TheKey', null, $authenticationManager);
+ $listener = new AnonymousAuthenticationListener($tokenStorage, 'TheSecret', null, $authenticationManager);
$listener->handle($this->getMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false));
}
@@ -82,7 +82,7 @@ class AnonymousAuthenticationListenerTest extends \PHPUnit_Framework_TestCase
$authenticationManager = $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface');
- $listener = new AnonymousAuthenticationListener($tokenStorage, 'TheKey', $logger, $authenticationManager);
+ $listener = new AnonymousAuthenticationListener($tokenStorage, 'TheSecret', $logger, $authenticationManager);
$listener->handle($this->getMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false));
}
}
diff --git a/Http/Tests/RememberMe/AbstractRememberMeServicesTest.php b/Http/Tests/RememberMe/AbstractRememberMeServicesTest.php
index 2225b6c..5a6a839 100644
--- a/Http/Tests/RememberMe/AbstractRememberMeServicesTest.php
+++ b/Http/Tests/RememberMe/AbstractRememberMeServicesTest.php
@@ -25,10 +25,10 @@ class AbstractRememberMeServicesTest extends \PHPUnit_Framework_TestCase
$this->assertEquals('foo', $service->getRememberMeParameter());
}
- public function testGetKey()
+ public function testGetSecret()
{
$service = $this->getService();
- $this->assertEquals('fookey', $service->getKey());
+ $this->assertEquals('foosecret', $service->getSecret());
}
public function testAutoLoginReturnsNullWhenNoCookie()
@@ -78,7 +78,7 @@ class AbstractRememberMeServicesTest extends \PHPUnit_Framework_TestCase
$returnedToken = $service->autoLogin($request);
$this->assertSame($user, $returnedToken->getUser());
- $this->assertSame('fookey', $returnedToken->getKey());
+ $this->assertSame('foosecret', $returnedToken->getSecret());
$this->assertSame('fookey', $returnedToken->getProviderKey());
}
@@ -268,7 +268,7 @@ class AbstractRememberMeServicesTest extends \PHPUnit_Framework_TestCase
}
return $this->getMockForAbstractClass('Symfony\Component\Security\Http\RememberMe\AbstractRememberMeServices', array(
- array($userProvider), 'fookey', 'fookey', $options, $logger,
+ array($userProvider), 'foosecret', 'fookey', $options, $logger,
));
}
diff --git a/Http/Tests/RememberMe/PersistentTokenBasedRememberMeServicesTest.php b/Http/Tests/RememberMe/PersistentTokenBasedRememberMeServicesTest.php
index 2fea626..889211c 100644
--- a/Http/Tests/RememberMe/PersistentTokenBasedRememberMeServicesTest.php
+++ b/Http/Tests/RememberMe/PersistentTokenBasedRememberMeServicesTest.php
@@ -174,7 +174,7 @@ class PersistentTokenBasedRememberMeServicesTest extends \PHPUnit_Framework_Test
$this->assertInstanceOf('Symfony\Component\Security\Core\Authentication\Token\RememberMeToken', $returnedToken);
$this->assertSame($user, $returnedToken->getUser());
- $this->assertEquals('fookey', $returnedToken->getKey());
+ $this->assertEquals('foosecret', $returnedToken->getSecret());
$this->assertTrue($request->attributes->has(RememberMeServicesInterface::COOKIE_ATTR_NAME));
}
@@ -311,7 +311,7 @@ class PersistentTokenBasedRememberMeServicesTest extends \PHPUnit_Framework_Test
$userProvider = $this->getProvider();
}
- return new PersistentTokenBasedRememberMeServices(array($userProvider), 'fookey', 'fookey', $options, $logger, new SecureRandom(sys_get_temp_dir().'/_sf2.seed'));
+ return new PersistentTokenBasedRememberMeServices(array($userProvider), 'foosecret', 'fookey', $options, $logger, new SecureRandom(sys_get_temp_dir().'/_sf2.seed'));
}
protected function getProvider()
diff --git a/Http/Tests/RememberMe/TokenBasedRememberMeServicesTest.php b/Http/Tests/RememberMe/TokenBasedRememberMeServicesTest.php
index 8383cec..2a892c3 100644
--- a/Http/Tests/RememberMe/TokenBasedRememberMeServicesTest.php
+++ b/Http/Tests/RememberMe/TokenBasedRememberMeServicesTest.php
@@ -140,7 +140,7 @@ class TokenBasedRememberMeServicesTest extends \PHPUnit_Framework_TestCase
$this->assertInstanceOf('Symfony\Component\Security\Core\Authentication\Token\RememberMeToken', $returnedToken);
$this->assertSame($user, $returnedToken->getUser());
- $this->assertEquals('fookey', $returnedToken->getKey());
+ $this->assertEquals('foosecret', $returnedToken->getSecret());
}
public function provideUsernamesForAutoLogin()
@@ -264,7 +264,7 @@ class TokenBasedRememberMeServicesTest extends \PHPUnit_Framework_TestCase
$userProvider = $this->getProvider();
}
- $service = new TokenBasedRememberMeServices(array($userProvider), 'fookey', 'fookey', $options, $logger);
+ $service = new TokenBasedRememberMeServices(array($userProvider), 'foosecret', 'fookey', $options, $logger);
return $service;
}
diff --git a/Http/composer.json b/Http/composer.json
index 1c49504..98bd8cd 100644
--- a/Http/composer.json
+++ b/Http/composer.json
@@ -17,7 +17,7 @@
],
"require": {
"php": ">=5.3.9",
- "symfony/security-core": "~2.6|~3.0.0",
+ "symfony/security-core": "~2.8|~3.0.0",
"symfony/event-dispatcher": "~2.1|~3.0.0",
"symfony/http-foundation": "~2.4|~3.0.0",
"symfony/http-kernel": "~2.4|~3.0.0"