Merge "Reduce duplicated code related to policies"

2 files changed, 184 insertions, 53 deletions

diff --git a/openstack/common/fileutils.py b/openstack/common/fileutils.py
index b988ad0..bb2ef61 100644
--- a/openstack/common/fileutils.py
+++ b/openstack/common/fileutils.py
@@ -19,6 +19,13 @@
 import errno
 import os
 
+from openstack.common.gettextutils import _
+from openstack.common import log as logging
+
+LOG = logging.getLogger(__name__)
+
+_FILE_CACHE = {}
+
 
 def ensure_tree(path):
     """Create a directory (and any ancestor directories required)
@@ -33,3 +40,28 @@ def ensure_tree(path):
                 raise
         else:
             raise
+
+
+def read_cached_file(filename, force_reload=False):
+    """Read from a file if it has been modified.
+
+    :param force_reload: Whether to reload the file.
+    :returns: A tuple with a boolean specifying if the data is fresh
+              or not.
+    """
+    global _FILE_CACHE
+
+    if force_reload and filename in _FILE_CACHE:
+        del _FILE_CACHE[filename]
+
+    reloaded = False
+    mtime = os.path.getmtime(filename)
+    cache_info = _FILE_CACHE.setdefault(filename, {})
+
+    if not cache_info or mtime > cache_info.get('mtime', 0):
+        LOG.debug(_("Reloading cached file %s") % filename)
+        with open(filename) as fap:
+            cache_info['data'] = fap.read()
+        cache_info['mtime'] = mtime
+        reloaded = True
+    return (reloaded, cache_info['data'])
diff --git a/openstack/common/policy.py b/openstack/common/policy.py
index cd6dcfc..3669f9b 100644
--- a/openstack/common/policy.py
+++ b/openstack/common/policy.py
@@ -60,21 +60,39 @@ import abc
 import re
 import urllib
 
+from oslo.config import cfg
 import six
 import urllib2
 
 from openstack.common.gettextutils import _
+from openstack.common import fileutils
 from openstack.common import jsonutils
 from openstack.common import log as logging
 
+policy_opts = [
+    cfg.StrOpt('policy_file',
+               default='policy.json',
+               help=_('JSON file containing policy')),
+    cfg.StrOpt('policy_default_rule',
+               default='default',
+               help=_('Rule enforced when requested rule is not found')),
+]
 
-LOG = logging.getLogger(__name__)
+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.
@@ -124,65 +142,146 @@ 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
+    :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.
+    """
 
-    _rules = rules
+    def __init__(self, policy_file=None, rules=None, default_rule=None):
+        self.rules = Rules(rules)
+        self.default_rule = default_rule or CONF.policy_default_rule
 
+        self.policy_path = None
+        self.policy_file = policy_file or CONF.policy_file
 
-# Ditto
-def reset():
-    """Clear the rules used for policy checks."""
+    def set_rules(self, rules, overwrite=True):
+        """
+        Create a new Rules object based on the provided dict of rules
 
-    global _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.
+        """
 
-    _rules = None
+        if not isinstance(rules, dict):
+            raise TypeError(_("Rules must be an instance of dict or Rules, "
+                            "got %s instead") % type(rules))
 
+        if overwrite:
+            self.rules = Rules(rules)
+        else:
+            self.update(rules)
 
-def check(rule, target, creds, exc=None, *args, **kwargs):
-    """
-    Checks authorization of a rule against the target and credentials.
+    def clear(self):
+        """
+        Clears Enforcer rules,  policy's cache
+        and policy's path.
+        """
+        self.set_rules({})
+        self.policy_path = None
 
-    :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.
+    def load_rules(self, force_reload=False):
+        """
+        Loads policy_path's rules. Policy file is cached
+        and will be reloaded if modified.
 
-    :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 force_reload: Whether to overwrite current rules.
+        """
 
-    # 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
+        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:
+            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(path=CONF.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 do_raise and not result:
+            if exc:
+                raise exc(*args, **kwargs)
 
-    # If it is False, raise the exception if requested
-    if exc and result is False:
-        raise exc(*args, **kwargs)
+            raise PolicyNotAuthorized(rule)
 
-    return result
+        return result
 
 
 class BaseCheck(object):
@@ -392,7 +491,7 @@ def _parse_check(rule):
     try:
         kind, match = rule.split(':', 1)
     except Exception:
-        LOG.exception(_("Failed to understand rule %(rule)s") % locals())
+        LOG.exception(_("Failed to understand rule %s") % rule)
         # If the rule is invalid, we'll fail closed
         return FalseCheck()
 
@@ -723,13 +822,13 @@ def register(name, func=None):
 
 @register("rule")
 class RuleCheck(Check):
-    def __call__(self, target, creds):
+    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
@@ -737,7 +836,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']]
@@ -745,7 +844,7 @@ class RoleCheck(Check):
 
 @register('http')
 class HttpCheck(Check):
-    def __call__(self, target, creds):
+    def __call__(self, target, creds, enforcer):
         """
         Check http: rules by calling to a remote server.
 
@@ -763,7 +862,7 @@ class HttpCheck(Check):
 
 @register(None)
 class GenericCheck(Check):
-    def __call__(self, target, creds):
+    def __call__(self, target, creds, enforcer):
         """
         Check an individual match.