Re: [Ejbca-develop] Samba smart card logon with EjbCA first step

[Gémes G. <ge...@kz...>]

Hello Branko,

Thank you for your suggestions!
> Hello again :)
>
> Once again, be warned, nothing really wrong with script, just me
> nitpicking :)
>
> - Ran a quick PEP8 on it again, and actually learned something
>    interesting myself. In line 94 you have check "hostname ==
>    None" (instead of "is None"). Seeing you used elsewhere "is not
>    None", probably just an oversight.
Yes, thanks, fixed.
>
> - You could probably optimise the Samba Python path guessing by setting
>    default value of samba_install_path to "/usr/local/samba/" (and
>    dropping the two elif's). One thing to keep in mind, though - if the
>    user had installed Samba from a package, they probably already have
>    their Python path set-up correctly to find Samba modules. Personally,
>    I would probably not try to guess the directory, and instead either
>    expect the user to set-up his/her PYTHON_PATH, or provide the path
>    explicitly with the --samba_install_path option.
Simplified it, although too many installs defaults to /usr/local/samba 
to just drop the default (in the worst case there are two useless 
os.path.exists checks).
>
> - On line 118 - typo, should say "could not".
Absolutely
>
> - Lines 156 through 166 - you can avoid lots of conditionals here by
>    simply having the default values setup for option/argument parser.
I'm probably too dumb/tried to get your point here, could you elaborate 
please.
>
> - One thing I'm curious about - when initialising the GetDC class, you
>    prompt for username/password when hostname has been specified, but if
>    it's not specified you set the username/password to ''. I'm guessing
>    when it connects to localhost it doesn't need to authenticate at all?
>    If so, maybe just add a small note for the "--hostname" option ("if
>    specified, you will be prompted for username and password for
>    logging-in into the provided serrver")
1. please have a look at the attached script it tries to connect via 
kerberos if python-krbV is installed and provides option to force 
kerberos connect if it is not, or the ntlm/simple bind otherwise.
2. in the case of running against the local host samba libraries use the 
ldb files which they access as an ldap like database and operate on 
them. The empty username and password specify that the connection should 
be allowed (with local ldb files it is always) based on file-system 
access rights.

>
> - Don't default the hostname parameter to None in function generate_req
>    - that's probably something that should always be provided (plus, if
>    it's None, the None + "somestring" will throw you an exception).
Fixed
>
> Otherwise, very nice - it's great you added the error handling and
> checks for existing files :)
>
> Best regards
>
>
>
Cheers

Geza Gemes