[Kimchi-devel] [PATCHv1 2/4] Split PAM and LDAP authentication

[Aline Manera]

On 10/20/2014 11:52 AM, lvroyce0210 at gmail.com wrote:
> From: Royce Lv <lvroyce at linux.vnet.ibm.com>
>
> Split PAM authentication implementation and abstract a common class.
>
> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
> ---
>   src/kimchi/auth.py | 111 ++++++++++++++++++++++++++++++++++-------------------
>   1 file changed, 71 insertions(+), 40 deletions(-)
>
> diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py
> index c8801a5..10c7c1f 100644
> --- a/src/kimchi/auth.py
> +++ b/src/kimchi/auth.py
> @@ -31,6 +31,7 @@ import urllib2
>
>
>   from kimchi import template
> +from kimchi.config import config
>   from kimchi.exception import InvalidOperation, OperationFailed
>   from kimchi.utils import get_all_tabs, run_command
>
> @@ -59,6 +60,22 @@ def debug(msg):
>
>
>   class User(object):

As it is related to authentication, I suggest renaming it to 
"Authentication"

> +    @classmethod
> +    def create(cls, auth_args):
> +        auth_type = auth_args.pop('auth_type')
> +        for klass in cls.__subclasses__():
> +            if auth_type == klass.auth_type:
> +                try:
> +                    if not klass.authenticate(**auth_args):
> +                        debug("cannot verify user with the given password")
> +                        return None
> +                except OperationFailed:
> +                    raise
> +                return klass(auth_args['username'])
> +
> +

We could do it in the __init__() function and return the right instance 
according to the authentication method.
Example:

class Authentication(object):
     def __init__(self, auth_type):

+        for klass in cls.__subclasses__():
+            if auth_type == klass.auth_type:
+                return klass()


> +class PAMUser(User):

class PAMAuthentication(Authentication);

> +    auth_type = "pam"
>
>       def __init__(self, username):

You can get the username/password only when doing the authentication

>           self.user = {}
> @@ -124,40 +141,54 @@ class User(object):
>       def get_user(self):
>           return self.user
>
> +    @staticmethod

It will not be static with you have a instance according to type as I 
suggested above.

> +    def authenticate(username, password, service="passwd"):
> +        '''Returns True if authenticate is OK via PAM.'''
> +        def _pam_conv(auth, query_list, userData=None):
> +            resp = []
> +            for i in range(len(query_list)):
> +                query, qtype = query_list[i]
> +                if qtype == PAM.PAM_PROMPT_ECHO_ON:
> +                    resp.append((username, 0))
> +                elif qtype == PAM.PAM_PROMPT_ECHO_OFF:
> +                    resp.append((password, 0))
> +                elif qtype == PAM.PAM_PROMPT_ERROR_MSG:
> +                    cherrypy.log.error_log.error(
> +                        "PAM authenticate prompt error: %s" % query)
> +                    resp.append(('', 0))
> +                elif qtype == PAM.PAM_PROMPT_TEXT_INFO:
> +                    resp.append(('', 0))
> +                else:
> +                    return None
> +            return resp
> +
> +        auth = PAM.pam()
> +        auth.start(service)
> +        auth.set_item(PAM.PAM_USER, username)
> +        auth.set_item(PAM.PAM_CONV, _pam_conv)
> +        try:
> +            auth.authenticate()
> +        except PAM.error, (resp, code):
> +            msg_args = {'username': username, 'code': code}
> +            raise OperationFailed("KCHAUTH0001E", msg_args)
>
> -def authenticate(username, password, service="passwd"):
> -    '''Returns True if authenticate is OK via PAM.'''
> -    def _pam_conv(auth, query_list, userData=None):
> -        resp = []
> -        for i in range(len(query_list)):
> -            query, qtype = query_list[i]
> -            if qtype == PAM.PAM_PROMPT_ECHO_ON:
> -                resp.append((username, 0))
> -            elif qtype == PAM.PAM_PROMPT_ECHO_OFF:
> -                resp.append((password, 0))
> -            elif qtype == PAM.PAM_PROMPT_ERROR_MSG:
> -                cherrypy.log.error_log.error("PAM authenticate prompt error "
> -                                             "message: %s" % query)
> -                resp.append(('', 0))
> -            elif qtype == PAM.PAM_PROMPT_TEXT_INFO:
> -                resp.append(('', 0))
> -            else:
> -                return None
> -        return resp
> -
> -    auth = PAM.pam()
> -    auth.start(service)
> -    auth.set_item(PAM.PAM_USER, username)
> -    auth.set_item(PAM.PAM_CONV, _pam_conv)
> -
> -    try:
> -        auth.authenticate()
> -    except PAM.error:
> -        raise
> -
> -    return True
> +        return True
>
>
> +class LDAPUser(User):

class LDAPAuthentication(Authentication):

> +    auth_type = "ldap"
> +    def __init__(self, username):

Same I commented above. You can receive the username/password only when 
doing the authentication itself.

> +        self.user = {}
> +        self.user[USER_NAME] = username
> +        self.user[USER_GROUPS] = None
> +        # FIXME: user roles will be changed according roles assignment after
> +        # objstore is integrated
> +        self.user[USER_ROLES] = dict.fromkeys(tabs, 'user')
> +
> +    @staticmethod
> +    def authenticate(username, password):
> +        return False
> +
>   def from_browser():
>       # Enable Basic Authentication for REST tools.
>       # Ajax request sent from jQuery in browser will have "X-Requested-With"
> @@ -216,15 +247,15 @@ def check_auth_httpba():
>
>
>   def login(username, password, **kwargs):
> -    try:
> -        if not authenticate(username, password):
> -            debug("User cannot be verified with the supplied password")
> -            return None
> -    except PAM.error, (resp, code):
> -        msg_args = {'username': username, 'code': code}
> -        raise OperationFailed("KCHAUTH0001E", msg_args)
> -
> -    user = User(username)

> +    auth_args = {'auth_type': config.get("authentication", "method"),
> +                 'username': username,
> +                 'password': password}
> +
> +    user = User.create(auth_args)
> +    if not user:
> +        debug("User cannot be verified with the supplied password")
> +        return None
> +

auth = Authentication(config.get("authentication", "method"))
user = auth.authentication(username, password)

>       debug("User verified, establishing session")
>       cherrypy.session.acquire_lock()
>       cherrypy.session.regenerate()