openstack
/
octavia
Code Issues Proposed changes

Don't reload the policy for every API request. 

Change-Id: I764e1bee45928370c11c767df7f00be7228de5b7
This commit is contained in:
Michael Johnson 2017-06-27 16:28:43 -07:00 committed by Adam Harwell
parent 1654ea06e7
commit e94ed48d0c
7 changed files with 78 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
octavia/api/v2/controllers/base.py
View File

@ -21,6 +21,7 @@ from stevedore import driver as stevedore_driver
from octavia.common import constants
from octavia.common import data_models
from octavia.common import exceptions
from octavia.common import policy
from octavia.db import repositories
CONF = cfg.CONF
@ -154,7 +155,8 @@ class BaseController(rest.RestController):
action = '{rbac_obj}{action}'.format(
rbac_obj=self.RBAC_TYPE, action=constants.RBAC_GET_ALL_GLOBAL)
target = {'project_id': project_id}
if not context.policy.authorize(action, target, do_raise=False):
if not policy.get_enforcer().authorize(action, target,
context, do_raise=False):
# Not a global observer or admin
if project_id is None:
project_id = context.project_id
@ -173,7 +175,7 @@ class BaseController(rest.RestController):
action = '{rbac_obj}{action}'.format(
rbac_obj=self.RBAC_TYPE, action=action)
target = {'project_id': project_id}
context.policy.authorize(action, target)
policy.get_enforcer().authorize(action, target, context)
def _filter_fields(self, object_list, fields):
if CONF.api_settings.allow_field_selection:

7
octavia/api/v2/controllers/l7rule.py
View File

@ -63,11 +63,8 @@ class L7RuleController(base.BaseController):
l7policy = self._get_db_l7policy(context.session, self.l7policy_id)
# Check that the user is authorized to list members for this l7rule
action = '{rbac_obj}{action}'.format(
rbac_obj=constants.RBAC_L7RULE, action='get_all')
target = {'project_id': l7policy.project_id}
context.policy.authorize(action, target)
self._auth_validate_action(context, l7policy.project_id,
constants.RBAC_GET_ALL)
db_l7rules, links = self.repositories.l7rule.get_all(
context.session, show_deleted=False, l7policy_id=self.l7policy_id,

7
octavia/api/v2/controllers/member.py
View File

@ -64,11 +64,8 @@ class MembersController(base.BaseController):
pool = self._get_db_pool(context.session, self.pool_id)
# Check that the user is authorized to list members for this pool
action = '{rbac_obj}{action}'.format(
rbac_obj=constants.RBAC_MEMBER, action='get_all')
target = {'project_id': pool.project_id}
context.policy.authorize(action, target)
self._auth_validate_action(context, pool.project_id,
constants.RBAC_GET_ALL)
db_members, links = self.repositories.member.get_all(
context.session, show_deleted=False,

8
octavia/api/v2/controllers/quotas.py
View File

@ -98,6 +98,7 @@ class QuotasController(base.BaseController):
class QuotasDefaultController(base.BaseController):
RBAC_TYPE = constants.RBAC_QUOTA
def __init__(self, project_id):
super(QuotasDefaultController, self).__init__()
@ -111,11 +112,8 @@ class QuotasDefaultController(base.BaseController):
if not self.project_id:
raise exceptions.MissingAPIProjectID()
# Check that the user is authorized to see quota defaults
action = '{rbac_obj}{action}'.format(
rbac_obj=constants.RBAC_QUOTA, action='get_defaults')
target = {'project_id': self.project_id}
context.policy.authorize(action, target)
self._auth_validate_action(context, self.project_id,
constants.RBAC_GET_DEFAULTS)
quotas = self._get_default_quotas(self.project_id)
return self._convert_db_to_type(quotas, quota_types.QuotaResponse)

4
octavia/common/context.py
View File

@ -33,9 +33,7 @@ class Context(common_context.RequestContext):
super(Context, self).__init__(**kwargs)
self.policy = policy.Policy(self)
self.is_admin = (self.policy.check_is_admin() or
self.is_admin = (policy.get_enforcer().check_is_admin(self) or
CONF.api_settings.auth_strategy == constants.NOAUTH)
@property

36
octavia/common/policy.py
View File

@ -24,11 +24,27 @@ from octavia import policies
LOG = logging.getLogger(__name__)
OCTAVIA_POLICY = None
def get_enforcer():
global OCTAVIA_POLICY
if OCTAVIA_POLICY is None:
LOG.debug('Loading octavia policy object.')
OCTAVIA_POLICY = Policy()
return OCTAVIA_POLICY
def reset():
global OCTAVIA_POLICY
if OCTAVIA_POLICY:
OCTAVIA_POLICY.clear()
OCTAVIA_POLICY = None
class Policy(oslo_policy.Enforcer):
def __init__(self, context, conf=cfg.CONF, policy_file=None, rules=None,
def __init__(self, conf=cfg.CONF, policy_file=None, rules=None,
default_rule=None, use_conf=True, overwrite=True):
"""Init an Enforcer class.
@ -52,12 +68,13 @@ class Policy(oslo_policy.Enforcer):
super(Policy, self).__init__(conf, policy_file, rules, default_rule,
use_conf, overwrite)
self.context = context
self.register_defaults(policies.list_rules())
def authorize(self, action, target, do_raise=True, exc=None):
def authorize(self, action, target, context, do_raise=True, exc=None):
"""Verifies that the action is valid on the target in this context.
:param context: The oslo context for this request.
:param action: string representing the action to be checked
this should be colon separated for clarity.
i.e. ``compute:create_instance``,
@ -83,10 +100,10 @@ class Policy(oslo_policy.Enforcer):
authorized, and the exact value False if not authorized and
do_raise is False.
"""
credentials = self.context.to_policy_values()
credentials = context.to_policy_values()
# Inject is_admin into the credentials to allow override via
# config auth_strategy = constants.NOAUTH
credentials['is_admin'] = self.context.is_admin
credentials['is_admin'] = context.is_admin
if not exc:
exc = exceptions.PolicyForbidden
@ -104,13 +121,12 @@ class Policy(oslo_policy.Enforcer):
'credentials %(credentials)s',
{'action': action, 'credentials': credentials})
def check_is_admin(self):
def check_is_admin(self, context):
"""Does roles contains 'admin' role according to policy setting.
"""
credentials = self.context.to_dict()
target = credentials
return self.enforce('context_is_admin', target, credentials)
credentials = context.to_dict()
return self.enforce('context_is_admin', credentials, credentials)
def get_rules(self):
return self.rules
@ -136,4 +152,4 @@ class IsAdminCheck(oslo_policy.Check):
# This is used for the oslopolicy-policy-generator tool
def get_no_context_enforcer():
config.init([])
return Policy(None)
return Policy()

72
octavia/tests/unit/common/test_policy.py
View File

@ -12,6 +12,7 @@
"""Test of Policy Engine For Octavia."""
import logging
import tempfile
from oslo_config import fixture as oslo_fixture
@ -25,6 +26,7 @@ from octavia.common import policy
from octavia.tests.unit import base
CONF = config.cfg.CONF
LOG = logging.getLogger(__name__)
class PolicyFileTestCase(base.TestCase):
@ -33,6 +35,7 @@ class PolicyFileTestCase(base.TestCase):
super(PolicyFileTestCase, self).setUp()
self.conf = self.useFixture(oslo_fixture.Config(CONF))
policy.reset()
self.target = {}
def test_modified_policy_reloads(self):
@ -46,18 +49,18 @@ class PolicyFileTestCase(base.TestCase):
self.context = context.Context('fake', 'fake')
rule = oslo_policy.RuleDefault('example:test', "")
self.context.policy.register_defaults([rule])
policy.get_enforcer().register_defaults([rule])
action = "example:test"
self.context.policy.authorize(action, self.target)
policy.get_enforcer().authorize(action, self.target, self.context)
tmp.seek(0)
tmp.write('{"example:test": "!"}')
tmp.flush()
self.context.policy.load_rules(True)
policy.get_enforcer().load_rules(True)
self.assertRaises(exceptions.PolicyForbidden,
self.context.policy.authorize,
action, self.target)
policy.get_enforcer().authorize,
action, self.target, self.context)
class PolicyTestCase(base.TestCase):
@ -69,6 +72,8 @@ class PolicyTestCase(base.TestCase):
# diltram: this one must be removed after fixing issue in oslo.config
# https://bugs.launchpad.net/oslo.config/+bug/1645868
self.conf.conf.__call__(args=[])
policy.reset()
self.context = context.Context('fake', 'fake', roles=['member'])
self.rules = [
oslo_policy.RuleDefault("true", "@"),
@ -86,30 +91,31 @@ class PolicyTestCase(base.TestCase):
oslo_policy.RuleDefault("example:uppercase_admin",
"role:ADMIN or role:sysadmin"),
]
self.context = context.Context('fake', 'fake', roles=['member'])
self.context.policy.register_defaults(self.rules)
policy.get_enforcer().register_defaults(self.rules)
self.target = {}
def test_authorize_nonexistent_action_throws(self):
action = "example:noexist"
self.assertRaises(
oslo_policy.PolicyNotRegistered, self.context.policy.authorize,
action, self.target)
oslo_policy.PolicyNotRegistered, policy.get_enforcer().authorize,
action, self.target, self.context)
def test_authorize_bad_action_throws(self):
action = "example:denied"
self.assertRaises(
exceptions.PolicyForbidden, self.context.policy.authorize,
action, self.target)
exceptions.PolicyForbidden, policy.get_enforcer().authorize,
action, self.target, self.context)
def test_authorize_bad_action_noraise(self):
action = "example:denied"
result = self.context.policy.authorize(action, self.target, False)
result = policy.get_enforcer().authorize(action, self.target,
self.context, False)
self.assertFalse(result)
def test_authorize_good_action(self):
action = "example:allowed"
result = self.context.policy.authorize(action, self.target)
result = policy.get_enforcer().authorize(action, self.target,
self.context)
self.assertTrue(result)
@requests_mock.mock()
@ -117,26 +123,28 @@ class PolicyTestCase(base.TestCase):
req_mock.post('http://www.example.com/', text='False')
action = "example:get_http"
self.assertRaises(exceptions.PolicyForbidden,
self.context.policy.authorize, action, self.target)
policy.get_enforcer().authorize, action, self.target,
self.context)
def test_templatized_authorization(self):
target_mine = {'project_id': 'fake'}
target_not_mine = {'project_id': 'another'}
action = "example:my_file"
self.context.policy.authorize(action, target_mine)
policy.get_enforcer().authorize(action, target_mine, self.context)
self.assertRaises(exceptions.PolicyForbidden,
self.context.policy.authorize,
action, target_not_mine)
policy.get_enforcer().authorize,
action, target_not_mine, self.context)
def test_early_AND_authorization(self):
action = "example:early_and_fail"
self.assertRaises(exceptions.PolicyForbidden,
self.context.policy.authorize, action, self.target)
policy.get_enforcer().authorize, action, self.target,
self.context)
def test_early_OR_authorization(self):
action = "example:early_or_success"
self.context.policy.authorize(action, self.target)
policy.get_enforcer().authorize(action, self.target, self.context)
def test_ignore_case_role_check(self):
lowercase_action = "example:lowercase_admin"
@ -145,19 +153,19 @@ class PolicyTestCase(base.TestCase):
# NOTE(dprince) we mix case in the Admin role here to ensure
# case is ignored
self.context = context.Context('admin', 'fake', roles=['AdMiN'])
self.context.policy.register_defaults(self.rules)
self.context.policy.authorize(lowercase_action, self.target)
self.context.policy.authorize(uppercase_action, self.target)
policy.get_enforcer().authorize(lowercase_action, self.target,
self.context)
policy.get_enforcer().authorize(uppercase_action, self.target,
self.context)
def test_check_is_admin_fail(self):
self.assertFalse(self.context.policy.check_is_admin())
self.assertFalse(policy.get_enforcer().check_is_admin(self.context))
def test_check_is_admin(self):
self.context = context.Context('admin', 'fake', roles=['AdMiN'])
self.context.policy.register_defaults(self.rules)
self.assertTrue(self.context.policy.check_is_admin())
self.assertTrue(policy.get_enforcer().check_is_admin(self.context))
def test_get_enforcer(self):
self.assertTrue(isinstance(policy.get_no_context_enforcer(),
@ -194,17 +202,17 @@ class IsAdminCheckTestCase(base.TestCase):
check = policy.IsAdminCheck('is_admin', 'True')
self.assertTrue(
check('target', dict(is_admin=True), self.context.policy))
check('target', dict(is_admin=True), policy.get_enforcer()))
self.assertFalse(
check('target', dict(is_admin=False), self.context.policy))
check('target', dict(is_admin=False), policy.get_enforcer()))
def test_call_false(self):
check = policy.IsAdminCheck('is_admin', 'False')
self.assertFalse(
check('target', dict(is_admin=True), self.context.policy))
check('target', dict(is_admin=True), policy.get_enforcer()))
self.assertTrue(
check('target', dict(is_admin=False), self.context.policy))
check('target', dict(is_admin=False), policy.get_enforcer()))
class AdminRolePolicyTestCase(base.TestCase):
@ -218,7 +226,7 @@ class AdminRolePolicyTestCase(base.TestCase):
self.conf.conf.__call__(args=[])
self.context = context.Context('fake', 'fake', roles=['member'])
self.actions = self.context.policy.get_rules().keys()
self.actions = policy.get_enforcer().get_rules().keys()
self.target = {}
def test_authorize_admin_actions_with_nonadmin_context_throws(self):
@ -228,5 +236,5 @@ class AdminRolePolicyTestCase(base.TestCase):
"""
for action in self.actions:
self.assertRaises(
exceptions.PolicyForbidden, self.context.policy.authorize,
action, self.target)
exceptions.PolicyForbidden, policy.get_enforcer().authorize,
action, self.target, self.context)