[Kimchi-devel] [PATCH 1/3] authorization: Update /login to return user roles instead of sudo parameter

Aline Manera alinefm at linux.vnet.ibm.com
Wed Jul 16 12:43:18 UTC 2014



On 07/16/2014 04:15 AM, Royce Lv wrote:
> On 2014年07月16日 03:44, alinefm at linux.vnet.ibm.com wrote:
>> From: Aline Manera <alinefm at linux.vnet.ibm.com>
>>
>> For now, Kimchi just supports 2 types of user roles: 'admin' - user has
>> full control of Kimchi features and 'user' with limited access. But in
>> future the idea is to have more and more roles so it is good to
>> already provide the authorization support with that in mind.
>>
>> That way, instead of only returning if user has or not sudo
>> permissions, the
>> /login API will return the user role for each tab.
>>
>> If the user has sudo permissions he/she will have 'admin' role,
>> otherwise, 'user' role.
>>
>> curl -H "Content-Type: application/json" -H "Accept: application/json"
>>    http://localhost:8010/login
>>    -d'{"username": "guest", "password": "guest-passwd"}' -X POST
>>
>> {
>>    "username": "guest",
>>    "roles": {"templates": "user",
>>              "host": "user",
>>              "storage": "user",
>>              "guests": "user",
>>              "network": "user"},
>>    "groups": []
>> }
>>
>> curl -H "Content-Type: application/json" -H "Accept: application/json"
>>    http://localhost:8010/login
>>    -d'{"username": "sysadmin", "password": "sysadmin-passwd"}' -X POST
>>
>> {
>>    "username": "sysadmin",
>>    "roles": {"templates": "admin",
>>              "host": "admin",
>>              "storage": "admin",
>>              "guests": "admin",
>>              "network": "admin"},
>>    "groups": []
>> }
>>
>> Having one role per tab, give us more flexibility to assign different
>> roles to user.
>>
>> Signed-off-by: Aline Manera <alinefm at linux.vnet.ibm.com>
>> ---
>>   src/kimchi/auth.py  | 19 ++++++++++++++-----
>>   src/kimchi/utils.py | 15 +++++++++++++++
>>   tests/test_rest.py  |  6 ++++++
>>   tests/utils.py      |  6 +++---
>>   4 files changed, 38 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py
>> index 6a4a610..8d20966 100644
>> --- a/src/kimchi/auth.py
>> +++ b/src/kimchi/auth.py
>> @@ -32,14 +32,17 @@
>>
>>   from kimchi import template
>>   from kimchi.exception import InvalidOperation, OperationFailed
>> -from kimchi.utils import run_command
>> +from kimchi.utils import get_all_tabs, run_command
>>
>>
>>   USER_NAME = 'username'
>>   USER_GROUPS = 'groups'
>>   USER_SUDO = 'sudo'
>> +USER_ROLES = 'roles'
>>   REFRESH = 'robot-refresh'
>>
>> +tabs = get_all_tabs()
>> +
>>
>>   def redirect_login():
>>       url = "/login.html"
>> @@ -62,7 +65,10 @@ def __init__(self, username):
>>           self.user = {}
>>           self.user[USER_NAME] = username
>>           self.user[USER_GROUPS] = None
>> -        self.user[USER_SUDO] = False
>> +        # after adding support to change user roles that info should
>> be read
>> +        # from a specific objstore and fallback to default only if
>> any entry is
>> +        # found
>> +        self.user[USER_ROLES] = dict.fromkeys(tabs, 'user')
>>
>>       def get_groups(self):
>>           self.user[USER_GROUPS] = [g.gr_name for g in grp.getgrall()
>> @@ -74,10 +80,13 @@ def has_sudo(self):
>>           p = multiprocessing.Process(target=self._has_sudo,
>> args=(result,))
>>           p.start()
>>           p.join()
>> -        self.user[USER_SUDO] = bool(result.value)
>> -        return self.user[USER_SUDO]
>> +        if result.value:
>> +            self.user[USER_ROLES] = dict.fromkeys(tabs, 'admin')
>> +        return result.value
> Since we introduced rolls in each tab. I think when judging whether a
> user's role of a tab, we need to depend on the access assigned to him
> rather than see if he has "sudo".
> "sudo" judgement divide users of two kinds: 'users' of all tabs OR
> 'admin' of all tabs.

 > + # after adding support to change user roles that info should be read
 > + # from a specific objstore and fallback to default only if any entry is
 > + # found

As I commented above, the idea is add supporting to change user roles 
instead of only checking sudo permissions

But for now (for 1.3 release) we will only support 2 roles: admin and 
user and they will be set according to sudo permissions


Also I would like we stop mapping system users to
> kimchi users--kimchi needs its own user/role/group maintaining to
> isolate security risks as they did in openstack/ovirt.

We will continue using system users in Kimchi and only store the 
additional data we need to set user roles in Kimchi.
As we have already discussed about.

>>
>>       def _has_sudo(self, result):
>> +        result.value = False
>> +
>>           _master, slave = pty.openpty()
>>           os.setsid()
>>           fcntl.ioctl(slave, termios.TIOCSCTTY, 0)
>> @@ -94,7 +103,7 @@ def _has_sudo(self, result):
>>                                             self.user[USER_NAME]])
>>               for line in out.split('\n'):
>>                   if line and re.search("(ALL)", line):
>> -                    result.value = 1
>> +                    result.value = True
>>                       debug("User %s can run any command with sudo" %
>>                             result.value)
>>                       return
>> diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py
>> index 97adbf8..798d9cd 100644
>> --- a/src/kimchi/utils.py
>> +++ b/src/kimchi/utils.py
>> @@ -27,6 +27,7 @@
>>   import subprocess
>>   import traceback
>>   import urllib2
>> +import xml.etree.ElementTree as ET
>>   from multiprocessing import Process, Queue
>>   from threading import Timer
>>
>> @@ -108,6 +109,20 @@ def get_enabled_plugins():
>>               except (TypeError, KeyError):
>>                   continue
>>
>> +def get_all_tabs():
>> +    files = [os.path.join(paths.prefix, 'config/ui/tabs.xml')]
>> +
>> +    for plugin, _ in get_enabled_plugins():
>> +        files.append(os.path.join(PluginPaths(plugin).ui_dir,
>> +                     'config/tab-ext.xml'))
>> +
>> +    tabs = []
>> +    for f in files:
>> +        root = ET.parse(f)
>> +        tabs.extend([t.text.lower() for t in root.iter('title')])
>> +
>> +    return tabs
>> +
>>
>>   def import_class(class_path):
>>       module_name, class_name = class_path.rsplit('.', 1)
>> diff --git a/tests/test_rest.py b/tests/test_rest.py
>> index 694d907..1a91eb3 100644
>> --- a/tests/test_rest.py
>> +++ b/tests/test_rest.py
>> @@ -1552,6 +1552,12 @@ def test_auth_session(self):
>>           req = json.dumps({'username': user, 'password': pw})
>>           resp = self.request('/login', req, 'POST', hdrs)
>>           self.assertEquals(200, resp.status)
>> +
>> +        user_info = json.loads(resp.read())
>> +        self.assertEquals(sorted(user_info.keys()),
>> +                          ['groups', 'roles', 'username'])
>> +        self.assertEquals(user_info['roles'], [u'admin'])
>> +
>>           cookie = resp.getheader('set-cookie')
>>           hdrs['Cookie'] = cookie
>>
>> diff --git a/tests/utils.py b/tests/utils.py
>> index fd9b23c..4853b7a 100644
>> --- a/tests/utils.py
>> +++ b/tests/utils.py
>> @@ -157,8 +157,8 @@ def patch_auth(sudo=True):
>>       def _get_groups(self):
>>           return None
>>
>> -    def _has_sudo(self):
>> -        return sudo
>> +    def _has_sudo(self, result):
>> +        result.value = sudo
>>
>>       def _authenticate(username, password, service="passwd"):
>>           try:
>> @@ -170,7 +170,7 @@ def _authenticate(username, password,
>> service="passwd"):
>>       import kimchi.auth
>>       kimchi.auth.authenticate = _authenticate
>>       kimchi.auth.User.get_groups = _get_groups
>> -    kimchi.auth.User.has_sudo = _has_sudo
>> +    kimchi.auth.User._has_sudo = _has_sudo
>>
>>
>>   def normalize_xml(xml_str):
>




More information about the Kimchi-devel mailing list