From c2be944fb89f94a10d7105b2e072eeab5582c5a7 Mon Sep 17 00:00:00 2001 From: Kristi Nikolla Date: Tue, 16 Apr 2019 14:11:36 -0400 Subject: [PATCH] Report correct domain in federated user token Regardless of what domain the user was in, the domain reported in the token would be hardcoded to 'Federated' (regardless of the federated_domain_name config option). This patch removes the places where the domain was overwritten, and allows the correct domain to flow to the rendered token. It also updates the tests where it was being checked for the 'Federated' domain. Change-Id: Idad4e077c488d87f75172664fb519232eb78e292 Closes-Bug: 1754048 --- keystone/common/render_token.py | 3 --- keystone/federation/utils.py | 7 +------ .../tests/unit/contrib/federation/test_utils.py | 15 ++++++--------- keystone/tests/unit/test_cli.py | 3 --- keystone/tests/unit/test_v3_federation.py | 5 +++-- ...correct-federated-domain-47cb889d88d7770a.yaml | 6 ++++++ 6 files changed, 16 insertions(+), 23 deletions(-) create mode 100644 releasenotes/notes/bug-1754048-correct-federated-domain-47cb889d88d7770a.yaml diff --git a/keystone/common/render_token.py b/keystone/common/render_token.py index a74b44f51a..9363639114 100644 --- a/keystone/common/render_token.py +++ b/keystone/common/render_token.py @@ -121,9 +121,6 @@ def render_token_response_from_model(token, include_catalog=True): token_reference['token']['user']['OS-FEDERATION'] = ( federated_dict ) - token_reference['token']['user']['domain'] = { - 'id': 'Federated', 'name': 'Federated' - } del token_reference['token']['user']['password_expires_at'] if token.access_token_id: token_reference['token']['OS-OAUTH1'] = { diff --git a/keystone/federation/utils.py b/keystone/federation/utils.py index f1c052869f..78deeb41bc 100644 --- a/keystone/federation/utils.py +++ b/keystone/federation/utils.py @@ -591,12 +591,7 @@ class RuleProcessor(object): raise exception.ValidationError(msg) if user_type is None: - user_type = user['type'] = UserType.EPHEMERAL - - if user_type == UserType.EPHEMERAL: - user['domain'] = { - 'id': CONF.federation.federated_domain_name - } + user['type'] = UserType.EPHEMERAL # initialize the group_ids as a set to eliminate duplicates user = {} diff --git a/keystone/tests/unit/contrib/federation/test_utils.py b/keystone/tests/unit/contrib/federation/test_utils.py index 7eee36b34c..b182cb10fd 100644 --- a/keystone/tests/unit/contrib/federation/test_utils.py +++ b/keystone/tests/unit/contrib/federation/test_utils.py @@ -44,19 +44,18 @@ class MappingRuleEngineTests(unit.BaseTestCase): """Check whether mapped properties object has 'user' within. According to today's rules, RuleProcessor does not have to issue user's - id or name. What's actually required is user's type and for ephemeral - users that would be service domain named 'Federated'. + id or name. What's actually required is user's type. """ self.assertIn('user', mapped_properties, message='Missing user object in mapped properties') user = mapped_properties['user'] self.assertIn('type', user) self.assertEqual(user_type, user['type']) - self.assertIn('domain', user) - domain = user['domain'] - domain_name_or_id = domain.get('id') or domain.get('name') - domain_ref = domain_id or 'Federated' - self.assertEqual(domain_ref, domain_name_or_id) + + if domain_id: + domain = user['domain'] + domain_name_or_id = domain.get('id') or domain.get('name') + self.assertEqual(domain_id, domain_name_or_id) def test_rule_engine_any_one_of_and_direct_mapping(self): """Should return user's name and group id EMPLOYEE_GROUP_ID. @@ -912,7 +911,6 @@ class TestMappingLocals(unit.BaseTestCase): expected = { 'user': { 'name': 'a_user', - 'domain': {'id': 'Federated'}, 'type': 'ephemeral' }, 'projects': [], @@ -930,7 +928,6 @@ class TestMappingLocals(unit.BaseTestCase): expected = { 'user': { 'name': 'test_a_user', - 'domain': {'id': 'Federated'}, 'type': 'ephemeral' }, 'projects': [], diff --git a/keystone/tests/unit/test_cli.py b/keystone/tests/unit/test_cli.py index 25f05be9e3..dcd7d03ec3 100644 --- a/keystone/tests/unit/test_cli.py +++ b/keystone/tests/unit/test_cli.py @@ -1798,9 +1798,6 @@ class TestMappingEngineTester(unit.BaseTestCase): "group_names": [], "user": { "type": "ephemeral", - "domain": { - "id": "Federated" - }, "name": "me" }, "projects": [], diff --git a/keystone/tests/unit/test_v3_federation.py b/keystone/tests/unit/test_v3_federation.py index b02b9a8ab0..3de15d50f3 100644 --- a/keystone/tests/unit/test_v3_federation.py +++ b/keystone/tests/unit/test_v3_federation.py @@ -84,8 +84,9 @@ class FederatedSetupMixin(object): } def _check_domains_are_valid(self, token): - self.assertEqual('Federated', token['user']['domain']['id']) - self.assertEqual('Federated', token['user']['domain']['name']) + domain = PROVIDERS.resource_api.get_domain(self.idp['domain_id']) + self.assertEqual(domain['id'], token['user']['domain']['id']) + self.assertEqual(domain['name'], token['user']['domain']['name']) def _project(self, project): return (project['id'], project['name']) diff --git a/releasenotes/notes/bug-1754048-correct-federated-domain-47cb889d88d7770a.yaml b/releasenotes/notes/bug-1754048-correct-federated-domain-47cb889d88d7770a.yaml new file mode 100644 index 0000000000..3e068ee83c --- /dev/null +++ b/releasenotes/notes/bug-1754048-correct-federated-domain-47cb889d88d7770a.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + [`bug 1754048 `_] + The correct user domain is now reported when validating a federated token. + Previously, the domain would always be validated as "Federated." \ No newline at end of file