Reject rule if assertion type unset

When the "type" of the remote rule is not present in the assertion this
should constitute a failure to match and reject the local change.

Change-Id: Idba9a95f31a28401b2d49545347175ee0f324ab5
Closes-Bug: #1487937

3 files changed, 86 insertions, 31 deletions

diff --git a/keystone/contrib/federation/utils.py b/keystone/contrib/federation/utils.py
index b0db3cdd4..bde19cfd3 100644
--- a/keystone/contrib/federation/utils.py
+++ b/keystone/contrib/federation/utils.py
@@ -672,15 +672,18 @@ class RuleProcessor(object):
 
         for requirement in requirements:
             requirement_type = requirement['type']
+            direct_map_values = assertion.get(requirement_type)
             regex = requirement.get('regex', False)
 
+            if not direct_map_values:
+                return None
+
             any_one_values = requirement.get(self._EvalType.ANY_ONE_OF)
             if any_one_values is not None:
                 if self._evaluate_requirement(any_one_values,
-                                              requirement_type,
+                                              direct_map_values,
                                               self._EvalType.ANY_ONE_OF,
-                                              regex,
-                                              assertion):
+                                              regex):
                     continue
                 else:
                     return None
@@ -688,10 +691,9 @@ class RuleProcessor(object):
             not_any_values = requirement.get(self._EvalType.NOT_ANY_OF)
             if not_any_values is not None:
                 if self._evaluate_requirement(not_any_values,
-                                              requirement_type,
+                                              direct_map_values,
                                               self._EvalType.NOT_ANY_OF,
-                                              regex,
-                                              assertion):
+                                              regex):
                     continue
                 else:
                     return None
@@ -699,23 +701,21 @@ class RuleProcessor(object):
             # If 'any_one_of' or 'not_any_of' are not found, then values are
             # within 'type'. Attempt to find that 'type' within the assertion,
             # and filter these values if 'whitelist' or 'blacklist' is set.
-            direct_map_values = assertion.get(requirement_type)
-            if direct_map_values:
-                blacklisted_values = requirement.get(self._EvalType.BLACKLIST)
-                whitelisted_values = requirement.get(self._EvalType.WHITELIST)
+            blacklisted_values = requirement.get(self._EvalType.BLACKLIST)
+            whitelisted_values = requirement.get(self._EvalType.WHITELIST)
 
-                # If a blacklist or whitelist is used, we want to map to the
-                # whole list instead of just its values separately.
-                if blacklisted_values is not None:
-                    direct_map_values = [v for v in direct_map_values
-                                         if v not in blacklisted_values]
-                elif whitelisted_values is not None:
-                    direct_map_values = [v for v in direct_map_values
-                                         if v in whitelisted_values]
+            # If a blacklist or whitelist is used, we want to map to the
+            # whole list instead of just its values separately.
+            if blacklisted_values is not None:
+                direct_map_values = [v for v in direct_map_values
+                                     if v not in blacklisted_values]
+            elif whitelisted_values is not None:
+                direct_map_values = [v for v in direct_map_values
+                                     if v in whitelisted_values]
 
-                direct_maps.add(direct_map_values)
+            direct_maps.add(direct_map_values)
 
-                LOG.debug('updating a direct mapping: %s', direct_map_values)
+            LOG.debug('updating a direct mapping: %s', direct_map_values)
 
         return direct_maps
 
@@ -726,8 +726,8 @@ class RuleProcessor(object):
                     return True
         return False
 
-    def _evaluate_requirement(self, values, requirement_type,
-                              eval_type, regex, assertion):
+    def _evaluate_requirement(self, values, assertion_values,
+                              eval_type, regex):
         """Evaluate the incoming requirement and assertion.
 
         If the requirement type does not exist in the assertion data, then
@@ -737,23 +737,16 @@ class RuleProcessor(object):
 
         :param values: list of allowed values, defined in the requirement
         :type values: list
-        :param requirement_type: key to look for in the assertion
-        :type requirement_type: string
+        :param assertion_values: The values from the assertion to evaluate
+        :type assertion_values: list/string
         :param eval_type: determine how to evaluate requirements
         :type eval_type: string
         :param regex: perform evaluation with regex
         :type regex: boolean
-        :param assertion: dict of attributes from the IdP
-        :type assertion: dict
 
         :returns: boolean, whether requirement is valid or not.
 
         """
-
-        assertion_values = assertion.get(requirement_type)
-        if not assertion_values:
-            return False
-
         if regex:
             any_match = self._evaluate_values_by_regex(values,
                                                        assertion_values)
diff --git a/keystone/tests/unit/contrib/federation/test_utils.py b/keystone/tests/unit/contrib/federation/test_utils.py
index a8b4ae76f..5804f1c02 100644
--- a/keystone/tests/unit/contrib/federation/test_utils.py
+++ b/keystone/tests/unit/contrib/federation/test_utils.py
@@ -10,6 +10,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import uuid
 
 from keystone.auth.plugins import mapped
 from keystone.contrib.federation import utils as mapping_utils
@@ -609,3 +610,25 @@ class MappingRuleEngineTests(unit.BaseTestCase):
             self.assertEqual(exp_user_name, mapped_properties['user']['name'])
             self.assertEqual('abc123%40example.com',
                              mapped_properties['user']['id'])
+
+    def test_whitelist_pass_through(self):
+        mapping = mapping_fixtures.MAPPING_GROUPS_WHITELIST_PASS_THROUGH
+        rp = mapping_utils.RuleProcessor(mapping['rules'])
+        assertion = mapping_fixtures.DEVELOPER_ASSERTION
+        mapped_properties = rp.process(assertion)
+        self.assertValidMappedUserObject(mapped_properties)
+
+        self.assertEqual('developacct', mapped_properties['user']['name'])
+        self.assertEqual('Developer',
+                         mapped_properties['group_names'][0]['name'])
+
+    def test_type_not_in_assertion(self):
+        """Test that if the remote "type" is not in the assertion it fails."""
+        mapping = mapping_fixtures.MAPPING_GROUPS_WHITELIST_PASS_THROUGH
+        rp = mapping_utils.RuleProcessor(mapping['rules'])
+        assertion = {uuid.uuid4().hex: uuid.uuid4().hex}
+        mapped_properties = rp.process(assertion)
+        self.assertValidMappedUserObject(mapped_properties)
+
+        self.assertNotIn('id', mapped_properties['user'])
+        self.assertNotIn('name', mapped_properties['user'])
diff --git a/keystone/tests/unit/mapping_fixtures.py b/keystone/tests/unit/mapping_fixtures.py
index 55b7ab68b..94b071335 100644
--- a/keystone/tests/unit/mapping_fixtures.py
+++ b/keystone/tests/unit/mapping_fixtures.py
@@ -1146,6 +1146,45 @@ MAPPING_FOR_DEFAULT_EPHEMERAL_USER = {
     ]
 }
 
+MAPPING_GROUPS_WHITELIST_PASS_THROUGH = {
+    "rules": [
+        {
+            "remote": [
+                {
+                    "type": "UserName"
+                }
+            ],
+            "local": [
+                {
+                    "user": {
+                        "name": "{0}",
+                        "domain": {
+                            "id": DEVELOPER_GROUP_DOMAIN_ID
+                        }
+                    }
+                }
+            ]
+        },
+        {
+            "remote": [
+                {
+                    "type": "orgPersonType",
+                    "whitelist": ['Developer']
+                }
+            ],
+            "local": [
+                {
+                    "groups": "{0}",
+                    "domain": {
+                        "id": DEVELOPER_GROUP_DOMAIN_ID
+                    }
+                }
+            ]
+        }
+    ]
+}
+
+
 EMPLOYEE_ASSERTION = {
     'Email': 'tim@example.com',
     'UserName': 'tbo',