From 2c104cc770da9901ce36b81833a2d2c6f9b14913 Mon Sep 17 00:00:00 2001 From: Colleen Murphy Date: Fri, 21 Oct 2016 13:37:35 +0200 Subject: [PATCH] Make default domain usage consistent The description for the OPENSTACK_KEYSTONE_DEFAULT_DOMAIN django variable claims it refers to the ID of the domain. However, the authenticate method of django_openstack_auth explicitly uses the name when it requests a token[1], and when multidomain support is enabled the user is asked for the domain name, not ID. If the operator tries to set this variable to the ID of any domain besides keystone's own Default domain, login will fail with "Could not find domain: " in the keystone logs. This patch forces horizon to use the variable as a name instead of an ID and updates the comment, so that everything using this variable is consistent with each other. This wasn't caught before because the unit tests were only testing against the default domain, so this patch also adds a second, enabled, non-default mock domain to test with. [1] http://git.openstack.org/cgit/openstack/django_openstack_auth/tree/openstack_auth/backend.py?h=2.4.1#n148 Change-Id: I4d16f831c9fc446859c9fb964b7609d5a76338fe --- openstack_dashboard/api/keystone.py | 13 ++++++++----- .../dashboards/identity/domains/tests.py | 5 +++-- .../dashboards/identity/groups/tests.py | 2 +- openstack_dashboard/local/local_settings.py.example | 6 +++--- openstack_dashboard/test/test_data/keystone_data.py | 7 ++++++- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/openstack_dashboard/api/keystone.py b/openstack_dashboard/api/keystone.py index df308cf973..5e02c66b2b 100644 --- a/openstack_dashboard/api/keystone.py +++ b/openstack_dashboard/api/keystone.py @@ -41,7 +41,7 @@ from openstack_dashboard import policy LOG = logging.getLogger(__name__) DEFAULT_ROLE = None DEFAULT_DOMAIN = getattr(settings, 'OPENSTACK_KEYSTONE_DEFAULT_DOMAIN', - 'default') + 'Default') # Set up our data structure for managing Identity API versions, and @@ -281,11 +281,14 @@ def get_default_domain(request, get_name=True): def get_effective_domain_id(request): - """Gets the id of the default domain to use when creating Identity objects. - If the requests default domain is the same as DEFAULT_DOMAIN, return None. + """Gets the id of the default domain to use when creating Identity + objects. If the requests default domain is the same as DEFAULT_DOMAIN, + return None. """ - domain_id = get_default_domain(request).get('id') - return None if domain_id == DEFAULT_DOMAIN else domain_id + default_domain = get_default_domain(request) + domain_id = default_domain.get('id') + domain_name = default_domain.get('name') + return None if domain_name == DEFAULT_DOMAIN else domain_id def is_cloud_admin(request): diff --git a/openstack_dashboard/dashboards/identity/domains/tests.py b/openstack_dashboard/dashboards/identity/domains/tests.py index 7974715a2f..96524ac16a 100644 --- a/openstack_dashboard/dashboards/identity/domains/tests.py +++ b/openstack_dashboard/dashboards/identity/domains/tests.py @@ -157,7 +157,7 @@ class DomainsViewTests(test.BaseAdminViewTests): @test.create_stubs({api.keystone: ('domain_get', 'domain_list', )}) def test_set_clear_domain_context(self): - domain = self.domains.get(id="1") + domain = self.domains.get(id="3") api.keystone.domain_get(IgnoreArg(), domain.id).AndReturn(domain) api.keystone.domain_get(IgnoreArg(), domain.id).AndReturn(domain) @@ -171,7 +171,7 @@ class DomainsViewTests(test.BaseAdminViewTests): self.assertTemplateUsed(res, constants.DOMAINS_INDEX_VIEW_TEMPLATE) self.assertItemsEqual(res.context['table'].data, [domain, ]) - self.assertContains(res, "test_domain:") + self.assertContains(res, "another_test_domain:") formData = {'action': 'domains__clear_domain_context__%s' % domain.id} res = self.client.post(DOMAINS_INDEX_URL, formData) @@ -179,6 +179,7 @@ class DomainsViewTests(test.BaseAdminViewTests): self.assertTemplateUsed(res, constants.DOMAINS_INDEX_VIEW_TEMPLATE) self.assertItemsEqual(res.context['table'].data, self.domains.list()) self.assertNotContains(res, "test_domain:") + self.assertNotContains(res, "another_test_domain:") class CreateDomainWorkflowTests(test.BaseAdminViewTests): diff --git a/openstack_dashboard/dashboards/identity/groups/tests.py b/openstack_dashboard/dashboards/identity/groups/tests.py index 52ef4b1d7c..48d58931bc 100644 --- a/openstack_dashboard/dashboards/identity/groups/tests.py +++ b/openstack_dashboard/dashboards/identity/groups/tests.py @@ -152,7 +152,7 @@ class GroupsViewTests(test.BaseAdminViewTests): @test.create_stubs({api.keystone: ('group_create',)}) def test_create_with_domain(self): - domain = self.domains.get(id="1") + domain = self.domains.get(id="3") group = self.groups.get(id="1") self.setSessionValues(domain_context=domain.id, diff --git a/openstack_dashboard/local/local_settings.py.example b/openstack_dashboard/local/local_settings.py.example index 2c579aedb1..98c2dbff5e 100644 --- a/openstack_dashboard/local/local_settings.py.example +++ b/openstack_dashboard/local/local_settings.py.example @@ -67,11 +67,11 @@ WEBROOT = '/' # Overrides the default domain used when running on single-domain model # with Keystone V3. All entities will be created in the default domain. -# NOTE: This value must be the ID of the default domain, NOT the name. +# NOTE: This value must be the name of the default domain, NOT the ID. # Also, you will most likely have a value in the keystone policy file like this # "cloud_admin": "rule:admin_required and domain_id:" -# This value must match the domain id specified there. -#OPENSTACK_KEYSTONE_DEFAULT_DOMAIN = 'default' +# This value must be the name of the domain whose ID is specified there. +#OPENSTACK_KEYSTONE_DEFAULT_DOMAIN = 'Default' # Set this to True to enable panels that provide the ability for users to # manage Identity Providers (IdPs) and establish a set of rules to map diff --git a/openstack_dashboard/test/test_data/keystone_data.py b/openstack_dashboard/test/test_data/keystone_data.py index 82762e1f91..1005405720 100644 --- a/openstack_dashboard/test/test_data/keystone_data.py +++ b/openstack_dashboard/test/test_data/keystone_data.py @@ -157,9 +157,14 @@ def data(TEST): 'name': 'disabled_domain', 'description': "a disabled test domain.", 'enabled': False} + domain_dict_3 = {'id': "3", + 'name': 'another_test_domain', + 'description': "another test domain.", + 'enabled': True} domain = domains.Domain(domains.DomainManager, domain_dict) disabled_domain = domains.Domain(domains.DomainManager, domain_dict_2) - TEST.domains.add(domain, disabled_domain) + another_domain = domains.Domain(domains.DomainManager, domain_dict_3) + TEST.domains.add(domain, disabled_domain, another_domain) TEST.domain = domain # Your "current" domain user_dict = {'id': "1",