openstack
/
neutron
Code Issues Proposed changes

Merge "Enable to apply policies to resources with special plural"

This commit is contained in:
Jenkins 2015-04-01 08:04:45 +00:00 committed by Gerrit Code Review
parent 8305c85898 88fb463b3c
commit b025e24998
3 changed files with 102 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
neutron/api/v2/base.py
View File

@ -144,7 +144,8 @@ class Controller(object):
context, context,
'%s:%s' % (self._plugin_handlers[self.SHOW], attr_name), '%s:%s' % (self._plugin_handlers[self.SHOW], attr_name),
data, data,
might_not_exist=True): might_not_exist=True,
pluralized=self._collection):
# this attribute is visible, check next one # this attribute is visible, check next one
continue continue
# if the code reaches this point then either the policy check # if the code reaches this point then either the policy check
@ -199,7 +200,10 @@ class Controller(object):
arg_list.append(body) arg_list.append(body)
# It is ok to raise a 403 because accessibility to the # It is ok to raise a 403 because accessibility to the
# object was checked earlier in this method # object was checked earlier in this method
policy.enforce(request.context, name, resource) policy.enforce(request.context,
name,
resource,
pluralized=self._collection)
return getattr(self._plugin, name)(*arg_list, **kwargs) return getattr(self._plugin, name)(*arg_list, **kwargs)
return _handle_action return _handle_action
else: else:
@ -254,7 +258,8 @@ class Controller(object):
if policy.check(request.context, if policy.check(request.context,
self._plugin_handlers[self.SHOW], self._plugin_handlers[self.SHOW],
obj, obj,
plugin=self._plugin)] plugin=self._plugin,
pluralized=self._collection)]
# Use the first element in the list for discriminating which attributes # Use the first element in the list for discriminating which attributes
# should be filtered out because of authZ policies # should be filtered out because of authZ policies
# fields_to_add contains a list of attributes added for request policy # fields_to_add contains a list of attributes added for request policy
@ -287,7 +292,10 @@ class Controller(object):
# FIXME(salvatore-orlando): obj_getter might return references to # FIXME(salvatore-orlando): obj_getter might return references to
# other resources. Must check authZ on them too. # other resources. Must check authZ on them too.
if do_authz: if do_authz:
policy.enforce(request.context, action, obj) policy.enforce(request.context,
action,
obj,
pluralized=self._collection)
return obj return obj
def _send_dhcp_notification(self, context, data, methodname): def _send_dhcp_notification(self, context, data, methodname):
@ -398,7 +406,8 @@ class Controller(object):
item[self._resource]) item[self._resource])
policy.enforce(request.context, policy.enforce(request.context,
action, action,
item[self._resource]) item[self._resource],
pluralized=self._collection)
try: try:
tenant_id = item[self._resource]['tenant_id'] tenant_id = item[self._resource]['tenant_id']
count = quota.QUOTAS.count(request.context, self._resource, count = quota.QUOTAS.count(request.context, self._resource,
@ -469,7 +478,8 @@ class Controller(object):
try: try:
policy.enforce(request.context, policy.enforce(request.context,
action, action,
obj) obj,
pluralized=self._collection)
except common_policy.PolicyNotAuthorized: except common_policy.PolicyNotAuthorized:
# To avoid giving away information, pretend that it # To avoid giving away information, pretend that it
# doesn't exist # doesn't exist
@ -524,7 +534,8 @@ class Controller(object):
try: try:
policy.enforce(request.context, policy.enforce(request.context,
action, action,
orig_obj) orig_obj,
pluralized=self._collection)
except common_policy.PolicyNotAuthorized: except common_policy.PolicyNotAuthorized:
with excutils.save_and_reraise_exception() as ctxt: with excutils.save_and_reraise_exception() as ctxt:
# If a tenant is modifying it's own object, it's safe to return # If a tenant is modifying it's own object, it's safe to return

37
neutron/policy.py
View File

@ -78,10 +78,11 @@ def refresh():
init() init()
def get_resource_and_action(action): def get_resource_and_action(action, pluralized=None):
"""Extract resource and action (write, read) from api operation.""" """Extract resource and action (write, read) from api operation."""
data = action.split(':', 1)[0].split('_', 1) data = action.split(':', 1)[0].split('_', 1)
return ("%ss" % data[-1], data[0] != 'get') resource = pluralized or ("%ss" % data[-1])
return (resource, data[0] != 'get')
def set_rules(policies, overwrite=True): def set_rules(policies, overwrite=True):
@ -183,7 +184,7 @@ def _process_rules_list(rules, match_rule):
return rules return rules
def _build_match_rule(action, target): def _build_match_rule(action, target, pluralized):
"""Create the rule to match for a given action. """Create the rule to match for a given action.
The policy rule to be matched is built in the following way: The policy rule to be matched is built in the following way:
@ -196,7 +197,7 @@ def _build_match_rule(action, target):
(e.g.: create_router:external_gateway_info:network_id) (e.g.: create_router:external_gateway_info:network_id)
""" """
match_rule = policy.RuleCheck('rule', action) match_rule = policy.RuleCheck('rule', action)
resource, is_write = get_resource_and_action(action) resource, is_write = get_resource_and_action(action, pluralized)
# Attribute-based checks shall not be enforced on GETs # Attribute-based checks shall not be enforced on GETs
if is_write: if is_write:
# assigning to variable with short name for improving readability # assigning to variable with short name for improving readability
@ -346,12 +347,12 @@ class FieldCheck(policy.Check):
return target_value == self.value return target_value == self.value
def _prepare_check(context, action, target): def _prepare_check(context, action, target, pluralized):
"""Prepare rule, target, and credentials for the policy engine.""" """Prepare rule, target, and credentials for the policy engine."""
# Compare with None to distinguish case in which target is {} # Compare with None to distinguish case in which target is {}
if target is None: if target is None:
target = {} target = {}
match_rule = _build_match_rule(action, target) match_rule = _build_match_rule(action, target, pluralized)
credentials = context.to_dict() credentials = context.to_dict()
return match_rule, target, credentials return match_rule, target, credentials
@ -362,7 +363,8 @@ def log_rule_list(match_rule):
LOG.debug("Enforcing rules: %s", rules) LOG.debug("Enforcing rules: %s", rules)
def check(context, action, target, plugin=None, might_not_exist=False): def check(context, action, target, plugin=None, might_not_exist=False,
pluralized=None):
"""Verifies that the action is valid on the target in this context. """Verifies that the action is valid on the target in this context.
:param context: neutron context :param context: neutron context
@ -376,20 +378,28 @@ def check(context, action, target, plugin=None, might_not_exist=False):
:param might_not_exist: If True the policy check is skipped (and the :param might_not_exist: If True the policy check is skipped (and the
function returns True) if the specified policy does not exist. function returns True) if the specified policy does not exist.
Defaults to false. Defaults to false.
:param pluralized: pluralized case of resource
e.g. firewall_policy -> pluralized = "firewall_policies"
:return: Returns True if access is permitted else False. :return: Returns True if access is permitted else False.
""" """
if might_not_exist and not (_ENFORCER.rules and action in _ENFORCER.rules): if might_not_exist and not (_ENFORCER.rules and action in _ENFORCER.rules):
return True return True
match_rule, target, credentials = _prepare_check(context, action, target) match_rule, target, credentials = _prepare_check(context,
result = _ENFORCER.enforce(match_rule, target, credentials) action,
target,
pluralized)
result = _ENFORCER.enforce(match_rule,
target,
credentials,
pluralized=pluralized)
# logging applied rules in case of failure # logging applied rules in case of failure
if not result: if not result:
log_rule_list(match_rule) log_rule_list(match_rule)
return result return result
def enforce(context, action, target, plugin=None): def enforce(context, action, target, plugin=None, pluralized=None):
"""Verifies that the action is valid on the target in this context. """Verifies that the action is valid on the target in this context.
:param context: neutron context :param context: neutron context
@ -400,11 +410,16 @@ def enforce(context, action, target, plugin=None):
location of the object e.g. ``{'project_id': context.project_id}`` location of the object e.g. ``{'project_id': context.project_id}``
:param plugin: currently unused and deprecated. :param plugin: currently unused and deprecated.
Kept for backward compatibility. Kept for backward compatibility.
:param pluralized: pluralized case of resource
e.g. firewall_policy -> pluralized = "firewall_policies"
:raises neutron.openstack.common.policy.PolicyNotAuthorized: :raises neutron.openstack.common.policy.PolicyNotAuthorized:
if verification fails. if verification fails.
""" """
rule, target, credentials = _prepare_check(context, action, target) rule, target, credentials = _prepare_check(context,
action,
target,
pluralized)
try: try:
result = _ENFORCER.enforce(rule, target, credentials, action=action, result = _ENFORCER.enforce(rule, target, credentials, action=action,
do_raise=True) do_raise=True)

88
neutron/tests/unit/test_policy.py
View File

@ -183,17 +183,29 @@ class DefaultPolicyTestCase(base.BaseTestCase):
policy.enforce(self.context, "example:noexist", {}) policy.enforce(self.context, "example:noexist", {})
FAKE_RESOURCE_NAME = 'something' FAKE_RESOURCE_NAME = 'fake_resource'
FAKE_RESOURCE = {"%ss" % FAKE_RESOURCE_NAME: FAKE_SPECIAL_RESOURCE_NAME = 'fake_policy'
{'attr': {'allow_post': True, FAKE_RESOURCES = {"%ss" % FAKE_RESOURCE_NAME:
'allow_put': True, {'attr': {'allow_post': True,
'is_visible': True, 'allow_put': True,
'default': None, 'is_visible': True,
'enforce_policy': True, 'default': None,
'validate': {'type:dict': 'enforce_policy': True,
{'sub_attr_1': {'type:string': None}, 'validate': {'type:dict':
'sub_attr_2': {'type:string': None}}} {'sub_attr_1': {'type:string': None},
}}} 'sub_attr_2': {'type:string': None}}}
}},
# special plural name
"%s" % FAKE_SPECIAL_RESOURCE_NAME.replace('y', 'ies'):
{'attr': {'allow_post': True,
'allow_put': True,
'is_visible': True,
'default': None,
'enforce_policy': True,
'validate': {'type:dict':
{'sub_attr_1': {'type:string': None},
'sub_attr_2': {'type:string': None}}}
}}}
class NeutronPolicyTestCase(base.BaseTestCase): class NeutronPolicyTestCase(base.BaseTestCase):
@ -207,8 +219,8 @@ class NeutronPolicyTestCase(base.BaseTestCase):
policy.refresh() policy.refresh()
self.admin_only_legacy = "role:admin" self.admin_only_legacy = "role:admin"
self.admin_or_owner_legacy = "role:admin or tenant_id:%(tenant_id)s" self.admin_or_owner_legacy = "role:admin or tenant_id:%(tenant_id)s"
# Add a Fake 'something' resource to RESOURCE_ATTRIBUTE_MAP # Add Fake resources to RESOURCE_ATTRIBUTE_MAP
attributes.RESOURCE_ATTRIBUTE_MAP.update(FAKE_RESOURCE) attributes.RESOURCE_ATTRIBUTE_MAP.update(FAKE_RESOURCES)
self.rules = dict((k, common_policy.parse_rule(v)) for k, v in { self.rules = dict((k, common_policy.parse_rule(v)) for k, v in {
"context_is_admin": "role:admin", "context_is_admin": "role:admin",
"context_is_advsvc": "role:advsvc", "context_is_advsvc": "role:advsvc",
@ -234,11 +246,12 @@ class NeutronPolicyTestCase(base.BaseTestCase):
"update_port": "rule:admin_or_owner or rule:context_is_advsvc", "update_port": "rule:admin_or_owner or rule:context_is_advsvc",
"get_port": "rule:admin_or_owner or rule:context_is_advsvc", "get_port": "rule:admin_or_owner or rule:context_is_advsvc",
"delete_port": "rule:admin_or_owner or rule:context_is_advsvc", "delete_port": "rule:admin_or_owner or rule:context_is_advsvc",
"create_something": "rule:admin_or_owner", "create_fake_resource": "rule:admin_or_owner",
"create_something:attr": "rule:admin_or_owner", "create_fake_resource:attr": "rule:admin_or_owner",
"create_something:attr:sub_attr_1": "rule:admin_or_owner", "create_fake_resource:attr:sub_attr_1": "rule:admin_or_owner",
"create_something:attr:sub_attr_2": "rule:admin_only", "create_fake_resource:attr:sub_attr_2": "rule:admin_only",
"create_fake_policy:": "rule:admin_or_owner",
"get_firewall_policy": "rule:admin_or_owner or " "get_firewall_policy": "rule:admin_or_owner or "
"rule:shared", "rule:shared",
"get_firewall_rule": "rule:admin_or_owner or " "get_firewall_rule": "rule:admin_or_owner or "
@ -374,17 +387,17 @@ class NeutronPolicyTestCase(base.BaseTestCase):
self.context, action, target) self.context, action, target)
def _test_build_subattribute_match_rule(self, validate_value): def _test_build_subattribute_match_rule(self, validate_value):
bk = FAKE_RESOURCE['%ss' % FAKE_RESOURCE_NAME]['attr']['validate'] bk = FAKE_RESOURCES['%ss' % FAKE_RESOURCE_NAME]['attr']['validate']
FAKE_RESOURCE['%ss' % FAKE_RESOURCE_NAME]['attr']['validate'] = ( FAKE_RESOURCES['%ss' % FAKE_RESOURCE_NAME]['attr']['validate'] = (
validate_value) validate_value)
action = "create_something" action = "create_" + FAKE_RESOURCE_NAME
target = {'tenant_id': 'fake', 'attr': {'sub_attr_1': 'x'}} target = {'tenant_id': 'fake', 'attr': {'sub_attr_1': 'x'}}
self.assertFalse(policy._build_subattr_match_rule( self.assertFalse(policy._build_subattr_match_rule(
'attr', 'attr',
FAKE_RESOURCE['%ss' % FAKE_RESOURCE_NAME]['attr'], FAKE_RESOURCES['%ss' % FAKE_RESOURCE_NAME]['attr'],
action, action,
target)) target))
FAKE_RESOURCE['%ss' % FAKE_RESOURCE_NAME]['attr']['validate'] = bk FAKE_RESOURCES['%ss' % FAKE_RESOURCE_NAME]['attr']['validate'] = bk
def test_build_subattribute_match_rule_empty_dict_validator(self): def test_build_subattribute_match_rule_empty_dict_validator(self):
self._test_build_subattribute_match_rule({}) self._test_build_subattribute_match_rule({})
@ -393,14 +406,27 @@ class NeutronPolicyTestCase(base.BaseTestCase):
self._test_build_subattribute_match_rule( self._test_build_subattribute_match_rule(
{'type:dict': 'wrong_stuff'}) {'type:dict': 'wrong_stuff'})
def test_build_match_rule_special_pluralized(self):
action = "create_" + FAKE_SPECIAL_RESOURCE_NAME
pluralized = "create_fake_policies"
target = {}
result = policy._build_match_rule(action, target, pluralized)
self.assertEqual("rule:" + action, str(result))
def test_build_match_rule_normal_pluralized_when_create(self):
action = "create_" + FAKE_RESOURCE_NAME
target = {}
result = policy._build_match_rule(action, target, None)
self.assertEqual("rule:" + action, str(result))
def test_enforce_subattribute(self): def test_enforce_subattribute(self):
action = "create_something" action = "create_" + FAKE_RESOURCE_NAME
target = {'tenant_id': 'fake', 'attr': {'sub_attr_1': 'x'}} target = {'tenant_id': 'fake', 'attr': {'sub_attr_1': 'x'}}
result = policy.enforce(self.context, action, target, None) result = policy.enforce(self.context, action, target, None)
self.assertEqual(result, True) self.assertEqual(result, True)
def test_enforce_admin_only_subattribute(self): def test_enforce_admin_only_subattribute(self):
action = "create_something" action = "create_" + FAKE_RESOURCE_NAME
target = {'tenant_id': 'fake', 'attr': {'sub_attr_1': 'x', target = {'tenant_id': 'fake', 'attr': {'sub_attr_1': 'x',
'sub_attr_2': 'y'}} 'sub_attr_2': 'y'}}
result = policy.enforce(context.get_admin_context(), result = policy.enforce(context.get_admin_context(),
@ -408,7 +434,7 @@ class NeutronPolicyTestCase(base.BaseTestCase):
self.assertEqual(result, True) self.assertEqual(result, True)
def test_enforce_admin_only_subattribute_nonadminctx_returns_403(self): def test_enforce_admin_only_subattribute_nonadminctx_returns_403(self):
action = "create_something" action = "create_" + FAKE_RESOURCE_NAME
target = {'tenant_id': 'fake', 'attr': {'sub_attr_1': 'x', target = {'tenant_id': 'fake', 'attr': {'sub_attr_1': 'x',
'sub_attr_2': 'y'}} 'sub_attr_2': 'y'}}
self.assertRaises(common_policy.PolicyNotAuthorized, policy.enforce, self.assertRaises(common_policy.PolicyNotAuthorized, policy.enforce,
@ -576,11 +602,12 @@ class NeutronPolicyTestCase(base.BaseTestCase):
expected_policies)) expected_policies))
def test_process_rules(self): def test_process_rules(self):
action = "create_something" action = "create_" + FAKE_RESOURCE_NAME
# Construct RuleChecks for an action, attribute and subattribute # Construct RuleChecks for an action, attribute and subattribute
match_rule = common_policy.RuleCheck('rule', action) match_rule = common_policy.RuleCheck('rule', action)
attr_rule = common_policy.RuleCheck('rule', '%s:%s' % attr_rule = common_policy.RuleCheck('rule', '%s:%ss' %
(action, 'somethings')) (action,
FAKE_RESOURCE_NAME))
sub_attr_rules = [common_policy.RuleCheck('rule', '%s:%s:%s' % sub_attr_rules = [common_policy.RuleCheck('rule', '%s:%s:%s' %
(action, 'attr', (action, 'attr',
'sub_attr_1'))] 'sub_attr_1'))]
@ -593,8 +620,9 @@ class NeutronPolicyTestCase(base.BaseTestCase):
match_rule = common_policy.AndCheck([match_rule, attr_rule]) match_rule = common_policy.AndCheck([match_rule, attr_rule])
# Assert that the rules are correctly extracted from the match_rule # Assert that the rules are correctly extracted from the match_rule
rules = policy._process_rules_list([], match_rule) rules = policy._process_rules_list([], match_rule)
self.assertEqual(['create_something', 'create_something:somethings', self.assertEqual(['create_fake_resource',
'create_something:attr:sub_attr_1'], rules) 'create_fake_resource:fake_resources',
'create_fake_resource:attr:sub_attr_1'], rules)
def test_log_rule_list(self): def test_log_rule_list(self):
with contextlib.nested( with contextlib.nested(