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

[Jan Cholasta]

Hi,

On 11.8.2016 09:55, Tibor Dudlak wrote:
> Hi,
>
> I think this patch is finished. If it does not suits you and it will not
> be merged please consider merging PATCH 0001 from
> http://www.redhat.com/archives/freeipa-devel/2016-August/msg00009.html
> at least.
>
> Thank you
>
> On Wed, Aug 10, 2016 at 10:17 AM, Tibor Dudlak <tdudlak at redhat.com
> <mailto:tdudlak at redhat.com>> wrote:
>
>     Hi,
>
>     I have improved my previous patch for authentication with user
>     certificate/smartcard.
>     This patch includes patches and plugin and apache configuration
>     described here:
>     http://www.freeipa.org/page/V4/External_Authentication/Setup
>     <http://www.freeipa.org/page/V4/External_Authentication/Setup>
>     It also contains steps to configure and test this feature. Once this
>     patch is merged and released I will simplify this page to not
>     confuse customers.
>
>     On Fri, Aug 5, 2016 at 3:58 PM, Petr Vobornik <pvoborni at redhat.com
>     <mailto:pvoborni at redhat.com>> wrote:
>
>         On 08/05/2016 02:57 PM, Tibor Dudlak wrote:
>         >...
>
>         Let's assume that we will go with this approach and not separate
>         RPM.
>
>         1. ipa.conf version needs to be bumped
>
>
>     We have found another problem with ipa.conf approach so I have moved
>     configuration of apache for plugin from ipa.conf into completely
>     separated file to be not configured in FreeIPA by default. As you
>     said it may cause some security issues and it definitely causes
>     errors when plugin dependences are not installed nor configured.
>
>         2. Do not put the web ui plugin in src/freeipa/plugins dir. That
>         is a
>         dir for core UI plugins. This one is sort of hybrid - basically
>         a third
>         party plugin added to core package  but enabled as third party
>         because
>         the feature is experimental.
>
>         Create rather a new dir for that. E.g. plugins.d as Alexander
>         suggested
>         ->  freeipa/install/ui/src/plugins.d/cert_auth/cert_auth.js
>
>         3. unrelated and "alternative solution"  comments needs to be
>         removed
>         from the UI plugin. They were added to the example plugin
>         https://pvoborni.fedorapeople.org/plugins/loginauth/loginauth.js
>         <https://pvoborni.fedorapeople.org/plugins/loginauth/loginauth.js>
>         mostly
>         to help you with the development.
>
>         4. Add comment to freeipa.spec.in <http://freeipa.spec.in>
>         describing what the plugin is and why
>         it is put there this way.
>
>         5. The plugin itself deserves better description as well. Right now
>         there is the general description.
>
>         6. I have not tried it, but make sure that it passes jslint
>         (`jsl -conf
>         jsl.conf`) Easiest may be to use temp(i.e. do not include it here)
>         jsl.conf e.g.:
>         https://pvoborni.fedorapeople.org/plugins/loginauth/jsl.conf
>         <https://pvoborni.fedorapeople.org/plugins/loginauth/jsl.conf>
>
>         --
>         Petr Vobornik
>
>
>     I hope result of jsl http://pastebin.test.redhat.com/400076
>     <http://pastebin.test.redhat.com/400076> means that things passed.
>     Thanks Petr for review and I hope this patch will cover all concerns
>     he had.
>
>     Addressing ticket: https://fedorahosted.org/freeipa/ticket/5764
>     <https://fedorahosted.org/freeipa/ticket/5764>
>
>     Thank you.

+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