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

[Jan Cholasta]

Dne 19.9.2014 v 14:39 Martin Basti napsal(a):
> 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
>>
> Thank you for review
>
>> 1) The patch need a rebase on top of current ipa-4-1.
>>
>>
>> 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.)
>>
> Seems like good Idea to me,
> is clear enough to have AUTO=None, DISABLED=False, ENABLED=True?
>

Well, I'm not sure, that's why I'm asking :)

-- 
Jan Cholasta