Review the KeystoneMiddleware code, fix some bugs in the authz functions.

Change-Id: I9d9966c061fc71cd8ef5ce88217dcdfa63e0722f

9 files changed, 75 insertions, 101 deletions

diff --git a/keystone-moon/examples/moon/policies/policy_authz/assignment.json b/keystone-moon/examples/moon/policies/policy_authz/assignment.json
index 6482830c..7a6c722e 100644
--- a/keystone-moon/examples/moon/policies/policy_authz/assignment.json
+++ b/keystone-moon/examples/moon/policies/policy_authz/assignment.json
@@ -43,25 +43,13 @@
 
     "object_assignments": {
         "object_security_level": {
-            "servers": ["low"],
-			"vm1": ["low"],
-			"vm2": ["medium"],
-			"file1": ["low"],
-			"file2": ["medium"]
+            "servers": ["low"]
         },
 		"type": {
-			"servers": ["computing"],
-			"vm1": ["computing"],
-			"vm2": ["computing"],
-			"file1": ["storage"],
-			"file2": ["storage"]
+			"servers": ["computing"]
 		},
 		"object_id": {
-			"servers": ["servers"],
-			"vm1": ["vm1"],
-			"vm2": ["vm2"],
-			"file1": ["file1"],
-			"file2": ["file2"]
+			"servers": ["servers"]
 		}
     }
 }
diff --git a/keystone-moon/examples/moon/policies/policy_authz/rule.json b/keystone-moon/examples/moon/policies/policy_authz/rule.json
index 73e791d7..25f9d93a 100644
--- a/keystone-moon/examples/moon/policies/policy_authz/rule.json
+++ b/keystone-moon/examples/moon/policies/policy_authz/rule.json
@@ -19,23 +19,7 @@
 	],
 	"rbac_rule":[
 		["dev", "xx", "read", "servers"],
-		["dev", "xx", "read", "vm1"],
-		["dev", "xx", "read", "vm2"],
-		["dev", "xx", "read", "file1"],
-		["dev", "xx", "read", "file2"],
-		["dev", "xx", "write", "vm1"],
-		["dev", "xx", "write", "vm2"],
-		["dev", "xx", "write", "file1"],
-		["dev", "xx", "write", "file2"],
 		["admin", "xx", "read", "servers"],
-		["admin", "ft", "read", "servers"],
-		["admin", "ft", "read", "vm1"],
-		["admin", "ft", "read", "vm2"],
-		["admin", "ft", "read", "file1"],
-		["admin", "ft", "read", "file2"],
-		["admin", "ft", "write", "vm1"],
-		["admin", "ft", "write", "vm2"],
-		["admin", "ft", "write", "file1"],
-		["admin", "ft", "write", "file2"]
+		["admin", "ft", "read", "servers"]
 	]
 }
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)")
 
diff --git a/keystonemiddleware-moon/keystonemiddleware/authz.py b/keystonemiddleware-moon/keystonemiddleware/authz.py
index bdb31ade..a24ac897 100644
--- a/keystonemiddleware-moon/keystonemiddleware/authz.py
+++ b/keystonemiddleware-moon/keystonemiddleware/authz.py
@@ -245,6 +245,7 @@ class AuthZProtocol(object):
         :return: the action or ""
         """
         action = ""
+        self.input = ""
         if component == "nova":
             length = int(env.get('CONTENT_LENGTH', '0'))
             # TODO (dthom): compute for Nova, Cinder, Neutron, ...
@@ -252,6 +253,7 @@ class AuthZProtocol(object):
             if length > 0:
                 try:
                     sub_action_object = env['wsgi.input'].read(length)
+                    self.input = sub_action_object
                     action = json.loads(sub_action_object).keys()[0]
                     body = StringIO(sub_action_object)
                     env['wsgi.input'] = body
@@ -272,8 +274,23 @@ class AuthZProtocol(object):
     @staticmethod
     def _get_object(env, component):
         if component == "nova":
-            # get the object ID which is located before "action" in the URL
-            return env.get("PATH_INFO").split("/")[-2]
+            # http://developer.openstack.org/api-ref-compute-v2.1.html
+            # nova URLs:
+            #    /<tenant_id>/servers/<server_id>
+            #       list details for server_id
+            #    /<tenant_id>/servers/<server_id>/action
+            #       execute action to server_id
+            #    /<tenant_id>/servers/<server_id>/metadata
+            #       show metadata from server_id
+            #    /<tenant_id>/servers/details
+            #       list servers
+            url = env.get("PATH_INFO").split("/")
+            if url[-1] == "detail":
+                return "servers"
+            try:
+                return url[3]
+            except IndexError:
+                return
         elif component == "swift":
             # remove the "/v1/" part of the URL
             return env.get("PATH_INFO").split("/", 2)[-1].replace("/", "-")
@@ -293,12 +310,11 @@ class AuthZProtocol(object):
         component = self._find_openstack_component(env)
         action_id = self._get_action(env, component)
         if action_id:
-            self._LOG.debug("OpenStack component {}".format(component))
             object_id = self._get_object(env, component)
-            self._LOG.debug("{}-{}-{}-{}".format(subject_id, object_id, action_id, tenant_id))
+            if not object_id:
+                object_id = "servers"
             self.__set_token()
             resp = self._get_authz_from_moon(self.x_subject_token, tenant_id, subject_id, object_id, action_id)
-            self._LOG.info("Moon answer: {}-{}".format(resp.status_code, resp.content))
             self.__unset_token()
             if resp.status_code == 200:
                 try:
@@ -306,8 +322,9 @@ class AuthZProtocol(object):
                     self._LOG.debug(answer)
                     if "authz" in answer and answer["authz"]:
                         return self._app(env, start_response)
-                except:
-                    raise exception.Unauthorized(message="You are not authorized to do that!")
+                except Exception as e:
+                    # self._LOG.error("You are not authorized to do that!")
+                    raise exception.Unauthorized(message="You are not authorized to do that! ({})".format(unicode(e)))
         self._LOG.debug("No action_id found for {}".format(env.get("PATH_INFO")))
         # If action is not found, we can't raise an exception because a lots of action is missing
         # in function self._get_action, it is not possible to get them all.