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

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

2016-08-12 Thread Tibor Dudlak 
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
From b84deeffe5ec7e5565817feffc69768aea345592 Mon Sep 17 00:00:00 2001
From: Tiboris 
Date: Fri, 5 Aug 2016 11:47:06 +0200
Subject: [PATCH] Added support for authentication with user certificate

https://fedorahosted.org/freeipa/ticket/5764
---
 freeipa.spec.in|   8 +
 install/conf/Makefile.am   |   1 +
 install/conf/xx-ipa-cert-auth.conf |  14 ++
 .../freeipa/plugins-dist/cert_auth/cert_auth.js| 169 +
 ipaserver/plugins/xmlserver.py |   3 +-
 ipaserver/rpcserver.py |  17 ++-
 6 files changed, 207 insertions(+), 5 deletions(-)
 create mode 100644 install/conf/xx-ipa-cert-auth.conf
 create mode 100644 install/ui/src/freeipa/plugins-dist/cert_auth/cert_auth.js

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 135e9c980011c6c2730c6c29a3c22098e48270d5..19828bc84f1f1d13d4bb0e08a4749da626e9dbb3 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -818,6 +818,10 @@ install daemons/dnssec/ipa-ods-exporter %{buildroot}%{_libexecdir}/ipa/ipa-ods-e
 # Web UI plugin dir
 mkdir -p %{buildroot}%{_usr}/share/ipa/ui/js/plugins
 
+# Experimental external authentication UI plugin - moved into plugins-dist to be disabled by default
+mkdir -p %{buildroot}%{_usr}/share/ipa/ui/js/plugins-dist/cert_auth
+install install/ui/src/freeipa/plugins-dist/cert_auth/cert_auth.js %{buildroot}%{_usr}/share/ipa/ui/js/plugins-dist/cert_auth/cert_auth.js
+
 # DNSSEC config
 mkdir -p %{buildroot}%{_sysconfdir}/ipa/dnssec
 
@@ -1210,6 +1214,9 @@ fi
 %{_usr}/share/ipa/ui/js/freeipa/app.js
 %{_usr}/share/ipa/ui/js/freeipa/core.js
 %dir %{_usr}/share/ipa/ui/js/plugins
+%dir %{_usr}/share/ipa/ui/js/plugins-dist
+%dir %{_usr}/share/ipa/ui/js/plugins-dist/cert_auth
+%{_usr}/share/ipa/ui/js/plugins-dist/cert_auth/cert_auth.js
 %dir %{_usr}/share/ipa/ui/images
 %{_usr}/share/ipa/ui/images/*.jpg
 %{_usr}/share/ipa/ui/images/*.png
@@ -1232,6 +1239,7 @@ fi
 %{_usr}/share/ipa/ipa-rewrite.conf
 %{_usr}/share/ipa/ipa-pki-proxy.conf
 %{_usr}/share/ipa/kdcproxy.conf
+%{_usr}/share/ipa/xx-ipa-cert-auth.conf
 %ghost %attr(0644,root,apache) %config(noreplace) %{_usr}/share/ipa/html/ca.crt
 %ghost %attr(0644,root,apache) %{_usr}/share/ipa/html/kerberosauth.xpi
 %ghost %attr(0644,root,apache) %{_usr}/share/ipa/html/krb.con
diff --git a/install/conf/Makefile.am b/install/conf/Makefile.am
index 5daac776f71c5d01187b46d51044a07bf5fd717a..2e6fbb84902c843fa6e43a96191d5cc58a1213c1 100644
--- a/install/conf/Makefile.am
+++ b/install/conf/Makefile.am
@@ -6,6 +6,7 @@ app_DATA =  \
 	ipa-kdc-proxy.conf.template	\
 	ipa-pki-proxy.conf		\
 	ipa-rewrite.conf		\
+	xx-ipa-cert-auth.conf		\
 	$(NULL)
 
 EXTRA_DIST =\
diff --git a/install/conf/xx-ipa-cert-auth.conf b/install/conf/xx-ipa-cert-auth.conf
new file mode 100644
index ..824d33113446176c0d1a9f6c99e95aa48a7a99c9
--- /dev/null
+++ b/install/conf/xx-ipa-cert-auth.conf
@@ -0,0 +1,14 @@
+# Login with user certificate/smartcard configuration
+# This configuration needs to be loaded after 
+
+  AuthType none
+  GssapiCredStore keytab:/etc/httpd/conf/ipa.keytab
+  GssapiCredStore client_keytab:/etc/httpd/conf/ipa.keytab
+  GssapiDelegCcacheDir /var/run/httpd/ipa/clientcaches
+  GssapiImpersonate On
+  NSSVerifyClient require
+  NSSUserName SSL_CLIENT_CERT
+  LookupUserByCertificate On
+  WSGIProcessGroup ipa
+  WSGIApplicationGroup ipa
+
diff --git a/install/ui/src/freeipa/plugins-dist/cert_auth/cert_auth.js b/install/ui/src/freeipa/plugins-dist/cert_auth/cert_auth.js
new file mode 100644
index ..7dd0c17c83ab34f0c8dc37c789f202ce42d52f52
--- /dev/null
+++ b/install/ui/src/freeipa/plugins-dist/cert_auth/cert_auth.js
@@ -0,0 +1,169 @@
+/*  Authors:
+ *Petr Vobornik