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

[John Magne]

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 -----
> From: "Christina Fu" <cfu at redhat.com>
> To: "pki-devel" <pki-devel at redhat.com>
> Sent: Wednesday, August 5, 2015 4:43:16 PM
> Subject: [Pki-devel] [PATCH]	pki-cfu-0090-Ticket-1531-Directory-auth-plugin-requires-LDAP-anon.patch
> 
> This patch is for ticket
> https://fedorahosted.org/pki/ticket/1531 Directory auth plugin requires
> LDAP anonymous binds
> 
>      This patch adds a feature to allow a directory based authentication
> plugin
>      to use bound ldap conneciton instead of anonymous.
>      Two files need to be edited
>      1. <instance>/conf/password.conf
>        add a "tag" and the password of the binding user dn to the file
>        e.g. externalLDAP=password123
>      2. <instance>/ca/CS.cfg
>        add the tag to cms.passwordlist:
>        e.g. cms.passwordlist=internaldb,replicationdb,externalLDAP
>        add the prefix of the auths entry for the authentication instance
>        e.g. externalLDAP.prefix=auths.instance.UserDirEnrollment
>        add relevant entries to the authenticaiton instance
>        e.g. auths.instance.UserDirEnrollment.ldap.ldapBoundConn=true
> auths.instance.UserDirEnrollment.ldap.ldapauth.authtype=BasicAuth
> auths.instance.UserDirEnrollment.ldap.ldapauth.bindDN=uid=rhcs,ou=serviceaccounts,dc=EXAMPLE,dc=com
> auths.instance.UserDirEnrollment.ldap.ldapauth.bindPWPrompt=externalLDAP
> 
> The code has been tested to work.
> The code (in its plugin form) has also been tested to work successfully
> with an ldap server that has its anonymous bind turned off.
> 
> thanks,
> Christina
> 
> _______________________________________________
> Pki-devel mailing list
> Pki-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel