I don't think we should be prefixing any of the algorithms with password4j because validating bcrypt work with any bcrypt implementation. Since we already have support for all of these algorithms, I'd prefer to just remove these for now.

After this ticket, we might consider adding a ticket Classpath Selection for PasswordEncoder Implementation. Before implementing it, we'd want to discuss how it would work and determine if this is something we want to do.

We do not use wildcard imports. You should run ./gradlew check to perform all checks (including checkstyle). You can run ./gradlew checkstyleMain checkstyleTest to just do checkstyle tests to speed things up.

Since this is a new feature it would get merged with 7.0

Please remove Password4jAlgorithm.

Generally, we try to avoid implementation specific APIs on our interfaces. However, behind an interface we encourage injecting the implementation directly rather than abstracting it out. This gives the users as much flexibility as possible and reduces the complexity.

This does not validate that the HashingFunction and the Password4jAlgorithm are aligned. For example, this should fail but doesn't

new Password4jPasswordEncoder(
 AlgorithmFinder.getBcryptInstance(),
 Password4jPasswordEncoder.Password4jAlgorithm.ARGON2);

As mentioned previously, I think we can remove Password4jAlgorithm all together. However, I wanted to point this out as another reason we do not want/need it.

Instead, please have a single constructor that allows injecting a non-null HashingFunction. Please be sure to document how to create instances in the javadoc. For example, defaults can be obtained from AlgorithmFinder. However, an example of a single algorithm that has been customized would be useful as well.

I think that all of the static factory methods can be removed. Users should be encouraged to take advantage of password4j's AlgorithmFinder API to create a HashingFunction instance. This reduces the maintenance for Spring Security and also gives users more flexibility.

We generally try to avoid exposing getters as this breaks encapsulation. For example, a user may inject a Jackson ObjectMapper and that might be used internally for a while.

public class SomeClass {
 // jackson object mapper
 private ObjectMapper objectMapper;
 public SomeClass(ObjectMapper objectMapper) {
 this.objectMapper = Objects.requireNonNull(objectMapper);
 }
 public SomePojo convert(String string) {
 // ... use objectMapper for conversion
 }
 // makes refactoring hard
 public ObjectMapper getObjectMapper() {
 return this.objectMapper;
 }
}

If we expose a getObjectMapper() method, then it makes breaks encapsulation and makes refactoring more difficult. For example, if we want more flexibility we might decide to support a Converter

public class SomeClass {
 private Converter converter;
 public SomeClass(Converter converter) {
 this.converter = Objects.requireNonNull(converter);
 }
 public SomeClass(ObjectMapper objectMapper) {
 this(new MappingJackson2HttpMessageConverter(objectMapper));
 }
 public SomePojo convert(String string) {
 // ... can update to use converter because its internal
 }
 // makes refactoring hard
 public ObjectMapper getObjectMapper() {
 // Cannot be passive here because the converter replaced ObjectMapper!!
 }
}