From 93b1d86f6d158a156e7bd2e43f5de84587bb5e6d Mon Sep 17 00:00:00 2001 From: Sam Morrison Date: Tue, 21 Nov 2017 15:34:27 +1100 Subject: [PATCH] Remove DB authorisation checking with quota API operations. This is now handled completely in policy. Change-Id: Ie75b6b135610a8bb206a6f941721d5e97f6db502 Closes-Bug: #1733480 --- cinder/api/contrib/quotas.py | 29 ++++++-------------- cinder/db/sqlalchemy/api.py | 2 +- cinder/policies/quotas.py | 2 +- cinder/tests/unit/api/contrib/test_quotas.py | 2 +- 4 files changed, 12 insertions(+), 23 deletions(-) diff --git a/cinder/api/contrib/quotas.py b/cinder/api/contrib/quotas.py index bda44466e1f..ec4029e4ab5 100644 --- a/cinder/api/contrib/quotas.py +++ b/cinder/api/contrib/quotas.py @@ -21,7 +21,6 @@ from oslo_utils import strutils from cinder.api import extensions from cinder.api.openstack import wsgi from cinder import db -from cinder.db.sqlalchemy import api as sqlalchemy_api from cinder import exception from cinder.i18n import _ from cinder.policies import quotas as policy @@ -166,9 +165,10 @@ class QuotaSetsController(wsgi.Controller): :param id: target project id that needs to be shown """ context = req.environ['cinder.context'] - context.authorize(policy.SHOW_POLICY) params = req.params target_project_id = id + context.authorize(policy.SHOW_POLICY, + target={'project_id': target_project_id}) if not hasattr(params, '__call__') and 'usage' in params: usage = utils.get_bool_param('usage', params) @@ -187,12 +187,6 @@ class QuotaSetsController(wsgi.Controller): self._authorize_show(context_project, target_project) - try: - sqlalchemy_api.authorize_project_context(context, - target_project_id) - except exception.NotAuthorized: - raise webob.exc.HTTPForbidden() - quotas = self._get_quotas(context, target_project_id, usage) return self._format_quota_set(target_project_id, quotas) @@ -209,7 +203,9 @@ class QuotaSetsController(wsgi.Controller): the resources if the update succeeds """ context = req.environ['cinder.context'] - context.authorize(policy.UPDATE_POLICY) + target_project_id = id + context.authorize(policy.UPDATE_POLICY, + target={'project_id': target_project_id}) self.validate_string_length(id, 'quota_set_name', min_length=1, max_length=255) @@ -230,7 +226,6 @@ class QuotaSetsController(wsgi.Controller): "validate it in Queens, please try to use " "skip_validation=False for quota updating now.") - target_project_id = id bad_keys = [] # NOTE(ankit): Pass #1 - In this loop for body['quota_set'].items(), @@ -351,7 +346,7 @@ class QuotaSetsController(wsgi.Controller): def defaults(self, req, id): context = req.environ['cinder.context'] - context.authorize(policy.SHOW_POLICY) + context.authorize(policy.SHOW_POLICY, target={'project_id': id}) defaults = QUOTAS.get_defaults(context, project_id=id) group_defaults = GROUP_QUOTAS.get_defaults(context, project_id=id) defaults.update(group_defaults) @@ -368,15 +363,12 @@ class QuotaSetsController(wsgi.Controller): :param id: target project id that needs to be deleted """ context = req.environ['cinder.context'] - context.authorize(policy.DELETE_POLICY) + context.authorize(policy.DELETE_POLICY, target={'project_id': id}) if QUOTAS.using_nested_quotas(): self._delete_nested_quota(context, id) else: - try: - db.quota_destroy_by_project(context, id) - except exception.AdminRequired: - raise webob.exc.HTTPForbidden() + db.quota_destroy_by_project(context, id) def _delete_nested_quota(self, ctxt, proj_id): # Get the parent_id of the target project to verify whether we are @@ -418,10 +410,7 @@ class QuotaSetsController(wsgi.Controller): self._validate_existing_resource( res, defaults[res], project_quotas) - try: - db.quota_destroy_by_project(ctxt, target_project.id) - except exception.AdminRequired: - raise webob.exc.HTTPForbidden() + db.quota_destroy_by_project(ctxt, target_project.id) for res, limit in project_quotas.items(): # Update child limit to 0 so the parent hierarchy gets it's diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index f3fe6f5dd4c..3086d2bcbf2 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -1437,7 +1437,7 @@ def quota_destroy_by_project(*args, **kwargs): quota_destroy_all_by_project(only_quotas=True, *args, **kwargs) -@require_admin_context +@require_context @_retry_on_deadlock def quota_destroy_all_by_project(context, project_id, only_quotas=False): """Destroy all quotas associated with a project. diff --git a/cinder/policies/quotas.py b/cinder/policies/quotas.py index ae572368494..ecdd1b57e8f 100644 --- a/cinder/policies/quotas.py +++ b/cinder/policies/quotas.py @@ -28,7 +28,7 @@ VALIDATE_NESTED_QUOTA_POLICY = \ quota_policies = [ policy.DocumentedRuleDefault( name=SHOW_POLICY, - check_str="", + check_str=base.RULE_ADMIN_OR_OWNER, description="Show project quota (including usage and default).", operations=[ { diff --git a/cinder/tests/unit/api/contrib/test_quotas.py b/cinder/tests/unit/api/contrib/test_quotas.py index f36792a451e..2a0c5cb1639 100644 --- a/cinder/tests/unit/api/contrib/test_quotas.py +++ b/cinder/tests/unit/api/contrib/test_quotas.py @@ -176,7 +176,7 @@ class QuotaSetsControllerTest(QuotaSetsControllerTestBase): self.req.environ['cinder.context'].is_admin = False self.req.environ['cinder.context'].user_id = fake.USER_ID self.req.environ['cinder.context'].project_id = fake.PROJECT_ID - self.assertRaises(webob.exc.HTTPForbidden, self.controller.show, + self.assertRaises(exception.PolicyNotAuthorized, self.controller.show, self.req, fake.PROJECT2_ID) def test_show_non_admin_user(self):