samba - Nov 2015 - [PATCH] Re: Samba 4.1.17 classic update w/LDAP - parsing error

On Sat, 2015-10-31 at 10:45 +0000, Rowland Penny wrote:
> On 31/10/15 08:51, Andrew Bartlett wrote:
> > On Wed, 2015-10-28 at 14:35 +0100, Mgr. Peter Tuharsky wrote:
> > > Hallo,
> > > 
> > > I have two news. The first one: the patch probably works. Second:
> > > there
> > > is another bug.
> > > 
> > > When I encountered the bug again after patching, I have raised
> > > debug
> > > level and figured out that the problem is with user
"guest" - he
> > > was
> > > in
> > > our old domain, however samba-tool probably creates him
> > > automatically
> > > and then couldn't import him.
> > > 
> > > So, please fix the tool so that it ignores such user, or update
> > > the
> > > DOCS
> > > so that forbidden users are known for admin before attempting the
> > > classicupdate.
> > > 
> > > The import FINALLY works with patched 4.3.1. But when I tested
> > > again
> > > with 4.1.17, it ends up with the bug. So the patch seems working
> > > for
> > > its
> > > purpose, but there is the bug with guest user and that needs to
> > > get
> > > fixed.
> > You are welcome to apply for an account to change the wiki page:
> > 
> > 
> > https://wiki.samba.org/index.php/Samba4/samba-tool/domain/classicup
> > grad
> > e/HOWTO#Preparations
> > 
> > Thanks!
> > 
> > Andrew Bartlett
> 
> Rather than adding something to the wiki, how about adding something 
> like this to upgrade.py:
> 
>              if username.lower() == 'guest':
>                  logger.warn("You have a user '%s' in your
existing
> directory, \
> this will be replaced by the builtin user 'Guest") %
> (userdata[username])
> 
> I 'think' what happened was the upgrade ran the intial provision
and
> this created the builtin user 'Guest' and then when the upgrade
tried
> to 
> add the user 'guest', this failed because it already existed.
The number of potential conflicts here seems endless, I would rather
not list them by hand in the code.  We can either have it print a
message based on a set-intersection with a search just after the
provision, and/or we can improve how the 'add' errors are presented to
make them clearer.

Andrew Bartlett

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba

On 01/11/15 18:30, Andrew Bartlett wrote:
> On Sat, 2015-10-31 at 10:45 +0000, Rowland Penny wrote:
>> On 31/10/15 08:51, Andrew Bartlett wrote:
>>> On Wed, 2015-10-28 at 14:35 +0100, Mgr. Peter Tuharsky wrote:
>>>> Hallo,
>>>>
>>>> I have two news. The first one: the patch probably works.
Second:
>>>> there
>>>> is another bug.
>>>>
>>>> When I encountered the bug again after patching, I have raised
>>>> debug
>>>> level and figured out that the problem is with user
"guest" - he
>>>> was
>>>> in
>>>> our old domain, however samba-tool probably creates him
>>>> automatically
>>>> and then couldn't import him.
>>>>
>>>> So, please fix the tool so that it ignores such user, or update
>>>> the
>>>> DOCS
>>>> so that forbidden users are known for admin before attempting
the
>>>> classicupdate.
>>>>
>>>> The import FINALLY works with patched 4.3.1. But when I tested
>>>> again
>>>> with 4.1.17, it ends up with the bug. So the patch seems
working
>>>> for
>>>> its
>>>> purpose, but there is the bug with guest user and that needs to
>>>> get
>>>> fixed.
>>> You are welcome to apply for an account to change the wiki page:
>>>
>>>
>>> https://wiki.samba.org/index.php/Samba4/samba-tool/domain/classicup
>>> grad
>>> e/HOWTO#Preparations
>>>
>>> Thanks!
>>>
>>> Andrew Bartlett
>> Rather than adding something to the wiki, how about adding something
>> like this to upgrade.py:
>>
>>               if username.lower() == 'guest':
>>                   logger.warn("You have a user '%s' in
your existing
>> directory, \
>> this will be replaced by the builtin user 'Guest") %
>> (userdata[username])
>>
>> I 'think' what happened was the upgrade ran the intial
provision and
>> this created the builtin user 'Guest' and then when the upgrade
tried
>> to
>> add the user 'guest', this failed because it already existed.
> The number of potential conflicts here seems endless, I would rather
> not list them by hand in the code.  We can either have it print a
> message based on a set-intersection with a search just after the
> provision, and/or we can improve how the 'add' errors are presented
to
> make them clearer.
>
> Andrew Bartlett
>
OK Andrew, if you don't want to put something into upgrade.py (something 
I can quite understand), I will discuss this with Marc and see if we can 
put something on the wiki, after all, we are only talking about 3 or 
possibly 4 users created by the provision, Administrator, guest, krbtgt 
and possibly dns-DCNAME if using Bind9

Rowland

I think the tool should act consistently. I.e., it can cope with
automatically provisioned groups - when they exist in imported domain
too, it simply displays a warning "group already exists" and goes on.

I think this is the right behaviour that should apply to users too.

Peter

Dňa 01.11.2015 o 19:54 Rowland Penny napísal(a):
> On 01/11/15 18:30, Andrew Bartlett wrote:
>> On Sat, 2015-10-31 at 10:45 +0000, Rowland Penny wrote:
>>> On 31/10/15 08:51, Andrew Bartlett wrote:
>>>> On Wed, 2015-10-28 at 14:35 +0100, Mgr. Peter Tuharsky wrote:
>>>>> Hallo,
>>>>>
>>>>> I have two news. The first one: the patch probably works.
Second:
>>>>> there
>>>>> is another bug.
>>>>>
>>>>> When I encountered the bug again after patching, I have
raised
>>>>> debug
>>>>> level and figured out that the problem is with user
"guest" - he
>>>>> was
>>>>> in
>>>>> our old domain, however samba-tool probably creates him
>>>>> automatically
>>>>> and then couldn't import him.
>>>>>
>>>>> So, please fix the tool so that it ignores such user, or
update
>>>>> the
>>>>> DOCS
>>>>> so that forbidden users are known for admin before
attempting the
>>>>> classicupdate.
>>>>>
>>>>> The import FINALLY works with patched 4.3.1. But when I
tested
>>>>> again
>>>>> with 4.1.17, it ends up with the bug. So the patch seems
working
>>>>> for
>>>>> its
>>>>> purpose, but there is the bug with guest user and that
needs to
>>>>> get
>>>>> fixed.
>>>> You are welcome to apply for an account to change the wiki
page:
>>>>
>>>>
>>>>
https://wiki.samba.org/index.php/Samba4/samba-tool/domain/classicup
>>>> grad
>>>> e/HOWTO#Preparations
>>>>
>>>> Thanks!
>>>>
>>>> Andrew Bartlett
>>> Rather than adding something to the wiki, how about adding
something
>>> like this to upgrade.py:
>>>
>>>               if username.lower() == 'guest':
>>>                   logger.warn("You have a user '%s' in
your existing
>>> directory, \
>>> this will be replaced by the builtin user 'Guest") %
>>> (userdata[username])
>>>
>>> I 'think' what happened was the upgrade ran the intial
provision and
>>> this created the builtin user 'Guest' and then when the
upgrade tried
>>> to
>>> add the user 'guest', this failed because it already
existed.
>> The number of potential conflicts here seems endless, I would rather
>> not list them by hand in the code.  We can either have it print a
>> message based on a set-intersection with a search just after the
>> provision, and/or we can improve how the 'add' errors are
presented to
>> make them clearer.
>>
>> Andrew Bartlett
>>
>
> OK Andrew, if you don't want to put something into upgrade.py
> (something I can quite understand), I will discuss this with Marc and
> see if we can put something on the wiki, after all, we are only
> talking about 3 or possibly 4 users created by the provision,
> Administrator, guest, krbtgt and possibly dns-DCNAME if using Bind9
>
> Rowland
>
>