Re: [Freeipa-devel] [PATCH] 0004 Added support for authentication with user certificate

2016-08-16 Thread Tibor Dudlak
Hi,

I have done the python part, you can find it in original thread as you
suggested.

Thank you.

On Tue, Aug 16, 2016 at 12:42 PM, Petr Vobornik  wrote:

> 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  >>> > 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
>



-- 
Tibor Dudlák
Intern - Identity management Special Projects
Red Hat
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0004 Added support for authentication with user certificate

2016-08-16 Thread Petr Vobornik
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 >> > 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

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 0004 Added support for authentication with user certificate

2016-08-16 Thread Jan Cholasta

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 > 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.


--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 0004 Added support for authentication with user certificate

2016-08-12 Thread Petr Vobornik
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  > 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.
-- 
Petr Vobornik

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code