From e805dcad93c349dbf3cf02be83076a27199c9828 Mon Sep 17 00:00:00 2001 From: TommyLike Date: Thu, 7 Dec 2017 17:29:59 +0800 Subject: [PATCH] Don't raise 'NotAuthorized' inside when soft authorization Using try/except block when performing soft authorization is less efficient and will lead to lots of confusing error messages, pass 'fatal' parameter directly to inner authorize function. Change-Id: I0bdf23e371ff8469080982553cb1ffd8a64c9e2d Closes-Bug: #1736854 --- cinder/api/v3/backups.py | 11 ++--------- cinder/context.py | 11 ++++------- cinder/tests/unit/api/contrib/test_services.py | 3 ++- cinder/tests/unit/api/contrib/test_used_limits.py | 3 +-- 4 files changed, 9 insertions(+), 19 deletions(-) diff --git a/cinder/api/v3/backups.py b/cinder/api/v3/backups.py index f720b9d8103..f8b646c1ed4 100644 --- a/cinder/api/v3/backups.py +++ b/cinder/api/v3/backups.py @@ -22,7 +22,6 @@ from cinder.api.contrib import backups as backups_v2 from cinder.api import microversions as mv from cinder.api.openstack import wsgi from cinder.api.v3.views import backups as backup_views -from cinder import exception from cinder.i18n import _ from cinder.policies import backups as policy @@ -80,11 +79,8 @@ class BackupsController(backups_v2.BackupsController): resp_backup = self._view_builder.detail(req, backup) if req_version.matches(mv.BACKUP_PROJECT): - try: - context.authorize(policy.BACKUP_ATTRIBUTES_POLICY) + if context.authorize(policy.BACKUP_ATTRIBUTES_POLICY, fatal=False): self._add_backup_project_attribute(req, resp_backup['backup']) - except exception.PolicyNotAuthorized: - pass return resp_backup def detail(self, req): @@ -93,12 +89,9 @@ class BackupsController(backups_v2.BackupsController): req_version = req.api_version_request if req_version.matches(mv.BACKUP_PROJECT): - try: - context.authorize(policy.BACKUP_ATTRIBUTES_POLICY) + if context.authorize(policy.BACKUP_ATTRIBUTES_POLICY, fatal=False): for bak in resp_backup['backups']: self._add_backup_project_attribute(req, bak) - except exception.PolicyNotAuthorized: - pass return resp_backup def _convert_sort_name(self, req_version, sort_keys): diff --git a/cinder/context.py b/cinder/context.py index a5b1d79bc56..ca693876127 100644 --- a/cinder/context.py +++ b/cinder/context.py @@ -160,7 +160,7 @@ class RequestContext(context.RequestContext): :param: target_obj: dictionary representing the object which will be used to update target. :param fatal: if False, will return False when an - exception.NotAuthorized occurs. + exception.PolicyNotAuthorized occurs. :raises cinder.exception.NotAuthorized: if verification fails and fatal is True. @@ -177,12 +177,9 @@ class RequestContext(context.RequestContext): target_obj.obj_to_primitive()['versioned_object.data'] or {}) else: target.update(target_obj or {}) - try: - return policy.authorize(self, action, target) - except exception.NotAuthorized: - if fatal: - raise - return False + + return policy.authorize(self, action, target, do_raise=fatal, + exc=exception.PolicyNotAuthorized) def to_policy_values(self): policy = super(RequestContext, self).to_policy_values() diff --git a/cinder/tests/unit/api/contrib/test_services.py b/cinder/tests/unit/api/contrib/test_services.py index f33d671c657..869b2a7fd02 100644 --- a/cinder/tests/unit/api/contrib/test_services.py +++ b/cinder/tests/unit/api/contrib/test_services.py @@ -181,7 +181,8 @@ def fake_service_update(context, service_id, values): 'disabled': values['disabled']} -def fake_policy_authorize(context, action, target): +def fake_policy_authorize(context, action, target, + do_raise=True, exc=exception.PolicyNotAuthorized): pass diff --git a/cinder/tests/unit/api/contrib/test_used_limits.py b/cinder/tests/unit/api/contrib/test_used_limits.py index 48698096ebb..15d40eb41b9 100644 --- a/cinder/tests/unit/api/contrib/test_used_limits.py +++ b/cinder/tests/unit/api/contrib/test_used_limits.py @@ -20,7 +20,6 @@ from cinder.api.contrib import used_limits from cinder.api import microversions as mv from cinder.api.openstack import api_version_request from cinder.api.openstack import wsgi -from cinder import exception from cinder import test from cinder.tests.unit.api import fakes from cinder.tests.unit import fake_constants as fake @@ -111,7 +110,7 @@ class UsedLimitsTestCase(test.TestCase): res = wsgi.ResponseObject(obj) # unallow user to access used limits - _mock_policy_authorize.side_effect = exception.NotAuthorized + _mock_policy_authorize.return_value = False self.controller.index(fake_req, res) abs_limits = res.obj['limits']['absolute']