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 <tdud...@redhat.com
<mailto:tdud...@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 <pvobo...@redhat.com
    <mailto:pvobo...@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

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

Reply via email to