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

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
@sjohnr
Copy link
Member

sjohnr commented Jan 30, 2025

@chschu, I'm going through some older PRs and noticed that this has not had an update in over a year. I'm going to go ahead and close this for now. If you would like to work on it again, feel free to create a new PR from a fresh (or rebased) branch and we'll take a look. Or if you'd like this PR reopened, please add a comment and ping myself or someone on the team and we'll be happy to reopen it for you.

@sjohnr sjohnr closed this Jan 30, 2025
@sjohnr sjohnr self-assigned this Jan 30, 2025
@sjohnr sjohnr added in: test An issue in spring-security-test type: enhancement A general enhancement status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test An issue in spring-security-test status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants