[Form] CsrfValidationListener marks the token as invalid if it is not a string #29884

umpirsky · 2019-01-14T16:41:15Z

Q	A
Branch?	3.4
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#29882
License	MIT

xabbuh · 2019-01-14T19:20:18Z

3.4 is affected by this too, isn't it?

src/Symfony/Component/Form/Extension/Csrf/EventListener/CsrfValidationListener.php

umpirsky · 2019-01-15T13:09:27Z

3.4 is affected by this too, isn't it?

Yes, the fix probably need to be applied there too.

src/Symfony/Component/Form/Tests/Extension/Csrf/EventListener/CsrfValidationListenerTest.php

xabbuh · 2019-01-15T16:21:34Z

@umpirsky Can you rebase on the 3.4 branch? :)

umpirsky · 2019-01-15T16:31:15Z

It will be easier to create a new PR. :)

xabbuh · 2019-01-15T16:44:30Z

@umpirsky if you'd like to, I can have a look at this PR and rebase it

umpirsky · 2019-01-15T16:50:22Z

I will look into it soon...

umpirsky · 2019-01-15T21:30:53Z

I reset to 3.4 and cherry-picked changes. Thanks for the patience!

umpirsky · 2019-01-18T08:29:55Z

Status?

nicolas-grekas

Please rebase + change the PR title to make the changelog usefull.
Good to me otherwise!

src/Symfony/Component/Form/Tests/Extension/Csrf/EventListener/CsrfValidationListenerTest.php

umpirsky · 2019-01-26T11:09:55Z

Updated.

… a string

nicolas-grekas · 2019-02-07T10:03:13Z

Thank you @umpirsky.

…f it is not a string (umpirsky) This PR was squashed before being merged into the 3.4 branch (closes #29884). Discussion ---------- [Form] CsrfValidationListener marks the token as invalid if it is not a string | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #29882 | License | MIT Commits ------- deb8e95 [Form] CsrfValidationListener marks the token as invalid if it is not a string

COil · 2019-03-04T10:39:12Z

Hello, my tests involving CSRF tokens are broken since 4.2.4. I guess it comes from this PR. Reverting to 4.2.3 fix the issue.

    $url.='/create';
    $client->request('POST', $url, [
        CategoryCreateType::FORM_ID => [
            'label' => 'New category', // id is 5
            '_token' => $this->getToken($client, CategoryCreateType::FORM_ID),
        ],
    ]);

It comes from:

protected function getToken(Client $client, string $tokenId): CsrfToken
{
    return $client->getContainer()->get('security.csrf.token_manager')->getToken($tokenId);
}

umpirsky · 2019-03-04T10:42:08Z

What does $this->getToken($client, CategoryCreateType::FORM_ID) returns?

COil · 2019-03-04T10:49:37Z

CsrfToken, casting to string fix the probem. :)

COil · 2019-03-04T10:53:51Z

@umpirsky What was the bug at first?

xabbuh · 2019-03-04T11:00:08Z

@COil When the submitted value was an array the listener resulted in a PHP error (see #29882).

matks · 2019-03-06T10:13:49Z

CsrfToken, casting to string fix the probem. :)

Same for me, but instead I use $token->getValue() 😉

PrestaShop/PrestaShop#12804

umpirsky · 2019-04-02T06:51:03Z

Will there be 4.1.* version with this change?

xabbuh · 2019-04-02T07:03:09Z

No, 4.1 doesn't receive bugfixes (except for security issues) anymore (see https://symfony.com/roadmap/4.1).

carsonbot added Status: Needs Review Bug labels Jan 14, 2019

xabbuh requested changes Jan 14, 2019

View reviewed changes

src/Symfony/Component/Form/Extension/Csrf/EventListener/CsrfValidationListener.php Outdated Show resolved Hide resolved

carsonbot added Status: Needs Work and removed Status: Needs Review labels Jan 14, 2019

OskarStark reviewed Jan 15, 2019

View reviewed changes

src/Symfony/Component/Form/Tests/Extension/Csrf/EventListener/CsrfValidationListenerTest.php Outdated Show resolved Hide resolved

src/Symfony/Component/Form/Tests/Extension/Csrf/EventListener/CsrfValidationListenerTest.php Outdated Show resolved Hide resolved

xabbuh changed the base branch from 4.2 to 3.4 January 15, 2019 16:21

umpirsky force-pushed the fix-29882-4.2 branch from 9feae63 to ad6f0dc Compare January 15, 2019 21:27

xabbuh approved these changes Jan 15, 2019

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Work labels Jan 15, 2019

xabbuh added this to the 3.4 milestone Jan 15, 2019

carsonbot added Status: Needs Work and removed Status: Reviewed labels Jan 25, 2019

nicolas-grekas requested changes Jan 25, 2019

View reviewed changes

src/Symfony/Component/Form/Tests/Extension/Csrf/EventListener/CsrfValidationListenerTest.php Outdated Show resolved Hide resolved

umpirsky force-pushed the fix-29882-4.2 branch from a905812 to 86aab3f Compare January 26, 2019 11:08

umpirsky changed the title ~~Fixes issue #29882~~ CsrfValidationListener marks the token as invalid if it is not a string Jan 26, 2019

umpirsky closed this Jan 26, 2019

umpirsky reopened this Jan 26, 2019

xabbuh added the Validator label Jan 26, 2019

xabbuh requested a review from nicolas-grekas February 2, 2019 07:48

carsonbot added Status: Needs Review and removed Status: Needs Work labels Feb 2, 2019

nicolas-grekas approved these changes Feb 2, 2019

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Feb 2, 2019

xabbuh added the Form label Feb 2, 2019

nicolas-grekas changed the title ~~CsrfValidationListener marks the token as invalid if it is not a string~~ [Form] CsrfValidationListener marks the token as invalid if it is not a string Feb 7, 2019

[Form] CsrfValidationListener marks the token as invalid if it is not…

deb8e95

… a string

nicolas-grekas force-pushed the fix-29882-4.2 branch from 86aab3f to deb8e95 Compare February 7, 2019 09:44

nicolas-grekas merged commit deb8e95 into symfony:3.4 Feb 7, 2019

umpirsky deleted the fix-29882-4.2 branch February 7, 2019 11:26

This was referenced Mar 3, 2019

Release v3.4.23 #30430

Merged

Release v4.2.4 #30431

Merged

matks mentioned this pull request Mar 6, 2019

Use string value for CSRF token in DeliveryControllerTest PrestaShop/PrestaShop#12804

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Form] CsrfValidationListener marks the token as invalid if it is not a string #29884

[Form] CsrfValidationListener marks the token as invalid if it is not a string #29884

umpirsky commented Jan 14, 2019 •

edited

xabbuh commented Jan 14, 2019

umpirsky commented Jan 15, 2019

xabbuh commented Jan 15, 2019

umpirsky commented Jan 15, 2019

xabbuh commented Jan 15, 2019

umpirsky commented Jan 15, 2019

umpirsky commented Jan 15, 2019

umpirsky commented Jan 18, 2019

nicolas-grekas left a comment

umpirsky commented Jan 26, 2019

nicolas-grekas commented Feb 7, 2019

COil commented Mar 4, 2019 •

edited

umpirsky commented Mar 4, 2019

COil commented Mar 4, 2019 •

edited

COil commented Mar 4, 2019 •

edited

xabbuh commented Mar 4, 2019

matks commented Mar 6, 2019

umpirsky commented Apr 2, 2019

xabbuh commented Apr 2, 2019 •

edited

[Form] CsrfValidationListener marks the token as invalid if it is not a string #29884

[Form] CsrfValidationListener marks the token as invalid if it is not a string #29884

Conversation

umpirsky commented Jan 14, 2019 • edited

xabbuh commented Jan 14, 2019

umpirsky commented Jan 15, 2019

xabbuh commented Jan 15, 2019

umpirsky commented Jan 15, 2019

xabbuh commented Jan 15, 2019

umpirsky commented Jan 15, 2019

umpirsky commented Jan 15, 2019

umpirsky commented Jan 18, 2019

nicolas-grekas left a comment

Choose a reason for hiding this comment

umpirsky commented Jan 26, 2019

nicolas-grekas commented Feb 7, 2019

COil commented Mar 4, 2019 • edited

umpirsky commented Mar 4, 2019

COil commented Mar 4, 2019 • edited

COil commented Mar 4, 2019 • edited

xabbuh commented Mar 4, 2019

matks commented Mar 6, 2019

umpirsky commented Apr 2, 2019

xabbuh commented Apr 2, 2019 • edited

umpirsky commented Jan 14, 2019 •

edited

COil commented Mar 4, 2019 •

edited

COil commented Mar 4, 2019 •

edited

COil commented Mar 4, 2019 •

edited

xabbuh commented Apr 2, 2019 •

edited