[Csrf] component fixes

Tobion · fabpot · commit d7eb8ff64a47 · 2013-10-17T23:29:16.000+02:00
diff --git a/src/Symfony/Component/HttpFoundation/Session/Attribute/AttributeBag.php b/src/Symfony/Component/HttpFoundation/Session/Attribute/AttributeBag.php
@@ -31,7 +31,7 @@ class AttributeBag implements AttributeBagInterface, \IteratorAggregate, \Counta
     /**
      * Constructor.
      *
-     * @param string $storageKey The key used to store attributes in the session.
+     * @param string $storageKey The key used to store attributes in the session
      */
     public function __construct($storageKey = '_sf2_attributes')
     {
@@ -148,7 +148,7 @@ public function getIterator()
     /**
      * Returns the number of attributes.
      *
-     * @return int The number of attributes
+     * @return integer The number of attributes
      */
     public function count()
     {
diff --git a/src/Symfony/Component/HttpFoundation/Session/Attribute/AttributeBagInterface.php b/src/Symfony/Component/HttpFoundation/Session/Attribute/AttributeBagInterface.php
@@ -33,7 +33,7 @@ public function has($name);
      * Returns an attribute.
      *
      * @param string $name    The attribute name
-     * @param mixed  $default The default value if not found.
+     * @param mixed  $default The default value if not found
      *
      * @return mixed
      */
@@ -66,7 +66,7 @@ public function replace(array $attributes);
      *
      * @param string $name
      *
-     * @return mixed The removed value
+     * @return mixed The removed value or null when it does not exist
      */
     public function remove($name);
 }
diff --git a/src/Symfony/Component/HttpFoundation/Session/SessionInterface.php b/src/Symfony/Component/HttpFoundation/Session/SessionInterface.php
@@ -163,7 +163,7 @@ public function replace(array $attributes);
      *
      * @param string $name
      *
-     * @return mixed The removed value
+     * @return mixed The removed value or null when it does not exist
      *
      * @api
      */
diff --git a/src/Symfony/Component/Security/Csrf/CsrfToken.php b/src/Symfony/Component/Security/Csrf/CsrfToken.php
@@ -28,6 +28,12 @@ class CsrfToken
      */
     private $value;
 
+    /**
+     * Constructor.
+     *
+     * @param string $id    The token ID
+     * @param string $value The actual token value
+     */
     public function  __construct($id, $value)
     {
         $this->id = (string) $id;
@@ -57,7 +63,7 @@ public function getValue()
     /**
      * Returns the value of the CSRF token.
      *
-     * @return string The token value.
+     * @return string The token value
      */
     public function __toString()
     {
diff --git a/src/Symfony/Component/Security/Csrf/CsrfTokenManager.php b/src/Symfony/Component/Security/Csrf/CsrfTokenManager.php
@@ -37,23 +37,14 @@ class CsrfTokenManager implements CsrfTokenManagerInterface
     /**
      * Creates a new CSRF provider using PHP's native session storage.
      *
-     * @param TokenGeneratorInterface $generator The token generator
-     * @param TokenStorageInterface   $storage   The storage for storing
-     *                                           generated CSRF tokens
-     *
+     * @param TokenGeneratorInterface|null $generator The token generator
+     * @param TokenStorageInterface|null   $storage   The storage for storing
+     *                                                generated CSRF tokens
      */
     public function __construct(TokenGeneratorInterface $generator = null, TokenStorageInterface $storage = null)
     {
-        if (null === $generator) {
-            $generator = new UriSafeTokenGenerator();
-        }
-
-        if (null === $storage) {
-            $storage = new NativeSessionTokenStorage();
-        }
-
-        $this->generator = $generator;
-        $this->storage = $storage;
+        $this->generator = $generator ?: new UriSafeTokenGenerator();
+        $this->storage = $storage ?: new NativeSessionTokenStorage();
     }
 
     /**
@@ -101,6 +92,6 @@ public function isTokenValid(CsrfToken $token)
             return false;
         }
 
-        return StringUtils::equals((string) $this->storage->getToken($token->getId()), $token->getValue());
+        return StringUtils::equals($this->storage->getToken($token->getId()), $token->getValue());
     }
 }
diff --git a/src/Symfony/Component/Security/Csrf/CsrfTokenManagerInterface.php b/src/Symfony/Component/Security/Csrf/CsrfTokenManagerInterface.php
@@ -23,7 +23,8 @@ interface CsrfTokenManagerInterface
      * Returns a CSRF token for the given ID.
      *
      * If previously no token existed for the given ID, a new token is
-     * generated. Otherwise the existing token is returned.
+     * generated. Otherwise the existing token is returned (with the same value,
+     * not the same instance).
      *
      * @param string $tokenId The token ID. You may choose an arbitrary value
      *                        for the ID
@@ -51,8 +52,8 @@ public function refreshToken($tokenId);
      *
      * @param string $tokenId The token ID
      *
-     * @return Boolean Returns true if a token existed for this ID, false
-     *                 otherwise
+     * @return string|null Returns the removed token value if one existed, NULL
+     *                     otherwise
      */
     public function removeToken($tokenId);
 
diff --git a/src/Symfony/Component/Security/Csrf/Tests/CsrfTokenManagerTest.php b/src/Symfony/Component/Security/Csrf/Tests/CsrfTokenManagerTest.php
@@ -9,7 +9,7 @@
  * file that was distributed with this source code.
  */
 
-namespace Symfony\Component\Form\Tests\Extension\Csrf\CsrfProvider;
+namespace Symfony\Component\Security\Csrf\Tests;
 
 use Symfony\Component\Security\Csrf\CsrfToken;
 use Symfony\Component\Security\Csrf\CsrfTokenManager;
diff --git a/src/Symfony/Component/Security/Csrf/Tests/TokenGenerator/UriSafeTokenGeneratorTest.php b/src/Symfony/Component/Security/Csrf/Tests/TokenGenerator/UriSafeTokenGeneratorTest.php
@@ -9,7 +9,7 @@
  * file that was distributed with this source code.
  */
 
-namespace Symfony\Component\Form\Tests\Extension\Csrf\CsrfProvider\TokenGenerator;
+namespace Symfony\Component\Security\Csrf\Tests\TokenGenerator;
 
 use Symfony\Component\Security\Csrf\TokenGenerator\UriSafeTokenGenerator;
 
diff --git a/src/Symfony/Component/Security/Csrf/Tests/TokenStorage/NativeSessionTokenStorageTest.php b/src/Symfony/Component/Security/Csrf/Tests/TokenStorage/NativeSessionTokenStorageTest.php
@@ -9,7 +9,7 @@
  * file that was distributed with this source code.
  */
 
-namespace Symfony\Component\Form\Tests\Extension\Csrf\CsrfProvider;
+namespace Symfony\Component\Security\Csrf\Tests\TokenStorage;
 
 use Symfony\Component\Security\Csrf\TokenStorage\NativeSessionTokenStorage;
 
diff --git a/src/Symfony/Component/Security/Csrf/Tests/TokenStorage/SessionTokenStorageTest.php b/src/Symfony/Component/Security/Csrf/Tests/TokenStorage/SessionTokenStorageTest.php
@@ -9,7 +9,7 @@
  * file that was distributed with this source code.
  */
 
-namespace Symfony\Component\Form\Tests\Extension\Csrf\CsrfProvider;
+namespace Symfony\Component\Security\Csrf\Tests\TokenStorage;
 
 use Symfony\Component\Security\Csrf\TokenStorage\SessionTokenStorage;
 
diff --git a/src/Symfony/Component/Security/Csrf/TokenGenerator/UriSafeTokenGenerator.php b/src/Symfony/Component/Security/Csrf/TokenGenerator/UriSafeTokenGenerator.php
@@ -39,24 +39,19 @@ class UriSafeTokenGenerator implements TokenGeneratorInterface
     /**
      * Generates URI-safe CSRF tokens.
      *
-     * @param SecureRandomInterface $random  The random value generator used for
-     *                                       generating entropy
-     * @param integer               $entropy The amount of entropy collected for
-     *                                       each token (in bits)
-     *
+     * @param SecureRandomInterface|null $random  The random value generator used for
+     *                                            generating entropy
+     * @param integer                    $entropy The amount of entropy collected for
+     *                                            each token (in bits)
      */
     public function __construct(SecureRandomInterface $random = null, $entropy = 256)
     {
-        if (null === $random) {
-            $random = new SecureRandom();
-        }
-
-        $this->random = $random;
+        $this->random = $random ?: new SecureRandom();
         $this->entropy = $entropy;
     }
 
     /**
-     * {@inheritDoc}
+     * {@inheritdoc}
      */
     public function generateToken()
     {
diff --git a/src/Symfony/Component/Security/Csrf/TokenStorage/NativeSessionTokenStorage.php b/src/Symfony/Component/Security/Csrf/TokenStorage/NativeSessionTokenStorage.php
@@ -98,7 +98,7 @@ public function removeToken($tokenId)
         }
 
         $token = isset($_SESSION[$this->namespace][$tokenId])
-            ? $_SESSION[$this->namespace][$tokenId]
+            ? (string) $_SESSION[$this->namespace][$tokenId]
             : null;
 
         unset($_SESSION[$this->namespace][$tokenId]);

Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ class AttributeBag implements AttributeBagInterface, \IteratorAggregate, \Counta`
`31`	`31`	`/**`
`32`	`32`	`* Constructor.`
`33`	`33`	`*`
`34`		`- * @param string $storageKey The key used to store attributes in the session.`
	`34`	`+ * @param string $storageKey The key used to store attributes in the session`
`35`	`35`	`*/`
`36`	`36`	`public function __construct($storageKey = '_sf2_attributes')`
`37`	`37`	`{`
`@@ -148,7 +148,7 @@ public function getIterator()`
`148`	`148`	`/**`
`149`	`149`	`* Returns the number of attributes.`
`150`	`150`	`*`
`151`		`- * @return int The number of attributes`
	`151`	`+ * @return integer The number of attributes`
`152`	`152`	`*/`
`153`	`153`	`public function count()`
`154`	`154`	`{`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ public function has($name);`
`33`	`33`	`* Returns an attribute.`
`34`	`34`	`*`
`35`	`35`	`* @param string $name The attribute name`
`36`		`- * @param mixed $default The default value if not found.`
	`36`	`+ * @param mixed $default The default value if not found`
`37`	`37`	`*`
`38`	`38`	`* @return mixed`
`39`	`39`	`*/`
`@@ -66,7 +66,7 @@ public function replace(array $attributes);`
`66`	`66`	`*`
`67`	`67`	`* @param string $name`
`68`	`68`	`*`
`69`		`- * @return mixed The removed value`
	`69`	`+ * @return mixed The removed value or null when it does not exist`
`70`	`70`	`*/`
`71`	`71`	`public function remove($name);`
`72`	`72`	`}`
Original file line number	Diff line number	Diff line change
`@@ -163,7 +163,7 @@ public function replace(array $attributes);`
`163`	`163`	`*`
`164`	`164`	`* @param string $name`
`165`	`165`	`*`
`166`		`- * @return mixed The removed value`
	`166`	`+ * @return mixed The removed value or null when it does not exist`
`167`	`167`	`*`
`168`	`168`	`* @api`
`169`	`169`	`*/`
Original file line number	Diff line number	Diff line change
`@@ -37,23 +37,14 @@ class CsrfTokenManager implements CsrfTokenManagerInterface`
`37`	`37`	`/**`
`38`	`38`	`* Creates a new CSRF provider using PHP's native session storage.`
`39`	`39`	`*`
`40`		`- * @param TokenGeneratorInterface $generator The token generator`
`41`		`- * @param TokenStorageInterface $storage The storage for storing`
`42`		`- * generated CSRF tokens`
`43`		`- *`
	`40`	`+ * @param TokenGeneratorInterface\|null $generator The token generator`
	`41`	`+ * @param TokenStorageInterface\|null $storage The storage for storing`
	`42`	`+ * generated CSRF tokens`
`44`	`43`	`*/`
`45`	`44`	`public function __construct(TokenGeneratorInterface $generator = null, TokenStorageInterface $storage = null)`
`46`	`45`	`{`
`47`		`- if (null === $generator) {`
`48`		`- $generator = new UriSafeTokenGenerator();`
`49`		`- }`
`50`		`-`
`51`		`- if (null === $storage) {`
`52`		`- $storage = new NativeSessionTokenStorage();`
`53`		`- }`
`54`		`-`
`55`		`- $this->generator = $generator;`
`56`		`- $this->storage = $storage;`
	`46`	`+ $this->generator = $generator ?: new UriSafeTokenGenerator();`
	`47`	`+ $this->storage = $storage ?: new NativeSessionTokenStorage();`
`57`	`48`	`}`
`58`	`49`
`59`	`50`	`/**`
`@@ -101,6 +92,6 @@ public function isTokenValid(CsrfToken $token)`
`101`	`92`	`return false;`
`102`	`93`	`}`
`103`	`94`
`104`		`- return StringUtils::equals((string) $this->storage->getToken($token->getId()), $token->getValue());`
	`95`	`+ return StringUtils::equals($this->storage->getToken($token->getId()), $token->getValue());`
`105`	`96`	`}`
`106`	`97`	`}`
Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,8 @@ interface CsrfTokenManagerInterface`
`23`	`23`	`* Returns a CSRF token for the given ID.`
`24`	`24`	`*`
`25`	`25`	`* If previously no token existed for the given ID, a new token is`
`26`		`- * generated. Otherwise the existing token is returned.`
	`26`	`+ * generated. Otherwise the existing token is returned (with the same value,`
	`27`	`+ * not the same instance).`
`27`	`28`	`*`
`28`	`29`	`* @param string $tokenId The token ID. You may choose an arbitrary value`
`29`	`30`	`* for the ID`
`@@ -51,8 +52,8 @@ public function refreshToken($tokenId);`
`51`	`52`	`*`
`52`	`53`	`* @param string $tokenId The token ID`
`53`	`54`	`*`
`54`		`- * @return Boolean Returns true if a token existed for this ID, false`
`55`		`- * otherwise`
	`55`	`+ * @return string\|null Returns the removed token value if one existed, NULL`
	`56`	`+ * otherwise`
`56`	`57`	`*/`
`57`	`58`	`public function removeToken($tokenId);`
`58`	`59`