[Security] : Aligning CSRF tokenId with other code sample
#19808
Conversation
Page: https://symfony.com/doc/5.x/security.html * I'm making this compatible with the `tokenId` used at https://symfony.com/doc/5.x/security/custom_authenticator.html#passport-badges * Where what the info coming from that it "must" be called `authenticate`? The docblock of `CsrfTokenBadge` just says it's an "arbitrary string"
carsonbot
changed the title
tokenId with other code sampletokenId with other code sample
security.rst
Outdated
| must be called ``_csrf_token`` and the string used to generate the value must | ||
| be ``authenticate``: | ||
| is called ``_csrf_token`` and takes an arbitrary string as argument ``tokenId``: |
There was a problem hiding this comment.
This section is talking about the build-in form login authenticator. You must define it as authenticate to make it work with this authenticator.
I think we can improve the wording on https://symfony.com/doc/current/security/csrf.html#csrf-protection-in-login-forms, but the change in this document must be reverted. A suggested rewording for the linked section:
- See :ref:`form_login-csrf` for a login form that is protected from CSRF
+ When using the ``form_login`` authenticator, see :ref:`form_login-csrf` to protected from
attacks. You can also configure the
:ref:`CSRF protection for the logout action <reference-security-logout-csrf>`.
+ When implementing a custom authenticator, use the ``CsrfTokenBadge`` on the
+ :doc:`security passport </security/custom_authenticator>`.There was a problem hiding this comment.
OK, I changed it, please take a look now.
For your suggested change, I'll come back to it after we found a solution for the below conversation.
| $username = $request->request->get('username'); | ||
| $csrfToken = $request->request->get('csrf_token'); | ||
| $password = $request->request->get('password'); | ||
| $csrfToken = $request->request->get('_csrf_token'); |
There was a problem hiding this comment.
I think we should also revert this change. The underscore prefix is only something used in the build-in authenticator (it's a convention in Symfony to prefix things with an underscore to avoid conflicts with application names).
When implementing a custom authenticator, you can name the field whatever you like and it's better to not use the underscore prefix as this counters the anti-conflict purpose of the prefix.
There was a problem hiding this comment.
OK, but now the docs are inconsistent (i.e. the HTML shown on one page doesn't work with the PHP code shown on the other page).
Solution? => Show the right HTML code here too!
To make this possible, the list at https://symfony.com/doc/5.x/security/custom_authenticator.html#passport-badges needs to be changed to sub-headings. Then the PHP code block we're talking about can be moved upwards under the (new) "CsrfTokenBadge" heading (=where it belongs anyway). Then I can add this HTML, resulting in a complete copy-pastable sample:
<input type="hidden" name="csrf_token" value="{{ csrf_token('login') }}">
<input type="text" name="username">
<input type="password" name="password">What do you think?
Page: https://symfony.com/doc/5.x/security.html
tokenIdused at https://symfony.com/doc/5.x/security/custom_authenticator.html#passport-badgesauthenticate? The docblock ofCsrfTokenBadgejust says it's an "arbitrary string"Second commit: Aligning CSRF token filed name at https://symfony.com/doc/5.x/security/custom_authenticator.html#passport-badges with https://symfony.com/doc/5.x/security.html#csrf-protection-in-login-forms
Question: I think the other names should be changed too (adding underscore):
_usernameand_password