Re: [Pki-devel] [PATCH] pki-cfu-0090-Ticket-1531-Directory-auth-plugin-requires-LDAP-anon.patch

This looks fine , with the caveat of tested to work of course, which you have already stated. Just a couple of minor things, and then a conditional ACK 1. In CMSEngine: this bloc: if (tag.equals("internaldb")) { authType = config.getString("internaldb.ldapauth.authtype", "BasicAuth"); @@ -382,8 +384,35 @@ public class CMSEngine implements ICMSEngine { binddn = config.getString("ca.publish.ldappublish.ldap.ldapauth.bindDN"); } else { - // ignore any others for now - continue; + /* + * This section assumes a generic format of + * <prefix>.ldap.xxx + * where <prefix> is specified under the tag substore + * + * e.g. if tag = "externalLDAP" + * cms.passwordlist=...,externalLDAP + * externalLDAP.prefix=auths.instance.UserDirEnrollment + * + * auths.instance.UserDirEnrollment.ldap.ldapauth.authtype=BasicAuth + * auths.instance.UserDirEnrollment.ldap.ldapauth.bindDN=cn=Corporate Directory Manager + * auths.instance.UserDirEnrollment.ldap.ldapauth.bindPWPrompt=externalLDAP + * auths.instance.UserDirEnrollment.ldap.ldapconn.host=host.example.com + * auths.instance.UserDirEnrollment.ldap.ldapconn.port=389 + * auths.instance.UserDirEnrollment.ldap.ldapconn.secureConn=false + */ + String prefix = config.getString(tag + ".prefix"); + System.out.println("CMSEngine.initializePasswordStore(): prefix=" + prefix); + authType = config.getString(prefix +".ldap.ldapauth.authtype", "BasicAuth"); + System.out.println("CMSEngine.initializePasswordStore(): authType " + authType); + if (!authType.equals("BasicAuth")) + continue; In the else clause could we short circuit processing earlier if we find something we don't like for instance: String prefix = config.getString(tag + ".prefix"); No need to go on if that fails. The same for the rest of the values checked. 2. Can we rename "prefix" to something more friendly to the user like "auths-prefix" to it is clearer to the user what the exact purpose of that setting is. ----- Original Message -----