[Freeipa-devel] [PATCH 0116] Refactoring of service autobind

[Martin Basti]

On 19/09/14 14:30, Jan Cholasta wrote:
> Dne 19.9.2014 v 13:32 Martin Basti napsal(a):
>> On 01/09/14 16:26, Martin Basti wrote:
>>> On 28/08/14 14:01, Jan Cholasta wrote:
>>>> Hi,
>>>>
>>>> Dne 27.8.2014 v 15:22 Martin Basti napsal(a):
>>>>> Patch attached.
>>>>>
>>>>
>>>> 1) Please rename object_exists to entry_exists.
>>>>
>>>>
>>>> 2) Use empty attribute list in get_entry() in
>>>> object_exists/entry_exists.
>>>>
>>>>
>>>> 3) Please update LDAPObject.get_dn_if_exists() to use
>>>> object_exists/entry_exists.
>>>>
>>>>
>>>> 4) I'm not a fan of how do_bind() is laid out, IMHO something like
>>>> this would be better (untested):
>>>>
>>>> +    def do_bind(self, dm_password=None, autobind=AUTOBIND_AUTO,
>>>> timeout=DEFAULT_TIMEOUT):
>>>> +        if dm_password:
>>>> +            self.do_simple_bind(bindpw=dm_password, timeout=timeout)
>>>> +            return
>>>> +
>>>> +        if autobind != AUTOBIND_DISABLED and os.getegid() == 0 and
>>>> self.ldapi:
>>>> +            try:
>>>> +                # autobind
>>>> +                pw_name = pwd.getpwuid(os.geteuid()).pw_name
>>>> +                self.do_external_bind(pw_name, timeout=timeout)
>>>> +                return
>>>> +            except errors.NotFound:
>>>> +                if autobind == AUTOBIND_ENABLED:
>>>> +                    # autobind was required and failed, raise
>>>> +                    # exception that it failed
>>>> +                    raise
>>>> +
>>>> +        # Fall back
>>>> +        self.do_sasl_gssapi_bind(timeout=timeout)
>>>>
>>>>
>>>> Honza
>>>>
>>> 3) skipped as we discuss on IRC
>>>
>>> Updated patch attached
>>>
>>>
>>>
>>> _______________________________________________
>>> Freeipa-devel mailing list
>>> Freeipa-devel at redhat.com
>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>> Please review, this should be in 4.1
>
> 1) The patch need a rebase on top of current ipa-4-1.
I can apply it (Am I doing something wrong?)
>
>
> 2) You can remove import pwd from service.py, it is no longer used there.
>
>
> 3) Are named constants for the autobind argument the right thing to 
> do? It is a tri-state which can be expressed with None/True/False. 
> (I'm just asking, I don't have a strong opinion on this.)
>
As we discussed on IRC, using None/True/False, is not good approach.
Updated patch attached

-- 
Martin Basti

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0116.3-Refactoring-of-autobind-object_exists.patch
Type: text/x-patch
Size: 9784 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140925/c7f21791/attachment.bin>