PATCHES: Password sync as active directory domain controller

[Stefan Metzmacher]

Hi Alexander,

> thanks for these patches. They are an impressive amount of work. I have
> few comments below mostly related to Python 2/3 compatibility and some
> idiomatic use of Python.
> 

>> +def get_crypt_value(alg, utf8pw):
>> +    algs = {
>> +        "5": {"length": 43},
>> +        "6": {"length": 86},
>> +    }
>> +    assert alg in algs.keys()
> 
> Please replace it with 
>      assert alg in algs
> 
> It is more Python idiomatic and also avoids creating a list every time
> only to drop it.
> 

Fixed.

>> +except ImportError as e:
>> +    reason = "hashlib.sha1()"
>> +    if random_reason:
>> +        reason += " and " + random_reason
>> +    reason += " required"
>> +    disabled_virtual_attributes["virtualSSHA"] = {
>> +        "reason" : reason,
>> +        }
>> +    pass
> 
> You don't need 'pass' here as there are already other statements under
> 'except'.

Fixed in all places.

>> +class GetPasswordCommand(Command):
>> +
>> +    def __init__(self):
>> +        Command.__init__(self)
> A nit-pick:
> It would be good to write all constructors in idiomatic way for 'new'
> classes (derived from 'object')
>            super(GetPasswordCommand, self).__init__()
> 

Fixed in all places.

>> +        except Exception, msg:
> Please use 'except Exception as e' like in the other code above because
> this makes the code Python 2 and 3 compatible ('except Exception, msg'
> is not compatible with Python 3).
>

Fixed in all places.


>> +            if rid == security.DOMAIN_RID_KRBTGT:
>> +                log_msg("# Dirsync[%d] SKIP: DOMAIN_RID_KRBTGT\n\n" % (idx))
>> +                return
>> +            for a in list(dirsync_obj.keys()):
> For iteration over keys it is better to use iterator objects which don't
> allocate the whole list at the same time:
> 
>    for a in dirsync_obj:
>        ....
> 
>> +                for h in dirsync_secret_attrs:
>> +                    if a.lower() == h.lower():
>> +                        del dirsync_obj[a]
>> +                        dirsync_obj["# %s::" % a] = ["REDACTED SECRET ATTRIBUTE"]

I'm doing this because I modify dirsync_obj within the loop
and I want the iteration of the original list of keys.

The current state can be found under
https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master4-gpgme

https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=2a21d72371287b487a19fce73879c6fbfd20e0c4
and
https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=00ce1c3b47dbaafd3ba7191900627ede62f7366f
are the changes which can be squashed.

Thanks!
metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160418/f9055366/signature.sig>