From a4c226f31a48c5ab3b990aa0e5ddcfb5f414dde5 Mon Sep 17 00:00:00 2001
From: Morgan Fainberg <morgan.fainberg@gmail.com>
Date: Mon, 30 Jan 2017 14:14:15 -0800
Subject: [PATCH] Add validation for mfa rule validator (storage)

Add the same validation as used for jsonschema for the
_mfa_rules_validator_list_of_lists_of_strings_no_duplicates function.
This ensures any validation changes to one will stay in lockstep with
the other.

The _mfa_rules_validator_list_of_lists_of_strings_no_duplicates function
received some minor updates for readability and comments added.

Change-Id: Ifd285acf01c15a8fe5525acb5458a85aaa76656f
bp: per-user-auth-plugin-reqs
---
 .../identity/backends/resource_options.py     | 21 +++++++++++-----
 keystone/tests/unit/test_validation.py        | 24 ++++++++++++-------
 2 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/keystone/identity/backends/resource_options.py b/keystone/identity/backends/resource_options.py
index 1832e9dba6..4bdbe8f796 100644
--- a/keystone/identity/backends/resource_options.py
+++ b/keystone/identity/backends/resource_options.py
@@ -29,22 +29,31 @@ def _mfa_rules_validator_list_of_lists_of_strings_no_duplicates(value):
             'Sub-lists may not be duplicated. Strings in sub-lists may not be '
             'duplicated.')
     if not isinstance(value, list):
+        # Value is not a List, TypeError
         raise TypeError(msg)
     sublists = []
-    for item in value:
+    for sublist in value:
+        # Sublist element tracker is reset for each sublist.
         string_set = set()
-        if not isinstance(item, list):
+        if not isinstance(sublist, list):
+            # Sublist is not a List, TypeError
             raise TypeError(msg)
-        if not item:
+        if not sublist:
+            # Sublist is Empty, ValueError
             raise ValueError(msg)
-        if item in sublists:
+        if sublist in sublists:
+            # Sublist is duplicated, ValueError
             raise ValueError(msg)
-        sublists.append(sublists)
-        for element in item:
+        # Add the sublist to the tracker
+        sublists.append(sublist)
+        for element in sublist:
             if not isinstance(element, six.string_types):
+                # Element of sublist is not a string, TypeError
                 raise TypeError(msg)
             if element in string_set:
+                # Element of sublist is duplicated, ValueError
                 raise ValueError(msg)
+            # add element to the sublist element tracker
             string_set.add(element)
 
 
diff --git a/keystone/tests/unit/test_validation.py b/keystone/tests/unit/test_validation.py
index 249a1b5654..25a575ff6f 100644
--- a/keystone/tests/unit/test_validation.py
+++ b/keystone/tests/unit/test_validation.py
@@ -1911,30 +1911,38 @@ class UserValidationTestCase(unit.BaseTestCase):
         }
         self.update_user_validator.validate(request_to_validate)
 
-    def test_user_update_with_invalid_mfa_rules_fails(self):
+    def test_user_option_validation_with_invalid_mfa_rules_fails(self):
+        # Test both json schema validation and the validator method in
+        # keystone.identity.backends.resource_options
         test_cases = [
             # Main Element Not an Array
-            True,
+            (True, TypeError),
             # Sub-Element Not an Array
-            [True, False],
+            ([True, False], TypeError),
             # Sub-element Element not string
-            [[True], [True, False]],
+            ([[True], [True, False]], TypeError),
             # Duplicate sub-array
-            [['duplicate_array'] for x in range(0, 2)],
+            ([['duplicate_array'] for x in range(0, 2)], ValueError),
             # Empty Sub element
-            [[uuid.uuid4().hex], []],
+            ([[uuid.uuid4().hex], []], ValueError),
             # Duplicate strings in sub-element
-            [['duplicate' for x in range(0, 2)]],
+            ([['duplicate' for x in range(0, 2)]], ValueError),
         ]
-        for ruleset in test_cases:
+        for ruleset, exception_class in test_cases:
             request_to_validate = {
                 'options': {
                     ro.MFA_RULES_OPT.option_name: ruleset
                 }
             }
+            # JSON Schema Validation
             self.assertRaises(exception.SchemaValidationError,
                               self.update_user_validator.validate,
                               request_to_validate)
+            # Data Store Validation
+            self.assertRaises(
+                exception_class,
+                ro._mfa_rules_validator_list_of_lists_of_strings_no_duplicates,
+                ruleset)
 
 
 class GroupValidationTestCase(unit.BaseTestCase):