Improve badPwdCount, lastLogon and logonCount updates

[Andrew Bartlett]

On Sat, 2016-07-09 at 07:51 +1200, Andrew Bartlett wrote:
> On Fri, 2016-07-08 at 16:10 +0200, Stefan Metzmacher wrote:
> > Hi Andrew,
> > 
> > > > > UNEXPECTED(failure):
> > > > > samba4.tombstone_reanimation.python.tombstone_reanimation.Res
> > > > > to
> > > > > reUs
> > > > > erOb
> > > > > jectTestCase.test_restore_user(ad_dc_ntvfs)
> > > > > REASON: Exception: Exception: Traceback (most recent call
> > > > > last):
> > > > >   File
> > > > > "/data/samba/git/samba/source4/dsdb/tests/python/tombstone_re
> > > > > an
> > > > > imat
> > > > > ion.
> > > > > py", line 364, in test_restore_user
> > > > >     self.assertAttributesEqual(obj, attr_orig, obj_restore,
> > > > > attr_rest)
> > > > >   File
> > > > > "/data/samba/git/samba/source4/dsdb/tests/python/tombstone_re
> > > > > an
> > > > > imat
> > > > > ion.
> > > > > py", line 88, in assertAttributesEqual
> > > > >     % (str(attrs_orig.difference(attrs_rest)),
> > > > > str(attrs_rest.difference(attrs_orig))))
> > > > > AssertionError: Actual object does not have expected
> > > > > attributes,
> > > > > missing from expected (set(['pwdLastSet'])), extra (set([]))
> > > > > 
> > > > > I'm sure I can figure it out - presumably pwdLastSet moved
> > > > > from
> > > > > some
> > > > > common code to a special case - but any clues give me more
> > > > > time
> > > > > to
> > > > > review your other patches ;-)
> > > > 
> > > > So are we expecting pwdLastSet to be there and it's not or
> > > > the other way?
> > > 
> > > s.difference(t) 	s - t 	new set with elements in s
> > > but
> > > not in t
> > > 
> > > It seems pwdLastSet isn't restored, but it expected it to be
> > > restored. 
> > > 
> > > I'm going to validate the test against Windows.  Currently
> > > hitting
> > > what
> > > I assume is the minPwdLen issue that seems common across all our
> > > tests.
> > 
> > I found and fixed the problems.
> > 
> > https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/head
> > s/
> > master4-tombstone-reanimation
> > 
> > I'm running a private autobuild with this.
> > 
> > I think we need a dbcheck update that removes secret attributes
> > from
> > deleted objects,
> > can you have a look and implement that?
> 
> I think that regression should only be in master, from when I moved
> operational below repl_meta_data.  But yes, I can do that. 
> 
> Inspired by this, but untested, is the attached.  

I've looked over your master4-tombstone-reanimation and it is *really*
good.  

Reviewed-by: Andrew Bartlett <abartlet at samba.org>

Thanks!

I'll continue working on this (I've promised to look over the code as
well), and I hope to push these soon!

I'm also planning on reviewing the remainder of your patches, building
the extra NDR tests that the gpgme test needs. 

Andrew Bartlett

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