diff --git a/CHANGELOG-2.1.md b/CHANGELOG-2.1.md index 39e668c405..ca7edd952a 100644 --- a/CHANGELOG-2.1.md +++ b/CHANGELOG-2.1.md @@ -269,6 +269,7 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c don't receive an options array anymore. * Deprecated FormValidatorInterface and substituted its implementations by event subscribers + * simplified CSRF protection and removed the csrf type ### HttpFoundation diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/form_csrf.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/form_csrf.xml index 188a099df2..72442bcbba 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/form_csrf.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/form_csrf.xml @@ -14,12 +14,9 @@ %kernel.secret% - - - - + %form.type_extension.csrf.enabled% %form.type_extension.csrf.field_name% diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 859ec489ab..2c7204e735 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -27,9 +27,9 @@ abstract class FrameworkExtensionTest extends TestCase $def = $container->getDefinition('form.type_extension.csrf'); $this->assertTrue($container->getParameter('form.type_extension.csrf.enabled')); - $this->assertEquals('%form.type_extension.csrf.enabled%', $def->getArgument(0)); + $this->assertEquals('%form.type_extension.csrf.enabled%', $def->getArgument(1)); $this->assertEquals('_csrf', $container->getParameter('form.type_extension.csrf.field_name')); - $this->assertEquals('%form.type_extension.csrf.field_name%', $def->getArgument(1)); + $this->assertEquals('%form.type_extension.csrf.field_name%', $def->getArgument(2)); $this->assertEquals('s3cr3t', $container->getParameterBag()->resolveValue($container->findDefinition('form.csrf_provider')->getArgument(1))); } diff --git a/src/Symfony/Component/Form/Extension/Csrf/CsrfExtension.php b/src/Symfony/Component/Form/Extension/Csrf/CsrfExtension.php index 42505e2b76..5330bbda45 100644 --- a/src/Symfony/Component/Form/Extension/Csrf/CsrfExtension.php +++ b/src/Symfony/Component/Form/Extension/Csrf/CsrfExtension.php @@ -32,27 +32,13 @@ class CsrfExtension extends AbstractExtension $this->csrfProvider = $csrfProvider; } - /** - * {@inheritDoc} - */ - protected function loadTypes() - { - return array( - new Type\CsrfType($this->csrfProvider), - ); - } - /** * {@inheritDoc} */ protected function loadTypeExtensions() { return array( - new Type\ChoiceTypeCsrfExtension(), - new Type\DateTypeCsrfExtension(), - new Type\FormTypeCsrfExtension(), - new Type\RepeatedTypeCsrfExtension(), - new Type\TimeTypeCsrfExtension(), + new Type\FormTypeCsrfExtension($this->csrfProvider), ); } } diff --git a/src/Symfony/Component/Form/Extension/Csrf/EventListener/CsrfValidationListener.php b/src/Symfony/Component/Form/Extension/Csrf/EventListener/CsrfValidationListener.php index 3de52ab867..dd74d87b2e 100644 --- a/src/Symfony/Component/Form/Extension/Csrf/EventListener/CsrfValidationListener.php +++ b/src/Symfony/Component/Form/Extension/Csrf/EventListener/CsrfValidationListener.php @@ -14,7 +14,7 @@ namespace Symfony\Component\Form\Extension\Csrf\EventListener; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\Form\FormEvents; use Symfony\Component\Form\FormError; -use Symfony\Component\Form\Event\DataEvent; +use Symfony\Component\Form\Event\FilterDataEvent; use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface; /** @@ -22,6 +22,12 @@ use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface; */ class CsrfValidationListener implements EventSubscriberInterface { + /** + * The name of the CSRF field + * @var string + */ + private $fieldName; + /** * The provider for generating and validating CSRF tokens * @var CsrfProviderInterface @@ -45,24 +51,26 @@ class CsrfValidationListener implements EventSubscriberInterface ); } - public function __construct(CsrfProviderInterface $csrfProvider, $intention) + public function __construct($fieldName, CsrfProviderInterface $csrfProvider, $intention) { + $this->fieldName = $fieldName; $this->csrfProvider = $csrfProvider; $this->intention = $intention; } - public function onBindClientData(DataEvent $event) + public function onBindClientData(FilterDataEvent $event) { $form = $event->getForm(); $data = $event->getData(); - if ((!$form->hasParent() || $form->getParent()->isRoot()) - && !$this->csrfProvider->isCsrfTokenValid($this->intention, $data)) { - $form->addError(new FormError('The CSRF token is invalid. Please try to resubmit the form')); + if ($form->isRoot() && $form->hasChildren() && isset($data[$this->fieldName])) { + if (!$this->csrfProvider->isCsrfTokenValid($this->intention, $data[$this->fieldName])) { + $form->addError(new FormError('The CSRF token is invalid. Please try to resubmit the form')); + } - // If the session timed out, the token is invalid now. - // Regenerate the token so that a resubmission is possible. - $event->setData($this->csrfProvider->generateCsrfToken($this->intention)); + unset($data[$this->fieldName]); } + + $event->setData($data); } } diff --git a/src/Symfony/Component/Form/Extension/Csrf/EventListener/EnsureCsrfFieldListener.php b/src/Symfony/Component/Form/Extension/Csrf/EventListener/EnsureCsrfFieldListener.php deleted file mode 100644 index 480d67c323..0000000000 --- a/src/Symfony/Component/Form/Extension/Csrf/EventListener/EnsureCsrfFieldListener.php +++ /dev/null @@ -1,66 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Form\Extension\Csrf\EventListener; - -use Symfony\Component\Form\Event\DataEvent; -use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface; -use Symfony\Component\Form\FormFactoryInterface; - -/** - * Ensures the CSRF field. - * - * @author Bulat Shakirzyanov - * @author Kris Wallsmith - */ -class EnsureCsrfFieldListener -{ - private $factory; - private $name; - private $intention; - private $provider; - - /** - * Constructor. - * - * @param FormFactoryInterface $factory The form factory - * @param string $name A name for the CSRF field - * @param string $intention The intention string - * @param CsrfProviderInterface $provider The CSRF provider - */ - public function __construct(FormFactoryInterface $factory, $name, $intention = null, CsrfProviderInterface $provider = null) - { - $this->factory = $factory; - $this->name = $name; - $this->intention = $intention; - $this->provider = $provider; - } - - /** - * Ensures a root form has a CSRF field. - * - * This method should be connected to both form.pre_set_data and form.pre_bind. - */ - public function ensureCsrfField(DataEvent $event) - { - $form = $event->getForm(); - - $options = array(); - if ($this->intention) { - $options['intention'] = $this->intention; - } - if ($this->provider) { - $options['csrf_provider'] = $this->provider; - } - - $form->add($this->factory->createNamed('csrf', $this->name, null, $options)); - } -} diff --git a/src/Symfony/Component/Form/Extension/Csrf/Type/ChoiceTypeCsrfExtension.php b/src/Symfony/Component/Form/Extension/Csrf/Type/ChoiceTypeCsrfExtension.php deleted file mode 100644 index dc1e6db15b..0000000000 --- a/src/Symfony/Component/Form/Extension/Csrf/Type/ChoiceTypeCsrfExtension.php +++ /dev/null @@ -1,27 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Form\Extension\Csrf\Type; - -use Symfony\Component\Form\AbstractTypeExtension; - -class ChoiceTypeCsrfExtension extends AbstractTypeExtension -{ - public function getDefaultOptions() - { - return array('csrf_protection' => false); - } - - public function getExtendedType() - { - return 'choice'; - } -} diff --git a/src/Symfony/Component/Form/Extension/Csrf/Type/CsrfType.php b/src/Symfony/Component/Form/Extension/Csrf/Type/CsrfType.php deleted file mode 100644 index 97c69d8a5a..0000000000 --- a/src/Symfony/Component/Form/Extension/Csrf/Type/CsrfType.php +++ /dev/null @@ -1,83 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Form\Extension\Csrf\Type; - - -use Symfony\Component\Form\AbstractType; -use Symfony\Component\Form\FormBuilder; -use Symfony\Component\Form\Extension\Csrf\EventListener\CsrfValidationListener; -use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface; - -class CsrfType extends AbstractType -{ - private $csrfProvider; - - /** - * Constructor. - * - * @param CsrfProviderInterface $csrfProvider The provider to use to generate the token - */ - public function __construct(CsrfProviderInterface $csrfProvider) - { - $this->csrfProvider = $csrfProvider; - } - - /** - * Builds the CSRF field. - * - * A validator is added to check the token value when the CSRF field is added to - * a root form - * - * @param FormBuilder $builder The form builder - * @param array $options The options - */ - public function buildForm(FormBuilder $builder, array $options) - { - $csrfProvider = $options['csrf_provider']; - $intention = $options['intention']; - - $builder - ->setData($csrfProvider->generateCsrfToken($intention)) - ->addEventSubscriber(new CsrfValidationListener($csrfProvider, $intention)) - ; - } - - /** - * {@inheritDoc} - */ - public function getDefaultOptions() - { - return array( - 'csrf_provider' => $this->csrfProvider, - 'intention' => null, - 'property_path' => false, - ); - } - - /** - * {@inheritDoc} - */ - public function getParent(array $options) - { - return 'hidden'; - } - - /** - * Returns the name of this form. - * - * @return string 'csrf' - */ - public function getName() - { - return 'csrf'; - } -} diff --git a/src/Symfony/Component/Form/Extension/Csrf/Type/DateTypeCsrfExtension.php b/src/Symfony/Component/Form/Extension/Csrf/Type/DateTypeCsrfExtension.php deleted file mode 100644 index dc54e94ddf..0000000000 --- a/src/Symfony/Component/Form/Extension/Csrf/Type/DateTypeCsrfExtension.php +++ /dev/null @@ -1,27 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Form\Extension\Csrf\Type; - -use Symfony\Component\Form\AbstractTypeExtension; - -class DateTypeCsrfExtension extends AbstractTypeExtension -{ - public function getDefaultOptions() - { - return array('csrf_protection' => false); - } - - public function getExtendedType() - { - return 'date'; - } -} diff --git a/src/Symfony/Component/Form/Extension/Csrf/Type/FormTypeCsrfExtension.php b/src/Symfony/Component/Form/Extension/Csrf/Type/FormTypeCsrfExtension.php index 8e2a57459a..c8687d4af4 100644 --- a/src/Symfony/Component/Form/Extension/Csrf/Type/FormTypeCsrfExtension.php +++ b/src/Symfony/Component/Form/Extension/Csrf/Type/FormTypeCsrfExtension.php @@ -12,21 +12,27 @@ namespace Symfony\Component\Form\Extension\Csrf\Type; use Symfony\Component\Form\AbstractTypeExtension; -use Symfony\Component\Form\Extension\Csrf\EventListener\EnsureCsrfFieldListener; +use Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface; +use Symfony\Component\Form\Extension\Csrf\EventListener\CsrfValidationListener; use Symfony\Component\Form\FormBuilder; use Symfony\Component\Form\FormView; use Symfony\Component\Form\FormEvents; use Symfony\Component\Form\FormInterface; +/** + * @author Bernhard Schussek + */ class FormTypeCsrfExtension extends AbstractTypeExtension { - private $enabled; - private $fieldName; + private $defaultCsrfProvider; + private $defaultEnabled; + private $defaultFieldName; - public function __construct($enabled = true, $fieldName = '_token') + public function __construct(CsrfProviderInterface $defaultCsrfProvider, $defaultEnabled = true, $defaultFieldName = '_token') { - $this->enabled = $enabled; - $this->fieldName = $fieldName; + $this->defaultCsrfProvider = $defaultCsrfProvider; + $this->defaultEnabled = $defaultEnabled; + $this->defaultFieldName = $defaultFieldName; } /** @@ -41,35 +47,35 @@ class FormTypeCsrfExtension extends AbstractTypeExtension return; } - $listener = new EnsureCsrfFieldListener( - $builder->getFormFactory(), - $options['csrf_field_name'], - $options['intention'], - $options['csrf_provider'] - ); - // use a low priority so higher priority listeners don't remove the field $builder ->setAttribute('csrf_field_name', $options['csrf_field_name']) - ->addEventListener(FormEvents::PRE_SET_DATA, array($listener, 'ensureCsrfField'), -10) - ->addEventListener(FormEvents::PRE_BIND, array($listener, 'ensureCsrfField'), -10) + ->setAttribute('csrf_provider', $options['csrf_provider']) + ->setAttribute('csrf_intention', $options['intention']) + ->setAttribute('csrf_factory', $builder->getFormFactory()) + ->addEventSubscriber(new CsrfValidationListener($options['csrf_field_name'], $options['csrf_provider'], $options['intention'])) ; } /** - * Removes CSRF fields from all the form views except the root one. + * Adds a CSRF field to the root form view. * * @param FormView $view The form view * @param FormInterface $form The form */ - public function buildViewBottomUp(FormView $view, FormInterface $form) + public function buildView(FormView $view, FormInterface $form) { - if ($view->hasParent() && $form->hasAttribute('csrf_field_name')) { + if ($form->isRoot() && $form->hasChildren() && $form->hasAttribute('csrf_field_name')) { $name = $form->getAttribute('csrf_field_name'); + $csrfProvider = $form->getAttribute('csrf_provider'); + $intention = $form->getAttribute('csrf_intention'); + $factory = $form->getAttribute('csrf_factory'); + $data = $csrfProvider->generateCsrfToken($intention); + $csrfForm = $factory->createNamed('hidden', $name, $data, array( + 'property_path' => false, + )); - if (isset($view[$name])) { - unset($view[$name]); - } + $view->addChild($csrfForm->createView($view)); } } @@ -79,9 +85,9 @@ class FormTypeCsrfExtension extends AbstractTypeExtension public function getDefaultOptions() { return array( - 'csrf_protection' => $this->enabled, - 'csrf_field_name' => $this->fieldName, - 'csrf_provider' => null, + 'csrf_protection' => $this->defaultEnabled, + 'csrf_field_name' => $this->defaultFieldName, + 'csrf_provider' => $this->defaultCsrfProvider, 'intention' => 'unknown', ); } diff --git a/src/Symfony/Component/Form/Extension/Csrf/Type/RepeatedTypeCsrfExtension.php b/src/Symfony/Component/Form/Extension/Csrf/Type/RepeatedTypeCsrfExtension.php deleted file mode 100644 index 1115ea4d69..0000000000 --- a/src/Symfony/Component/Form/Extension/Csrf/Type/RepeatedTypeCsrfExtension.php +++ /dev/null @@ -1,27 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Form\Extension\Csrf\Type; - -use Symfony\Component\Form\AbstractTypeExtension; - -class RepeatedTypeCsrfExtension extends AbstractTypeExtension -{ - public function getDefaultOptions() - { - return array('csrf_protection' => false); - } - - public function getExtendedType() - { - return 'repeated'; - } -} diff --git a/src/Symfony/Component/Form/Extension/Csrf/Type/TimeTypeCsrfExtension.php b/src/Symfony/Component/Form/Extension/Csrf/Type/TimeTypeCsrfExtension.php deleted file mode 100644 index dbd7c0d2df..0000000000 --- a/src/Symfony/Component/Form/Extension/Csrf/Type/TimeTypeCsrfExtension.php +++ /dev/null @@ -1,27 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Form\Extension\Csrf\Type; - -use Symfony\Component\Form\AbstractTypeExtension; - -class TimeTypeCsrfExtension extends AbstractTypeExtension -{ - public function getDefaultOptions() - { - return array('csrf_protection' => false); - } - - public function getExtendedType() - { - return 'time'; - } -} diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php index 8f9e7b6de2..4798c7e5cd 100644 --- a/src/Symfony/Component/Form/Form.php +++ b/src/Symfony/Component/Form/Form.php @@ -975,7 +975,7 @@ class Form implements \IteratorAggregate, FormInterface $parent = $this->parent->createView(); } - $view = new FormView(); + $view = new FormView($this->name); $view->setParent($parent); @@ -989,14 +989,10 @@ class Form implements \IteratorAggregate, FormInterface } } - $childViews = array(); - - foreach ($this->children as $key => $child) { - $childViews[$key] = $child->createView($view); + foreach ($this->children as $child) { + $view->addChild($child->createView($view)); } - $view->setChildren($childViews); - foreach ($types as $type) { $type->buildViewBottomUp($view, $this); diff --git a/src/Symfony/Component/Form/FormView.php b/src/Symfony/Component/Form/FormView.php index 5c8ab13da1..5d88346edc 100644 --- a/src/Symfony/Component/Form/FormView.php +++ b/src/Symfony/Component/Form/FormView.php @@ -11,8 +11,17 @@ namespace Symfony\Component\Form; -class FormView implements \ArrayAccess, \IteratorAggregate, \Countable +use ArrayAccess; +use IteratorAggregate; +use Countable; + +/** + * @author Bernhard Schussek + */ +class FormView implements ArrayAccess, IteratorAggregate, Countable { + private $name; + private $vars = array( 'value' => null, 'attr' => array(), @@ -33,6 +42,16 @@ class FormView implements \ArrayAccess, \IteratorAggregate, \Countable */ private $rendered = false; + public function __construct($name) + { + $this->name = $name; + } + + public function getName() + { + return $this->name; + } + /** * @param string $name * @param mixed $value @@ -177,15 +196,29 @@ class FormView implements \ArrayAccess, \IteratorAggregate, \Countable } /** - * Sets the children view. + * Adds a child view. * - * @param array $children The children as instances of FormView + * @param FormView $child The child view to add. * * @return FormView The current view */ - public function setChildren(array $children) + public function addChild(FormView $child) { - $this->children = $children; + $this->children[$child->getName()] = $child; + + return $this; + } + + /** + * Removes a child view. + * + * @param string $name The name of the removed child view. + * + * @return FormView The current view + */ + public function removeChild($name) + { + unset($this->children[$name]); return $this; } @@ -222,6 +255,18 @@ class FormView implements \ArrayAccess, \IteratorAggregate, \Countable return count($this->children) > 0; } + /** + * Returns whether this view has a given child. + * + * @param string $name The name of the child + * + * @return Boolean Whether the child with the given name exists + */ + public function hasChild($name) + { + return isset($this->children[$name]); + } + /** * Returns a child by name (implements \ArrayAccess). * diff --git a/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php index d22113245a..986f99b7b4 100644 --- a/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php @@ -363,6 +363,31 @@ abstract class AbstractDivLayoutTest extends AbstractLayoutTest ); } + public function testCsrf() + { + $this->csrfProvider->expects($this->any()) + ->method('generateCsrfToken') + ->will($this->returnValue('foo&bar')); + + $form = $this->factory->createNamedBuilder('form', 'name') + ->add($this->factory + // No CSRF protection on nested forms + ->createNamedBuilder('form', 'child') + ->add($this->factory->createNamedBuilder('text', 'grandchild')) + ) + ->getForm(); + + $this->assertWidgetMatchesXpath($form->createView(), array(), +'/div + [ + ./input[@type="hidden"][@id="name__token"][@value="foo&bar"] + /following-sibling::div + ] + [count(.//input[@type="hidden"])=1] +' + ); + } + public function testRepeated() { $form = $this->factory->createNamed('repeated', 'name', 'foobar', array( @@ -372,7 +397,8 @@ abstract class AbstractDivLayoutTest extends AbstractLayoutTest $this->assertWidgetMatchesXpath($form->createView(), array(), '/div [ - ./div + ./input[@type="hidden"][@id="name__token"] + /following-sibling::div [ ./label[@for="name_first"] /following-sibling::input[@type="text"][@id="name_first"] @@ -383,7 +409,7 @@ abstract class AbstractDivLayoutTest extends AbstractLayoutTest /following-sibling::input[@type="text"][@id="name_second"] ] ] - [count(.//input)=2] + [count(.//input)=3] ' ); } @@ -399,7 +425,8 @@ abstract class AbstractDivLayoutTest extends AbstractLayoutTest $this->assertWidgetMatchesXpath($form->createView(), array(), '/div [ - ./div + ./input[@type="hidden"][@id="name__token"] + /following-sibling::div [ ./label[@for="name_first"][.="[trans]Test[/trans]"] /following-sibling::input[@type="text"][@id="name_first"][@required="required"] @@ -410,7 +437,7 @@ abstract class AbstractDivLayoutTest extends AbstractLayoutTest /following-sibling::input[@type="text"][@id="name_second"][@required="required"] ] ] - [count(.//input)=2] + [count(.//input)=3] ' ); } diff --git a/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php index 32aabb86a6..5d48b9921c 100644 --- a/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php @@ -646,12 +646,13 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase $this->assertWidgetMatchesXpath($form->createView(), array(), '/div [ - ./input[@type="radio"][@name="name"][@id="name_0"][@value="&a"][@checked] + ./input[@type="hidden"][@id="name__token"] + /following-sibling::input[@type="radio"][@name="name"][@id="name_0"][@value="&a"][@checked] /following-sibling::label[@for="name_0"][.="[trans]Choice&A[/trans]"] /following-sibling::input[@type="radio"][@name="name"][@id="name_1"][@value="&b"][not(@checked)] /following-sibling::label[@for="name_1"][.="[trans]Choice&B[/trans]"] ] - [count(./input)=2] + [count(./input)=3] ' ); } @@ -668,12 +669,13 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase $this->assertWidgetMatchesXpath($form->createView(), array(), '/div [ - ./input[@type="radio"][@name="name"][@id="name_0"][@checked] + ./input[@type="hidden"][@id="name__token"] + /following-sibling::input[@type="radio"][@name="name"][@id="name_0"][@checked] /following-sibling::label[@for="name_0"][.="[trans]Choice&A[/trans]"] /following-sibling::input[@type="radio"][@name="name"][@id="name_1"][not(@checked)] /following-sibling::label[@for="name_1"][.="[trans]Choice&B[/trans]"] ] - [count(./input)=2] + [count(./input)=3] ' ); } @@ -689,12 +691,13 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase $this->assertWidgetMatchesXpath($form->createView(), array(), '/div [ - ./input[@type="radio"][@name="name"][@id="name_0"][@checked] + ./input[@type="hidden"][@id="name__token"] + /following-sibling::input[@type="radio"][@name="name"][@id="name_0"][@checked] /following-sibling::label[@for="name_0"][.="[trans]Choice&A[/trans]"] /following-sibling::input[@type="radio"][@name="name"][@id="name_1"][not(@checked)] /following-sibling::label[@for="name_1"][.="[trans]Choice&B[/trans]"] ] - [count(./input)=2] + [count(./input)=3] ' ); } @@ -711,14 +714,15 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase $this->assertWidgetMatchesXpath($form->createView(), array(), '/div [ - ./input[@type="checkbox"][@name="name[]"][@id="name_0"][@checked][not(@required)] + ./input[@type="hidden"][@id="name__token"] + /following-sibling::input[@type="checkbox"][@name="name[]"][@id="name_0"][@checked][not(@required)] /following-sibling::label[@for="name_0"][.="[trans]Choice&A[/trans]"] /following-sibling::input[@type="checkbox"][@name="name[]"][@id="name_1"][not(@checked)][not(@required)] /following-sibling::label[@for="name_1"][.="[trans]Choice&B[/trans]"] /following-sibling::input[@type="checkbox"][@name="name[]"][@id="name_2"][@checked][not(@required)] /following-sibling::label[@for="name_2"][.="[trans]Choice&C[/trans]"] ] - [count(./input)=3] + [count(./input)=4] ' ); } @@ -753,22 +757,6 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase ); } - public function testCsrf() - { - $this->csrfProvider->expects($this->any()) - ->method('generateCsrfToken') - ->will($this->returnValue('foo&bar')); - - $form = $this->factory->createNamed('csrf', 'name'); - - $this->assertWidgetMatchesXpath($form->createView(), array(), -'/input - [@type="hidden"] - [@value="foo&bar"] -' - ); - } - public function testDateTime() { $form = $this->factory->createNamed('datetime', 'name', '2011-02-03 04:05:06', array( diff --git a/src/Symfony/Component/Form/Tests/AbstractTableLayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractTableLayoutTest.php index d47b15564f..5ea8780d89 100644 --- a/src/Symfony/Component/Form/Tests/AbstractTableLayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractTableLayoutTest.php @@ -45,36 +45,10 @@ abstract class AbstractTableLayoutTest extends AbstractLayoutTest $html = $this->renderRow($form->createView()); $this->assertMatchesXpath($html, -'/tr - [ - ./td - [./label[@for="name_first"]] - /following-sibling::td - [./input[@id="name_first"]] - ] -/following-sibling::tr - [ - ./td - [./label[@for="name_second"]] - /following-sibling::td - [./input[@id="name_second"]] - ] - [count(../tr)=2] -' - ); - } - - public function testRepeatedRowWithErrors() - { - $form = $this->factory->createNamed('repeated', 'name'); - $form->addError(new FormError('Error!')); - $view = $form->createView(); - $html = $this->renderRow($view); - - $this->assertMatchesXpath($html, -'/tr - [./td[@colspan="2"]/ul - [./li[.="[trans]Error![/trans]"]] +'/tr[@style="display: none"] + [./td[@colspan="2"]/input + [@type="hidden"] + [@id="name__token"] ] /following-sibling::tr [ @@ -95,6 +69,42 @@ abstract class AbstractTableLayoutTest extends AbstractLayoutTest ); } + public function testRepeatedRowWithErrors() + { + $form = $this->factory->createNamed('repeated', 'name'); + $form->addError(new FormError('Error!')); + $view = $form->createView(); + $html = $this->renderRow($view); + + $this->assertMatchesXpath($html, +'/tr + [./td[@colspan="2"]/ul + [./li[.="[trans]Error![/trans]"]] + ] +/following-sibling::tr[@style="display: none"] + [./td[@colspan="2"]/input + [@type="hidden"] + [@id="name__token"] + ] +/following-sibling::tr + [ + ./td + [./label[@for="name_first"]] + /following-sibling::td + [./input[@id="name_first"]] + ] +/following-sibling::tr + [ + ./td + [./label[@for="name_second"]] + /following-sibling::td + [./input[@id="name_second"]] + ] + [count(../tr)=4] +' + ); + } + public function testRest() { $view = $this->factory->createNamedBuilder('form', 'name') @@ -151,9 +161,9 @@ abstract class AbstractTableLayoutTest extends AbstractLayoutTest $this->assertWidgetMatchesXpath($form->createView(), array(), '/table [ - ./tr[./td/input[@type="text"][@value="a"]] + ./tr[@style="display: none"][./td[@colspan="2"]/input[@type="hidden"][@id="name__token"]] + /following-sibling::tr[./td/input[@type="text"][@value="a"]] /following-sibling::tr[./td/input[@type="text"][@value="b"]] - /following-sibling::tr[./td/input[@type="hidden"][@id="name__token"]] ] [count(./tr[./td/input])=3] ' @@ -217,6 +227,34 @@ abstract class AbstractTableLayoutTest extends AbstractLayoutTest ); } + public function testCsrf() + { + $this->csrfProvider->expects($this->any()) + ->method('generateCsrfToken') + ->will($this->returnValue('foo&bar')); + + $form = $this->factory->createNamedBuilder('form', 'name') + ->add($this->factory + // No CSRF protection on nested forms + ->createNamedBuilder('form', 'child') + ->add($this->factory->createNamedBuilder('text', 'grandchild')) + ) + ->getForm(); + + $this->assertWidgetMatchesXpath($form->createView(), array(), +'/table + [ + ./tr[@style="display: none"] + [./td[@colspan="2"]/input + [@type="hidden"] + [@id="name__token"] + ] + ] + [count(.//input[@type="hidden"])=1] +' + ); + } + public function testRepeated() { $form = $this->factory->createNamed('repeated', 'name', 'foobar', array( @@ -226,7 +264,12 @@ abstract class AbstractTableLayoutTest extends AbstractLayoutTest $this->assertWidgetMatchesXpath($form->createView(), array(), '/table [ - ./tr + ./tr[@style="display: none"] + [./td[@colspan="2"]/input + [@type="hidden"] + [@id="name__token"] + ] + /following-sibling::tr [ ./td [./label[@for="name_first"]] @@ -241,7 +284,7 @@ abstract class AbstractTableLayoutTest extends AbstractLayoutTest [./input[@type="text"][@id="name_second"]] ] ] - [count(.//input)=2] + [count(.//input)=3] ' ); } @@ -257,7 +300,12 @@ abstract class AbstractTableLayoutTest extends AbstractLayoutTest $this->assertWidgetMatchesXpath($form->createView(), array(), '/table [ - ./tr + ./tr[@style="display: none"] + [./td[@colspan="2"]/input + [@type="hidden"] + [@id="name__token"] + ] + /following-sibling::tr [ ./td [./label[@for="name_first"][.="[trans]Test[/trans]"]] @@ -272,7 +320,7 @@ abstract class AbstractTableLayoutTest extends AbstractLayoutTest [./input[@type="password"][@id="name_second"][@required="required"]] ] ] - [count(.//input)=2] + [count(.//input)=3] ' ); } diff --git a/src/Symfony/Component/Form/Tests/Extension/Csrf/EventListener/EnsureCsrfFieldListenerTest.php b/src/Symfony/Component/Form/Tests/Extension/Csrf/EventListener/EnsureCsrfFieldListenerTest.php deleted file mode 100644 index 9b87717e5d..0000000000 --- a/src/Symfony/Component/Form/Tests/Extension/Csrf/EventListener/EnsureCsrfFieldListenerTest.php +++ /dev/null @@ -1,87 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Form\Tests\Extension\Csrf\EventListener; - -use Symfony\Component\Form\Event\DataEvent; -use Symfony\Component\Form\Extension\Csrf\EventListener\EnsureCsrfFieldListener; - -class EnsureCsrfFieldListenerTest extends \PHPUnit_Framework_TestCase -{ - private $form; - private $formFactory; - private $field; - private $event; - - protected function setUp() - { - if (!class_exists('Symfony\Component\EventDispatcher\EventDispatcher')) { - $this->markTestSkipped('The "EventDispatcher" component is not available'); - } - - $this->formFactory = $this->getMock('Symfony\\Component\\Form\\FormFactoryInterface'); - $this->form = $this->getMock('Symfony\\Component\\Form\\Tests\\FormInterface'); - $this->field = $this->getMock('Symfony\\Component\\Form\\Tests\\FormInterface'); - $this->event = new DataEvent($this->form, array()); - } - - protected function tearDown() - { - $this->form = null; - $this->formFactory = null; - $this->field = null; - $this->event = null; - } - - public function testAddField() - { - $this->formFactory->expects($this->once()) - ->method('createNamed') - ->with('csrf', '_token', null, array()) - ->will($this->returnValue($this->field)); - $this->form->expects($this->once()) - ->method('add') - ->with($this->isInstanceOf('Symfony\\Component\\Form\\Tests\\FormInterface')); - - $listener = new EnsureCsrfFieldListener($this->formFactory, '_token'); - $listener->ensureCsrfField($this->event); - } - - public function testIntention() - { - $this->formFactory->expects($this->once()) - ->method('createNamed') - ->with('csrf', '_token', null, array('intention' => 'something')) - ->will($this->returnValue($this->field)); - $this->form->expects($this->once()) - ->method('add') - ->with($this->isInstanceOf('Symfony\\Component\\Form\\Tests\\FormInterface')); - - $listener = new EnsureCsrfFieldListener($this->formFactory, '_token', 'something'); - $listener->ensureCsrfField($this->event); - } - - public function testProvider() - { - $provider = $this->getMock('Symfony\\Component\\Form\\Extension\\Csrf\\CsrfProvider\\CsrfProviderInterface'); - - $this->formFactory->expects($this->once()) - ->method('createNamed') - ->with('csrf', '_token', null, array('csrf_provider' => $provider)) - ->will($this->returnValue($this->field)); - $this->form->expects($this->once()) - ->method('add') - ->with($this->isInstanceOf('Symfony\\Component\\Form\\Tests\\FormInterface')); - - $listener = new EnsureCsrfFieldListener($this->formFactory, '_token', null, $provider); - $listener->ensureCsrfField($this->event); - } -} diff --git a/src/Symfony/Component/Form/Tests/Extension/Csrf/Type/CsrfTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Csrf/Type/CsrfTypeTest.php deleted file mode 100644 index 4483e7f392..0000000000 --- a/src/Symfony/Component/Form/Tests/Extension/Csrf/Type/CsrfTypeTest.php +++ /dev/null @@ -1,112 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Form\Tests\Extension\Csrf\Type; - -class CsrfTypeTest extends TypeTestCase -{ - protected $provider; - - protected function setUp() - { - parent::setUp(); - - $this->provider = $this->getMock('Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface'); - } - - protected function tearDown() - { - parent::tearDown(); - - $this->provider = null; - } - - protected function getNonRootForm() - { - $form = $this->getMock('Symfony\Component\Form\Tests\FormInterface'); - $form->expects($this->any()) - ->method('isRoot') - ->will($this->returnValue(false)); - - return $form; - } - - public function testGenerateCsrfToken() - { - $this->provider->expects($this->once()) - ->method('generateCsrfToken') - ->with('%INTENTION%') - ->will($this->returnValue('token')); - - $form = $this->factory->create('csrf', null, array( - 'csrf_provider' => $this->provider, - 'intention' => '%INTENTION%' - )); - - $this->assertEquals('token', $form->getData()); - } - - public function testValidateTokenOnBind() - { - $this->provider->expects($this->once()) - ->method('isCsrfTokenValid') - ->with('%INTENTION%', 'token') - ->will($this->returnValue(true)); - - $form = $this->factory->create('csrf', null, array( - 'csrf_provider' => $this->provider, - 'intention' => '%INTENTION%' - )); - $form->bind('token'); - - $this->assertEquals('token', $form->getData()); - } - - public function testDontValidateTokenIfParentIsNotRoot() - { - $this->provider->expects($this->never()) - ->method('isCsrfTokenValid'); - - $form = $this->factory->create('csrf', null, array( - 'csrf_provider' => $this->provider, - 'intention' => '%INTENTION%' - )); - $form->setParent($this->getNonRootForm()); - $form->bind('token'); - } - - public function testCsrfTokenIsRegeneratedIfValidationFails() - { - $this->provider->expects($this->at(0)) - ->method('generateCsrfToken') - ->with('%INTENTION%') - ->will($this->returnValue('token1')); - $this->provider->expects($this->at(1)) - ->method('isCsrfTokenValid') - ->with('%INTENTION%', 'invalid') - ->will($this->returnValue(false)); - - // The token is regenerated to avoid stalled tokens, for example when - // the session ID changed - $this->provider->expects($this->at(2)) - ->method('generateCsrfToken') - ->with('%INTENTION%') - ->will($this->returnValue('token2')); - - $form = $this->factory->create('csrf', null, array( - 'csrf_provider' => $this->provider, - 'intention' => '%INTENTION%' - )); - $form->bind('invalid'); - - $this->assertEquals('token2', $form->getData()); - } -} diff --git a/src/Symfony/Component/Form/Tests/Extension/Csrf/Type/FormTypeCsrfExtensionTest.php b/src/Symfony/Component/Form/Tests/Extension/Csrf/Type/FormTypeCsrfExtensionTest.php index 20da376d79..9024002b52 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Csrf/Type/FormTypeCsrfExtensionTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Csrf/Type/FormTypeCsrfExtensionTest.php @@ -11,43 +11,188 @@ namespace Symfony\Component\Form\Tests\Extension\Csrf\Type; +use Symfony\Component\Form\Extension\Csrf\CsrfExtension; +use Symfony\Component\Form\Tests\Extension\Core\Type\TypeTestCase; + class FormTypeCsrfExtensionTest extends TypeTestCase { - public function testCsrfProtectionByDefault() - { - $form = $this->factory->create('form', null, array( - 'csrf_field_name' => 'csrf', - )); + protected $csrfProvider; - $this->assertTrue($form->has('csrf')); + protected function setUp() + { + $this->csrfProvider = $this->getMock('Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface'); + + parent::setUp(); + } + + protected function tearDown() + { + $this->csrfProvider = null; + + parent::tearDown(); + } + + protected function getExtensions() + { + return array_merge(parent::getExtensions(), array( + new CsrfExtension($this->csrfProvider), + )); + } + + public function testCsrfProtectionByDefaultIfRootAndChildren() + { + $view = $this->factory + ->createBuilder('form', null, array( + 'csrf_field_name' => 'csrf', + )) + ->add($this->factory->createNamedBuilder('form', 'child')) + ->getForm() + ->createView(); + + $this->assertTrue($view->hasChild('csrf')); + } + + public function testNoCsrfProtectionByDefaultIfChildrenButNotRoot() + { + $view = $this->factory + ->createNamedBuilder('form', 'root') + ->add($this->factory + ->createNamedBuilder('form', 'form', null, array( + 'csrf_field_name' => 'csrf', + )) + ->add($this->factory->createNamedBuilder('form', 'child')) + ) + ->getForm() + ->get('form') + ->createView(); + + $this->assertFalse($view->hasChild('csrf')); + } + + public function testNoCsrfProtectionByDefaultIfRootButNoChildren() + { + $view = $this->factory + ->createBuilder('form', null, array( + 'csrf_field_name' => 'csrf', + )) + ->getForm() + ->createView(); + + $this->assertFalse($view->hasChild('csrf')); } public function testCsrfProtectionCanBeDisabled() { - $form = $this->factory->create('form', null, array( - 'csrf_protection' => false, - )); - - $this->assertCount(0, $form); - } - - public function testCsrfTokenIsOnlyIncludedInRootView() - { - $view = - $this->factory->createBuilder('form', null, array( + $view = $this->factory + ->createBuilder('form', null, array( 'csrf_field_name' => 'csrf', + 'csrf_protection' => false, )) - ->add('notCsrf', 'text') - ->add( - $this->factory->createNamedBuilder('form', 'child', null, array( - 'csrf_field_name' => 'csrf', - )) - ->add('notCsrf', 'text') - ) + ->add($this->factory->createNamedBuilder('form', 'child')) ->getForm() ->createView(); - $this->assertEquals(array('csrf', 'notCsrf', 'child'), array_keys(iterator_to_array($view))); - $this->assertEquals(array('notCsrf'), array_keys(iterator_to_array($view['child']))); + $this->assertFalse($view->hasChild('csrf')); + } + + public function testGenerateCsrfToken() + { + $this->csrfProvider->expects($this->once()) + ->method('generateCsrfToken') + ->with('%INTENTION%') + ->will($this->returnValue('token')); + + $view = $this->factory + ->createBuilder('form', null, array( + 'csrf_field_name' => 'csrf', + 'csrf_provider' => $this->csrfProvider, + 'intention' => '%INTENTION%' + )) + ->add($this->factory->createNamedBuilder('form', 'child')) + ->getForm() + ->createView(); + + $this->assertEquals('token', $view->getChild('csrf')->get('value')); + } + + public function provideBoolean() + { + return array( + array(true), + array(false), + ); + } + + /** + * @dataProvider provideBoolean + */ + public function testValidateTokenOnBindIfRootAndChildren($valid) + { + $this->csrfProvider->expects($this->once()) + ->method('isCsrfTokenValid') + ->with('%INTENTION%', 'token') + ->will($this->returnValue($valid)); + + $form = $this->factory + ->createBuilder('form', null, array( + 'csrf_field_name' => 'csrf', + 'csrf_provider' => $this->csrfProvider, + 'intention' => '%INTENTION%' + )) + ->add($this->factory->createNamedBuilder('form', 'child')) + ->getForm(); + + $form->bind(array( + 'child' => 'foobar', + 'csrf' => 'token', + )); + + // Remove token from data + $this->assertSame(array('child' => 'foobar'), $form->getData()); + + // Validate accordingly + $this->assertSame($valid, $form->isValid()); + } + + public function testDontValidateTokenIfChildrenButNoRoot() + { + $this->csrfProvider->expects($this->never()) + ->method('isCsrfTokenValid'); + + $form = $this->factory + ->createNamedBuilder('form', 'root') + ->add($this->factory + ->createNamedBuilder('form', 'form', null, array( + 'csrf_field_name' => 'csrf', + 'csrf_provider' => $this->csrfProvider, + 'intention' => '%INTENTION%' + )) + ->add($this->factory->createNamedBuilder('form', 'child')) + ) + ->getForm() + ->get('form'); + + $form->bind(array( + 'child' => 'foobar', + 'csrf' => 'token', + )); + } + + public function testDontValidateTokenIfRootButNoChildren() + { + $this->csrfProvider->expects($this->never()) + ->method('isCsrfTokenValid'); + + $form = $this->factory + ->createBuilder('form', null, array( + 'csrf_field_name' => 'csrf', + 'csrf_provider' => $this->csrfProvider, + 'intention' => '%INTENTION%' + )) + ->getForm(); + + $form->bind(array( + 'csrf' => 'token', + )); } } diff --git a/src/Symfony/Component/Form/Tests/Extension/Csrf/Type/TypeTestCase.php b/src/Symfony/Component/Form/Tests/Extension/Csrf/Type/TypeTestCase.php deleted file mode 100644 index 3b1ce91783..0000000000 --- a/src/Symfony/Component/Form/Tests/Extension/Csrf/Type/TypeTestCase.php +++ /dev/null @@ -1,41 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Symfony\Component\Form\Tests\Extension\Csrf\Type; - -use Symfony\Component\Form\Tests\Extension\Core\Type\TypeTestCase as BaseTestCase; -use Symfony\Component\Form\Extension\Csrf\CsrfExtension; - -abstract class TypeTestCase extends BaseTestCase -{ - protected $csrfProvider; - - protected function setUp() - { - $this->csrfProvider = $this->getMock('Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface'); - - parent::setUp(); - } - - protected function tearDown() - { - $this->csrfProvider = null; - - parent::tearDown(); - } - - protected function getExtensions() - { - return array_merge(parent::getExtensions(), array( - new CsrfExtension($this->csrfProvider), - )); - } -} diff --git a/src/Symfony/Component/Form/Tests/FormTest.php b/src/Symfony/Component/Form/Tests/FormTest.php index a5c20dcbcf..bc25e16a1f 100644 --- a/src/Symfony/Component/Form/Tests/FormTest.php +++ b/src/Symfony/Component/Form/Tests/FormTest.php @@ -1247,7 +1247,7 @@ class FormTest extends \PHPUnit_Framework_TestCase public function testCreateViewAcceptsParent() { - $parent = new FormView(); + $parent = new FormView('form'); $form = $this->getBuilder()->getForm(); $view = $form->createView($parent);