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

InitializeUserDetailsBeanManagerConfigurer does configure a DaoAuthenticationProvider without Encoder although there are encoders found #15751

Open
tkrah opened this issue Sep 6, 2024 · 1 comment
Labels
status: waiting-for-triage An issue we've not yet triaged type: bug A general bug

Comments

@tkrah
Copy link

tkrah commented Sep 6, 2024

Hi,

using spring-security 6.3.3 the InitializeUserDetailsBeanManagerConfigurer does have this code:

PasswordEncoder passwordEncoder = getBeanOrNull(PasswordEncoder.class);

It does look for a password encoder and if this one returns null, a new DaoAuthenticationProvider(); is used.

The problem is, that if more than one encoder is in the context, getBeanOrNull(PasswordEncoder.class) does return null too. This is imho unexpected see here

Expected behavior
My expectation would be, that if more than one PasswordEncoder is found, that the context build fails here and issues an error OR tell the user with a WARN message that the first one found is used.

But simple not using any encoder at all, although there are some configured is a problem. The problem was found, because an upstream project used by me configured its own encoder (which is not that easy to discover with component scanning enabled) and I had myself already one configured and wondered, why NO encoder at all was registered on the DaoAuthenticationProvider - a warning or an error would be nice here.

@tkrah tkrah added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Sep 6, 2024
@Kehrlann
Copy link
Contributor

Hello @tkrah , thank you for the report. I agree, this is surprising behavior.

Unfortunately, we can't afford to make the context build fail in the current 6.x line ; there may be users who do not rely on global authentication and have two password encoders.

While we could add a warning, this means we should probably add a warning for UserDetailsPasswordService and CompromisedPasswordChecker. It also means we would need to consider when users want to silence this warning, as we've had the case recently (gh-15538), which makes the warning more complex.

For now, let's leave this issue open, and see if other users in the community request this too by upvoting the issue.

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 type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants