From feaf03443807676e0cf56b7a4458b978a065a132 Mon Sep 17 00:00:00 2001 From: Jason Anderson Date: Fri, 22 May 2020 16:37:36 -0500 Subject: [PATCH] Support regexes in whitelists/blacklists This adds support for the "regex" flag for both the "whitelist" and "blacklist" conditional types. Before, only the "any_one_of" and "not_any_of" conditionals supported this. Similar to the pre-existing regex logic, the patterns are matched from the beginning of the string, meaning you may need prefix them with ".*" if you do not care about the first characters of the match. Closes-Bug: #1880252 Change-Id: Ia51f47a58712c7230753f2cfa0c87b83a7339bf9 --- .../admin/federation/mapping_combinations.rst | 61 +++++++++++++--- keystone/federation/utils.py | 71 +++++++++++++------ .../unit/contrib/federation/test_utils.py | 38 ++++++++++ keystone/tests/unit/mapping_fixtures.py | 56 +++++++++++++++ .../notes/bug-1880252-51036d5353125e15.yaml | 10 +++ 5 files changed, 206 insertions(+), 30 deletions(-) create mode 100644 releasenotes/notes/bug-1880252-51036d5353125e15.yaml diff --git a/doc/source/admin/federation/mapping_combinations.rst b/doc/source/admin/federation/mapping_combinations.rst index 7af5a2e7ba..b9e0b4bbd5 100644 --- a/doc/source/admin/federation/mapping_combinations.rst +++ b/doc/source/admin/federation/mapping_combinations.rst @@ -276,7 +276,11 @@ empty condition - Mapping to user with the email matching value in remote attribute Email - Mapping to a group(s) with the name matching the value(s) in remote attribute OIDC_GROUPS +.. NOTE:: + If the user id and name are not specified in the mapping, the server tries to + directly map ``REMOTE_USER`` environment variable. If this variable is also + unavailable the server returns an HTTP 401 Unauthorized error. Groups can have multiple values. Each value must be separated by a `;` Example: OIDC_GROUPS=developers;testers @@ -354,13 +358,42 @@ In ```` shown below, please supply one of the following: ] } -.. NOTE:: +In the above example, a whitelist can be used to only map the user into a few of +the groups in their ``HTTP_OIDC_GROUPIDS`` remote attribute: - If the user id and name are not specified in the mapping, the server tries to - directly map ``REMOTE_USER`` environment variable. If this variable is also - unavailable the server returns an HTTP 401 Unauthorized error. +.. code-block:: json -Group ids and names can be provided in the local section: + { + "type": "HTTP_OIDC_GROUPIDS", + "whitelist": [ + "Developers", + "OpsTeam" + ] + } + +A blacklist can map the user into all groups except those matched: + +.. code-block:: json + + { + "type": "HTTP_OIDC_GROUPIDS", + "blacklist": [ + "Finance" + ] + } + +Regular expressions can be used in any condition for more flexible matches: + +.. code-block:: json + + { + "type": "HTTP_OIDC_GROUPIDS", + "whitelist": [ + ".*Team$" + ] + } + +When mapping into groups, either ids or names can be provided in the local section: .. code-block:: json @@ -504,7 +537,10 @@ setting it to ``true``. "name": "{0}" }, "group": { - "id": "0cd5e9" + "name": "{1}", + "domain": { + "id": "abc1234" + } } }, ], @@ -518,14 +554,23 @@ setting it to ``true``. ".*@yeah.com$" ] "regex": true - } + }, + { + "type": "HTTP_OIDC_GROUPIDS", + "whitelist": [ + "Project.*$" + ], + "regex": true + } ] } ] } This allows any user with a claim containing a key with any value in -``HTTP_OIDC_GROUPIDS`` to be mapped to group with id ``0cd5e9``. +``HTTP_OIDC_GROUPIDS`` to be mapped to group with id ``0cd5e9``. Additionally, +for every value in the ``HTTP_OIDC_GROUPIDS`` claim matching the string +``Project.*``, the user will be assigned to the project with that name. Condition Combinations ---------------------- diff --git a/keystone/federation/utils.py b/keystone/federation/utils.py index e4b7ecc0e5..5f53dfbb54 100644 --- a/keystone/federation/utils.py +++ b/keystone/federation/utils.py @@ -190,6 +190,9 @@ MAPPING_SCHEMA = { }, "blacklist": { "type": "array" + }, + "regex": { + "type": "boolean" } } }, @@ -203,6 +206,9 @@ MAPPING_SCHEMA = { }, "whitelist": { "type": "array" + }, + "regex": { + "type": "boolean" } } }, @@ -844,11 +850,17 @@ class RuleProcessor(object): # 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] + direct_map_values = ( + self._evaluate_requirement(blacklisted_values, + direct_map_values, + self._EvalType.BLACKLIST, + regex)) elif whitelisted_values is not None: - direct_map_values = [v for v in direct_map_values - if v in whitelisted_values] + direct_map_values = ( + self._evaluate_requirement(whitelisted_values, + direct_map_values, + self._EvalType.WHITELIST, + regex)) direct_maps.add(direct_map_values) @@ -857,20 +869,26 @@ class RuleProcessor(object): return direct_maps def _evaluate_values_by_regex(self, values, assertion_values): - for value in values: - for assertion_value in assertion_values: - if re.search(value, assertion_value): - return True - return False + return [ + assertion for assertion in assertion_values + if any([re.search(regex, assertion) for regex in values]) + ] 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 - return False. If regex is specified, then compare the values and - assertion values. Otherwise, grab the intersection of the values - and use that to compare against the evaluation type. + Filter the incoming assertions against the requirement values. If regex + is specified, the assertion list is filtered by checking if any of the + requirement regexes matches. Otherwise, the list is filtered by string + equality with any of the allowed values. + + Once the assertion values are filtered, the output is determined by the + evaluation type: + any_one_of: return True if there are any matches, False otherwise + not_any_of: return True if there are no matches, False otherwise + blacklist: return the incoming values minus any matches + whitelist: return only the matched values :param values: list of allowed values, defined in the requirement :type values: list @@ -881,20 +899,29 @@ class RuleProcessor(object): :param regex: perform evaluation with regex :type regex: boolean - :returns: boolean, whether requirement is valid or not. + :returns: list of filtered assertion values (if evaluation type is + 'blacklist' or 'whitelist'), or boolean indicating if the + assertion values fulfill the requirement (if evaluation type + is 'any_one_of' or 'not_any_of') """ if regex: - any_match = self._evaluate_values_by_regex(values, - assertion_values) + matches = self._evaluate_values_by_regex(values, assertion_values) else: - any_match = bool(set(values).intersection(set(assertion_values))) - if any_match and eval_type == self._EvalType.ANY_ONE_OF: - return True - if not any_match and eval_type == self._EvalType.NOT_ANY_OF: - return True + matches = set(values).intersection(set(assertion_values)) - return False + if eval_type == self._EvalType.ANY_ONE_OF: + return bool(matches) + elif eval_type == self._EvalType.NOT_ANY_OF: + return not bool(matches) + elif eval_type == self._EvalType.BLACKLIST: + return list(set(assertion_values).difference(set(matches))) + elif eval_type == self._EvalType.WHITELIST: + return list(matches) + else: + raise exception.UnexpectedError( + _('Unexpected evaluation type "%(eval_type)s"') % { + 'eval_type': eval_type}) def assert_enabled_identity_provider(federation_api, idp_id): diff --git a/keystone/tests/unit/contrib/federation/test_utils.py b/keystone/tests/unit/contrib/federation/test_utils.py index 29b7f11a01..f233ac56ea 100644 --- a/keystone/tests/unit/contrib/federation/test_utils.py +++ b/keystone/tests/unit/contrib/federation/test_utils.py @@ -262,6 +262,44 @@ class MappingRuleEngineTests(unit.BaseTestCase): self._rule_engine_regex_match_and_many_groups( mapping_fixtures.MALFORMED_TESTER_ASSERTION) + def test_rule_engine_regex_blacklist(self): + mapping = mapping_fixtures.MAPPING_GROUPS_BLACKLIST_REGEX + assertion = mapping_fixtures.EMPLOYEE_PARTTIME_ASSERTION + rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules']) + mapped = rp.process(assertion) + + expected = { + 'user': {'type': 'ephemeral'}, + 'projects': [], + 'group_ids': [], + 'group_names': [ + {'name': 'Manager', 'domain': { + 'id': mapping_fixtures.FEDERATED_DOMAIN}} + ] + } + + self.assertEqual(expected, mapped) + + def test_rule_engine_regex_whitelist(self): + mapping = mapping_fixtures.MAPPING_GROUPS_WHITELIST_REGEX + assertion = mapping_fixtures.EMPLOYEE_PARTTIME_ASSERTION + rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules']) + mapped = rp.process(assertion) + + expected = { + 'user': {'type': 'ephemeral'}, + 'projects': [], + 'group_ids': [], + 'group_names': [ + {'name': 'Employee', 'domain': { + 'id': mapping_fixtures.FEDERATED_DOMAIN}}, + {'name': 'PartTimeEmployee', 'domain': { + 'id': mapping_fixtures.FEDERATED_DOMAIN}} + ] + } + + self.assertEqual(expected, mapped) + def test_rule_engine_fails_after_discarding_nonstring(self): """Check whether RuleProcessor discards non string objects. diff --git a/keystone/tests/unit/mapping_fixtures.py b/keystone/tests/unit/mapping_fixtures.py index 4275960a95..51f1526bbf 100644 --- a/keystone/tests/unit/mapping_fixtures.py +++ b/keystone/tests/unit/mapping_fixtures.py @@ -887,6 +887,54 @@ MAPPING_GROUPS_BLACKLIST = { ] } +MAPPING_GROUPS_BLACKLIST_REGEX = { + "rules": [ + { + "remote": [ + { + "type": "orgPersonType", + "blacklist": [ + ".*Employee$" + ], + "regex": True + }, + ], + "local": [ + { + "groups": "{0}", + "domain": { + "id": FEDERATED_DOMAIN + } + }, + ] + } + ] +} + +MAPPING_GROUPS_WHITELIST_REGEX = { + "rules": [ + { + "remote": [ + { + "type": "orgPersonType", + "whitelist": [ + ".*Employee$" + ], + "regex": True + }, + ], + "local": [ + { + "groups": "{0}", + "domain": { + "id": FEDERATED_DOMAIN + } + }, + ] + } + ] +} + # Exercise all possibilities of user identification. Values are hardcoded on # purpose. MAPPING_USER_IDS = { @@ -1529,6 +1577,14 @@ EMPLOYEE_ASSERTION = { 'orgPersonType': 'Employee;BuildingX' } +EMPLOYEE_PARTTIME_ASSERTION = { + 'Email': 'tim@example.com', + 'UserName': 'tbo', + 'FirstName': 'Tim', + 'LastName': 'Bo', + 'orgPersonType': 'Employee;PartTimeEmployee;Manager' +} + EMPLOYEE_ASSERTION_MULTIPLE_GROUPS = { 'Email': 'tim@example.com', 'UserName': 'tbo', diff --git a/releasenotes/notes/bug-1880252-51036d5353125e15.yaml b/releasenotes/notes/bug-1880252-51036d5353125e15.yaml new file mode 100644 index 0000000000..106bfc9be8 --- /dev/null +++ b/releasenotes/notes/bug-1880252-51036d5353125e15.yaml @@ -0,0 +1,10 @@ +--- +features: + - | + Mappings can now specify "whitelist" and "blacklist" conditionals as + regular expressions. Prior, only "not_any_of" and "any_one_of" conditionals + supported regular expression matching. +fixes: + - | + [`bug 1880252 `_] + Regexes are not allowed in "whitelist" and "blacklist" conditionals