diff --git a/doc/source/property-protections.rst b/doc/source/property-protections.rst index 72048c68..819e725d 100644 --- a/doc/source/property-protections.rst +++ b/doc/source/property-protections.rst @@ -52,6 +52,10 @@ roles that are permitted to perform that operation in the Glance API. **If any o the keys are not specified, then the glance api service will not start successfully.** +In the list of user roles, ``@`` means all roles and ``!`` means no role. +**If both @ and ! are specified for the same rule then the glance api service +will not start** + .. note:: Only one policy rule is allowed per property operation. **If multiple are diff --git a/glance/common/property_utils.py b/glance/common/property_utils.py index be0dadcf..8a86aa96 100644 --- a/glance/common/property_utils.py +++ b/glance/common/property_utils.py @@ -22,7 +22,6 @@ except ImportError: from ordereddict import OrderedDict from oslo.config import cfg -import webob.exc import glance.api.policy from glance.common import exception @@ -47,6 +46,11 @@ property_opts = [ CONF = cfg.CONF CONF.register_opts(property_opts) +# NOTE (spredzy): Due to the particularly lengthy name of the exception +# and the number of occurence it is raise in this file, a variable is +# created +InvalidPropProtectConf = exception.InvalidPropertyProtectionConfiguration + def is_property_protection_enabled(): if CONF.property_protection_file: @@ -73,7 +77,7 @@ class PropertyRules(object): msg = (_("Couldn't find property protection file %s:%s.") % (CONF.property_protection_file, e)) LOG.error(msg) - raise exception.InvalidPropertyProtectionConfiguration() + raise InvalidPropProtectConf() if self.prop_prot_rule_format not in ['policies', 'roles']: msg = _("Invalid value '%s' for " @@ -81,7 +85,7 @@ class PropertyRules(object): "The permitted values are " "'roles' and 'policies'") % self.prop_prot_rule_format LOG.error(msg) - raise exception.InvalidPropertyProtectionConfiguration() + raise InvalidPropProtectConf() operations = ['create', 'read', 'update', 'delete'] properties = CONFIG.sections() @@ -99,8 +103,7 @@ class PropertyRules(object): "for a given operation. Policies can be " "combined in the policy file"), permissions) - raise exception.\ - InvalidPropertyProtectionConfiguration() + raise InvalidPropProtectConf() self.prop_exp_mapping[compiled_rule] = property_exp self._add_policy_rules(property_exp, operation, permissions) @@ -108,6 +111,16 @@ class PropertyRules(object): else: permissions = [permission.strip() for permission in permissions.split(',')] + if '@' in permissions and '!' in permissions: + msg = (_( + "Malformed property protection rule in " + "[%(prop)s] %(op)s=%(perm)s: '@' and '!' " + "are mutually exclusive") % + dict(prop=property_exp, + op=operation, + perm=permissions)) + LOG.error(msg) + raise InvalidPropProtectConf() property_dict[operation] = permissions else: property_dict[operation] = [] @@ -125,7 +138,7 @@ class PropertyRules(object): msg = (_("Encountered a malformed property protection rule %s:%s.") % (rule, e)) LOG.error(msg) - raise exception.InvalidPropertyProtectionConfiguration() + raise InvalidPropProtectConf() def _add_policy_rules(self, property_exp, action, rule): """Add policy rules to the policy enforcer. @@ -164,16 +177,10 @@ class PropertyRules(object): if rule_exp.search(str(property_name)): rule_roles = rule.get(action) if rule_roles: - if '@' in rule_roles and '!' in rule_roles: - msg = _( - "Malformed property protection rule '%s': '@' " - "and '!' are mutually exclusive") % property_name - LOG.error(msg) - raise webob.exc.HTTPInternalServerError(msg) + if '!' in rule_roles: + return False elif '@' in rule_roles: return True - elif '!' in rule_roles: - return False if self.prop_prot_rule_format == 'policies': prop_exp_key = self.prop_exp_mapping[rule_exp] return self._check_policy(prop_exp_key, action, diff --git a/glance/tests/etc/property-protections.conf b/glance/tests/etc/property-protections.conf index b663fc01..5f0e27b8 100644 --- a/glance/tests/etc/property-protections.conf +++ b/glance/tests/etc/property-protections.conf @@ -52,12 +52,6 @@ read = ! update = ! delete = ! -[x_invalid_all_and_none] -create = !,@ -read = @,! -update = !,@ -delete = @,! - [x_none_read] create = admin,member read = ! diff --git a/glance/tests/unit/common/test_property_utils.py b/glance/tests/unit/common/test_property_utils.py index c148245a..484debe9 100644 --- a/glance/tests/unit/common/test_property_utils.py +++ b/glance/tests/unit/common/test_property_utils.py @@ -13,8 +13,6 @@ # License for the specific language governing permissions and limitations # under the License. -import webob.exc - from glance.api import policy from glance.common import exception from glance.common import property_utils @@ -31,7 +29,6 @@ CONFIG_SECTIONS = [ 'spl_delete_prop', '^x_all_permitted.*', '^x_none_permitted.*', - 'x_invalid_all_and_none', 'x_none_read', 'x_none_update', 'x_none_delete', @@ -67,6 +64,15 @@ class TestPropertyRulesWithRoles(base.IsolatedUnitTest): self.assertRaises(exception.InvalidPropertyProtectionConfiguration, property_utils.PropertyRules) + def test_property_protection_with_mutually_exclusive_rule(self): + exclusive_rules = {'.*': {'create': ['@', '!'], + 'read': ['fake-role'], + 'update': ['fake-role'], + 'delete': ['fake-role']}} + self.set_property_protection_rules(exclusive_rules) + self.assertRaises(exception.InvalidPropertyProtectionConfiguration, + property_utils.PropertyRules) + def test_property_protection_with_malformed_rule(self): malformed_rules = {'^[0-9)': {'create': ['fake-role'], 'read': ['fake-role'], @@ -236,34 +242,6 @@ class TestPropertyRulesWithRoles(base.IsolatedUnitTest): self.assertFalse(self.rules_checker.check_property_rules( 'x_none_permitted', 'delete', create_context(self.policy, ['']))) - def test_check_property_rules_create_all_and_none(self): - self.rules_checker = property_utils.PropertyRules() - self.assertRaises(webob.exc.HTTPInternalServerError, - self.rules_checker.check_property_rules, - 'x_invalid_all_and_none', 'create', - create_context(self.policy, [''])) - - def test_check_property_rules_read_all_and_none(self): - self.rules_checker = property_utils.PropertyRules() - self.assertRaises(webob.exc.HTTPInternalServerError, - self.rules_checker.check_property_rules, - 'x_invalid_all_and_none', 'read', - create_context(self.policy, [''])) - - def test_check_property_rules_update_all_and_none(self): - self.rules_checker = property_utils.PropertyRules() - self.assertRaises(webob.exc.HTTPInternalServerError, - self.rules_checker.check_property_rules, - 'x_invalid_all_and_none', 'update', - create_context(self.policy, [''])) - - def test_check_property_rules_delete_all_and_none(self): - self.rules_checker = property_utils.PropertyRules() - self.assertRaises(webob.exc.HTTPInternalServerError, - self.rules_checker.check_property_rules, - 'x_invalid_all_and_none', 'delete', - create_context(self.policy, [''])) - def test_check_property_rules_read_none(self): self.rules_checker = property_utils.PropertyRules() self.assertTrue(self.rules_checker.check_property_rules(