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

[Christina Fu]

pushed to master:
commit c13593770108b6d683ab3d3b43b92d67ac64a1ef

thanks,
Christina

On 08/07/2015 10:44 AM, John Magne wrote:
> After the fixes and some further discussion over the connection issue being resolved:
>
> ACK
>
> ----- Original Message -----
> From: "Christina Fu" <cfu at redhat.com>
> To: "pki-devel" <pki-devel at redhat.com>
> Sent: Thursday, August 6, 2015 5:51:03 PM
> Subject: Re: [Pki-devel] [PATCH]	pki-cfu-0090-Ticket-1531-Directory-auth-plugin-requires-LDAP-anon.patch
>
> Updated per jack's suggestion.
>
> Also, during testing, one issue was discovered where a failed
> authentication would cause the next one to fail.  Investigation shows
> that a bad connection gets recycled back to the pool and somehow the
> underlying connection framework does not seem to clear it out.
> My solution was to just disconnect the bad connection once it's
> determined that it's botched, before it is returned back to the pool.
> That seems to reset it and works well now.
> Since this extra disconnect code needs to go into all authentication
> plugins that extends the DirBasedAuthentication, I have to modify all
> four of them to do the disconnect in case of ldap authentication failure.
>
> thanks,
> Christina
>
> On 08/05/2015 05:57 PM, John Magne wrote:
>> 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
>
> _______________________________________________
> Pki-devel mailing list
> Pki-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel