[SSSD] [PATCHES] Additional AD provider enhancements

Stephen Gallagher sgallagh at redhat.com
Fri Jul 6 15:48:27 UTC 2012


On Fri, 2012-07-06 at 11:20 -0400, Simo Sorce wrote:
> 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.


Pushed to master.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120706/990c14ef/attachment.sig>


More information about the sssd-devel mailing list