Merge "Remove db layer hard-code permission checks for quota_get_all_*"

etc/nova/policy.json

@@ -328,7 +328,8 @@
     "os_compute_api:os-personality:discoverable": "",
     "os_compute_api:os-preserve-ephemeral-rebuild:discoverable": "",
     "os_compute_api:os-quota-sets:discoverable": "",
-    "os_compute_api:os-quota-sets:show": "",
+    "os_compute_api:os-quota-sets:show": "rule:admin_or_owner",
+    "os_compute_api:os-quota-sets:defaults": "",
     "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",

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

@@ -116,6 +116,15 @@ class QuotaSetsController(wsgi.Controller):
     def update(self, req, id, body):
         context = req.environ['nova.context']
         authorize_update(context)
+        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()
+
         project_id = id
 
         bad_keys = []
@@ -137,6 +146,9 @@ class QuotaSetsController(wsgi.Controller):
             user_id = params.get('user_id', [None])[0]
 
         try:
+            # NOTE(alex_xu): back-compatible with db layer hard-code admin
+            # permission checks.
+            nova.context.authorize_project_context(context, id)
             settable_quotas = QUOTAS.get_settable_quotas(context, project_id,
                                                          user_id=user_id)
         except exception.Forbidden:
@@ -199,8 +211,6 @@ class QuotaSetsController(wsgi.Controller):
             except exception.QuotaExists:
                 objects.Quotas.update_limit(context, project_id,
                                             key, value, user_id=user_id)
-            except exception.AdminRequired:
-                raise webob.exc.HTTPForbidden()
         values = self._get_quotas(context, id, user_id=user_id)
         return self._format_quota_set(None, values)
 

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

@@ -22,7 +22,6 @@ from nova.api.openstack.compute.schemas.v3 import quota_sets
 from nova.api.openstack import extensions
 from nova.api.openstack import wsgi
 from nova.api import validation
-import nova.context
 from nova import exception
 from nova.i18n import _
 from nova import objects
@@ -83,37 +82,29 @@ class QuotaSetsController(wsgi.Controller):
         else:
             return {k: v['limit'] for k, v in values.items()}
 
-    @extensions.expected_errors(403)
+    @extensions.expected_errors(())
     def show(self, req, id):
         context = req.environ['nova.context']
-        authorize(context, action='show')
+        authorize(context, action='show', target={'project_id': id})
         params = urlparse.parse_qs(req.environ.get('QUERY_STRING', ''))
         user_id = params.get('user_id', [None])[0]
-        try:
+        return self._format_quota_set(id,
-            nova.context.authorize_project_context(context, id)
+            self._get_quotas(context, id, user_id=user_id))
-            return self._format_quota_set(id,
-                    self._get_quotas(context, id, user_id=user_id))
-        except exception.Forbidden:
-            raise webob.exc.HTTPForbidden()
 
-    @extensions.expected_errors(403)
+    @extensions.expected_errors(())
     def detail(self, req, id):
         context = req.environ['nova.context']
-        authorize(context, action='detail')
+        authorize(context, action='detail', target={'project_id': id})
         user_id = req.GET.get('user_id', None)
-        try:
+        return self._format_quota_set(id, self._get_quotas(context, id,
-            nova.context.authorize_project_context(context, id)
+                                                           user_id=user_id,
-            return self._format_quota_set(id, self._get_quotas(context, id,
+                                                           usages=True))
-                                                               user_id=user_id,
-                                                               usages=True))
-        except exception.Forbidden:
-            raise webob.exc.HTTPForbidden()
 
-    @extensions.expected_errors((400, 403))
+    @extensions.expected_errors(400)
     @validation.schema(quota_sets.update)
     def update(self, req, id, body):
         context = req.environ['nova.context']
-        authorize(context, action='update')
+        authorize(context, action='update', target={'project_id': id})
         project_id = id
         params = urlparse.parse_qs(req.environ.get('QUERY_STRING', ''))
         user_id = params.get('user_id', [None])[0]
@@ -121,12 +112,8 @@ class QuotaSetsController(wsgi.Controller):
         quota_set = body['quota_set']
         force_update = strutils.bool_from_string(quota_set.get('force',
                                                                'False'))
-
+        settable_quotas = QUOTAS.get_settable_quotas(context, project_id,
-        try:
+                                                     user_id=user_id)
-            settable_quotas = QUOTAS.get_settable_quotas(context, project_id,
-                                                         user_id=user_id)
-        except exception.Forbidden:
-            raise webob.exc.HTTPForbidden()
 
         # NOTE(dims): Pass #1 - In this loop for quota_set.items(), we validate
         # min/max values and bail out if any of the items in the set is bad.
@@ -155,8 +142,6 @@ class QuotaSetsController(wsgi.Controller):
             except exception.QuotaExists:
                 objects.Quotas.update_limit(context, project_id,
                                             key, value, user_id=user_id)
-            except exception.AdminRequired:
-                raise webob.exc.HTTPForbidden()
         # Note(gmann): Removed 'id' from update's response to make it same
         # as V2. If needed it can be added with microversion.
         return self._format_quota_set(None, self._get_quotas(context, id,
@@ -165,7 +150,7 @@ class QuotaSetsController(wsgi.Controller):
     @extensions.expected_errors(())
     def defaults(self, req, id):
         context = req.environ['nova.context']
-        authorize(context, action='show')
+        authorize(context, action='defaults', target={'project_id': id})
         values = QUOTAS.get_defaults(context)
         return self._format_quota_set(id, values)
 

nova/db/sqlalchemy/api.py

@@ -3060,8 +3060,6 @@ def quota_get(context, project_id, resource, user_id=None):
 
 @require_context
 def quota_get_all_by_project_and_user(context, project_id, user_id):
-    nova.context.authorize_project_context(context, project_id)
-
     user_quotas = model_query(context, models.ProjectUserQuota,
                               (models.ProjectUserQuota.resource,
                                models.ProjectUserQuota.hard_limit)).\
@@ -3078,8 +3076,6 @@ def quota_get_all_by_project_and_user(context, project_id, user_id):
 
 @require_context
 def quota_get_all_by_project(context, project_id):
-    nova.context.authorize_project_context(context, project_id)
-
     rows = model_query(context, models.Quota, read_deleted="no").\
                    filter_by(project_id=project_id).\
                    all()
@@ -3093,8 +3089,6 @@ def quota_get_all_by_project(context, project_id):
 
 @require_context
 def quota_get_all(context, project_id):
-    nova.context.authorize_project_context(context, project_id)
-
     result = model_query(context, models.ProjectUserQuota).\
                    filter_by(project_id=project_id).\
                    all()
@@ -3102,7 +3096,6 @@ def quota_get_all(context, project_id):
     return result
 
 
-@require_admin_context
 def quota_create(context, project_id, resource, limit, user_id=None):
     per_user = user_id and resource not in PER_PROJECT_QUOTAS
     quota_ref = models.ProjectUserQuota() if per_user else models.Quota()
@@ -3118,7 +3111,6 @@ def quota_create(context, project_id, resource, limit, user_id=None):
     return quota_ref
 
 
-@require_admin_context
 def quota_update(context, project_id, resource, limit, user_id=None):
     per_user = user_id and resource not in PER_PROJECT_QUOTAS
     model = models.ProjectUserQuota if per_user else models.Quota
@@ -3224,7 +3216,6 @@ def quota_usage_get(context, project_id, resource, user_id=None):
 
 
 def _quota_usage_get_all(context, project_id, user_id=None):
-    nova.context.authorize_project_context(context, project_id)
     query = model_query(context, models.QuotaUsage, read_deleted="no").\
                    filter_by(project_id=project_id)
     result = {'project_id': project_id}

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

@@ -177,49 +177,38 @@ class QuotaSetsTestV21(BaseQuotaSetsTest):
 
         self.assertEqual(res_dict, expected)
 
-    def test_quotas_show_as_admin(self):
+    def test_quotas_show(self):
         self.setup_mock_for_show()
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234',
+        req = self._get_http_request()
-                                      use_admin_context=True)
         res_dict = self.controller.show(req, 1234)
 
         ref_quota_set = quota_set('1234', self.include_server_group_quotas)
         self.assertEqual(res_dict, ref_quota_set)
 
-    def test_quotas_show_as_unauthorized_user(self):
+    def test_quotas_update(self):
-        self.setup_mock_for_show()
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234')
-        self.assertRaises(webob.exc.HTTPForbidden, self.controller.show,
-                          req, 1234)
-
-    def test_quotas_update_as_admin(self):
         self.setup_mock_for_update()
         self.default_quotas.update({
             'instances': 50,
             'cores': 50
         })
         body = {'quota_set': self.default_quotas}
-
+        req = self._get_http_request()
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
-                                      use_admin_context=True)
         res_dict = self.controller.update(req, 'update_me', body=body)
         self.assertEqual(body, res_dict)
 
     @mock.patch('nova.objects.Quotas.create_limit')
-    def test_quotas_update_with_good_data_as_admin(self, mock_createlimit):
+    def test_quotas_update_with_good_data(self, mock_createlimit):
         self.setup_mock_for_update()
         self.default_quotas.update({})
         body = {'quota_set': self.default_quotas}
-
+        req = self._get_http_request()
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
-                                      use_admin_context=True)
         self.controller.update(req, 'update_me', body=body)
         self.assertEqual(len(self.default_quotas),
                          len(mock_createlimit.mock_calls))
 
     @mock.patch('nova.api.validation.validators._SchemaValidator.validate')
     @mock.patch('nova.objects.Quotas.create_limit')
-    def test_quotas_update_with_bad_data_as_admin(self, mock_createlimit,
+    def test_quotas_update_with_bad_data(self, mock_createlimit,
                                                   mock_validate):
         self.setup_mock_for_update()
         self.default_quotas.update({
@@ -227,15 +216,13 @@ class QuotaSetsTestV21(BaseQuotaSetsTest):
             'cores': -50
         })
         body = {'quota_set': self.default_quotas}
-
+        req = self._get_http_request()
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
-                                      use_admin_context=True)
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
                           req, 'update_me', body=body)
         self.assertEqual(0,
                          len(mock_createlimit.mock_calls))
 
-    def test_quotas_update_zero_value_as_admin(self):
+    def test_quotas_update_zero_value(self):
         self.setup_mock_for_update()
         body = {'quota_set': {'instances': 0, 'cores': 0,
                               'ram': 0, 'floating_ips': 0,
@@ -250,27 +237,13 @@ class QuotaSetsTestV21(BaseQuotaSetsTest):
             body['quota_set']['server_groups'] = 10
             body['quota_set']['server_group_members'] = 10
 
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
+        req = self._get_http_request()
-                                      use_admin_context=True)
         res_dict = self.controller.update(req, 'update_me', body=body)
         self.assertEqual(body, res_dict)
 
-    def test_quotas_update_as_user(self):
-        self.setup_mock_for_update()
-        self.default_quotas.update({
-            'instances': 50,
-            'cores': 50
-        })
-        body = {'quota_set': self.default_quotas}
-
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me')
-        self.assertRaises(webob.exc.HTTPForbidden, self.controller.update,
-                          req, 'update_me', body=body)
-
     def _quotas_update_bad_request_case(self, body):
         self.setup_mock_for_update()
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
+        req = self._get_http_request()
-                                      use_admin_context=True)
         self.assertRaises(self.validation_error, self.controller.update,
                           req, 'update_me', body=body)
 
@@ -371,6 +344,9 @@ class ExtendedQuotasTestV21(BaseQuotaSetsTest):
                           'maximum': -1},
         }
 
+    def _get_http_request(self, url=''):
+        return fakes.HTTPRequest.blank(url)
+
     def test_quotas_update_exceed_in_used(self):
         patcher = mock.patch.object(quota.QUOTAS, 'get_settable_quotas')
         get_settable_quotas = patcher.start()
@@ -378,8 +354,7 @@ class ExtendedQuotasTestV21(BaseQuotaSetsTest):
         body = {'quota_set': {'cores': 10}}
 
         get_settable_quotas.side_effect = self.fake_get_settable_quotas
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
+        req = self._get_http_request()
-                                      use_admin_context=True)
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
                           req, 'update_me', body=body)
         mock.patch.stopall()
@@ -395,8 +370,7 @@ class ExtendedQuotasTestV21(BaseQuotaSetsTest):
 
         get_settable_quotas.side_effect = self.fake_get_settable_quotas
         _get_quotas.side_effect = self.fake_get_quotas
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
+        req = self._get_http_request()
-                                      use_admin_context=True)
         self.controller.update(req, 'update_me', body=body)
         mock.patch.stopall()
 
@@ -443,21 +417,14 @@ class UserQuotasTestV21(BaseQuotaSetsTest):
         self.ext_mgr = self.mox.CreateMock(extensions.ExtensionManager)
         self.controller = self.plugin.QuotaSetsController(self.ext_mgr)
 
-    def test_user_quotas_show_as_admin(self):
+    def test_user_quotas_show(self):
         self.setup_mock_for_show()
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234?user_id=1',
+        req = self._get_http_request('/v2/fake4/os-quota-sets/1234?user_id=1')
-                                      use_admin_context=True)
         res_dict = self.controller.show(req, 1234)
         ref_quota_set = quota_set('1234', self.include_server_group_quotas)
         self.assertEqual(res_dict, ref_quota_set)
 
-    def test_user_quotas_show_as_unauthorized_user(self):
+    def test_user_quotas_update(self):
-        self.setup_mock_for_show()
-        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234?user_id=1')
-        self.assertRaises(webob.exc.HTTPForbidden, self.controller.show,
-                          req, 1234)
-
-    def test_user_quotas_update_as_admin(self):
         self.setup_mock_for_update()
         body = {'quota_set': {'instances': 10, 'cores': 20,
                               'ram': 51200, 'floating_ips': 10,
@@ -473,35 +440,17 @@ class UserQuotasTestV21(BaseQuotaSetsTest):
             body['quota_set']['server_group_members'] = 10
 
         url = '/v2/fake4/os-quota-sets/update_me?user_id=1'
-        req = fakes.HTTPRequest.blank(url, use_admin_context=True)
+        req = self._get_http_request(url)
         res_dict = self.controller.update(req, 'update_me', body=body)
 
         self.assertEqual(body, res_dict)
 
-    def test_user_quotas_update_as_user(self):
-        self.setup_mock_for_update()
-        body = {'quota_set': {'instances': 10, 'cores': 20,
-                              '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,
-                              'server_groups': 10,
-                              'server_group_members': 10}}
-
-        url = '/v2/fake4/os-quota-sets/update_me?user_id=1'
-        req = fakes.HTTPRequest.blank(url)
-        self.assertRaises(webob.exc.HTTPForbidden, self.controller.update,
-                          req, 'update_me', body=body)
-
     def test_user_quotas_update_exceed_project(self):
         self.setup_mock_for_update()
         body = {'quota_set': {'instances': 20}}
 
         url = '/v2/fake4/os-quota-sets/update_me?user_id=1'
-        req = fakes.HTTPRequest.blank(url, use_admin_context=True)
+        req = self._get_http_request(url)
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
                           req, 'update_me', body=body)
 
@@ -622,6 +571,23 @@ class QuotaSetsTestV2(QuotaSetsTestV21):
         self.assertRaises(webob.exc.HTTPForbidden, self.controller.delete,
                           req, 1234)
 
+    def test_quotas_show_as_unauthorized_user(self):
+        self.setup_mock_for_show()
+        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234')
+        self.assertRaises(webob.exc.HTTPForbidden, self.controller.show,
+                          req, 1234)
+
+    def test_quotas_update_as_user(self):
+        self.default_quotas.update({
+            'instances': 50,
+            'cores': 50
+        })
+        body = {'quota_set': self.default_quotas}
+
+        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me')
+        self.assertRaises(webob.exc.HTTPForbidden, self.controller.update,
+                          req, 'update_me', body=body)
+
 
 class QuotaSetsTestV2WithoutServerGroupQuotas(QuotaSetsTestV2):
     include_server_group_quotas = False
@@ -653,6 +619,9 @@ class ExtendedQuotasTestV2(ExtendedQuotasTestV21):
         self.controller = self.plugin.QuotaSetsController(self.ext_mgr)
         self.mox.ResetAll()
 
+    def _get_http_request(self, url=''):
+        return fakes.HTTPRequest.blank(url, use_admin_context=True)
+
 
 class UserQuotasTestV2(UserQuotasTestV21):
     plugin = quotas_v2
@@ -674,6 +643,27 @@ class UserQuotasTestV2(UserQuotasTestV21):
         self.assertRaises(webob.exc.HTTPForbidden, self.controller.delete,
                           req, '1234')
 
+    def test_user_quotas_show_as_unauthorized_user(self):
+        self.setup_mock_for_show()
+        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/1234?user_id=1')
+        self.assertRaises(webob.exc.HTTPForbidden, self.controller.show,
+                          req, 1234)
+
+    def test_user_quotas_update_as_user(self):
+        body = {'quota_set': {'instances': 10, 'cores': 20,
+                              'ram': 51200, 'floating_ips': 10,
+                              'fixed_ips': -1, 'metadata_items': 128,
+                              'injected_files': 5,
+                              'injected_file_content_bytes': 10240,
+                              'key_pairs': 100,
+                              'security_groups': 10,
+                              'security_group_rules': 20}}
+
+        url = '/v2/fake4/os-quota-sets/update_me?user_id=1'
+        req = fakes.HTTPRequest.blank(url)
+        self.assertRaises(webob.exc.HTTPForbidden, self.controller.update,
+                          req, 'update_me', body=body)
+
 
 class UserQuotasTestV2WithoutServerGroupQuotas(UserQuotasTestV2):
     include_server_group_quotas = False
@@ -716,3 +706,44 @@ class QuotaSetsPolicyEnforcementV21(test.NoDBTestCase):
         self.assertEqual(
             "Policy doesn't allow %s to be performed." % rule_name,
             exc.format_message())
+
+    def test_defaults_policy_failed(self):
+        rule_name = "os_compute_api:os-quota-sets:defaults"
+        self.policy.set_rules({rule_name: "project_id:non_fake"})
+        exc = self.assertRaises(
+            exception.PolicyNotAuthorized,
+            self.controller.defaults, self.req, fakes.FAKE_UUID)
+        self.assertEqual(
+            "Policy doesn't allow %s to be performed." % rule_name,
+            exc.format_message())
+
+    def test_show_policy_failed(self):
+        rule_name = "os_compute_api:os-quota-sets:show"
+        self.policy.set_rules({rule_name: "project_id:non_fake"})
+        exc = self.assertRaises(
+            exception.PolicyNotAuthorized,
+            self.controller.show, self.req, fakes.FAKE_UUID)
+        self.assertEqual(
+            "Policy doesn't allow %s to be performed." % rule_name,
+            exc.format_message())
+
+    def test_detail_policy_failed(self):
+        rule_name = "os_compute_api:os-quota-sets:detail"
+        self.policy.set_rules({rule_name: "project_id:non_fake"})
+        exc = self.assertRaises(
+            exception.PolicyNotAuthorized,
+            self.controller.detail, self.req, fakes.FAKE_UUID)
+        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-sets:update"
+        self.policy.set_rules({rule_name: "project_id:non_fake"})
+        exc = self.assertRaises(
+            exception.PolicyNotAuthorized,
+            self.controller.update, self.req, fakes.FAKE_UUID,
+            body={'quota_set': {}})
+        self.assertEqual(
+            "Policy doesn't allow %s to be performed." % rule_name,
+            exc.format_message())

nova/tests/unit/fake_policy.py

@@ -300,6 +300,7 @@ policy_data = """
     "os_compute_api:os-quota-sets:update": "",
     "os_compute_api:os-quota-sets:delete": "",
     "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": "",
     "compute_extension:rescue": "",