From 21b0c7e81691a513b7f6c39a71313ff06b957c94 Mon Sep 17 00:00:00 2001 From: Jamie Lennox Date: Mon, 24 Aug 2015 13:33:07 +1000 Subject: [PATCH] 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 --- keystone/contrib/federation/utils.py | 55 ++++++++----------- .../unit/contrib/federation/test_utils.py | 23 ++++++++ keystone/tests/unit/mapping_fixtures.py | 39 +++++++++++++ 3 files changed, 86 insertions(+), 31 deletions(-) diff --git a/keystone/contrib/federation/utils.py b/keystone/contrib/federation/utils.py index b0db3cdd48..bde19cfd3a 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 a8b4ae76fa..5804f1c02a 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 55b7ab68b7..94b0713359 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',