Broken idmap interface design

[Jeremy Allison]

On Thu, Apr 19, 2007 at 09:49:21AM -0500, Gerald (Jerry) Carter wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Simo,
> 
> > On Thu, 2007-04-19 at 23:30 +1000, Luke Howard wrote:
> >> Sorry to jump in here, one thing I'd like to see 
> >> in idmap_ad is support for using the Global Catalog. Shouldn't
> >> be too hard. Thoughts?
> > 
> > Well IIRC rfc2307 attributes are not exposed via GC by 
> > default, so to use it we must have fallback code in place.
> > Not that hard, but I guess this is more of a 3.0.26 feature.
> > I am working only to stabilize the code for offline
> > usage right now.
> 
> It's actually worse than that.  The idmap interface is
> badly broken.  I hate to say this, but the calls into
> winbindd from the idmap child has to go.  I know how you
> arrived at the design assumptions.
> 
> You designed the unixids_to_sids() and sids_to_unixids()
> with the assumption that the idmap plugin would have
> knowledge about the SID type.  I didn't catch this
> because the backend I'm using for primary testing operates
> similarly to idmap_ad and can obtain the SID type based
> on LDAP searches.  This is ok for something like idmap_ad
> which can get the information.  But the general and
> default case is idmap_tdb (or even idmap_ldap).
> 
> Requiring the idmap_tdb code (or idmap_rid) to issues a
> winbindd client call is wrong and a layering violation.  The
> caller should specify the SID type which is exactly what
> the WINBINDD_SID_TO_UID, et. al. calls used to do.

Indeed. Looking at this interface cold after ignoring
it for a while I think the SID_TYPE enum needs to be
present as input on all calls into a "map SID to XXX".

Jeremy.