Skip to content

[Form][FrameworkBundle] Use auto-configuration to make the default CSRF token id apply only to the app; not to bundles #59728

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 10, 2025
Merged

[Form][FrameworkBundle] Use auto-configuration to make the default CSRF token id apply only to the app; not to bundles #59728

merged 1 commit into from
Feb 10, 2025

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Feb 7, 2025 •
edited
Loading

Q A
Branch? 7.2
Bug fix? yes
New feature? no
Deprecations? no
Issues -
License MIT

After EasyCorp/EasyAdminBundle#6724, I realized I made a mistake in #58095:

The framework.form.csrf_protection.token_id config option should not configure the default CSRF token id for all forms. Instead, we want this option to apply only to forms managed by the app. Bundles shouldn't be affected.

This is what this PR does: it switches from global config to auto-configured form types only (which means app's form types).

@carsonbot carsonbot added this to the 7.2 milestone Feb 7, 2025
@nicolas-grekas nicolas-grekas mentioned this pull request Feb 7, 2025
Closed
chalasr
chalasr approved these changes Feb 7, 2025
View reviewed changes
Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

once open comment resolved

@nicolas-grekas
[Form][FrameworkBundle] Use auto-configuration to make the default CS…
bf1e312 
…RF token id apply only to the app; not to bundles
@nicolas-grekas nicolas-grekas merged commit 7c709de into symfony:7.2 Feb 10, 2025
10 of 11 checks passed
@nicolas-grekas nicolas-grekas deleted the form-csrf branch February 10, 2025 07:48
@fracsi fracsi mentioned this pull request Feb 20, 2025
Closed
@fabpot fabpot mentioned this pull request Feb 26, 2025
Merged
@PhilETaylor
Copy link
Contributor

PhilETaylor commented Feb 26, 2025 •
edited
Loading

Sorry. This broke in production :(

Symfony\Component\Security\Csrf\CsrfTokenManager::getToken(): Argument #1 ($tokenId) must be of type string, null given, called in /app/vendor/symfony/form/Extension/Csrf/Type/FormTypeCsrfExtension.php on line 80

Minimum reproducer is

$this->createFormBuilder()->setAttributes([])->getForm()->createView();

Seems passing any array to the setAttributes causes this unless you pass csrf_token_id value in that array, there is no sane default applied. In my case I was passing other attributes in this array, but not the csrf_token_id, as that was not needed before today.

@nicolas-grekas
Copy link
Member Author

New issue please 🙏

@PhilETaylor PhilETaylor mentioned this pull request Feb 26, 2025
Open
@PhilETaylor
Copy link
Contributor

Done - #59867

renovate bot added a commit to Runroom/archetype-symfony that referenced this pull request Feb 28, 2025
@renovate
Update symfony packages to v7.2.4 (patch) (#4441)
9c12819 
This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [symfony/framework-bundle](https://symfony.com)
([source](https://redirect.github.com/symfony/framework-bundle)) |
`7.2.3` -> `7.2.4` |
[![age](https://developer.mend.io/api/mc/badges/age/packagist/symfony%2fframework-bundle/7.2.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/packagist/symfony%2fframework-bundle/7.2.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/packagist/symfony%2fframework-bundle/7.2.3/7.2.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/packagist/symfony%2fframework-bundle/7.2.3/7.2.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [symfony/messenger](https://symfony.com)
([source](https://redirect.github.com/symfony/messenger)) | `7.2.3` ->
`7.2.4` |
[![age](https://developer.mend.io/api/mc/badges/age/packagist/symfony%2fmessenger/7.2.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/packagist/symfony%2fmessenger/7.2.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/packagist/symfony%2fmessenger/7.2.3/7.2.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/packagist/symfony%2fmessenger/7.2.3/7.2.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [symfony/stopwatch](https://symfony.com)
([source](https://redirect.github.com/symfony/stopwatch)) | `7.2.2` ->
`7.2.4` |
[![age](https://developer.mend.io/api/mc/badges/age/packagist/symfony%2fstopwatch/7.2.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/packagist/symfony%2fstopwatch/7.2.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/packagist/symfony%2fstopwatch/7.2.2/7.2.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/packagist/symfony%2fstopwatch/7.2.2/7.2.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [symfony/web-profiler-bundle](https://symfony.com)
([source](https://redirect.github.com/symfony/web-profiler-bundle)) |
`7.2.3` -> `7.2.4` |
[![age](https://developer.mend.io/api/mc/badges/age/packagist/symfony%2fweb-profiler-bundle/7.2.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/packagist/symfony%2fweb-profiler-bundle/7.2.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/packagist/symfony%2fweb-profiler-bundle/7.2.3/7.2.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/packagist/symfony%2fweb-profiler-bundle/7.2.3/7.2.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>symfony/framework-bundle (symfony/framework-bundle)</summary>

###
[`v7.2.4`](https://redirect.github.com/symfony/framework-bundle/releases/tag/v7.2.4)

[Compare
Source](https://redirect.github.com/symfony/framework-bundle/compare/v7.2.3...v7.2.4)

**Changelog**
(symfony/framework-bundle@v7.2.3...v7.2.4)

- bug
[symfony/symfony#59198](https://redirect.github.com/symfony/symfony/issues/59198)
\[Messenger] Filter out non-consumable receivers when registering
`ConsumeMessagesCommand` (@&#8203;wazum)
- bug
[symfony/symfony#59781](https://redirect.github.com/symfony/symfony/issues/59781)
\[Mailer] fix multiple transports default injection
([@&#8203;fkropfhamer](https://redirect.github.com/fkropfhamer))
- bug
[symfony/symfony#59829](https://redirect.github.com/symfony/symfony/issues/59829)
\[FrameworkBundle] Disable the keys normalization of the CSRF form field
attributes ([@&#8203;sukei](https://redirect.github.com/sukei))
- bug
[symfony/symfony#59728](https://redirect.github.com/symfony/symfony/issues/59728)
\[Form]\[FrameworkBundle] Use auto-configuration to make the default
CSRF token id apply only to the app; not to bundles
([@&#8203;nicolas-grekas](https://redirect.github.com/nicolas-grekas))

</details>

<details>
<summary>symfony/messenger (symfony/messenger)</summary>

###
[`v7.2.4`](https://redirect.github.com/symfony/messenger/releases/tag/v7.2.4)

[Compare
Source](https://redirect.github.com/symfony/messenger/compare/v7.2.3...v7.2.4)

**Changelog**
(symfony/messenger@v7.2.3...v7.2.4)

- bug
[symfony/symfony#59198](https://redirect.github.com/symfony/symfony/issues/59198)
\[Messenger] Filter out non-consumable receivers when registering
`ConsumeMessagesCommand` (@&#8203;wazum)

</details>

<details>
<summary>symfony/stopwatch (symfony/stopwatch)</summary>

###
[`v7.2.4`](https://redirect.github.com/symfony/stopwatch/releases/tag/v7.2.4)

[Compare
Source](https://redirect.github.com/symfony/stopwatch/compare/v7.2.2...v7.2.4)

**Changelog**
(symfony/stopwatch@v7.2.3...v7.2.4)

-   no significant changes

</details>

<details>
<summary>symfony/web-profiler-bundle
(symfony/web-profiler-bundle)</summary>

###
[`v7.2.4`](https://redirect.github.com/symfony/web-profiler-bundle/releases/tag/v7.2.4)

[Compare
Source](https://redirect.github.com/symfony/web-profiler-bundle/compare/v7.2.3...v7.2.4)

**Changelog**
(symfony/web-profiler-bundle@v7.2.3...v7.2.4)

- bug
[symfony/symfony#59776](https://redirect.github.com/symfony/symfony/issues/59776)
\[WebProfilerBundle] fix rendering notifier message options
([@&#8203;xabbuh](https://redirect.github.com/xabbuh))
- bug
[symfony/symfony#59033](https://redirect.github.com/symfony/symfony/issues/59033)
\[WebProfilerBundle] Fix interception for non conventional redirects
([@&#8203;Huluti](https://redirect.github.com/Huluti))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/Runroom/archetype-symfony).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xNzYuMiIsInVwZGF0ZWRJblZlciI6IjM5LjE3Ni4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xabbuh xabbuh xabbuh approved these changes

@yceruto yceruto yceruto approved these changes

@chalasr chalasr chalasr approved these changes

@HeahDude HeahDude HeahDude approved these changes

Assignees
No one assigned
Labels
Bug Form FrameworkBundle Status: Reviewed
Projects
None yet
Milestone
7.2
Development

Successfully merging this pull request may close these issues.
7 participants
@nicolas-grekas @PhilETaylor @xabbuh @yceruto @chalasr @HeahDude @carsonbot