From c7fae97d873f72068ca65538ec5b5919c0ac7d5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Piliszek?= Date: Tue, 6 Aug 2019 13:25:17 +0200 Subject: [PATCH] Honor group_members_are_ids for user_enabled_emulation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applied when group config is to be honored (i.e. set user_enabled_emulation_use_group_config). Conditionals follow usage of group_members_are_ids. Added new test for the case with ids. It fails without fix. The original test expanded to ensure the change did not break its internals either. It passes without fix as well. Additionally some TODOs are added for observed potential issues. Change-Id: I7874a70e6109219baee80309c3a27f8af9905a6d Closes-Bug: #1839133 Signed-off-by: Radosław Piliszek --- keystone/identity/backends/ldap/common.py | 25 ++++++++-- keystone/tests/unit/test_backend_ldap.py | 46 +++++++++++++++++++ .../notes/bug-1839133-24570c9fbacb530d.yaml | 5 ++ 3 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/bug-1839133-24570c9fbacb530d.yaml diff --git a/keystone/identity/backends/ldap/common.py b/keystone/identity/backends/ldap/common.py index b9becea749..91e6f6e2d9 100644 --- a/keystone/identity/backends/ldap/common.py +++ b/keystone/identity/backends/ldap/common.py @@ -1781,6 +1781,7 @@ class EnabledEmuMixIn(BaseLdap): DEFAULT_GROUP_OBJECTCLASS = 'groupOfNames' DEFAULT_MEMBER_ATTRIBUTE = 'member' + DEFAULT_GROUP_MEMBERS_ARE_IDS = False def __init__(self, conf): super(EnabledEmuMixIn, self).__init__(conf) @@ -1797,9 +1798,11 @@ class EnabledEmuMixIn(BaseLdap): if not self.use_group_config: self.member_attribute = self.DEFAULT_MEMBER_ATTRIBUTE self.group_objectclass = self.DEFAULT_GROUP_OBJECTCLASS + self.group_members_are_ids = self.DEFAULT_GROUP_MEMBERS_ARE_IDS else: self.member_attribute = conf.ldap.group_member_attribute self.group_objectclass = conf.ldap.group_objectclass + self.group_members_are_ids = conf.ldap.group_members_are_ids if not self.enabled_emulation_dn: naming_attr_name = 'cn' @@ -1815,8 +1818,13 @@ class EnabledEmuMixIn(BaseLdap): naming_rdn[1]) self.enabled_emulation_naming_attr = naming_attr + # TODO(yoctozepto): methods below use _id_to_dn which requests another LDAP connection - optimize it + def _get_enabled(self, object_id, conn): - dn = self._id_to_dn(object_id) + if self.group_members_are_ids: + dn = object_id + else: + dn = self._id_to_dn(object_id) query = '(%s=%s)' % (self.member_attribute, ldap.filter.escape_filter_chars(dn)) try: @@ -1829,24 +1837,33 @@ class EnabledEmuMixIn(BaseLdap): return bool(enabled_value) def _add_enabled(self, object_id): + if self.group_members_are_ids: + dn = object_id + else: + dn = self._id_to_dn(object_id) with self.get_connection() as conn: + # TODO(yoctozepto): _get_enabled potentially calls _id_to_dn 2nd time - optimize it if not self._get_enabled(object_id, conn): modlist = [(ldap.MOD_ADD, self.member_attribute, - [self._id_to_dn(object_id)])] + [dn])] try: conn.modify_s(self.enabled_emulation_dn, modlist) except ldap.NO_SUCH_OBJECT: attr_list = [('objectClass', [self.group_objectclass]), (self.member_attribute, - [self._id_to_dn(object_id)]), + [dn]), self.enabled_emulation_naming_attr] conn.add_s(self.enabled_emulation_dn, attr_list) def _remove_enabled(self, object_id): + if self.group_members_are_ids: + dn = object_id + else: + dn = self._id_to_dn(object_id) modlist = [(ldap.MOD_DELETE, self.member_attribute, - [self._id_to_dn(object_id)])] + [dn])] with self.get_connection() as conn: try: conn.modify_s(self.enabled_emulation_dn, modlist) diff --git a/keystone/tests/unit/test_backend_ldap.py b/keystone/tests/unit/test_backend_ldap.py index aa7a50747f..9ac84deb38 100644 --- a/keystone/tests/unit/test_backend_ldap.py +++ b/keystone/tests/unit/test_backend_ldap.py @@ -2046,9 +2046,17 @@ class LDAPIdentityEnabledEmulation(LDAPIdentity, unit.TestCase): "Enabled emulation conflicts with enabled mask") def test_user_enabled_use_group_config(self): + # Establish enabled-emulation group name to later query its members + group_name = 'enabled_users' + driver = PROVIDERS.identity_api._select_identity_driver( + CONF.identity.default_domain_id) + group_dn = 'cn=%s,%s' % (group_name, driver.group.tree_dn) + self.config_fixture.config( group='ldap', user_enabled_emulation_use_group_config=True, + user_enabled_emulation_dn=group_dn, + group_name_attribute='cn', group_member_attribute='uniqueMember', group_objectclass='groupOfUniqueNames') self.ldapdb.clear() @@ -2064,6 +2072,44 @@ class LDAPIdentityEnabledEmulation(LDAPIdentity, unit.TestCase): user_ref = PROVIDERS.identity_api.get_user(user_ref['id']) self.assertIs(True, user_ref['enabled']) + # Ensure state matches the group config + group_ref = PROVIDERS.identity_api.get_group_by_name(group_name, + CONF.identity.default_domain_id) + PROVIDERS.identity_api.check_user_in_group(user_ref['id'], group_ref['id']) + + def test_user_enabled_use_group_config_with_ids(self): + # Establish enabled-emulation group name to later query its members + group_name = 'enabled_users' + driver = PROVIDERS.identity_api._select_identity_driver( + CONF.identity.default_domain_id) + group_dn = 'cn=%s,%s' % (group_name, driver.group.tree_dn) + + self.config_fixture.config( + group='ldap', + user_enabled_emulation_use_group_config=True, + user_enabled_emulation_dn=group_dn, + group_name_attribute='cn', + group_member_attribute='memberUid', + group_members_are_ids=True, + group_objectclass='posixGroup') + self.ldapdb.clear() + self.load_backends() + + # Create a user and ensure they are enabled. + user1 = unit.new_user_ref(enabled=True, + domain_id=CONF.identity.default_domain_id) + user_ref = PROVIDERS.identity_api.create_user(user1) + self.assertIs(True, user_ref['enabled']) + + # Get a user and ensure they are enabled. + user_ref = PROVIDERS.identity_api.get_user(user_ref['id']) + self.assertIs(True, user_ref['enabled']) + + # Ensure state matches the group config + group_ref = PROVIDERS.identity_api.get_group_by_name(group_name, + CONF.identity.default_domain_id) + PROVIDERS.identity_api.check_user_in_group(user_ref['id'], group_ref['id']) + def test_user_enabled_invert(self): self.config_fixture.config(group='ldap', user_enabled_invert=True, user_enabled_default='False') diff --git a/releasenotes/notes/bug-1839133-24570c9fbacb530d.yaml b/releasenotes/notes/bug-1839133-24570c9fbacb530d.yaml new file mode 100644 index 0000000000..b6ed1556de --- /dev/null +++ b/releasenotes/notes/bug-1839133-24570c9fbacb530d.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + [`bug 1839133 `_] + Makes user_enabled_emulation_use_group_config honor group_members_are_ids.