[FrameworkBundle][TwigBridge] make csrf_token() usable without forms #25197
Conversation
| new TwigFunction('is_granted', array($this, 'isGranted')), | ||
| ); | ||
|
|
||
| if (null !== $this->csrfTokenManager) { | ||
| $functions[] = new TwigFunction('csrf_token', array($this, 'getCsrfToken')); |
There was a problem hiding this comment.
Regarding DX, wouldn't it be better to always add the method but throw if no csrfTokenManager injected?
There was a problem hiding this comment.
I would say it's better this way because you will notice the error when templates are parsed. Having the function fail at runtime can just go without notice.
|
|
||
| public function __construct(AuthorizationCheckerInterface $securityChecker = null) | ||
| public function __construct(AuthorizationCheckerInterface $securityChecker = null, CsrfTokenManagerInterface $csrfTokenManager = null) |
There was a problem hiding this comment.
Adding the CSRF manager adds more deps, would it be better to create a Twig runtime class at this point?
|
Rather than coupling it to the The authorization layer and the CSRF layer are totally independant components (the CSRF one is not even configured by SecurityBundle but by FrameworkBundle) |
The Twig function `csrf_token()` is currently only registered when the Form component is installed. However, this function is also useful, for example, when creating simple login forms for which you do not need the full Form component.
2abd80e to
709efa3
Compare
|
I moved the new function into a new extension class inside FrameworkBundle. Do we nonetheless want to use a runtime class here? |
|
@xabbuh just asking: is there any reason yo create so many small Twig extensions in https://github.com/xabbuh/symfony/tree/master/src/Symfony/Bridge/Twig/Extension? Could we add this tiny new function in the existing SecurityExtension instead? Thanks! |
Maybe I am missing something, but I don't see how to deprecate registering a single function from a Twig extension. But as far as I can see that's not a big deal as the extension being registered later will just replace the existing definition which does no harm as both implementations internally work the same. |
|
@fabpot in our case both filters have the same name. We could make registering the additional filters an opt-out feature. But I am not sure if it's worth it. |
|
Thank you @xabbuh. |
… without forms (xabbuh) This PR was merged into the 4.1-dev branch. Discussion ---------- [FrameworkBundle][TwigBridge] make csrf_token() usable without forms | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | The Twig function `csrf_token()` is currently only registered when the Form component is installed. However, this function is also useful, for example, when creating simple login forms for which you do not need the full Form component. Commits ------- 709efa3 make csrf_token() usable without forms
|
@xabbuh Thank you! 👍 |
…endency on CSRF token storage (tgalopin) This PR was merged into the 4.1 branch. Discussion ---------- [FrameworkBundle][TwigBridge] Fix BC break from strong dependency on CSRF token storage | Q | A | ------------- | --- | Branch? | 4.1 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - The PR #25197 introduced the `csrf_token` function in Twig. This extension relies on `CsrfTokenManagerInterface`, which itself relies on the session. In some contexts such as when sessions are stored in Redis and we try to warmup the cache in CLI without Redis available, this makes the process fails. This PR fixes this by using a Twig runtime instead of a direct extension to avoid a strong dependency on `CsrfTokenManagerInterface`. Commits ------- 68994a6 [FrameworkBundle][TwigBridge] Fix BC break from strong dependency on CSRF token storage
The Twig function
csrf_token()is currently only registered when theForm component is installed. However, this function is also useful, for
example, when creating simple login forms for which you do not need the
full Form component.