Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only check the request token for master requests #2204

Merged

Conversation

fritzmg
Copy link
Contributor

@fritzmg fritzmg commented Aug 27, 2020

Summary

If you have a form on a page and a fragment element after the form, and if, during the POST request, the form does not validate, hence the form does not throw a RedirectException and rendering of the page continues, the RequestTokenListener will be executed for the subsequent sub-requests for fragments, where the session will be set (due to the failing form before), but no CSRF Token will be present and thus the listener will throw the InvalidRequestTokenException.

Reproduction

1. Create a form

1.1. Create a form and disable its HTML5 validation.
1.2. Create a single text field and make it mandatory.
1.3. Create a submit button.
1.4. Insert the form on a page via a form include content element.

2. Create a fragment content element

// src/Controller/ContentElement/ExampleElementController.php
namespace App\Controller\ContentElement;

use Contao\CoreBundle\Controller\AbstractFragmentController;
use Contao\CoreBundle\ServiceAnnotation\ContentElement;
use Symfony\Component\HttpFoundation\Response;

/**
 * @ContentElement(category="texts")
 */
class ExampleElementController extends AbstractFragmentController
{
    public function __invoke(): Response
    {
        return new Response('<p>Hello World!</p>');
    }
}

Create a content element of type example_element after (❗) the form include element.

3. Execute the POST request

3.1. Open a private browsing window, or clear all cookies for the domain of your Contao installation.
3.2. Open the page in the front end where the form and fragment is included.
3.3. Submit the form without any input in the text element.

This will show the invalid request token error in the front end.

Cause

Usually \Contao\Form will either reload the current page or redirect to a new page, thus halting execution. However, when a form field does not validate, the POST request continues and the remaining fragments are rendered. Since the fragments are rendered via sub-requests, the kernel.request event is dispatched for each sub-request, thus also triggering the RequestTokenListener again. But by this point, \Contao\Form has initiated a session and therefore fulfilling all of the conditions for the request to require a valid request token - if the fragment is rendered with the forward renderer (which Contao Content Element fragments are by default), because those fragments get a full duplicate of the original request. Of course no request token will be present in the request, since no request token was necessary yet before the POST request was initiated.

Solution

As discussed with @Toflar in Slack, the RequestTokenListener should probably check whether the request is actually a master request. Thus in this PR I have added this in the beginning of the listener.

@fritzmg fritzmg added the bug label Aug 27, 2020
@fritzmg fritzmg added this to the 4.9 milestone Aug 27, 2020
@fritzmg fritzmg requested a review from a team August 27, 2020 17:45
@fritzmg fritzmg self-assigned this Aug 27, 2020
@leofeyer leofeyer requested review from aschempp, ausi and Toflar and removed request for a team August 28, 2020 06:15
Copy link
Member

@Toflar Toflar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imho that's correct.

@leofeyer leofeyer merged commit 2d11a48 into contao:4.9 Aug 31, 2020
@leofeyer
Copy link
Member

Thank you @fritzmg.

@fritzmg fritzmg deleted the only-check-request-token-for-master-request branch August 31, 2020 08:37
@leofeyer leofeyer changed the title Only check request token for master requests Only check the request token for master requests Sep 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants