[Freeipa-devel] [PATCH] 0004 Added support for authentication with user certificate
Petr Vobornik
pvoborni at redhat.com
Tue Aug 16 10:42:59 UTC 2016
On 08/16/2016 10:17 AM, Jan Cholasta wrote:
> On 12.8.2016 15:02, Petr Vobornik wrote:
>> On 08/12/2016 02:54 PM, Tibor Dudlak wrote:
>>> Hi,
>>>
>>> I have edited my previous patch.
>>>
>>> On Thu, Aug 11, 2016 at 11:52 AM, Jan Cholasta <jcholast at redhat.com
>>> <mailto:jcholast at redhat.com>> wrote:
>>>
>>> Hi,
>>>
>>> On 11.8.2016 09:55, Tibor Dudlak wrote:
>>>
>>> Hi,
>>>
>>> ...
>>>
>>>
>>> +class login_x509(login_kerberos, KerberosSession, HTTP_Status):
>>> + key = '/session/login_x509'
>>>
>>> login_kerberos already subclasses KerberosSession and
>>> HTTP_Status, no need
>>> to do it again here. In fact, it would be best to split off the
>>> bussiness
>>> logic from login_kerberos into a separate class and inherit both
>>> login_kerberos and login_x509 from it:
>>>
>>> class KerberosLogin(Backend, KerberosSession, HTTP_Status):
>>> def _on_finalize(self):
>>> ...
>>>
>>> def __call__(self, ...):
>>> ...
>>>
>>> class login_kerberos(KerberosLogin):
>>> key = '/session/login_kerberos'
>>>
>>> class login_x509(KerberosLogin):
>>> key = '/session/login_x509'
>>>
>>> Honza
>>>
>>> --
>>> Jan Cholasta
>>>
>>>
>>> Thank jcholast for review, it should be all right now.
>>>
>>> --
>>> Tibor Dudlák
>>> Intern - Identity management Special Projects
>>> Red Hat
>>>
>>
>> Tibor, please reuse the original thread and patch number in each patch
>> iteration. But append new patch version. E.g.
>> freeipa-ddudla-0003-2-Added...
>>
>> Starting new thread for each patch revision makes it hard to track.
>
> +1
>
> As far as the patch is concerned, LGTM.
>
Anyway, I'd split the patch into two pieces:
1. the python part
2. the webui plugin + related conf
Reason: there is a wide agreement that #1 will be push. It's not about #2.
--
Petr Vobornik
More information about the Freeipa-devel
mailing list