openstack
/
keystone
Code Issues Proposed changes

Merge "Refactor some ldap code to implement TODOs" into stable/train

This commit is contained in:
Zuul 2020-05-19 18:22:53 +00:00 committed by Gerrit Code Review
parent 7eeb144b7d 93f548a725
commit a50abb80ba
2 changed files with 21 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
keystone/identity/backends/ldap/common.py
View File

@ -1818,16 +1818,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,
@ -1838,34 +1841,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)
@ -1890,7 +1885,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):
@ -1902,7 +1897,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:

4
keystone/tests/unit/test_backend_ldap.py
View File

@ -2223,7 +2223,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,
@ -2232,7 +2232,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])