3:39 p.m.
From: Aline Manera <alinefm(a)linux.vnet.ibm.com>
Also move autorization mechanism to controller to be able to distinguish
resource and collection configuration
If we use UrlSubNode to also handle the authorization configuration, we won't be
able to specify different configuration to collection and its resource
as Kimchi uses the same base URL to both.
Example:
@UrlSubNode("vms", True, ["POST", "PUT",
"DELETE"], 'guests')
It meant that all the methods listed were exclusive for admin users.
Which it is not correct, as a user assigned to a VM can also perform POST,
PUT and DELETE actions. So fix it by moving the authorization mechanism to
controller
Aline Manera (5):
authorization: Filter resources by users and groups
authorization: Restrict Collection access based on admin_methods
parameter
authorization: Restrict access to Resource instance
authorization: Update control files to set role_key and admin_methods
authorization: Remove authorization config from UrlSubNode
Crístian Viana (4):
Return some groups for every user in mockmodel
Move "fake_user" credentials to mockmodel
List "admin" as a valid system user in mockmodel
authorization: Update test cases based on last changes
src/kimchi/auth.py | 16 +----------
src/kimchi/control/base.py | 56 +++++++++++++++++++++++++++++++-----
src/kimchi/control/debugreports.py | 8 +++++-
src/kimchi/control/host.py | 26 +++++++++++++++--
src/kimchi/control/interfaces.py | 6 +++-
src/kimchi/control/networks.py | 6 +++-
src/kimchi/control/storagepools.py | 6 +++-
src/kimchi/control/storageservers.py | 8 +++++-
src/kimchi/control/templates.py | 6 +++-
src/kimchi/control/utils.py | 14 +++++----
src/kimchi/control/vms.py | 6 +++-
src/kimchi/exception.py | 4 +++
src/kimchi/i18n.py | 1 +
src/kimchi/mockmodel.py | 5 +++-
src/kimchi/server.py | 4 ---
tests/test_authorization.py | 30 +++++++++++++++++--
tests/test_rest.py | 19 ++++++------
tests/utils.py | 9 +++---
18 files changed, 172 insertions(+), 58 deletions(-)
--
1.9.3
3:39 p.m.
New subject: [PATCH 1/9] authorization: Filter resources by users and groups
From: Aline Manera <alinefm(a)linux.vnet.ibm.com>
Currently, every user with 'admin' role can perform any operation on any virtual
machine.
In order to add more security, Kimchi will only allow users listed in
the VM metadata - along with those with 'admin' role - to be able to
perform actions on it. A VM may contain a list of system users and groups
associated with it. If a user is not listed to access a VM, they will
not be able to see it or to perform any operation on it.
Signed-off-by: Aline Manera <alinefm(a)linux.vnet.ibm.com>
Signed-off-by: Crístian Viana <vianac(a)linux.vnet.ibm.com>
---
src/kimchi/control/base.py | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py
index f8a5210..572f980 100644
--- a/src/kimchi/control/base.py
+++ b/src/kimchi/control/base.py
@@ -22,6 +22,7 @@
import kimchi.template
+from kimchi.auth import USER_GROUPS, USER_NAME, USER_ROLES
from kimchi.control.utils import get_class_name, internal_redirect, model_fn
from kimchi.control.utils import parse_request, validate_method
from kimchi.control.utils import validate_params
@@ -53,6 +54,8 @@ def __init__(self, model, ident=None):
self.ident = ident
self.model_args = (ident,)
self.update_params = []
+ self.role_key = None
+ self.admin_methods = []
def _redirect(self, ident, code=303):
if ident is not None and ident != self.ident:
@@ -134,6 +137,22 @@ def index(self):
except KimchiException, e:
raise cherrypy.HTTPError(500, e.message)
+ 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)
+
+ users = self.data.get("users", None)
+ groups = self.data.get("groups", None)
+
+ if (users is not None or groups is not None) and \
+ user_role and user_role != 'admin' and \
+ (user_name not in users or \
+ (groups and list(set(user_groups) & set(groups)) == [])):
+ return False
+
+ return True
+
def update(self):
try:
update = getattr(self.model, model_fn(self, 'update'))
@@ -195,6 +214,8 @@ def __init__(self, model):
self.resource = Resource
self.resource_args = []
self.model_args = []
+ self.role_key = None
+ self.admin_methods = []
def create(self, params, *args):
try:
@@ -239,6 +260,9 @@ def _cp_dispatch(self, vpath):
def filter_data(self, resources, fields_filter):
data = []
for res in resources:
+ if not res.is_authorized():
+ continue
+
if all(key in res.data and res.data[key] == val
for key, val in fields_filter.iteritems()):
data.append(res.data)
--
1.9.3
3:39 p.m.
New subject: [PATCH 2/9] authorization: Restrict Collection access based on admin_methods parameter
From: Aline Manera <alinefm(a)linux.vnet.ibm.com>
GET and POST are the allowed methods for a Collection
If you want to restrict access to the Collection based on the request
method, you must set the self.admin_method parameter accordingly
Kimchi will restrict all the POST request to a Collection, ie, only the
admin can create new resources in the Collection
Signed-off-by: Aline Manera <alinefm(a)linux.vnet.ibm.com>
---
src/kimchi/control/base.py | 4 +++-
src/kimchi/control/utils.py | 8 +++++++-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py
index 572f980..674c13b 100644
--- a/src/kimchi/control/base.py
+++ b/src/kimchi/control/base.py
@@ -284,7 +284,9 @@ def _split_filter(params):
@cherrypy.expose
def index(self, *args, **kwargs):
- method = validate_method(('GET', 'POST'))
+ method = validate_method(('GET', 'POST'),
+ self.role_key, self.admin_methods)
+
try:
if method == 'GET':
filter_params = cherrypy.request.params
diff --git a/src/kimchi/control/utils.py b/src/kimchi/control/utils.py
index aa592ef..aa5f452 100644
--- a/src/kimchi/control/utils.py
+++ b/src/kimchi/control/utils.py
@@ -25,6 +25,7 @@
from jsonschema import Draft3Validator, ValidationError, FormatChecker
+from kimchi.auth import USER_ROLES
from kimchi.exception import InvalidParameter, OperationFailed
from kimchi.utils import import_module, listPathModules
@@ -41,10 +42,15 @@ def model_fn(cls, fn_name):
return '%s_%s' % (get_class_name(cls), fn_name)
-def validate_method(allowed):
+def validate_method(allowed, role_key, 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)
+ if user_role and user_role != 'admin' and method in admin_methods:
+ raise cherrypy.HTTPError(403)
+
return method
--
1.9.3
3:39 p.m.
New subject: [PATCH 3/9] authorization: Restrict access to Resource instance
From: Aline Manera <alinefm(a)linux.vnet.ibm.com>
An user only can perform actions to a Resource if it is not protected or
if the logged user is listed in the resource users and groups parameters.
Also update the tests to reflect those changes.
Signed-off-by: Aline Manera <alinefm(a)linux.vnet.ibm.com>
---
src/kimchi/control/base.py | 28 ++++++++++++++++++++++------
src/kimchi/control/host.py | 2 +-
src/kimchi/exception.py | 4 ++++
src/kimchi/i18n.py | 1 +
tests/test_authorization.py | 12 ++++++++++--
tests/test_rest.py | 2 +-
6 files changed, 39 insertions(+), 10 deletions(-)
diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py
index 674c13b..ac24b3f 100644
--- a/src/kimchi/control/base.py
+++ b/src/kimchi/control/base.py
@@ -27,8 +27,8 @@
from kimchi.control.utils import parse_request, validate_method
from kimchi.control.utils import validate_params
from kimchi.exception import InvalidOperation, InvalidParameter
-from kimchi.exception import KimchiException
-from kimchi.exception import MissingParameter, NotFoundError, OperationFailed
+from kimchi.exception import KimchiException, MissingParameter, NotFoundError
+from kimchi.exception import OperationFailed, UnauthorizedError
class Resource(object):
@@ -65,8 +65,14 @@ def _redirect(self, ident, code=303):
def generate_action_handler(self, action_name, action_args=None):
def wrapper(*args, **kwargs):
- validate_method(('POST'))
+ method = validate_method(('POST'),
+ self.role_key, self.admin_methods)
+
try:
+ self.lookup()
+ if not self.is_authorized():
+ raise UnauthorizedError('KCHAPI0009E')
+
model_args = list(self.model_args)
if action_args is not None:
request = parse_request()
@@ -87,10 +93,12 @@ def wrapper(*args, **kwargs):
raise cherrypy.HTTPError(400, e.message)
except InvalidOperation, e:
raise cherrypy.HTTPError(400, e.message)
- except OperationFailed, e:
- raise cherrypy.HTTPError(500, e.message)
+ except UnauthorizedError, e:
+ raise cherrypy.HTTPError(403, e.message)
except NotFoundError, e:
raise cherrypy.HTTPError(404, e.message)
+ except OperationFailed, e:
+ raise cherrypy.HTTPError(500, e.message)
except KimchiException, e:
raise cherrypy.HTTPError(500, e.message)
@@ -121,8 +129,14 @@ def delete(self):
@cherrypy.expose
def index(self):
- method = validate_method(('GET', 'DELETE', 'PUT'))
+ method = validate_method(('GET', 'DELETE', 'PUT'),
+ self.role_key, self.admin_methods)
+
try:
+ self.lookup()
+ if not self.is_authorized():
+ raise UnauthorizedError('KCHAPI0009E')
+
return {'GET': self.get,
'DELETE': self.delete,
'PUT': self.update}[method]()
@@ -130,6 +144,8 @@ def index(self):
raise cherrypy.HTTPError(400, e.message)
except InvalidParameter, e:
raise cherrypy.HTTPError(400, e.message)
+ except UnauthorizedError, e:
+ raise cherrypy.HTTPError(403, e.message)
except NotFoundError, e:
raise cherrypy.HTTPError(404, e.message)
except OperationFailed, e:
diff --git a/src/kimchi/control/host.py b/src/kimchi/control/host.py
index e1971cc..c962472 100644
--- a/src/kimchi/control/host.py
+++ b/src/kimchi/control/host.py
@@ -42,7 +42,7 @@ def __init__(self, model, id=None):
@cherrypy.expose
def swupdate(self):
- validate_method(('POST'))
+ validate_method(('POST'), self.role_key, self.admin_methods)
try:
task = self.model.host_swupdate()
cherrypy.response.status = 202
diff --git a/src/kimchi/exception.py b/src/kimchi/exception.py
index d84ddb9..9828223 100644
--- a/src/kimchi/exception.py
+++ b/src/kimchi/exception.py
@@ -92,3 +92,7 @@ class IsoFormatError(KimchiException):
class TimeoutExpired(KimchiException):
pass
+
+
+class UnauthorizedError(KimchiException):
+ pass
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
index 0c76145..be6c532 100644
--- a/src/kimchi/i18n.py
+++ b/src/kimchi/i18n.py
@@ -31,6 +31,7 @@
"KCHAPI0006E": _("Unable to parse JSON request"),
"KCHAPI0007E": _("This API only supports JSON"),
"KCHAPI0008E": _("Parameters does not match requirement in schema:
%(err)s"),
+ "KCHAPI0009E": _("You don't have permission to perform this
operation."),
"KCHASYNC0001E": _("Datastore is not initiated in the model
object."),
"KCHASYNC0002E": _("Unable to start task due error: %(err)s"),
diff --git a/tests/test_authorization.py b/tests/test_authorization.py
index 03f8a88..3d0b357 100644
--- a/tests/test_authorization.py
+++ b/tests/test_authorization.py
@@ -116,7 +116,15 @@ def test_nonroot_access(self):
self.assertEquals(200, resp.status)
resp = self.request('/vms', req, 'POST')
self.assertEquals(403, resp.status)
- resp = self.request('/vms', '{}', 'PUT')
+
+ # Create a vm using mockmodel directly to test Resource access
+ model.templates_create({'name': 'test', 'cdrom':
'/nonexistent.iso'})
+ model.vms_create({'name': 'test', 'template':
'/templates/test'})
+
+ resp = self.request('/vms/test', '{}', 'PUT')
self.assertEquals(403, resp.status)
- resp = self.request('/vms', '{}', 'DELETE')
+ resp = self.request('/vms/test', '{}', 'DELETE')
self.assertEquals(403, resp.status)
+
+ model.template_delete('test')
+ model.vm_delete('test')
diff --git a/tests/test_rest.py b/tests/test_rest.py
index 54209ef..3c8c537 100644
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -1207,7 +1207,7 @@ def verify_template(t, res):
# Test nonexistent fields, specify a field 'foo' isn't in the
Template
t['foo'] = "bar"
req = json.dumps(t)
- resp = self.request('/templates/%s' % oldname, req, 'PUT')
+ resp = self.request('/templates/%s' % tmpl_name, req, 'PUT')
self.assertEquals(400, resp.status)
# Delete the template
--
1.9.3
3:39 p.m.
New subject: [PATCH 4/9] authorization: Update control files to set role_key and admin_methods
From: Aline Manera <alinefm(a)linux.vnet.ibm.com>
Each resource or collection must specify the role_key and admin_methods
values if it wants to be protected.
Signed-off-by: Aline Manera <alinefm(a)linux.vnet.ibm.com>
---
src/kimchi/control/debugreports.py | 6 ++++++
src/kimchi/control/host.py | 22 ++++++++++++++++++++++
src/kimchi/control/interfaces.py | 4 ++++
src/kimchi/control/networks.py | 4 ++++
src/kimchi/control/storagepools.py | 4 ++++
src/kimchi/control/storageservers.py | 6 ++++++
src/kimchi/control/templates.py | 4 ++++
src/kimchi/control/vms.py | 4 ++++
8 files changed, 54 insertions(+)
diff --git a/src/kimchi/control/debugreports.py b/src/kimchi/control/debugreports.py
index f0d5dcf..a561b99 100644
--- a/src/kimchi/control/debugreports.py
+++ b/src/kimchi/control/debugreports.py
@@ -27,6 +27,8 @@ class DebugReports(AsyncCollection):
def __init__(self, model):
super(DebugReports, self).__init__(model)
self.resource = DebugReport
+ self.role_key = 'host'
+ self.admin_methods = ['GET', 'POST']
def _get_resources(self, filter_params):
res_list = super(DebugReports, self)._get_resources(filter_params)
@@ -36,6 +38,8 @@ def _get_resources(self, filter_params):
class DebugReport(Resource):
def __init__(self, model, ident):
super(DebugReport, self).__init__(model, ident)
+ self.role_key = 'host'
+ self.admin_methods = ['GET', 'PUT', 'POST']
self.update_params = ["name"]
self.uri_fmt = '/debugreports/%s'
self.content = DebugReportContent(model, ident)
@@ -50,6 +54,8 @@ def data(self):
class DebugReportContent(Resource):
def __init__(self, model, ident):
super(DebugReportContent, self).__init__(model, ident)
+ self.role_key = 'host'
+ self.admin_methods = ['GET']
def get(self):
self.lookup()
diff --git a/src/kimchi/control/host.py b/src/kimchi/control/host.py
index c962472..1b55a29 100644
--- a/src/kimchi/control/host.py
+++ b/src/kimchi/control/host.py
@@ -29,6 +29,8 @@
class Host(Resource):
def __init__(self, model, id=None):
super(Host, self).__init__(model, id)
+ self.role_key = 'host'
+ self.admin_methods = ['GET', 'POST']
self.uri_fmt = '/host/%s'
self.reboot = self.generate_action_handler('reboot')
self.shutdown = self.generate_action_handler('shutdown')
@@ -58,6 +60,8 @@ def data(self):
class HostStats(Resource):
def __init__(self, model, id=None):
super(HostStats, self).__init__(model, id)
+ self.role_key = 'host'
+ self.admin_methods = ['GET']
self.history = HostStatsHistory(self.model)
@property
@@ -74,6 +78,8 @@ def data(self):
class Partitions(Collection):
def __init__(self, model):
super(Partitions, self).__init__(model)
+ self.role_key = 'storage'
+ self.admin_methods = ['GET']
self.resource = Partition
# Defining get_resources in order to return list of partitions in UI
@@ -87,6 +93,8 @@ def _get_resources(self, flag_filter):
class Partition(Resource):
def __init__(self, model, id):
+ self.role_key = 'storage'
+ self.admin_methods = ['GET']
super(Partition, self).__init__(model, id)
@property
@@ -100,11 +108,15 @@ def data(self):
class Devices(Collection):
def __init__(self, model):
super(Devices, self).__init__(model)
+ self.role_key = 'storage'
+ self.admin_methods = ['GET']
self.resource = Device
class Device(Resource):
def __init__(self, model, id):
+ self.role_key = 'storage'
+ self.admin_methods = ['GET']
super(Device, self).__init__(model, id)
@property
@@ -115,12 +127,16 @@ def data(self):
class PackagesUpdate(Collection):
def __init__(self, model):
super(PackagesUpdate, self).__init__(model)
+ self.role_key = 'host'
+ self.admin_methods = ['GET']
self.resource = PackageUpdate
class PackageUpdate(Resource):
def __init__(self, model, id=None):
super(PackageUpdate, self).__init__(model, id)
+ self.role_key = 'host'
+ self.admin_methods = ['GET']
@property
def data(self):
@@ -130,12 +146,16 @@ def data(self):
class Repositories(Collection):
def __init__(self, model):
super(Repositories, self).__init__(model)
+ self.role_key = 'host'
+ self.admin_methods = ['GET', 'POST']
self.resource = Repository
class Repository(Resource):
def __init__(self, model, id):
super(Repository, self).__init__(model, id)
+ self.role_key = 'host'
+ self.admin_methods = ['GET', 'PUT', 'POST',
'DELETE']
self.update_params = ["config", "baseurl"]
self.uri_fmt = "/host/repositories/%s"
self.enable = self.generate_action_handler('enable')
@@ -149,8 +169,10 @@ def data(self):
class Users(SimpleCollection):
def __init__(self, model):
super(Users, self).__init__(model)
+ self.role_key = 'guests'
class Groups(SimpleCollection):
def __init__(self, model):
super(Groups, self).__init__(model)
+ self.role_key = 'guests'
diff --git a/src/kimchi/control/interfaces.py b/src/kimchi/control/interfaces.py
index 4aa77b6..944cae6 100644
--- a/src/kimchi/control/interfaces.py
+++ b/src/kimchi/control/interfaces.py
@@ -25,12 +25,16 @@
class Interfaces(Collection):
def __init__(self, model):
super(Interfaces, self).__init__(model)
+ self.role_key = 'network'
+ self.admin_methods = ['GET']
self.resource = Interface
class Interface(Resource):
def __init__(self, model, ident):
super(Interface, self).__init__(model, ident)
+ self.role_key = 'network'
+ self.admin_methods = ['GET']
self.uri_fmt = "/interfaces/%s"
@property
diff --git a/src/kimchi/control/networks.py b/src/kimchi/control/networks.py
index 6bcc871..f7696e7 100644
--- a/src/kimchi/control/networks.py
+++ b/src/kimchi/control/networks.py
@@ -25,12 +25,16 @@
class Networks(Collection):
def __init__(self, model):
super(Networks, self).__init__(model)
+ self.role_key = 'network'
+ self.admin_methods = ['POST']
self.resource = Network
class Network(Resource):
def __init__(self, model, ident):
super(Network, self).__init__(model, ident)
+ self.role_key = 'network'
+ self.admin_methods = ['PUT', 'POST', 'DELETE']
self.uri_fmt = "/networks/%s"
self.activate = self.generate_action_handler('activate')
self.deactivate = self.generate_action_handler('deactivate')
diff --git a/src/kimchi/control/storagepools.py b/src/kimchi/control/storagepools.py
index 8c8b522..9e57992 100644
--- a/src/kimchi/control/storagepools.py
+++ b/src/kimchi/control/storagepools.py
@@ -32,6 +32,8 @@
class StoragePools(Collection):
def __init__(self, model):
super(StoragePools, self).__init__(model)
+ self.role_key = 'storage'
+ self.admin_methods = ['POST']
self.resource = StoragePool
isos = IsoPool(model)
setattr(self, ISO_POOL_NAME, isos)
@@ -73,6 +75,8 @@ def _get_resources(self, filter_params):
class StoragePool(Resource):
def __init__(self, model, ident):
super(StoragePool, self).__init__(model, ident)
+ self.role_key = 'storage'
+ self.admin_methods = ['PUT', 'POST', 'DELETE']
self.update_params = ["autostart", "disks"]
self.uri_fmt = "/storagepools/%s"
self.activate = self.generate_action_handler('activate')
diff --git a/src/kimchi/control/storageservers.py b/src/kimchi/control/storageservers.py
index 9c7bebc..e86f920 100644
--- a/src/kimchi/control/storageservers.py
+++ b/src/kimchi/control/storageservers.py
@@ -26,12 +26,16 @@
class StorageServers(Collection):
def __init__(self, model):
super(StorageServers, self).__init__(model)
+ self.role_key = 'storage'
+ self.admin_methods = ['GET']
self.resource = StorageServer
class StorageServer(Resource):
def __init__(self, model, ident):
super(StorageServer, self).__init__(model, ident)
+ self.role_key = 'storage'
+ self.admin_methods = ['GET']
self.storagetargets = StorageTargets(self.model,
self.ident.decode("utf-8"))
@@ -43,6 +47,8 @@ def data(self):
class StorageTargets(Collection):
def __init__(self, model, server):
super(StorageTargets, self).__init__(model)
+ self.role_key = 'storage'
+ self.admin_methods = ['GET']
self.server = server
self.resource_args = [self.server, ]
self.model_args = [self.server, ]
diff --git a/src/kimchi/control/templates.py b/src/kimchi/control/templates.py
index 907929f..167e19e 100644
--- a/src/kimchi/control/templates.py
+++ b/src/kimchi/control/templates.py
@@ -25,12 +25,16 @@
class Templates(Collection):
def __init__(self, model):
super(Templates, self).__init__(model)
+ self.role_key = 'templates'
+ self.admin_methods = ['GET', 'POST']
self.resource = Template
class Template(Resource):
def __init__(self, model, ident):
super(Template, self).__init__(model, ident)
+ self.role_key = 'templates'
+ self.admin_methods = ['PUT', 'POST', 'DELETE']
self.update_params = ["name", "folder", "icon",
"os_distro",
"storagepool", "os_version",
"cpus",
"memory", "cdrom", "disks",
"networks",
diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py
index cf427fa..c36d72a 100644
--- a/src/kimchi/control/vms.py
+++ b/src/kimchi/control/vms.py
@@ -27,11 +27,14 @@ class VMs(Collection):
def __init__(self, model):
super(VMs, self).__init__(model)
self.resource = VM
+ self.role_key = 'guests'
+ self.admin_methods = ['POST']
class VM(Resource):
def __init__(self, model, ident):
super(VM, self).__init__(model, ident)
+ self.role_key = 'guests'
self.update_params = ["name", "users", "groups",
"cpus", "memory"]
self.screenshot = VMScreenShot(model, ident)
self.uri_fmt = '/vms/%s'
@@ -51,6 +54,7 @@ def data(self):
class VMScreenShot(Resource):
def __init__(self, model, ident):
super(VMScreenShot, self).__init__(model, ident)
+ self.role_key = 'guests'
def get(self):
self.lookup()
--
1.9.3
3:39 p.m.
New subject: [PATCH 5/9] authorization: Remove authorization config from UrlSubNode
From: Aline Manera <alinefm(a)linux.vnet.ibm.com>
UrlSubNode is used to automatically load the application configuration
and set kimchiauth tool when needed.
If we use it to also handle the authorization configuration, we won't be
able to specify different configuration to collection and its resource
as Kimchi uses the same base URL to both.
Example:
@UrlSubNode("vms", True, ["POST", "PUT",
"DELETE"], 'guests')
It meant that all the methods listed were exclusive for admin users.
Which it is not correct, as a user assigned to a VM can also perform POST,
PUT and DELETE actions.
To be able to distinguish the configuration for resource and collection, the
autorization mechanism was moved to controller.
Signed-off-by: Aline Manera <alinefm(a)linux.vnet.ibm.com>
---
src/kimchi/auth.py | 16 +---------------
src/kimchi/control/debugreports.py | 2 +-
src/kimchi/control/host.py | 2 +-
src/kimchi/control/interfaces.py | 2 +-
src/kimchi/control/networks.py | 2 +-
src/kimchi/control/storagepools.py | 2 +-
src/kimchi/control/storageservers.py | 2 +-
src/kimchi/control/templates.py | 2 +-
src/kimchi/control/utils.py | 6 +-----
src/kimchi/control/vms.py | 2 +-
src/kimchi/server.py | 4 ----
11 files changed, 10 insertions(+), 32 deletions(-)
diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py
index aabcb6c..93a47b3 100644
--- a/src/kimchi/auth.py
+++ b/src/kimchi/auth.py
@@ -243,27 +243,13 @@ def logout():
cherrypy.lib.sessions.close()
-def has_permission(admin_methods, tab):
- cherrypy.session.acquire_lock()
- role = cherrypy.session.get(USER_ROLES, {}).get(tab, 'user')
- cherrypy.session.release_lock()
-
- return not admin_methods or \
- cherrypy.request.method not in admin_methods or \
- (cherrypy.request.method in admin_methods and role == "admin")
-
-
-def kimchiauth(admin_methods=None, tab=None):
+def kimchiauth():
debug("Entering kimchiauth...")
session_missing = cherrypy.session.missing
if check_auth_session():
- if not has_permission(admin_methods, tab):
- raise cherrypy.HTTPError(403)
return
if check_auth_httpba():
- if not has_permission(admin_methods, tab):
- raise cherrypy.HTTPError(403)
return
# not a REST full request, redirect login page directly
diff --git a/src/kimchi/control/debugreports.py b/src/kimchi/control/debugreports.py
index a561b99..debc2eb 100644
--- a/src/kimchi/control/debugreports.py
+++ b/src/kimchi/control/debugreports.py
@@ -22,7 +22,7 @@
from kimchi.control.utils import UrlSubNode
-@UrlSubNode('debugreports', True, ['GET', 'PUT', 'POST',
'DELETE'], 'host')
+@UrlSubNode('debugreports', True)
class DebugReports(AsyncCollection):
def __init__(self, model):
super(DebugReports, self).__init__(model)
diff --git a/src/kimchi/control/host.py b/src/kimchi/control/host.py
index 1b55a29..172f4fe 100644
--- a/src/kimchi/control/host.py
+++ b/src/kimchi/control/host.py
@@ -25,7 +25,7 @@
from kimchi.template import render
-@UrlSubNode('host', True, ['GET', 'PUT', 'POST',
'DELETE'], 'host')
+@UrlSubNode('host', True)
class Host(Resource):
def __init__(self, model, id=None):
super(Host, self).__init__(model, id)
diff --git a/src/kimchi/control/interfaces.py b/src/kimchi/control/interfaces.py
index 944cae6..317cc6f 100644
--- a/src/kimchi/control/interfaces.py
+++ b/src/kimchi/control/interfaces.py
@@ -21,7 +21,7 @@
from kimchi.control.utils import UrlSubNode
-@UrlSubNode('interfaces', True, ['GET'], 'network')
+@UrlSubNode('interfaces', True)
class Interfaces(Collection):
def __init__(self, model):
super(Interfaces, self).__init__(model)
diff --git a/src/kimchi/control/networks.py b/src/kimchi/control/networks.py
index f7696e7..760295c 100644
--- a/src/kimchi/control/networks.py
+++ b/src/kimchi/control/networks.py
@@ -21,7 +21,7 @@
from kimchi.control.utils import UrlSubNode
-@UrlSubNode('networks', True, ['PUT', 'POST', 'DELETE'],
'network')
+@UrlSubNode('networks', True)
class Networks(Collection):
def __init__(self, model):
super(Networks, self).__init__(model)
diff --git a/src/kimchi/control/storagepools.py b/src/kimchi/control/storagepools.py
index 9e57992..c023505 100644
--- a/src/kimchi/control/storagepools.py
+++ b/src/kimchi/control/storagepools.py
@@ -28,7 +28,7 @@
from kimchi.control.utils import UrlSubNode
-@UrlSubNode('storagepools', True, ['PUT', 'POST',
'DELETE'], 'storage')
+@UrlSubNode('storagepools', True)
class StoragePools(Collection):
def __init__(self, model):
super(StoragePools, self).__init__(model)
diff --git a/src/kimchi/control/storageservers.py b/src/kimchi/control/storageservers.py
index e86f920..4b70c39 100644
--- a/src/kimchi/control/storageservers.py
+++ b/src/kimchi/control/storageservers.py
@@ -22,7 +22,7 @@
from kimchi.control.utils import get_class_name, model_fn, UrlSubNode
-@UrlSubNode('storageservers', True, ['GET'], 'storage')
+@UrlSubNode('storageservers', True)
class StorageServers(Collection):
def __init__(self, model):
super(StorageServers, self).__init__(model)
diff --git a/src/kimchi/control/templates.py b/src/kimchi/control/templates.py
index 167e19e..020902d 100644
--- a/src/kimchi/control/templates.py
+++ b/src/kimchi/control/templates.py
@@ -21,7 +21,7 @@
from kimchi.control.utils import UrlSubNode
-@UrlSubNode('templates', True, ['GET', 'PUT', 'POST',
'DELETE'], 'templates')
+@UrlSubNode('templates', True)
class Templates(Collection):
def __init__(self, model):
super(Templates, self).__init__(model)
diff --git a/src/kimchi/control/utils.py b/src/kimchi/control/utils.py
index aa5f452..c39dbd8 100644
--- a/src/kimchi/control/utils.py
+++ b/src/kimchi/control/utils.py
@@ -113,21 +113,17 @@ def validate_params(params, instance, action):
class UrlSubNode(object):
- def __init__(self, name, auth=False, admin_methods=None, tab=None):
+ def __init__(self, name, auth=False):
"""
admin_methods must be None, or a list containing zero or more of the
string values ['GET', 'POST', 'PUT', 'DELETE']
"""
self.name = name
self.auth = auth
- self.tab = tab
- self.admin_methods = admin_methods
def __call__(self, fun):
fun._url_sub_node_name = {"name": self.name}
fun.url_auth = self.auth
- fun.tab = self.tab
- fun.admin_methods = self.admin_methods
return fun
diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py
index c36d72a..28ad775 100644
--- a/src/kimchi/control/vms.py
+++ b/src/kimchi/control/vms.py
@@ -22,7 +22,7 @@
from kimchi.control.vm import sub_nodes
-@UrlSubNode('vms', True, ['POST', 'PUT', 'DELETE'],
'guests')
+@UrlSubNode('vms', True)
class VMs(Collection):
def __init__(self, model):
super(VMs, self).__init__(model)
diff --git a/src/kimchi/server.py b/src/kimchi/server.py
index b0e9474..3f49f6c 100644
--- a/src/kimchi/server.py
+++ b/src/kimchi/server.py
@@ -129,10 +129,6 @@ def __init__(self, options):
cfg = self.configObj
ident = "/%s" % ident
cfg[ident] = {'tools.kimchiauth.on': True}
- if node.admin_methods:
- cfg[ident]['tools.kimchiauth.tab'] = node.tab
- cfg[ident][
- 'tools.kimchiauth.admin_methods'] = node.admin_methods
self.app = cherrypy.tree.mount(KimchiRoot(model_instance, dev_env),
config=self.configObj)
--
1.9.3
3:39 p.m.
New subject: [PATCH 6/9] Return some groups for every user in mockmodel
From: Crístian Viana <vianac(a)linux.vnet.ibm.com>
In the current mockmodel implementation, the "environment" returns None
as the group list for all system users.
In order to make the mockmodel environment more dynamic, every user will
belong to the same list of groups - instead of none.
Signed-off-by: Crístian Viana <vianac(a)linux.vnet.ibm.com>
---
tests/utils.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/utils.py b/tests/utils.py
index 4853b7a..12961cc 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -155,7 +155,7 @@ def patch_auth(sudo=True):
"""
def _get_groups(self):
- return None
+ return [ 'groupA', 'groupB', 'wheel' ]
def _has_sudo(self, result):
result.value = sudo
--
1.9.3
3:39 p.m.
New subject: [PATCH 7/9] Move "fake_user" credentials to mockmodel
From: Crístian Viana <vianac(a)linux.vnet.ibm.com>
The "fake_user" credentials are used along with the mockmodel. Instead
of having it declared on a test file, move the declaration to a more
generic place.
Move the "fake_user" credentials from the test files to the mockmodel.
Signed-off-by: Crístian Viana <vianac(a)linux.vnet.ibm.com>
---
src/kimchi/mockmodel.py | 3 +++
tests/test_rest.py | 8 ++++----
tests/utils.py | 7 +++----
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
index 0e45d1e..f8e33b7 100644
--- a/src/kimchi/mockmodel.py
+++ b/src/kimchi/mockmodel.py
@@ -57,6 +57,9 @@
from kimchi.vmtemplate import VMTemplate
+fake_user = { 'admin': 'letmein!' }
+
+
class MockModel(object):
def __init__(self, objstore_loc=None):
self.reset()
diff --git a/tests/test_rest.py b/tests/test_rest.py
index 3c8c537..935ed81 100644
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -35,7 +35,7 @@
import kimchi.mockmodel
import kimchi.server
from kimchi.rollbackcontext import RollbackContext
-from utils import fake_user, get_free_port, patch_auth, request
+from utils import get_free_port, patch_auth, request
from utils import run_server
@@ -108,7 +108,7 @@ def test_404(self):
# We must be authenticated first. Otherwise all requests will return
# HTTP:401. Since HTTP Simple Auth is not allowed for text/html, we
# need to use the login API and establish a session.
- user, pw = fake_user.items()[0]
+ user, pw = kimchi.mockmodel.fake_user.items()[0]
req = json.dumps({'username': user, 'password': pw})
resp = self.request('/login', req, 'POST')
self.assertEquals(200, resp.status)
@@ -1502,7 +1502,7 @@ def test_auth_unprotected(self):
resp = self.request(uri, None, 'HEAD', hdrs)
self.assertEquals(200, resp.status)
- user, pw = fake_user.items()[0]
+ user, pw = kimchi.mockmodel.fake_user.items()[0]
req = json.dumps({'username': user, 'password': pw})
resp = self.request('/login', req, 'POST', hdrs)
self.assertEquals(200, resp.status)
@@ -1548,7 +1548,7 @@ def test_auth_session(self):
self.assertEquals(401, resp.status)
# Execute a login call
- user, pw = fake_user.items()[0]
+ user, pw = kimchi.mockmodel.fake_user.items()[0]
req = json.dumps({'username': user, 'password': pw})
resp = self.request('/login', req, 'POST', hdrs)
self.assertEquals(200, resp.status)
diff --git a/tests/utils.py b/tests/utils.py
index 12961cc..f452854 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -32,13 +32,12 @@
from lxml import etree
+import kimchi.mockmodel
import kimchi.server
from kimchi.exception import OperationFailed
_ports = {}
-fake_user = {'admin': 'letmein!'}
-
# provide missing unittest decorators and API for python 2.6; these decorators
# do not actually work, just avoid the syntax failure
if sys.version_info[:2] == (2, 6):
@@ -136,7 +135,7 @@ def _request(conn, path, data, method, headers):
headers = {'Content-Type': 'application/json',
'Accept': 'application/json'}
if 'AUTHORIZATION' not in headers.keys():
- user, pw = fake_user.items()[0]
+ user, pw = kimchi.mockmodel.fake_user.items()[0]
hdr = "Basic " + base64.b64encode("%s:%s" % (user, pw))
headers['AUTHORIZATION'] = hdr
conn.request(method, path, data, headers)
@@ -162,7 +161,7 @@ def _has_sudo(self, result):
def _authenticate(username, password, service="passwd"):
try:
- return fake_user[username] == password
+ return kimchi.mockmodel.fake_user[username] == password
except KeyError, e:
raise OperationFailed("KCHAUTH0001E", {'username':
'username',
'code': e.message})
--
1.9.3
3:39 p.m.
New subject: [PATCH 8/9] List "admin" as a valid system user in mockmodel
From: Crístian Viana <vianac(a)linux.vnet.ibm.com>
The mockmodel environment reports a static list of users available on
the system. Currently, it does not include the fake user which is used
by the mockmodel authentication.
In order to make the code consistent, the user which authenticates to
the mockmodel environment should be listed as a valid system user.
Signed-off-by: Crístian Viana <vianac(a)linux.vnet.ibm.com>
---
src/kimchi/mockmodel.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
index f8e33b7..94d7adf 100644
--- a/src/kimchi/mockmodel.py
+++ b/src/kimchi/mockmodel.py
@@ -867,7 +867,7 @@ def hoststatshistory_lookup(self, *name):
'net_sent_rate': random.sample(range(4000), 30)}
def users_get_list(self):
- return ["userA", "userB", "userC"]
+ return ["userA", "userB", "userC",
"admin"]
def groups_get_list(self):
return ["groupA", "groupB", "groupC",
"groupD"]
--
1.9.3
3:39 p.m.
New subject: [PATCH 9/9] authorization: Update test cases based on last changes
From: Crístian Viana <vianac(a)linux.vnet.ibm.com>
---
tests/test_authorization.py | 20 ++++++++++++++++++--
tests/test_rest.py | 9 ++++++---
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/tests/test_authorization.py b/tests/test_authorization.py
index 3d0b357..2fca62e 100644
--- a/tests/test_authorization.py
+++ b/tests/test_authorization.py
@@ -111,14 +111,24 @@ def test_nonroot_access(self):
resp = self.request('/templates/test', '{}', 'DELETE')
self.assertEquals(403, resp.status)
- # Non-root users can only get vms
+
+ # Non-root users can only get vms authorized to them
+ model.templates_create({'name': u'test', 'cdrom':
'/nonexistent.iso'})
+
+ model.vms_create({'name': u'test-me', 'template':
'/templates/test'})
+ model.vm_update(u'test-me', {'users': [
kimchi.mockmodel.fake_user.keys()[0] ], 'groups': []})
+
+ model.vms_create({'name': u'test-usera', 'template':
'/templates/test'})
+ model.vm_update(u'test-usera', {'users': [ 'userA' ],
'groups': []})
+
resp = self.request('/vms', '{}', 'GET')
self.assertEquals(200, resp.status)
+ vms_data = json.loads(resp.read())
+ self.assertEquals([ u'test-me' ], [ v['name'] for v in vms_data
])
resp = self.request('/vms', req, 'POST')
self.assertEquals(403, resp.status)
# Create a vm using mockmodel directly to test Resource access
- model.templates_create({'name': 'test', 'cdrom':
'/nonexistent.iso'})
model.vms_create({'name': 'test', 'template':
'/templates/test'})
resp = self.request('/vms/test', '{}', 'PUT')
@@ -126,5 +136,11 @@ def test_nonroot_access(self):
resp = self.request('/vms/test', '{}', 'DELETE')
self.assertEquals(403, resp.status)
+ # Non-root users can only update VMs authorized by them
+ resp = self.request('/vms/test-me/start', '{}', 'POST')
+ self.assertEquals(200, resp.status)
+ resp = self.request('/vms/test-usera/start', '{}',
'POST')
+ self.assertEquals(403, resp.status)
+
model.template_delete('test')
model.vm_delete('test')
diff --git a/tests/test_rest.py b/tests/test_rest.py
index 935ed81..06d9f9e 100644
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -175,10 +175,13 @@ def test_get_vms(self):
resp = self.request('/templates', req, 'POST')
self.assertEquals(201, resp.status)
+ test_users = [ 'user1', 'user2', 'root']
+ test_groups = [ 'group1', 'group2', 'admin' ]
# Now add a couple of VMs to the mock model
for i in xrange(10):
name = 'vm-%i' % i
- req = json.dumps({'name': name, 'template':
'/templates/test'})
+ req = json.dumps({'name': name, 'template':
'/templates/test',
+ 'users': test_users, 'groups':
test_groups})
resp = self.request('/vms', req, 'POST')
self.assertEquals(201, resp.status)
@@ -188,8 +191,8 @@ def test_get_vms(self):
vm = json.loads(self.request('/vms/vm-1').read())
self.assertEquals('vm-1', vm['name'])
self.assertEquals('shutoff', vm['state'])
- self.assertEquals(['user1', 'user2', 'root'],
vm['users'])
- self.assertEquals(['group1', 'group2', 'admin'],
vm['groups'])
+ self.assertEquals(test_users, vm['users'])
+ self.assertEquals(test_groups, vm['groups'])
def test_edit_vm(self):
req = json.dumps({'name': 'test', 'cdrom':
'/nonexistent.iso'})
--
1.9.3
9:43 a.m.
Reviewed-by: Crístian Viana <vianac(a)linux.vnet.ibm.com>
On 23-07-2014 17:39, alinefm(a)linux.vnet.ibm.com wrote:
From: Aline Manera <alinefm(a)linux.vnet.ibm.com>
Also move autorization mechanism to controller to be able to distinguish
resource and collection configuration
If we use UrlSubNode to also handle the authorization configuration, we won't be
able to specify different configuration to collection and its resource
as Kimchi uses the same base URL to both.
Example:
@UrlSubNode("vms", True, ["POST", "PUT",
"DELETE"], 'guests')
It meant that all the methods listed were exclusive for admin users.
Which it is not correct, as a user assigned to a VM can also perform POST,
PUT and DELETE actions. So fix it by moving the authorization mechanism to
controller
Aline Manera (5):
authorization: Filter resources by users and groups
authorization: Restrict Collection access based on admin_methods
parameter
authorization: Restrict access to Resource instance
authorization: Update control files to set role_key and admin_methods
authorization: Remove authorization config from UrlSubNode
Crístian Viana (4):
Return some groups for every user in mockmodel
Move "fake_user" credentials to mockmodel
List "admin" as a valid system user in mockmodel
authorization: Update test cases based on last changes
src/kimchi/auth.py | 16 +----------
src/kimchi/control/base.py | 56 +++++++++++++++++++++++++++++++-----
src/kimchi/control/debugreports.py | 8 +++++-
src/kimchi/control/host.py | 26 +++++++++++++++--
src/kimchi/control/interfaces.py | 6 +++-
src/kimchi/control/networks.py | 6 +++-
src/kimchi/control/storagepools.py | 6 +++-
src/kimchi/control/storageservers.py | 8 +++++-
src/kimchi/control/templates.py | 6 +++-
src/kimchi/control/utils.py | 14 +++++----
src/kimchi/control/vms.py | 6 +++-
src/kimchi/exception.py | 4 +++
src/kimchi/i18n.py | 1 +
src/kimchi/mockmodel.py | 5 +++-
src/kimchi/server.py | 4 ---
tests/test_authorization.py | 30 +++++++++++++++++--
tests/test_rest.py | 19 ++++++------
tests/utils.py | 9 +++---
18 files changed, 172 insertions(+), 58 deletions(-)
3730
days inactive
3731
days old
11 comments
3 participants
participants (3)
-
Aline Manera -
alinefm＠linux.vnet.ibm.com
-
Crístian Viana