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
This commit is contained in:
		@@ -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):
 | 
			
		||||
 
 | 
			
		||||
@@ -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()
 | 
			
		||||
 
 | 
			
		||||
@@ -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
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -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']
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user