On Aug 6, 2007, at 7:31 PM, ando@sys-net.it wrote:
Sebastian,
Thanks for the contribution.
I have a few comments (also gathered from others):
- you should provide patches against HEAD code; there has been some
limited changes in the API related to overlay initialization and so.
- you could try to rework the overlay to avoid any specific reference
to Active Directory, since your cache should apply to any remote system implementing Kerberos V. It could be abstracted even more, to act as a replacement of saslauthd, by allowing it to auth via LDAP, pam and more, not just Kerberos.
s/to act as a replacement/to defer external authentication to saslauthd or the like/
In slapd(8), we deferred verification against external password stores to saslauthd via the {SASL} userPassword mechanism, thus avoiding needing to implement and support each possible external password store (such as a KDC). This module should likewise avoid supporting (some subset of) external passwords stores. saslauthd(8) can easily be extended (or replaced) to support additional password stores as needed. As provided in Cyrus SASL today, it supports both LDAP and Kerberos as external password stores.
- you should add a (configurable) TTL, so that the cache could
eventually be notified of an account lockout at the remote server's side.
- you should add support for dynamic configuration, so that the
module can fit into the new configuration paradigm for possible release with 2.4.
- you should follow coding guidelines (indentation and so) as in most
of the code.
p.