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

[Form] Add csrf_token_lazy option #54705

Open
wants to merge 1 commit into
base: 7.1
Choose a base branch
from

Conversation

tugrul
Copy link

@tugrul tugrul commented Apr 23, 2024

Q A
Branch? 7.2
Bug fix? no
New feature? yes
Deprecations? no
Issues #40270 , #13464
License MIT

Description

This pull request introduces csrf_token_lazy, a new form option in Symfony. This option allows for deferred CSRF token generation until form submission, aiding in performance optimization and mitigating conflicts with caching mechanisms.

Motivation

The necessity for this option arises from the default behavior of Symfony's CSRF protection mechanism, which initiates sessions for token generation. However, in scenarios where caching is employed, this behavior can lead to cache misses or bypasses, impacting application performance adversely. By introducing csrf_token_lazy, developers gain the ability to generate tokens only when necessary, thus avoiding unnecessary session starts during form rendering.

Functionality

With csrf_token_lazy enabled, token generation occurs exclusively during form submission. This ensures that sessions are not initiated unnecessarily during form rendering, thereby maintaining security while optimizing performance, particularly in environments sensitive to caching concerns.

Usage

Developers can enable csrf_token_lazy in form configurations:

public function formAction(Request $request, CsrfTokenManagerInterface $csrfTokenManager): Response {

    $lazy = !$request->hasPreviousSession();

    if ($lazy && $request->isMethod('POST')
        && $request->isXmlHttpRequest()
        && $request->request->has('token')) {
        $response = new Response($csrfTokenManager->getToken('lazy_form_token')->getValue());
        $response->headers->set('Content-Type', 'text/plain');
        return $response;
    }
    
    $form = $this->createFormBuilder($formData, [
        'csrf_token_id' => 'lazy_form_token', 
        'csrf_token_lazy' => $lazy,
        'block_prefix' => ''
    ])->add('field_name')->getForm();
    
    return $this->render('form.html.twig', ['form' => $form->createView()]);
}
const field = document.getElementById('field_name');
const token = document.getElementById('_token');

field.addEventListener('change', () => {
    if(token.value === '') {
        fetch('/form', {
            method: 'POST', body: 'token',
            credentials: 'same-origin',
            headers: {
                'X-Requested-With': 'XMLHttpRequest',
                'Content-Type': 'application/x-www-form-urlencoded; charset=utf-8'
            }
        }).then(response => response.text()).then(tokenVal=> token.value = tokenVal);
    }
});

@carsonbot
Copy link

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

  • Always add tests
  • Keep backward compatibility (see https://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
  • Features and deprecations must be submitted against the 7.1 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

@carsonbot
Copy link

Hey!

Thanks for your PR. You are targeting branch "7.1" but it seems your PR description refers to branch "7.1 for features".
Could you update the PR description or change target branch? This helps core maintainers a lot.

Cheers!

Carsonbot

@chalasr chalasr modified the milestones: 7.1, 7.2 Apr 23, 2024
@chalasr
Copy link
Member

chalasr commented Apr 23, 2024

Thanks for the PR.
Related : #40270 #13464
I think lazy CSRF-token generation is desired. The logic behind the proposed option seems to be missing from the diff though.

@tugrul
Copy link
Author

tugrul commented Apr 23, 2024

@chalasr I've decided against implementing an internal controller action for AJAX requests. It's important to note that there's no missing code, as the CSRF token will still be validated by Symfony's FormTypeCsrfExtension in all cases. This mechanism ensures that token validation fails when the token field is an empty string on the form, thereby maintaining security standards.

Instead, I suggest providing guidance for developers to determine whether to handle token generation internally or independently. This approach preserves flexibility while mitigating potential risks associated with token handling.

I'm open to alternative suggestions regarding the implementation of lazy CSRF-token generation. If you have any ideas on how to address potential risks while maintaining flexibility, I'd love to hear them.

Edit: Indeed, achieving lazy CSRF-token generation can be approached in various ways. For developers who prefer not to rely on JavaScript and AJAX requests, an alternative method could involve utilizing form submissions to initiate the required form. For instance, a "show form" button could trigger a form submission to start the required form with the CSRF token initialized.

This provides developers with flexibility in choosing the method that best suits their preferences and project requirements.

public function buildForm(FormBuilderInterface $builder, array $options): void
{
     if($options['csrf_token_lazy']) {
         $builder->add('show_form', SubmitType::class);
         return;
     }

     $builder->add('field_name', TextType::class)->add('submit', SubmitType:class);
}

@OskarStark OskarStark added the Form label May 2, 2024
@carsonbot carsonbot changed the title csrf_token_lazy form option [Form] csrf_token_lazy form option May 2, 2024
@OskarStark OskarStark changed the title [Form] csrf_token_lazy form option [Form] Add csrf_token_lazy option May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants