tasansga / haraka-ldap

Developing LDAP plugins for Haraka
MIT License
5 stars 6 forks source link

ideas #1

Closed msimerson closed 7 years ago

msimerson commented 7 years ago

First off, nice job!

I was just working on the two LDAP plugins currently in Haraka, and what I'm heading towards is collapsing them into a single haraka-plugin-ldap plugin that has options for enabling each feature (recipient validation, auth, etc..).

Might you consider doing that with this, and then we could deprecate the existing plugins and replace them with this one?

msimerson commented 7 years ago

pros

cons

tasansga commented 7 years ago

You got a good point there! I'm certainly open to the approach. One possible con: As far as I understand Haraka's plugin architecture, plugin order matters. So, if we have only one LDAP plugin, that would limit our possibilities in what we can do with LDAP, like flagging a user as spammer long after we've authenticated him.

msimerson commented 7 years ago

As far as I understand Haraka's plugin architecture, plugin order matters.

Plugin order can matter but it usually does not. When it does matter is when one plugin depends on the results of another plugin that runs on the same hook (ie, two RCPT plugins, or two DATA plugins), or when one plugin never gets a chance to run because some previous one that runs on the same hook ended the connection before the latter got its chance. We've mostly solved the latter problem by defining the connect_init hook, which always gets run for every plugin.

I avoid the first issue by configuring every plugin to be non-blocking and letting my karma plugin do all the connection termination and cleanup. @smfreegard does it a little differently with delay_deny.

msimerson commented 7 years ago

Shorter version of the above paragraph: don't worry about it. :-)

tasansga commented 7 years ago

After thinking things through, I agree. It's the smarter way for LDAP support. So, I'll refactor the plugins.

Major goals to reach:

Also, if someone shows up with a usecase that a single ldap plugin can't fully cover, there should still be a way to easily extend the generic ldap plugin by adding separate ldap-plugins.

tasansga commented 7 years ago

The plugins are now integrated in the current release: https://www.npmjs.com/package/haraka-plugin-ldap