Merge "Ensure ephemeral user's user_id is url-safe" into stable/kilo

3 files changed, 77 insertions, 25 deletions

diff --git a/keystone/auth/plugins/mapped.py b/keystone/auth/plugins/mapped.py
index 720974408..dc56c6027 100644
--- a/keystone/auth/plugins/mapped.py
+++ b/keystone/auth/plugins/mapped.py
@@ -247,6 +247,8 @@ def setup_username(context, mapped_properties):
         user['name'] = user_id
 
     elif not user_id:
-        user['id'] = parse.quote(user_name)
+        user_id = user_name
+
+    user['id'] = parse.quote(user_id)
 
     return user
diff --git a/keystone/tests/unit/mapping_fixtures.py b/keystone/tests/unit/mapping_fixtures.py
index fbffb0afb..06f75b61e 100644
--- a/keystone/tests/unit/mapping_fixtures.py
+++ b/keystone/tests/unit/mapping_fixtures.py
@@ -786,6 +786,7 @@ MAPPING_USER_IDS = {
                 {
                     "user": {
                         "name": "{0}",
+                        "id": "abc123@example.com",
                         "domain": {
                             "id": "federated"
                         }
@@ -828,7 +829,7 @@ MAPPING_USER_IDS = {
             "local": [
                 {
                     "user": {
-                        "id": "abc123",
+                        "id": "abc123@example.com",
                         "name": "{0}",
                         "domain": {
                             "id": "federated"
@@ -1002,6 +1003,7 @@ TESTER_ASSERTION = {
 }
 
 ANOTHER_TESTER_ASSERTION = {
+    'Email': 'testacct@example.com',
     'UserName': 'IamTester'
 }
 
diff --git a/keystone/tests/unit/test_v3_federation.py b/keystone/tests/unit/test_v3_federation.py
index 06ec62334..c1efc4da1 100644
--- a/keystone/tests/unit/test_v3_federation.py
+++ b/keystone/tests/unit/test_v3_federation.py
@@ -331,7 +331,8 @@ class FederatedSetupMixin(object):
                         },
                         {
                             'user': {
-                                'name': '{0}'
+                                'name': '{0}',
+                                'id': '{1}'
                             }
                         }
                     ],
@@ -340,6 +341,9 @@ class FederatedSetupMixin(object):
                             'type': 'UserName'
                         },
                         {
+                            'type': 'Email',
+                        },
+                        {
                             'type': 'orgPersonType',
                             'any_one_of': [
                                 'Employee'
@@ -356,7 +360,8 @@ class FederatedSetupMixin(object):
                         },
                         {
                             'user': {
-                                'name': '{0}'
+                                'name': '{0}',
+                                'id': '{1}'
                             }
                         }
                     ],
@@ -365,6 +370,9 @@ class FederatedSetupMixin(object):
                             'type': self.ASSERTION_PREFIX + 'UserName'
                         },
                         {
+                            'type': self.ASSERTION_PREFIX + 'Email',
+                        },
+                        {
                             'type': self.ASSERTION_PREFIX + 'orgPersonType',
                             'any_one_of': [
                                 'SuperEmployee'
@@ -381,7 +389,8 @@ class FederatedSetupMixin(object):
                         },
                         {
                             'user': {
-                                'name': '{0}'
+                                'name': '{0}',
+                                'id': '{1}'
                             }
                         }
                     ],
@@ -390,6 +399,9 @@ class FederatedSetupMixin(object):
                             'type': 'UserName'
                         },
                         {
+                            'type': 'Email'
+                        },
+                        {
                             'type': 'orgPersonType',
                             'any_one_of': [
                                 'Customer'
@@ -417,7 +429,8 @@ class FederatedSetupMixin(object):
 
                         {
                             'user': {
-                                'name': '{0}'
+                                'name': '{0}',
+                                'id': '{1}'
                             }
                         }
                     ],
@@ -426,6 +439,9 @@ class FederatedSetupMixin(object):
                             'type': 'UserName'
                         },
                         {
+                            'type': 'Email'
+                        },
+                        {
                             'type': 'orgPersonType',
                             'any_one_of': [
                                 'Admin',
@@ -448,7 +464,8 @@ class FederatedSetupMixin(object):
                         },
                         {
                             'user': {
-                                'name': '{0}'
+                                'name': '{0}',
+                                'id': '{1}'
                             }
                         }
                     ],
@@ -457,6 +474,9 @@ class FederatedSetupMixin(object):
                             'type': 'UserName',
                         },
                         {
+                            'type': 'Email',
+                        },
+                        {
                             'type': 'FirstName',
                             'any_one_of': [
                                 'Jill'
@@ -479,7 +499,8 @@ class FederatedSetupMixin(object):
                         },
                         {
                             'user': {
-                                'name': '{0}'
+                                'name': '{0}',
+                                'id': '{1}'
                             }
                         }
                     ],
@@ -489,6 +510,9 @@ class FederatedSetupMixin(object):
                         },
                         {
                             'type': 'Email',
+                        },
+                        {
+                            'type': 'Email',
                             'any_one_of': [
                                 'testacct@example.com'
                             ]
@@ -506,7 +530,8 @@ class FederatedSetupMixin(object):
                     "local": [
                         {
                             'user': {
-                                'name': '{0}'
+                                'name': '{0}',
+                                'id': '{1}'
                             }
                         },
                         {
@@ -523,6 +548,9 @@ class FederatedSetupMixin(object):
                             'type': 'UserName',
                         },
                         {
+                            'type': 'Email',
+                        },
+                        {
                             "type": "orgPersonType",
                             "any_one_of": [
                                 "CEO",
@@ -535,7 +563,8 @@ class FederatedSetupMixin(object):
                     "local": [
                         {
                             'user': {
-                                'name': '{0}'
+                                'name': '{0}',
+                                'id': '{1}'
                             }
                         },
                         {
@@ -552,6 +581,9 @@ class FederatedSetupMixin(object):
                             "type": "UserName",
                         },
                         {
+                            "type": "Email",
+                        },
+                        {
                             "type": "orgPersonType",
                             "any_one_of": [
                                 "Managers"
@@ -563,7 +595,8 @@ class FederatedSetupMixin(object):
                     "local": [
                         {
                             "user": {
-                                "name": "{0}"
+                                "name": "{0}",
+                                "id": "{1}"
                             }
                         },
                         {
@@ -580,6 +613,9 @@ class FederatedSetupMixin(object):
                             "type": "UserName",
                         },
                         {
+                            "type": "Email",
+                        },
+                        {
                             "type": "UserName",
                             "any_one_of": [
                                 "IamTester"
@@ -1913,7 +1949,8 @@ class MappingRuleEngineTests(FederationTests):
         self.assertValidMappedUserObject(mapped_properties)
         mapped.setup_username({}, mapped_properties)
         self.assertEqual('tbo', mapped_properties['user']['name'])
-        self.assertEqual('tbo', mapped_properties['user']['id'])
+        self.assertEqual('abc123%40example.com',
+                         mapped_properties['user']['id'])
 
     def test_user_identification_id(self):
         """Test varius mapping options and how users are identified.
@@ -1951,17 +1988,28 @@ class MappingRuleEngineTests(FederationTests):
         - Check if user's id is properly set and and equal to value hardcoded
         in the mapping
 
-        """
-        mapping = mapping_fixtures.MAPPING_USER_IDS
-        rp = mapping_utils.RuleProcessor(mapping['rules'])
-        assertion = mapping_fixtures.CUSTOMER_ASSERTION
-        mapped_properties = rp.process(assertion)
-        context = {'environment': {}}
-        self.assertIsNotNone(mapped_properties)
-        self.assertValidMappedUserObject(mapped_properties)
-        mapped.setup_username(context, mapped_properties)
-        self.assertEqual('bwilliams', mapped_properties['user']['name'])
-        self.assertEqual('abc123', mapped_properties['user']['id'])
+        This test does two iterations with different assertions used as input
+        for the Mapping Engine.  Different assertions will be matched with
+        different rules in the ruleset, effectively issuing different user_id
+        (hardcoded values). In the first iteration, the hardcoded user_id is
+        not url-safe and we expect Keystone to make it url safe. In the latter
+        iteration, provided user_id is already url-safe and we expect server
+        not to change it.
+
+        """
+        testcases = [(mapping_fixtures.CUSTOMER_ASSERTION, 'bwilliams'),
+                     (mapping_fixtures.EMPLOYEE_ASSERTION, 'tbo')]
+        for assertion, exp_user_name in testcases:
+            mapping = mapping_fixtures.MAPPING_USER_IDS
+            rp = mapping_utils.RuleProcessor(mapping['rules'])
+            mapped_properties = rp.process(assertion)
+            context = {'environment': {}}
+            self.assertIsNotNone(mapped_properties)
+            self.assertValidMappedUserObject(mapped_properties)
+            mapped.setup_username(context, mapped_properties)
+            self.assertEqual(exp_user_name, mapped_properties['user']['name'])
+            self.assertEqual('abc123%40example.com',
+                             mapped_properties['user']['id'])
 
     def test_whitelist_pass_through(self):
         mapping = mapping_fixtures.MAPPING_GROUPS_WHITELIST_PASS_THROUGH
@@ -2922,12 +2970,12 @@ class FernetFederatedTokenTests(FederationTests, FederatedSetupMixin):
 
     def test_federated_unscoped_token(self):
         resp = self._issue_unscoped_token()
-        self.assertEqual(186, len(resp.headers['X-Subject-Token']))
+        self.assertEqual(204, len(resp.headers['X-Subject-Token']))
 
     def test_federated_unscoped_token_with_multiple_groups(self):
         assertion = 'ANOTHER_CUSTOMER_ASSERTION'
         resp = self._issue_unscoped_token(assertion=assertion)
-        self.assertEqual(204, len(resp.headers['X-Subject-Token']))
+        self.assertEqual(232, len(resp.headers['X-Subject-Token']))
 
     def test_validate_federated_unscoped_token(self):
         resp = self._issue_unscoped_token()