From 43da0e268fd88c05e49a3d949e3685a13fa43926 Mon Sep 17 00:00:00 2001 From: asteroide Date: Sat, 26 Sep 2015 23:31:49 +0200 Subject: Review the KeystoneMiddleware code, fix some bugs in the authz functions. Change-Id: I9d9966c061fc71cd8ef5ce88217dcdfa63e0722f --- keystone-moon/keystone/contrib/moon/algorithms.py | 3 ++ keystone-moon/keystone/contrib/moon/controllers.py | 8 ++-- keystone-moon/keystone/contrib/moon/core.py | 19 +++----- keystone-moon/keystone/contrib/moon/routers.py | 6 +-- .../unit/test_unit_core_intra_extension_admin.py | 22 ++++------ .../unit/test_unit_core_intra_extension_authz.py | 51 ++++++++++------------ 6 files changed, 47 insertions(+), 62 deletions(-) (limited to 'keystone-moon/keystone') diff --git a/keystone-moon/keystone/contrib/moon/algorithms.py b/keystone-moon/keystone/contrib/moon/algorithms.py index 30305fc1..2f997efc 100644 --- a/keystone-moon/keystone/contrib/moon/algorithms.py +++ b/keystone-moon/keystone/contrib/moon/algorithms.py @@ -1,4 +1,7 @@ import itertools +from oslo_log import log +LOG = log.getLogger(__name__) + """ an example of authz_buffer, sub_meta_rule_dict, rule_dict authz_buffer = { diff --git a/keystone-moon/keystone/contrib/moon/controllers.py b/keystone-moon/keystone/contrib/moon/controllers.py index c4fc0add..58e62a28 100644 --- a/keystone-moon/keystone/contrib/moon/controllers.py +++ b/keystone-moon/keystone/contrib/moon/controllers.py @@ -134,11 +134,11 @@ class Authz_v3(controller.V3Controller): super(Authz_v3, self).__init__() @controller.protected() - def get_authz(self, context, tenant_name, subject_name, object_name, action_name): + def get_authz(self, context, tenant_id, subject_k_id, object_name, action_name): try: - return self.authz_api.authz(tenant_name, subject_name, object_name, action_name) - except: - return False + return self.authz_api.authz(tenant_id, subject_k_id, object_name, action_name) + except Exception as e: + return {'authz': False, 'comment': unicode(e)} @dependency.requires('admin_api', 'authz_api') diff --git a/keystone-moon/keystone/contrib/moon/core.py b/keystone-moon/keystone/contrib/moon/core.py index 97d18ca5..1b07dfd1 100644 --- a/keystone-moon/keystone/contrib/moon/core.py +++ b/keystone-moon/keystone/contrib/moon/core.py @@ -584,7 +584,7 @@ class IntraExtensionManager(manager.Manager): decision = one_true(decision_buffer) if not decision: raise AuthzException("{} {}-{}-{}".format(intra_extension_id, subject_id, action_id, object_id)) - return decision + return {'authz': decision, 'comment': ''} @enforce("read", "intra_extensions") def get_intra_extensions_dict(self, user_id): @@ -1808,7 +1808,7 @@ class IntraExtensionAuthzManager(IntraExtensionManager): def __init__(self): super(IntraExtensionAuthzManager, self).__init__() - def authz(self, tenant_name, subject_name, object_name, action_name, genre="authz"): + def authz(self, tenant_id, subject_k_id, object_name, action_name, genre="authz"): """Check authorization for a particular action. :return: True or False or raise an exception """ @@ -1818,27 +1818,20 @@ class IntraExtensionAuthzManager(IntraExtensionManager): genre = "intra_admin_extension_id" tenants_dict = self.tenant_api.get_tenants_dict(self.root_api.get_root_admin_id()) - tenant_id = None - for _tenant_id in tenants_dict: - if tenants_dict[_tenant_id]["name"] == tenant_name: - tenant_id = _tenant_id - break - if not tenant_id: - raise TenantUnknown + + if tenant_id not in tenants_dict: + raise TenantUnknown() intra_extension_id = tenants_dict[tenant_id][genre] if not intra_extension_id: raise TenantNoIntraExtension() - subjects_dict = self.driver.get_subjects_dict(intra_extension_id) subject_id = None for _subject_id in subjects_dict: - if subjects_dict[_subject_id]['keystone_name'] == subject_name: - # subject_id = subjects_dict[_subject_id]['keystone_id'] + if subjects_dict[_subject_id]['keystone_id'] == subject_k_id: subject_id = _subject_id break if not subject_id: raise SubjectUnknown() - objects_dict = self.driver.get_objects_dict(intra_extension_id) object_id = None for _object_id in objects_dict: diff --git a/keystone-moon/keystone/contrib/moon/routers.py b/keystone-moon/keystone/contrib/moon/routers.py index 4da672cf..357ae060 100644 --- a/keystone-moon/keystone/contrib/moon/routers.py +++ b/keystone-moon/keystone/contrib/moon/routers.py @@ -76,12 +76,12 @@ class Routers(wsgi.V3ExtensionRouter): # Authz route self._add_resource( mapper, authz_controller, - path=self.PATH_PREFIX+'/authz/{tenant_name}/{subject_name}/{object_name}/{action_name}', + path=self.PATH_PREFIX+'/authz/{tenant_id}/{subject_k_id}/{object_name}/{action_name}', get_action='get_authz', rel=self._get_rel('authz'), path_vars={ - 'tenant_name': self._get_path('tenants'), - 'subject_name': self._get_path('subjects'), + 'tenant_id': self._get_path('tenants'), + 'subject_k_id': self._get_path('subjects'), 'object_name': self._get_path('objects'), 'action_name': self._get_path('actions'), }) diff --git a/keystone-moon/keystone/tests/moon/unit/test_unit_core_intra_extension_admin.py b/keystone-moon/keystone/tests/moon/unit/test_unit_core_intra_extension_admin.py index c97776d3..f92d1e3b 100644 --- a/keystone-moon/keystone/tests/moon/unit/test_unit_core_intra_extension_admin.py +++ b/keystone-moon/keystone/tests/moon/unit/test_unit_core_intra_extension_admin.py @@ -598,13 +598,10 @@ class TestIntraExtensionAdminManagerOK(tests.TestCase): objects_dict = self.authz_manager.get_objects_dict(admin_subject_id, authz_ie_dict["id"]) - object_vm1_id = None - object_vm2_id = None - for _object_id in objects_dict: - if objects_dict[_object_id]['name'] == 'vm1': - object_vm1_id = _object_id - if objects_dict[_object_id]['name'] == 'vm2': - object_vm2_id = _object_id + object_vm1 = self.admin_manager.add_object_dict(admin_subject_id, authz_ie_dict["id"], {"name": "vm1", "description": "vm1"}) + object_vm2 = self.admin_manager.add_object_dict(admin_subject_id, authz_ie_dict["id"], {"name": "vm2", "description": "vm2"}) + object_vm1_id = object_vm1.keys()[0] + object_vm2_id = object_vm2.keys()[0] if not object_vm1_id or not object_vm2_id: raise Exception("Cannot run tests, database is corrupted ? (need upload and list in objects)") @@ -1690,13 +1687,10 @@ class TestIntraExtensionAdminManagerKO(tests.TestCase): objects_dict = self.authz_manager.get_objects_dict(admin_subject_id, authz_ie_dict["id"]) - object_vm1_id = None - object_vm2_id = None - for _object_id in objects_dict: - if objects_dict[_object_id]['name'] == 'vm1': - object_vm1_id = _object_id - if objects_dict[_object_id]['name'] == 'vm2': - object_vm2_id = _object_id + object_vm1 = self.admin_manager.add_object_dict(admin_subject_id, authz_ie_dict["id"], {"name": "vm1", "description": "vm1"}) + object_vm2 = self.admin_manager.add_object_dict(admin_subject_id, authz_ie_dict["id"], {"name": "vm2", "description": "vm2"}) + object_vm1_id = object_vm1.keys()[0] + object_vm2_id = object_vm2.keys()[0] if not object_vm1_id or not object_vm2_id: raise Exception("Cannot run tests, database is corrupted ? (need upload and list in objects)") diff --git a/keystone-moon/keystone/tests/moon/unit/test_unit_core_intra_extension_authz.py b/keystone-moon/keystone/tests/moon/unit/test_unit_core_intra_extension_authz.py index 8efa4ab8..ff7010fe 100644 --- a/keystone-moon/keystone/tests/moon/unit/test_unit_core_intra_extension_authz.py +++ b/keystone-moon/keystone/tests/moon/unit/test_unit_core_intra_extension_authz.py @@ -586,13 +586,10 @@ class TestIntraExtensionAuthzManagerAuthzOK(tests.TestCase): objects_dict = self.authz_manager.get_objects_dict(admin_subject_id, authz_ie_dict["id"]) - object_vm1_id = None - object_vm2_id = None - for _object_id in objects_dict: - if objects_dict[_object_id]['name'] == 'vm1': - object_vm1_id = _object_id - if objects_dict[_object_id]['name'] == 'vm2': - object_vm2_id = _object_id + object_vm1 = self.admin_manager.add_object_dict(admin_subject_id, authz_ie_dict["id"], {"name": "vm1", "description": "vm1"}) + object_vm2 = self.admin_manager.add_object_dict(admin_subject_id, authz_ie_dict["id"], {"name": "vm2", "description": "vm2"}) + object_vm1_id = object_vm1.keys()[0] + object_vm2_id = object_vm2.keys()[0] if not object_vm1_id or not object_vm2_id: raise Exception("Cannot run tests, database is corrupted ? (need upload and list in objects)") @@ -1021,7 +1018,7 @@ class TestIntraExtensionAuthzManagerAuthzKO(tests.TestCase): self.assertRaises( SubjectUnknown, self.authz_manager.authz, - tenant["name"], uuid.uuid4().hex, uuid.uuid4().hex, uuid.uuid4().hex + tenant["id"], uuid.uuid4().hex, uuid.uuid4().hex, uuid.uuid4().hex ) # Test when subject is known but not the object @@ -1037,7 +1034,7 @@ class TestIntraExtensionAuthzManagerAuthzKO(tests.TestCase): self.assertRaises( ObjectUnknown, self.authz_manager.authz, - tenant["name"], demo_subject_dict["name"], uuid.uuid4().hex, uuid.uuid4().hex + tenant["id"], demo_subject_dict["keystone_id"], uuid.uuid4().hex, uuid.uuid4().hex ) # Test when subject and object are known but not the action @@ -1052,7 +1049,7 @@ class TestIntraExtensionAuthzManagerAuthzKO(tests.TestCase): self.assertRaises( ActionUnknown, self.authz_manager.authz, - tenant["name"], demo_subject_dict["name"], my_object["name"], uuid.uuid4().hex + tenant["id"], demo_subject_dict["keystone_id"], my_object["name"], uuid.uuid4().hex ) # Test when subject and object and action are known @@ -1067,7 +1064,7 @@ class TestIntraExtensionAuthzManagerAuthzKO(tests.TestCase): self.assertRaises( AuthzException, self.authz_manager.authz, - tenant["name"], demo_subject_dict["name"], my_object["name"], my_action["name"] + tenant["id"], demo_subject_dict["keystone_id"], my_object["name"], my_action["name"] ) # Add a subject scope and test ObjectCategoryAssignmentOutOfScope @@ -1091,7 +1088,7 @@ class TestIntraExtensionAuthzManagerAuthzKO(tests.TestCase): self.assertRaises( AuthzException, self.authz_manager.authz, - tenant["name"], demo_subject_dict["name"], my_object["name"], my_action["name"] + tenant["id"], demo_subject_dict["keystone_id"], my_object["name"], my_action["name"] ) # Add an object scope and test ActionCategoryAssignmentOutOfScope @@ -1115,7 +1112,7 @@ class TestIntraExtensionAuthzManagerAuthzKO(tests.TestCase): self.assertRaises( AuthzException, self.authz_manager.authz, - tenant["name"], demo_subject_dict["name"], my_object["name"], my_action["name"] + tenant["id"], demo_subject_dict["keystone_id"], my_object["name"], my_action["name"] ) # Add an action scope and test SubjectCategoryAssignmentUnknown @@ -1139,7 +1136,7 @@ class TestIntraExtensionAuthzManagerAuthzKO(tests.TestCase): self.assertRaises( AuthzException, self.authz_manager.authz, - tenant["name"], demo_subject_dict["name"], my_object["name"], my_action["name"] + tenant["id"], demo_subject_dict["keystone_id"], my_object["name"], my_action["name"] ) # Add a subject assignment and test ObjectCategoryAssignmentUnknown @@ -1154,7 +1151,7 @@ class TestIntraExtensionAuthzManagerAuthzKO(tests.TestCase): self.assertRaises( AuthzException, self.authz_manager.authz, - tenant["name"], demo_subject_dict["name"], my_object["name"], my_action["name"] + tenant["id"], demo_subject_dict["keystone_id"], my_object["name"], my_action["name"] ) # Add an object assignment and test ActionCategoryAssignmentUnknown @@ -1169,7 +1166,7 @@ class TestIntraExtensionAuthzManagerAuthzKO(tests.TestCase): self.assertRaises( AuthzException, self.authz_manager.authz, - tenant["name"], demo_subject_dict["name"], my_object["name"], my_action["name"] + tenant["id"], demo_subject_dict["keystone_id"], my_object["name"], my_action["name"] ) # Add an action assignment and test RuleUnknown @@ -1184,7 +1181,7 @@ class TestIntraExtensionAuthzManagerAuthzKO(tests.TestCase): self.assertRaises( AuthzException, self.authz_manager.authz, - tenant["name"], admin_subject_dict["name"], my_object["name"], my_action["name"] + tenant["id"], admin_subject_dict["keystone_id"], my_object["name"], my_action["name"] ) # Add the correct rule and test that no exception is raised @@ -1200,7 +1197,6 @@ class TestIntraExtensionAuthzManagerAuthzKO(tests.TestCase): authz_ie_dict["id"] ) - print("authz_ie_dict[\"id\"]", authz_ie_dict["id"]) self.assertRaises( SubMetaRuleAlgorithmNotExisting, self.admin_manager.add_sub_meta_rule_dict, @@ -1243,11 +1239,13 @@ class TestIntraExtensionAuthzManagerAuthzKO(tests.TestCase): self.assertRaises( AuthzException, self.authz_manager.authz, - tenant["name"], admin_subject_dict["name"], my_object["name"], my_action["name"] + tenant["id"], admin_subject_dict["keystone_id"], my_object["name"], my_action["name"] ) - result = self.authz_manager.authz(tenant["name"], demo_subject_dict["name"], my_object["name"], my_action["name"]) - self.assertEqual(True, result) + result = self.authz_manager.authz(tenant["id"], demo_subject_dict["keystone_id"], my_object["name"], my_action["name"]) + self.assertIsInstance(result, dict) + self.assertIn('authz', result) + self.assertEquals(result['authz'], True) def test_subjects(self): authz_ie_dict = create_intra_extension(self, "policy_authz") @@ -1916,13 +1914,10 @@ class TestIntraExtensionAuthzManagerAuthzKO(tests.TestCase): objects_dict = self.authz_manager.get_objects_dict(admin_subject_id, authz_ie_dict["id"]) - object_vm1_id = None - object_vm2_id = None - for _object_id in objects_dict: - if objects_dict[_object_id]['name'] == 'vm1': - object_vm1_id = _object_id - if objects_dict[_object_id]['name'] == 'vm2': - object_vm2_id = _object_id + object_vm1 = self.admin_manager.add_object_dict(admin_subject_id, authz_ie_dict["id"], {"name": "vm1", "description": "vm1"}) + object_vm2 = self.admin_manager.add_object_dict(admin_subject_id, authz_ie_dict["id"], {"name": "vm2", "description": "vm2"}) + object_vm1_id = object_vm1.keys()[0] + object_vm2_id = object_vm2.keys()[0] if not object_vm1_id or not object_vm2_id: raise Exception("Cannot run tests, database is corrupted ? (need upload and list in objects)") -- cgit 1.2.3-korg