Consider @,! in properties protection rule as a configuration error

In roles based property protection, if '@' and '!' are in the same rule then the
glance api will not start considering this an Invalid Configuration.

DocImpact
Closes-bug: #1260333

Change-Id: I1d304f5c505ae9e2486ff653dda205fc2d851c2b

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

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.\
+                            raise InvalidPropProtectConf()
-                                InvalidPropertyProtectionConfiguration()
                         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:
+                    if '!' in rule_roles:
-                        msg = _(
+                        return False
-                            "Malformed property protection rule '%s': '@' "
-                            "and '!' are mutually exclusive") % property_name
-                        LOG.error(msg)
-                        raise webob.exc.HTTPInternalServerError(msg)
                     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,

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 = !

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(