Remove db layer hard-code permission checks for quota_class_create/update

This patch removes db layer hard-code permission checks for
quota_class_create/update.

For v2 API, this patch adds back-comptiable permission checks at REST
API layer.

For v2.1 API, this patch adds new rule for update method.

Partially implements bp nova-api-policy-final-part

SecurityImpact
UpgradeImpact: Due to the db layer permission checks deleted, the policy
rule "os_compute_api:os-quota-class-sets:update" was updated with
a default that match the permission as before. Admin should be notified
to update their policy configuration to keep permission as before.

Change-Id: Icddc7e5cc1c11ab3d272f61a2ef623d3f750030c

etc/nova/policy.json

@@ -337,7 +337,7 @@
     "os_compute_api:os-quota-sets:update": "rule:admin_api",
     "os_compute_api:os-quota-sets:delete": "rule:admin_api",
     "os_compute_api:os-quota-sets:detail": "rule:admin_api",
-    "os_compute_api:os-quota-class-sets": "",
+    "os_compute_api:os-quota-class-sets:update": "rule:admin_api",
     "os_compute_api:os-quota-class-sets:show": "is_admin:True or quota_class:%(quota_class)s",
     "os_compute_api:os-quota-class-sets:discoverable": "",
     "os_compute_api:os-rescue": "",

nova/api/openstack/compute/contrib/quota_classes.py

@@ -94,13 +94,20 @@ class QuotaClassSetsController(wsgi.Controller):
             msg = _("Bad key(s) %s in quota_set") % ",".join(bad_keys)
             raise webob.exc.HTTPBadRequest(explanation=msg)
 
+        try:
+            # NOTE(alex_xu): back-compatible with db layer hard-code admin
+            # permission checks. This has to be left only for API v2.0 because
+            # this version has to be stable even if it means that only admins
+            # can call this method while the policy could be changed.
+            nova.context.require_admin_context(context)
+        except exception.AdminRequired:
+            raise webob.exc.HTTPForbidden()
+
         for key, value in quota_class_set.items():
             try:
                 db.quota_class_update(context, quota_class, key, value)
             except exception.QuotaClassNotFound:
                 db.quota_class_create(context, quota_class, key, value)
-            except exception.AdminRequired:
-                raise webob.exc.HTTPForbidden()
 
         values = QUOTAS.get_class_quotas(context, quota_class)
         return self._format_quota_set(None, values)

nova/api/openstack/compute/plugins/v3/quota_classes.py

@@ -14,7 +14,6 @@
 #    under the License.
 
 import six
-import webob
 
 from nova.api.openstack.compute.schemas.v3 import quota_classes
 from nova.api.openstack import extensions
@@ -68,11 +67,11 @@ class QuotaClassSetsController(wsgi.Controller):
         values = QUOTAS.get_class_quotas(context, id)
         return self._format_quota_set(id, values)
 
-    @extensions.expected_errors((403))
+    @extensions.expected_errors(())
     @validation.schema(quota_classes.update)
     def update(self, req, id, body):
         context = req.environ['nova.context']
-        authorize(context)
+        authorize(context, action='update', target={'quota_class': id})
         quota_class = id
 
         for key, value in six.iteritems(body['quota_class_set']):
@@ -80,8 +79,6 @@ class QuotaClassSetsController(wsgi.Controller):
                 db.quota_class_update(context, quota_class, key, value)
             except exception.QuotaClassNotFound:
                 db.quota_class_create(context, quota_class, key, value)
-            except exception.AdminRequired:
-                raise webob.exc.HTTPForbidden()
 
         values = QUOTAS.get_class_quotas(context, quota_class)
         return self._format_quota_set(None, values)

nova/db/sqlalchemy/api.py

@@ -3166,7 +3166,6 @@ def quota_class_get_all_by_name(context, class_name):
     return result
 
 
-@require_admin_context
 def quota_class_create(context, class_name, resource, limit):
     quota_class_ref = models.QuotaClass()
     quota_class_ref.class_name = class_name
@@ -3176,7 +3175,6 @@ def quota_class_create(context, class_name, resource, limit):
     return quota_class_ref
 
 
-@require_admin_context
 def quota_class_update(context, class_name, resource, limit):
     result = model_query(context, models.QuotaClass, read_deleted="no").\
                      filter_by(class_name=class_name).\

nova/tests/unit/api/openstack/compute/contrib/test_quota_classes.py

@@ -105,20 +105,6 @@ class QuotaClassSetsTestV21(test.TestCase):
 
         self.assertEqual(res_dict, body)
 
-    def test_quotas_update_as_user(self):
-        body = {'quota_class_set': {'instances': 50, 'cores': 50,
-                                    'ram': 51200, 'floating_ips': 10,
-                                    'fixed_ips': -1, 'metadata_items': 128,
-                                    'injected_files': 5,
-                                    'injected_file_content_bytes': 10240,
-                                    'security_groups': 10,
-                                    'security_group_rules': 20,
-                                    'key_pairs': 100,
-                                    }}
-
-        self.assertRaises(webob.exc.HTTPForbidden, self.controller.update,
-                          self.req, 'test_class', body=body)
-
     def test_quotas_update_with_empty_body(self):
         body = {}
         self.assertRaises(self.validation_error, self.controller.update,
@@ -162,6 +148,11 @@ class QuotaClassSetsTestV2(QuotaClassSetsTestV21):
         self.assertRaises(webob.exc.HTTPForbidden, self.controller.show,
                           self.req, 'test_class')
 
+    def test_quotas_update_as_user(self):
+        body = {'quota_class_set': {}}
+        self.assertRaises(webob.exc.HTTPForbidden, self.controller.update,
+                          self.req, 'test_class', body=body)
+
 
 class QuotaClassesPolicyEnforcementV21(test.NoDBTestCase):
 
@@ -181,3 +172,14 @@ class QuotaClassesPolicyEnforcementV21(test.NoDBTestCase):
         self.assertEqual(
             "Policy doesn't allow %s to be performed." % rule_name,
             exc.format_message())
+
+    def test_update_policy_failed(self):
+        rule_name = "os_compute_api:os-quota-class-sets:update"
+        self.policy.set_rules({rule_name: "quota_class:non_fake"})
+        exc = self.assertRaises(
+            exception.PolicyNotAuthorized,
+            self.controller.update, self.req, fakes.FAKE_UUID,
+            body={'quota_class_set': {}})
+        self.assertEqual(
+            "Policy doesn't allow %s to be performed." % rule_name,
+            exc.format_message())

nova/tests/unit/fake_policy.py

@@ -299,7 +299,7 @@ policy_data = """
     "os_compute_api:os-quota-sets:detail": "",
     "os_compute_api:os-quota-sets:defaults": "",
     "compute_extension:quota_classes": "",
-    "os_compute_api:os-quota-class-sets": "",
+    "os_compute_api:os-quota-class-sets:update": "",
     "os_compute_api:os-quota-class-sets:show": "",
     "compute_extension:rescue": "",
     "os_compute_api:os-rescue": "",