From 3aaeadf895e96598acc64d54d8dd55e2b616e7ad Mon Sep 17 00:00:00 2001 From: Mitya_Eremeev Date: Thu, 13 May 2021 21:29:27 +0300 Subject: [PATCH] Default role checker should be case-insensitive. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keystone role names are case-insensistive and Horizon should handle role names in a case-insensitive manner. For example, when keystone bootstraps default roles, it creates “admin”, “member”, and “reader”. If another role, “Member” (note the upper case ‘M’) is created, keystone will return a 409 Conflict since it considers the name “Member” equivalent to “member”. Note that case is preserved in this event. https://docs.openstack.org/keystone/latest/admin/case-insensitive.html#roles Also whatever is written in defaults can be overridden in settings by the operator - especially these days when actually the default should be 'member' (one of the default roles created by Keystone during the bootstrap), not _member_ which is there for legacy reasons I presume. Change-Id: Ibfb80a47a8aaed8f33e4e1dcfb428e70c829f0dd --- openstack_dashboard/api/keystone.py | 4 ++-- .../test/unit/api/test_keystone.py | 22 +++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/openstack_dashboard/api/keystone.py b/openstack_dashboard/api/keystone.py index 6195ee79c7..38931e52da 100644 --- a/openstack_dashboard/api/keystone.py +++ b/openstack_dashboard/api/keystone.py @@ -781,7 +781,7 @@ def get_default_role(request): to request. Supports lookup by name or id. """ global DEFAULT_ROLE - default = settings.OPENSTACK_KEYSTONE_DEFAULT_ROLE + default = settings.OPENSTACK_KEYSTONE_DEFAULT_ROLE.lower() if default and DEFAULT_ROLE is None: try: roles = keystoneclient(request, admin=True).roles.list() @@ -789,7 +789,7 @@ def get_default_role(request): roles = [] exceptions.handle(request) for role in roles: - if default in (role.id, role.name): + if default in (role.id.lower(), role.name.lower()): DEFAULT_ROLE = role break return DEFAULT_ROLE diff --git a/openstack_dashboard/test/unit/api/test_keystone.py b/openstack_dashboard/test/unit/api/test_keystone.py index 82221b7570..4598c2d504 100644 --- a/openstack_dashboard/test/unit/api/test_keystone.py +++ b/openstack_dashboard/test/unit/api/test_keystone.py @@ -15,6 +15,7 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +from random import choice from unittest import mock @@ -68,6 +69,27 @@ class RoleAPITests(test.APIMockTestCase): role = api.keystone.get_default_role(self.request) keystoneclient.roles.list.assert_called_once_with() + @mock.patch.object(api.keystone, 'keystoneclient') + @mock.patch.object(api.keystone, 'DEFAULT_ROLE', new=None) + def test_get_case_insensitive_default_role(self, mock_keystoneclient): + def convert_to_random_case(role_): + new_name = list() + for char in list(role_.name): + new_name.append(getattr(char, choice(["lower", "upper"]))()) + role_.name = role_._info["name"] = "".join(new_name) + return role_ + keystoneclient = mock_keystoneclient.return_value + keystoneclient.roles.list.return_value = list() + for role in self.roles: + role = convert_to_random_case(role) + keystoneclient.roles.list.return_value.append(role) + role = api.keystone.get_default_role(self.request) + self.assertEqual(self.role.name.lower(), role.name.lower()) + # Verify that a second call doesn't hit the API again, + # so we use assert_called_once_with() here. + api.keystone.get_default_role(self.request) + keystoneclient.roles.list.assert_called_once_with() + class ServiceAPITests(test.APIMockTestCase): @override_settings(OPENSTACK_ENDPOINT_TYPE='internalURL')