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

Handle empty sessions in the CSRF cookie subscriber #6801

Merged
merged 3 commits into from Jan 30, 2024

Conversation

Toflar
Copy link
Member

@Toflar Toflar commented Jan 29, 2024

If you have some code that adds something to the session (and thus starts it) and some code that removes it again, our CsrfTokenCookieSubscriber would still add the CSRF cookie token as the session was started. This makes no sense if the session is empty, thus we need to add this check on top :)

@Toflar Toflar requested a review from ausi January 29, 2024 13:07
@github-actions github-actions bot added this to the 4.13 milestone Jan 29, 2024
fritzmg
fritzmg previously approved these changes Jan 29, 2024
ausi
ausi previously approved these changes Jan 29, 2024
fritzmg
fritzmg previously approved these changes Jan 29, 2024
@leofeyer leofeyer dismissed stale reviews from fritzmg and ausi via 46dfa3b January 29, 2024 15:13
@leofeyer leofeyer changed the title Fixed CSRF cookie subscriber not considering empty sessions Consider empty sessions in the CSRF cookie subscriber Jan 29, 2024
@leofeyer leofeyer changed the title Consider empty sessions in the CSRF cookie subscriber Handle empty sessions in the CSRF cookie subscriber Jan 30, 2024
@leofeyer leofeyer merged commit 78984be into contao:4.13 Jan 30, 2024
17 checks passed
@leofeyer
Copy link
Member

Thank you @Toflar.

@Toflar Toflar deleted the fix/cookie-subscriber branch January 30, 2024 08:32
@leofeyer
Copy link
Member

leofeyer commented Jan 31, 2024

Unfortunately, this breaks the Cypress tests: https://github.com/contao/contao/actions/runs/7711662213/job/21017463546

So maybe the changes are not correct?

leofeyer pushed a commit that referenced this pull request Mar 5, 2024
Description
-----------

Same as #6801 for the `MakeResponsePrivateListener`.

Commits
-------

611dec1 Only make a response private if the session was not empty
leofeyer pushed a commit to contao/core-bundle that referenced this pull request Mar 5, 2024
Description
-----------

Same as contao/contao#6801 for the `MakeResponsePrivateListener`.

Commits
-------

611dec17 Only make a response private if the session was not empty
@fritzmg fritzmg mentioned this pull request Mar 22, 2024
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.

None yet

4 participants