[SSSD] [PATCHES] Additional AD provider enhancements

Simo Sorce simo at redhat.com
Fri Jul 6 15:20:36 UTC 2012


On Fri, 2012-07-06 at 10:59 -0400, Stephen Gallagher wrote:
> On Fri, 2012-07-06 at 08:09 -0400, Simo Sorce wrote:
> > On Thu, 2012-07-05 at 20:53 -0400, Stephen Gallagher wrote:
> > > 
> > > 
> > > These patches should finish off the AD provider functionality for
> > > 1.9.0.
> > > All of these patches depend on those in the thread "Add Active
> > > Directory
> > > identity, auth and chpass providers to SSSD"
> > > 
> > > Patch 0001: Add another option 'ad_keytab' that unifies the
> > > krb5_keytab
> > > and ldap_krb5_keytab options.
> > 
> > Nack, please just use krb5_keytab, let's not proliferate options names
> > when unnecessary.
> > 
> 
> Agreed and fixed.
> 
> > > Patch 0002: Add manpages and SSSDConfig API references for new AD
> > > provider options. This patch depends on "MAN: Unify "SEE ALSO"
> > > sections"
> > 
> > Here you also need to remove ad_keytab, but in it's place you may
> > specify that for the ad provider krb5_keytab is used both for validation
> > and for protecting the LDAP connections.
> > 
> 
> I didn't bother, I just removed the reference entirely.
> 
> > > Patch 0003: Add a function to the confdb to allow us to write a
> > > boolean
> > > value into the configuration database. (Used in patch 0004)
> > 
> > Why didn't you use ldb_msg_add_string() to set the bool ?
> 
> Because I forgot that function existed. I've made the change you
> recommended and it's MUCH nicer.
> 
> > 
> > ldb_msg_add_empty(msg, attribute, LDB_FLAG_MOD_REPLACE, NULL);
> > If (val) {
> >     ldb_msg_add_string(msg, attribute, "True");
> > } else {
> >     ldb_msg_add_string(msg, attribute, "False");
> > }
> > 
> > would make the code  a lot more compact.
> > 
> > > Patch 0004: During initial configuration of the AD provider, force the
> > > option case_sensitive to false, since all AD deployments are
> > > case-insensitive. We do this here (and set it in the confdb) so that
> > > the
> > > responders will pick it up. Responders all start after the providers
> > > have registered with the monitor (or if the monitor gives up waiting
> > > after five seconds), so there should be no race here.
> > 
> > in confdb_set_bool() please use dom->case_sensitive that you just set,
> > and not 'false' directly, this way if we ever need a change (or someone
> > copy/paste the code elsewhere) we are consistent and only one line needs
> > to be changed to change behavior.
> 
> Sure, easy enough. Done.
> 
> New patches attached. They have been tested.

Ack to all four.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York




More information about the sssd-devel mailing list