[Kimchi-devel] [PATCHv3 3/8] Add LDAP authentication

[Royce Lv]

On 2014年11月14日 03:04, Crístian Viana wrote:
> On 13-11-2014 05:44, Royce Lv wrote:
>>> I think the code would look better without the external try/except 
>>> block.
>> Do you mean get them handled all by kimchi exception?
>> I've considered that, but like:
>>     connect.search_s()
>>    connect.bind_s()
>>    connect.unbind_s()
>> all throw ldap.LDAPError, they also throw exception like: 
>> ldap.INVALD_CREDENTIALS.
>> so if delete the external try/except, I need to add 3 excepts to each 
>> operation.
>> You can see I do this because ldap lib does not handle all exception 
>> with ldap.LDAPError.
>>
>
> Wouldn't something like this work?
>
> try:
>     result = connect.search_s(
>         ldap_search_base, ldap.SCOPE_SUBTREE, ldap_search_filter)
>     if len(result) == 0:
>         entity = ldap_search_filter % {'username': username}
>         raise ldap.LDAPError("Invalid ldap entity:%s" % entity)
>
>     connect.bind_s(result[0][0], password)
>     connect.unbind_s()
>     return True
> except ldap.INVALID_CREDENTIALS:
>     # invalid user password
>     arg = {"username": username, "code": "invalid user/passwd"}
>     raise OperationFailed("KCHAUTH0001E", arg)
> except ldap.NO_SUCH_OBJECT:
>     # ldap search base specified wrongly.
>     arg = {"username": username, "code": "invalid ldap search base %s" 
> % ldap_search_base)}
>     raise OperationFailed("KCHAUTH0001E", arg)
> except ldap.LDAPError, e:
>     arg = {"username": username, "code": e.message}
>     raise OperationFailed("KCHAUTH0001E", arg)
>
> Maybe you could even have different messages ID instead of passing 
> "code" like that. Keep in mind that those "codes" won't be translated...
>
> But that's just code preference, I just think this looks better than 
> using a nested block.
ACK, I was too used to catch things where they throw.