Check target_tenant when create rbac policy

Currently the default value of target_tenant in rbac_policies resource
is None, if a user creates rbac policy without a target_tenant,
neutron-server raises an internal error.
This patch adds a check for target_tenant, it must be a string which
length is TENANT_ID_MAX_LEN.
This patch also does a change for policy engine which now allows
enforce_policy to work on attrs without default.

Change-Id: I757b4b56a8cb1a8060cee3103ee2aead9be574de
Closes-bug: #1517331

neutron/extensions/rbac.py

@@ -60,12 +60,12 @@ RESOURCE_ATTRIBUTE_MAP = {
                         'enforce_policy': True},
         'object_id': {'allow_post': True, 'allow_put': False,
                       'validate': {'type:uuid': None},
-                      'is_visible': True, 'default': None,
-                      'enforce_policy': True},
+                      'is_visible': True, 'enforce_policy': True},
         'target_tenant': {'allow_post': True, 'allow_put': True,
-                          'is_visible': True, 'enforce_policy': True,
-                          'default': None},
+                          'validate': {'type:string': attr.TENANT_ID_MAX_LEN},
+                          'is_visible': True, 'enforce_policy': True},
         'tenant_id': {'allow_post': True, 'allow_put': False,
+                      'validate': {'type:string': attr.TENANT_ID_MAX_LEN},
                       'required_by_policy': True, 'is_visible': True},
         'action': {'allow_post': True, 'allow_put': False,
                    # action depends on type so validation has to occur in
@@ -73,8 +73,7 @@ RESOURCE_ATTRIBUTE_MAP = {
                    'validate': {'type:string': attr.DESCRIPTION_MAX_LEN},
                    # we set enforce_policy so operators can define policies
                    # that restrict actions
-                   'is_visible': True, 'enforce_policy': True,
-                   'default': None},
+                   'is_visible': True, 'enforce_policy': True}
     }
 }
 

neutron/policy.py

@@ -92,10 +92,11 @@ def _is_attribute_explicitly_set(attribute_name, resource, target, action):
         # marked as being updated instead.
         return (attribute_name in target[const.ATTRIBUTES_TO_UPDATE] and
                 target[attribute_name] is not constants.ATTR_NOT_SPECIFIED)
-    return ('default' in resource[attribute_name] and
-            attribute_name in target and
-            target[attribute_name] is not constants.ATTR_NOT_SPECIFIED and
-            target[attribute_name] != resource[attribute_name]['default'])
+    result = (attribute_name in target and
+              target[attribute_name] is not constants.ATTR_NOT_SPECIFIED)
+    if result and 'default' in resource[attribute_name]:
+        return target[attribute_name] != resource[attribute_name]['default']
+    return result
 
 
 def _should_validate_sub_attributes(attribute, sub_attr):

neutron/tests/tempest/api/admin/test_shared_network_extension.py

@@ -192,6 +192,22 @@ class RBACSharedNetworksTest(base.BaseAdminNetworkTest):
         )['rbac_policy']
         return {'network': net, 'subnet': subnet, 'policy': pol}
 
+    @test.attr(type='smoke')
+    @test.idempotent_id('86c3529b-1231-40de-803c-bfffffff1eee')
+    def test_create_rbac_policy_with_target_tenant_none(self):
+        with testtools.ExpectedException(lib_exc.BadRequest):
+            self._make_admin_net_and_subnet_shared_to_tenant_id(
+                tenant_id=None)
+
+    @test.attr(type='smoke')
+    @test.idempotent_id('86c3529b-1231-40de-803c-bfffffff1fff')
+    def test_create_rbac_policy_with_target_tenant_too_long_id(self):
+        with testtools.ExpectedException(lib_exc.BadRequest):
+            target_tenant = '1234' * 100
+            self._make_admin_net_and_subnet_shared_to_tenant_id(
+                tenant_id=target_tenant)
+
+    @test.attr(type='smoke')
     @test.idempotent_id('86c3529b-1231-40de-803c-afffffff1fff')
     def test_network_only_visible_to_policy_target(self):
         net = self._make_admin_net_and_subnet_shared_to_tenant_id(

neutron/tests/unit/test_policy.py

@@ -617,3 +617,43 @@ class NeutronPolicyTestCase(base.BaseTestCase):
         policy.log_rule_list(oslo_policy.RuleCheck('rule', 'create_'))
         self.assertTrue(mock_is_e.called)
         self.assertTrue(mock_debug.called)
+
+    def test__is_attribute_explicitly_set(self):
+        action = 'create'
+        attr = 'attr'
+
+        target = {attr: 'valueA', 'tgt-tenant': 'tenantA'}
+        resource = {attr: {'allow_post': True,
+                           'allow_put': True,
+                           'is_visible': True,
+                           'enforce_policy': True,
+                           'validate': {'type:string': 10}}}
+
+        result = policy._is_attribute_explicitly_set(
+            attr, resource, target, action)
+        self.assertTrue(result)
+
+        target = {'tgt-tenant': 'tenantA'}
+        result = policy._is_attribute_explicitly_set(
+            attr, resource, target, action)
+        self.assertFalse(result)
+
+        resource = {attr: {'allow_post': True,
+                           'allow_put': True,
+                           'is_visible': True,
+                           'default': 'DfltValue',
+                           'enforce_policy': True,
+                           'validate': {'type:string': 10}}}
+        result = policy._is_attribute_explicitly_set(
+            attr, resource, target, action)
+        self.assertFalse(result)
+
+        target = {attr: 'DfltValue', 'tgt-tenant': 'tenantA'}
+        result = policy._is_attribute_explicitly_set(
+            attr, resource, target, action)
+        self.assertFalse(result)
+
+        target = {attr: const.ATTR_NOT_SPECIFIED, 'tgt-tenant': 'tenantA'}
+        result = policy._is_attribute_explicitly_set(
+            attr, resource, target, action)
+        self.assertFalse(result)