[Kimchi-devel] [PATCH] [Wok] Do not link user role with UI tabs
Aline Manera
alinefm at linux.vnet.ibm.com
Thu Jan 19 12:17:46 UTC 2017
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({
--
2.7.4
More information about the Kimchi-devel
mailing list