Added dynamic LDAP configuration feature.

[bnielsen1965]

I required the ability to dynamically adjust LDAP settings based on the authentication request and passport-ldapauth provides the option to pass a configuration function in place of a static configuration option, but the feathers-authentication-ldap package only supported static configuration.

This pull request adds a server configuration option that can be used to define an asynchronous method for LDAP settings that will override any default or static settings.

[bnielsen1965]

Anyone have time to review this pull request, provide some feedback?

[daffl]

Sorry about that and thank you for the PR! Could we just make the options object a promise instead and wait for it to resolve? We are trying to have all new asynchronous APIs work with Promises instead of callbacks.

Something like

function getConfiguration() {
  return Promise.resolve({ someOptions });
}

app.configure(ldap(getConfiguration())

And it would wait for the Promise to resolve before doing any of the setup.

[bnielsen1965]

Looking into this now, I'll get back to you when I have it ready.

[daffl]

Cool. Let me know if you need anything.

[bnielsen1965]

Ready for another review, but I need to do some explaining.

I don't think what I came up with is exactly what you had suggested. I could not find a way around having a method that uses a callback because this is required by passport-ldapauth if you need per request configuration. Configuration in the feathers setup method would only support a one time configuration.

So to get around this what I ended up doing is implementing the function with a callback in the authentication module while accepting a function that returns a Promise as the configuration option.

I think the code is much cleaner than what I had before and it eliminates that ugly configuration option used to pass a function. Now you pass either a static configuration or you pass a function that will return a Promise which resolves the configuration.

When you have some time take a look and let me know what you think.

[bnielsen1965]

Not trying to be a pest, honest, but can I get another review on this PR? ;)

I'm using this module in a feathersjs based project and it would be awesome if I could use the official npm linked module in place of my fork.

If I need to put more work into the changes then let me know and I'll jump on it. Thanks

[ekryski]

Heyo! Sorry @bnielsen1965 I need to double check that this repo is connected to Slack. Can be hard to keep tabs on everything across all repos. Looking....

[bnielsen1965]

Closing this PR because I am working on a new PR.