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

Fix Symfony 5.4 compatibility of DisabledCsrfTokenManager #307

Merged
merged 4 commits into from
Nov 30, 2021

Conversation

eekes
Copy link
Contributor

@eekes eekes commented Nov 30, 2021

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
License MIT

What's in this PR?

Make DisabledCsrfTokenManager compatible with Symfony's CsrfTokenManagerInterface

Why?

DisabledCsrfTokenManager doesn't seem be compatible with Symfony 5.4 because of the return types that were added.

Compile Error: Declaration of Sulu\Bundle\FormBundle\Csrf\DisabledCsrfTokenManager::getToken($tokenId) must be compatible with Symfony\Component\Security\Csrf\CsrfTokenManagerInterface::getToken(string $tokenId): Symfony\Component\Security\Csrf\CsrfToken

Example Usage

Simply by using a form in a Symfony 5.4 Sulu project.

BC Breaks/Deprecations

Don't think so?

@alexander-schranz
Copy link
Member

Looks for me correctly for 5.4 but it seems to install a newer version. So I think our error is that we forget to define the dependency to csrf token package.

@alexander-schranz
Copy link
Member

alexander-schranz commented Nov 30, 2021

Can you add "symfony/security-csrf": "^4.4 || ^5.4" to the composer.json then there should be no adjustments needed. We got problems when adding the return types on PHP 7.2 on Symfony 4.4 so we can not add them there.

@eekes
Copy link
Contributor Author

eekes commented Nov 30, 2021

That seems to fix it, installed 6.0 without defining the version in composer.

@alexander-schranz alexander-schranz enabled auto-merge (squash) November 30, 2021 16:16
@alexander-schranz
Copy link
Member

@eekes Thx for the report and the pull request.

@alexander-schranz alexander-schranz changed the title Make DisabledCsrfTokenManager compatible with CsrfTokenManagerInterface Fix Symfony 5.4 compatibility of DisabledCsrfTokenManager Nov 30, 2021
@alexander-schranz alexander-schranz enabled auto-merge (squash) November 30, 2021 16:19
@alexander-schranz alexander-schranz merged commit 80f7922 into sulu:2.x Nov 30, 2021
@alexander-schranz
Copy link
Member

@eekes released as 2.2.1.

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.

2 participants