Delegate user credential checking to PasswordChecker #584
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Storage component expects user/password to come from the database. I have a situation in which I would like to forward the credentials along to an external processor (LDAP, in my case). My first thought was to use a custom encoder with an LDAP service embedded, but that won't work because the encoder factory generates the encoder. Second approach is to extend the storage class or delegate to it. In either case, this leads to a lot of boilerplate (either messy constructor in the first case, or a lot of unnecessarily overridden methods to fulfill the interfaces in the latter), when I really only needed one line changed.
This approach allows the storage service to delegate the actual password checking, which defaults to using the encoder as before (no BC break). With this change, it's easy to override the password checker to implement whatever functionality is needed to check the password. I also made the password checker not nullable and swapped the order of the arguments to keep the nullable argument at the end. It didn't make sense to me that prior logic had the encoder factory as nullable, but called
getEncoder
on it directly without checking for null anyway.This PR represents the simplest approach I could think of that provides extensibility without a BC break. Please let me know if there's a more Symfony-esque way of accomplishing this - I'm open to placing the functionality somewhere else, renaming files/methods, etc.