-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
"The CSRF token is invalid." in tests after upgrading to 6.2.6 to fix CVE #49194
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
Comments
Not sure if it is the same issue but as it sounds similar I will add my observation here: |
@nicolasleborgne asks something similar in #49188
To me, all use cases that hit this were likely vulnerable to CVE-2022-24894, so we're going to have find another way to prevent CSRF, because the previous one didn't work. I'm writing "we" because I don't know if Symfony can provide something - doc or code - to cover this. From afar, I suppose that when there is a session, the session should be used to NOT "login" at every request. |
Login from the session does not cause any issue because it is done with the Symfony Demo’s tests authenticate using the |
Good call: let's figure out how to fix the demo app, that will give us the food for thoughts we need here. Anyone up to give it a try? |
Using |
I've just upgraded To authenticate the user, we use a custom "I am logged in with email XXX" step that sets an HTTP Header, caught by a custom Authenticator.
The "url should match" check fails because we're back on /some/page, and printing the last response shows "Invalid CSRF token" error in the HTML content. Here's the detailed code /**
* @Given I am logged in with email :identifier
*/
public function iAmLoggedInWithEmail(string $identifier): void
{
$this->restContext->iAddHeaderEqualTo('Authorization', HeaderAuthenticator::AUTHORIZATION_HEADER_PREFIX . ' ' . $identifier);
} #[When(env: 'dev'), When(env: 'test')]
final class HeaderAuthenticator extends AbstractAuthenticator
{
public const AUTHORIZATION_HEADER_PREFIX = 'USER';
public function __construct(
private UserRepository $userRepository,
) {
}
public function supports(Request $request): ?bool
{
$header = (string) $request->headers->get('Authorization');
return str_starts_with($header, self::AUTHORIZATION_HEADER_PREFIX);
}
public function authenticate(Request $request): Passport
{
$headerParts = explode(' ', (string) $request->headers->get('Authorization'));
Assert::count($headerParts, 2, 'The authorization header should have two parts: the prefix and the user identifier.');
$passport = new SelfValidatingPassport(new UserBadge(
userIdentifier: $headerParts[1],
userLoader: fn (string $identifier): UserInterface => $this->userRepository->loadUserByIdentifier($identifier),
));
return $passport;
}
public function createToken(Passport $passport, string $firewallName): TokenInterface
{
return new TestToken(
interactive: false,
user: $passport->getUser(),
firewallName: $firewallName,
roles: $passport->getUser()->getRoles(),
);
}
public function onAuthenticationFailure(Request $request, AuthenticationException $exception): Response
{
throw $exception;
}
public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $firewallName): ?Response
{
// Let the request continue
return null;
}
} |
I can confirm i have simmalair problem with version |
@nicolas-grekas would deferring the CSRF token removal on |
Confirmed on 5.4.20. When using x.509 authentication all forms stop working due to invalid CSRF token. |
Any advice what we should do until a proper solution is found if we have x509 and CSRF protection in production? Either pin symphony/security to a pre-patch version or disabled CSRF protection? The former might be less of a security risk? |
|
I had the same issue, but actually it was caused by this commit symfony/http-kernel@2a91996 that resulted in the caching by the browser of some form pages containing a Set-Cookie header |
This reverts commit 8d93b24. There seems to be a regression regarding CSRF handling, breaking all forms. See symfony/symfony#49194.
Same issue here following the https://symfony.com/doc/5.0/testing/http_authentication.html#using-a-faster-authentication-mechanism-only-for-tests documentation |
I fixed it by using the |
Could someone confirm that #49319 fixes their issue? |
@MatTheCat #49319 fixes our issue (combination http basic auth and csrf protection) in unit test and dev env. |
@MatTheCat @rivallinkil I'd like to try your patch out in our project, but I'm unsure how to apply the patch (as it seems to be impacting multiple Symfony components/bundles). Would you mind providing some instructions on how to upgrade my project's dependencies or apply the patch locally ? |
@gnutix I asked the same question in #49319 and I was given these resources:
|
Sooo #49194 (comment) is not a viable solution; sorry everyone..! Maybe it is time to revive #33171 🤔 |
Please try the fix in #49526 (it's for 5.4 but it shouldn't be that hard to adapt for 6.2) |
Symfony version(s) affected
6.2.6
Description
After installing the fix for https://symfony.com/blog/cve-2022-24895-csrf-token-fixation, tests fail with the message "The CSRF token is invalid." I have only been able to reproduce this in tests, not (yet) when running the application in the browser.
@nicolas-grekas
How to reproduce
./bin/phpunit
. All tests should succeed.composer update
../bin/phpunit
. Multiple tests should fail. Add| grep CSRF
and you will find the error message.Possible Solution
I have no clue yet. My conclusion is that
$this->csrfTokenStorage->clear();
is the cause of this bug (see 5909d74#diff-0ff1412624a79146c346925f2407eb4783b144da38ddb369ca30e49d046fab70R59), but removing this is obviously not an option as it is the fix for the CVE.Additional Context
Docker one-liner:
The text was updated successfully, but these errors were encountered: