FriendsOfSymfony / FOSOAuthServerBundle

A server side OAuth2 Bundle for Symfony
1.09k stars 451 forks source link

Delegate user credential checking to PasswordChecker #584

Open natelenart opened 6 years ago

natelenart commented 6 years ago

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.