diff --git a/keystone/identity/backends/ldap/common.py b/keystone/identity/backends/ldap/common.py index be734960d4..058aec423c 100644 --- a/keystone/identity/backends/ldap/common.py +++ b/keystone/identity/backends/ldap/common.py @@ -1807,16 +1807,19 @@ 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): + def _id_to_member_attribute_value(self, object_id): + """Convert id to value expected by member_attribute.""" if self.group_members_are_ids: - dn = object_id - else: - dn = self._id_to_dn(object_id) + return object_id + return self._id_to_dn(object_id) + + def _is_id_enabled(self, object_id, conn): + member_attr_val = self._id_to_member_attribute_value(object_id) + return self._is_member_enabled(member_attr_val, conn) + + def _is_member_enabled(self, member_attr_val, conn): query = '(%s=%s)' % (self.member_attribute, - ldap.filter.escape_filter_chars(dn)) + ldap.filter.escape_filter_chars(member_attr_val)) try: enabled_value = conn.search_s(self.enabled_emulation_dn, ldap.SCOPE_BASE, @@ -1827,34 +1830,26 @@ 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) + member_attr_val = self._id_to_member_attribute_value(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): + if not self._is_member_enabled(member_attr_val, conn): modlist = [(ldap.MOD_ADD, self.member_attribute, - [dn])] + [member_attr_val])] try: conn.modify_s(self.enabled_emulation_dn, modlist) except ldap.NO_SUCH_OBJECT: attr_list = [('objectClass', [self.group_objectclass]), (self.member_attribute, - [dn]), + [member_attr_val]), 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) + member_attr_val = self._id_to_member_attribute_value(object_id) modlist = [(ldap.MOD_DELETE, self.member_attribute, - [dn])] + [member_attr_val])] with self.get_connection() as conn: try: conn.modify_s(self.enabled_emulation_dn, modlist) @@ -1879,7 +1874,7 @@ class EnabledEmuMixIn(BaseLdap): ref = super(EnabledEmuMixIn, self).get(object_id, ldap_filter) if ('enabled' not in self.attribute_ignore and self.enabled_emulation): - ref['enabled'] = self._get_enabled(object_id, conn) + ref['enabled'] = self._is_id_enabled(object_id, conn) return ref def get_all(self, ldap_filter=None, hints=None): @@ -1891,7 +1886,7 @@ class EnabledEmuMixIn(BaseLdap): if x[0] != self.enabled_emulation_dn] with self.get_connection() as conn: for obj_ref in obj_list: - obj_ref['enabled'] = self._get_enabled( + obj_ref['enabled'] = self._is_id_enabled( obj_ref['id'], conn) return obj_list else: diff --git a/keystone/tests/unit/test_backend_ldap.py b/keystone/tests/unit/test_backend_ldap.py index 920e34c11a..560c5da252 100644 --- a/keystone/tests/unit/test_backend_ldap.py +++ b/keystone/tests/unit/test_backend_ldap.py @@ -2224,7 +2224,7 @@ class LDAPIdentityEnabledEmulation(LDAPIdentity, unit.TestCase): # Override the tree_dn, it's used to build the enabled member filter mixin_impl.tree_dn = sample_dn - # The filter that _get_enabled is going to build contains the + # The filter, which _is_id_enabled is going to build, contains the # tree_dn, which better be escaped in this case. exp_filter = '(%s=%s=%s,%s)' % ( mixin_impl.member_attribute, mixin_impl.id_attr, object_id, @@ -2233,7 +2233,7 @@ class LDAPIdentityEnabledEmulation(LDAPIdentity, unit.TestCase): with mixin_impl.get_connection() as conn: m = self.useFixture( fixtures.MockPatchObject(conn, 'search_s')).mock - mixin_impl._get_enabled(object_id, conn) + mixin_impl._is_id_enabled(object_id, conn) # The 3rd argument is the DN. self.assertEqual(exp_filter, m.call_args[0][2])