[SCM] Samba Shared Repository - branch master updated - release-4-0-0alpha6-917-g8e19a28

[Jeremy Allison]

On Tue, Feb 17, 2009 at 02:32:56PM -0800, Tim Prouty wrote:
>
> On Feb 16, 2009, at 8:18 PM, Jeremy Allison wrote:
>
>> This looks ugly to me. I haven't gone through the
>> code carefully enough yet, but I will :-). I'm tempted
>> to push this back as a "vendor-specific" change, but
>> I need to understand it first, and this code is very
>> dangerous to change for historical reasons. Please
>> consider this commit temporary until we've reviewed
>> it thoroughly :-).
>
>
> After a lot of internal discussion we have decided that we would
> be OK with keeping this as an internal patch.  This patch solves
> a specific case of a general problem that we have had multiple
> high-profile customers request.  The general problem is that
> customers in mixed unix/windows environments want the ability to
> have group transitivity between the SIDs in their nt token and
> their unix group membership in nis/ldap.  It turns out that we can
> make most of those customers happy by just ignoring the group
> membership in the nt token and replacing it with the user's unix
> group membership.  This was the goal of Zach's patch, and we
> believed it was something that would be generally useful to other
> users of samba who have the same requirements as our customers.
>
> In the future we would like to implement full group transitivity
> support which may include introducing a new interface into this
> code path that allows for modularization and some serious
> cleanup.  The goal would be to make the token an opaque structure
> to create a cleaner abstraction that would allow for more
> flexibility going forward.  Since this would almost cetainly make
> this new parameter obsolete, I can live with making this patch
> vendor-specific and removing it from the upstream repository if
> it's not useful for others.
>
> I'll plan on reverting it tomorrow to give some time for any
> further comment.  We sincerely appreciate everyone's feedback on
> this patch.

Ok, I've gone though it pretty carefuly and it's still
really ugly :-). I can see the need for it, but the change
itself is still pretty ugly and complicates some already
complex code paths.

How about changing the patch to call a new function
create_token_from_username_nss() which just splits out
the code paths from create_token_from_username() you
actually use to do this ? That at least would make the
logic a lot easier to follow, and would not be a danger
for future maintainers of create_token_from_username()
and removes the hideous "force_nss" parameter change.

Does anyone else have any thoughts ? Don't remove it
quite just yet until we've finished discussing it.

Jeremy.