Merge "fix(federation): allow using numerical group names"

3 files changed, 47 insertions, 15 deletions

diff --git a/keystone/federation/utils.py b/keystone/federation/utils.py
index 7c1f0c901..71e6318a4 100644
--- a/keystone/federation/utils.py
+++ b/keystone/federation/utils.py
@@ -562,17 +562,31 @@ class RuleProcessor(object):
         LOG.debug('mapped_properties: %s', mapped_properties)
         return mapped_properties
 
+    def _ast_literal_eval(self, value):
+        # This is a workaround for the fact that ast.literal_eval handles the
+        # case of either a string or a list of strings, but not a potential
+        # list of ints.
+
+        try:
+            values = ast.literal_eval(value)
+            # NOTE(mnaser): It's possible that the group_names_list is a
+            #               numerical value which would successfully parse
+            #               and not raise an exception, so we forcefully
+            #               raise is here.
+            if not isinstance(values, list):
+                raise ValueError
+        except (ValueError, SyntaxError):
+            values = [value]
+
+        return values
+
     def _normalize_groups(self, identity_value):
         # In this case, identity_value['groups'] is a string
         # representation of a list, and we want a real list.  This is
         # due to the way we do direct mapping substitutions today (see
         # function _update_local_mapping() )
         if 'name' in identity_value['groups']:
-            try:
-                group_names_list = ast.literal_eval(
-                    identity_value['groups'])
-            except (ValueError, SyntaxError):
-                group_names_list = [identity_value['groups']]
+            group_names_list = self._ast_literal_eval(identity_value['groups'])
 
             def convert_json(group):
                 if group.startswith('JSON:'):
@@ -594,11 +608,8 @@ class RuleProcessor(object):
                         "specified.")
                 msg = msg % {'identity_value': identity_value}
                 raise exception.ValidationError(msg)
-            try:
-                group_names_list = ast.literal_eval(
-                    identity_value['groups'])
-            except (ValueError, SyntaxError):
-                group_names_list = [identity_value['groups']]
+            group_names_list = self._ast_literal_eval(
+                identity_value['groups'])
             domain = identity_value['domain']
             group_dicts = [{'name': name, 'domain': domain} for name in
                            group_names_list]
@@ -699,11 +710,8 @@ class RuleProcessor(object):
                 # group_ids parameter contains only one element, it will be
                 # parsed as a simple string, and not a list or the
                 # representation of a list.
-                try:
-                    group_ids.update(
-                        ast.literal_eval(identity_value['group_ids']))
-                except (ValueError, SyntaxError):
-                    group_ids.update([identity_value['group_ids']])
+                group_ids.update(
+                    self._ast_literal_eval(identity_value['group_ids']))
             if 'projects' in identity_value:
                 projects = identity_value['projects']
 
diff --git a/keystone/tests/unit/contrib/federation/test_utils.py b/keystone/tests/unit/contrib/federation/test_utils.py
index f9153cb09..4d9f98f2d 100644
--- a/keystone/tests/unit/contrib/federation/test_utils.py
+++ b/keystone/tests/unit/contrib/federation/test_utils.py
@@ -764,6 +764,24 @@ class MappingRuleEngineTests(unit.BaseTestCase):
         self.assertEqual('ALL USERS',
                          mapped_properties['group_names'][0]['name'])
 
+    def test_rule_engine_groups_mapping_only_one_numerical_group(self):
+        """Test mapping engine when groups is explicitly set.
+
+        If the groups list has only one group,
+        test if the transformation is done correctly
+
+        """
+        mapping = mapping_fixtures.MAPPING_GROUPS_WITH_EMAIL
+        assertion = mapping_fixtures.GROUPS_ASSERTION_ONLY_ONE_NUMERICAL_GROUP
+        rp = mapping_utils.RuleProcessor(FAKE_MAPPING_ID, mapping['rules'])
+        mapped_properties = rp.process(assertion)
+        self.assertIsNotNone(mapped_properties)
+        self.assertEqual('jsmith', mapped_properties['user']['name'])
+        self.assertEqual('jill@example.com',
+                         mapped_properties['user']['email'])
+        self.assertEqual('1234',
+                         mapped_properties['group_names'][0]['name'])
+
     def test_rule_engine_group_ids_mapping_whitelist(self):
         """Test mapping engine when group_ids is explicitly set.
 
diff --git a/keystone/tests/unit/mapping_fixtures.py b/keystone/tests/unit/mapping_fixtures.py
index 51f1526bb..5a6dbf8c3 100644
--- a/keystone/tests/unit/mapping_fixtures.py
+++ b/keystone/tests/unit/mapping_fixtures.py
@@ -1735,6 +1735,12 @@ GROUPS_ASSERTION_ONLY_ONE_GROUP = {
     'groups': 'ALL USERS'
 }
 
+GROUPS_ASSERTION_ONLY_ONE_NUMERICAL_GROUP = {
+    'userEmail': 'jill@example.com',
+    'UserName': 'jsmith',
+    'groups': '1234'
+}
+
 GROUPS_DOMAIN_ASSERTION = {
     'openstack_user': 'bwilliams',
     'openstack_user_domain': 'default',