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,24 @@ 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:
+            for network in requested_networks:
+                # TODO(JMC): I realize this doesn't work for quantum nets yet...
+                (net_id, _i) = network
+                network_obj = self.network_api.get(context, net_id)
+                policy.enforce(context, "compute:attach_network", network_obj)
+                policy.enforce(context, "network:attach_network", network_obj)
+
+        if block_device_mapping:
+            for bdm in block_device_mapping:
+                volume_obj = self.volume_api.get(context, bdm['volume_id'])
+                policy.enforce(context, "compute:attach_volume", volume_obj)
+                policy.enforce(context, "volume:attach_volume", volume_obj)
 
         # 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/network/api.py

@@ -33,6 +33,14 @@
 class API(base.Base):
     """API for interacting with the network manager."""
 
+    def get(self, context, network_id):
+        """ Returns a network db model.
+            Does *not* work for quantum defined nets.
+            TODO(JMC)
+        """
+        rv = self.db.network_get_by_uuid(context, network_id) 
+        return dict(rv.iteritems())
+
     def get_floating_ip(self, context, id):
         return rpc.call(context,
                         FLAGS.network_topic,

nova/policy.py

@@ -17,6 +17,7 @@
 
 """Policy Engine For Nova"""
 
+import os
 
 from nova import exception
 from nova import flags
@@ -27,6 +28,25 @@
 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 reset():
+    global _POLICY_PATH
+    global _POLICY_MTIME
+    _POLICY_PATH = None
+    _POLICY_MTIME = None
+    policy.Brain.rules = {}
+
+
 def enforce(context, action, target):
     """Verifies that the action is valid on the target in this context.
 
@@ -44,14 +64,13 @@ 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/fake_flags.py

@@ -41,3 +41,4 @@
 FLAGS['use_ipv6'].SetDefault(True)
 FLAGS['flat_network_bridge'].SetDefault('br100')
 FLAGS['sqlite_synchronous'].SetDefault(False)
+FLAGS['policy_file'].SetDefault('nova/tests/policy.json')

nova/tests/policy.json

@@ -0,0 +1,11 @@
+{
+    "true" : [],
+    "compute:create_instance" :          [],
+    "compute:attach_network" :  [],
+    "compute:attach_volume" :   [],
+    "compute:list_instances":   [],
+    "compute:get_instance":     [],
+    "network:attach_network" :  [],
+    "volume:create_volume":     [],
+    "volume:attach_volume":     []
+}

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)