[Csrf] component fixes

symfony · Oct 17, 2013 · d7eb8ff · d7eb8ff
1 parent 4c164ca
commit d7eb8ff
Show file tree

Hide file tree

Showing 12 changed files with 33 additions and 40 deletions.
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]);