Hi, Please find my comment below. Adding kimchi dev mailing and Aline as changes are suggested in wok util. Thanks, Archana Singh On 3/23/2016 12:33 PM, Daniel Henrique Barboza wrote:

On 03/23/2016 02:24 PM, Archana Singh wrote:

...

On 3/23/2016 12:19 PM, Abhiram wrote:

...

On Wed, 2016-03-23 at 11:39 -0500, Archana Singh wrote:

...

Hi Suresh,

As all of these methods are not private methods, I will suggest to have check in all the methods. Eventhough the methods arent private, the initial caller has to be create,getlist,delete,lookup atleast as suresh mentioned from ui point. I feel it has to be one place, rather than having in all sub methods which is redundant in nature.

If we want to have all places, do we want to have the unicode checking and conversion into one common method and reuse the same everywhere? If at all we have any change in future probably only one method we can modify.

I do understand the redundant point of view, but as these are not private methods can be called from any place.

If it's a widespread issue then can we look at WoK engine code to fix it for all cases and plug-ins?

I don't know if this is actually viable but at first it makes sense to me. Would need some investigation.

Daniel

Hi Daniel, Are you pointing to have common wok util method for checking isinstance of unicode and encoding to utf-8? @Aline: Adding kimchi dev list and you to discuss this further as change is suggested in wok. Please provide you input. Thanks, Archana Singh

...

As we do not have type check for the parameter in the method, it is safe to ensure that we do not end up in some encoding issue.

...

...

Thanks, Archana Singh

On 3/23/2016 2:05 AM, Suresh Babu Angadi wrote:

...

Hi Archana, Instead of checking unicode in every method, can we have it only in UsersModel and UserModel - create, getlist, delete, lookup?

On 03/22/2016 03:52 AM, archus@linux.vnet.ibm.com wrote:

...

From: Archana Singh <archus@linux.vnet.ibm.com>

--- i18n.py | 3 ++- model/users.py | 69 ++++++++++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 59 insertions(+), 13 deletions(-)

diff --git a/i18n.py b/i18n.py index 78260a0..6729bf3 100644 --- a/i18n.py +++ b/i18n.py @@ -124,10 +124,11 @@ messages = { "GINUSER0006E": _("Could not add user '%(user)s' to kvm group."), "GINUSER0007E": _("Could not add user '%(user)s' to sudoers list."), "GINUSER0008E": _("The user name '%(user)s' is already in use'."), - "GINUSER0009E": _("Could not create user '%(user)s'."), + "GINUSER0009E": _("Could not create user '%(user)s', error: %(err)s'."), "GINUSER0010E": _("Could not delete user '%(user)s'."), "GINUSER0011E": _("User '%(user)s' does not exist."), "GINUSER0012E": _("Could not delete group '%(group)s'"), + "GINUSER0013E": _("Could not create group name'%(group)s', error: %(err)s'."),

"GINFW0001E": _("Cannot update system firmware while running VMs."), "GINFW0002E": _("Firmware image unpack failed: rc = %(rc)s. " diff --git a/model/users.py b/model/users.py index a05bd5f..f8f450d 100644 --- a/model/users.py +++ b/model/users.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- # # Project Ginger # @@ -51,6 +52,8 @@ def get_group_obj_by_gid(gid):

def get_group_gid(groupname): adm = libuser.admin() + if isinstance(groupname, unicode): + groupname = groupname.encode('utf-8') group = adm.lookupGroupByName(groupname) if group is None: return None @@ -58,19 +61,30 @@ def get_group_gid(groupname):

def create_group(groupname, gid): - adm = libuser.admin() - group = adm.lookupGroupByName(groupname) - if not group: - new_group = adm.initGroup(groupname) - new_group.set(libuser.GIDNUMBER, gid) - adm.addGroup(new_group) - return gid - else: - return group.get('pw_gid')[0] + try: + adm = libuser.admin() + if isinstance(groupname, unicode): + groupname = groupname.encode('utf-8') + if isinstance(gid, unicode): + gid = gid.encode('utf-8') + group = adm.lookupGroupByName(groupname) + if not group: + new_group = adm.initGroup(groupname) + new_group.set(libuser.GIDNUMBER, gid) + adm.addGroup(new_group) + return gid + else: + return group.get('pw_gid')[0] + except Exception as e: + wok_log.error('Could not create group name %s, error: %s', + groupname, e) + raise OperationFailed('GINUSER0013E', {'group': groupname, 'err': e})

def delete_group(groupname): adm = libuser.admin() + if isinstance(groupname, unicode): + groupname = groupname.encode('utf-8') group_obj = adm.lookupGroupById( int(get_group_gid(groupname)) ) @@ -89,6 +103,8 @@ def delete_group(groupname):

def get_users_from_group(groupname): adm = libuser.admin() + if isinstance(groupname, unicode): + groupname = groupname.encode('utf-8') group_obj = adm.lookupGroupById( int(get_group_gid(groupname)) ) @@ -108,6 +124,8 @@ def get_users(exclude_system_users=True):

def get_user_obj(username): adm = libuser.admin() + if isinstance(username, unicode): + username = username.encode('utf-8') return adm.lookupUserByName(username)

@@ -120,6 +138,10 @@ def gen_salt():

def create_user(name, plain_passwd, profile=None, gid=None): adm = libuser.admin() + if isinstance(name, unicode): + name = name.encode('utf-8') + if isinstance(gid, unicode): + gid = gid.encode('utf-8') user = adm.lookupUserByName(name) if user: msg = 'User/Login "%s" already in use' % name @@ -150,15 +172,23 @@ def create_user(name, plain_passwd, profile=None, gid=None): enc_pwd = crypt.crypt(plain_passwd, salt)

adm.setpassUser(new_user, enc_pwd, True) + except UnicodeEncodeError as ue: + err_msg = ue.message if ue.message else 'Username / password \ + has NON - ASCII charater' + wok_log.error('Could not create user %s, error: %s', name, err_msg) + raise OperationFailed('GINUSER0009E', {'user': name, 'err': err_msg}) except Exception as e: - wok_log.error('Could not create user %s, error: %s', name, e) - raise OperationFailed('GINUSER0009E', {'user': name}) + err_msg = e.message if e.message else e + wok_log.error('Could not create user %s, error: %s', name, err_msg) + raise OperationFailed('GINUSER0009E', {'user': name, 'err': err_msg})

return new_user

def delete_user(username): adm = libuser.admin() + if isinstance(username, unicode): + username = username.encode('utf-8') user_obj = adm.lookupUserByName(username)

if user_obj is None: @@ -182,6 +212,11 @@ class UsersModel(object): profile = params['profile'] groupname = params['group']

+ if isinstance(username, unicode): + username = username.encode('utf-8') + if isinstance(groupname, unicode): + groupname = groupname.encode('utf-8') + gid = get_group_gid(groupname) if gid is None: user = create_user(username, passwd, profile=profile) @@ -213,6 +248,8 @@ class UsersModel(object): # Add new user to KVM group adm = libuser.admin() kvmgrp = get_group_obj('kvm') + if isinstance(user, unicode): + user = user.encode('utf-8') kvmgrp.add('gr_mem', user) ret = adm.modifyGroup(kvmgrp) if ret != 1: @@ -227,6 +264,7 @@ class UsersModel(object):

class UserModel(object): + def delete(self, user): user_obj = get_user_obj(user) group_obj = get_group_obj_by_gid( @@ -252,7 +290,10 @@ class UserModel(object): group_name = grp.getgrgid(user_info.pw_gid).gr_name

except KeyError: - group_name = str(user_info.pw_gid) + pw_gid = user_info.pw_gid + if isinstance(pw_gid, unicode): + pw_gid = pw_gid.encode('utf-8') + group_name = str(pw_gid)

return {"name": user, "uid": user_info.pw_uid, @@ -263,6 +304,8 @@ class UserModel(object): def _get_user_profile(self, user): # ADMIN: Check /etc/sudoers.d adm = libuser.admin() + if isinstance(user, unicode): + user = user.encode('utf-8') if os.path.isfile(SUDOERS_FILE % user): return 'admin' else: @@ -278,6 +321,8 @@ class UserModel(object): return 'kimchiuser'

def _delete_profile_settings(self, user): + if isinstance(user, unicode): + user = user.encode('utf-8') profile = self._get_user_profile(user) if profile == 'kimchiuser': return