Added the _check_role method to the Brain, making role matching explicit. is_admin causes user to have all roles.

etc/nova/policy.json

@@ -1,8 +1,11 @@
 {
-    'compute:get_volume': ('role:compute_admin', ('tenant_id:%(tenant_id)s', 'role:compute_sysadmin')), 
-    'compute:get_instance': ('role:compute_admin', ('tenant_id:%(tenant_id)s', 'role:compute_sysadmin')),
-    'example:get_google': (('http:http://www.google.com',), ('role:compute_sysadmin',)),
-    'example:my_file': ('role:compute_admin', ('tenant_id:%(tenant_id)s',))
-    'example:allowed' : (),
-    'example:denied' : ('false:false',),
-}
+    "true" : [],
+    "compute:create_instance" :          [["role:admin"], ["project_id:%(project_id)s"]],
+    "compute:attach_network" :  [["role:admin"], ["project_id:%(project_id)s"]],
+    "compute:attach_volume" :   [["role:admin"], ["project_id:%(project_id)s"]],
+    "compute:list_instances":   [["role:admin"], ["project_id:%(project_id)s"]],
+    "compute:get_instance":     [["role:admin"], ["project_id:%(project_id)s"]],
+    "network:attach_network" :  [["role:admin"], ["project_id:%(project_id)s"]],
+    "volume:create_volume":     [["role:admin"], ["project_id:%(project_id)s"]],
+    "volume:attach_volume":     [["role:admin"], ["project_id:%(project_id)s"]]
+}

etc/nova/roles.yaml

@@ -0,0 +1,7 @@
+roles:
+- 'netadmin'
+- 'sysadmin'
+- 'admin'
+- 'member'
+- 'keystoneadmin'
+- 'keystoneserviceadmin'

nova/common/policy.py

@@ -22,7 +22,7 @@
 import urllib2
 
 
-def NotAllowed(Exception):
+class NotAllowed(Exception):
         pass
 
 
@@ -46,7 +46,7 @@ def enforce(match_list, target_dict, credentials_dict):
     performing the action.
 
     """
-    b = Brain()
+    b = HttpBrain()
     if not b.check(match_list, target_dict, credentials_dict):
         raise NotAllowed()
 
@@ -63,18 +63,22 @@ def add_rule(self, key, match):
         self.rules[key] = match
 
     def check(self, match_list, target_dict, cred_dict):
+        if not match_list:
+            return True
         for and_list in match_list:
             matched = False
+            if isinstance(and_list, basestring):
+                and_list = (and_list,)
             for match in and_list:
-                match_kind, match = match.split(':', 2)
+                match_kind, match_value = match.split(':', 1)
                 if hasattr(self, '_check_%s' % match_kind):
                     f = getattr(self, '_check_%s' % match_kind)
-                    rv = f(match, target_dict, cred_dict)
+                    rv = f(match_value, target_dict, cred_dict)
                     if not rv:
                         matched = False
                         break
                 else:
-                    rv = self._check(match, target_dict, cred_dict)
+                    rv = self._check_generic(match, target_dict, cred_dict)
                     if not rv:
                         matched = False
                         break
@@ -88,9 +92,14 @@ def check(self, match_list, target_dict, cred_dict):
         return False
 
     def _check_rule(self, match, target_dict, cred_dict):
-        new_match_list = self.rules.get(match[5:])
+        new_match_list = self.rules.get(match)
+        if new_match_list is None:
+            return False
         return self.check(new_match_list, target_dict, cred_dict)
 
+    def _check_role(self, match, target_dict, cred_dict):
+        return match in cred_dict['roles']
+
     def _check_generic(self, match, target_dict, cred_dict):
         """Check an individual match.
 
@@ -103,13 +112,13 @@ def _check_generic(self, match, target_dict, cred_dict):
 
         # TODO(termie): do dict inspection via dot syntax
         match = match % target_dict
-        key, value = match.split(':', 2)
+        key, value = match.split(':', 1)
         if key in cred_dict:
             return value == cred_dict[key]
         return False
 
 
-class HttpBrain(object):
+class HttpBrain(Brain):
     """A brain that can check external urls a
 
     Posts json blobs for target and credentials.
@@ -119,14 +128,17 @@ class HttpBrain(object):
     def _check_http(self, match, target_dict, cred_dict):
         url = match % target_dict
         data = {'target': json.dumps(target_dict),
-                        'credentials': json.dumps(cred_dict)}
+                'credentials': json.dumps(cred_dict)}
         post_data = urllib.urlencode(data)
         f = urllib2.urlopen(url, post_data)
-        if f.read():
+        # NOTE(vish): This is to show how we could do remote requests,
+        #             but some fancier method for response codes should
+        #             probably be defined
+        if f.read() == "True":
             return True
         return False
 
 
 def load_json(path):
     rules_dict = json.load(open(path))
-    b = Brain(rules=rules_dict)
+    b = HttpBrain(rules=rules_dict)

nova/compute/api.py

@@ -29,6 +29,7 @@
 import nova.image
 from nova import log as logging
 from nova import network
+from nova import policy
 from nova import quota
 from nova import rpc
 from nova import utils
@@ -542,6 +543,20 @@ def create(self, context, instance_type,
         could be 'None' or a list of instance dicts depending on if
         we waited for information from the scheduler or not.
         """
+        target = {'project_id' : context.project_id,
+                  'user_id' : context.user_id,
+                  'availability_zone' : availability_zone}
+        policy.enforce(context, "compute:create_instance", target)
+
+        if requested_networks:
+            target['requested_networks'] = requested_networks
+            policy.enforce(context, "compute:attach_network", target)
+            policy.enforce(context, "network:attach_network", target)
+
+        if block_device_mapping:
+            target['block_device'] = block_device_mapping
+            policy.enforce(context, "compute:attach_volume", target)
+            policy.enforce(context, "volume:attach_volume", target)
 
         # We can create the DB entry for the instance here if we're
         # only going to create 1 instance and we're in a single

nova/policy.py

@@ -17,6 +17,7 @@
 
 """Policy Engine For Nova"""
 
+import os
 
 from nova import exception
 from nova import flags
@@ -27,6 +28,18 @@
 flags.DEFINE_string('policy_file', 'policy.json',
                     _('JSON file representing policy'))
 
+_POLICY_PATH = None
+_POLICY_MTIME = None
+
+def _load_if_modified(path):
+    global _POLICY_MTIME
+    mtime = os.path.getmtime(path)
+    if mtime != _POLICY_MTIME:
+        policy.load_json(path)
+        _POLICY_MTIME = mtime
+
+
+
 def enforce(context, action, target):
     """Verifies that the action is valid on the target in this context.
 
@@ -44,14 +57,14 @@ def enforce(context, action, target):
        :raises: `nova.exception.PolicyNotAllowed` if verification fails.
 
     """
-    if not policy.Brain.rules:
-        #TODO(vish): check mtime and reload
-        path = utils.find_config(FLAGS.policy_file)
-        policy.load_json(path)
+    global _POLICY_PATH
 
+    if not _POLICY_PATH:
+        _POLICY_PATH = utils.find_config(FLAGS.policy_file)
+    _load_if_modified(_POLICY_PATH)
     match_list = ('rule:%s' % action,)
     target_dict = target
-    credentials_dict = context
+    credentials_dict = context.to_dict()
     try:
         policy.enforce(match_list, target_dict, credentials_dict)
     except policy.NotAllowed:

nova/tests/test_compute.py

@@ -108,7 +108,7 @@ def setUp(self):
         self.compute = utils.import_object(FLAGS.compute_manager)
         self.user_id = 'fake'
         self.project_id = 'fake'
-        self.context = context.RequestContext(self.user_id, self.project_id)
+        self.context = context.RequestContext(self.user_id, self.project_id, roles=['member'])
         test_notifier.NOTIFICATIONS = []
 
         def fake_show(meh, context, id):
@@ -674,7 +674,7 @@ def test_lock(self):
         instance_uuid = instance['uuid']
         self.compute.run_instance(self.context, instance_uuid)
 
-        non_admin_context = context.RequestContext(None, None, is_admin=False)
+        non_admin_context = context.RequestContext(None, None, roles=['member'], is_admin=False)
 
         # decorator should return False (fail) with locked nonadmin context
         self.compute.lock_instance(self.context, instance_uuid)

nova/tests/test_policy.py

@@ -17,38 +17,101 @@
 
 """Test of Policy Engine For Nova"""
 
-from nova import test
-from nova import policy
+import urllib2
+import StringIO
+
+from nova import context
 from nova import exception
+from nova import flags
+from nova import policy
+from nova import test
+from nova import utils
+from nova.common import policy as common_policy
+
+FLAGS = flags.FLAGS
+
+
+class PolicyFileTestCase(test.TestCase):
+    def setUp(self):
+        self.flags(policy_file='nova/tests/policy.json')
+
+
+
+class PolicyTestCase(test.TestCase):
+    def setUp(self):
+        super(PolicyTestCase, self).setUp()
+        # NOTE(vish): preload rules to circumvent reloading from file
+        policy._load_if_modified(utils.find_config(FLAGS.policy_file))
+        common_policy.Brain.rules = None
+        rules = {
+            "true" : [],
+            "example:allowed" : [],
+            "example:denied" : [["false:false"]],
+            "example:get_http": [["http:http://www.example.com"]],
+            "example:my_file": [["role:compute_admin"],
+                                ["project_id:%(project_id)s"]],
+            "example:early_and_fail" : [["false:false", "rule:true"]],
+            "example:early_or_success" : [["rule:true"], ["false:false"]],
+            "example:sysadmin_allowed" : [["role:admin"], ["role:sysadmin"]],
+        }
+        common_policy.HttpBrain(rules)
+        self.context = context.RequestContext('fake', 'fake', roles=['member'])
+        self.admin_context = context.RequestContext('admin', 'fake', roles=['admin'], is_admin=True)
+        self.target = {}
+
+    def test_admin_has_all_roles(self):
+        action = "example:sysadmin_allowed"
+        self.assertRaises(exception.PolicyNotAllowed, policy.enforce,
+                          self.context, action, self.target)
+        policy.enforce(self.admin_context, action, self.target)
 
+    def test_enforce_nonexistent_action_throws(self):
+        action = "example:noexist"
+        self.assertRaises(exception.PolicyNotAllowed, policy.enforce,
+                          self.context, action, self.target)
 
-class PolicyCheckTestCase(test.TestCase):
-
     def test_enforce_bad_action_throws(self):
-        context = {}
         action = "example:denied"
-        target = {}
-        self.assertRaises(exception.PolicyNotAllowed, policy.enforce, context, action, target)    
-        
+        self.assertRaises(exception.PolicyNotAllowed, policy.enforce,
+                          self.context, action, self.target)
+
     def test_enforce_good_action(self):
-        context = {}
         action = "example:allowed"
+        policy.enforce(self.context, action, self.target)
+
+    def test_enforce_http_true(self):
+
+        def fakeurlopen(url, post_data):
+            return StringIO.StringIO("True")
+        self.stubs.Set(urllib2, 'urlopen', fakeurlopen)
+        action = "example:get_http"
         target = {}
-        result = policy.enforce(context, action, target)
+        result = policy.enforce(self.context, action, target)
         self.assertEqual(result, None)
-
-    def test_enforce_http_check(self):
-        action = "example:get_google"
-        context = {}
+
+    def test_enforce_http_false(self):
+
+        def fakeurlopen(url, post_data):
+            return StringIO.StringIO("False")
+        self.stubs.Set(urllib2, 'urlopen', fakeurlopen)
+        action = "example:get_http"
         target = {}
-        result = policy.enforce(context, action, target)
-        self.assertEqual(result, None)
-    
+        self.assertRaises(exception.PolicyNotAllowed, policy.enforce,
+                          self.context, action, target)
+
     def test_templatized_enforcement(self):
-        context = {'tenant_id' : 'bob'}
-        target_mine = {'tenant_id' : 'bob'}
-        target_not_mine = {'tenant_id' : 'fred'}
+        target_mine = {'project_id' : 'fake'}
+        target_not_mine = {'project_id' : 'another'}
         action = "example:my_file"
-        result = policy.enforce(context, action, target_mine)
-        self.assertEqual(result, None)
-        self.assertRaises(exception.PolicyNotAllowed, policy.enforce, context, action, target_not_mine)
+        policy.enforce(self.context, action, target_mine)
+        self.assertRaises(exception.PolicyNotAllowed, policy.enforce,
+                          self.context, action, target_not_mine)
+
+    def test_early_AND_enforcement(self):
+        action = "example:early_and_fail"
+        self.assertRaises(exception.PolicyNotAllowed, policy.enforce,
+                          self.context, action, self.target)
+
+    def test_early_OR_enforcement(self):
+        action = "example:early_or_success"
+        policy.enforce(self.context, action, self.target)