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

Replace SpringUtils in DelegatingPasswordEncoder.UnmappedIdPasswordEncoder #16442

Open
ChristianHoesel opened this issue Jan 18, 2025 · 3 comments · May be fixed by #16479
Open

Replace SpringUtils in DelegatingPasswordEncoder.UnmappedIdPasswordEncoder #16442

ChristianHoesel opened this issue Jan 18, 2025 · 3 comments · May be fixed by #16479
Assignees
Labels
in: crypto An issue in spring-security-crypto type: enhancement A general enhancement

Comments

@ChristianHoesel
Copy link

Expected Behavior

The class org.springframework.security.crypto.password.DelegatingPasswordEncoder.UnmappedIdPasswordEncoder should not use org.springframework.util.StringUtils to allow usage in non Spring environments.

Current Behavior

org.springframework:spring-core has to be within my dependencies to use the DelegatingPasswordEncoder and it's only used in one methode

public boolean matches(CharSequence rawPassword, String prefixEncodedPassword) {

Right now the dependency to org.springframework:spring-core is optional and for the usage of the most PasswordEncoder implementations not necessary.

The Usage of StringUtils can easily be replaced.

Context
I am having an Eclipse RCP (OSGi) project, where I want to use the DelegatingPasswordEncoder right away without adding org.springframework:spring-core to my dependencies.

@ChristianHoesel ChristianHoesel added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Jan 18, 2025
@kse-music
Copy link
Contributor

I found that in KeyStoreKeyFactory not only StringUtils but also Resource are used.

@ChristianHoesel
Copy link
Author

I found that in KeyStoreKeyFactory not only StringUtils but also Resource are used.

You are right, Resource from spring-core is used in KeyStoreKeyFactory, but as far as I see this does not interfere with using DelegatingPasswordEncoder and PasswordEncoderFactories.

So after replacing StringUtils the optional dependency to spring-core still exists, but it is a little less.

@sjohnr sjohnr self-assigned this Jan 23, 2025
@sjohnr sjohnr added in: crypto An issue in spring-security-crypto and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 23, 2025
@sjohnr
Copy link
Member

sjohnr commented Jan 23, 2025

@ChristianHoesel thanks for reaching out!

So after replacing StringUtils the optional dependency to spring-core still exists, but it is a little less.

I can understand wanting to reduce/remove its use in the spring-security-crypto module. I think the issue is that spring-core usage is very common throughout the project and so it may pop up again. Removing this usage doesn't guarantee you won't run into issues in the future. Having said that,

The Usage of StringUtils can easily be replaced.

Would you like to submit a PR? We're happy to look at improving this situation since the crypto module definitely has many uses such as yours.

ChristianHoesel added a commit to ChristianHoesel/spring-security that referenced this issue Jan 24, 2025
Removes the use of `org.springframework.util.StringUtils` from `DelegatingPasswordEncoder`

Closes spring-projectsgh-16442

Signed-off-by: Christian Hösel <[email protected]>
@ChristianHoesel ChristianHoesel linked a pull request Jan 24, 2025 that will close this issue
ChristianHoesel added a commit to ChristianHoesel/spring-security that referenced this issue Jan 24, 2025
Removes the use of `org.springframework.util.StringUtils` from `DelegatingPasswordEncoder`

Closes spring-projectsgh-16442

Signed-off-by: Christian Hösel <[email protected]>
ChristianHoesel added a commit to ChristianHoesel/spring-security that referenced this issue Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: crypto An issue in spring-security-crypto type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants