[Kimchi-devel] [PATCH] [Wok] Do not link user role with UI tabs
Daniel Henrique Barboza
dhbarboza82 at gmail.com
Fri Jan 20 18:24:00 UTC 2017
Reviewed-by: Daniel Barboza <danielhb at linux.vnet.ibm.com>
Tested-by: Daniel Barboza <danielhb at linux.vnet.ibm.com>
On 01/19/2017 10:17 AM, Aline Manera wrote:
> Thinking about providing more granularity on authorization (ie, grant
> access to a normal user to create a VM on Kimchi, for example) the user
> role (sysadmin or normal user) was linked to the UI tabs instead
> of the API itself
>
> It can cause multiple issues, for example:
> - different plugins with the same tab name (the case of Ginger and
> Kimchi) will get the authorization settings merged
> - what about an API in use for different tabs?
>
> To avoid those problems, remove the role_key parameter and also do not
> link UI tabs with user role.
>
> Signed-off-by: Aline Manera <alinefm at linux.vnet.ibm.com>
> ---
> src/wok/auth.py | 86 +++++++++++++++++++++++-------------------------
> src/wok/control/base.py | 16 ++++-----
> src/wok/control/logs.py | 3 +-
> src/wok/control/utils.py | 8 ++---
> tests/test_server.py | 7 ++--
> tests/utils.py | 18 ++++------
> ui/js/src/wok.login.js | 4 +--
> ui/js/src/wok.main.js | 9 +++--
> 8 files changed, 67 insertions(+), 84 deletions(-)
>
> diff --git a/src/wok/auth.py b/src/wok/auth.py
> index 6b1c160..976d729 100644
> --- a/src/wok/auth.py
> +++ b/src/wok/auth.py
> @@ -1,7 +1,7 @@
> #
> # Project Wok
> #
> -# Copyright IBM Corp, 2015-2016
> +# Copyright IBM Corp, 2015-2017
> #
> # Code derived from Project Kimchi
> #
> @@ -35,13 +35,11 @@ import urllib2
> from wok import template
> from wok.config import config
> from wok.exception import InvalidOperation, OperationFailed
> -from wok.utils import get_all_tabs, run_command
> +from wok.utils import run_command
>
> USER_NAME = 'username'
> +USER_ROLE = 'role'
> USER_GROUPS = 'groups'
> -USER_ROLES = 'roles'
> -
> -tabs = get_all_tabs()
>
>
> def redirect_login():
> @@ -60,6 +58,20 @@ def debug(msg):
>
>
> class User(object):
> + def __init__(self, username):
> + self.name = username
> + self.groups = self._get_groups()
> + # 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.role = self._get_role()
> +
> + def _get_groups(self):
> + pass
> +
> + def _get_role(self):
> + pass
> +
> @classmethod
> def get(cls, auth_args):
> auth_type = auth_args.pop('auth_type')
> @@ -78,29 +90,23 @@ class PAMUser(User):
> auth_type = "pam"
>
> def __init__(self, username):
> - self.user = {}
> - self.user[USER_NAME] = username
> - self.user[USER_GROUPS] = None
> - # 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')
> + super(PAMUser, self).__init__(username)
>
> - def get_groups(self):
> - out, err, rc = run_command(['id', '-Gn', self.user[USER_NAME]])
> + def _get_groups(self):
> + out, err, rc = run_command(['id', '-Gn', self.name])
> if rc == 0:
> - self.user[USER_GROUPS] = out.rstrip().split(" ")
> + return out.rstrip().split(" ")
>
> - return self.user[USER_GROUPS]
> + return None
>
> - def get_roles(self):
> + def _get_role(self):
> if self.has_sudo():
> # 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, 'admin')
> + return 'admin'
>
> - return self.user[USER_ROLES]
> + return 'user'
>
> def has_sudo(self):
> result = multiprocessing.Value('i', 0, lock=False)
> @@ -117,16 +123,16 @@ class PAMUser(User):
> os.setsid()
> fcntl.ioctl(slave, termios.TIOCSCTTY, 0)
>
> - out, err, exit = run_command(['sudo', '-l', '-U', self.user[USER_NAME],
> + out, err, exit = run_command(['sudo', '-l', '-U', self.name,
> 'sudo'])
> if exit == 0:
> - debug("User %s is allowed to run sudo" % self.user[USER_NAME])
> + debug("User %s is allowed to run sudo" % self.name)
> # sudo allows a wide range of configurations, such as controlling
> # which binaries the user can execute with sudo.
> # For now, we will just check whether the user is allowed to run
> # any command with sudo.
> out, err, exit = run_command(['sudo', '-l', '-U',
> - self.user[USER_NAME]])
> + self.name])
> for line in out.split('\n'):
> if line and re.search("(ALL)", line):
> result.value = True
> @@ -134,12 +140,9 @@ class PAMUser(User):
> result.value)
> return
> debug("User %s can only run some commands with sudo" %
> - self.user[USER_NAME])
> + self.name)
> else:
> - debug("User %s is not allowed to run sudo" % self.user[USER_NAME])
> -
> - def get_user(self):
> - return self.user
> + debug("User %s is not allowed to run sudo" % self.name)
>
> @staticmethod
> def authenticate(username, password, service="passwd"):
> @@ -189,12 +192,7 @@ class LDAPUser(User):
> auth_type = "ldap"
>
> def __init__(self, username):
> - self.user = {}
> - self.user[USER_NAME] = username
> - self.user[USER_GROUPS] = list()
> - # FIXME: user roles will be changed according roles assignment after
> - # objstore is integrated
> - self.user[USER_ROLES] = dict.fromkeys(tabs, 'user')
> + super(PAMUser, self).__init__(username)
>
> @staticmethod
> def authenticate(username, password):
> @@ -227,19 +225,16 @@ class LDAPUser(User):
> arg = {"username": username, "code": e.message}
> raise OperationFailed("WOKAUTH0001E", arg)
>
> - def get_groups(self):
> - return self.user[USER_GROUPS]
> + def _get_groups(self):
> + return None
>
> - def get_roles(self):
> + def _get_role(self):
> admin_ids = config.get(
> "authentication", "ldap_admin_id").strip('"').split(',')
> for admin_id in admin_ids:
> - if self.user[USER_NAME] == admin_id.strip():
> - self.user[USER_ROLES] = dict.fromkeys(tabs, 'admin')
> - return self.user[USER_ROLES]
> -
> - def get_user(self):
> - return self.user
> + if self.name == admin_id.strip():
> + return 'admin'
> + return 'user'
>
>
> def from_browser():
> @@ -313,11 +308,12 @@ def login(username, password, **kwargs):
> cherrypy.session.acquire_lock()
> cherrypy.session.regenerate()
> cherrypy.session[USER_NAME] = username
> - cherrypy.session[USER_GROUPS] = user.get_groups()
> - cherrypy.session[USER_ROLES] = user.get_roles()
> + cherrypy.session[USER_GROUPS] = user.groups
> + cherrypy.session[USER_ROLE] = user.role
> cherrypy.session[template.REFRESH] = time.time()
> cherrypy.session.release_lock()
> - return user.get_user()
> + return {USER_NAME: user.name, USER_GROUPS: user.groups,
> + USER_ROLE: user.role}
>
>
> def logout():
> diff --git a/src/wok/control/base.py b/src/wok/control/base.py
> index 786ecb7..3070e53 100644
> --- a/src/wok/control/base.py
> +++ b/src/wok/control/base.py
> @@ -2,7 +2,7 @@
> #
> # Project Wok
> #
> -# Copyright IBM Corp, 2015-2016
> +# Copyright IBM Corp, 2015-2017
> #
> # Code derived from Project Kimchi
> #
> @@ -27,7 +27,7 @@ import urllib2
>
> import wok.template
> from wok.asynctask import save_request_log_id
> -from wok.auth import USER_GROUPS, USER_NAME, USER_ROLES
> +from wok.auth import USER_GROUPS, USER_NAME, USER_ROLE
> from wok.control.utils import get_class_name, internal_redirect, model_fn
> from wok.control.utils import parse_request, validate_method
> from wok.control.utils import validate_params
> @@ -66,7 +66,6 @@ class Resource(object):
> self.model = model
> self.ident = ident
> self.model_args = (ident,)
> - self.role_key = None
> self.admin_methods = []
> self.log_map = {}
> self.log_args = {
> @@ -124,7 +123,7 @@ class Resource(object):
> status = 500
>
> method = 'POST'
> - validate_method((method), self.role_key, self.admin_methods)
> + validate_method((method), self.admin_methods)
> try:
> request = parse_request()
> validate_params(request, self, action_name)
> @@ -191,8 +190,7 @@ class Resource(object):
> details = None
> status = 500
>
> - method = validate_method(('GET', 'DELETE', 'PUT'),
> - self.role_key, self.admin_methods)
> + method = validate_method(('GET', 'DELETE', 'PUT'), self.admin_methods)
>
> try:
> self.lookup()
> @@ -222,7 +220,7 @@ class Resource(object):
> def is_authorized(self):
> user_name = cherrypy.session.get(USER_NAME, '')
> user_groups = cherrypy.session.get(USER_GROUPS, [])
> - user_role = cherrypy.session.get(USER_ROLES, {}).get(self.role_key)
> + user_role = cherrypy.session.get(USER_ROLE, None)
>
> users = self.data.get("users", None)
> groups = self.data.get("groups", None)
> @@ -331,7 +329,6 @@ class Collection(object):
> self.resource = Resource
> self.resource_args = []
> self.model_args = []
> - self.role_key = None
> self.admin_methods = []
> self.log_map = {}
> self.log_args = {}
> @@ -432,8 +429,7 @@ class Collection(object):
> status = 500
>
> params = {}
> - method = validate_method(('GET', 'POST'),
> - self.role_key, self.admin_methods)
> + method = validate_method(('GET', 'POST'), self.admin_methods)
>
> try:
> if method == 'GET':
> diff --git a/src/wok/control/logs.py b/src/wok/control/logs.py
> index 4844b89..739270c 100644
> --- a/src/wok/control/logs.py
> +++ b/src/wok/control/logs.py
> @@ -1,7 +1,7 @@
> #
> # Project Wok
> #
> -# Copyright IBM Corp, 2016
> +# Copyright IBM Corp, 2016-2017
> #
> # This library is free software; you can redistribute it and/or
> # modify it under the terms of the GNU Lesser General Public
> @@ -28,7 +28,6 @@ from wok.control.utils import UrlSubNode
> class Logs(SimpleCollection):
> def __init__(self, model):
> super(Logs, self).__init__(model)
> - self.role_key = 'logs'
> self.admin_methods = ['GET']
>
> def get(self, filter_params):
> diff --git a/src/wok/control/utils.py b/src/wok/control/utils.py
> index 987dd00..ef7b054 100644
> --- a/src/wok/control/utils.py
> +++ b/src/wok/control/utils.py
> @@ -1,7 +1,7 @@
> #
> # Project Wok
> #
> -# Copyright IBM Corp, 2015-2016
> +# Copyright IBM Corp, 2015-2017
> #
> # Code derived from Project Kimchi
> #
> @@ -24,7 +24,7 @@ import cherrypy
> import json
> from jsonschema import Draft3Validator, ValidationError, FormatChecker
>
> -from wok.auth import USER_ROLES
> +from wok.auth import USER_ROLE
> from wok.exception import InvalidParameter, OperationFailed
> from wok.utils import import_module, listPathModules
>
> @@ -41,12 +41,12 @@ def model_fn(cls, fn_name):
> return '%s_%s' % (get_class_name(cls), fn_name)
>
>
> -def validate_method(allowed, role_key, admin_methods):
> +def validate_method(allowed, admin_methods):
> method = cherrypy.request.method.upper()
> if method not in allowed:
> raise cherrypy.HTTPError(405)
>
> - user_role = cherrypy.session.get(USER_ROLES, {}).get(role_key)
> + user_role = cherrypy.session.get(USER_ROLE, None)
> if user_role and user_role != 'admin' and method in admin_methods:
> raise cherrypy.HTTPError(403)
>
> diff --git a/tests/test_server.py b/tests/test_server.py
> index 5541a4a..9c12b27 100644
> --- a/tests/test_server.py
> +++ b/tests/test_server.py
> @@ -224,10 +224,9 @@ class ServerTests(unittest.TestCase):
>
> user_info = json.loads(resp.read())
> self.assertEquals(sorted(user_info.keys()),
> - ['groups', 'roles', 'username'])
> - roles = user_info['roles']
> - for tab, role in roles.iteritems():
> - self.assertEquals(role, u'admin')
> + ['groups', 'role', 'username'])
> + role = user_info['role']
> + self.assertEquals(role, u'admin')
>
> cookie = resp.getheader('set-cookie')
> hdrs['Cookie'] = cookie
> diff --git a/tests/utils.py b/tests/utils.py
> index f26aeff..739434f 100644
> --- a/tests/utils.py
> +++ b/tests/utils.py
> @@ -34,7 +34,7 @@ import unittest
>
> import wok.server
>
> -from wok.auth import User, USER_NAME, USER_GROUPS, USER_ROLES, tabs
> +from wok.auth import User
> from wok.config import config
> from wok.exception import NotFoundError, OperationFailed
> from wok.utils import wok_log
> @@ -140,21 +140,15 @@ class FakeUser(User):
> sudo = True
>
> def __init__(self, username):
> - self.user = {}
> - self.user[USER_NAME] = username
> - self.user[USER_GROUPS] = None
> - self.user[USER_ROLES] = dict.fromkeys(tabs, 'user')
> + super(FakeUser, self).__init__(username)
>
> - def get_groups(self):
> + def _get_groups(self):
> return sorted([group.gr_name for group in grp.getgrall()])[0:3]
>
> - def get_roles(self):
> + def _get_role(self):
> if self.sudo:
> - self.user[USER_ROLES] = dict.fromkeys(tabs, 'admin')
> - return self.user[USER_ROLES]
> -
> - def get_user(self):
> - return self.user
> + return 'admin'
> + return 'user'
>
> @staticmethod
> def authenticate(username, password, service="passwd"):
> diff --git a/ui/js/src/wok.login.js b/ui/js/src/wok.login.js
> index fa2a98a..666a339 100644
> --- a/ui/js/src/wok.login.js
> +++ b/ui/js/src/wok.login.js
> @@ -1,7 +1,7 @@
> /*
> * Project Wok
> *
> - * Copyright IBM Corp, 2015-2016
> + * Copyright IBM Corp, 2015-2017
> *
> * Code derived from Project Kimchi
> *
> @@ -78,7 +78,7 @@ wok.login_main = function() {
> var lastPage = wok.cookie.get('lastPage');
> var next_url = lastPage ? lastPage.replace(/\"/g,'') : "/";
> }
> - wok.cookie.set('roles',JSON.stringify(data.roles));
> + wok.cookie.set('user_role', data.role);
> window.location.replace(window.location.pathname.replace(/\/+login.html/, '') + next_url);
> }, function(jqXHR, textStatus, errorThrown) {
> if (jqXHR.responseText == "") {
> diff --git a/ui/js/src/wok.main.js b/ui/js/src/wok.main.js
> index 658974a..c67e97c 100644
> --- a/ui/js/src/wok.main.js
> +++ b/ui/js/src/wok.main.js
> @@ -1,7 +1,7 @@
> /*
> * Project Wok
> *
> - * Copyright IBM Corp, 2015-2016
> + * Copyright IBM Corp, 2015-2017
> *
> * Code derived from Project Kimchi
> *
> @@ -91,12 +91,11 @@ wok.main = function() {
> var titleKey = tab.find('title').text();
> var title = i18n[titleKey] ? i18n[titleKey] : titleKey;
> var path = tab.find('path').text();
> - var roles = wok.cookie.get('roles');
> + var user_role = wok.cookie.get('user_role');
> var order = tab.find('order').text();
>
> - if (roles) {
> - var role = JSON.parse(roles)[titleKey.toLowerCase()];
> - var mode = tab.find('[role="' + role + '"]').attr('mode');
> + if (user_role) {
> + var mode = tab.find('[role=' + user_role + ']').attr('mode');
> wok.tabMode[titleKey.toLowerCase()] = mode;
> if (mode != 'none') {
> tabs.push({
More information about the Kimchi-devel
mailing list