From a88d9d5936aabc52046ed3ae566741422bfaed78 Mon Sep 17 00:00:00 2001 From: Shuangtai Tian Date: Wed, 5 Mar 2014 19:10:32 +0800 Subject: [PATCH] Rewrite nova policy to use the new changes of common policy The oslo has rewritten the common policy for a long time, using a Enforer class to replace the old check function. In order to sync the common policy to nova, We have to also changed the nova policy and related unittests. Sync policy and relate module from Oslo. Some related change-id:Ife909bdf3277ef33c2fb1eae16ae261fa6374c63 change-id:Ife84189be4b86a3ee90da4539ff2dbed125be23d Closes-Bug: #1288178 Change-Id: Ic106851a24cfdc9d847f7d0e0d8a6ecbff888e05 --- nova/openstack/common/policy.py | 519 +++++++++++------- nova/policy.py | 99 ++-- nova/tests/api/ec2/test_cloud.py | 26 +- .../compute/contrib/test_instance_actions.py | 25 +- .../compute/contrib/test_keypairs.py | 35 +- .../compute/contrib/test_server_start_stop.py | 5 +- .../openstack/compute/contrib/test_shelve.py | 32 +- .../contrib/test_simple_tenant_usage.py | 2 +- .../compute/plugins/v3/test_certificates.py | 5 +- .../plugins/v3/test_instance_actions.py | 26 +- .../compute/plugins/v3/test_keypairs.py | 35 +- .../compute/plugins/v3/test_servers.py | 16 +- .../compute/plugins/v3/test_shelve.py | 33 +- .../api/openstack/compute/test_servers.py | 8 +- nova/tests/conf_fixture.py | 2 +- nova/tests/policy_fixture.py | 6 +- nova/tests/test_policy.py | 33 +- 17 files changed, 515 insertions(+), 392 deletions(-) diff --git a/nova/openstack/common/policy.py b/nova/openstack/common/policy.py index c15c0d5fe12b..01bc437af507 100644 --- a/nova/openstack/common/policy.py +++ b/nova/openstack/common/policy.py @@ -1,5 +1,3 @@ -# vim: tabstop=4 shiftwidth=4 softtabstop=4 - # Copyright (c) 2012 OpenStack Foundation. # All Rights Reserved. # @@ -48,6 +46,27 @@ policy rule:: project_id:%(project_id)s and not role:dunce +It is possible to perform policy checks on the following user +attributes (obtained through the token): user_id, domain_id or +project_id:: + + domain_id: + +Attributes sent along with API calls can be used by the policy engine +(on the right side of the expression), by using the following syntax:: + + :user.id + +Contextual attributes of objects identified by their IDs are loaded +from the database. They are also available to the policy engine and +can be checked through the `target` keyword:: + + :target.role.name + +All these attributes (related to users, API calls, and context) can be +checked against each other or against constants, be it literals (True, +) or strings. + Finally, two special policy checks should be mentioned; the policy check "@" will always accept an access, and the policy check "!" will always reject an access. (Note that if a rule is either the empty @@ -57,33 +76,51 @@ as it allows particular rules to be explicitly disabled. """ import abc +import ast import re -import urllib -import urllib2 +from oslo.config import cfg +import six +import six.moves.urllib.parse as urlparse +import six.moves.urllib.request as urlrequest -from nova.openstack.common.gettextutils import _ +from nova.openstack.common import fileutils +from nova.openstack.common.gettextutils import _, _LE from nova.openstack.common import jsonutils from nova.openstack.common import log as logging +policy_opts = [ + cfg.StrOpt('policy_file', + default='policy.json', + help=_('The JSON file that defines policies.')), + cfg.StrOpt('policy_default_rule', + default='default', + help=_('Default rule. Enforced when a requested rule is not ' + 'found.')), +] + +CONF = cfg.CONF +CONF.register_opts(policy_opts) + LOG = logging.getLogger(__name__) - -_rules = None _checks = {} +class PolicyNotAuthorized(Exception): + + def __init__(self, rule): + msg = _("Policy doesn't allow %s to be performed.") % rule + super(PolicyNotAuthorized, self).__init__(msg) + + class Rules(dict): - """ - A store for rules. Handles the default_rule setting directly. - """ + """A store for rules. Handles the default_rule setting directly.""" @classmethod def load_json(cls, data, default_rule=None): - """ - Allow loading of JSON rule data. - """ + """Allow loading of JSON rule data.""" # Suck in the JSON data and parse the rules rules = dict((k, parse_rule(v)) for k, v in @@ -100,12 +137,23 @@ class Rules(dict): def __missing__(self, key): """Implements the default rule handling.""" - # If the default rule isn't actually defined, do something - # reasonably intelligent - if not self.default_rule or self.default_rule not in self: + if isinstance(self.default_rule, dict): raise KeyError(key) - return self[self.default_rule] + # If the default rule isn't actually defined, do something + # reasonably intelligent + if not self.default_rule: + raise KeyError(key) + + if isinstance(self.default_rule, BaseCheck): + return self.default_rule + + # We need to check this or we can get infinite recursion + if self.default_rule not in self: + raise KeyError(key) + + elif isinstance(self.default_rule, six.string_types): + return self[self.default_rule] def __str__(self): """Dumps a string representation of the rules.""" @@ -123,87 +171,164 @@ class Rules(dict): return jsonutils.dumps(out_rules, indent=4) -# Really have to figure out a way to deprecate this -def set_rules(rules): - """Set the rules in use for policy checks.""" +class Enforcer(object): + """Responsible for loading and enforcing rules. - global _rules - - _rules = rules - - -# Ditto -def reset(): - """Clear the rules used for policy checks.""" - - global _rules - - _rules = None - - -def check(rule, target, creds, exc=None, *args, **kwargs): - """ - Checks authorization of a rule against the target and credentials. - - :param rule: The rule to evaluate. - :param target: As much information about the object being operated - on as possible, as a dictionary. - :param creds: As much information about the user performing the - action as possible, as a dictionary. - :param exc: Class of the exception to raise if the check fails. - Any remaining arguments passed to check() (both - positional and keyword arguments) will be passed to - the exception class. If exc is not provided, returns - False. - - :return: Returns False if the policy does not allow the action and - exc is not provided; otherwise, returns a value that - evaluates to True. Note: for rules using the "case" - expression, this True value will be the specified string - from the expression. + :param policy_file: Custom policy file to use, if none is + specified, `CONF.policy_file` will be + used. + :param rules: Default dictionary / Rules to use. It will be + considered just in the first instantiation. If + `load_rules(True)`, `clear()` or `set_rules(True)` + is called this will be overwritten. + :param default_rule: Default rule to use, CONF.default_rule will + be used if none is specified. + :param use_conf: Whether to load rules from cache or config file. """ - # Allow the rule to be a Check tree - if isinstance(rule, BaseCheck): - result = rule(target, creds) - elif not _rules: - # No rules to reference means we're going to fail closed - result = False - else: - try: - # Evaluate the rule - result = _rules[rule](target, creds) - except KeyError: - # If the rule doesn't exist, fail closed + def __init__(self, policy_file=None, rules=None, + default_rule=None, use_conf=True): + self.rules = Rules(rules, default_rule) + self.default_rule = default_rule or CONF.policy_default_rule + + self.policy_path = None + self.policy_file = policy_file or CONF.policy_file + self.use_conf = use_conf + + def set_rules(self, rules, overwrite=True, use_conf=False): + """Create a new Rules object based on the provided dict of rules. + + :param rules: New rules to use. It should be an instance of dict. + :param overwrite: Whether to overwrite current rules or update them + with the new rules. + :param use_conf: Whether to reload rules from cache or config file. + """ + + if not isinstance(rules, dict): + raise TypeError(_("Rules must be an instance of dict or Rules, " + "got %s instead") % type(rules)) + self.use_conf = use_conf + if overwrite: + self.rules = Rules(rules, self.default_rule) + else: + self.rules.update(rules) + + def clear(self): + """Clears Enforcer rules, policy's cache and policy's path.""" + self.set_rules({}) + self.default_rule = None + self.policy_path = None + + def load_rules(self, force_reload=False): + """Loads policy_path's rules. + + Policy file is cached and will be reloaded if modified. + + :param force_reload: Whether to overwrite current rules. + """ + + if force_reload: + self.use_conf = force_reload + + if self.use_conf: + if not self.policy_path: + self.policy_path = self._get_policy_path() + + reloaded, data = fileutils.read_cached_file( + self.policy_path, force_reload=force_reload) + if reloaded or not self.rules: + rules = Rules.load_json(data, self.default_rule) + self.set_rules(rules) + LOG.debug("Rules successfully reloaded") + + def _get_policy_path(self): + """Locate the policy json data file. + + :param policy_file: Custom policy file to locate. + + :returns: The policy path + + :raises: ConfigFilesNotFoundError if the file couldn't + be located. + """ + policy_file = CONF.find_file(self.policy_file) + + if policy_file: + return policy_file + + raise cfg.ConfigFilesNotFoundError((self.policy_file,)) + + def enforce(self, rule, target, creds, do_raise=False, + exc=None, *args, **kwargs): + """Checks authorization of a rule against the target and credentials. + + :param rule: A string or BaseCheck instance specifying the rule + to evaluate. + :param target: As much information about the object being operated + on as possible, as a dictionary. + :param creds: As much information about the user performing the + action as possible, as a dictionary. + :param do_raise: Whether to raise an exception or not if check + fails. + :param exc: Class of the exception to raise if the check fails. + Any remaining arguments passed to check() (both + positional and keyword arguments) will be passed to + the exception class. If not specified, PolicyNotAuthorized + will be used. + + :return: Returns False if the policy does not allow the action and + exc is not provided; otherwise, returns a value that + evaluates to True. Note: for rules using the "case" + expression, this True value will be the specified string + from the expression. + """ + + # NOTE(flaper87): Not logging target or creds to avoid + # potential security issues. + LOG.debug("Rule %s will be now enforced" % rule) + + self.load_rules() + + # Allow the rule to be a Check tree + if isinstance(rule, BaseCheck): + result = rule(target, creds, self) + elif not self.rules: + # No rules to reference means we're going to fail closed result = False + else: + try: + # Evaluate the rule + result = self.rules[rule](target, creds, self) + except KeyError: + LOG.debug("Rule [%s] doesn't exist" % rule) + # If the rule doesn't exist, fail closed + result = False - # If it is False, raise the exception if requested - if exc and result is False: - raise exc(*args, **kwargs) + # If it is False, raise the exception if requested + if do_raise and not result: + if exc: + raise exc(*args, **kwargs) - return result + raise PolicyNotAuthorized(rule) + + return result +@six.add_metaclass(abc.ABCMeta) class BaseCheck(object): - """ - Abstract base class for Check classes. - """ - - __metaclass__ = abc.ABCMeta + """Abstract base class for Check classes.""" @abc.abstractmethod def __str__(self): - """ - Retrieve a string representation of the Check tree rooted at - this node. - """ + """String representation of the Check tree rooted at this node.""" pass @abc.abstractmethod - def __call__(self, target, cred): - """ - Perform the check. Returns False to reject the access or a + def __call__(self, target, cred, enforcer): + """Triggers if instance of the class is called. + + Performs the check. Returns False to reject the access or a true value (not necessary True) to accept the access. """ @@ -211,44 +336,39 @@ class BaseCheck(object): class FalseCheck(BaseCheck): - """ - A policy check that always returns False (disallow). - """ + """A policy check that always returns False (disallow).""" def __str__(self): """Return a string representation of this check.""" return "!" - def __call__(self, target, cred): + def __call__(self, target, cred, enforcer): """Check the policy.""" return False class TrueCheck(BaseCheck): - """ - A policy check that always returns True (allow). - """ + """A policy check that always returns True (allow).""" def __str__(self): """Return a string representation of this check.""" return "@" - def __call__(self, target, cred): + def __call__(self, target, cred, enforcer): """Check the policy.""" return True class Check(BaseCheck): - """ - A base class to allow for user-defined policy checks. - """ + """A base class to allow for user-defined policy checks.""" def __init__(self, kind, match): - """ + """Initiates Check instance. + :param kind: The kind of the check, i.e., the field before the ':'. :param match: The match of the check, i.e., the field after @@ -265,14 +385,13 @@ class Check(BaseCheck): class NotCheck(BaseCheck): - """ + """Implements the "not" logical operator. + A policy check that inverts the result of another policy check. - Implements the "not" operator. """ def __init__(self, rule): - """ - Initialize the 'not' check. + """Initialize the 'not' check. :param rule: The rule to negate. Must be a Check. """ @@ -284,24 +403,23 @@ class NotCheck(BaseCheck): return "not %s" % self.rule - def __call__(self, target, cred): - """ - Check the policy. Returns the logical inverse of the wrapped - check. + def __call__(self, target, cred, enforcer): + """Check the policy. + + Returns the logical inverse of the wrapped check. """ - return not self.rule(target, cred) + return not self.rule(target, cred, enforcer) class AndCheck(BaseCheck): - """ - A policy check that requires that a list of other checks all - return True. Implements the "and" operator. + """Implements the "and" logical operator. + + A policy check that requires that a list of other checks all return True. """ def __init__(self, rules): - """ - Initialize the 'and' check. + """Initialize the 'and' check. :param rules: A list of rules that will be tested. """ @@ -313,20 +431,21 @@ class AndCheck(BaseCheck): return "(%s)" % ' and '.join(str(r) for r in self.rules) - def __call__(self, target, cred): - """ - Check the policy. Requires that all rules accept in order to - return True. + def __call__(self, target, cred, enforcer): + """Check the policy. + + Requires that all rules accept in order to return True. """ for rule in self.rules: - if not rule(target, cred): + if not rule(target, cred, enforcer): return False return True def add_check(self, rule): - """ + """Adds rule to be tested. + Allows addition of another rule to the list of rules that will be tested. Returns the AndCheck object for convenience. """ @@ -336,14 +455,14 @@ class AndCheck(BaseCheck): class OrCheck(BaseCheck): - """ + """Implements the "or" operator. + A policy check that requires that at least one of a list of other - checks returns True. Implements the "or" operator. + checks returns True. """ def __init__(self, rules): - """ - Initialize the 'or' check. + """Initialize the 'or' check. :param rules: A list of rules that will be tested. """ @@ -355,20 +474,20 @@ class OrCheck(BaseCheck): return "(%s)" % ' or '.join(str(r) for r in self.rules) - def __call__(self, target, cred): - """ - Check the policy. Requires that at least one rule accept in - order to return True. + def __call__(self, target, cred, enforcer): + """Check the policy. + + Requires that at least one rule accept in order to return True. """ for rule in self.rules: - if rule(target, cred): + if rule(target, cred, enforcer): return True - return False def add_check(self, rule): - """ + """Adds rule to be tested. + Allows addition of another rule to the list of rules that will be tested. Returns the OrCheck object for convenience. """ @@ -378,9 +497,7 @@ class OrCheck(BaseCheck): def _parse_check(rule): - """ - Parse a single base check rule into an appropriate Check object. - """ + """Parse a single base check rule into an appropriate Check object.""" # Handle the special checks if rule == '!': @@ -391,7 +508,7 @@ def _parse_check(rule): try: kind, match = rule.split(':', 1) except Exception: - LOG.exception(_("Failed to understand rule %(rule)s") % locals()) + LOG.exception(_LE("Failed to understand rule %s") % rule) # If the rule is invalid, we'll fail closed return FalseCheck() @@ -401,14 +518,14 @@ def _parse_check(rule): elif None in _checks: return _checks[None](kind, match) else: - LOG.error(_("No handler for matches of kind %s") % kind) + LOG.error(_LE("No handler for matches of kind %s") % kind) return FalseCheck() def _parse_list_rule(rule): - """ - Provided for backwards compatibility. Translates the old - list-of-lists syntax into a tree of Check objects. + """Translates the old list-of-lists syntax into a tree of Check objects. + + Provided for backwards compatibility. """ # Empty rule defaults to True @@ -423,7 +540,7 @@ def _parse_list_rule(rule): continue # Handle bare strings - if isinstance(inner_rule, basestring): + if isinstance(inner_rule, six.string_types): inner_rule = [inner_rule] # Parse the inner rules into Check objects @@ -436,7 +553,7 @@ def _parse_list_rule(rule): or_list.append(AndCheck(and_list)) # If we have only one check, omit the "or" - if len(or_list) == 0: + if not or_list: return FalseCheck() elif len(or_list) == 1: return or_list[0] @@ -449,8 +566,7 @@ _tokenize_re = re.compile(r'\s+') def _parse_tokenize(rule): - """ - Tokenizer for the policy language. + """Tokenizer for the policy language. Most of the single-character tokens are specified in the _tokenize_re; however, parentheses need to be handled specially, @@ -499,16 +615,16 @@ def _parse_tokenize(rule): class ParseStateMeta(type): - """ - Metaclass for the ParseState class. Facilitates identifying - reduction methods. + """Metaclass for the ParseState class. + + Facilitates identifying reduction methods. """ def __new__(mcs, name, bases, cls_dict): - """ - Create the class. Injects the 'reducers' list, a list of - tuples matching token sequences to the names of the - corresponding reduction methods. + """Create the class. + + Injects the 'reducers' list, a list of tuples matching token sequences + to the names of the corresponding reduction methods. """ reducers = [] @@ -525,10 +641,10 @@ class ParseStateMeta(type): def reducer(*tokens): - """ - Decorator for reduction methods. Arguments are a sequence of - tokens, in order, which should trigger running this reduction - method. + """Decorator for reduction methods. + + Arguments are a sequence of tokens, in order, which should trigger running + this reduction method. """ def decorator(func): @@ -544,11 +660,12 @@ def reducer(*tokens): return decorator +@six.add_metaclass(ParseStateMeta) class ParseState(object): - """ - Implement the core of parsing the policy language. Uses a greedy - reduction algorithm to reduce a sequence of tokens into a single - terminal, the value of which will be the root of the Check tree. + """Implement the core of parsing the policy language. + + Uses a greedy reduction algorithm to reduce a sequence of tokens into + a single terminal, the value of which will be the root of the Check tree. Note: error reporting is rather lacking. The best we can get with this parser formulation is an overall "parse failed" error. @@ -556,8 +673,6 @@ class ParseState(object): shouldn't be that big a problem. """ - __metaclass__ = ParseStateMeta - def __init__(self): """Initialize the ParseState.""" @@ -565,11 +680,11 @@ class ParseState(object): self.values = [] def reduce(self): - """ - Perform a greedy reduction of the token stream. If a reducer - method matches, it will be executed, then the reduce() method - will be called recursively to search for any more possible - reductions. + """Perform a greedy reduction of the token stream. + + If a reducer method matches, it will be executed, then the + reduce() method will be called recursively to search for any more + possible reductions. """ for reduction, methname in self.reducers: @@ -599,9 +714,9 @@ class ParseState(object): @property def result(self): - """ - Obtain the final result of the parse. Raises ValueError if - the parse failed to reduce to a single result. + """Obtain the final result of the parse. + + Raises ValueError if the parse failed to reduce to a single result. """ if len(self.values) != 1: @@ -618,35 +733,31 @@ class ParseState(object): @reducer('check', 'and', 'check') def _make_and_expr(self, check1, _and, check2): - """ - Create an 'and_expr' from two checks joined by the 'and' - operator. + """Create an 'and_expr'. + + Join two checks by the 'and' operator. """ return [('and_expr', AndCheck([check1, check2]))] @reducer('and_expr', 'and', 'check') def _extend_and_expr(self, and_expr, _and, check): - """ - Extend an 'and_expr' by adding one more check. - """ + """Extend an 'and_expr' by adding one more check.""" return [('and_expr', and_expr.add_check(check))] @reducer('check', 'or', 'check') def _make_or_expr(self, check1, _or, check2): - """ - Create an 'or_expr' from two checks joined by the 'or' - operator. + """Create an 'or_expr'. + + Join two checks by the 'or' operator. """ return [('or_expr', OrCheck([check1, check2]))] @reducer('or_expr', 'or', 'check') def _extend_or_expr(self, or_expr, _or, check): - """ - Extend an 'or_expr' by adding one more check. - """ + """Extend an 'or_expr' by adding one more check.""" return [('or_expr', or_expr.add_check(check))] @@ -658,7 +769,8 @@ class ParseState(object): def _parse_text_rule(rule): - """ + """Parses policy to the tree. + Translates a policy written in the policy language into a tree of Check objects. """ @@ -676,26 +788,23 @@ def _parse_text_rule(rule): return state.result except ValueError: # Couldn't parse the rule - LOG.exception(_("Failed to understand rule %(rule)r") % locals()) + LOG.exception(_LE("Failed to understand rule %r") % rule) # Fail closed return FalseCheck() def parse_rule(rule): - """ - Parses a policy rule into a tree of Check objects. - """ + """Parses a policy rule into a tree of Check objects.""" # If the rule is a string, it's in the policy language - if isinstance(rule, basestring): + if isinstance(rule, six.string_types): return _parse_text_rule(rule) return _parse_list_rule(rule) def register(name, func=None): - """ - Register a function or Check class as a policy check. + """Register a function or Check class as a policy check. :param name: Gives the name of the check type, e.g., 'rule', 'role', etc. If name is None, a default check type @@ -722,13 +831,11 @@ def register(name, func=None): @register("rule") class RuleCheck(Check): - def __call__(self, target, creds): - """ - Recursively checks credentials based on the defined rules. - """ + def __call__(self, target, creds, enforcer): + """Recursively checks credentials based on the defined rules.""" try: - return _rules[self.match](target, creds) + return enforcer.rules[self.match](target, creds, enforcer) except KeyError: # We don't have any matching rule; fail closed return False @@ -736,7 +843,7 @@ class RuleCheck(Check): @register("role") class RoleCheck(Check): - def __call__(self, target, creds): + def __call__(self, target, creds, enforcer): """Check that there is a matching role in the cred dict.""" return self.match.lower() in [x.lower() for x in creds['roles']] @@ -744,9 +851,8 @@ class RoleCheck(Check): @register('http') class HttpCheck(Check): - def __call__(self, target, creds): - """ - Check http: rules by calling to a remote server. + def __call__(self, target, creds, enforcer): + """Check http: rules by calling to a remote server. This example implementation simply verifies that the response is exactly 'True'. @@ -755,25 +861,38 @@ class HttpCheck(Check): url = ('http:' + self.match) % target data = {'target': jsonutils.dumps(target), 'credentials': jsonutils.dumps(creds)} - post_data = urllib.urlencode(data) - f = urllib2.urlopen(url, post_data) + post_data = urlparse.urlencode(data) + f = urlrequest.urlopen(url, post_data) return f.read() == "True" @register(None) class GenericCheck(Check): - def __call__(self, target, creds): - """ - Check an individual match. + def __call__(self, target, creds, enforcer): + """Check an individual match. Matches look like: tenant:%(tenant_id)s role:compute:admin + True:%(user.enabled)s + 'Member':%(role.name)s """ # TODO(termie): do dict inspection via dot syntax - match = self.match % target - if self.kind in creds: - return match == unicode(creds[self.kind]) - return False + try: + match = self.match % target + except KeyError: + # While doing GenericCheck if key not + # present in Target return false + return False + + try: + # Try to interpret self.kind as a literal + leftval = ast.literal_eval(self.kind) + except ValueError: + try: + leftval = creds[self.kind] + except KeyError: + return False + return match == six.text_type(leftval) diff --git a/nova/policy.py b/nova/policy.py index a19370958c1b..124efe85d04a 100644 --- a/nova/policy.py +++ b/nova/policy.py @@ -15,59 +15,54 @@ """Policy Engine For Nova.""" -import os.path - -from oslo.config import cfg - from nova import exception -from nova.openstack.common.gettextutils import _ from nova.openstack.common import policy -from nova import utils -policy_opts = [ - cfg.StrOpt('policy_file', - default='policy.json', - help=_('JSON file representing policy')), - cfg.StrOpt('policy_default_rule', - default='default', - help=_('Rule checked when requested rule is not found')), - ] - -CONF = cfg.CONF -CONF.register_opts(policy_opts) - -_POLICY_PATH = None -_POLICY_CACHE = {} +_ENFORCER = None def reset(): - global _POLICY_PATH - global _POLICY_CACHE - _POLICY_PATH = None - _POLICY_CACHE = {} - policy.reset() + global _ENFORCER + if _ENFORCER: + _ENFORCER.clear() + _ENFORCER = None -def init(): - global _POLICY_PATH - global _POLICY_CACHE - if not _POLICY_PATH: - _POLICY_PATH = CONF.policy_file - if not os.path.exists(_POLICY_PATH): - _POLICY_PATH = CONF.find_file(_POLICY_PATH) - if not _POLICY_PATH: - raise exception.ConfigNotFound(path=CONF.policy_file) - utils.read_cached_file(_POLICY_PATH, _POLICY_CACHE, - reload_func=_set_rules) +def init(policy_file=None, rules=None, default_rule=None, use_conf=True): + """Init an Enforcer class. + + :param policy_file: Custom policy file to use, if none is specified, + `CONF.policy_file` will be used. + :param rules: Default dictionary / Rules to use. It will be + considered just in the first instantiation. + :param default_rule: Default rule to use, CONF.default_rule will + be used if none is specified. + :param use_conf: Whether to load rules from config file. + """ + + global _ENFORCER + if not _ENFORCER: + _ENFORCER = policy.Enforcer(policy_file=policy_file, + rules=rules, + default_rule=default_rule, + use_conf=use_conf) -def _set_rules(data): - default_rule = CONF.policy_default_rule - policy.set_rules(policy.Rules.load_json(data, default_rule)) +def set_rules(rules, overwrite=True, use_conf=False): + """Set rules based on the provided dict of rules. + + :param rules: New rules to use. It should be an instance of dict. + :param overwrite: Whether to overwrite current rules or update them + with the new rules. + :param use_conf: Whether to reload rules from config file. + """ + + init(use_conf=False) + _ENFORCER.set_rules(rules, overwrite, use_conf) -def enforce(context, action, target, do_raise=True): +def enforce(context, action, target, do_raise=True, exc=None): """Verifies that the action is valid on the target in this context. :param context: nova context @@ -90,28 +85,23 @@ def enforce(context, action, target, do_raise=True): do_raise is False. """ init() - credentials = context.to_dict() - - # Add the exception arguments if asked to do a raise - extra = {} - if do_raise: - extra.update(exc=exception.PolicyNotAuthorized, action=action) - - return policy.check(action, target, credentials, **extra) + if not exc: + exc = exception.PolicyNotAuthorized + return _ENFORCER.enforce(action, target, credentials, do_raise=do_raise, + exc=exc, action=action) def check_is_admin(context): """Whether or not roles contains 'admin' role according to policy setting. """ - init() - #the target is user-self + init() + # the target is user-self credentials = context.to_dict() target = credentials - - return policy.check('context_is_admin', target, credentials) + return _ENFORCER.enforce('context_is_admin', target, credentials) @policy.register('is_admin') @@ -125,11 +115,12 @@ class IsAdminCheck(policy.Check): super(IsAdminCheck, self).__init__(kind, str(self.expected)) - def __call__(self, target, creds): + def __call__(self, target, creds, enforcer): """Determine whether is_admin matches the requested value.""" return creds['is_admin'] == self.expected def get_rules(): - return policy._rules + if _ENFORCER: + return _ENFORCER.rules diff --git a/nova/tests/api/ec2/test_cloud.py b/nova/tests/api/ec2/test_cloud.py index d373f9e604eb..e6e288c1a1fd 100644 --- a/nova/tests/api/ec2/test_cloud.py +++ b/nova/tests/api/ec2/test_cloud.py @@ -54,6 +54,7 @@ from nova.openstack.common import log as logging from nova.openstack.common import policy as common_policy from nova.openstack.common import timeutils from nova.openstack.common import uuidutils +from nova import policy from nova import test from nova.tests.api.openstack.compute.contrib import ( test_neutron_security_groups as test_neutron) @@ -499,10 +500,9 @@ class CloudTestCase(test.TestCase): self.assertRaises(exception.MissingParameter, delete, self.context) def test_delete_security_group_policy_not_allowed(self): - rules = common_policy.Rules( - {'compute_extension:security_groups': - common_policy.parse_rule('project_id:%(project_id)s')}) - common_policy.set_rules(rules) + rules = {'compute_extension:security_groups': + common_policy.parse_rule('project_id:%(project_id)s')} + policy.set_rules(rules) with mock.patch.object(self.cloud.security_group_api, 'get') as get: @@ -513,10 +513,9 @@ class CloudTestCase(test.TestCase): 'fake-name', 'fake-id') def test_authorize_security_group_ingress_policy_not_allowed(self): - rules = common_policy.Rules( - {'compute_extension:security_groups': - common_policy.parse_rule('project_id:%(project_id)s')}) - common_policy.set_rules(rules) + rules = {'compute_extension:security_groups': + common_policy.parse_rule('project_id:%(project_id)s')} + policy.set_rules(rules) with mock.patch.object(self.cloud.security_group_api, 'get') as get: @@ -647,10 +646,9 @@ class CloudTestCase(test.TestCase): db.security_group_destroy(self.context, sec1['id']) def test_revoke_security_group_ingress_policy_not_allowed(self): - rules = common_policy.Rules( - {'compute_extension:security_groups': - common_policy.parse_rule('project_id:%(project_id)s')}) - common_policy.set_rules(rules) + rules = {'compute_extension:security_groups': + common_policy.parse_rule('project_id:%(project_id)s')} + policy.set_rules(rules) with mock.patch.object(self.cloud.security_group_api, 'get') as get: @@ -2277,7 +2275,7 @@ class CloudTestCase(test.TestCase): "compute:start": common_policy.parse_rule("project_id:non_fake"), } - common_policy.set_rules(common_policy.Rules(rules)) + policy.set_rules(rules) exc = self.assertRaises(exception.PolicyNotAuthorized, self.cloud.start_instances, self.context, [instance_id]) @@ -2312,7 +2310,7 @@ class CloudTestCase(test.TestCase): "compute:stop": common_policy.parse_rule("project_id:non_fake") } - common_policy.set_rules(common_policy.Rules(rules)) + policy.set_rules(rules) exc = self.assertRaises(exception.PolicyNotAuthorized, self.cloud.stop_instances, self.context, [instance_id]) diff --git a/nova/tests/api/openstack/compute/contrib/test_instance_actions.py b/nova/tests/api/openstack/compute/contrib/test_instance_actions.py index 1a85fedc9eb4..0c4f0c155858 100644 --- a/nova/tests/api/openstack/compute/contrib/test_instance_actions.py +++ b/nova/tests/api/openstack/compute/contrib/test_instance_actions.py @@ -24,7 +24,8 @@ from nova.compute import api as compute_api from nova import db from nova.db.sqlalchemy import models from nova import exception -from nova.openstack.common import policy +from nova.openstack.common import policy as common_policy +from nova import policy from nova import test from nova.tests.api.openstack import fakes from nova.tests import fake_instance @@ -71,9 +72,9 @@ class InstanceActionsPolicyTest(test.NoDBTestCase): self.controller = instance_actions.InstanceActionsController() def test_list_actions_restricted_by_project(self): - rules = policy.Rules({'compute:get': policy.parse_rule(''), - 'compute_extension:instance_actions': - policy.parse_rule('project_id:%(project_id)s')}) + rules = {'compute:get': common_policy.parse_rule(''), + 'compute_extension:instance_actions': + common_policy.parse_rule('project_id:%(project_id)s')} policy.set_rules(rules) def fake_instance_get_by_uuid(context, instance_id, @@ -89,9 +90,9 @@ class InstanceActionsPolicyTest(test.NoDBTestCase): str(uuid.uuid4())) def test_get_action_restricted_by_project(self): - rules = policy.Rules({'compute:get': policy.parse_rule(''), - 'compute_extension:instance_actions': - policy.parse_rule('project_id:%(project_id)s')}) + rules = {'compute:get': common_policy.parse_rule(''), + 'compute_extension:instance_actions': + common_policy.parse_rule('project_id:%(project_id)s')} policy.set_rules(rules) def fake_instance_get_by_uuid(context, instance_id, @@ -176,11 +177,11 @@ class InstanceActionsTest(test.NoDBTestCase): self.stubs.Set(db, 'action_get_by_request_id', fake_get_action) self.stubs.Set(db, 'action_events_get', fake_get_events) - rules = policy.Rules({'compute:get': policy.parse_rule(''), - 'compute_extension:instance_actions': - policy.parse_rule(''), - 'compute_extension:instance_actions:events': - policy.parse_rule('is_admin:True')}) + rules = {'compute:get': common_policy.parse_rule(''), + 'compute_extension:instance_actions': + common_policy.parse_rule(''), + 'compute_extension:instance_actions:events': + common_policy.parse_rule('is_admin:True')} policy.set_rules(rules) req = fakes.HTTPRequest.blank( '/v2/123/servers/12/os-instance-actions/1') diff --git a/nova/tests/api/openstack/compute/contrib/test_keypairs.py b/nova/tests/api/openstack/compute/contrib/test_keypairs.py index 7434ddd69716..dd1851f05620 100644 --- a/nova/tests/api/openstack/compute/contrib/test_keypairs.py +++ b/nova/tests/api/openstack/compute/contrib/test_keypairs.py @@ -21,7 +21,8 @@ from nova.api.openstack import wsgi from nova import db from nova import exception from nova.openstack.common import jsonutils -from nova.openstack.common import policy +from nova.openstack.common import policy as common_policy +from nova import policy from nova import quota from nova import test from nova.tests.api.openstack import fakes @@ -380,8 +381,8 @@ class KeypairPolicyTest(test.TestCase): db_key_pair_destroy) def test_keypair_list_fail_policy(self): - rules = policy.Rules({'compute_extension:keypairs:index': - policy.parse_rule('role:admin')}) + rules = {'compute_extension:keypairs:index': + common_policy.parse_rule('role:admin')} policy.set_rules(rules) req = fakes.HTTPRequest.blank('/v2/fake/os-keypairs') self.assertRaises(exception.Forbidden, @@ -389,16 +390,16 @@ class KeypairPolicyTest(test.TestCase): req) def test_keypair_list_pass_policy(self): - rules = policy.Rules({'compute_extension:keypairs:index': - policy.parse_rule('')}) + rules = {'compute_extension:keypairs:index': + common_policy.parse_rule('')} policy.set_rules(rules) req = fakes.HTTPRequest.blank('/v2/fake/os-keypairs') res = self.KeyPairController.index(req) self.assertIn('keypairs', res) def test_keypair_show_fail_policy(self): - rules = policy.Rules({'compute_extension:keypairs:show': - policy.parse_rule('role:admin')}) + rules = {'compute_extension:keypairs:show': + common_policy.parse_rule('role:admin')} policy.set_rules(rules) req = fakes.HTTPRequest.blank('/v2/fake/os-keypairs/FAKE') self.assertRaises(exception.Forbidden, @@ -406,16 +407,16 @@ class KeypairPolicyTest(test.TestCase): req, 'FAKE') def test_keypair_show_pass_policy(self): - rules = policy.Rules({'compute_extension:keypairs:show': - policy.parse_rule('')}) + rules = {'compute_extension:keypairs:show': + common_policy.parse_rule('')} policy.set_rules(rules) req = fakes.HTTPRequest.blank('/v2/fake/os-keypairs/FAKE') res = self.KeyPairController.show(req, 'FAKE') self.assertIn('keypair', res) def test_keypair_create_fail_policy(self): - rules = policy.Rules({'compute_extension:keypairs:create': - policy.parse_rule('role:admin')}) + rules = {'compute_extension:keypairs:create': + common_policy.parse_rule('role:admin')} policy.set_rules(rules) req = fakes.HTTPRequest.blank('/v2/fake/os-keypairs') req.method = 'POST' @@ -425,8 +426,8 @@ class KeypairPolicyTest(test.TestCase): def test_keypair_create_pass_policy(self): body = {'keypair': {'name': 'create_test'}} - rules = policy.Rules({'compute_extension:keypairs:create': - policy.parse_rule('')}) + rules = {'compute_extension:keypairs:create': + common_policy.parse_rule('')} policy.set_rules(rules) req = fakes.HTTPRequest.blank('/v2/fake/os-keypairs') req.method = 'POST' @@ -434,8 +435,8 @@ class KeypairPolicyTest(test.TestCase): self.assertIn('keypair', res) def test_keypair_delete_fail_policy(self): - rules = policy.Rules({'compute_extension:keypairs:delete': - policy.parse_rule('role:admin')}) + rules = {'compute_extension:keypairs:delete': + common_policy.parse_rule('role:admin')} policy.set_rules(rules) req = fakes.HTTPRequest.blank('/v2/fake/os-keypairs/FAKE') req.method = 'DELETE' @@ -444,8 +445,8 @@ class KeypairPolicyTest(test.TestCase): req, 'FAKE') def test_keypair_delete_pass_policy(self): - rules = policy.Rules({'compute_extension:keypairs:delete': - policy.parse_rule('')}) + rules = {'compute_extension:keypairs:delete': + common_policy.parse_rule('')} policy.set_rules(rules) req = fakes.HTTPRequest.blank('/v2/fake/os-keypairs/FAKE') req.method = 'DELETE' diff --git a/nova/tests/api/openstack/compute/contrib/test_server_start_stop.py b/nova/tests/api/openstack/compute/contrib/test_server_start_stop.py index 1d6177df3bbc..bc5b132258bf 100644 --- a/nova/tests/api/openstack/compute/contrib/test_server_start_stop.py +++ b/nova/tests/api/openstack/compute/contrib/test_server_start_stop.py @@ -20,6 +20,7 @@ from nova.compute import api as compute_api from nova import db from nova import exception from nova.openstack.common import policy as common_policy +from nova import policy from nova import test from nova.tests.api.openstack import fakes @@ -69,7 +70,7 @@ class ServerStartStopTest(test.TestCase): "compute:start": common_policy.parse_rule("project_id:non_fake") } - common_policy.set_rules(common_policy.Rules(rules)) + policy.set_rules(rules) self.stubs.Set(db, 'instance_get_by_uuid', fake_instance_get) req = fakes.HTTPRequest.blank('/v2/fake/servers/test_inst/action') body = dict(start="") @@ -117,7 +118,7 @@ class ServerStartStopTest(test.TestCase): "compute:stop": common_policy.parse_rule("project_id:non_fake") } - common_policy.set_rules(common_policy.Rules(rules)) + policy.set_rules(rules) self.stubs.Set(db, 'instance_get_by_uuid', fake_instance_get) req = fakes.HTTPRequest.blank('/v2/fake/servers/test_inst/action') body = dict(stop="") diff --git a/nova/tests/api/openstack/compute/contrib/test_shelve.py b/nova/tests/api/openstack/compute/contrib/test_shelve.py index 40bedf2309e0..003dc5a563d6 100644 --- a/nova/tests/api/openstack/compute/contrib/test_shelve.py +++ b/nova/tests/api/openstack/compute/contrib/test_shelve.py @@ -19,7 +19,8 @@ from nova.api.openstack.compute.contrib import shelve from nova.compute import api as compute_api from nova import db from nova import exception -from nova.openstack.common import policy +from nova.openstack.common import policy as common_policy +from nova import policy from nova import test from nova.tests.api.openstack import fakes from nova.tests import fake_instance @@ -41,8 +42,8 @@ class ShelvePolicyTest(test.NoDBTestCase): self.controller = shelve.ShelveController() def test_shelve_restricted_by_role(self): - rules = policy.Rules({'compute_extension:shelve': - policy.parse_rule('role:admin')}) + rules = {'compute_extension:shelve': + common_policy.parse_rule('role:admin')} policy.set_rules(rules) req = fakes.HTTPRequest.blank('/v2/123/servers/12/os-shelve') @@ -50,9 +51,9 @@ class ShelvePolicyTest(test.NoDBTestCase): req, str(uuid.uuid4()), {}) def test_shelve_allowed(self): - rules = policy.Rules({'compute:get': policy.parse_rule(''), - 'compute_extension:shelve': - policy.parse_rule('')}) + rules = {'compute:get': common_policy.parse_rule(''), + 'compute_extension:shelve': + common_policy.parse_rule('')} policy.set_rules(rules) self.stubs.Set(db, 'instance_get_by_uuid', fake_instance_get_by_uuid) @@ -70,8 +71,8 @@ class ShelvePolicyTest(test.NoDBTestCase): req, str(uuid.uuid4()), {}) def test_unshelve_restricted_by_role(self): - rules = policy.Rules({'compute_extension:unshelve': - policy.parse_rule('role:admin')}) + rules = {'compute_extension:unshelve': + common_policy.parse_rule('role:admin')} policy.set_rules(rules) req = fakes.HTTPRequest.blank('/v2/123/servers/12/os-shelve') @@ -79,9 +80,8 @@ class ShelvePolicyTest(test.NoDBTestCase): req, str(uuid.uuid4()), {}) def test_unshelve_allowed(self): - rules = policy.Rules({'compute:get': policy.parse_rule(''), - 'compute_extension:unshelve': - policy.parse_rule('')}) + rules = {'compute:get': common_policy.parse_rule(''), + 'compute_extension:unshelve': common_policy.parse_rule('')} policy.set_rules(rules) self.stubs.Set(db, 'instance_get_by_uuid', fake_instance_get_by_uuid) @@ -99,8 +99,8 @@ class ShelvePolicyTest(test.NoDBTestCase): req, str(uuid.uuid4()), {}) def test_shelve_offload_restricted_by_role(self): - rules = policy.Rules({'compute_extension:shelveOffload': - policy.parse_rule('role:admin')}) + rules = {'compute_extension:shelveOffload': + common_policy.parse_rule('role:admin')} policy.set_rules(rules) req = fakes.HTTPRequest.blank('/v2/123/servers/12/os-shelve') @@ -108,9 +108,9 @@ class ShelvePolicyTest(test.NoDBTestCase): self.controller._shelve_offload, req, str(uuid.uuid4()), {}) def test_shelve_offload_allowed(self): - rules = policy.Rules({'compute:get': policy.parse_rule(''), - 'compute_extension:shelveOffload': - policy.parse_rule('')}) + rules = {'compute:get': common_policy.parse_rule(''), + 'compute_extension:shelveOffload': + common_policy.parse_rule('')} policy.set_rules(rules) self.stubs.Set(db, 'instance_get_by_uuid', fake_instance_get_by_uuid) diff --git a/nova/tests/api/openstack/compute/contrib/test_simple_tenant_usage.py b/nova/tests/api/openstack/compute/contrib/test_simple_tenant_usage.py index 8458c7e53382..ac3209396c4f 100644 --- a/nova/tests/api/openstack/compute/contrib/test_simple_tenant_usage.py +++ b/nova/tests/api/openstack/compute/contrib/test_simple_tenant_usage.py @@ -232,7 +232,7 @@ class SimpleTenantUsageTest(test.TestCase): ["role:admin"], ["project_id:%(project_id)s"] ]) } - common_policy.set_rules(common_policy.Rules(rules)) + policy.set_rules(rules) try: res = req.get_response(fakes.wsgi_app( diff --git a/nova/tests/api/openstack/compute/plugins/v3/test_certificates.py b/nova/tests/api/openstack/compute/plugins/v3/test_certificates.py index fb0f6cf539d4..8f7c55c37add 100644 --- a/nova/tests/api/openstack/compute/plugins/v3/test_certificates.py +++ b/nova/tests/api/openstack/compute/plugins/v3/test_certificates.py @@ -20,6 +20,7 @@ from nova.api.openstack.compute.plugins.v3 import certificates from nova import context from nova import exception from nova.openstack.common import policy as common_policy +from nova import policy from nova import test from nova.tests.api.openstack import fakes @@ -55,7 +56,7 @@ class CertificatesTest(test.NoDBTestCase): "compute_extension:v3:os-certificates:show": common_policy.parse_rule("!") } - common_policy.set_rules(common_policy.Rules(rules)) + policy.set_rules(rules) req = fakes.HTTPRequestV3.blank('/os-certificates/root') exc = self.assertRaises(exception.PolicyNotAuthorized, self.controller.show, req, 'root') @@ -87,7 +88,7 @@ class CertificatesTest(test.NoDBTestCase): "compute_extension:v3:os-certificates:create": common_policy.parse_rule("!") } - common_policy.set_rules(common_policy.Rules(rules)) + policy.set_rules(rules) req = fakes.HTTPRequestV3.blank('/os-certificates/') exc = self.assertRaises(exception.PolicyNotAuthorized, self.controller.create, req) diff --git a/nova/tests/api/openstack/compute/plugins/v3/test_instance_actions.py b/nova/tests/api/openstack/compute/plugins/v3/test_instance_actions.py index 8cdb649299fb..29ed96a32b68 100644 --- a/nova/tests/api/openstack/compute/plugins/v3/test_instance_actions.py +++ b/nova/tests/api/openstack/compute/plugins/v3/test_instance_actions.py @@ -23,7 +23,8 @@ from nova.compute import api as compute_api from nova import db from nova.db.sqlalchemy import models from nova import exception -from nova.openstack.common import policy +from nova.openstack.common import policy as common_policy +from nova import policy from nova import test from nova.tests.api.openstack import fakes from nova.tests import fake_instance @@ -76,9 +77,9 @@ class ServerActionsPolicyTest(test.NoDBTestCase): self.controller = server_actions.ServerActionsController() def test_list_actions_restricted_by_project(self): - rules = policy.Rules({'compute:get': policy.parse_rule(''), - 'compute_extension:v3:os-server-actions': - policy.parse_rule('project_id:%(project_id)s')}) + rules = {'compute:get': common_policy.parse_rule(''), + 'compute_extension:v3:os-server-actions': + common_policy.parse_rule('project_id:%(project_id)s')} policy.set_rules(rules) def fake_instance_get_by_uuid(context, instance_id, @@ -93,9 +94,9 @@ class ServerActionsPolicyTest(test.NoDBTestCase): str(uuid.uuid4())) def test_get_action_restricted_by_project(self): - rules = policy.Rules({'compute:get': policy.parse_rule(''), - 'compute_extension:v3:os-server-actions': - policy.parse_rule('project_id:%(project_id)s')}) + rules = {'compute:get': common_policy.parse_rule(''), + 'compute_extension:v3:os-server-actions': + common_policy.parse_rule('project_id:%(project_id)s')} policy.set_rules(rules) def fake_instance_get_by_uuid(context, instance_id, @@ -183,12 +184,11 @@ class ServerActionsTest(test.NoDBTestCase): self.stubs.Set(db, 'action_get_by_request_id', fake_get_action) self.stubs.Set(db, 'action_events_get', fake_get_events) - rules = policy.Rules({ - 'compute:get': policy.parse_rule(''), - 'compute_extension:v3:os-server-actions': - policy.parse_rule(''), - 'compute_extension:v3:os-server-actions:events': - policy.parse_rule('is_admin:True')}) + rules = {'compute:get': common_policy.parse_rule(''), + 'compute_extension:v3:os-server-actions': + common_policy.parse_rule(''), + 'compute_extension:v3:os-server-actions:events': + common_policy.parse_rule('is_admin:True')} policy.set_rules(rules) req = fakes.HTTPRequestV3.blank( '/servers/12/os-server-actions/1') diff --git a/nova/tests/api/openstack/compute/plugins/v3/test_keypairs.py b/nova/tests/api/openstack/compute/plugins/v3/test_keypairs.py index 5bf2d48abf5e..b926d175e8b4 100644 --- a/nova/tests/api/openstack/compute/plugins/v3/test_keypairs.py +++ b/nova/tests/api/openstack/compute/plugins/v3/test_keypairs.py @@ -19,7 +19,8 @@ from nova.api.openstack.compute.plugins.v3 import keypairs from nova import db from nova import exception from nova.openstack.common import jsonutils -from nova.openstack.common import policy +from nova.openstack.common import policy as common_policy +from nova import policy from nova import quota from nova import test from nova.tests.api.openstack import fakes @@ -399,8 +400,8 @@ class KeypairPolicyTest(test.TestCase): db_key_pair_destroy) def test_keypair_list_fail_policy(self): - rules = policy.Rules({'compute_extension:v3:keypairs:index': - policy.parse_rule('role:admin')}) + rules = {'compute_extension:v3:keypairs:index': + common_policy.parse_rule('role:admin')} policy.set_rules(rules) req = fakes.HTTPRequestV3.blank('/keypairs') self.assertRaises(exception.Forbidden, @@ -408,16 +409,16 @@ class KeypairPolicyTest(test.TestCase): req) def test_keypair_list_pass_policy(self): - rules = policy.Rules({'compute_extension:v3:keypairs:index': - policy.parse_rule('')}) + rules = {'compute_extension:v3:keypairs:index': + common_policy.parse_rule('')} policy.set_rules(rules) req = fakes.HTTPRequestV3.blank('/keypairs') res = self.KeyPairController.index(req) self.assertIn('keypairs', res) def test_keypair_show_fail_policy(self): - rules = policy.Rules({'compute_extension:v3:keypairs:show': - policy.parse_rule('role:admin')}) + rules = {'compute_extension:v3:keypairs:show': + common_policy.parse_rule('role:admin')} policy.set_rules(rules) req = fakes.HTTPRequestV3.blank('/keypairs/FAKE') self.assertRaises(exception.Forbidden, @@ -425,16 +426,16 @@ class KeypairPolicyTest(test.TestCase): req, 'FAKE') def test_keypair_show_pass_policy(self): - rules = policy.Rules({'compute_extension:v3:keypairs:show': - policy.parse_rule('')}) + rules = {'compute_extension:v3:keypairs:show': + common_policy.parse_rule('')} policy.set_rules(rules) req = fakes.HTTPRequestV3.blank('/keypairs/FAKE') res = self.KeyPairController.show(req, 'FAKE') self.assertIn('keypair', res) def test_keypair_create_fail_policy(self): - rules = policy.Rules({'compute_extension:v3:keypairs:create': - policy.parse_rule('role:admin')}) + rules = {'compute_extension:v3:keypairs:create': + common_policy.parse_rule('role:admin')} policy.set_rules(rules) req = fakes.HTTPRequestV3.blank('/keypairs') req.method = 'POST' @@ -444,8 +445,8 @@ class KeypairPolicyTest(test.TestCase): def test_keypair_create_pass_policy(self): body = {'keypair': {'name': 'create_test'}} - rules = policy.Rules({'compute_extension:v3:keypairs:create': - policy.parse_rule('')}) + rules = {'compute_extension:v3:keypairs:create': + common_policy.parse_rule('')} policy.set_rules(rules) req = fakes.HTTPRequestV3.blank('/keypairs') req.method = 'POST' @@ -453,8 +454,8 @@ class KeypairPolicyTest(test.TestCase): self.assertIn('keypair', res) def test_keypair_delete_fail_policy(self): - rules = policy.Rules({'compute_extension:v3:keypairs:delete': - policy.parse_rule('role:admin')}) + rules = {'compute_extension:v3:keypairs:delete': + common_policy.parse_rule('role:admin')} policy.set_rules(rules) req = fakes.HTTPRequestV3.blank('/keypairs/FAKE') req.method = 'DELETE' @@ -463,8 +464,8 @@ class KeypairPolicyTest(test.TestCase): req, 'FAKE') def test_keypair_delete_pass_policy(self): - rules = policy.Rules({'compute_extension:v3:keypairs:delete': - policy.parse_rule('')}) + rules = {'compute_extension:v3:keypairs:delete': + common_policy.parse_rule('')} policy.set_rules(rules) req = fakes.HTTPRequestV3.blank('/keypairs/FAKE') req.method = 'DELETE' diff --git a/nova/tests/api/openstack/compute/plugins/v3/test_servers.py b/nova/tests/api/openstack/compute/plugins/v3/test_servers.py index 0a09ddf57399..ccbb9e5d6b7c 100644 --- a/nova/tests/api/openstack/compute/plugins/v3/test_servers.py +++ b/nova/tests/api/openstack/compute/plugins/v3/test_servers.py @@ -864,7 +864,7 @@ class ServersControllerTest(ControllerTest): common_policy.parse_rule("project_id:fake"), } - common_policy.set_rules(common_policy.Rules(rules)) + policy.set_rules(rules) req = fakes.HTTPRequestV3.blank('/servers?all_tenants=1') res = self.controller.index(req) @@ -885,7 +885,7 @@ class ServersControllerTest(ControllerTest): common_policy.parse_rule("project_id:fake"), } - common_policy.set_rules(common_policy.Rules(rules)) + policy.set_rules(rules) self.stubs.Set(db, 'instance_get_all_by_filters', fake_get_all) @@ -1492,7 +1492,7 @@ class ServersControllerRebuildInstanceTest(ControllerTest): "compute:v3:servers:start": common_policy.parse_rule("project_id:non_fake") } - common_policy.set_rules(common_policy.Rules(rules)) + policy.set_rules(rules) req = fakes.HTTPRequestV3.blank('/servers/%s/action' % FAKE_UUID) body = dict(start="") exc = self.assertRaises(exception.PolicyNotAuthorized, @@ -1536,7 +1536,7 @@ class ServersControllerRebuildInstanceTest(ControllerTest): "compute:v3:servers:stop": common_policy.parse_rule("project_id:non_fake") } - common_policy.set_rules(common_policy.Rules(rules)) + policy.set_rules(rules) req = fakes.HTTPRequestV3.blank('/servers/%s/action' % FAKE_UUID) body = dict(stop='') exc = self.assertRaises(exception.PolicyNotAuthorized, @@ -1678,7 +1678,7 @@ class ServersControllerUpdateTest(ControllerTest): def test_update_server_policy_fail(self): rule = {'compute:update': common_policy.parse_rule('role:admin')} - common_policy.set_rules(common_policy.Rules(rule)) + policy.set_rules(rule) body = {'server': {'name': 'server_test'}} req = self._get_request(body, {'name': 'server_test'}) self.assertRaises(exception.PolicyNotAuthorized, @@ -1724,7 +1724,7 @@ class ServerStatusTest(test.TestCase): rule = {'compute:reboot': common_policy.parse_rule('role:admin')} - common_policy.set_rules(common_policy.Rules(rule)) + policy.set_rules(rule) req = fakes.HTTPRequestV3.blank('/servers/1234/action') self.assertRaises(exception.PolicyNotAuthorized, self.controller._action_reboot, req, '1234', @@ -1752,7 +1752,7 @@ class ServerStatusTest(test.TestCase): rule = {'compute:confirm_resize': common_policy.parse_rule('role:admin')} - common_policy.set_rules(common_policy.Rules(rule)) + policy.set_rules(rule) req = fakes.HTTPRequestV3.blank('/servers/1234/action') self.assertRaises(exception.PolicyNotAuthorized, self.controller._action_confirm_resize, req, '1234', {}) @@ -1774,7 +1774,7 @@ class ServerStatusTest(test.TestCase): rule = {'compute:revert_resize': common_policy.parse_rule('role:admin')} - common_policy.set_rules(common_policy.Rules(rule)) + policy.set_rules(rule) req = fakes.HTTPRequestV3.blank('/servers/1234/action') self.assertRaises(exception.PolicyNotAuthorized, self.controller._action_revert_resize, req, '1234', {}) diff --git a/nova/tests/api/openstack/compute/plugins/v3/test_shelve.py b/nova/tests/api/openstack/compute/plugins/v3/test_shelve.py index a8f851cb5c94..717d9b27a37b 100644 --- a/nova/tests/api/openstack/compute/plugins/v3/test_shelve.py +++ b/nova/tests/api/openstack/compute/plugins/v3/test_shelve.py @@ -19,7 +19,8 @@ from nova.api.openstack.compute.plugins.v3 import shelve from nova.compute import api as compute_api from nova import db from nova import exception -from nova.openstack.common import policy +from nova.openstack.common import policy as common_policy +from nova import policy from nova import test from nova.tests.api.openstack import fakes from nova.tests import fake_instance @@ -41,8 +42,8 @@ class ShelvePolicyTest(test.NoDBTestCase): self.controller = shelve.ShelveController() def test_shelve_restricted_by_role(self): - rules = policy.Rules({'compute_extension:v3:os-shelve:shelve': - policy.parse_rule('role:admin')}) + rules = {'compute_extension:v3:os-shelve:shelve': + common_policy.parse_rule('role:admin')} policy.set_rules(rules) req = fakes.HTTPRequestV3.blank('/servers/12/os-shelve') @@ -50,9 +51,9 @@ class ShelvePolicyTest(test.NoDBTestCase): req, str(uuid.uuid4()), {}) def test_shelve_allowed(self): - rules = policy.Rules({'compute:get': policy.parse_rule(''), - 'compute_extension:v3:os-shelve:shelve': - policy.parse_rule('')}) + rules = {'compute:get': common_policy.parse_rule(''), + 'compute_extension:v3:os-shelve:shelve': + common_policy.parse_rule('')} policy.set_rules(rules) self.stubs.Set(db, 'instance_get_by_uuid', fake_instance_get_by_uuid) @@ -70,8 +71,8 @@ class ShelvePolicyTest(test.NoDBTestCase): req, str(uuid.uuid4()), {}) def test_unshelve_restricted_by_role(self): - rules = policy.Rules({'compute_extension:v3:os-shelve:unshelve': - policy.parse_rule('role:admin')}) + rules = {'compute_extension:v3:os-shelve:unshelve': + common_policy.parse_rule('role:admin')} policy.set_rules(rules) req = fakes.HTTPRequestV3.blank('/servers/12/os-shelve') @@ -79,9 +80,9 @@ class ShelvePolicyTest(test.NoDBTestCase): req, str(uuid.uuid4()), {}) def test_unshelve_allowed(self): - rules = policy.Rules({'compute:get': policy.parse_rule(''), - 'compute_extension:v3:os-shelve:unshelve': - policy.parse_rule('')}) + rules = {'compute:get': common_policy.parse_rule(''), + 'compute_extension:v3:os-shelve:unshelve': + common_policy.parse_rule('')} policy.set_rules(rules) self.stubs.Set(db, 'instance_get_by_uuid', fake_instance_get_by_uuid) @@ -99,8 +100,8 @@ class ShelvePolicyTest(test.NoDBTestCase): req, str(uuid.uuid4()), {}) def test_shelve_offload_restricted_by_role(self): - rules = policy.Rules({'compute_extension:v3:os-shelve:shelve_offload': - policy.parse_rule('role:admin')}) + rules = {'compute_extension:v3:os-shelve:shelve_offload': + common_policy.parse_rule('role:admin')} policy.set_rules(rules) req = fakes.HTTPRequestV3.blank('/servers/12/os-shelve') @@ -108,9 +109,9 @@ class ShelvePolicyTest(test.NoDBTestCase): self.controller._shelve_offload, req, str(uuid.uuid4()), {}) def test_shelve_offload_allowed(self): - rules = policy.Rules({'compute:get': policy.parse_rule(''), - 'compute_extension:v3:shelve_offload': - policy.parse_rule('')}) + rules = {'compute:get': common_policy.parse_rule(''), + 'compute_extension:v3:shelve_offload': + common_policy.parse_rule('')} policy.set_rules(rules) self.stubs.Set(db, 'instance_get_by_uuid', fake_instance_get_by_uuid) diff --git a/nova/tests/api/openstack/compute/test_servers.py b/nova/tests/api/openstack/compute/test_servers.py index bfcb189e7540..34e0616645e8 100644 --- a/nova/tests/api/openstack/compute/test_servers.py +++ b/nova/tests/api/openstack/compute/test_servers.py @@ -788,7 +788,7 @@ class ServersControllerTest(ControllerTest): common_policy.parse_rule("project_id:fake"), } - common_policy.set_rules(common_policy.Rules(rules)) + policy.set_rules(rules) req = fakes.HTTPRequest.blank('/fake/servers?all_tenants=1') res = self.controller.index(req) @@ -809,7 +809,7 @@ class ServersControllerTest(ControllerTest): common_policy.parse_rule("project_id:fake"), } - common_policy.set_rules(common_policy.Rules(rules)) + policy.set_rules(rules) self.stubs.Set(db, 'instance_get_all_by_filters', fake_get_all) @@ -1361,7 +1361,7 @@ class ServersControllerUpdateTest(ControllerTest): def test_update_server_policy_fail(self): rule = {'compute:update': common_policy.parse_rule('role:admin')} - common_policy.set_rules(common_policy.Rules(rule)) + policy.set_rules(rule) body = {'server': {'name': 'server_test'}} req = self._get_request(body, {'name': 'server_test'}) self.assertRaises(exception.PolicyNotAuthorized, @@ -1659,7 +1659,7 @@ class ServerStatusTest(test.TestCase): def _req_with_policy_fail(self, policy_rule_name): rule = {'compute:%s' % policy_rule_name: common_policy.parse_rule('role:admin')} - common_policy.set_rules(common_policy.Rules(rule)) + policy.set_rules(rule) return fakes.HTTPRequest.blank('/fake/servers/1234/action') def test_active(self): diff --git a/nova/tests/conf_fixture.py b/nova/tests/conf_fixture.py index f595e34d7ad0..8cfd960c73ce 100644 --- a/nova/tests/conf_fixture.py +++ b/nova/tests/conf_fixture.py @@ -32,7 +32,7 @@ CONF.import_opt('network_size', 'nova.network.manager') CONF.import_opt('num_networks', 'nova.network.manager') CONF.import_opt('floating_ip_dns_manager', 'nova.network.floating_ips') CONF.import_opt('instance_dns_manager', 'nova.network.floating_ips') -CONF.import_opt('policy_file', 'nova.policy') +CONF.import_opt('policy_file', 'nova.openstack.common.policy') CONF.import_opt('compute_driver', 'nova.virt.driver') CONF.import_opt('api_paste_config', 'nova.wsgi') diff --git a/nova/tests/policy_fixture.py b/nova/tests/policy_fixture.py index 9c6dbd5e7419..8f7e7206fdde 100644 --- a/nova/tests/policy_fixture.py +++ b/nova/tests/policy_fixture.py @@ -40,9 +40,9 @@ class PolicyFixture(fixtures.Fixture): self.addCleanup(nova.policy.reset) def set_rules(self, rules): - common_policy.set_rules(common_policy.Rules( - dict((k, common_policy.parse_rule(v)) - for k, v in rules.items()))) + policy = nova.policy._ENFORCER + policy.set_rules(dict((k, common_policy.parse_rule(v)) + for k, v in rules.items())) class RoleBasedPolicyFixture(fixtures.Fixture): diff --git a/nova/tests/test_policy.py b/nova/tests/test_policy.py index 70b15b32015b..7a3569fc1037 100644 --- a/nova/tests/test_policy.py +++ b/nova/tests/test_policy.py @@ -51,9 +51,7 @@ class PolicyFileTestCase(test.NoDBTestCase): policy.enforce(self.context, action, self.target) with open(tmpfilename, "w") as policyfile: policyfile.write('{"example:test": "!"}') - # NOTE(vish): reset stored policy cache so we don't have to - # sleep(1) - policy._POLICY_CACHE = {} + policy._ENFORCER.load_rules(True) self.assertRaises(exception.PolicyNotAuthorized, policy.enforce, self.context, action, self.target) @@ -73,7 +71,10 @@ class PolicyTestCase(test.NoDBTestCase): "example:lowercase_admin": "role:admin or role:sysadmin", "example:uppercase_admin": "role:ADMIN or role:sysadmin", } - self.policy.set_rules(rules) + policy.reset() + policy.init() + policy.set_rules(dict((k, common_policy.parse_rule(v)) + for k, v in rules.items())) self.context = context.RequestContext('fake', 'fake', roles=['member']) self.target = {} @@ -161,10 +162,10 @@ class DefaultPolicyTestCase(test.NoDBTestCase): self.context = context.RequestContext('fake', 'fake') def _set_rules(self, default_rule): - rules = common_policy.Rules( - dict((k, common_policy.parse_rule(v)) - for k, v in self.rules.items()), default_rule) - common_policy.set_rules(rules) + policy.reset() + rules = dict((k, common_policy.parse_rule(v)) + for k, v in self.rules.items()) + policy.init(rules=rules, default_rule=default_rule, use_conf=False) def test_policy_called(self): self.assertRaises(exception.PolicyNotAuthorized, policy.enforce, @@ -180,6 +181,10 @@ class DefaultPolicyTestCase(test.NoDBTestCase): class IsAdminCheckTestCase(test.NoDBTestCase): + def setUp(self): + super(IsAdminCheckTestCase, self).setUp() + policy.init() + def test_init_true(self): check = policy.IsAdminCheck('is_admin', 'True') @@ -197,14 +202,18 @@ class IsAdminCheckTestCase(test.NoDBTestCase): def test_call_true(self): check = policy.IsAdminCheck('is_admin', 'True') - self.assertEqual(check('target', dict(is_admin=True)), True) - self.assertEqual(check('target', dict(is_admin=False)), False) + self.assertEqual(check('target', dict(is_admin=True), + policy._ENFORCER), True) + self.assertEqual(check('target', dict(is_admin=False), + policy._ENFORCER), False) def test_call_false(self): check = policy.IsAdminCheck('is_admin', 'False') - self.assertEqual(check('target', dict(is_admin=True)), False) - self.assertEqual(check('target', dict(is_admin=False)), True) + self.assertEqual(check('target', dict(is_admin=True), + policy._ENFORCER), False) + self.assertEqual(check('target', dict(is_admin=False), + policy._ENFORCER), True) class AdminRolePolicyTestCase(test.NoDBTestCase):