From 2042c955c81929deb47bc8cc77082b085faaa47d Mon Sep 17 00:00:00 2001 From: Roxana Gherle Date: Wed, 29 Jun 2016 11:21:13 -0700 Subject: [PATCH] Fix the username value in federated tokens Currently, in both unscoped and scoped federated tokens, the username value in the token is equal to the userid and not to the value of the username in the external identity provider. This makes WebSSO login to show the userid of the logged-in user in the Horizon dashboard, whereas before it was showing the actual user name. This patch fixes the value of the username in the federated tokens, which will fix the WebSSO issue as well, since Horizon looks at the username value and displays that as the logged-in user. Closes-Bug: #1597101 Closes-Bug: #1482701 Change-Id: I33a0274641c4e6bc4e127f5206ba9bc7dbd8e5a8 --- keystone/tests/unit/test_v3_federation.py | 19 +++++++++++++++++-- .../tests/unit/token/test_fernet_provider.py | 3 ++- keystone/token/providers/common.py | 6 +++++- keystone/token/providers/fernet/core.py | 7 +++++-- 4 files changed, 29 insertions(+), 6 deletions(-) diff --git a/keystone/tests/unit/test_v3_federation.py b/keystone/tests/unit/test_v3_federation.py index 0c06adc896..ed6e3bf8cf 100644 --- a/keystone/tests/unit/test_v3_federation.py +++ b/keystone/tests/unit/test_v3_federation.py @@ -151,8 +151,8 @@ class FederatedSetupMixin(object): self.assertIn('identity_provider', user['OS-FEDERATION']) self.assertIn('protocol', user['OS-FEDERATION']) - # Make sure user_id is url safe - self.assertEqual(urllib.parse.quote(user['name']), user['id']) + # Make sure user_name is url safe + self.assertEqual(urllib.parse.quote(user['name']), user['name']) def _issue_unscoped_token(self, idp=None, @@ -2504,6 +2504,21 @@ class FederatedTokenTests(test_v3.RestfulTestCase, FederatedSetupMixin): self._issue_unscoped_token, assertion='ANOTHER_LOCAL_USER_ASSERTION') + def test_user_name_and_id_in_federation_token(self): + r = self._issue_unscoped_token(assertion='EMPLOYEE_ASSERTION') + token = r.json_body['token'] + self.assertEqual( + mapping_fixtures.EMPLOYEE_ASSERTION['UserName'], + token['user']['name']) + self.assertNotEqual(token['user']['name'], token['user']['id']) + r = self.v3_create_token( + self.TOKEN_SCOPE_PROJECT_EMPLOYEE_FROM_EMPLOYEE) + token = r.json_body['token'] + self.assertEqual( + mapping_fixtures.EMPLOYEE_ASSERTION['UserName'], + token['user']['name']) + self.assertNotEqual(token['user']['name'], token['user']['id']) + class FernetFederatedTokenTests(test_v3.RestfulTestCase, FederatedSetupMixin): AUTH_METHOD = 'token' diff --git a/keystone/tests/unit/token/test_fernet_provider.py b/keystone/tests/unit/token/test_fernet_provider.py index bcb0674d71..55b9f41c0f 100644 --- a/keystone/tests/unit/token/test_fernet_provider.py +++ b/keystone/tests/unit/token/test_fernet_provider.py @@ -129,6 +129,7 @@ class TestValidate(unit.TestCase): protocol = uuid.uuid4().hex auth_context = { 'user_id': user_ref['id'], + 'user_name': user_ref['name'], 'group_ids': group_ids, federation_constants.IDENTITY_PROVIDER: identity_provider, federation_constants.PROTOCOL: protocol, @@ -140,7 +141,7 @@ class TestValidate(unit.TestCase): token = token_data['token'] exp_user_info = { 'id': user_ref['id'], - 'name': user_ref['id'], + 'name': user_ref['name'], 'domain': {'id': CONF.federation.federated_domain_name, 'name': CONF.federation.federated_domain_name, }, federation_constants.FEDERATION: { diff --git a/keystone/token/providers/common.py b/keystone/token/providers/common.py index c40732c7e7..a2c7a8060e 100644 --- a/keystone/token/providers/common.py +++ b/keystone/token/providers/common.py @@ -641,10 +641,14 @@ class BaseProvider(provider.Provider): group_ids = auth_context['group_ids'] idp = auth_context[federation_constants.IDENTITY_PROVIDER] protocol = auth_context[federation_constants.PROTOCOL] + + user_dict = self.identity_api.get_user(user_id) + user_name = user_dict['name'] + token_data = { 'user': { 'id': user_id, - 'name': parse.unquote(user_id), + 'name': parse.unquote(user_name), federation_constants.FEDERATION: { 'groups': [{'id': x} for x in group_ids], 'identity_provider': {'id': idp}, diff --git a/keystone/token/providers/fernet/core.py b/keystone/token/providers/fernet/core.py index 6f51223a57..cb9ed6cc64 100644 --- a/keystone/token/providers/fernet/core.py +++ b/keystone/token/providers/fernet/core.py @@ -25,7 +25,7 @@ from keystone.token.providers.fernet import token_formatters as tf CONF = keystone.conf.CONF -@dependency.requires('trust_api', 'oauth_api') +@dependency.requires('trust_api', 'oauth_api', 'identity_api') class Provider(common.BaseProvider): def __init__(self, *args, **kwargs): super(Provider, self).__init__(*args, **kwargs) @@ -124,11 +124,14 @@ class Provider(common.BaseProvider): 'protocol': {'id': protocol_id} } + user_dict = self.identity_api.get_user(user_id) + user_name = user_dict['name'] + token_dict = { 'user': { federation_constants.FEDERATION: federated_info, 'id': user_id, - 'name': user_id, + 'name': user_name, 'domain': {'id': CONF.federation.federated_domain_name, 'name': CONF.federation.federated_domain_name, }, }