From 84fbfe09e10b330a5668e99422247801f370d0f9 Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Tue, 21 Sep 2010 16:57:08 -0400 Subject: Rewrite rbac tests to use Authorizer middleware --- doc/source/auth.rst | 8 ---- nova/api/ec2/__init__.py | 3 ++ nova/auth/manager.py | 4 +- nova/tests/access_unittest.py | 108 ++++++++++++++++-------------------------- 4 files changed, 47 insertions(+), 76 deletions(-) diff --git a/doc/source/auth.rst b/doc/source/auth.rst index 70aca704a..3fcb309cd 100644 --- a/doc/source/auth.rst +++ b/doc/source/auth.rst @@ -172,14 +172,6 @@ Further Challenges -The :mod:`rbac` Module --------------------------- - -.. automodule:: nova.auth.rbac - :members: - :undoc-members: - :show-inheritance: - The :mod:`signer` Module ------------------------ diff --git a/nova/api/ec2/__init__.py b/nova/api/ec2/__init__.py index a7b10e428..b041787c2 100644 --- a/nova/api/ec2/__init__.py +++ b/nova/api/ec2/__init__.py @@ -25,6 +25,7 @@ import webob.dec import webob.exc from nova import exception +from nova import flags from nova import wsgi from nova.api.ec2 import apirequest from nova.api.ec2 import context @@ -33,6 +34,7 @@ from nova.api.ec2 import cloud from nova.auth import manager +FLAGS = flags.FLAGS _log = logging.getLogger("api") _log.setLevel(logging.DEBUG) @@ -176,6 +178,7 @@ class Authorizer(wsgi.Middleware): controller_name = req.environ['ec2.controller'].__class__.__name__ action = req.environ['ec2.action'] allowed_roles = self.action_roles[controller_name].get(action, []) + allowed_roles.extend(FLAGS.superuser_roles) if self._matches_any_role(context, allowed_roles): return self.application else: diff --git a/nova/auth/manager.py b/nova/auth/manager.py index bc3a8a12e..928e0fd69 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -44,7 +44,7 @@ flags.DEFINE_list('allowed_roles', # NOTE(vish): a user with one of these roles will be a superuser and # have access to all api commands flags.DEFINE_list('superuser_roles', ['cloudadmin'], - 'Roles that ignore rbac checking completely') + 'Roles that ignore authorization checking completely') # NOTE(vish): a user with one of these roles will have it for every # project, even if he or she is not a member of the project @@ -304,7 +304,7 @@ class AuthManager(object): return "%s:%s" % (user.access, Project.safe_id(project)) def is_superuser(self, user): - """Checks for superuser status, allowing user to bypass rbac + """Checks for superuser status, allowing user to bypass authorization @type user: User or uid @param user: User to check. diff --git a/nova/tests/access_unittest.py b/nova/tests/access_unittest.py index 59e1683db..d85f559f3 100644 --- a/nova/tests/access_unittest.py +++ b/nova/tests/access_unittest.py @@ -18,12 +18,13 @@ import unittest import logging +import webob from nova import exception from nova import flags from nova import test +from nova.api import ec2 from nova.auth import manager -from nova.auth import rbac FLAGS = flags.FLAGS @@ -72,9 +73,14 @@ class AccessTestCase(test.BaseTestCase): try: self.project.add_role(self.testsys, 'sysadmin') except: pass - self.context = Context() - self.context.project = self.project #user is set in each test + self.mw = ec2.Authorizer(lambda x,y: y('200 OK', []) and '') + self.mw.action_roles = {'str': { + '_allow_all': ['all'], + '_allow_none': [], + '_allow_project_manager': ['projectmanager'], + '_allow_sys_and_net': ['sysadmin', 'netadmin'], + '_allow_sysadmin': ['sysadmin']}} def tearDown(self): um = manager.AuthManager() @@ -87,76 +93,46 @@ class AccessTestCase(test.BaseTestCase): um.delete_user('testsys') super(AccessTestCase, self).tearDown() + def response_status(self, user, methodName): + context = Context() + context.project = self.project + context.user = user + environ = {'ec2.context' : context, + 'ec2.controller': 'some string', + 'ec2.action': methodName} + req = webob.Request.blank('/', environ) + resp = req.get_response(self.mw) + return resp.status_int + + def shouldAllow(self, user, methodName): + self.assertEqual(200, self.response_status(user, methodName)) + + def shouldDeny(self, user, methodName): + self.assertEqual(401, self.response_status(user, methodName)) + def test_001_allow_all(self): - self.context.user = self.testadmin - self.assertTrue(self._allow_all(self.context)) - self.context.user = self.testpmsys - self.assertTrue(self._allow_all(self.context)) - self.context.user = self.testnet - self.assertTrue(self._allow_all(self.context)) - self.context.user = self.testsys - self.assertTrue(self._allow_all(self.context)) + users = [self.testadmin, self.testpmsys, self.testnet, self.testsys] + for user in users: + self.shouldAllow(user, '_allow_all') def test_002_allow_none(self): - self.context.user = self.testadmin - self.assertTrue(self._allow_none(self.context)) - self.context.user = self.testpmsys - self.assertRaises(exception.NotAuthorized, self._allow_none, self.context) - self.context.user = self.testnet - self.assertRaises(exception.NotAuthorized, self._allow_none, self.context) - self.context.user = self.testsys - self.assertRaises(exception.NotAuthorized, self._allow_none, self.context) + self.shouldAllow(self.testadmin, '_allow_none') + users = [self.testpmsys, self.testnet, self.testsys] + for user in users: + self.shouldDeny(user, '_allow_none') def test_003_allow_project_manager(self): - self.context.user = self.testadmin - self.assertTrue(self._allow_project_manager(self.context)) - self.context.user = self.testpmsys - self.assertTrue(self._allow_project_manager(self.context)) - self.context.user = self.testnet - self.assertRaises(exception.NotAuthorized, self._allow_project_manager, self.context) - self.context.user = self.testsys - self.assertRaises(exception.NotAuthorized, self._allow_project_manager, self.context) + for user in [self.testadmin, self.testpmsys]: + self.shouldAllow(user, '_allow_project_manager') + for user in [self.testnet, self.testsys]: + self.shouldDeny(user, '_allow_project_manager') def test_004_allow_sys_and_net(self): - self.context.user = self.testadmin - self.assertTrue(self._allow_sys_and_net(self.context)) - self.context.user = self.testpmsys # doesn't have the per project sysadmin - self.assertRaises(exception.NotAuthorized, self._allow_sys_and_net, self.context) - self.context.user = self.testnet - self.assertTrue(self._allow_sys_and_net(self.context)) - self.context.user = self.testsys - self.assertTrue(self._allow_sys_and_net(self.context)) - - def test_005_allow_sys_no_pm(self): - self.context.user = self.testadmin - self.assertTrue(self._allow_sys_no_pm(self.context)) - self.context.user = self.testpmsys - self.assertRaises(exception.NotAuthorized, self._allow_sys_no_pm, self.context) - self.context.user = self.testnet - self.assertRaises(exception.NotAuthorized, self._allow_sys_no_pm, self.context) - self.context.user = self.testsys - self.assertTrue(self._allow_sys_no_pm(self.context)) - - @rbac.allow('all') - def _allow_all(self, context): - return True - - @rbac.allow('none') - def _allow_none(self, context): - return True - - @rbac.allow('projectmanager') - def _allow_project_manager(self, context): - return True - - @rbac.allow('sysadmin', 'netadmin') - def _allow_sys_and_net(self, context): - return True - - @rbac.allow('sysadmin') - @rbac.deny('projectmanager') - def _allow_sys_no_pm(self, context): - return True + for user in [self.testadmin, self.testnet, self.testsys]: + self.shouldAllow(user, '_allow_sys_and_net') + # denied because it doesn't have the per project sysadmin + for user in [self.testpmsys]: + self.shouldDeny(user, '_allow_sys_and_net') if __name__ == "__main__": # TODO: Implement use_fake as an option -- cgit From ffa426d68bfb3d1c2acaeef4c48d2662e88fc878 Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Tue, 21 Sep 2010 16:58:08 -0400 Subject: Reenable access_unittest now that it works with new rbac --- run_tests.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/run_tests.py b/run_tests.py index bea97c0b3..4121f4c06 100644 --- a/run_tests.py +++ b/run_tests.py @@ -49,8 +49,7 @@ from nova import datastore from nova import flags from nova import twistd -#TODO(gundlach): rewrite and readd this after merge -#from nova.tests.access_unittest import * +from nova.tests.access_unittest import * from nova.tests.auth_unittest import * from nova.tests.api_unittest import * from nova.tests.cloud_unittest import * -- cgit From f3f271644eac4ec74ce3786840a7743aac4f6032 Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Wed, 22 Sep 2010 15:57:24 -0400 Subject: Responding to eday's feedback -- make a clearer inner wsgi app --- nova/tests/access_unittest.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/nova/tests/access_unittest.py b/nova/tests/access_unittest.py index d85f559f3..c8a49d2ca 100644 --- a/nova/tests/access_unittest.py +++ b/nova/tests/access_unittest.py @@ -74,7 +74,10 @@ class AccessTestCase(test.BaseTestCase): self.project.add_role(self.testsys, 'sysadmin') except: pass #user is set in each test - self.mw = ec2.Authorizer(lambda x,y: y('200 OK', []) and '') + def noopWSGIApp(environ, start_response): + start_response('200 OK', []) + return [''] + self.mw = ec2.Authorizer(noopWSGIApp) self.mw.action_roles = {'str': { '_allow_all': ['all'], '_allow_none': [], -- cgit