On 2014年10月22日 02:39, Aline Manera wrote:
On 10/20/2014 11:52 AM, lvroyce0210@gmail.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com>
Split PAM authentication implementation and abstract a common class.
Signed-off-by: Royce Lv <lvroyce@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"
It is not just authentication, it is related to user authentication, authorization, user role/group get. (see things already there in PAMUser) See below: In my refactor, Users only allows to be created when it has right auth_type as well as pass the authentication of this type. Otherwise we don't have the need to create such type of user.
+ @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)
Nope, this part already covered in factory method
debug("User verified, establishing session") cherrypy.session.acquire_lock() cherrypy.session.regenerate()
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel