openstack
/
keystone
Code Issues Proposed changes

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
This commit is contained in:
Morgan Fainberg 2017-01-30 21:13:53 -08:00
parent 113055774a
commit 5c861c0d77
1 changed files with 17 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
keystone/auth/core.py
View File

@ -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)