Skip to content

Conversation

@nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Aug 27, 2024

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

Could be instead of #58082 and #54742

@Seb33300
Copy link
Contributor

I unfortunately do not work anymore for the company where I encountered this issue.
But I tried to create a small reproducer and I cannot see the warning anymore.

But if I understand your solution, you clear the CSRF token even if we are on a stateless route?

@Seb33300
Copy link
Contributor

Seb33300 commented Aug 27, 2024

Because currently, the user is not logged out from stateful request when he logs out on stateless request.
So I believe we should not clear his csrf tokens too.

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Aug 28, 2024

Clearing the CSRF tokens doesn't log out from the session. It just prevents reuse of existing tokens for next requests.

@Seb33300
Copy link
Contributor

Clearing the CSRF tokens doesn't log out from the session. It just prevents reuse of existing tokens for next requests.

Yes, but the file you modified is a listener on the logout event.

And when I tested it few month ago, I saw that logging out on a stateless route doesn't affect stateful session.
Users keep their session in stateful routes but with your change they will lose their csrf tokens.

@nicolas-grekas
Copy link
Member Author

Sure. And not sure about the business requirements on this topic. This might be app-specific.

How do you feel about the new approach in #54742? Up to finish the PR?

@nicolas-grekas
Copy link
Member Author

Closing in favor of #54742

@nicolas-grekas nicolas-grekas deleted the csrf-stateless-clear branch August 30, 2024 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants