From f40fba7b9c160ccc5091d6a0d15e82a2c34170d7 Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Tue, 21 Sep 2010 16:57:08 -0400 Subject: [PATCH 1/6] Rewrite rbac tests to use Authorizer middleware --- nova/auth/manager.py | 4 +- nova/tests/access_unittest.py | 108 +++++++++++++--------------------- 2 files changed, 44 insertions(+), 68 deletions(-) diff --git a/nova/auth/manager.py b/nova/auth/manager.py index bc3a8a12..928e0fd6 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 59e1683d..d85f559f 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 From e32eed189e8e55710bd1f9344d5e50a567fa1781 Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Tue, 21 Sep 2010 16:58:08 -0400 Subject: [PATCH 2/6] 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 bea97c0b..4121f4c0 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 * From 4f4706cbe0854241dc3b69e98eacf5e8107ef13f Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Wed, 22 Sep 2010 15:57:24 -0400 Subject: [PATCH 3/6] 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 d85f559f..c8a49d2c 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': [], From c4514bb23c938afc51d9cf61728dec0e409cabaf Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Thu, 23 Sep 2010 09:13:27 -0400 Subject: [PATCH 4/6] Apply vish's patch --- bin/nova-api-new | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bin/nova-api-new b/bin/nova-api-new index 8625c487..6f25ad8c 100755 --- a/bin/nova-api-new +++ b/bin/nova-api-new @@ -34,12 +34,11 @@ if os.path.exists(os.path.join(possible_topdir, 'nova', '__init__.py')): from nova import api from nova import flags -from nova import utils from nova import wsgi FLAGS = flags.FLAGS flags.DEFINE_integer('api_port', 8773, 'API port') if __name__ == '__main__': - utils.default_flagfile() + FLAGS(sys.argv) wsgi.run_server(api.API(), FLAGS.api_port) From 54a5a74653f027c6547b2ab40d77cc9c11703302 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Thu, 23 Sep 2010 15:47:29 +0200 Subject: [PATCH 5/6] Wrap WSGI container in server.serve to make it properly handle command line arguments as well as daemonise properly. Moved api and wsgi imports in the main() function to delay their inclusion until after python-daemon has closed all the file descriptors. Without this, eventlet's epoll fd gets opened before daemonize is called and thus its fd gets closed leading to very, very, very confusing errors. --- bin/nova-api-new | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/bin/nova-api-new b/bin/nova-api-new index 6f25ad8c..a5027700 100755 --- a/bin/nova-api-new +++ b/bin/nova-api-new @@ -32,13 +32,18 @@ possible_topdir = os.path.normpath(os.path.join(os.path.abspath(sys.argv[0]), if os.path.exists(os.path.join(possible_topdir, 'nova', '__init__.py')): sys.path.insert(0, possible_topdir) -from nova import api from nova import flags -from nova import wsgi +from nova import utils +from nova import server FLAGS = flags.FLAGS flags.DEFINE_integer('api_port', 8773, 'API port') -if __name__ == '__main__': - FLAGS(sys.argv) +def main(_args): + from nova import api + from nova import wsgi wsgi.run_server(api.API(), FLAGS.api_port) + +if __name__ == '__main__': + utils.default_flagfile() + server.serve('nova-api', main) From c85fddf553d7f297e5133d0037b6bb1208290bc8 Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Thu, 23 Sep 2010 10:19:27 -0400 Subject: [PATCH 6/6] Spot-fix endpoint reference --- bin/nova-manage | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/nova-manage b/bin/nova-manage index 824e00ac..e9219c51 100755 --- a/bin/nova-manage +++ b/bin/nova-manage @@ -73,7 +73,7 @@ from nova import quota from nova import utils from nova.auth import manager from nova.cloudpipe import pipelib -from nova.endpoint import cloud +from nova.api.ec2 import cloud FLAGS = flags.FLAGS