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

Polish WebSecurityConfiguration #16348

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

Conversation

kse-music
Copy link
Contributor

  1. Remove unused code,webSecurityConfigurers and beanClassLoader

  2. Use ObjectProvider instead of @Autowired HttpSecurity because when the user provides a SecurityFilterChain bean, it will still inject HttpSecurity. Since HttpSecurity is a prototype bean, this avoids its instantiation.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 27, 2024
@rwinch rwinch self-requested a review January 7, 2025 20:16
@rwinch
Copy link
Member

rwinch commented Jan 7, 2025

Thanks for the PR @kse-music!

Can you please split this up into two commits and pull requests? This helps us to better track changes.

Use ObjectProvider instead of @Autowired HttpSecurity because when the user provides a SecurityFilterChain bean, it will still inject HttpSecurity. Since HttpSecurity is a prototype bean, this avoids its instantiation.

Can you please add a test that includes a comment to the PR (e.g. gh-16348) to verify that is the case?

Copy link
Member

@rwinch rwinch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again for the PR. I've provided feedback in a comment that needs addressed.

@kse-music
Copy link
Contributor Author

@rwinch All the feedback has been addressed as you expected.

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.

3 participants