From 5c861c0d77fb432a921af0bd8841042ea640104b Mon Sep 17 00:00:00 2001 From: Morgan Fainberg Date: Mon, 30 Jan 2017 21:13:53 -0800 Subject: [PATCH] Remove de-dupe for MFA Rule parsing. The de-duplication was over optimisation and not of value it is less expensive to have 1 or 2 (at most) rules loaded form storage (which is already protected against having multiple rules stored both via json schema and the data storage bits itself) than to join the strings and sort them just to eliminate another iteration. Change-Id: I88b3ab0e956e32be91f87d85cb4e19069dd3d08c --- keystone/auth/core.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/keystone/auth/core.py b/keystone/auth/core.py index dd6ed2d829..b67de7fbe8 100644 --- a/keystone/auth/core.py +++ b/keystone/auth/core.py @@ -157,17 +157,21 @@ class UserMFARulesValidator(object): # any exceptions, but just produce a usable set of data for rules # processing. rule_set = [] - found_rules = set() if not isinstance(rules, list): LOG.error(_LE('Corrupt rule data structure for user %(user_id)s, ' 'no rules loaded.'), {'user_id': user_id}) + # Corrupt Data means no rules. Auth success > MFA rules in this + # case. return rule_set elif not rules: + # Exit early, nothing to do here. return rule_set for r_list in rules: if not isinstance(r_list, list): + # Rule was not a list, it is invalid, drop the rule from + # being considered. LOG.info(_LI('Ignoring Rule %(rule)r; rule must be a list of ' 'strings.'), {'type': type(r_list)}) @@ -183,13 +187,21 @@ class UserMFARulesValidator(object): LOG.info(_LI('Ignoring Rule %(rule)r; rule contains ' 'non-string values.'), {'rule': r_list}) + # Rule is known to be bad, drop it from consideration. _ok_rule = False break + # NOTE(notmorgan): No FOR/ELSE used here! Though it could be + # done and avoid the use of _ok_rule. This is a note for + # future developers to avoid using for/else and as an example + # of how to implement it that is readable and maintainable. if _ok_rule: - # De-dupe rule and add to the return value - rule_string = ';'.join(sorted(r_list)) - if rule_string not in found_rules: - found_rules.add(rule_string) + # Unique the r_list and cast back to a list and then append + # as we know the rule is ok (matches our requirements). + # This is outside the for loop, as the for loop is + # only used to validate the elements in the list. The + # This de-dupe should never be needed, but we are being + # extra careful at all levels of validation for the MFA + # rules. r_list = list(set(r_list)) rule_set.append(r_list)