[Kimchi-devel] [ginger-dev-list] [PATCH] [Ginger 4/4] Added support for UTF-8 encoding in user management.
Archana Singh
archus at linux.vnet.ibm.com
Thu Mar 24 15:42:10 UTC 2016
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 at linux.vnet.ibm.com wrote:
>>>>>> From: Archana Singh <archus at 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
>>>
>>
>
More information about the Kimchi-devel
mailing list