From 9e1e2c2156f365078085db54dfbbfff50e2c2b84 Mon Sep 17 00:00:00 2001
From: Eric Brown <browne@vmware.com>
Date: Tue, 17 Jan 2017 17:42:52 -0800
Subject: [PATCH] Catch potential SyntaxError in federation mapping

When using the 'groups' keyword in a federation mapping, the value
passed in the assertion map be a simple string with a space. For
example, "ALL USERS". This results in ast.literal_eval() raising
a SyntaxError and not ValueError, which bubbles up to the API as
an uncaught 500 Internal Server Error.

Change-Id: I61f93a6c54b62ba8719d2603f93dc18c33b581ce
Closes-Bug: #1629446
---
 keystone/federation/utils.py                  |  2 +-
 .../unit/contrib/federation/test_utils.py     | 18 +++++++++
 keystone/tests/unit/mapping_fixtures.py       | 38 +++++++++++++++++++
 3 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/keystone/federation/utils.py b/keystone/federation/utils.py
index 1e8080b8a5..30d64e3f0a 100644
--- a/keystone/federation/utils.py
+++ b/keystone/federation/utils.py
@@ -615,7 +615,7 @@ class RuleProcessor(object):
                 try:
                     group_names_list = ast.literal_eval(
                         identity_value['groups'])
-                except ValueError:
+                except (ValueError, SyntaxError):
                     group_names_list = [identity_value['groups']]
                 domain = identity_value['domain']
                 group_dicts = [{'name': name, 'domain': domain} for name in
diff --git a/keystone/tests/unit/contrib/federation/test_utils.py b/keystone/tests/unit/contrib/federation/test_utils.py
index 975eb09c7a..998fe847dc 100644
--- a/keystone/tests/unit/contrib/federation/test_utils.py
+++ b/keystone/tests/unit/contrib/federation/test_utils.py
@@ -682,6 +682,24 @@ class MappingRuleEngineTests(unit.BaseTestCase):
                           rp.process,
                           assertion)
 
+    def test_rule_engine_groups_mapping_only_one_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_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('ALL USERS',
+                         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 87f835fdc6..cc74994ce6 100644
--- a/keystone/tests/unit/mapping_fixtures.py
+++ b/keystone/tests/unit/mapping_fixtures.py
@@ -1401,6 +1401,38 @@ MAPPING_BAD_LOCAL_SETUP = {
     ]
 }
 
+MAPPING_GROUPS_WITH_EMAIL = {
+    "rules": [
+        {
+            "remote": [
+                {
+                    "type": "groups",
+                },
+                {
+                    "type": "userEmail",
+                },
+                {
+                    "type": "UserName"
+                }
+            ],
+            "local": [
+                {
+                    "groups": "{0}",
+                    "domain": {
+                        "id": DEVELOPER_GROUP_DOMAIN_ID
+                    }
+                },
+                {
+                    "user": {
+                        "name": "{2}",
+                        "email": "{1}"
+                    }
+                }
+            ]
+        }
+    ]
+}
+
 EMPLOYEE_ASSERTION = {
     'Email': 'tim@example.com',
     'UserName': 'tbo',
@@ -1553,6 +1585,12 @@ UNICODE_NAME_ASSERTION = {
     'PFX_orgPersonType': 'Admin;Chief'
 }
 
+GROUPS_ASSERTION_ONLY_ONE_GROUP = {
+    'userEmail': 'jill@example.com',
+    'UserName': 'jsmith',
+    'groups': 'ALL USERS'
+}
+
 MAPPING_UNICODE = {
     "rules": [
         {