Re: [Kimchi-devel] [PATCH] [Wok] Do not link user role with UI tabs

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(a)linux.vnet.ibm.com&gt; --- 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({