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

TestCsrfTokenRepository should delegate to the configured CsrfTokenRepository #13082

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chschu
Copy link

@chschu chschu commented Apr 24, 2023

I'm creating this as a draft, because there are some failing tests I need to fix before this can be merged.

CsrfRequestPostProcessor currently reconfigures CsrfFilter with a TestCsrfTokenRepository that delegates to a newly created HttpSessionCsrfTokenRepository. This is fine as long as the configured CsrfTokenRepository is a HttpSessionCsrfTokenRepository as well, because it accesses the same session attribute.

However, if the configured CsrfTokenRepository is not a HttpSessionCsrfTokenRepository, this change affects (and may break) any test that uses the same cached ApplicationContext, because it now basically uses a HttpSessionCsrfTokenRepository instead of the configured CsrfTokenRepository.

TestCsrfTokenRepository should delegate to the configured CsrfTokenRepository to avoid affecting unrelated tests.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 24, 2023
@chschu chschu force-pushed the bugfix/TestCsrfTokenRepository-delegate-to-actual-CsrfTokenRepository branch from 847b02e to 9a736dd Compare April 26, 2023 14:00
@chschu chschu force-pushed the bugfix/TestCsrfTokenRepository-delegate-to-actual-CsrfTokenRepository branch from 9a736dd to b673748 Compare January 29, 2024 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants