fix(federation): allow using numerical group names

When using a numerical group name, the current codebase which
relies on ast.literal_eval does not account for the value
being a number.  Therefore, it can be parsed as a number and
fail in further steps since it will not be a list.

This patch adds a test to handle that use case and refactor the
code that leverages ast.literal_eval to be the same everywhere
so that it adds that fix everywhere.

Closes-Bug: #1992186
Change-Id: I665b7e0234650ba07e0d030a2d442d6599d0888a
(cherry picked from commit c70d0c3)

keystone/federation/utils.py

@@ -562,17 +562,31 @@ def process(self, assertion_data):
         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 @@ def convert_json(group):
                         "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 @@ def normalize_user(user):
                 # 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']
 

keystone/tests/unit/contrib/federation/test_utils.py

@@ -764,6 +764,24 @@ def test_rule_engine_groups_mapping_only_one_group(self):
         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.
 

keystone/tests/unit/mapping_fixtures.py

@@ -1735,6 +1735,12 @@
     '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',